From be4800d75a2c5685e7c3f505696c03f51ee7804b Mon Sep 17 00:00:00 2001 From: Samuel Chiang Date: Sat, 18 Jan 2025 01:30:53 +0000 Subject: [PATCH] Add support for PKCS12_set_mac --- crypto/pkcs8/pkcs12_test.cc | 97 +++++++++++++-------- crypto/pkcs8/pkcs8_x509.c | 163 ++++++++++++++++++++++++++++++------ include/openssl/pkcs8.h | 22 ++++- 3 files changed, 216 insertions(+), 66 deletions(-) diff --git a/crypto/pkcs8/pkcs12_test.cc b/crypto/pkcs8/pkcs12_test.cc index 2a78a9794c..024fb1cf86 100644 --- a/crypto/pkcs8/pkcs12_test.cc +++ b/crypto/pkcs8/pkcs12_test.cc @@ -533,7 +533,7 @@ static bssl::UniquePtr MakeTestCert(EVP_PKEY *key) { return x509; } -static bool PKCS12CreateVector(std::vector *out, EVP_PKEY *pkey, +static bool PKCS12CreateVector(bssl::UniquePtr *p12, EVP_PKEY *pkey, const std::vector &certs) { bssl::UniquePtr chain(sk_X509_new_null()); if (!chain) { @@ -546,35 +546,21 @@ static bool PKCS12CreateVector(std::vector *out, EVP_PKEY *pkey, } } - bssl::UniquePtr p12(PKCS12_create(kPassword, nullptr /* name */, pkey, - nullptr /* cert */, chain.get(), 0, - 0, 0, 0, 0)); - if (!p12) { + p12->reset(PKCS12_create(kPassword, nullptr /* name */, pkey, + nullptr /* cert */, chain.get(), 0, 0, 0, 0, 0)); + if (!p12->get()) { return false; } - - int len = i2d_PKCS12(p12.get(), nullptr); - if (len < 0) { - return false; - } - out->resize(static_cast(len)); - uint8_t *ptr = out->data(); - return i2d_PKCS12(p12.get(), &ptr) == len; + return true; } -static void ExpectPKCS12Parse(bssl::Span in, - EVP_PKEY *expect_key, X509 *expect_cert, +static void ExpectPKCS12Parse(PKCS12 *p12, EVP_PKEY *expect_key, + X509 *expect_cert, const std::vector &expect_ca_certs) { - bssl::UniquePtr bio(BIO_new_mem_buf(in.data(), in.size())); - ASSERT_TRUE(bio); - - bssl::UniquePtr p12(d2i_PKCS12_bio(bio.get(), nullptr)); - ASSERT_TRUE(p12); - EVP_PKEY *key = nullptr; X509 *cert = nullptr; STACK_OF(X509) *ca_certs = nullptr; - ASSERT_TRUE(PKCS12_parse(p12.get(), kPassword, &key, &cert, &ca_certs)); + ASSERT_TRUE(PKCS12_parse(p12, kPassword, &key, &cert, &ca_certs)); bssl::UniquePtr delete_key(key); bssl::UniquePtr delete_cert(cert); @@ -618,34 +604,37 @@ TEST(PKCS12Test, Order) { ASSERT_TRUE(cert3); // PKCS12_parse uses the key to select the main certificate. - std::vector p12; + bssl::UniquePtr p12; ASSERT_TRUE(PKCS12CreateVector(&p12, key1.get(), {cert1.get(), cert2.get(), cert3.get()})); - ExpectPKCS12Parse(p12, key1.get(), cert1.get(), {cert2.get(), cert3.get()}); + ExpectPKCS12Parse(p12.get(), key1.get(), cert1.get(), + {cert2.get(), cert3.get()}); ASSERT_TRUE(PKCS12CreateVector(&p12, key1.get(), {cert3.get(), cert1.get(), cert2.get()})); - ExpectPKCS12Parse(p12, key1.get(), cert1.get(), {cert3.get(), cert2.get()}); + ExpectPKCS12Parse(p12.get(), key1.get(), cert1.get(), + {cert3.get(), cert2.get()}); ASSERT_TRUE(PKCS12CreateVector(&p12, key1.get(), {cert2.get(), cert3.get(), cert1.get()})); - ExpectPKCS12Parse(p12, key1.get(), cert1.get(), {cert2.get(), cert3.get()}); + ExpectPKCS12Parse(p12.get(), key1.get(), cert1.get(), + {cert2.get(), cert3.get()}); // In case of duplicates, the last one is selected. (It is unlikely anything // depends on which is selected, but we match OpenSSL.) ASSERT_TRUE( PKCS12CreateVector(&p12, key1.get(), {cert1.get(), cert1b.get()})); - ExpectPKCS12Parse(p12, key1.get(), cert1b.get(), {cert1.get()}); + ExpectPKCS12Parse(p12.get(), key1.get(), cert1b.get(), {cert1.get()}); // If there is no key, all certificates are returned as "CA" certificates. ASSERT_TRUE(PKCS12CreateVector(&p12, nullptr, {cert1.get(), cert2.get(), cert3.get()})); - ExpectPKCS12Parse(p12, nullptr, nullptr, + ExpectPKCS12Parse(p12.get(), nullptr, nullptr, {cert1.get(), cert2.get(), cert3.get()}); // The same happens if there is a key, but it does not match any certificate. ASSERT_TRUE(PKCS12CreateVector(&p12, key1.get(), {cert2.get(), cert3.get()})); - ExpectPKCS12Parse(p12, key1.get(), nullptr, {cert2.get(), cert3.get()}); + ExpectPKCS12Parse(p12.get(), key1.get(), nullptr, {cert2.get(), cert3.get()}); } TEST(PKCS12Test, CreateWithAlias) { @@ -663,13 +652,8 @@ TEST(PKCS12Test, CreateWithAlias) { ASSERT_EQ(res, 1); std::vector certs = {cert1.get(), cert2.get()}; - std::vector der; - ASSERT_TRUE(PKCS12CreateVector(&der, key.get(), certs)); - - bssl::UniquePtr bio(BIO_new_mem_buf(der.data(), der.size())); - ASSERT_TRUE(bio); - bssl::UniquePtr p12(d2i_PKCS12_bio(bio.get(), nullptr)); - ASSERT_TRUE(p12); + bssl::UniquePtr p12; + ASSERT_TRUE(PKCS12CreateVector(&p12, key.get(), certs)); EVP_PKEY *parsed_key = nullptr; X509 *parsed_cert = nullptr; @@ -695,3 +679,44 @@ TEST(PKCS12Test, BasicAlloc) { bssl::UniquePtr p12(PKCS12_new()); ASSERT_TRUE(p12); } + +TEST(PKCS12Test, SetMac) { + bssl::UniquePtr key1 = MakeTestKey(); + ASSERT_TRUE(key1); + bssl::UniquePtr cert1 = MakeTestCert(key1.get()); + ASSERT_TRUE(cert1); + bssl::UniquePtr cert1b = MakeTestCert(key1.get()); + ASSERT_TRUE(cert1b); + bssl::UniquePtr key2 = MakeTestKey(); + ASSERT_TRUE(key2); + bssl::UniquePtr cert2 = MakeTestCert(key2.get()); + ASSERT_TRUE(cert2); + bssl::UniquePtr key3 = MakeTestKey(); + ASSERT_TRUE(key3); + bssl::UniquePtr cert3 = MakeTestCert(key3.get()); + ASSERT_TRUE(cert3); + + // PKCS12_parse uses the key to select the main certificate. + bssl::UniquePtr p12; + ASSERT_TRUE(PKCS12CreateVector(&p12, key1.get(), + {cert1.get(), cert2.get(), cert3.get()})); + EXPECT_TRUE(PKCS12_set_mac(p12.get(), kPassword, strlen(kPassword), nullptr, + 0, 0, nullptr)); + ExpectPKCS12Parse(p12.get(), key1.get(), cert1.get(), + {cert2.get(), cert3.get()}); + + ASSERT_TRUE(PKCS12CreateVector(&p12, key1.get(), + {cert3.get(), cert1.get(), cert2.get()})); + EXPECT_TRUE(PKCS12_set_mac(p12.get(), kPassword, strlen(kPassword), nullptr, + 0, 0, nullptr)); + ExpectPKCS12Parse(p12.get(), key1.get(), cert1.get(), + {cert3.get(), cert2.get()}); + + ASSERT_TRUE(PKCS12CreateVector(&p12, key1.get(), + {cert2.get(), cert3.get(), cert1.get()})); + EXPECT_TRUE(PKCS12_set_mac(p12.get(), kPassword, strlen(kPassword), nullptr, + 0, 0, nullptr)); + ExpectPKCS12Parse(p12.get(), key1.get(), cert1.get(), + {cert2.get(), cert3.get()}); +} + diff --git a/crypto/pkcs8/pkcs8_x509.c b/crypto/pkcs8/pkcs8_x509.c index 2e7148c45e..93ff8df459 100644 --- a/crypto/pkcs8/pkcs8_x509.c +++ b/crypto/pkcs8/pkcs8_x509.c @@ -1114,6 +1114,44 @@ static int add_encrypted_data(CBB *out, int pbe_nid, const char *password, return ret; } +static int pkcs12_gen_and_write_mac(CBB *out_pfx, const uint8_t *auth_safe_data, + size_t auth_safe_data_len, + const char *password, size_t password_len, + uint8_t *mac_salt, size_t salt_len, + int mac_iterations, const EVP_MD *md) { + int ret = 0; + uint8_t mac_key[EVP_MAX_MD_SIZE]; + uint8_t mac[EVP_MAX_MD_SIZE]; + unsigned mac_len; + if (!pkcs12_key_gen(password, password_len, mac_salt, salt_len, PKCS12_MAC_ID, + mac_iterations, EVP_MD_size(md), mac_key, md) || + !HMAC(md, mac_key, EVP_MD_size(md), auth_safe_data, auth_safe_data_len, + mac, &mac_len)) { + goto out; + } + + CBB mac_data, digest_info, mac_cbb, mac_salt_cbb; + if (!CBB_add_asn1(out_pfx, &mac_data, CBS_ASN1_SEQUENCE) || + !CBB_add_asn1(&mac_data, &digest_info, CBS_ASN1_SEQUENCE) || + !EVP_marshal_digest_algorithm(&digest_info, md) || + !CBB_add_asn1(&digest_info, &mac_cbb, CBS_ASN1_OCTETSTRING) || + !CBB_add_bytes(&mac_cbb, mac, mac_len) || + !CBB_add_asn1(&mac_data, &mac_salt_cbb, CBS_ASN1_OCTETSTRING) || + !CBB_add_bytes(&mac_salt_cbb, mac_salt, salt_len) || + // The iteration count has a DEFAULT of 1, but RFC 7292 says "The default + // is for historical reasons and its use is deprecated." Thus we + // explicitly encode the iteration count, though it is not valid DER. + !CBB_add_asn1_uint64(&mac_data, mac_iterations) || + !CBB_flush(out_pfx)) { + goto out; + } + ret = 1; + +out: + OPENSSL_cleanse(mac_key, sizeof(mac_key)); + return ret; +} + PKCS12 *PKCS12_create(const char *password, const char *name, const EVP_PKEY *pkey, X509 *cert, const STACK_OF(X509)* chain, int key_nid, int cert_nid, @@ -1194,9 +1232,7 @@ PKCS12 *PKCS12_create(const char *password, const char *name, PKCS12 *ret = NULL; CBB cbb, pfx, auth_safe, auth_safe_oid, auth_safe_wrapper, auth_safe_data, content_infos; - uint8_t mac_key[EVP_MAX_MD_SIZE]; - if (!CBB_init(&cbb, 0) || - !CBB_add_asn1(&cbb, &pfx, CBS_ASN1_SEQUENCE) || + if (!CBB_init(&cbb, 0) || !CBB_add_asn1(&cbb, &pfx, CBS_ASN1_SEQUENCE) || !CBB_add_asn1_uint64(&pfx, 3) || // auth_safe is a data ContentInfo. !CBB_add_asn1(&pfx, &auth_safe, CBS_ASN1_SEQUENCE) || @@ -1301,30 +1337,11 @@ PKCS12 *PKCS12_create(const char *password, const char *name, // covers |auth_safe_data|. const EVP_MD *mac_md = EVP_sha1(); uint8_t mac_salt[PKCS5_SALT_LEN]; - uint8_t mac[EVP_MAX_MD_SIZE]; - unsigned mac_len; if (!CBB_flush(&auth_safe_data) || !RAND_bytes(mac_salt, sizeof(mac_salt)) || - !pkcs12_key_gen(password, password_len, mac_salt, sizeof(mac_salt), - PKCS12_MAC_ID, mac_iterations, EVP_MD_size(mac_md), - mac_key, mac_md) || - !HMAC(mac_md, mac_key, EVP_MD_size(mac_md), CBB_data(&auth_safe_data), - CBB_len(&auth_safe_data), mac, &mac_len)) { - goto err; - } - - CBB mac_data, digest_info, mac_cbb, mac_salt_cbb; - if (!CBB_add_asn1(&pfx, &mac_data, CBS_ASN1_SEQUENCE) || - !CBB_add_asn1(&mac_data, &digest_info, CBS_ASN1_SEQUENCE) || - !EVP_marshal_digest_algorithm(&digest_info, mac_md) || - !CBB_add_asn1(&digest_info, &mac_cbb, CBS_ASN1_OCTETSTRING) || - !CBB_add_bytes(&mac_cbb, mac, mac_len) || - !CBB_add_asn1(&mac_data, &mac_salt_cbb, CBS_ASN1_OCTETSTRING) || - !CBB_add_bytes(&mac_salt_cbb, mac_salt, sizeof(mac_salt)) || - // The iteration count has a DEFAULT of 1, but RFC 7292 says "The default - // is for historical reasons and its use is deprecated." Thus we - // explicitly encode the iteration count, though it is not valid DER. - !CBB_add_asn1_uint64(&mac_data, mac_iterations)) { + !pkcs12_gen_and_write_mac( + &pfx, CBB_data(&auth_safe_data), CBB_len(&auth_safe_data), password, + password_len, mac_salt, sizeof(mac_salt), mac_iterations, mac_md)) { goto err; } @@ -1337,7 +1354,6 @@ PKCS12 *PKCS12_create(const char *password, const char *name, } err: - OPENSSL_cleanse(mac_key, sizeof(mac_key)); CBB_cleanup(&cbb); return ret; } @@ -1353,3 +1369,98 @@ void PKCS12_free(PKCS12 *p12) { OPENSSL_free(p12->ber_bytes); OPENSSL_free(p12); } + +int PKCS12_set_mac(PKCS12 *p12, const char *password, int password_len, + unsigned char *salt, int salt_len, int mac_iterations, + const EVP_MD *md) { + GUARD_PTR(p12); + int ret = 0; + + if (mac_iterations == 0) { + mac_iterations = 1; + } + if (salt_len == 0) { + salt_len = PKCS5_SALT_LEN; + } + // Generate |mac_salt| if |salt| is NULL and copy if NULL. + uint8_t *mac_salt = OPENSSL_malloc(salt_len); + if (salt == NULL) { + if (!RAND_bytes(mac_salt, salt_len)) { + goto out; + } + } else { + OPENSSL_memcpy(mac_salt, salt, salt_len); + } + // Match OpenSSL in using SHA-1 as the default hash function. + if (md == NULL) { + md = EVP_sha1(); + } + + uint8_t *storage = NULL; + CBS ber_bytes, in, pfx, authsafe, content_type, wrapped_authsafes, authsafes; + uint64_t version; + // The input may be in BER format. + CBS_init(&ber_bytes, p12->ber_bytes, p12->ber_len); + if (!CBS_asn1_ber_to_der(&ber_bytes, &in, &storage)) { + OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA); + goto out; + } + // There's no use case for |storage| anymore, so we free early. + OPENSSL_free(storage); + + if (!CBS_get_asn1(&in, &pfx, CBS_ASN1_SEQUENCE) || CBS_len(&in) != 0 || + !CBS_get_asn1_uint64(&pfx, &version)) { + OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA); + goto out; + } + if (version < 3) { + OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_VERSION); + goto out; + } + + if (!CBS_get_asn1(&pfx, &authsafe, CBS_ASN1_SEQUENCE)) { + OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA); + goto out; + } + // Save contents of |authsafe| to write back before the CBS is advanced. + const uint8_t *orig_authsafe = CBS_data(&authsafe); + size_t orig_authsafe_len = CBS_len(&authsafe); + + // Parse for |authsafes| which is the data that we should be running HMAC on. + if (!CBS_get_asn1(&authsafe, &content_type, CBS_ASN1_OBJECT) || + !CBS_get_asn1(&authsafe, &wrapped_authsafes, + CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0) || + !CBS_get_asn1(&wrapped_authsafes, &authsafes, CBS_ASN1_OCTETSTRING)) { + OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA); + goto out; + } + + // Rewrite contents of |p12| with the original contents and updated MAC. + CBB cbb, out_pfx, out_auth_safe; + if (!CBB_init(&cbb, 0) || !CBB_add_asn1(&cbb, &out_pfx, CBS_ASN1_SEQUENCE) || + !CBB_add_asn1_uint64(&out_pfx, version) || + !CBB_add_asn1(&out_pfx, &out_auth_safe, CBS_ASN1_SEQUENCE) || + !CBB_add_bytes(&out_auth_safe, orig_authsafe, orig_authsafe_len) || + !pkcs12_gen_and_write_mac(&out_pfx, CBS_data(&authsafes), + CBS_len(&authsafes), password, password_len, + mac_salt, salt_len, mac_iterations, md)) { + CBB_cleanup(&cbb); + goto out; + } + + // Verify that the new password is consistent with the original. This is + // behavior specific to AWS-LC. + OPENSSL_free(p12->ber_bytes); + if(!CBB_finish(&cbb, &p12->ber_bytes, &p12->ber_len) || + !PKCS12_verify_mac(p12, password, password_len)) { + CBB_cleanup(&cbb); + goto out; + } + + ret = 1; + +out: + OPENSSL_free(mac_salt); + return ret; +} + diff --git a/include/openssl/pkcs8.h b/include/openssl/pkcs8.h index a6fd1c4cfe..96ff8340a4 100644 --- a/include/openssl/pkcs8.h +++ b/include/openssl/pkcs8.h @@ -125,8 +125,8 @@ OPENSSL_EXPORT EVP_PKEY *PKCS8_parse_encrypted_private_key(CBS *cbs, // Any friendlyName attributes (RFC 2985) in the PKCS#12 structure will be // returned on the |X509| objects as aliases. See also |X509_alias_get0|. OPENSSL_EXPORT int PKCS12_get_key_and_certs(EVP_PKEY **out_key, - STACK_OF(X509) *out_certs, - CBS *in, const char *password); + STACK_OF(X509) *out_certs, CBS *in, + const char *password); // Deprecated functions. @@ -149,10 +149,10 @@ OPENSSL_EXPORT PKCS12 *d2i_PKCS12(PKCS12 **out_p12, const uint8_t **ber_bytes, size_t ber_len); // d2i_PKCS12_bio acts like |d2i_PKCS12| but reads from a |BIO|. -OPENSSL_EXPORT PKCS12* d2i_PKCS12_bio(BIO *bio, PKCS12 **out_p12); +OPENSSL_EXPORT PKCS12 *d2i_PKCS12_bio(BIO *bio, PKCS12 **out_p12); // d2i_PKCS12_fp acts like |d2i_PKCS12| but reads from a |FILE|. -OPENSSL_EXPORT PKCS12* d2i_PKCS12_fp(FILE *fp, PKCS12 **out_p12); +OPENSSL_EXPORT PKCS12 *d2i_PKCS12_fp(FILE *fp, PKCS12 **out_p12); // i2d_PKCS12 is a dummy function which copies the contents of |p12|. If |out| // is not NULL then the result is written to |*out| and |*out| is advanced just @@ -188,6 +188,20 @@ OPENSSL_EXPORT int PKCS12_parse(const PKCS12 *p12, const char *password, EVP_PKEY **out_pkey, X509 **out_cert, STACK_OF(X509) **out_ca_certs); +// PKCS12_set_mac generates the MAC for |p12| with the designated |password|, +// |salt|, |mac_iterations|, and |md| specified. |password| MUST be the same +// password originally used to encrypt |p12|. Although OpenSSL will allow an +// invalid state with a different |password|, AWS-LC will throw an error and +// return 0. +// +// If |salt| is NULL, a random salt of |salt_len| bytes is generated. If +// |salt_len| is zero, a default salt length is used instead. +// If |md| is NULL, the default is use SHA1 to align with OpenSSL. +OPENSSL_EXPORT int PKCS12_set_mac(PKCS12 *p12, const char *password, + int password_len, unsigned char *salt, + int salt_len, int mac_iterations, + const EVP_MD *md); + // PKCS12_verify_mac returns one if |password| is a valid password for |p12| // and zero otherwise. Since |PKCS12_parse| doesn't take a length parameter, // it's not actually possible to use a non-NUL-terminated password to actually