Reject obviously invalid DSA parameters during signing.
If g is zero, the retry loop will run infinitely. See
8f506274029903457c5f1d8663a012763f55cd37 from upstream.
Change-Id: I9e36002f2907dee3b5905e414e3c931d62b1a447
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35924
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/dsa/dsa.c b/crypto/dsa/dsa.c
index 288e2c8..51dca7f 100644
--- a/crypto/dsa/dsa.c
+++ b/crypto/dsa/dsa.c
@@ -558,29 +558,34 @@
}
DSA_SIG *DSA_do_sign(const uint8_t *digest, size_t digest_len, const DSA *dsa) {
- BIGNUM *kinv = NULL, *r = NULL, *s = NULL;
- BIGNUM m;
- BIGNUM xr;
- BN_CTX *ctx = NULL;
- int reason = ERR_R_BN_LIB;
- DSA_SIG *ret = NULL;
-
- BN_init(&m);
- BN_init(&xr);
-
if (!dsa->p || !dsa->q || !dsa->g) {
- reason = DSA_R_MISSING_PARAMETERS;
- goto err;
+ OPENSSL_PUT_ERROR(DSA, DSA_R_MISSING_PARAMETERS);
+ return NULL;
+ }
+
+ // Reject invalid parameters. In particular, the algorithm will infinite loop
+ // if |g| is zero.
+ if (BN_is_zero(dsa->p) || BN_is_zero(dsa->q) || BN_is_zero(dsa->g)) {
+ OPENSSL_PUT_ERROR(DSA, DSA_R_INVALID_PARAMETERS);
+ return NULL;
}
// We only support DSA keys that are a multiple of 8 bits. (This is a weaker
// check than the one in |DSA_do_check_signature|, which only allows 160-,
// 224-, and 256-bit keys.
if (BN_num_bits(dsa->q) % 8 != 0) {
- reason = DSA_R_BAD_Q_VALUE;
- goto err;
+ OPENSSL_PUT_ERROR(DSA, DSA_R_BAD_Q_VALUE);
+ return NULL;
}
+ BIGNUM *kinv = NULL, *r = NULL, *s = NULL;
+ BIGNUM m;
+ BIGNUM xr;
+ BN_CTX *ctx = NULL;
+ DSA_SIG *ret = NULL;
+
+ BN_init(&m);
+ BN_init(&xr);
s = BN_new();
if (s == NULL) {
goto err;
@@ -640,7 +645,7 @@
err:
if (ret == NULL) {
- OPENSSL_PUT_ERROR(DSA, reason);
+ OPENSSL_PUT_ERROR(DSA, ERR_R_BN_LIB);
BN_free(r);
BN_free(s);
}
diff --git a/crypto/dsa/dsa_test.cc b/crypto/dsa/dsa_test.cc
index 295a7fd..4682131 100644
--- a/crypto/dsa/dsa_test.cc
+++ b/crypto/dsa/dsa_test.cc
@@ -62,6 +62,8 @@
#include <stdio.h>
#include <string.h>
+#include <vector>
+
#include <gtest/gtest.h>
#include <openssl/bn.h>
@@ -315,3 +317,18 @@
ADD_FAILURE() << "Tests failed";
}
}
+
+TEST(DSATest, InvalidGroup) {
+ bssl::UniquePtr<DSA> dsa = GetFIPSDSA();
+ ASSERT_TRUE(dsa);
+ BN_zero(dsa->g);
+
+ std::vector<uint8_t> sig(DSA_size(dsa.get()));
+ unsigned sig_len;
+ static const uint8_t kDigest[32] = {0};
+ EXPECT_FALSE(
+ DSA_sign(0, kDigest, sizeof(kDigest), sig.data(), &sig_len, dsa.get()));
+ uint32_t err = ERR_get_error();
+ EXPECT_EQ(ERR_LIB_DSA, ERR_GET_LIB(err));
+ EXPECT_EQ(DSA_R_INVALID_PARAMETERS, ERR_GET_REASON(err));
+}
diff --git a/crypto/err/dsa.errordata b/crypto/err/dsa.errordata
index 6f4bc13..1cf5206 100644
--- a/crypto/err/dsa.errordata
+++ b/crypto/err/dsa.errordata
@@ -2,6 +2,7 @@
DSA,104,BAD_VERSION
DSA,105,DECODE_ERROR
DSA,106,ENCODE_ERROR
+DSA,107,INVALID_PARAMETERS
DSA,101,MISSING_PARAMETERS
DSA,102,MODULUS_TOO_LARGE
DSA,103,NEED_NEW_SETUP_VALUES
diff --git a/include/openssl/dsa.h b/include/openssl/dsa.h
index bed93c5..8e3b9b3 100644
--- a/include/openssl/dsa.h
+++ b/include/openssl/dsa.h
@@ -436,5 +436,6 @@
#define DSA_R_BAD_VERSION 104
#define DSA_R_DECODE_ERROR 105
#define DSA_R_ENCODE_ERROR 106
+#define DSA_R_INVALID_PARAMETERS 107
#endif // OPENSSL_HEADER_DSA_H
