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

ECDSA SigGen K-409 and K-233 R & S padding? #1037

Closed
mtdownz opened this issue Oct 8, 2020 · 10 comments
Closed

ECDSA SigGen K-409 and K-233 R & S padding? #1037

mtdownz opened this issue Oct 8, 2020 · 10 comments
Assignees

Comments

@mtdownz
Copy link

mtdownz commented Oct 8, 2020

Continuing this thread that was Closed:

#1034

In looking at this further we don't think what you are describing is occurring? The R & S components of an ECDSA signature are not field elements: They are reduced modulo the order of the curve, and the order of K-233 is encoded in 29 bytes rather than 30 bytes (which would be needed to encode a field element):
n = 0x80 00000000 00000000 00000000 00069d5b b915bcd4 6efb1ad5 f173abdf

Any feedback on this would be helpful?

Thanks!

@celic
Copy link
Collaborator

celic commented Oct 9, 2020

The order is 29 bytes. It is difficult to detect the padding is "extra" when it is very unlikely to have a value starting with 0x80. Even though 29 bytes is 232 bits, the name of the curve is a tad misleading (or derived from something else). Using a 233-bit order, the padding would bring it up to 240-bits or 30 bytes. This is what the server is doing behind the scenes... using the 233 value to pad up to the next byte boundary. This is unnecessary in this case and likely for K-409 due to the orders being one bit below the name would suggest.

@celic
Copy link
Collaborator

celic commented Oct 9, 2020

After looking at it a bit more, the order (n) for the curve is 232 bits, which is used as a modulo for the signature (r, s). The issue is that we pad to the next complete byte on 233-bits which is as large as the coordinates of a point on the curve may be. Normally the padding matches up between the curves, but these curves are special cases where a point on the curve may need more bits to be represented than the values within the signature.

@mtdownz
Copy link
Author

mtdownz commented Oct 14, 2020

Hi Chris, thank you again for your feedback. We will take it as this is the correct behavior and move forward.

@Kritner
Copy link
Contributor

Kritner commented Oct 14, 2020

@mtdownz we were going to make a correction on our side to not pad the r and s values as we were, they'll just be padded to the next byte of the bitlength of the orderN - so 232 and 408 bits would be the maximum length of either value.

@mtdownz
Copy link
Author

mtdownz commented Oct 14, 2020

Very good. Please let me know when you anticipate this to be corrected and I will inform our vendor so they can make the adjustment? They are looking to validate ECDSA with these curves early November. Thank you.

@Kritner
Copy link
Contributor

Kritner commented Oct 15, 2020

We should be able to get a prod release with the fix out prior to then. We'll first be making the change to our demo environment, which should be in the next few days.

Will this be something you'll be able to confirm the fix for on demo?

@Kritner
Copy link
Contributor

Kritner commented Oct 15, 2020

This change is on demo in release v1.1.0.13.

@Kritner
Copy link
Contributor

Kritner commented Oct 21, 2020

@mtdownz will you have a chance to confirm this change on the demo environment?

@mtdownz
Copy link
Author

mtdownz commented Oct 21, 2020

Hi Russ, I did indeed confirm that the changes were in place. Thank you for making the update. I have handed off the demo vectors to one of our vendors and if there appears to be any further issues I will let you know.

@Kritner
Copy link
Contributor

Kritner commented Oct 23, 2020

This change is now on prod in release https://github.com/usnistgov/ACVP-Server/releases/tag/v1.1.0.13

@Kritner Kritner closed this as completed Oct 23, 2020
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

No branches or pull requests

3 participants