RSA-OAEP & RSA-PSS errors different between spki and pkcs8 key importing

[lucacasonato]

For RSA-PSS the spec says the following for pkcs8 importing in the block that validates algorithm OID:

If params is not defined, or is not an instance of the RSASSA-PSS-params ASN.1 type defined in [RFC3447], throw a NotSupportedError.

For spki:

If params is not defined, or is not an instance of the RSASSA-PSS-params ASN.1 type defined in [RFC3447], throw a DataError.

One throws a NotSupportedError, the other a DataError for the same class of issue. This is also not tested in WPT (having it wrong doesn't make the tests fail). Found during some routine maintenance of Deno's Web Crypto implementation.

The same is the case for the same bit of code in RSA-OAEP's import key operation.

The question is: is this wrong? Shouldn't both return DataError?

[twiss]

Yep, I believe you're right, thanks for the report.

I tested some other implementations and most throw a DataError if params is null (though some accept the key without complaining). I'll make a PR to change the spec to that. It would be good to have some tests for this as well indeed.

Somewhat unrelated; while testing, I noticed that when exporting, all implementations I tested set the OID to rsaEncryption for both algorithms, rather than id-RSASSA-PSS and id-RSAES-OAEP respectively. Even though I think that's not ideal, maybe we should change the spec to match that as well, given such broad consensus.

[panva]

@twiss this is also the case for id-ecDH - i've only seen exports of ecPublicKey despite the current language.

The problem is underlying crypto library support for these three oids (id-RSASSA-PSS, id-RSASSA-OAEP, and id-ecDH). Given that the most widely used browser as well as server js runtime won't support these I wouldn't mind the spec reflecting these deviations from the spec that have, essentially, become the expected behaviour.

[lucacasonato]

Yeah, I'd be very much in favor of switching the OID the spec mandates. Deno also uses rsaEncryption for all three, and as far as I know this is what the WPT also tests.

[lucacasonato]

For tracking: we should add some form of WPT test for this behaviour (the error kind) before closing this issue. @twiss If you can provide the spki/pkcs8 keys you used for testing, I can go add the WPTs (unless you want to do it yourself) :-)

[lucacasonato]

I split out the rsaEncryption OID thing into the separate issue #300.

[twiss]

For tracking: we should add some form of WPT test for this behaviour (the error kind) before closing this issue. @twiss If you can provide the spki/pkcs8 keys you used for testing, I can go add the WPTs (unless you want to do it yourself) :-)

Before we test the error case, we should test the success case as well for these algorithm-specific OIDs (otherwise the easiest way to pass the tests would be to reject all keys with these OIDs 😅). I've created #307 for that, and will create some test keys (I have malformed ones but creating well-formed ones is a bit more tricky).

I'll close this issue as fixed by #299 :)