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>
diff --git a/crypto/dh_extra/dh_asn1.c b/crypto/dh_extra/dh_asn1.c
index 9d32180..de01077 100644
--- a/crypto/dh_extra/dh_asn1.c
+++ b/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) {
diff --git a/crypto/dh_extra/dh_test.cc b/crypto/dh_extra/dh_test.cc
index 7933a8c..b88702b 100644
--- a/crypto/dh_extra/dh_test.cc
+++ b/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 @@
}
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 @@
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 @@
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 @@
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") ||
diff --git a/crypto/dh_extra/params.c b/crypto/dh_extra/params.c
index f50786c..0e76747 100644
--- a/crypto/dh_extra/params.c
+++ b/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 @@
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;
}
diff --git a/crypto/dsa/dsa.c b/crypto/dsa/dsa.c
index b8e4653..f1fc02f 100644
--- a/crypto/dsa/dsa.c
+++ b/crypto/dsa/dsa.c
@@ -74,6 +74,7 @@
#include "internal.h"
#include "../fipsmodule/bn/internal.h"
+#include "../fipsmodule/dh/internal.h"
#include "../internal.h"
diff --git a/crypto/fipsmodule/dh/check.c b/crypto/fipsmodule/dh/check.c
index 5b6e03a..0c82c17 100644
--- a/crypto/fipsmodule/dh/check.c
+++ b/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 @@
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) {
diff --git a/crypto/fipsmodule/dh/dh.c b/crypto/fipsmodule/dh/dh.c
index 1a3c974..11dbfc2 100644
--- a/crypto/fipsmodule/dh/dh.c
+++ b/crypto/fipsmodule/dh/dh.c
@@ -101,9 +101,6 @@
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);
diff --git a/crypto/fipsmodule/dh/internal.h b/crypto/fipsmodule/dh/internal.h
index c40172d..fb525d4 100644
--- a/crypto/fipsmodule/dh/internal.h
+++ b/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.
diff --git a/include/openssl/dh.h b/include/openssl/dh.h
index 6aad778..660627d 100644
--- a/include/openssl/dh.h
+++ b/include/openssl/dh.h
@@ -244,7 +244,6 @@
#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 @@
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
