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

Add ES* family #21

Closed
Keats opened this issue Oct 11, 2016 · 15 comments
Closed

Add ES* family #21

Keats opened this issue Oct 11, 2016 · 15 comments

Comments

@Keats
Copy link
Owner

Keats commented Oct 11, 2016

ES256, ES384 and ES512

@Keats Keats modified the milestone: v2 Oct 12, 2016
@pimeys
Copy link

pimeys commented Jan 25, 2017

Hey,

Have you been investigating this? We need it for Apple push notifications and are looking into ways of implementing them in Rust. We could fork and use some C bindings to get the job done, but I guess the changes would never get into the mainline here.

There are options to do the signing with:

Is there any progression or people who've been trying to do this with Rust?

@Keats
Copy link
Owner Author

Keats commented Jan 26, 2017

I didn't look into it yet but from a very quick research, it seems that Ring does support verifying ECDS https://briansmith.org/rustdoc/ring/?search=ECDSA but not signing.
briansmith/ring#207 seems to be tracking it

I'm not a crypto expert either so I definitely don't want to build it myself

@pimeys
Copy link

pimeys commented Jan 26, 2017

I asked about ECDSA in #rust-crypto yesterday and they gave me this: https://gitlab.com/ilari_l/btls/tree/master I'll investigate how and where should I use it and at least fork your project for basis. I don't know do you want to have this include in the HEAD, but we're in serious need for this with Apple notifications.

@briansmith
Copy link
Contributor

ECDSA signing with P-256 and P-384 is in ring 0.13.0-alpha4. See the comments in briansmith/ring#207 for details. Please try it out and let me know if you run into any issues before I release the 0.13.0 final version.

@Keats
Copy link
Owner Author

Keats commented Jun 1, 2018

Thanks for the notice!
I'll update the crate once the version is released with briansmith/ring#619 so upgrading ring is not a breaking change

@Keats Keats removed this from the v2 milestone Jul 26, 2018
@Keats Keats mentioned this issue Jul 27, 2018
Merged
@Keats
Copy link
Owner Author

Keats commented Aug 9, 2018

I don't have the bandwidth to work on that but I would welcome a PR

@manifest
Copy link

I'm willing to work on this issue, but there are currently some obstacles in ring API. I've described them in the related issue.

@manifest
Copy link

manifest commented Sep 3, 2018

@Keats Do you mind changing the type of the key from &[u8] to enum ?

enum Key {
    Bytes(Vec<u8>),
    ECDSAKeyPair(ring::signature::ECDSAKeyPair),
}

It will allow us to use ring's ECDSAKeyPair in sign function and thus add support of ES-* algorithms family today. It also looks rationale becase we may use any external library's key format this way.

@Keats
Copy link
Owner Author

Keats commented Sep 4, 2018

That would expose ring to end users which isn't really nice. I would rather wait for supports for signing from bytes to be implemented in ring

@manifest
Copy link

manifest commented Sep 4, 2018

Unfortunately there is no activity on that matter :-(

@jbg
Copy link
Contributor

jbg commented Jan 9, 2019

I've got ECDSA signing working in my fork, tested working for APNS JWT auth using my fork of apns2-rust. I didn't implement signature verification yet since I didn't need it, but can add that and submit a PR if there's interest.

Unfortunately, ECDSA signing requires ring 0.14, so the long-standing issue with linking multiple versions of ring into the same project might bite people here. For example, if you use this you can't also use rustls (which uses ring 0.13) in the same project.

@briansmith
Copy link
Contributor

Unfortunately, ECDSA signing requires ring 0.14, so the long-standing issue with linking multiple versions of ring into the same project might bite people here. For example, if you use this you can't also use rustls (which uses ring 0.13) in the same project.

I will update Rustls soon. I already have a PR for it.

@Keats
Copy link
Owner Author

Keats commented Jan 10, 2019

Oh I thought the versioning fix was meant to be merged in 0.13 :(
Would be nice to have a PR for ECDSA, there are quite a few breaking changes piling up.

@jbg
Copy link
Contributor

jbg commented Jan 11, 2019

OK, I'll try to implement verification this weekend and submit a pull request

@jbg
Copy link
Contributor

jbg commented Feb 1, 2019

Finally got around to this today: #73

@Keats Keats closed this as completed Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants