nodejs-github-bot · GitHub

@mscdex

@nodejs-github-bot nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

crypto

Issues and PRs related to the crypto subsystem.

labels

Feb 26, 2019

@mscdex

bnoordhuis

sam-github

bnoordhuis

tniessen

@panva panva mentioned this pull request

Mar 1, 2019

Closed

@mscdex

@mscdex

ryzokuken

@sam-github

It's hard to know if this catches all key types that are possible, I don't understand enough of the context this is used, or the OpenSSL APIs.

A check of the OpenSSL headers don't make it easy to see what EVP_PKEY_ types we should handle:

# define EVP_PKEY_NONE NID_undef
# define EVP_PKEY_RSA NID_rsaEncryption
# define EVP_PKEY_RSA2 NID_rsa
# define EVP_PKEY_RSA_PSS NID_rsassaPss
# define EVP_PKEY_DSA NID_dsa
# define EVP_PKEY_DSA1 NID_dsa_2
# define EVP_PKEY_DSA2 NID_dsaWithSHA
# define EVP_PKEY_DSA3 NID_dsaWithSHA1
# define EVP_PKEY_DSA4 NID_dsaWithSHA1_2
# define EVP_PKEY_DH NID_dhKeyAgreement
# define EVP_PKEY_DHX NID_dhpublicnumber
# define EVP_PKEY_EC NID_X9_62_id_ecPublicKey
# define EVP_PKEY_SM2 NID_sm2
# define EVP_PKEY_HMAC NID_hmac
# define EVP_PKEY_CMAC NID_cmac
# define EVP_PKEY_SCRYPT NID_id_scrypt
# define EVP_PKEY_TLS1_PRF NID_tls1_prf
# define EVP_PKEY_HKDF NID_hkdf
# define EVP_PKEY_POLY1305 NID_poly1305
# define EVP_PKEY_SIPHASH NID_siphash
# define EVP_PKEY_X25519 NID_X25519
# define EVP_PKEY_ED25519 NID_ED25519
# define EVP_PKEY_X448 NID_X448
# define EVP_PKEY_ED448 NID_ED448

I can only find one case statement handling EVP_PKEY_ED448, and it could be taken to imply we are missing a couple PKEY types that we should perhaps handle,EVP_PKEY_RSA_PSS, EVP_PKEY_DH, and the three Gost key types:

case EVP_PKEY_RSA:
ret = EVP_PK_RSA | EVP_PKT_SIGN;
/* if (!sign only extension) */
ret |= EVP_PKT_ENC;
break;
case EVP_PKEY_RSA_PSS:
ret = EVP_PK_RSA | EVP_PKT_SIGN;
break;
case EVP_PKEY_DSA:
ret = EVP_PK_DSA | EVP_PKT_SIGN;
break;
case EVP_PKEY_EC:
ret = EVP_PK_EC | EVP_PKT_SIGN | EVP_PKT_EXCH;
break;
case EVP_PKEY_ED448:
case EVP_PKEY_ED25519:
ret = EVP_PKT_SIGN;
break;
case EVP_PKEY_DH:
ret = EVP_PK_DH | EVP_PKT_EXCH;
break;
case NID_id_GostR3410_2001:
case NID_id_GostR3410_2012_256:
case NID_id_GostR3410_2012_512:
ret = EVP_PKT_EXCH | EVP_PKT_SIGN;
break;
default:

@mscdex Do you think its worth adding a couple more case statements to match X509_certificate_type(), or is that fn meant to handle a different situtation?

Its fine to leave that for a follow up PR if it takes more research. It is in my TODO list of things to look at, but that list is already large and growing faster than my free time, so no promises.

tniessen added a commit that referenced this pull request

Mar 18, 2019
PR-URL: #26554
Refs: #26319
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

This was referenced

Mar 19, 2019

Closed

Closed

targos pushed a commit to targos/node that referenced this pull request

Mar 27, 2019

@panva panva mentioned this pull request

Mar 28, 2019

Merged

targos pushed a commit that referenced this pull request

Mar 28, 2019
PR-URL: #26319
Fixes: #26316
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>

targos pushed a commit that referenced this pull request

Mar 28, 2019
PR-URL: #26554
Refs: #26319
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

Closed

16 tasks

This was referenced

Apr 23, 2019

Closed

Closed

Closed

Closed

Closed

Closed

Closed

Closed

Closed

Closed

Read the original on github.com ↗