-
Notifications
You must be signed in to change notification settings - Fork 694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ECDSA signature functions #518
Conversation
crypto/s2n_ecdsa.c
Outdated
uint8_t digest_out[MAX_DIGEST_LENGTH]; | ||
GUARD(s2n_hash_digest(digest, digest_out, digest_length)); | ||
|
||
// ECDSA_verify ignores the first parameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* */ for comments
"iwVevOxBJ1GL0usqhWNqOKoNp048H4rCmfyMN97E\n" | ||
"-----END EC PRIVATE KEY-----\n"; | ||
|
||
int main(int argc, char **argv) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good positive and negative tests. Do you think it's worth extending this to iterate across digest sizes: 0 -> MAX_DIGEST_LENGTH
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signature functions expects an s2n_hash_state parameter, which isn't easy to stuff bytes into. I opted to modify the test to attempt each hash algorithm so the signature function has to handle varied digest sizes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems reasonable and should cover all of our happy cases. Thanks for adding that!
Adding ECDSA functions to sign/verify, and parse keys. They are currently modeled after the equivalent RSA functions and use a mixture of the EVP and low-level OpenSSL APIs. Once the EVP refactoring in #478 has been completed for s2n_hash, these can switch over as well.