minimalist RSA private JWKs cannot be imported

[daniel-ac-martin]

Describe the bug

When importing a private RSA JWK the library requires properties to be present that should be optional according to the spec. (See section 6.3.2 of https://www.rfc-editor.org/rfc/rfc7518.txt )

This is found at https://github.com/panva/jose/blob/master/lib/help/key_utils.js#L194 . When the property does not exist an error is triggered when the replace method is called on undefined.

Simply replacing these missing properties allows the import to complete but results in the follwing error when signing:

Error: PEM_read_bio_PrivateKey failed

To Reproduce

Steps to reproduce the behaviour:

1. Call JWK.importKey() on an RSA private key with only the mandatory properties in place. Such a key can be generated at https://mkjwk.org/

Expected behaviour
The key should be imported without error.

Environment:

• @panva/jose version: v1.0.2
• node version: v12.3.0

[panva]

The PEM representation of such key without the other primes cannot be used with OpenSSL and therefore by node as well, the only thing left to do is recompute them.

  Error {
    code: 'ERR_OSSL_ASN1_FIELD_MISSING',
    function: 'asn1_item_embed_d2i',
    library: 'asn1 encoding routines',
    opensslErrorStack: [
      'error:0907B00D:PEM routines:PEM_read_bio_PrivateKey:ASN1 lib',
      'error:0D08303A:asn1 encoding routines:asn1_template_noexp_d2i:nested asn1 error',
      'error:0D07803A:asn1 encoding routines:asn1_item_embed_d2i:nested asn1 error',
      'error:0D0680A8:asn1 encoding routines:asn1_check_tlen:wrong tag',
      'error:04093004:rsa routines:old_rsa_priv_decode:RSA lib',
    ],
    reason: 'field missing',
    message: 'error:0D078079:asn1 encoding routines:asn1_item_embed_d2i:field missing',
  }

However, the JWA spec as you've noted also says

The others enable optimizations and SHOULD be included by producers of JWKs representing RSA private keys.

And you can use e.g. https://github.com/MatthiasKunnen/jwk-rsa-compute-primes to compute the missing key parameters.

I will try to get the computation in using native BigInt but I'll actually think twice about shipping it enabled. The computation is expensive and i'd only like someone explicitly allowing it having to suffer the consequences. And if someone knows they'll be receiving incomplete not-optimized keys, they might as well require the linked package to recompute on-demand before passing it to @panva/jose.

WDYT @daniel-ac-martin?

[daniel-ac-martin]

Thanks for the background information @panva.

I think your approach makes sense. - An option on JWK.importKey to compute the missing parameters would probably be ideal.

That said, one more thing I should point out is that the error message when providing a 'minimalist' JWK is not very helpful but rather a generic "JWKImportFailed: import failed" message. This is due to the error being swallowed here: https://github.com/panva/jose/blob/master/lib/jwk/import.js#L55

From a user's point of view it would be nice to flag up what is wrong and to suggest supplying the option to compute the parameters or using a library prior to importing as you suggested.

Thanks for the help.

[panva]

Great feedback, very much inline with what i’m thinking. Expect updates soon.

[panva]

Without an option afterall, just with native BigInt

[panva]

I've deprecated the import functions in favour of new ones where this functionality is behind an off-by default option. When importing keys from untrusted sources this calculation can result in blocking the process and I would generally advise to run this only as a one-off to back-fill the missing parameters, never as part of any critical runtime.