Add RSA key

[clehner]

Test vector generated with this implementation: spruceid/ssi#309 - modified to use a JWS2020 @context URL.

The payload is a DER-encoded RSAPublicKey (2048-bit). Edit: this does not follow existing usage (multiformats/multicodec#230 (comment)); PR is marked draft until this is resolved. Edit 2: that is resolved in multiformats/multicodec#233 but pending additional discussion (#41 (comment)). Edit 3: moving forward with this anyway.

Re: #33

cc @OR13 @bblfish @expede

[msporny]

I suggest that we don't support RSA in did:key. I know that might be controversial to say, but I don't think anyone is suggesting that RSA should be used in new systems being deployed. Current NIST/FIPS compatible replacements seem to be secp256r1 or ed25519.

We could make RSA support optional via MAY, but when we do that, we weaken interoperability.

Thoughts?

[dmitrizagidulin]

Thanks @clehner!
The PR looks reasonable (though I haven't looked too closely at the implications of using an RSA key for keyAgreement; I'm assuming that's legit.)

I agree with Manu, though -- do we want to do this at all? (As in, do you have pressing use cases for RSA did:keys?)

[clehner]

I don't think we have a pressing use case for it, honestly.

Also, I think a new multicodec entry may be needed, since the current one is apparently for SubjectPublicKeyInfo (multiformats/multicodec#230 (comment)), but the approach here uses RSAPublicKey.

I don't have an implementation actually using this keyAgreement. But I understand RSA keys can be used for key agreement in SSL: https://datatracker.ietf.org/doc/html/rfc6101#section-6.1.1

[expede]

I don't think we have a pressing use case for it, honestly.

Totally possible that you have a different use case, but a bit of context on "why RSA at all" in case it's useful 👇

The WebCrypto API only supports RSA and the ECDSA. If you have an in-browser application that wants to use non-exportable asymmetric keys, those are your options. ECDSA has some suspicious parameters, which makes folks nervous that it's been backdoored.

There's been some very recent movement towards including Curve25519 (and presumably EdDSA) in the WebCrypto API, but until then (probably months-to-years away), RSA is the larger but more widely trusted algorithm.

[OR13]

I strongly support covering 100% of JWK representations listed here:

https://www.iana.org/assignments/jose/jose.xhtml#web-key-types

There are many cases where RSA may be required, I've been hoping for something like this for a while now... thanks @clehner.

[OR13]

I would prefer to see examples start with RSA 2048 and end with RSA 4096, I expect folks to ROFL at the 4096 did document.

[clehner]

Added one for 4096. Look out for ROFLcopters!

I put it in a separate section because the ASCII prefix for the multicodec value is different depending on the length of the payload, apparently.

[dmitrizagidulin]

👍

[clehner]

@dmitrizagidulin thanks for approving.

It seems there are multiple implementers who want to be able to interop with RSA in did:key, as @b5 mentioned in multiformats/multicodec#233 (comment).

What @expede said in #41 (comment) makes sense to me.

This PR is aligned with the following PR in multicodec for RSA public key encoding: multiformats/multicodec#233

@msporny do you think it would be worth discussing optional support in a new issue? (Edit: or do you think it would be harmful to support RSA here at all?)

[dlongley]

I'd also like to avoid having to support RSA. I'd rather see efforts put into ed25519/x25519 support in the WebCrypto API sooner (there are already several implementations awaiting review in chromium: https://chromium-review.googlesource.com/q/ed25519 and mozilla has signaled support). My understanding is that the Web Application Security (webappsec) WG is undergoing rechartering right now (https://w3c.github.io/webappsec/admin/webappsec-charter-2021.html) that would enable them to take on WebCrypto work.

All that being said, I understand if we're just in a tough spot and implementers need to move forward. I can see a lot of did:key resolver libraries not wanting to take on the legacy code required to parse DER / ASN.1 to validate RSA keys ... so enabling that to be avoided should be a consideration as well.

[clehner]

@dlongley thanks for the feedback. Here is a proposal to use a more compact encoding: multiformats/multicodec#235

[b5]

Jumping in to ask that RSA remain required by the did:key spec.

I don't think anyone is suggesting that RSA should be used in new systems being deployed

If we could re-deploy all of our PKI today, sure I'd pick ED25519. We happen to have provisioned thousands of RSA keys over the years, want to use did:key, and would rather give our users a choice on when to update their keys, instead of being forced to do so.

But the suggestion of not doing the work of requiring RSA is missing a broader point: We're talking about cryptography here, and specs like did:key share some of the responsibility of making safer choices easy. Do we really want to design ourselves into a situation where we couldn't rotate to RSA keys in the event of an emergency because did:key doesn't support it?

[OR13]

related to #39

should RSA keys be allowed for keyAgreement?

https://github.com/Spomky-Labs/jose/blob/master/doc/operation/Encrypt.md#example

https://github.com/panva/jose/blob/HEAD/docs/classes/jwe_compact_encrypt.CompactEncrypt.md#class-compactencrypt

[clehner]

I've marked this PR as ready for review again, as it is in alignment with multicodec. There is a thread about using a more compact non-ASN.1 representation, here: multiformats/multicodec#235 but I don't see significant interest in using a representation other than the current one (ASN.1 RSAPublicKey). This representation has some overhead (~4% for 2048-bit key, compared to a more compact representation) but it is standardized and commonly supported. It's simple enough that I think implementers could produce/consume it by manipulating byte strings directly rather than needing a full ASN.1 implementation (I think you just need to know when to add the leading zero in the modulus, the rest should be straightforward).

[clehner]

About keyAgreement, I think it should be allowed - basically following the "encode all possible registered purposes" approach described in #39 (comment). My understanding is that this would not in necessarily imply any RSA-specific key agreement protocol (although there are some, e.g. https://link.springer.com/article/10.1007/BF02831832, https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.593.4857), but rather would mean that the key may sign and/or encrypt for the purpose of key agreement, as part of some protocol - for example, to establish a shared secret for an encrypted communication channel.

[msporny]

Do we really want to design ourselves into a situation where we couldn't rotate to RSA keys in the event of an emergency because did:key doesn't support it?

I don't understand why we would rotate backwards in time? Are we assuming some sort of future where secp256r1, secp256k1, and ed25519 are all simultaneously broken and we have no choice but to escape back to RSA? That sounds like tin foil hat territory to me. What am I missing?

With respect to supporting RSA -- if you can avoid the DER/ASN.1 encoding, then great... the key-sorted CWK representation seemed to be ok (except you're asking implementers to pull in a CBOR library there). @clehner, what encoding mechanism is the easiest to implement across many environments... we're probably looking for something public key format that openssl just spits out and that's supported by .Net, Java, Node.js, etc. (and the answer might be the ASN.1 encoding). Has anyone done a review of the space wrt. this question?

[OR13]

@msporny for RSA keys, using older key formats and extra libraries seems unavoidable... another reason not to use RSA, but IMO not a reason to avoid registering and refining it...

the multiformat registry does not define the key types very well either, it does not say if they are compressed or not, etc, leading prefixes such as 04 for secp256k1, etc... compared to those issues saying something uses ASN.1 is way better imo.

I do think RSA highlights some problems derived from relying on or inventing new key formats... IMO, building on existing standards such as ASN.1, JWK, CWK is better than inventing and registering new key types.

I've not implemented RSA for did:key yet, and I expect I might have complaints when I do, but we registered secp256r1 in uncompressed, then switched it to compressed, and I hope we are willing to improve RSA over time as folks add support for it as well.

[dlongley]

+1 for reusing existing key formats, however, if we're going with ASN.1 / DER, then instead of assuming everyone will be ok with importing bulky libraries, the spec should call out the specific DER bytes (prefixes, etc.) that can be used as (hopefully) constants. That would allow for a much simpler implementation without the dependency cruft.

[OR13]

+1 @dlongley I opened #42 so we can consider improvements related to RSA implementation details separately from the spec examples

[clehner]

@msporny

what encoding mechanism is the easiest to implement across many environments... we're probably looking for something public key format that openssl just spits out and that's supported by .Net, Java, Node.js, etc. (and the answer might be the ASN.1 encoding). Has anyone done a review of the space wrt. this question?

The current encoding (RSAPublicKey/DER) is sometimes called PKCS#1 format, I understand because it was recommended as the representation for RSA public keys in PKCS#1 (RFC 3447).

In Node.js, RSAPublicKey format is supported as "pkcs1" in Node.js's crypto module. The representation can be obtained using keyObject.export:

bytes = keyObject.export({format: "der", type: "pkcs1"})

and imported using createPublicKey:

crypto.createPublicKey({key: bytes, format: "der", type: "pkcs1"})

The other format supported by Node.js crypto, besides JWK, is called "spki", for SubjectPublicKeyInfo from RFC 5280. That is what multicodec used previously before changing to RsaPublicKey (multiformats/multicodec#233). spki is supported also in WebCrypto: https://w3c.github.io/webcrypto/#dom-keyformat-spki - SubjectPublicKeyInfo is not a RSA-specific representation but is a wrapper/container format, containing a bit string field subjectPublicKey, in which a RSAPublicKey is used, in this case. (Edit: I added this here: https://celehner.com/2021/11/rsapk/#spki)

SubjectPublicKeyInfo appears to be supported in Java, called "X.509" format: https://docs.oracle.com/javase/8/docs/api/java/security/Key.html#getFormat--
I don't see any additional formats mentioned about Java's RSA public key type - https://docs.oracle.com/javase/8/docs/api/java/security/interfaces/RSAPublicKey.html - so I assume only SubjectPublicKeyInfo (wrapping ASN.1 RSAPublicKey) is supported.

Go appears to support parsing both RSAPublicKey and SubjectPublicKeyInfo:
https://pkg.go.dev/crypto/x509#MarshalPKIXPublicKey
https://pkg.go.dev/crypto/x509#MarshalPKCS1PublicKey

I haven't checked .NET.

A RSAPublicKey/DER key can be converted to SubjectPublicKeyInfo/DER using the OpenSSL CLI:

openssl rsa -RSAPublicKey_in -in rsapk.der -inform DER -outform DER -out spki.der

[msporny]

If we are going to support RSA in did:key, this PR needs to get much more specific about what the RSA key encoding format is going to be in multicodec -- are we going with what's here, or something else? It seems like the conversation is still ongoing.

[expede]

It seems like the conversation is still ongoing

I just commented multiformats/multicodec#235 . We may have ended up in a situation where each group was waiting for the other. The general agreement in multicodec seems to be staying with ASN.1. If there's not strong disagreement on this, we can call it rough consensus and move forward.

[clehner]

Agreed with @expede, discussion seems to resolve in favor of using the currently proposed representation: DER-encoded RSAPublicKey from RFC 3447 (PKCS #1), multicodec code 0x1205.

@msporny do you think this PR should be updated to state the representation specifically? Opened #43 to discuss this more generally.

[msporny]

Please line wrap these key values. If you don't, the horizontal scrollbar for this spec is going to go on forever. We might want to add a CSS property for line-wrapping pre tags.

[clehner]

The pre blocks have CSS to limit their width and add an internal horizontal scrollbar, rather than causing a horizontal scrollbar on the whole page. So there shouldn't be horizontal scrollbar on the page introduced here, unless the browser doesn't support the CSS that limits the width of the code blocks. To cause line wrapping for such browsers (e.g. Dillo, w3m), I think it would require adding whitespace - which would break the ability to copy-paste the value, reducing functionality.

---

If you mean about the horizontal scrollbar in the code blocks, that would look like this currently:

P-521 has a horizontal scrollbar already. It has two example values:

With white-space: pre-wrap on the code or pre tag, RSA 2048 would look like this:

RSA 4096:

And P-521 would look like this:

[msporny]

With white-space: pre-wrap on the code or pre tag, RSA 2048 would look like this:

Yeah, let's do this for the time being -- can be done in a separate PR since I'm about to merge this one. As an aside, I've been wanting to add a re-spec extension to cleanly pre-wrap examples for a while now (and make a copy button that "unwraps/cleans" the example properly (remove comments, collapse multi-line into single line, etc.) when copied to the paste buffer so it's easy to paste it into other websites).

[msporny]

Agreed with @expede, discussion seems to resolve in favor of using the currently proposed representation: DER-encoded RSAPublicKey from RFC 3447 (PKCS #1), multicodec code 0x1205.

Ok, good, the did:key spec should be crystal clear about this. Please add this language to the did:key spec. Please detail the exact byte representation with no variation in order to make sure people can hard-code the ASN.1 values in their implementation instead of needing to bring in a full blown ASN.1 parser.

@msporny do you think this PR should be updated to state the representation specifically? Opened #43 to discuss this more generally.

Ideally yes, but I wouldn't object to that update being put in another PR to resolve #43. Let us know what path you want to take @clehner and then we can take appropriate action on this PR.

[clehner]

@msporny

Please detail the exact byte representation with no variation in order to make sure people can hard-code the ASN.1 values in their implementation instead of needing to bring in a full blown ASN.1 parser.

To fully detail the representation I think would require some (pseudo-)code, for encoding and decoding. This is because of the complexity in the variable length encoding. (I started to describe the encoding in #42 (comment)). If, however, the lengths are fixed to specific values (e.g. 2048-, 3072-, and 4096-bit modulus; and 3-byte exponent) I think the representation could be described using a simpler declarative style.
If this would be helpful, I could add the details in an appendix for these specific lengths. Later it could potentially be updated to allow for the more complex case of arbitrary lengths. I would prefer to use separate PRs if that would be okay. But here is what I started on: spruceid@a827986

[msporny]

I would prefer to use separate PRs if that would be okay.

Yep, that would be good. I'll merge this PR now.