Skip to content

Ensure DSA signatures are encoded according to spec#115

Merged
tarcieri merged 1 commit intoRustCrypto:masterfrom
nresare:openssh_dsa_signature_encoding
May 17, 2023
Merged

Ensure DSA signatures are encoded according to spec#115
tarcieri merged 1 commit intoRustCrypto:masterfrom
nresare:openssh_dsa_signature_encoding

Conversation

@nresare
Copy link
Contributor

@nresare nresare commented May 17, 2023

This change implements the raw encoding and decoding of DSA signatures according to RFC4253 Section 6.6 instead of relying on the internal representation in the dsa crate. See #114 for additional detail.

@nresare
Copy link
Contributor Author

nresare commented May 17, 2023

With this change in place I can successfully validate DSA signatures emitted by the upstream ssh-agent using https://github.com/nresare/ssh-agent-client-rs

@nresare nresare force-pushed the openssh_dsa_signature_encoding branch from 90fa201 to 62d9063 Compare May 17, 2023 15:34
@tarcieri
Copy link
Member

I was surprised at first this wasn't caught by a test, but I guess the only test ensures it can verify its own signatures:

https://github.com/RustCrypto/SSH/blob/master/ssh-key/tests/sshsig.rs#L121-L135

It'd probably be good to have a test it can verify an OpenSSH-generated DSA signature.

This change implements the raw encoding and decoding of DSA
signatures according to RFC4253 Section 6.6 instead of relying
on the internal representation in the dsa crate. See RustCrypto#114 for
additional detail.

This change also includes a test case, verifying that we
can verify the response from ssh-agent for a DSA signing request.
@nresare nresare force-pushed the openssh_dsa_signature_encoding branch from 62d9063 to ba8be05 Compare May 17, 2023 16:54
@nresare
Copy link
Contributor Author

nresare commented May 17, 2023

Thank you for the quick review. Please see the updated diff with a previously failing test case verifying a signature generated by ssh-agent

@tarcieri tarcieri merged commit 1fcdeef into RustCrypto:master May 17, 2023
@tarcieri
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants