Skip to content
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

Fix transport encoding of nistp521 signatures #623

Merged
merged 2 commits into from
Jul 28, 2020

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Jul 28, 2020

SignatureECDSA.encode() does not correctly handle signatures longer
than 128 bytes, which affects signatures using the nistp521 curve.

This commits fixes the issue by replacing the ad-hoc ASN.1 DER
parsing with a use of ASN1InputStream.

SignatureECDSA.encode() does not correctly handle signatures longer
than 128 bytes, which affects signatures using the nistp521 curve.

This commits fixes the issue by replacing the ad-hoc ASN.1 DER
parsing with a use of ASN1InputStream.
@hierynomus
Copy link
Owner

Would be great to have a test here also :).

@fmeum
Copy link
Contributor Author

fmeum commented Jul 28, 2020

Would be great to have a test here also :).

The test will be added with #607. Unfortunately, due to #600, we could only add a test here if we were to add an ecdsa-sha2-nistp521 host key, which then would mean that the tests in #607 would no longer verify that #600 has been fixed. This is a confusing situation, but once #623 and #607 have been merged (in this order), both will be covered by passing tests.

@hierynomus hierynomus merged commit 4b1619d into hierynomus:master Jul 28, 2020
@fmeum fmeum deleted the ecdsa_521_fix branch July 28, 2020 10:05
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.

3 participants