-
Notifications
You must be signed in to change notification settings - Fork 111
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
Support Secp256r1 curve (NIST P-256) #81
base: master
Are you sure you want to change the base?
Conversation
[dependencies.ring] | ||
git = "https://github.com/KZen-networks/ring.git" | ||
branch = "feature/p256" | ||
optional = true | ||
|
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.
You probably explained it to me before, but why can't we take Ring from crate.io ?
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.
Because I forked and modified it (in order to extend the API of the keypair which is needed for this PR).
If we'll merge it to master (see ZenGo-X/ring#1) then we can publish KZen's repo with another name. How does that sound?
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.
can you give me a sense of what API extension were needed?
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.
Point arithmetics, serialization etc. (see PR)
src/elliptic/curves/secp256_r1.rs
Outdated
/// The size (in bytes) of a P-256 signature | ||
pub const SIGNATURE_SIZE: usize = 64; |
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.
what is P-256 signature ?
where do we use this const ?
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.
You're right, we don't we use it.
I'll push a commit to delete it.
BigInt::from(CURVE_ORDER.as_ref()) | ||
} | ||
|
||
fn add(&self, other: &Seed) -> Secp256r1Scalar { |
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.
Ring is not providing a way to do add/mul/sub/inv?
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.
Not for a field element.
I followed the same pattern as used in the Secp256k1 source, see https://github.com/KZen-networks/curv/blob/master/src/elliptic/curves/secp256_k1.rs#L160.
Hey @oleiba , Have you checked https://github.com/RustCrypto/elliptic-curves ? |
It looks like a very new library, first commit in Jan 2020, even after I started my work on this P256 integration. |
There is another issue there: RustCrypto/elliptic-curves#29 |
The traits in that library started off in the |
Note: ring's branch dependency can be changed to
master
once ZenGo-X/ring#1 is merged.