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 signing and verification (ES256, ES384) #73

Merged
merged 6 commits into from
Mar 22, 2019
Merged

Conversation

jbg
Copy link
Contributor

@jbg jbg commented Feb 1, 2019

Also updates Ring to 0.14 and updates the RSA signing code to match Ring API changes.

@jbg jbg mentioned this pull request Feb 1, 2019
@jbg
Copy link
Contributor Author

jbg commented Feb 2, 2019

I was emailed a comment on this pull request asking why I used ring's ECDSA_P256_SHA256_FIXED_SIGNING instead of ECDSA_P256_SHA256_ASN1_SIGNING, and indicating that it makes this incompatible with the keys generated by OpenSSL. However, the comment doesn't appear here. Perhaps it was deleted, but I'll answer it anyway in case GitHub has just eaten the comment somehow.

The relevant spec is JSON Web Algorithms Section 3.4 which specifies a simple concatenation of R and then S, which is what ECDSA_P256_SHA256_FIXED_SIGNING gives, as indicated in the ring docs.

So the short answer is, using ECDSA_P256_SHA256_ASN1_SIGNING would create invalid JWT signatures.

The question of what format the keys are provided in is pretty much orthogonal to how the signature is encoded. I chose to take them as PKCS#8 keys because that's what ring accepts and because that's what Apple gives as APNS keys, and being able to authenticate to APNS using JWT was my primary reason for implementing this in the first place.

@jbg
Copy link
Contributor Author

jbg commented Feb 2, 2019

It's also worth noting that OpenSSL provides tools for encoding keys in PKCS#8 structures, so it should not be a problem to use the OpenSSL tools to create suitable keys. I haven't tried, though.

@briansmith
Copy link
Contributor

It's also worth noting that OpenSSL provides tools for encoding keys in PKCS#8 structures, so it should not be a problem to use the OpenSSL tools to create suitable keys. I haven't tried, though.

I documented exactly how to generate PKCS#8 formatted keys using OpenSSL that interop with ring at at https://gist.github.com/briansmith/2ee42439923d8e65a266994d0f70180b.

@manifest
Copy link

manifest commented Feb 4, 2019

Actually, the public key generated with instructions above isn't in a format that ring expects.

openssl genpkey -algorithm EC -pkeyopt ec_paramgen_curve:P-256 -pkeyopt ec_param_enc:named_curve -out private_key.pem
openssl pkcs8 -in private_key.pem -topk8 -nocrypt -outform DER -out private_key.p8.der
openssl ec -in private_key.pem -pubout -out public_key.pem
openssl pkey -pubout -inform der -outform der -in private_key.p8.der -out public_key.p8.der

openssl version
OpenSSL 1.0.2q  20 Nov 2018

At the moment, the best solution I came with is to use openssl asn1parse. Another way is to use ASN1 parser to extract the public key in the application.

openssl ecparam -name prime256v1 -genkey -noout -out private_key.pem
openssl pkcs8 -in private_key.pem -topk8 -nocrypt -outform DER -out private_key.p8.der

openssl ec -in private_key.pem -pubout -out public_key.pem
openssl asn1parse -in public_key.pem -offset $((23 + 2)) -out public_key.p8.der.block
dd bs=1 skip=1 if=public_key.p8.der.block of=public_key.p8.der

At first, I missed that leading zero in asn1parse output, so I thought that the problem in the signing algorithm, but the example above just works with both ring's ECDSA_P256_SHA256_FIXED_SIGNING and ECDSA_P256_SHA256_ASN1_SIGNING, that is why I removed my comment, but probably it worth mentioning in case someone else will be confused in the similar way.

@gdamjan
Copy link

gdamjan commented Feb 4, 2019

is there any rust create that can convert a pem public key file to what ring expects?
I'm getting the key from AWS dynamically

@Keats Keats mentioned this pull request Feb 4, 2019
@manifest
Copy link

manifest commented Feb 4, 2019

@gdamjan you can implement it by yoursef using pem and yasna crates.

fn parse_pem(input: &[u8]) -> Result<Vec<u8>, Error> {
    use failure::SyncFailure;

    let pem_contents = pem::parse(input).map_err(SyncFailure::new)?.contents;
    let asn_contents = yasna::parse_der(&pem_contents, |reader| {
        reader.read_sequence(|reader| {
            reader.next().read_sequence(|reader| {
                reader.next().read_oid()?;
                reader.next().read_oid()?;
                Ok(())
            })?;
            let bytes = reader.next().read_bitvec()?;
            return Ok(bytes)
        })
    }).map_err(SyncFailure::new)?;

    Ok(asn_contents.to_bytes())
}

@briansmith
Copy link
Contributor

briansmith commented Feb 4, 2019

@gdamjan you can implement it by yoursef using pem and yasna crates.

It looks like it's SubjectPublicKeyInfo format, right? If so, we can move the SPKI logic from webpki into ring so that this can be done without yasna (but pem would still be needed). This is ring issue briansmith/ring#476.

@manifest
Copy link

manifest commented Feb 4, 2019

@briansmith I'm not sure about it, probably it's not

openssl asn1parse -in public_key.pem
    0:d=0  hl=2 l=  89 cons: SEQUENCE
    2:d=1  hl=2 l=  19 cons: SEQUENCE
    4:d=2  hl=2 l=   7 prim: OBJECT            :id-ecPublicKey
   13:d=2  hl=2 l=   8 prim: OBJECT            :prime256v1
23:d=1 hl=2 l= 66 prim: BIT STRING

@manifest
Copy link

manifest commented Feb 8, 2019

@Keats Is there something that need to be done to have this one merged?

@Keats
Copy link
Owner

Keats commented Feb 8, 2019

Mostly the discussion on #76

ES256,

/// ECDSA using SHA-384
ES384,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ring doesn't have ES512?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keats Keats merged commit da0f01a into Keats:next Mar 22, 2019
JadedBlueEyes referenced this pull request in JadedBlueEyes/jsonwebtoken Apr 13, 2023
ECDSA signing and verification (ES256, ES384)
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.

None yet

5 participants