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

Web Cryptography API compliance wrt. key import/export #39959

Closed
panva opened this issue Aug 31, 2021 · 3 comments · Fixed by #42816
Closed

Web Cryptography API compliance wrt. key import/export #39959

panva opened this issue Aug 31, 2021 · 3 comments · Fixed by #42816
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. discuss Issues opened for discussions and feedbacks. webcrypto

Comments

@panva
Copy link
Member

panva commented Aug 31, 2021

Having gone through Web Cryptography API W3C Editor's Draft here are the discrepancies between the specification and Node.js v16.8.0 Web Crypto API Module actual implementation when it comes to SubtleCrypto.importKey, SubtleCrypto.exportKey, and in the case of RSA-PSS also SubtleCrypto.sign and SubtleCrypto.verify.

The point is to make a catalogue of known issues and open a discussion about how and if to handle them, knowing upfront that not everything can be achieved the possible resolution for some may just be a note in the documentation about the caveats, similar to what the Chromium Project > WebCrypto page lists.

RSASSA-PKCS1-v1_5 importKey "spki"

  • does not assert the Public Key type to be an RSA one, it is possible to import any public key
    • proposal: assert that keyObject.asymmetricKeyType is 'rsa' and throw DataError if it is not
  • does not allow for keys with OID sha1WithRSAEncryption, sha256WithRSAEncryption, sha384WithRSAEncryption, and sha512WithRSAEncryption
    • proposal: document this lack of support since OpenSSL does not support these specialized OIDs

RSASSA-PKCS1-v1_5 importKey "pkcs8"

  • does not assert the Private Key type to be an RSA one, it is possible to import any private key
    • proposal: assert that keyObject.asymmetricKeyType is 'rsa' and throw DataError if it is not
  • does not allow for keys with OID sha1WithRSAEncryption, sha256WithRSAEncryption, sha384WithRSAEncryption, and sha512WithRSAEncryption
    • proposal: document this lack of support since OpenSSL does not support these specialized OIDs

RSA-PSS importKey "spki"

  • does not assert the Public Key type to be an RSA or RSA-PSS one, it is possible to import any public key
    • proposal: assert that keyObject.asymmetricKeyType is 'rsa' or 'rsa-pss' and throw DataError if it is not
  • does not assert that rsa-pss key RSASSA-PSS-params sequence is present
    • proposal: when keyObject.asymmetricKeyType is 'rsa-pss' assert that keyObject.asymmetricKeyDetails.mgf1HashAlgorithm is not undefined
  • does not assert that rsa-pss key RSASSA-PSS-params sequence maskGenAlgorithm and hashAlgorithm match the values required for the given algorithm
    • proposal: when keyObject.asymmetricKeyType is 'rsa-pss' assert that keyObject.asymmetricKeyDetails.hashAlgorithm which in turn equals the Node.js normalized digest algorithm name for the WebCrypto algorithm specified hash.name

RSA-PSS exportKey "spki"

  • does not respect the requirement to export OID id-RSASSA-PSS keys. The OID and content of the key will always depend on the internal KeyObject representation. If it is an 'rsa' key, the result will use rsaEncryption.
  • does not respect the requirement to export OID id-RSASSA-PSS keys with RSASSA-PSS-params sequence with values depending on the WebCrypto algorithm of the CryptoKey. The behaviour will only be per-spec if the key imported was an RSA-PSS one (given we choose to fix the import steps), but when the imported key was an rsaEncryption one (allowed) this requirement will not be upheld.

RSA-PSS verify

  • fails the verify operation with a rejection when the internal rsa-pss KeyObject has keyObject.asymmetricKeyDetails.saltLength specified larger than the one requested to be used.
    • proposal: since the saltLength is not specified at key import but rather the verify operation, according to the spec, if errors are encountered the promise should be resolved with false.

RSA-PSS importKey "pkcs8"

  • does not assert the Private Key type to be an RSA or RSA-PSS one, it is possible to import any private key
    • proposal: assert that keyObject.asymmetricKeyType is 'rsa' or 'rsa-pss' and throw DataError if it is not
  • does not assert that rsa-pss key RSASSA-PSS-params sequence is present
    • proposal: when keyObject.asymmetricKeyType is 'rsa-pss' assert that keyObject.asymmetricKeyDetails.mgf1HashAlgorithm is not undefined
  • does not assert that rsa-pss key RSASSA-PSS-params sequence maskGenAlgorithm and hashAlgorithm match the values required for the given algorithm
    • proposal: when keyObject.asymmetricKeyType is 'rsa-pss' assert that keyObject.asymmetricKeyDetails.hashAlgorithm which in turn equals the Node.js normalized digest algorithm name for the WebCrypto algorithm specified hash.name

RSA-PSS exportKey "pkcs8"

  • does not respect the requirement to export OID id-RSASSA-PSS keys. The OID and content of the key will always depend on the internal KeyObject representation. If it is an 'rsa' key, the result will use rsaEncryption.
  • does not respect the requirement to export OID id-RSASSA-PSS keys with RSASSA-PSS-params sequence with values depending on the WebCrypto algorithm of the CryptoKey. The behaviour will only be per-spec if the key imported was an RSA-PSS one (given we choose to fix the import steps), but when the imported key was an rsaEncryption one (allowed) this requirement will not be upheld.

RSA-PSS sign

  • fails the sign operation with an openssl rejection when the internal rsa-pss KeyObject has keyObject.asymmetricKeyDetails.saltLength specified larger than the one requested to be used.
    • proposal: since the saltLength is not specified at key import but rather the sign operation, according to the spec, if errors are encountered the promise should be rejected with OperationError.

RSA-OAEP importKey "spki"

  • does not assert the Public Key type to be an RSA one, it is possible to import any public key
    • proposal: assert that keyObject.asymmetricKeyType is 'rsa' and throw DataError if it is not
  • does not support import of OID id-RSAES-OAEP keys
    • proposal: document this lack of support since OpenSSL does not support? this specialized OID

RSA-OAEP importKey "pkcs8"

  • does not assert the Private Key type to be an RSA or RSA-PSS one, it is possible to import any private key
    • proposal: assert that keyObject.asymmetricKeyType is 'rsa' or 'rsa-pss' and throw DataError if it is not
  • does not support import of OID id-RSAES-OAEP keys
    • proposal: document this lack of support since OpenSSL does not support? this specialized OID

RSA-OAEP exportKey "spki" and "pkcs8"

  • given the lack of support for OID id-RSAES-OAEP we will always export OID rsaEncryption
    • proposal: document this caveat since OpenSSL does not support? this specialized OID

ECDH exportKey "spki" and "pkcs8"

  • given the lack of support for OID id-ecDH we will always export OID id-ecPublicKey
    • proposal: document this caveat since OpenSSL does not support? this specialized OID

General

CryptoKey implementation is a rather thin layer on top of a KeyObject instance and that's where most of the import/export OID issues stem from, this is further elevated when 'node.keyObject' is used as the import key format.


NB: spki and pkcs8 OID mappings

@panva panva added crypto Issues and PRs related to the crypto subsystem. webcrypto labels Aug 31, 2021
@panva
Copy link
Member Author

panva commented Aug 31, 2021

There are a number of quick easy fixes that I'll work on - that is checking the key type upon import. See #39962

That's however about the only thing with a sure resolution from the above list.

When coming up with a resolution for these points it's important to keep in mind that we should strive for the possibility of generate/import/export or import/export/import round trips. We should not force export compliance with the standard if it means we won't be able to import the key material.

Likewise we have the other implementations of Web Cryptography API to interoperate with.

I did not test every browser vendor out there. But I did find a list of caveats from the Chromium project which, wrt. support key OIDs only chose to support (via import and export) the rsaEncryption OID for everything RSA based. That means they lack support for the specialized OIDs in the import step, and choose to ignore the specialized OID requirement in the export step. Likewise for ECDH it will only support id-ecPublicKey.

Firefox apparently also only supports rsaEncryption and likewise the upcoming Deno implementation.

@panva
Copy link
Member Author

panva commented Aug 31, 2021

I believe another reasonable fixes would be to the RSA-PSS import steps checking the RSASSA-PSS-params values matching the WebCrypto Algorithm. But that depends on which route we'll take with the OID support.

It is easy to say we only support import/export of rsaEncryption and id-ecPublicKey but it is harder to explain we'll support import of id-RSASSA-PSS but not its 100% spec compliant export.

@panva
Copy link
Member Author

panva commented Aug 31, 2021

cc @nodejs/crypto @tniessen @jasnell

@panva panva added confirmed-bug Issues with confirmed bugs. discuss Issues opened for discussions and feedbacks. labels Aug 31, 2021
nodejs-github-bot pushed a commit that referenced this issue May 25, 2022
bengl pushed a commit that referenced this issue May 30, 2022
targos pushed a commit that referenced this issue Jul 12, 2022
targos pushed a commit that referenced this issue Jul 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. discuss Issues opened for discussions and feedbacks. webcrypto
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant