-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
crypto: add support for SM2 #37066
base: main
Are you sure you want to change the base?
crypto: add support for SM2 #37066
Conversation
CI failures are expected due to #37055. |
@@ -2371,6 +2378,7 @@ changes: | |||
required only if the `format` is `'der'` and ignored if it is `'pem'`. | |||
* `passphrase`: {string | Buffer} The passphrase to use for decryption. | |||
* `encoding`: {string} The string encoding to use when `key` is a string. | |||
* `asymmetricKeyType` {string} The requested asymmetric key type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to document the expected values here (or link to where they are documented)
(I know there's a paragraph added below but listing the values here would be helpful)
@@ -3740,6 +3777,8 @@ additional properties can be passed: | |||
`crypto.constants.RSA_PSS_SALTLEN_DIGEST` sets the salt length to the digest | |||
size, `crypto.constants.RSA_PSS_SALTLEN_MAX_SIGN` (default) sets it to the | |||
maximum permissible value. | |||
* `sm2Identifier` {ArrayBuffer|Buffer|TypedArray|DataView} For SM2, this option |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to make this a more generic name, e.g. just identifier
perhaps.
else if (typeStr === 'x448') | ||
asymmetricKeyType = EVP_PKEY_X448; | ||
else | ||
throw new ERR_INVALID_ARG_VALUE('options.asymmetricKeyType', typeStr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A switch statement would be nicer here from a code readability point of view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest key-map object instead:
const keyTypeMap = {
'dh': EVP_PKEY_DH,
'dsa': EVP_PKEY_DSA,
'ec': EVP_PKEY_EC,
'ed25519': EVP_PKEY_ED25519,
'ed448': EVP_PKEY_ED448,
'rsa': EVP_PKEY_RSA,
'rsa-pss': EVP_PKEY_RSA_PSS,
'sm2': EVP_PKEY_SM2,
'x25519': EVP_PKEY_X25519,
'x448': EVP_PKEY_X448,
null: undefined // Map null to undefined
};
const asymmetricKeyType = keyTypeMap[typeStr];
if (asymmetricKeyType === undefined) { // if asymmetricKeyType null or not in keyTypeMap
throw new ERR_INVALID_ARG_VALUE('options.asymmetricKeyType', typeStr);
}
Where is SM2 used? |
Not sure. I am trying to address the remaining key types from #26996. |
As far as I know, SM2 is primarily used in China (it was developed by a Chinese agency). There's not a lot of literature available on it or where exactly it is used, if at all, outside of that region. It seems specialized enough that I'm not really convinced that there's justification for exposing it generally in core. |
I would vote to just leave it out for now and if there is a pressing need to add it in the future we can revisit it then, especially considering it requires some hacks to add support for it. |
@nodejs/crypto @jasnell @mscdex The switch to OpenSSL 3 has changed how Node.js treats EC keys on the SM2 curve, see #40106 (comment) and #38512 (comment). While OpenSSL now defaults to SM2 for such keys, I understand that there is no interest in this feature from this project. Given the status quo, which allows importing keys that Node.js does not support, I see the following options:
I can update this PR with whatever approach is chosen. |
Ping @nodejs/crypto |
Of the choices, I'd vote for option 3. |
These commits add support for SM2 from the SM9 standard. It requires a fair amount of hacks within our codebase, so if anyone has any other integration ideas, I'd be happy to hear them.
Asymmetric Key Type
'sm2'
A new asymmetric key type is added:
'sm2'
.Unlike previous asymmetric key types, it does not have a unique OID, and does, in fact, share an OID with
'ec'
, meaning that there is no longer a 1:1 correlation between OIDs and asymmetric key types.Key Pair Generation
SM2 key pairs can be generated using
crypto.generateKeyPair('sm2')
.No options are accepted.
Digital Signatures
SM2 signatures require an additional "identifier" that must be specified by the signing and the verifying party. To accomodate this, an
sm2Identifier
option was added tocrypto.sign
andcrypto.verify
.There is no documented way of producing SM2 signatures with the OpenSSL APIs that we are using in the
Sign
andVerify
classes. Therefore, these classes currently do not support SM2 signatures.Key Agreement
Currently not implemented due to lack of support within OpenSSL. Might also be insecure in its original form, see Xu et al. doi:10.1007/978-3-642-25513-7_12:
Should be added to
crypto.diffieHellman()
if it makes sense in the future.Key Import
Because the PKCS#8 and SPKI encodings of SM2 keys are indistinguishable from EC keys on the SM2 curve, the
createPublicKey
andcreatePrivateKey
functions now accept an additionalasymmetricKeyType
option. If specified, Node.js will attempt to create aKeyObject
of the given type. As a side effect, this allows users to enforce a specific asymmetric key type when importing key material, e.g.,'rsa'
. For SM2, this means that the key will first be parsed as EC, and its type will then be changed to SM2 if the user specifiedasymmetricKeyType: 'sm2'
.The default behavior matches OpenSSL: When loading a key with OID
1.2.840.10045.2.1
on the SM2 curve without specifyingasymmetricKeyType: 'sm2'
, it will create an EC key, not an SM2 key.Note on SM3
While OpenSSL 1.1.1 supports SM3 as a hash function and as the digest for SM2 signatures, it does not allow SM3 as the digest for other signatures. Future releases might change that.