WebCrypto reports unsupported JWK key type when exporting a key

SakerGT commented 3 years ago

Version

v16.7.0

Platform

Linux cloud 5.4.0-81-generic #91-Ubuntu SMP Thu Jul 15 19:09:17 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

WebCrypto

What steps will reproduce the bug?

This is a bit long winded, but here is what causes the issue for me.

Create a RSA-PSS certificate (e.g. using OpenSSL), 4096 bit in file pss-cert.pem (private key in separate file).

Read the certificate file and import the public key:

const cert = new X509Certificate(fs.readFileSync("pss-cert.pem"));
const publicKey = await subtle.importKey('node.keyObject', 
                        cert.publicKey, 
                        { hash: 'SHA-512', name: 'RSA-PSS'},
                        true,
                        ['verify']);

export the JWK

const jwk = await subtle.exportKey('jwk', publicKey);

Results in:

Error: Unsupported JWK Key Type.
    at exportKeyJWK (node:internal/crypto/webcrypto:325:40)
    at SubtleCrypto.exportKey (node:internal/crypto/webcrypto:394:24)
    at start (/usr/src/app/webcrypto.js:73:21) {
  code: 'ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE'
}

Works in v16.5.0 and earlier

How often does it reproduce? Is there a required condition?

Always reproducible

What is the expected behavior?

Should export a JWK with kty: 'RSA'

What do you see instead?

Error: Unsupported JWK Key Type.
    at exportKeyJWK (node:internal/crypto/webcrypto:325:40)
    at SubtleCrypto.exportKey (node:internal/crypto/webcrypto:394:24)
    at start (/usr/src/app/webcrypto.js:73:21) {
  code: 'ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE'
}

Additional information

No response

panva commented 3 years ago

@SakerGT can you provide the value of the certificate?

panva commented 3 years ago

The problem with rsa-pss key export to JWK is that we don't know if the key has any constraining parameters or not and that JWK format has no way of preserving them.

This is similar to why we don't allow rsa-pss private keys to be exported to pkcs1.

panva commented 3 years ago

cc @nodejs/crypto

SakerGT commented 3 years ago

@SakerGT can you provide the value of the certificate?

@panva Absolutely, sorry I should have included earlier. Here is the contents of the PEM file (github won't allow me to attach):

SakerGT commented 3 years ago

The problem with rsa-pss key export to JWK is that we don't know if the key has any constraining parameters or not and that JWK format has no way of preserving them.

This is similar to why we don't allow rsa-pss private keys to be exported to pkcs1.

Ah yes, I see what you mean. In my example, I have not set any constraints on the key.

As a last resort, could the full certificate be exported in x5c of JWK?

panva commented 3 years ago

Ah yes, I see what you mean. In my example, I have not set any constraints on the key.

The certificate does contain PSS parameters, for those JWK does not have a representation for. But even if the key had no PSS parameters we have no way of knowing - @tniessen am i right here? Is that why pkcs1 private rsa-pss key export is out right blocked as well? Because pkcs1 cannot represent those parameters?

As a last resort, could the full certificate be exported in x5c of JWK?

No, x5c cannot be in a JWK on its own, we would still need a way to represent the PSS parameters in a JWK which the standard simply does not have.

jasnell commented 3 years ago

Is that why pkcs1 private rsa-pss key export is out right blocked as well? Because pkcs1 cannot represent those parameters?

@tniessen would likely know better than I, but that's my understanding also.

SakerGT commented 3 years ago

Ah yes, I see what you mean. In my example, I have not set any constraints on the key.

The certificate does contain PSS parameters, for those JWK does not have a representation for. But even if the key had no PSS parameters we have no way of knowing - @tniessen am i right here? Is that why pkcs1 private rsa-pss key export is out right blocked as well? Because pkcs1 cannot represent those parameters?

Hmm, perhaps I have misunderstood what you mean by parameter restrictions. It is obviously necessary to have information that the certificate is PSS, but I didn't (at least intentionally) set any key restrictions per OpenSSL. My apologies if I've caused any confusion.

For RSA keys in JWK, aren't only kty, n and e mandatory? I understand the PSS parameters cannot currently be included as individual fields in the JWK. So (purely as a workaround) to represent the PSS parameters, wouldn't the addition of the certin optional x5c ensure that parameters are included?

As a last resort, could the full certificate be exported in x5c of JWK?

No, x5c cannot be in a JWK on its own, we would still need a way to represent the PSS parameters in a JWK which the standard simply does not have.

I wasn't suggesting only x5c, but rather adding x5c to the JWK, along with kty, n, e - I understand the first entry in x5c must contain the key specified in n and e ? By including the cert in x5c, the PSS parameters will be included by default?

I accept the standard simply does not have any other means to accommodate PSS parameters this today.

tniessen commented 3 years ago

Is that why pkcs1 private rsa-pss key export is out right blocked as well? Because pkcs1 cannot represent those parameters?

@tniessen would likely know better than I, but that's my understanding also.

It's part of the reason. The main reason is that RSA-PSS has the OID 1.2.840.113549.1.1.10, but PKCS#1 always decodes to the OID 1.2.840.113549.1.1.1. Unlike PKCS#8, PKCS#1 does not allow storing the OID. Applications should always use PKCS#8 (unless they absolutely need to support JWK).

SakerGT commented 3 years ago

Just to be clear, I don't want to export the private rsa-pss key to JWK. Only the public rsa-pss key contained in a certificate.

panva commented 3 years ago

A number of points I have

1) ending up with a CryptoKey as a result of an X509 Certificate key material import is only possible because of the node.keyObject node.js-specific import key format extension. Since CryptoKey is built as a layer on top of KeyObject and crypto.createPublicKey supports X509 cert values it seems like the certificate is the source of material but it is in fact just its subjectPublicKeyInfo that is used. 2) related to the previous point - when the above aforementioned X509 is imported as KeyObject and then re-exported to SPKI its id-RSASSA-PSS parameters are gone. 3) related to the previous point - when an actual SPKI Public Key is imported as KeyObject and then re-exported to SPKI its id-RSASSA-PSS parameters are still present. 4) to my knowledge (@jasnell please correct me if i'm wrong) - we do not perform validation when importing SPKI/PKCS8 format to CryptoKey that the PSS Parameters match the Algorithm values. 5) WebCrypto's mandatory alg export would make sure the parameters are actually persisted.

🐛 Because of 4) we rely on OpenSSL to throw if the PSS Parameters in the key don't match the SubtleCrypto Algorithm, but we don't do so when importing the key, it is done so for us during sign/verify.

🐛 ~Because of 1) and 2) when exporting a CryptoKey imported through X509 > node.KeyObject to spki - its OID will be id-RSASSA-PSS but despite webcrypto requiring it - PSS Parameters will be gone.~

🐛 We should allow RSA-PSS key export as JWK in WebCrypto but because of the lack of information that PSS Parameters are present and lack of Algorithm support on KeyObject instances we shouldn't do the same in keyObject.export({ format: 'jwk' })

🐛 ~@tniessen how come importing the X509 above strips the PSS Parameters when exporting to spki?~

I am not entirely sure how to proceed on these.

NB: Chromium chose to not support id-RSASSA-PSS - https://www.chromium.org/blink/webcrypto#TOC-Supported-key-formats - while citing it not being supported in OpenSSL/BoringSSL I would assume it's also because it is not possible to read the PSS Parameters without an ugly hack.

panva commented 3 years ago

Just to be clear, I don't want to export the private rsa-pss key to JWK. Only the public rsa-pss key contained in a certificate.

As noted above I've checked the specification and WebCryptoAPI is fine to export your key as JWK because the alg parameter carries the fixed allowed PSS parameters, altho (also as noted above) we do not perform the necessary validations that the pss parameters match the subtlecrypto algorithm.

I wasn't suggesting only x5c, but rather adding x5c to the JWK, along with kty, n, e - I understand the first entry in x5c must contain the key specified in n and e ? By including the cert in x5c, the PSS parameters will be included by default?

You're free to assign the x5c parameter after the export. But our KeyObject abstraction has no knowledge of the value of the certificate, the same applies to WebCrypto API where X.509 is not even supported.

SakerGT commented 3 years ago

As noted above I've checked the specification and WebCryptoAPI is fine to export your key as JWK because the alg parameter carries the fixed allowed PSS parameters, altho (also as noted above) we do not perform the necessary validations that the pss parameters match the subtlecrypto algorithm.

Cool, that's sort of what I'd thought from reading the specs.

You're free to assign the x5c parameter after the export. But our KeyObject abstraction has no knowledge of the value of the certificate, the same applies to WebCrypto API where X.509 is not even supported.

Understood and all good.

I appreciate you taking the time to follow up on this.

tniessen commented 3 years ago

@tniessen how come importing the X509 above strips the PSS Parameters when exporting to spki?

It's been a while since I worked on Node.js crypto internals, but that should not happen.

panva commented 3 years ago

@tniessen how come importing the X509 above strips the PSS Parameters when exporting to spki?

It's been a while since I worked on Node.js crypto internals, but that should not happen.

X.509 > createPublicKey > keyObject.export({ format: 'pem', type: 'spki' })

tniessen commented 3 years ago

@panva That looks correct to me.

panva commented 3 years ago

@panva That looks correct to me.

The lack of sha-512 and pkcs1-MGF in the SPKI doesn't to me, but i'll leave this be as it's not my forte.

I guess what it looks like to me is that certificate has constraints that the SPKI does not?

tniessen commented 3 years ago

Oh, sorry, I see the source of confusion. The certificate you posted only contains PSS parameters in the signature field, which is unrelated to subjectPublicKeyInfo, which contains the actual public key.

TBSCertificate  ::=  SEQUENCE  {
    version         [0]  Version DEFAULT v1,
    serialNumber         CertificateSerialNumber,
    signature            AlgorithmIdentifier{SIGNATURE-ALGORITHM,
                              {SignatureAlgorithms}},
    issuer               Name,
    validity             Validity,
    subject              Name,
    subjectPublicKeyInfo SubjectPublicKeyInfo,
    ... ,
    [[2:               -- If present, version MUST be v2
    issuerUniqueID  [1]  IMPLICIT UniqueIdentifier OPTIONAL,
    subjectUniqueID [2]  IMPLICIT UniqueIdentifier OPTIONAL
    ]],
    [[3:               -- If present, version MUST be v3 --
    extensions      [3]  Extensions{{CertExtensions}} OPTIONAL
    ]], ... }

You could have a certificate that is signed with EC but contains an RSA public key.

panva commented 3 years ago

Oh, sorry, I see the source of confusion. The certificate you posted only contains PSS parameters in the signature field, which is unrelated to subjectPublicKeyInfo, which contains the actual public key.

TBSCertificate  ::=  SEQUENCE  {
    version         [0]  Version DEFAULT v1,
    serialNumber         CertificateSerialNumber,
    signature            AlgorithmIdentifier{SIGNATURE-ALGORITHM,
                              {SignatureAlgorithms}},
    issuer               Name,
    validity             Validity,
    subject              Name,
    subjectPublicKeyInfo SubjectPublicKeyInfo,
    ... ,
    [[2:               -- If present, version MUST be v2
    issuerUniqueID  [1]  IMPLICIT UniqueIdentifier OPTIONAL,
    subjectUniqueID [2]  IMPLICIT UniqueIdentifier OPTIONAL
    ]],
    [[3:               -- If present, version MUST be v3 --
    extensions      [3]  Extensions{{CertExtensions}} OPTIONAL
    ]], ... }

You could have a certificate that is signed with EC but contains an RSA public key.

🙇 thank you for enlightening me

panva commented 2 years ago

This was fixed with #39828

nodejs / node