From d29d7a7ff22e8e3be1c8bbdb8edd3ab9c72ed021 Mon Sep 17 00:00:00 2001 From: Shane Lontis Date: Wed, 5 May 2021 16:58:37 +1000 Subject: [PATCH] Fix i2d_PKCS8PrivateKey_nid_bio() regression. This method ignores the nid and could end up saving out the private key unencrypted In earlier alpha releases OSSL_num_encoders() returned 0 for this test case, which then meant that the legacy path was run, and the key was then correctly encrypted. Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/15152) --- crypto/pem/pem_pk8.c | 8 +++++++- test/evp_extra_test2.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/crypto/pem/pem_pk8.c b/crypto/pem/pem_pk8.c index 86a66b586c..5e28907be3 100644 --- a/crypto/pem/pem_pk8.c +++ b/crypto/pem/pem_pk8.c @@ -93,7 +93,13 @@ static int do_pk8pkey(BIO *bp, const EVP_PKEY *x, int isder, int nid, } } - if (OSSL_ENCODER_CTX_get_num_encoders(ctx) != 0) { + /* + * NOTE: There is no attempt to do a EVP_CIPHER_fetch() using the nid, + * since the nid is a PBE algorithm which can't be fetched currently. + * (e.g. NID_pbe_WithSHA1And2_Key_TripleDES_CBC). Just use the legacy + * path if the NID is passed. + */ + if (nid == -1 && OSSL_ENCODER_CTX_get_num_encoders(ctx) != 0) { ret = 1; if (enc != NULL) { ret = 0; diff --git a/test/evp_extra_test2.c b/test/evp_extra_test2.c index 6d5303ab9d..2e5861c77f 100644 --- a/test/evp_extra_test2.c +++ b/test/evp_extra_test2.c @@ -290,6 +290,40 @@ done: return ret; } +#ifndef OPENSSL_NO_DES +static int test_pkcs8key_nid_bio(void) +{ + int ret; + const int nid = NID_pbe_WithSHA1And3_Key_TripleDES_CBC; + static const char pwd[] = "PASSWORD"; + EVP_PKEY *pkey = NULL, *pkey_dec = NULL; + BIO *in = NULL, *enc_bio = NULL; + char *enc_data = NULL; + long enc_datalen = 0; + OSSL_PROVIDER *provider = NULL; + + ret = TEST_ptr(provider = OSSL_PROVIDER_load(NULL, "default")) + && TEST_ptr(enc_bio = BIO_new(BIO_s_mem())) + && TEST_ptr(in = BIO_new_mem_buf(kExampleRSAKeyPKCS8, + sizeof(kExampleRSAKeyPKCS8))) + && TEST_ptr(pkey = d2i_PrivateKey_ex_bio(in, NULL, NULL, NULL)) + && TEST_int_eq(i2d_PKCS8PrivateKey_nid_bio(enc_bio, pkey, nid, + pwd, sizeof(pwd) - 1, + NULL, NULL), 1) + && TEST_int_gt(enc_datalen = BIO_get_mem_data(enc_bio, &enc_data), 0) + && TEST_ptr(pkey_dec = d2i_PKCS8PrivateKey_bio(enc_bio, NULL, NULL, + (void *)pwd)) + && TEST_true(EVP_PKEY_eq(pkey, pkey_dec)); + + EVP_PKEY_free(pkey_dec); + EVP_PKEY_free(pkey); + BIO_free(in); + BIO_free(enc_bio); + OSSL_PROVIDER_unload(provider); + return ret; +} +#endif /* OPENSSL_NO_DES */ + static int test_alternative_default(void) { OSSL_LIB_CTX *oldctx; @@ -727,6 +761,9 @@ int setup_tests(void) ADD_TEST(test_pkey_todata_null); ADD_TEST(test_pkey_export_null); ADD_TEST(test_pkey_export); +#ifndef OPENSSL_NO_DES + ADD_TEST(test_pkcs8key_nid_bio); +#endif return 1; }