From cc696073cffe7978d489297fbdeac4c0030384aa Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 21 Dec 2023 18:40:17 -0500 Subject: [PATCH] Fix X509_PUBKEY_set0_param to clear the cached EVP_PKEY Change-Id: I661c1284ba23138074339aec712ee6ba86905518 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65048 Commit-Queue: David Benjamin Reviewed-by: Bob Beck --- crypto/x509/x509_test.cc | 21 +++++++++++++++++++++ crypto/x509/x_pubkey.c | 11 +++++++++++ 2 files changed, 32 insertions(+) diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index 8b9cd64f1..7d46a9d6e 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -6993,3 +6993,24 @@ TEST(X509Test, ParamInheritance) { EXPECT_EQ(X509_VERIFY_PARAM_get_depth(dest.get()), 10); } } + +TEST(X509Test, PublicKeyCache) { + bssl::UniquePtr key = PrivateKeyFromPEM(kP256Key); + ASSERT_TRUE(key); + + X509_PUBKEY *pub = nullptr; + ASSERT_TRUE(X509_PUBKEY_set(&pub, key.get())); + bssl::UniquePtr free_pub(pub); + + bssl::UniquePtr key2(X509_PUBKEY_get(pub)); + ASSERT_TRUE(key2); + EXPECT_EQ(1, EVP_PKEY_cmp(key.get(), key2.get())); + + // Replace |pub| with different (garbage) values. + ASSERT_TRUE(X509_PUBKEY_set0_param(pub, OBJ_nid2obj(NID_subject_alt_name), + V_ASN1_NULL, nullptr, nullptr, 0)); + + // The cached key should no longer be returned. + key2.reset(X509_PUBKEY_get(pub)); + EXPECT_FALSE(key2); +} diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c index cd0cfefa0..67ce46482 100644 --- a/crypto/x509/x_pubkey.c +++ b/crypto/x509/x_pubkey.c @@ -70,6 +70,15 @@ #include "../internal.h" #include "internal.h" + +static void x509_pubkey_changed(X509_PUBKEY *pub) { + // TODO(davidben): Instead of just dropping the key, also compute the new + // cached key. This will let us implement |X509_get0_pubkey| and remove the + // need for a mutex. + EVP_PKEY_free(pub->pkey); + pub->pkey = NULL; +} + // Minor tweak to operation: free up EVP_PKEY static int pubkey_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it, void *exarg) { @@ -190,6 +199,8 @@ int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *obj, int param_type, // Set the number of unused bits to zero. pub->public_key->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); pub->public_key->flags |= ASN1_STRING_FLAG_BITS_LEFT; + + x509_pubkey_changed(pub); return 1; }