GNOME Bugzilla – Bug 790910
ssh-agent interface does not support SHA2 extension
Last modified: 2018-06-30 17:03:47 UTC
Created attachment 364516 [details] [review]
Support SHA2 extension for RSA signatures
OpenSSH since version 7.2 (Feb 2016)  is using SHA2 extension to provide RSA signatures using safe hash algorithm . This extension is not negotiated with the ssh-agents and so far it worked because of the bug in OpenSSH signature verification , but it will hopefully be soon fixed and at that time, RSA signatures provided be gnome-keyring will stop working (because it is ignoring the passed flags).
The attached patch adds a support to provide these signatures.
The testsuite is adjusted accordingly and passes. Also the behavior was verified against upstream OpenSSH with the patch attached to the OpenSSH bug.
Review of attachment 364516 [details] [review]:
Thank you, nice. I only have a stylistic comment about the gkd_ssh_agent_proto_algo_to_keytype function.
@@ +151,2 @@
+gkd_ssh_agent_proto_algo_to_keytype (gulong algo, GQuark curve_oid, GChecksumType halgo)
This interface is becoming a bit bloated; curve_oid is only relevant to ECC, and halgo is only relevant to RSA. It is not obvious how this function is supposed to work, by only looking at the caller.
I would suggest to either:
- expand this at the caller side, or
- split this function to:
This would increase the code complexity, but I guess it would be acceptable amount, given that there are only two places where the function is called.
@@ +682,3 @@
+ salgo = gkd_ssh_agent_proto_algo_to_keytype (algo, oid, 0);
Here, 0 == G_CHECKSUM_MD5. As I mentioned above, it's not obvious how gkd_ssh_agent_proto_algo_to_keytype() treats that value without seeing the function definition.
Created attachment 364557 [details] [review]
Support SHA2 extension for RSA signatures v2
Thank you for the review. It was a bit cumbersome.
I revised it and expanded the problematic parts little bit. And created separate functions for separate key types and moved the handling of them into keytype-specific places. It makes the code little bit longer, but hopefully more understandable.
For the gkd_ssh_agent_proto_write_public(), now I used directly SHA1 constant (default), rather than bogus 0, since this is not identification of signature algorithm, but simple raw key type of the same name.
I hope it is more clear now.
Review of attachment 364557 [details] [review]:
Thank you, looks fine.
(In reply to Jakub Jelen from comment #0)
> ... but it will hopefully be soon fixed and
> at that time, RSA signatures provided be gnome-keyring will stop working
> (because it is ignoring the passed flags).
Does that mean we should backport the patch to the stable branch?
I don't think it is needed. Next regular release should be fine, since the change in OpenSSH will not be earlier than in their next release (in 3 to 6 months I guess) and then there will be some time before distributions will update if they ever do.
OK, thanks for the confirmation. Pushed as: