Make DSA opaque

Update-Note: Accessing the DSA struct directly is no longer supported. Use accessors instead. Bug: 325 Change-Id: I73dc20f2e275d48648ca84d2db7806fe155c567d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60425 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> (cherry picked from commit f4a4e2771542b90b01b15a9fe19d3fafe0fda0d0)
andrewhop · Oct 20, 2023 · e802f61 · e802f61
1 parent dc888d5
commit e802f61
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 29 deletions.
diff --git a/crypto/dsa/dsa_test.cc b/crypto/dsa/dsa_test.cc
@@ -257,7 +257,10 @@ TEST(DSATest, Verify) {
 TEST(DSATest, InvalidGroup) {
   bssl::UniquePtr<DSA> dsa = GetFIPSDSA();
   ASSERT_TRUE(dsa);
-  BN_zero(dsa->g);
+  bssl::UniquePtr<BIGNUM> zero(BN_new());
+  ASSERT_TRUE(zero);
+  ASSERT_TRUE(DSA_set0_pqg(dsa.get(), /*p=*/nullptr, /*q=*/nullptr,
+                           /*g=*/zero.release()));
 
   std::vector<uint8_t> sig(DSA_size(dsa.get()));
   unsigned sig_len;
@@ -305,7 +308,10 @@ TEST(DSATest, MissingPrivate) {
 TEST(DSATest, ZeroPrivateKey) {
   bssl::UniquePtr<DSA> dsa = GetFIPSDSA();
   ASSERT_TRUE(dsa);
-  BN_zero(dsa->priv_key);
+  bssl::UniquePtr<BIGNUM> zero(BN_new());
+  ASSERT_TRUE(zero);
+  ASSERT_TRUE(DSA_set0_key(dsa.get(), /*pub_key=*/nullptr,
+                           /*priv_key=*/zero.release()));
 
   static const uint8_t kZeroDigest[32] = {0};
   std::vector<uint8_t> sig(DSA_size(dsa.get()));

diff --git a/crypto/dsa/internal.h b/crypto/dsa/internal.h
@@ -17,11 +17,29 @@
 
 #include <openssl/dsa.h>
 
+#include <openssl/thread.h>
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
 
 
+struct dsa_st {
+  BIGNUM *p;
+  BIGNUM *q;
+  BIGNUM *g;
+
+  BIGNUM *pub_key;
+  BIGNUM *priv_key;
+
+  // Normally used to cache montgomery values
+  CRYPTO_MUTEX method_mont_lock;
+  BN_MONT_CTX *method_mont_p;
+  BN_MONT_CTX *method_mont_q;
+  CRYPTO_refcount_t references;
+  CRYPTO_EX_DATA ex_data;
+};
+
 // dsa_check_key performs cheap self-checks on |dsa|, and ensures it is within
 // DoS bounds. It returns one on success and zero on error.
 int dsa_check_key(const DSA *dsa);

diff --git a/crypto/evp_extra/print.c b/crypto/evp_extra/print.c
@@ -200,12 +200,12 @@ static int rsa_priv_print(BIO *bp, const EVP_PKEY *pkey, int indent) {
 static int do_dsa_print(BIO *bp, const DSA *x, int off, int ptype) {
   const BIGNUM *priv_key = NULL;
   if (ptype == 2) {
-    priv_key = x->priv_key;
+    priv_key = DSA_get0_priv_key(x);
   }
 
   const BIGNUM *pub_key = NULL;
   if (ptype > 0) {
-    pub_key = x->pub_key;
+    pub_key = DSA_get0_pub_key(x);
   }
 
   const char *ktype = "DSA-Parameters";
@@ -216,14 +216,15 @@ static int do_dsa_print(BIO *bp, const DSA *x, int off, int ptype) {
   }
 
   if (!BIO_indent(bp, off, 128) ||
-      BIO_printf(bp, "%s: (%u bit)\n", ktype, BN_num_bits(x->p)) <= 0 ||
+      BIO_printf(bp, "%s: (%u bit)\n", ktype, BN_num_bits(DSA_get0_p(x))) <=
+          0 ||
       // |priv_key| and |pub_key| may be NULL, in which case |bn_print| will
       // silently skip them.
       !bn_print(bp, "priv:", priv_key, off) ||
       !bn_print(bp, "pub:", pub_key, off) ||
-      !bn_print(bp, "P:", x->p, off) ||
-      !bn_print(bp, "Q:", x->q, off) ||
-      !bn_print(bp, "G:", x->g, off)) {
+      !bn_print(bp, "P:", DSA_get0_p(x), off) ||
+      !bn_print(bp, "Q:", DSA_get0_q(x), off) ||
+      !bn_print(bp, "G:", DSA_get0_g(x), off)) {
     return 0;
   }
 

diff --git a/include/openssl/dsa.h b/include/openssl/dsa.h
@@ -62,9 +62,7 @@
 
 #include <openssl/base.h>
 
-#include <openssl/engine.h>
 #include <openssl/ex_data.h>
-#include <openssl/thread.h>
 
 #if defined(__cplusplus)
 extern "C" {
@@ -389,25 +387,6 @@ OPENSSL_EXPORT DSA *d2i_DSAparams(DSA **out, const uint8_t **inp, long len);
 OPENSSL_EXPORT int i2d_DSAparams(const DSA *in, uint8_t **outp);
 
 
-struct dsa_st {
-  long version;
-  BIGNUM *p;
-  BIGNUM *q;  // == 20
-  BIGNUM *g;
-
-  BIGNUM *pub_key;   // y public key
-  BIGNUM *priv_key;  // x private key
-
-  int flags;
-  // Normally used to cache montgomery values
-  CRYPTO_MUTEX method_mont_lock;
-  BN_MONT_CTX *method_mont_p;
-  BN_MONT_CTX *method_mont_q;
-  CRYPTO_refcount_t references;
-  CRYPTO_EX_DATA ex_data;
-};
-
-
 #if defined(__cplusplus)
 }  // extern C