Make DH opaque.

In doing so, remove some X9.42 placeholder fields, since it's impossible
to set them. I switched dh_test.cc to the getters where it was easy, but
OpenSSL's new setters are so tedious that I just gave it access to the
internal struct.

With this, there are now only two public structs (DSA and RSA) that
reference CRYPTO_MUTEX. After that's removed, we can stop worrying about
pthread_rwlock_t feature flags in the public headers.

Update-Note: DH is now an opaque structure. Callers should use accessors
instead of accessing fields.

Change-Id: Ia53702f8ab58884a90d85718ee26eb03d062d234
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54625
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/dh_extra/dh_asn1.c

@@ -63,6 +63,7 @@
 #include <openssl/err.h>
 
 #include "../bytestring/internal.h"
+#include "../fipsmodule/dh/internal.h"
 
 
 static int parse_integer(CBS *cbs, BIGNUM **out) {

crypto/dh_extra/dh_test.cc

@@ -70,6 +70,7 @@
 #include <openssl/err.h>
 #include <openssl/mem.h>
 
+#include "../fipsmodule/dh/internal.h"
 #include "../internal.h"
 #include "../test/test_util.h"
 
@@ -134,42 +135,36 @@ static bool RunBasicTests() {
   }
 
   printf("\np    = ");
-  BN_print_fp(stdout, a->p);
+  BN_print_fp(stdout, DH_get0_p(a.get()));
   printf("\ng    = ");
-  BN_print_fp(stdout, a->g);
+  BN_print_fp(stdout, DH_get0_g(a.get()));
   printf("\n");
 
-  bssl::UniquePtr<DH> b(DH_new());
+  bssl::UniquePtr<DH> b(DHparams_dup(a.get()));
   if (!b) {
     return false;
   }
 
-  b->p = BN_dup(a->p);
-  b->g = BN_dup(a->g);
-  if (b->p == nullptr || b->g == nullptr) {
-    return false;
-  }
-
   if (!DH_generate_key(a.get())) {
     return false;
   }
   printf("pri1 = ");
-  BN_print_fp(stdout, a->priv_key);
+  BN_print_fp(stdout, DH_get0_priv_key(a.get()));
   printf("\npub1 = ");
-  BN_print_fp(stdout, a->pub_key);
+  BN_print_fp(stdout, DH_get0_pub_key(a.get()));
   printf("\n");
 
   if (!DH_generate_key(b.get())) {
     return false;
   }
   printf("pri2 = ");
-  BN_print_fp(stdout, b->priv_key);
+  BN_print_fp(stdout, DH_get0_priv_key(b.get()));
   printf("\npub2 = ");
-  BN_print_fp(stdout, b->pub_key);
+  BN_print_fp(stdout, DH_get0_pub_key(b.get()));
   printf("\n");
 
   std::vector<uint8_t> key1(DH_size(a.get()));
-  int ret = DH_compute_key(key1.data(), b->pub_key, a.get());
+  int ret = DH_compute_key(key1.data(), DH_get0_pub_key(b.get()), a.get());
   if (ret < 0) {
     return false;
   }
@@ -182,7 +177,7 @@ static bool RunBasicTests() {
   printf("\n");
 
   std::vector<uint8_t> key2(DH_size(b.get()));
-  ret = DH_compute_key(key2.data(), a->pub_key, b.get());
+  ret = DH_compute_key(key2.data(), DH_get0_pub_key(a.get()), b.get());
   if (ret < 0) {
     return false;
   }
@@ -343,9 +338,9 @@ static bool TestASN1() {
   bssl::UniquePtr<DH> dh(DH_parse_parameters(&cbs));
   if (!dh || CBS_len(&cbs) != 0 ||
       !BIGNUMEqualsHex(
-          dh->p,
+          DH_get0_p(dh.get()),
           "d72034a3274fdfbf04fd246825b656d8ab2a412d740a52087c40714ed2579313") ||
-      !BIGNUMEqualsHex(dh->g, "2") || dh->priv_length != 0) {
+      !BIGNUMEqualsHex(DH_get0_g(dh.get()), "2") || dh->priv_length != 0) {
     return false;
   }
 
@@ -383,11 +378,11 @@ static bool TestASN1() {
   CBS_init(&cbs, kParamsDSA, sizeof(kParamsDSA));
   dh.reset(DH_parse_parameters(&cbs));
   if (!dh || CBS_len(&cbs) != 0 ||
-      !BIGNUMEqualsHex(dh->p,
+      !BIGNUMEqualsHex(DH_get0_p(dh.get()),
                        "93f3c11801e662b6d1469a2c72ea31d91810302863e2347d80caee8"
                        "22b193c19bb42830270dddb8c03abe99cc4004d705f5203312ca467"
                        "3451952aac11e26a55") ||
-      !BIGNUMEqualsHex(dh->g,
+      !BIGNUMEqualsHex(DH_get0_g(dh.get()),
                        "44c8105344323163d8d18c75c898533b5b4a2a0a09e7d03c5372a86"
                        "b70419c267144fc7f0875e102ab7441e82a3d3c263309e48bb441ec"
                        "a6a8ba1a078a77f55f") ||

crypto/dh_extra/params.c

@@ -57,6 +57,7 @@
 #include <openssl/mem.h>
 
 #include "../fipsmodule/bn/internal.h"
+#include "../fipsmodule/dh/internal.h"
 
 
 static BIGNUM *get_params(BIGNUM *ret, const BN_ULONG *words, size_t num_words) {
@@ -452,23 +453,10 @@ static int int_dh_param_copy(DH *to, const DH *from, int is_x942) {
     return 1;
   }
 
-  if (!int_dh_bn_cpy(&to->q, from->q) ||
-      !int_dh_bn_cpy(&to->j, from->j)) {
+  if (!int_dh_bn_cpy(&to->q, from->q)) {
     return 0;
   }
 
-  OPENSSL_free(to->seed);
-  to->seed = NULL;
-  to->seedlen = 0;
-
-  if (from->seed) {
-    to->seed = OPENSSL_memdup(from->seed, from->seedlen);
-    if (!to->seed) {
-      return 0;
-    }
-    to->seedlen = from->seedlen;
-  }
-
   return 1;
 }
 

crypto/dsa/dsa.c

@@ -74,6 +74,7 @@
 
 #include "internal.h"
 #include "../fipsmodule/bn/internal.h"
+#include "../fipsmodule/dh/internal.h"
 #include "../internal.h"
 
 

crypto/fipsmodule/dh/check.c

@@ -58,6 +58,8 @@
 
 #include <openssl/bn.h>
 
+#include "internal.h"
+
 
 int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *out_flags) {
   *out_flags = 0;
@@ -165,9 +167,6 @@ int DH_check(const DH *dh, int *out_flags) {
     if (!BN_is_one(t2)) {
       *out_flags |= DH_CHECK_INVALID_Q_VALUE;
     }
-    if (dh->j && BN_cmp(dh->j, t1)) {
-      *out_flags |= DH_CHECK_INVALID_J_VALUE;
-    }
   } else if (BN_is_word(dh->g, DH_GENERATOR_2)) {
     l = BN_mod_word(dh->p, 24);
     if (l == (BN_ULONG)-1) {

crypto/fipsmodule/dh/dh.c

@@ -101,9 +101,6 @@ void DH_free(DH *dh) {
   BN_clear_free(dh->p);
   BN_clear_free(dh->g);
   BN_clear_free(dh->q);
-  BN_clear_free(dh->j);
-  OPENSSL_free(dh->seed);
-  BN_clear_free(dh->counter);
   BN_clear_free(dh->pub_key);
   BN_clear_free(dh->priv_key);
   CRYPTO_MUTEX_cleanup(&dh->method_mont_p_lock);

crypto/fipsmodule/dh/internal.h

@@ -17,11 +17,31 @@
 
 #include <openssl/base.h>
 
+#include <openssl/thread.h>
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
 
 
+struct dh_st {
+  BIGNUM *p;
+  BIGNUM *g;
+  BIGNUM *q;
+  BIGNUM *pub_key;   // g^x mod p
+  BIGNUM *priv_key;  // x
+
+  // priv_length contains the length, in bits, of the private value. If zero,
+  // the private value will be the same length as |p|.
+  unsigned priv_length;
+
+  CRYPTO_MUTEX method_mont_p_lock;
+  BN_MONT_CTX *method_mont_p;
+
+  int flags;
+  CRYPTO_refcount_t references;
+};
+
 // dh_compute_key_padded_no_self_test does the same as |DH_compute_key_padded|,
 // but doesn't try to run the self-test first. This is for use in the self tests
 // themselves, to prevent an infinite loop.

include/openssl/dh.h

@@ -244,7 +244,6 @@ OPENSSL_EXPORT unsigned DH_num_bits(const DH *dh);
 #define DH_CHECK_NOT_SUITABLE_GENERATOR 0x08
 #define DH_CHECK_Q_NOT_PRIME 0x10
 #define DH_CHECK_INVALID_Q_VALUE 0x20
-#define DH_CHECK_INVALID_J_VALUE 0x40
 
 // These are compatibility defines.
 #define DH_NOT_SUITABLE_GENERATOR DH_CHECK_NOT_SUITABLE_GENERATOR
@@ -330,31 +329,6 @@ OPENSSL_EXPORT int DH_compute_key(uint8_t *out, const BIGNUM *peers_key,
                                   DH *dh);
 
 
-struct dh_st {
-  BIGNUM *p;
-  BIGNUM *g;
-  BIGNUM *pub_key;   // g^x mod p
-  BIGNUM *priv_key;  // x
-
-  // priv_length contains the length, in bits, of the private value. If zero,
-  // the private value will be the same length as |p|.
-  unsigned priv_length;
-
-  CRYPTO_MUTEX method_mont_p_lock;
-  BN_MONT_CTX *method_mont_p;
-
-  // Place holders if we want to do X9.42 DH
-  BIGNUM *q;
-  BIGNUM *j;
-  unsigned char *seed;
-  int seedlen;
-  BIGNUM *counter;
-
-  int flags;
-  CRYPTO_refcount_t references;
-};
-
-
 #if defined(__cplusplus)
 }  // extern C