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

Implement signature verification #232

Merged
merged 4 commits into from
Jan 12, 2024
Merged

Implement signature verification #232

merged 4 commits into from
Jan 12, 2024

Conversation

MarshalX
Copy link
Owner

@MarshalX MarshalX commented Jan 11, 2024

2 tests are failing for now:
image

@MarshalX
Copy link
Owner Author

hi @DavidBuchanan314 @snarfed could you pls help me with fixing 2 tests? we should deny non-low-S signature somehow

@DavidBuchanan314
Copy link

DavidBuchanan314 commented Jan 11, 2024

Your _encode_signature method already extracts the r and s values, you just need to compare s against N//2, where N is the group order (I thiiiiiink, I'm slightly rusty...), and throw an exception or something if it doesn't. Note that N is different for both p256 and k256 curves

@DavidBuchanan314
Copy link

DavidBuchanan314 commented Jan 11, 2024

This is where I implement low-s signing in picopds, but I didn't do verification yet https://github.com/DavidBuchanan314/picopds/blob/main/signing.py

Your logic would be similar, but rather than correcting the value of s, you'd raise an exception.

@DavidBuchanan314
Copy link

DavidBuchanan314 commented Jan 11, 2024

Also, for the record, I'm not 100% sure I got the check correct... it might be off-by-one, and I haven't written/found test cases to test the boundary cases. Like, I'm 99% sure, but I wouldn't give it that last 1% until it's been tested fully.

@MarshalX
Copy link
Owner Author

MarshalX commented Jan 11, 2024

@DavidBuchanan314 thank you! Now it passes tests. Could you pls explain where you get values of curve order?

Upd. understood. This this was helpful: https://stackoverflow.com/a/70811080/8032027

@DavidBuchanan314
Copy link

DavidBuchanan314 commented Jan 12, 2024

Great question, you can find P-256 given in NIST SP 800-186 https://csrc.nist.gov/pubs/sp/800/186/final https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-186.pdf section 3.2.1.3.

SEC 2 specifies both P-256 and K-256 (aka secp256r1, secp256k1) https://www.secg.org/sec2-v2.pdf

@MarshalX MarshalX merged commit 7cf829e into main Jan 12, 2024
25 checks passed
@MarshalX MarshalX deleted the verify-sign branch January 12, 2024 11:37
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