Support loading RSA/ECC/Ed25519 public keys from SubjectPublicKeyInfo DER

[marshallpierce]

When validating an RSA signature with signature::validate, the public_key parameter is intended to be the subjectPublicKey DER bitstring (which in RSA's case further encapsulates a sequence of the modulus and the public exponent). This makes sense for a low-level API, but users who are manipulating RSA keys loaded from disk will usually be interacting with the DER for a SubjectPublicKeyInfo, essentially an aggregation of an AlgorithmIdentifier with the aforementioned subjectPulicKey.

webpki already has code to parse this structure. I don't have the context on the structure or goals of either project to judge if it makes sense to move the various structs, etc, into this project, but if that isn't desirable, it seems like we could get away with something simpler. Since this is RSA-specific (though maybe ECDSA has its own equivalent problem?), we could just add something like the following to rsa::verification:

/// documentation explaining that you probably want to use this if you're working 
/// with a public key file that openssl generated
fn __strawman_name_spk_der_from_spki_der(spki_der: untrusted::input) 
        -> Result<Input, error::Unspecified> {
    // parse algorithm id
    // validate match with rsaEncryption OID from 
    //     https://tools.ietf.org/html/rfc3279#section-2.3.1
    // validate null params
    // return Input of just the remaining subjectPublicKey DER
}

This wouldn't need the SubjectPublicKeyInfo and AlgorithmIdentifier structs since it's not trying to be general purpose.

Also, is it necessary to mask errors by returning Unspecified in this case? I did that since it seems that most things follow that pattern, but my usage, at least, would be at app startup when reading keys from disk, which isn't particularly sensitive (or available to attackers over the network).

[briansmith]

Hi Marshall! Thanks for filing the feature request.

You are right in pointing out that there are two possible routes: Either just do something for RSA, or do something for all signature algorithms. You are right when you imply it would probably be easier to do something RSA-specific. If you want to do something RSA-specific then it would be best to add it to alongside verify_rsa, perhaps named verify_rsa_using_spki. Search the code for verify_rsa, to see how this is done.

It is OK to move the parse_spki_value function and everything it uses into ring. There's a chunk of code that takes the resultant SubjectPublicKeyInfo and uses it to verify a signature, which can also be brought into ring.

In the end, even if we start with just RSA keys, we'll eventually extend the support to all algorithms. Because the webpki code is already written to support all algorithms, it might actually be simpler to just add a generic ring::signature::verify_with_spki API analogous to ring::signature::verify but which takes an encoded SPKI instead of the encoded RSAPublicKey.

[briansmith]

Also, is it necessary to mask errors by returning Unspecified in this case? I did that since it seems that most things follow that pattern, but my usage, at least, would be at app startup when reading keys from disk, which isn't particularly sensitive (or available to attackers over the network).

I would suggest that we start out with just using error::Unspecified since that's the only error type we've ever used! But, I agree that in the case of signature verification it would be better to have at least slightly more detailed error reporting. Probably that can/should be done as a separate commit.

[marshallpierce]

I would prefer to support all algorithms if you're OK with the necessary API changes, and verify_with_spki seems like a practical way to do that. I'll take a stab at implementing this.

[briansmith]

Yes, that's what I prefer as well! Looking forward to the PR. Let me know if you need any help.

[marshallpierce]

1. Pulling over verify_signature that accepts a SignatureAlgorithm means pulling in all the static structs like pub static ECDSA_P256_SHA256: SignatureAlgorithm, which also means pulling in all the algorithms (const ECDSA_P256: AlgorithmIdentifier) and the associated data files. At that point, there isn't much left of webpki's signed_data.rs. I don't think that's necessarily a bad thing; I think having an abstraction to represent a signature algorithm makes sense, especially given that there's already a VerificationAlgorithm struct, but I wanted to make sure that's what you had in mind.

2. It seems like the SignatureAlgorithm and AlgorithmId concepts (and the related constants) are purely x509-related. Given that, it seems reasonable to put them in a signature::x509 module to avoid cluttering signature.rs. Does this feel advisable?

3. As expected, there's a bit of an impedance mismatch on error handling between the two projects. Pulling in parse_spki_value necessitated moving bit_string_with_no_unused_bits with it, which was also converted to use error::Unspecified. verify_signature also loses access to Error::UnsupportedSignatureAlgorithmForPublicKey. This means that from webpki's perspective it now cannot distinguish between a verification with an SPKI key that fails because the DER was bad, because the algorithm id didn't match the provided SignatureAlgorithm, or because the signature was just plain wrong. This also shows up in the tests in signed_data.rs, which can be copied over too: they depend on specific Error constants. Some (e.g. UnsupportedSignatureAlgorithm) are inapplicable outside of webpki, so those tests are probably ripe for deletion (at least in ring), but others do make more sense in this project. How would you like me to handle this? One commit that's just a straight copy of the source from webpki (which won't compile), and another that introduces an error enum that verify_signature will use? Or, would you prefer a copy that squashes error handling to only use Unspecified, and then another that restores more detailed errors? Or something else?

[briansmith]

It seems like the SignatureAlgorithm and AlgorithmId concepts (and the related constants) are purely x509-related. Given that, it seems reasonable to put them in a signature::x509 module to avoid cluttering signature.rs. Does this feel advisable?

I suggest signature::spki instead. In particular, the SPKI structure is used in contexts other than x.509.

How would you like me to handle this?

I suggest that you start out by just copy/pasting webpki::Error as signature::VerifyWithSPKIError, removing the unused enum labels. Then webpki can map the ring::signature::VerifyWithSPKIErrors to its own error type. That might not be the final solution, but it's a good and AFAICT straightforward starting place.

[marshallpierce]

Sounds good. Sorry for the hand-holding; I'm new to rust (and these projects) and I'd rather get your suggestions early than late. I like the spki name -- it's more specific to the contents. Just for my own learning, where else does SPKI get used? I don't have enough RFC tabs open yet, so maybe you can help... ;)

[briansmith]

like the spki name -- it's more specific to the contents. Just for my own learning, where else does SPKI get used?

For example, it is used in https://tools.ietf.org/html/rfc7250, which is specifically about using SubjectPublicKeyInfo without x.509. Also DANE/TLSA uses SubjectPublicKeyInfo specifically without x.509.

However, your question caused me to search through some RFCs and it does seem like all uses of SubjectPublicKeyInfo are for PKI. This makes me wonder: If SubjectPublicKeyInfo is only used for PKI stuff, why don't we just add the verify_using_raw_spki to the webpki crate?

I think you said before that you're working on some code for JWT or similar, which isn't PKIX stuff. However, JWT defines its own JSON-based public key format, right? So are public keys in SubjectPublicKeyInfo format really relevant for JWT? It seems like maybe you might only need to deal with them in a JWT implementation until you have the JSON-based format working. Or, you might want to have a utility in your JWT code for translating a SPKI-format public key to JWT format. But, it doesn't seem bad to rely on the webpki crate for that case.

WDYT?

[marshallpierce]

Hm, interesting. Maybe webpki really is a better home for this. I didn't even consider looking in webpki for this because in my head webpki was for validating x509 cert chains and all that, but that may be more of a branding/awareness problem than an actual architectural poor fit. webpki's signed_data::verify_signature may be all I need. Upon reflection it also feels weird to embed knowledge of SPKI in this lower-level library.

Just in case it helps, I'll go over the use cases I see for ring/webpki when processing JWS, JWE, and JWT, which, together with JWA and JWK are collectively known as JOSE. This suite of RFCs seems to indulge in way more functionality and complexity than is needed, and this suspicion is borne out by the fact that usage in the wild is of only the very simplest parts (as far as I can tell, anyway). So, while I'm certainly going to support the common cases, the extent to which I expend effort to support exotic things like fetching public keys via URI (which seems like a terrible idea anyway) greatly depends on whether or not webpki/ring/etc make it easy for me to hand off the relevant crypto bits.

• JWS RSA validation, the simple case: need to load a configured, static public key from the DER that people will have commonly available in a file (namely, SPKI, since that's what openssl emits when you ask it for a pub key) and verify a &[u8]. Most users will only be using one key that they configured at app startup. This is certainly satisfied by verify_using_raw_spki no matter what library hosts it.
• Same as the above, but with ECDSA. Looks like openssl likes to export this in SPKI as well.
• JWS signature creation for RSA and ECDSA, since those plus HMAC overs everything needed for JWS signatures.
• JWS jku and jwk headers (also used in JWE: This is where the goofy JSON serialization of keys resides. With these, I'll be starting from the serialized bytes for an EC key's x and y parameters separately. This part I'm not sure how to proceed on. In this case it's inconvenient that ring's signature::verify takes the encoded form. We could make helpers for the various types of keys that emits an Input for the encoded form of the key material, but it seems somewhat clunky to wrap bytes in DER only to unwrap them again.
• JWS x5u, x5c, x5t, and x5t#S256 headers (also used in JWK and JWE): these are all ways to get PEM-encoded x509 public key certs or cert chains. x5u is a url to a chain, but providing a signature along with "the url to get the public key to validate it with" seems pretty ripe for abuse. x5t is a lookup (of unspecified nature) via SHA-1, and x5c specifies the chain directly in the JSON. Anyway, I'd want to be able to feed an x.509 chain to webpki and validate it, and then sign with the validated public key.
• JWE encryption and decryption with keys loaded from convenient on-disk formats for the interesting algorithms in the encryption algorithm list. The key modes that look useful are:
  • Key Encryption: generate a symmetric key, use it to encrypt the content, and encrypt the key with an asymmetric key. This would ideally use RSA-OAEP and ECDH-ES. I see the modes for making ECDSA and RSA PKCS1.5 signatures, but I don't see how to simply do encryption (without the hash step). Am I missing something? Also, it looks like there isn't RSA OAEP support. PKCS1.5 is permitted in the spec, but with a frowny-face "Recommended-".
  • Direct Encryption: symmetric encryption used directly on the content with a shared secret key. AES-GCM should do the trick.

That's all I can think of. In summary, verification with SPKI public keys via webpki's signed_data::verify_signature should solve my verification needs, but there are some other things that I either don't know how to accomplish or aren't yet supported. I'm happy to move discussion on those to other new issues as needed to avoid going OT on this issue.

[briansmith]

@marshallpierce. Thanks for the detailed summary. I suggest we focus right now on just the decision about how to cope with SPKI.

Based on what you said, I think we should go ahead and move the SPKI support from webpki to ring. In particular, I remember now that WebCrypto also uses SPI, and that really has nothing to do with Web PKI stuff.

As for your other JOSE-related issues, most of them are already filed. For example, there's an open issue for ECDSA signing, and there's an open issue for supporting ECDSA signatures in the format that JOSE uses. For the rest, I recommend filing separate issues.

[marshallpierce]

OK, I'll proceed with moving it to ring as in https://github.com/briansmith/ring/issues/370#issuecomment-266169221.

[erickt]

Hello everyone. It seems like there hasn't been much progress on this since #378 trying to implement it was closed. Are you still receptive to adding this functionality to ring, or do you now think it'd be best to do this work outside of ring, such as the approach described in https://github.com/lawliet89/biscuit/issues/71?

[briansmith]

We should put it into ring.

[briansmith]

Somewhere I have a branch that moves the logic from webpki into ring to do this. If people have thoughts on what they'd like the API to look like, it would be good to see those ideas.

[Keats]

I think a free function to keep it simple would be optimal, unless you want it to be in on RsaKeyPair/Ecdsa structs for example but that is more constraining.

[Ralith]

I'd like to help move this work forwards. @briansmith, is your WIP branch published anywhere? What can I do without stepping on your toes there? If the main remaining question is what the API should look like, it might be good to get a rough draft up for people to have opinions about.

[Keats]

@briansmith is that still planned?

[anlumo]

I just spent a whole evening discovering this very same issue… 😩

I had to drill down openid ➡️ biscuit ➡️ ring ➡️ ring::io::der and use the debugger, because it's not documented anywhere.

[lawliet89]

@anlumo Sorry about the bad experience with biscuit. I mentioned this in the docs:

But I think that wasn't clear enough it was related to this issue. (Or I was wrong in the documentation, it's been a while and I forgot the specifics!) I have been keeping an eye out on this issue since I first ran into it.

[anlumo]

OpenSSL private keys are actually not compatible. What I had to do was to extract the inner part using

> openssl asn1parse -inform DER -in pubkey.der -offset 24 -out pubkey-extracted.der

The offset I got from parsing the DER format on this page. On offset 24, the inner SEQUENCE starts.

This extracted key no longer works with OpenSSL:

> openssl rsa -pubin -inform DER -in pubkey-extracted.der
unable to load Public Key
140497900831872:error:0D0680A8:asn1 encoding routines:asn1_check_tlen:wrong tag:../crypto/asn1/tasn_dec.c:1130:
140497900831872:error:0D07803A:asn1 encoding routines:asn1_item_embed_d2i:nested asn1 error:../crypto/asn1/tasn_dec.c:290:Type=X509_ALGOR
140497900831872:error:0D08303A:asn1 encoding routines:asn1_template_noexp_d2i:nested asn1 error:../crypto/asn1/tasn_dec.c:627:Field=algor, Type=X509_PUBKEY

[lawliet89]

I see. I got it working when I wrote the docs a few years ago. If you have the time, I'd appreciate if you could lend a hand and submit a patch for the documentation.