commit 4b040e562e950b168d49c1a1dd453cf460ab81c9
author David Benjamin <davidben@google.com> Fri Aug 11 15:02:00 2023 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Aug 14 18:31:58 2023 +0000
tree 1c75309cee92643ccf5941ef2a5c54fd773c324e
parent 2e1191725f4d291cbaf9d35bb5bcbad3852fc5d2

Document and test DH_generate_key's weird key reuse behavior

If the DH object already has a private key, DH_generate_key is actually
a function to compute the corresponding public key. This is very weird,
but as we don't really care about DH, just document and test it.

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

crypto/dh_extra/dh_test.cc

diff --git a/crypto/dh_extra/dh_test.cc b/crypto/dh_extra/dh_test.cc
index 8d2c587..f27c8f0 100644
--- a/crypto/dh_extra/dh_test.cc
+++ b/crypto/dh_extra/dh_test.cc
@@ -427,3 +427,32 @@
   ASSERT_GT(DH_compute_key_padded(buf2.data(), peer_key.get(), key2.get()), 0);
   EXPECT_EQ(Bytes(buf1), Bytes(buf2));
 }
+
+TEST(DHTest, GenerateKeyTwice) {
+  bssl::UniquePtr<BIGNUM> p(BN_get_rfc3526_prime_2048(nullptr));
+  ASSERT_TRUE(p);
+  bssl::UniquePtr<BIGNUM> g(BN_new());
+  ASSERT_TRUE(g);
+  ASSERT_TRUE(BN_set_word(g.get(), 2));
+  bssl::UniquePtr<DH> key1(DH_new());
+  ASSERT_TRUE(key1);
+  ASSERT_TRUE(DH_set0_pqg(key1.get(), p.get(), /*q=*/nullptr, g.get()));
+  p.release();
+  g.release();
+  ASSERT_TRUE(DH_generate_key(key1.get()));
+
+  // Copy the parameters and private key to a new DH object.
+  bssl::UniquePtr<DH> key2(DHparams_dup(key1.get()));
+  ASSERT_TRUE(key2);
+  bssl::UniquePtr<BIGNUM> priv_key(BN_dup(DH_get0_priv_key(key1.get())));
+  ASSERT_TRUE(DH_set0_key(key2.get(), /*pub_key=*/NULL, priv_key.get()));
+  priv_key.release();
+
+  // This time, calling |DH_generate_key| preserves the old key and recomputes
+  // the public key.
+  ASSERT_TRUE(DH_generate_key(key2.get()));
+  EXPECT_EQ(BN_cmp(DH_get0_priv_key(key1.get()), DH_get0_priv_key(key2.get())),
+            0);
+  EXPECT_EQ(BN_cmp(DH_get0_pub_key(key1.get()), DH_get0_pub_key(key2.get())),
+            0);
+}

include/openssl/dh.h

diff --git a/include/openssl/dh.h b/include/openssl/dh.h
index 660627d..b83fb5e 100644
--- a/include/openssl/dh.h
+++ b/include/openssl/dh.h
@@ -193,7 +193,9 @@
 // Diffie-Hellman operations.
 
 // DH_generate_key generates a new, random, private key and stores it in
-// |dh|. It returns one on success and zero on error.
+// |dh|, if |dh| does not already have a private key. Otherwise, it updates
+// |dh|'s public key to match the private key. It returns one on success and
+// zero on error.
 OPENSSL_EXPORT int DH_generate_key(DH *dh);
 
 // DH_compute_key_padded calculates the shared key between |dh| and |peers_key|