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

Using JWK as encoding format for public key in crypto.generateKeyPairSync() throws an error #39205

Closed
johannesnydahl opened this issue Jun 30, 2021 · 7 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.

Comments

@johannesnydahl
Copy link

  • Version: v16.4.0
  • Platform: Microsoft Windows NT 10.0.18363.0 x64
  • Subsystem: Crypto

What steps will reproduce the bug?

main.js

const crypto = require("crypto");

const keys = crypto.generateKeyPairSync("ec", {
	namedCurve: "P-384",
	publicKeyEncoding: { type: "spki", format: "jwk" },
	privateKeyEncoding: { type: "pkcs8", format: "pem" }
});

console.log(keys.publicKey);

then just execute the command node main.js

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

Everytime. If I change the public key encoding format to "pem" or "der" it dosen't throw an error. It seems like "jwk" is the only broken format.

What is the expected behavior?

Should probably look something like the output below. I used the keyObject.export() to create this output because as mentioned above, crypto.generateKeyPairSync() dosen't work at all with "jwk".

Code to produce the expected output:

const crypto = require("crypto");

const keys = crypto.generateKeyPairSync("ec", {
	namedCurve: "P-384"
});

console.log(keys.publicKey.export({ format: "jwk" }));

Expected output:

{
  crv: 'P-384',
  kty: 'EC',
  x: 'BC-y_ZyH6rYMv_YAREfUainM1c9iwhHc9KEPPRD1u2zGJMuGV1LvEWh2igD3kAS5',
  y: 'LX7TofWICe4_nJZNBkS0rtqDCDoTp9_TuKHqKQHh1wDH3hvgSZjPWZN1TnrvbfR4'
}

What do you see instead?

node:internal/crypto/keys:268
  throw new ERR_INVALID_ARG_VALUE(optionName, formatStr);
  ^

TypeError [ERR_INVALID_ARG_VALUE]: The property 'options.publicKeyEncoding.format' is invalid. Received 'jwk'
    at new NodeError (node:internal/errors:363:5)
    at parseKeyFormat (node:internal/crypto/keys:268:9)
    at parseKeyFormatAndType (node:internal/crypto/keys:304:18)
    at parseKeyEncoding (node:internal/crypto/keys:332:7)
    at parsePublicKeyEncoding (node:internal/crypto/keys:371:10)
    at parseKeyEncoding (node:internal/crypto/keygen:125:9)
    at createJob (node:internal/crypto/keygen:161:42)
    at Object.generateKeyPairSync (node:internal/crypto/keygen:95:22)
    at Object.<anonymous> (C:\Users\johannes.nydahl\Desktop\Playground\main.js:3:21)
    at Module._compile (node:internal/modules/cjs/loader:1095:14) {
  code: 'ERR_INVALID_ARG_VALUE'
}

Additional information

Tested on both node version v16.4.0 and v15.9.0 (v15.9.0 was when keyObject.export() first started to support "jwk" as format.

@Ayase-252 Ayase-252 added the crypto Issues and PRs related to the crypto subsystem. label Jul 1, 2021
@tniessen
Copy link
Member

tniessen commented Jul 1, 2021

cc @nodejs/crypto (especially @panva @jasnell)

@tniessen
Copy link
Member

tniessen commented Jul 1, 2021

The combination { type: "spki", format: "jwk" } makes little sense. SPKI is a traditional cryptographic format, JWK is a new format that's incompatible with these older formats.

@johannesnydahl
Copy link
Author

@tniessen

My cryptographic skills are lacking so I just picked "spki" because it was the only type that private EC keys supported according to the documentation. But from what I've tested, not using the "type" option at all throws the same error.

@panva
Copy link
Member

panva commented Jul 2, 2021

🤦‍♂️ i forgot about this interaction

If a publicKeyEncoding or privateKeyEncoding was specified, this function behaves as if keyObject.export() had been called on its result.

A simple { format: 'jwk' } should work for the supported key types but currently doesn't.

@panva panva added the confirmed-bug Issues with confirmed bugs. label Jul 2, 2021
@himself65

This comment has been minimized.

@himself65 himself65 self-assigned this Jul 9, 2021
himself65 added a commit to himself65/node that referenced this issue Jul 9, 2021
himself65 added a commit to himself65/node that referenced this issue Jul 9, 2021
himself65 added a commit to himself65/node that referenced this issue Jul 9, 2021
himself65 added a commit to himself65/node that referenced this issue Jul 14, 2021
himself65 added a commit to himself65/node that referenced this issue Jul 14, 2021
himself65 added a commit to himself65/node that referenced this issue Jul 19, 2021
targos pushed a commit that referenced this issue Aug 2, 2021
Fixes: #39205

PR-URL: #39319
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@D4nte
Copy link

D4nte commented Jan 5, 2022

I believe the issue also exist on node 15 but was not fixed: https://github.com/D4nte/js-libp2p-crypto/runs/4438572378?check_suite_focus=true

Can the fix be backported to 15?

Cc @jasnell

@panva
Copy link
Member

panva commented Jan 5, 2022

@D4nte the fix was never meant to land on v15.x since it has reached End-Of-Life on 2021-06-01.

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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants