nodejs-github-bot
added
c++
labels
Feb 26, 2019
panva
mentioned this pull request
Closed
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, 2019This was referenced
Mar 19, 2019Closed
Closed
panva
mentioned this pull request
Merged
targos pushed a commit that referenced this pull request
Mar 28, 2019targos pushed a commit that referenced this pull request
Mar 28, 2019Closed
16 tasks
This was referenced
Apr 23, 2019Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed