Always use a 32-byte shared secret for Kyber

Although the round-3 specification has a variable-length output, the
final ML-KEM construction is expected to use a fixed 32-byte output. To
simplify the future transition, we apply the same restriction.

Update-Note: The Kyber public APIs have changed slightly, but we do not
believe there are any users of them yet.

Change-Id: Iea4fb1b13ecfcc3fead62989cee79de011f413c5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64349
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>

6 files changed, 63 insertions, 56 deletions

diff --git a/crypto/kyber/internal.h b/crypto/kyber/internal.h
index b112117..7f4a08c 100644
--- a/crypto/kyber/internal.h
+++ b/crypto/kyber/internal.h
@@ -42,15 +42,15 @@ OPENSSL_EXPORT void KYBER_generate_key_external_entropy(
     struct KYBER_private_key *out_private_key,
     const uint8_t entropy[KYBER_GENERATE_KEY_ENTROPY]);
 
-// KYBER_encap_external_entropy is a deterministic function to encapsulate
-// |out_shared_secret_len| bytes of |out_shared_secret| to |ciphertext|, using
-// |KYBER_ENCAP_ENTROPY| bytes of |entropy| for randomization. The
-// decapsulating side will be able to recover |entropy| in full. This
-// function is should only be used for tests, regular callers should use the
-// non-deterministic |KYBER_encap| directly.
+// KYBER_encap_external_entropy behaves like |KYBER_encap|, but uses
+// |KYBER_ENCAP_ENTROPY| bytes of |entropy| for randomization. The decapsulating
+// side will be able to recover |entropy| in full. This function should only be
+// used for tests, regular callers should use the non-deterministic
+// |KYBER_encap| directly.
 OPENSSL_EXPORT void KYBER_encap_external_entropy(
-    uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES], uint8_t *out_shared_secret,
-    size_t out_shared_secret_len, const struct KYBER_public_key *public_key,
+    uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES],
+    uint8_t out_shared_secret[KYBER_SHARED_SECRET_BYTES],
+    const struct KYBER_public_key *public_key,
     const uint8_t entropy[KYBER_ENCAP_ENTROPY]);
 
 #if defined(__cplusplus)
diff --git a/crypto/kyber/kyber.c b/crypto/kyber/kyber.c
index d3ea020..5d7971f 100644
--- a/crypto/kyber/kyber.c
+++ b/crypto/kyber/kyber.c
@@ -682,12 +682,12 @@ static void encrypt_cpa(uint8_t out[KYBER_CIPHERTEXT_BYTES],
 
 // Calls KYBER_encap_external_entropy| with random bytes from |RAND_bytes|
 void KYBER_encap(uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES],
-                 uint8_t *out_shared_secret, size_t out_shared_secret_len,
+                 uint8_t out_shared_secret[KYBER_SHARED_SECRET_BYTES],
                  const struct KYBER_public_key *public_key) {
   uint8_t entropy[KYBER_ENCAP_ENTROPY];
   RAND_bytes(entropy, KYBER_ENCAP_ENTROPY);
-  KYBER_encap_external_entropy(out_ciphertext, out_shared_secret,
-                               out_shared_secret_len, public_key, entropy);
+  KYBER_encap_external_entropy(out_ciphertext, out_shared_secret, public_key,
+                               entropy);
 }
 
 // Algorithm 8 of the Kyber spec, safe for line 2 of the spec. The spec there
@@ -697,8 +697,9 @@ void KYBER_encap(uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES],
 // number generator is used, the caller should switch to a secure one before
 // calling this method.
 void KYBER_encap_external_entropy(
-    uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES], uint8_t *out_shared_secret,
-    size_t out_shared_secret_len, const struct KYBER_public_key *public_key,
+    uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES],
+    uint8_t out_shared_secret[KYBER_SHARED_SECRET_BYTES],
+    const struct KYBER_public_key *public_key,
     const uint8_t entropy[KYBER_ENCAP_ENTROPY]) {
   const struct public_key *pub = public_key_from_external(public_key);
   uint8_t input[64];
@@ -711,7 +712,7 @@ void KYBER_encap_external_entropy(
   encrypt_cpa(out_ciphertext, pub, entropy, prekey_and_randomness + 32);
   BORINGSSL_keccak(prekey_and_randomness + 32, 32, out_ciphertext,
                    KYBER_CIPHERTEXT_BYTES, boringssl_sha3_256);
-  BORINGSSL_keccak(out_shared_secret, out_shared_secret_len,
+  BORINGSSL_keccak(out_shared_secret, KYBER_SHARED_SECRET_BYTES,
                    prekey_and_randomness, sizeof(prekey_and_randomness),
                    boringssl_shake256);
 }
@@ -739,7 +740,7 @@ static void decrypt_cpa(uint8_t out[32], const struct private_key *priv,
 // failure to be passed on to the caller, and instead returns a result that is
 // deterministic but unpredictable to anyone without knowledge of the private
 // key.
-void KYBER_decap(uint8_t *out_shared_secret, size_t out_shared_secret_len,
+void KYBER_decap(uint8_t out_shared_secret[KYBER_SHARED_SECRET_BYTES],
                  const uint8_t ciphertext[KYBER_CIPHERTEXT_BYTES],
                  const struct KYBER_private_key *private_key) {
   const struct private_key *priv = private_key_from_external(private_key);
@@ -764,7 +765,7 @@ void KYBER_decap(uint8_t *out_shared_secret, size_t out_shared_secret_len,
   }
   BORINGSSL_keccak(input + 32, 32, ciphertext, KYBER_CIPHERTEXT_BYTES,
                    boringssl_sha3_256);
-  BORINGSSL_keccak(out_shared_secret, out_shared_secret_len, input,
+  BORINGSSL_keccak(out_shared_secret, KYBER_SHARED_SECRET_BYTES, input,
                    sizeof(input), boringssl_shake256);
 }
 
diff --git a/crypto/kyber/kyber_test.cc b/crypto/kyber/kyber_test.cc
index b9daa87..c42db78 100644
--- a/crypto/kyber/kyber_test.cc
+++ b/crypto/kyber/kyber_test.cc
@@ -95,12 +95,12 @@ TEST(KyberTest, Basic) {
             Bytes(Marshal(KYBER_marshal_private_key, &priv2)));
 
   uint8_t ciphertext[KYBER_CIPHERTEXT_BYTES];
-  uint8_t shared_secret1[64];
-  uint8_t shared_secret2[sizeof(shared_secret1)];
-  KYBER_encap(ciphertext, shared_secret1, sizeof(shared_secret1), &pub);
-  KYBER_decap(shared_secret2, sizeof(shared_secret2), ciphertext, &priv);
+  uint8_t shared_secret1[KYBER_SHARED_SECRET_BYTES];
+  uint8_t shared_secret2[KYBER_SHARED_SECRET_BYTES];
+  KYBER_encap(ciphertext, shared_secret1, &pub);
+  KYBER_decap(shared_secret2, ciphertext, &priv);
   EXPECT_EQ(Bytes(shared_secret1), Bytes(shared_secret2));
-  KYBER_decap(shared_secret2, sizeof(shared_secret2), ciphertext, &priv2);
+  KYBER_decap(shared_secret2, ciphertext, &priv2);
   EXPECT_EQ(Bytes(shared_secret1), Bytes(shared_secret2));
 }
 
@@ -125,8 +125,8 @@ static void KyberFileTest(FileTest *t) {
   uint8_t ciphertext[KYBER_CIPHERTEXT_BYTES];
   uint8_t gen_key_entropy[KYBER_GENERATE_KEY_ENTROPY];
   uint8_t encap_entropy[KYBER_ENCAP_ENTROPY];
-  uint8_t encapsulated_key[32];
-  uint8_t decapsulated_key[32];
+  uint8_t encapsulated_key[KYBER_SHARED_SECRET_BYTES];
+  uint8_t decapsulated_key[KYBER_SHARED_SECRET_BYTES];
   // The test vectors provide a CTR-DRBG seed which is used to generate the
   // input entropy.
   ASSERT_EQ(seed.size(), size_t{CTR_DRBG_ENTROPY_LEN});
@@ -157,9 +157,9 @@ static void KyberFileTest(FileTest *t) {
   CBS_init(&encoded_public_key_cbs, encoded_public_key,
            sizeof(encoded_public_key));
   ASSERT_TRUE(KYBER_parse_public_key(&pub, &encoded_public_key_cbs));
-  KYBER_encap_external_entropy(ciphertext, encapsulated_key,
-                               sizeof(encapsulated_key), &pub, encap_entropy);
-  KYBER_decap(decapsulated_key, sizeof(decapsulated_key), ciphertext, &priv);
+  KYBER_encap_external_entropy(ciphertext, encapsulated_key, &pub,
+                               encap_entropy);
+  KYBER_decap(decapsulated_key, ciphertext, &priv);
 
   EXPECT_EQ(Bytes(encapsulated_key), Bytes(decapsulated_key));
   EXPECT_EQ(Bytes(private_key_expected), Bytes(encoded_private_key));
@@ -170,9 +170,8 @@ static void KyberFileTest(FileTest *t) {
   uint8_t corrupted_ciphertext[KYBER_CIPHERTEXT_BYTES];
   OPENSSL_memcpy(corrupted_ciphertext, ciphertext, KYBER_CIPHERTEXT_BYTES);
   corrupted_ciphertext[3] ^= 0x40;
-  uint8_t corrupted_decapsulated_key[32];
-  KYBER_decap(corrupted_decapsulated_key, sizeof(corrupted_decapsulated_key),
-              corrupted_ciphertext, &priv);
+  uint8_t corrupted_decapsulated_key[KYBER_SHARED_SECRET_BYTES];
+  KYBER_decap(corrupted_decapsulated_key, corrupted_ciphertext, &priv);
   // It would be nice to have actual test vectors for the failure case, but the
   // NIST submission currently does not include those, so we are just testing
   // for inequality.
diff --git a/include/openssl/kyber.h b/include/openssl/kyber.h
index cafae9d..9f5a3ca 100644
--- a/include/openssl/kyber.h
+++ b/include/openssl/kyber.h
@@ -23,6 +23,9 @@ extern "C" {
 
 
 // Kyber768.
+//
+// This implements the round-3 specification of Kyber, defined at
+// https://pq-crystals.org/kyber/data/kyber-specification-round3-20210804.pdf
 
 
 // KYBER_public_key contains a Kyber768 public key. The contents of this
@@ -47,6 +50,12 @@ struct KYBER_private_key {
 // key.
 #define KYBER_PUBLIC_KEY_BYTES 1184
 
+// KYBER_SHARED_SECRET_BYTES is the number of bytes in the Kyber768 shared
+// secret. Although the round-3 specification has a variable-length output, the
+// final ML-KEM construction is expected to use a fixed 32-byte output. To
+// simplify the future transition, we apply the same restriction.
+#define KYBER_SHARED_SECRET_BYTES 32
+
 // KYBER_generate_key generates a random public/private key pair, writes the
 // encoded public key to |out_encoded_public_key| and sets |out_private_key| to
 // the private key.
@@ -65,25 +74,24 @@ OPENSSL_EXPORT void KYBER_public_from_private(
 // KYBER_CIPHERTEXT_BYTES is number of bytes in the Kyber768 ciphertext.
 #define KYBER_CIPHERTEXT_BYTES 1088
 
-// KYBER_encap encrypts a random secret key of length |out_shared_secret_len| to
-// |public_key|, writes the ciphertext to |ciphertext|, and writes the random
-// key to |out_shared_secret|. The party calling |KYBER_decap| must already know
-// the correct value of |out_shared_secret_len|.
-OPENSSL_EXPORT void KYBER_encap(uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES],
-                                uint8_t *out_shared_secret,
-                                size_t out_shared_secret_len,
-                                const struct KYBER_public_key *public_key);
-
-// KYBER_decap decrypts a key of length |out_shared_secret_len| from
-// |ciphertext| using |private_key| and writes it to |out_shared_secret|. If
-// |ciphertext| is invalid, |out_shared_secret| is filled with a key that
-// will always be the same for the same |ciphertext| and |private_key|, but
-// which appears to be random unless one has access to |private_key|. These
-// alternatives occur in constant time. Any subsequent symmetric encryption
-// using |out_shared_secret| must use an authenticated encryption scheme in
-// order to discover the decapsulation failure.
+// KYBER_encap encrypts a random shared secret for |public_key|, writes the
+// ciphertext to |out_ciphertext|, and writes the random shared secret to
+// |out_shared_secret|.
+OPENSSL_EXPORT void KYBER_encap(
+    uint8_t out_ciphertext[KYBER_CIPHERTEXT_BYTES],
+    uint8_t out_shared_secret[KYBER_SHARED_SECRET_BYTES],
+    const struct KYBER_public_key *public_key);
+
+// KYBER_decap decrypts a shared secret from |ciphertext| using |private_key|
+// and writes it to |out_shared_secret|. If |ciphertext| is invalid,
+// |out_shared_secret| is filled with a key that will always be the same for the
+// same |ciphertext| and |private_key|, but which appears to be random unless
+// one has access to |private_key|. These alternatives occur in constant time.
+// Any subsequent symmetric encryption using |out_shared_secret| must use an
+// authenticated encryption scheme in order to discover the decapsulation
+// failure.
 OPENSSL_EXPORT void KYBER_decap(
-    uint8_t *out_shared_secret, size_t out_shared_secret_len,
+    uint8_t *out_shared_secret,
     const uint8_t ciphertext[KYBER_CIPHERTEXT_BYTES],
     const struct KYBER_private_key *private_key);
 
diff --git a/ssl/ssl_key_share.cc b/ssl/ssl_key_share.cc
index 694bec1..80317d8 100644
--- a/ssl/ssl_key_share.cc
+++ b/ssl/ssl_key_share.cc
@@ -217,7 +217,7 @@ class X25519Kyber768KeyShare : public SSLKeyShare {
   bool Encap(CBB *out_ciphertext, Array<uint8_t> *out_secret,
              uint8_t *out_alert, Span<const uint8_t> peer_key) override {
     Array<uint8_t> secret;
-    if (!secret.Init(32 + 32)) {
+    if (!secret.Init(32 + KYBER_SHARED_SECRET_BYTES)) {
       return false;
     }
 
@@ -241,8 +241,7 @@ class X25519Kyber768KeyShare : public SSLKeyShare {
     }
 
     uint8_t kyber_ciphertext[KYBER_CIPHERTEXT_BYTES];
-    KYBER_encap(kyber_ciphertext, secret.data() + 32, secret.size() - 32,
-                &peer_kyber_pub);
+    KYBER_encap(kyber_ciphertext, secret.data() + 32, &peer_kyber_pub);
 
     if (!CBB_add_bytes(out_ciphertext, x25519_public_key,
                        sizeof(x25519_public_key)) ||
@@ -260,7 +259,7 @@ class X25519Kyber768KeyShare : public SSLKeyShare {
     *out_alert = SSL_AD_INTERNAL_ERROR;
 
     Array<uint8_t> secret;
-    if (!secret.Init(32 + 32)) {
+    if (!secret.Init(32 + KYBER_SHARED_SECRET_BYTES)) {
       return false;
     }
 
@@ -271,7 +270,7 @@ class X25519Kyber768KeyShare : public SSLKeyShare {
       return false;
     }
 
-    KYBER_decap(secret.data() + 32, secret.size() - 32, ciphertext.data() + 32,
+    KYBER_decap(secret.data() + 32, ciphertext.data() + 32,
                 &kyber_private_key_);
     *out_secret = std::move(secret);
     return true;
diff --git a/tool/speed.cc b/tool/speed.cc
index 942dcad..5473909 100644
--- a/tool/speed.cc
+++ b/tool/speed.cc
@@ -1094,8 +1094,8 @@ static bool SpeedKyber(const std::string &selected) {
         KYBER_private_key priv;
         uint8_t encoded_public_key[KYBER_PUBLIC_KEY_BYTES];
         KYBER_generate_key(encoded_public_key, &priv);
-        uint8_t shared_secret[32];
-        KYBER_decap(shared_secret, sizeof(shared_secret), ciphertext, &priv);
+        uint8_t shared_secret[KYBER_SHARED_SECRET_BYTES];
+        KYBER_decap(shared_secret, ciphertext, &priv);
         return true;
       })) {
     fprintf(stderr, "Failed to time KYBER_generate_key + KYBER_decap.\n");
@@ -1115,8 +1115,8 @@ static bool SpeedKyber(const std::string &selected) {
         if (!KYBER_parse_public_key(&pub, &encoded_public_key_cbs)) {
           return false;
         }
-        uint8_t shared_secret[32];
-        KYBER_encap(ciphertext, shared_secret, sizeof(shared_secret), &pub);
+        uint8_t shared_secret[KYBER_SHARED_SECRET_BYTES];
+        KYBER_encap(ciphertext, shared_secret, &pub);
         return true;
       })) {
     fprintf(stderr, "Failed to time KYBER_encap.\n");