search

MarshalX / atproto

The AT Protocol (🦋 Bluesky) SDK for Python 🐍
https://atproto.blue
MIT License
309 stars 33 forks source link

Implement signature verification #232

Closed MarshalX closed 9 months ago

MarshalX commented 9 months ago

2 tests are failing for now:

image
MarshalX commented 9 months ago

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

DavidBuchanan314 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

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