diff --git a/crypto/hpke/hpke.c b/crypto/hpke/hpke.c index a4ae56861aa..b3238d5f95a 100644 --- a/crypto/hpke/hpke.c +++ b/crypto/hpke/hpke.c @@ -24,6 +24,13 @@ // This file implements RFC 9180 and draft-ietf-hpke-pq-05. +// Callers stack-allocate |EVP_HPKE_KEY|, so its size is ABI: a caller built +// against one release and run against a later one reserves the older size and +// overflows it. The struct is four pointers on any ABI, so spelling that out +// fails the build if KEM-dependent key material moves back inline. +OPENSSL_STATIC_ASSERT(sizeof(EVP_HPKE_KEY) == 4 * sizeof(void *), + evp_hpke_key_must_not_hold_inline_buffers) + // MAX_SEED_LEN is the largest |seed_len| of any KEM and MAX_SHARED_SECRET_LEN // the largest Nsecret. Both are 32 for every KEM this file implements: X25519 // seeds an ephemeral private key with 32 bytes and ML-KEM takes 32 bytes of @@ -35,6 +42,10 @@ struct evp_hpke_kem_st { uint16_t id; size_t public_key_len; size_t private_key_len; + // expanded_private_key_len is the length of the KEM's internal private key + // representation, or zero for KEMs which use the serialized private key + // directly. + size_t expanded_private_key_len; size_t seed_len; size_t enc_len; int (*init_key)(EVP_HPKE_KEY *key, const uint8_t *priv_key, @@ -340,6 +351,7 @@ const EVP_HPKE_KEM *EVP_hpke_x25519_hkdf_sha256(void) { /*id=*/EVP_HPKE_DHKEM_X25519_HKDF_SHA256, /*public_key_len=*/X25519_PUBLIC_VALUE_LEN, /*private_key_len=*/X25519_PRIVATE_KEY_LEN, + /*expanded_private_key_len=*/0, /*seed_len=*/X25519_PRIVATE_KEY_LEN, /*enc_len=*/X25519_PUBLIC_VALUE_LEN, x25519_init_key, @@ -383,9 +395,6 @@ OPENSSL_STATIC_ASSERT(MLKEM_SEED_LEN == MLKEM512_KEYGEN_SEED_LEN && ml_kem_keygen_seed_is_not_64_bytes) OPENSSL_STATIC_ASSERT(MLKEM_SEED_LEN <= EVP_HPKE_MAX_PRIVATE_KEY_LENGTH, evp_hpke_key_private_key_too_small_for_ml_kem) -OPENSSL_STATIC_ASSERT(MLKEM1024_SECRET_KEY_BYTES <= - EVP_HPKE_MAX_EXPANDED_PRIVATE_KEY_LENGTH, - evp_hpke_key_expanded_private_key_too_small) OPENSSL_STATIC_ASSERT(MLKEM1024_PUBLIC_KEY_BYTES <= EVP_HPKE_MAX_PUBLIC_KEY_LENGTH, evp_hpke_key_public_key_too_small_for_ml_kem) @@ -592,6 +601,7 @@ const EVP_HPKE_KEM *EVP_hpke_mlkem512(void) { /*id=*/EVP_HPKE_MLKEM512, /*public_key_len=*/MLKEM512_PUBLIC_KEY_BYTES, /*private_key_len=*/MLKEM_SEED_LEN, + /*expanded_private_key_len=*/MLKEM512_SECRET_KEY_BYTES, /*seed_len=*/MLKEM512_ENCAPS_SEED_LEN, /*enc_len=*/MLKEM512_CIPHERTEXT_BYTES, mlkem512_init_key, @@ -637,6 +647,7 @@ const EVP_HPKE_KEM *EVP_hpke_mlkem768(void) { /*id=*/EVP_HPKE_MLKEM768, /*public_key_len=*/MLKEM768_PUBLIC_KEY_BYTES, /*private_key_len=*/MLKEM_SEED_LEN, + /*expanded_private_key_len=*/MLKEM768_SECRET_KEY_BYTES, /*seed_len=*/MLKEM768_ENCAPS_SEED_LEN, /*enc_len=*/MLKEM768_CIPHERTEXT_BYTES, mlkem768_init_key, @@ -682,6 +693,7 @@ const EVP_HPKE_KEM *EVP_hpke_mlkem1024(void) { /*id=*/EVP_HPKE_MLKEM1024, /*public_key_len=*/MLKEM1024_PUBLIC_KEY_BYTES, /*private_key_len=*/MLKEM_SEED_LEN, + /*expanded_private_key_len=*/MLKEM1024_SECRET_KEY_BYTES, /*seed_len=*/MLKEM1024_ENCAPS_SEED_LEN, /*enc_len=*/MLKEM1024_CIPHERTEXT_BYTES, mlkem1024_init_key, @@ -706,6 +718,45 @@ size_t EVP_HPKE_KEM_private_key_len(const EVP_HPKE_KEM *kem) { size_t EVP_HPKE_KEM_enc_len(const EVP_HPKE_KEM *kem) { return kem->enc_len; } +// hpke_key_buffer_len returns the size of the single allocation backing a key +// for |kem|, which is partitioned by |hpke_key_alloc|. +static size_t hpke_key_buffer_len(const EVP_HPKE_KEM *kem) { + return kem->private_key_len + kem->public_key_len + + kem->expanded_private_key_len; +} + +// hpke_key_alloc points |key|'s buffers into one allocation sized for +// |key->kem|, which must already be set. It returns one on success and zero on +// allocation failure, after which the buffers are left NULL. +static int hpke_key_alloc(EVP_HPKE_KEY *key) { + const EVP_HPKE_KEM *kem = key->kem; + uint8_t *buf = OPENSSL_zalloc(hpke_key_buffer_len(kem)); + if (buf == NULL) { + return 0; + } + // |private_key| owns the allocation, so it comes first and is what + // |hpke_key_free| frees. + key->private_key = buf; + key->public_key = buf + kem->private_key_len; + key->expanded_private_key = + kem->expanded_private_key_len == 0 + ? NULL + : buf + kem->private_key_len + kem->public_key_len; + return 1; +} + +// hpke_key_free cleanses and releases |key|'s buffers, leaving the pointers +// dangling for the caller to clear. +static void hpke_key_free(EVP_HPKE_KEY *key) { + if (key->private_key == NULL) { + return; + } + // Cleanse with a barrier, which |OPENSSL_free| does not guarantee for the + // whole buffer and which a plain memset does not guarantee at all. + OPENSSL_cleanse(key->private_key, hpke_key_buffer_len(key->kem)); + OPENSSL_free(key->private_key); +} + void EVP_HPKE_KEY_zero(EVP_HPKE_KEY *key) { OPENSSL_memset(key, 0, sizeof(EVP_HPKE_KEY)); } @@ -714,18 +765,15 @@ void EVP_HPKE_KEY_cleanup(EVP_HPKE_KEY *key) { if (key == NULL) { return; } - // Cleanse the private key with a barrier, which |EVP_HPKE_KEY_zero|'s plain - // memset does not guarantee on its own, then return the whole struct to the - // zero state so that the result is indistinguishable from a key which has - // only ever been passed to |EVP_HPKE_KEY_zero|. + // Return the struct to the zero state so that the result is indistinguishable + // from a key which has only ever been passed to |EVP_HPKE_KEY_zero|. // // Clearing |kem| in particular means that using |key| after cleanup fails, // rather than silently operating on whatever key the all-zero private key // describes: every 64-byte string is a valid ML-KEM seed, and a zero X25519 // scalar is clamped to a valid one, so a cleansed key would otherwise still // "work". - OPENSSL_cleanse(key->private_key, sizeof(key->private_key)); - OPENSSL_cleanse(key->expanded_private_key, sizeof(key->expanded_private_key)); + hpke_key_free(key); EVP_HPKE_KEY_zero(key); } @@ -751,13 +799,18 @@ int EVP_HPKE_KEY_copy(EVP_HPKE_KEY *dst, const EVP_HPKE_KEY *src) { // undefined. There is also nothing to do. return 1; } - // The copy below already overwrites every byte of |dst|, including any key - // material it held, so this cleanse is not load-bearing today. It is here so - // that a future field which the copy does not cover cannot silently leave a - // private key behind, and to match |EVP_HPKE_KEY_move|. EVP_HPKE_KEY_cleanup(dst); - // For now, |EVP_HPKE_KEY| is trivially copyable. - OPENSSL_memcpy(dst, src, sizeof(EVP_HPKE_KEY)); + if (src->kem == NULL) { + // |src| is in the zero state, which |dst| now matches. + return 1; + } + dst->kem = src->kem; + if (!hpke_key_alloc(dst)) { + EVP_HPKE_KEY_cleanup(dst); + return 0; + } + OPENSSL_memcpy(dst->private_key, src->private_key, + hpke_key_buffer_len(src->kem)); return 1; } @@ -768,24 +821,21 @@ void EVP_HPKE_KEY_move(EVP_HPKE_KEY *out, EVP_HPKE_KEY *in) { return; } EVP_HPKE_KEY_cleanup(out); - // For now, |EVP_HPKE_KEY| is trivially movable. OPENSSL_memcpy(out, in, sizeof(EVP_HPKE_KEY)); - // Cleanse rather than zero |in|: it still holds the private key and, for - // ML-KEM, the expanded private key, and |EVP_HPKE_KEY_zero| alone is a plain - // memset. |EVP_HPKE_KEY_cleanup| leaves |in| in the zero state as well. - EVP_HPKE_KEY_cleanup(in); + // |out| now owns the buffers, so |in| is only cleared. Cleansing it here + // would erase and free key material |out| still points at. + EVP_HPKE_KEY_zero(in); } static int hpke_key_init(EVP_HPKE_KEY *key, const EVP_HPKE_KEM *kem, const uint8_t *priv_key, size_t priv_key_len) { - // |key| may already hold key material, so cleanse it before reusing the - // struct. |EVP_HPKE_KEY_zero| alone would be a plain memset, which is not a - // guaranteed erase, and |EVP_HPKE_KEY_cleanup| leaves |key| in the zero state - // anyway. It only writes to |key|, so it is also safe on an uninitialized one - // and imposes no precondition on callers. + // |key| may already own buffers, so release them before reusing the struct. + // |EVP_HPKE_KEY_zero| would drop the pointers and leak. Callers reach this + // from the zero state or from an initialized key, which is what + // |EVP_HPKE_KEY_cleanup| requires. EVP_HPKE_KEY_cleanup(key); key->kem = kem; - if (!kem->init_key(key, priv_key, priv_key_len)) { + if (!hpke_key_alloc(key) || !kem->init_key(key, priv_key, priv_key_len)) { // |init_key| may have failed partway and left key material behind, so // cleanse rather than only clearing |kem|. EVP_HPKE_KEY_cleanup(key); @@ -795,10 +845,10 @@ static int hpke_key_init(EVP_HPKE_KEY *key, const EVP_HPKE_KEM *kem, } static int hpke_key_generate(EVP_HPKE_KEY *key, const EVP_HPKE_KEM *kem) { - // See |hpke_key_init| for why |key| is cleansed before it is reused. + // See |hpke_key_init| for why |key| is cleaned up before it is reused. EVP_HPKE_KEY_cleanup(key); key->kem = kem; - if (!kem->generate_key(key)) { + if (!hpke_key_alloc(key) || !kem->generate_key(key)) { // As in |hpke_key_init|, cleanse in case |generate_key| failed partway. EVP_HPKE_KEY_cleanup(key); return 0; diff --git a/crypto/hpke/hpke_test.cc b/crypto/hpke/hpke_test.cc index d067f66d700..ce5502822a1 100644 --- a/crypto/hpke/hpke_test.cc +++ b/crypto/hpke/hpke_test.cc @@ -1380,6 +1380,69 @@ TEST(HPKETest, SelfCopyAndMoveAreSafe) { } } +// A key's buffers are sized for its KEM, so re-initializing one across KEMs +// resizes them. Going from the largest KEM to the smallest and back exercises +// both directions. +TEST(HPKETest, ReinitAcrossKEMsResizesBuffers) { + const EVP_HPKE_KEM *kems[] = {EVP_hpke_mlkem1024(), + EVP_hpke_x25519_hkdf_sha256(), + EVP_hpke_mlkem512()}; + ScopedEVP_HPKE_KEY key; + for (const EVP_HPKE_KEM *kem : kems) { + SCOPED_TRACE(EVP_HPKE_KEM_id(kem)); + ASSERT_TRUE(EVP_HPKE_KEY_generate(key.get(), kem)); + ASSERT_EQ(EVP_HPKE_KEM_id(EVP_HPKE_KEY_kem(key.get())), + EVP_HPKE_KEM_id(kem)); + + uint8_t public_key[EVP_HPKE_MAX_PUBLIC_KEY_LENGTH]; + size_t public_key_len = 0; + ASSERT_TRUE(EVP_HPKE_KEY_public_key(key.get(), public_key, &public_key_len, + sizeof(public_key))); + EXPECT_EQ(public_key_len, EVP_HPKE_KEM_public_key_len(kem)); + } +} + +// Setting up the same context twice must release the first set of buffers and +// leave the context usable, rather than leaking them. +TEST(HPKETest, RepeatedSetupIsUsable) { + ScopedEVP_HPKE_KEY key; + ASSERT_TRUE(EVP_HPKE_KEY_generate(key.get(), EVP_hpke_mlkem768())); + + uint8_t public_key[EVP_HPKE_MAX_PUBLIC_KEY_LENGTH]; + size_t public_key_len = 0; + ASSERT_TRUE(EVP_HPKE_KEY_public_key(key.get(), public_key, &public_key_len, + sizeof(public_key))); + + ScopedEVP_HPKE_CTX sender_ctx; + for (int i = 0; i < 2; i++) { + SCOPED_TRACE(i); + uint8_t enc[EVP_HPKE_MAX_ENC_LENGTH]; + size_t enc_len = 0; + ASSERT_TRUE(EVP_HPKE_CTX_setup_sender( + sender_ctx.get(), enc, &enc_len, sizeof(enc), EVP_HPKE_KEY_kem(key.get()), + EVP_hpke_hkdf_sha256(), EVP_hpke_aes_128_gcm(), public_key, + public_key_len, nullptr, 0)); + + static const uint8_t kPlaintext[] = "setup twice"; + uint8_t ciphertext[sizeof(kPlaintext) + EVP_HPKE_MAX_OVERHEAD]; + size_t ciphertext_len = 0; + ASSERT_TRUE(EVP_HPKE_CTX_seal(sender_ctx.get(), ciphertext, &ciphertext_len, + sizeof(ciphertext), kPlaintext, + sizeof(kPlaintext), nullptr, 0)); + + ScopedEVP_HPKE_CTX recipient_ctx; + ASSERT_TRUE(EVP_HPKE_CTX_setup_recipient( + recipient_ctx.get(), key.get(), EVP_hpke_hkdf_sha256(), + EVP_hpke_aes_128_gcm(), enc, enc_len, nullptr, 0)); + uint8_t plaintext[sizeof(kPlaintext)]; + size_t plaintext_len = 0; + ASSERT_TRUE(EVP_HPKE_CTX_open(recipient_ctx.get(), plaintext, &plaintext_len, + sizeof(plaintext), ciphertext, ciphertext_len, + nullptr, 0)); + EXPECT_EQ(Bytes(kPlaintext), Bytes(plaintext, plaintext_len)); + } +} + // ML-KEM decapsulation is implicitly rejecting: a corrupt but correctly-sized // "enc" yields a pseudorandom shared secret rather than an error, so recipient // setup succeeds and only the AEAD detects the problem. This is the opposite of diff --git a/include/openssl/hpke.h b/include/openssl/hpke.h index f02a1ec501c..32e84ea003e 100644 --- a/include/openssl/hpke.h +++ b/include/openssl/hpke.h @@ -51,10 +51,8 @@ OPENSSL_EXPORT uint16_t EVP_HPKE_KEM_id(const EVP_HPKE_KEM *kem); // for all KEMs currently supported by this library. // // Note this grew from 32 to 1568 when the ML-KEM KEMs were added. Callers which -// size buffers by this constant must be rebuilt against this header, as must -// callers which stack-allocate an |EVP_HPKE_KEY|: that struct grew from 72 bytes -// to 4808, both because of this constant and because it now caches an expanded -// ML-KEM decapsulation key. See |EVP_HPKE_MAX_EXPANDED_PRIVATE_KEY_LENGTH|. +// size buffers by this constant must be rebuilt against this header. Prefer +// |EVP_HPKE_KEM_public_key_len|, which reports the length for one KEM. #define EVP_HPKE_MAX_PUBLIC_KEY_LENGTH 1568 // EVP_HPKE_KEM_public_key_len returns the length of a public key for |kem|. @@ -129,10 +127,12 @@ OPENSSL_EXPORT const EVP_AEAD *EVP_HPKE_AEAD_aead(const EVP_HPKE_AEAD *aead); // This may be used for more uniform cleanup of |EVP_HPKE_KEY|. OPENSSL_EXPORT void EVP_HPKE_KEY_zero(EVP_HPKE_KEY *key); -// EVP_HPKE_KEY_cleanup cleanses the private key material in |key| and returns -// |key| to the zero state, as if it had been passed to |EVP_HPKE_KEY_zero|. -// |key| may be NULL. This clears the KEM too, so, as with a zeroed key, |key| -// must be re-initialized before it is used again. +// EVP_HPKE_KEY_cleanup cleanses the private key material in |key|, releases the +// memory it references, and returns |key| to the zero state, as if it had been +// passed to |EVP_HPKE_KEY_zero|. |key| may be NULL, but otherwise must have been +// initialized with |EVP_HPKE_KEY_zero| or one of the functions that set up a +// key. This clears the KEM too, so, as with a zeroed key, |key| must be +// re-initialized before it is used again. OPENSSL_EXPORT void EVP_HPKE_KEY_cleanup(EVP_HPKE_KEY *key); // EVP_HPKE_KEY_new returns a newly-allocated |EVP_HPKE_KEY|, or NULL on error. @@ -386,6 +386,11 @@ OPENSSL_EXPORT const EVP_HPKE_KDF *EVP_HPKE_CTX_kdf(const EVP_HPKE_CTX *ctx); // // The following structures are exported so their types are stack-allocatable, // but accessing or modifying their fields is forbidden. +// +// |EVP_HPKE_KEY| holds its key material out of line, sized for the configured +// KEM, so that adding a KEM cannot change the structure's size. A caller which +// stack-allocates one against one release and runs against a later one would +// otherwise overflow it. struct evp_hpke_ctx_st { const EVP_HPKE_KEM *kem; @@ -398,21 +403,16 @@ struct evp_hpke_ctx_st { int is_sender; }; -// EVP_HPKE_MAX_EXPANDED_PRIVATE_KEY_LENGTH is the maximum length of a KEM's -// internal private key representation, where it differs from the serialized -// form. It is sized for an expanded ML-KEM-1024 decapsulation key. -#define EVP_HPKE_MAX_EXPANDED_PRIVATE_KEY_LENGTH 3168 - struct evp_hpke_key_st { const EVP_HPKE_KEM *kem; - uint8_t private_key[EVP_HPKE_MAX_PRIVATE_KEY_LENGTH]; - uint8_t public_key[EVP_HPKE_MAX_PUBLIC_KEY_LENGTH]; + uint8_t *private_key; + uint8_t *public_key; // expanded_private_key holds the KEM's internal private key representation, // for KEMs where it differs from the serialized form. It is derived once when // the key is initialized, so that decapsulation does not have to repeat key // generation. KEMs which use the serialized private key directly leave it - // unused. - uint8_t expanded_private_key[EVP_HPKE_MAX_EXPANDED_PRIVATE_KEY_LENGTH]; + // NULL. + uint8_t *expanded_private_key; };