Upstream merge 2023 04 14

BUILDING.md

@@ -104,8 +104,8 @@ Unpack the Android NDK somewhere and export `ANDROID_NDK` to point to the
 directory. Then make a build directory as above and run CMake like this:
 
     cmake -DANDROID_ABI=armeabi-v7a \
+          -DANDROID_PLATFORM=android-19 \
           -DCMAKE_TOOLCHAIN_FILE=${ANDROID_NDK}/build/cmake/android.toolchain.cmake \
-          -DANDROID_NATIVE_API_LEVEL=16 \
           -GNinja ..
 
 Once you've run that, Ninja should produce Android-compatible binaries.  You

crypto/chacha/asm/chacha-x86_64.pl

@@ -82,6 +82,7 @@
 
 .extern OPENSSL_ia32cap_P
 
+.section .rodata
 .align	64
 .Lzero:
 .long	0,0,0,0
@@ -111,6 +112,7 @@
 .Lsixteen:
 .long	16,16,16,16,16,16,16,16,16,16,16,16,16,16,16,16
 .asciz	"ChaCha20 for x86_64, CRYPTOGAMS by <appro\@openssl.org>"
+.text
 ___
 
 sub AUTOLOAD()          # thunk [simplified] 32-bit style perlasm

crypto/cipher_extra/asm/aes128gcmsiv-x86_64.pl

@@ -38,7 +38,7 @@
 *STDOUT=*OUT;
 
 $code.=<<___;
-.data
+.section .rodata
 
 .align 16
 one:

crypto/cipher_extra/asm/chacha20_poly1305_x86_64.pl

@@ -46,6 +46,7 @@
 
 chacha20_poly1305_constants:
 
+.section .rodata
 .align 64
 .Lchacha20_consts:
 .byte 'e','x','p','a','n','d',' ','3','2','-','b','y','t','e',' ','k'
@@ -83,6 +84,7 @@
 .byte 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x00,0x00
 .byte 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x00
 .byte 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff
+.text
 ___
 
 my ($oup,$inp,$inl,$adp,$keyp,$itr1,$itr2,$adl)=("%rdi","%rsi","%rbx","%rcx","%r9","%rcx","%r8","%r8");

crypto/dsa/dsa.c

@@ -589,7 +589,12 @@ static int mod_mul_consttime(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
 }
 
 DSA_SIG *DSA_do_sign(const uint8_t *digest, size_t digest_len, const DSA *dsa) {
-  if (!dsa_check_parameters(dsa)) {
+  if (!dsa_check_key(dsa)) {
+    return NULL;
+  }
+
+  if (dsa->priv_key == NULL) {
+    OPENSSL_PUT_ERROR(DSA, DSA_R_MISSING_PARAMETERS);
     return NULL;
   }
 
@@ -610,6 +615,14 @@ DSA_SIG *DSA_do_sign(const uint8_t *digest, size_t digest_len, const DSA *dsa) {
     goto err;
   }
 
+  // Cap iterations so that invalid parameters do not infinite loop. This does
+  // not impact valid parameters because the probability of requiring even one
+  // retry is negligible, let alone 32. Unfortunately, DSA was mis-specified, so
+  // invalid parameters are reachable from most callers handling untrusted
+  // private keys. (The |dsa_check_key| call above is not sufficient. Checking
+  // whether arbitrary paremeters form a valid DSA group is expensive.)
+  static const int kMaxIterations = 32;
+  int iters = 0;
 redo:
   if (!dsa_sign_setup(dsa, ctx, &kinv, &r)) {
     goto err;
@@ -649,8 +662,14 @@ DSA_SIG *DSA_do_sign(const uint8_t *digest, size_t digest_len, const DSA *dsa) {
   // Redo if r or s is zero as required by FIPS 186-3: this is
   // very unlikely.
   if (BN_is_zero(r) || BN_is_zero(s)) {
+    iters++;
+    if (iters > kMaxIterations) {
+      OPENSSL_PUT_ERROR(DSA, DSA_R_TOO_MANY_ITERATIONS);
+      goto err;
+    }
     goto redo;
   }
+
   ret = DSA_SIG_new();
   if (ret == NULL) {
     goto err;
@@ -684,7 +703,12 @@ int DSA_do_verify(const uint8_t *digest, size_t digest_len, DSA_SIG *sig,
 int DSA_do_check_signature(int *out_valid, const uint8_t *digest,
                            size_t digest_len, DSA_SIG *sig, const DSA *dsa) {
   *out_valid = 0;
-  if (!dsa_check_parameters(dsa)) {
+  if (!dsa_check_key(dsa)) {
+    return 0;
+  }
+
+  if (dsa->pub_key == NULL) {
+    OPENSSL_PUT_ERROR(DSA, DSA_R_MISSING_PARAMETERS);
     return 0;
   }
 
@@ -843,6 +867,10 @@ static size_t der_len_len(size_t len) {
 }
 
 int DSA_size(const DSA *dsa) {
+  if (dsa->q == NULL) {
+    return 0;
+  }
+
   size_t order_len = BN_num_bytes(dsa->q);
   // Compute the maximum length of an |order_len| byte integer. Defensively
   // assume that the leading 0x00 is included.
@@ -865,11 +893,6 @@ int DSA_size(const DSA *dsa) {
 
 static int dsa_sign_setup(const DSA *dsa, BN_CTX *ctx, BIGNUM **out_kinv,
                           BIGNUM **out_r) {
-  if (!dsa->p || !dsa->q || !dsa->g) {
-    OPENSSL_PUT_ERROR(DSA, DSA_R_MISSING_PARAMETERS);
-    return 0;
-  }
-
   int ret = 0;
   BIGNUM k;
   BN_init(&k);

crypto/dsa/dsa_asn1.c

@@ -70,15 +70,25 @@
 // This function is in dsa_asn1.c rather than dsa.c because it is reachable from
 // |EVP_PKEY| parsers. This makes it easier for the static linker to drop most
 // of the DSA implementation.
-int dsa_check_parameters(const DSA *dsa) {
+int dsa_check_key(const DSA *dsa) {
   if (!dsa->p || !dsa->q || !dsa->g) {
     OPENSSL_PUT_ERROR(DSA, DSA_R_MISSING_PARAMETERS);
     return 0;
   }
 
-  // Reject invalid parameters. In particular, signing will infinite loop if |g|
-  // is zero.
-  if (BN_is_zero(dsa->p) || BN_is_zero(dsa->q) || BN_is_zero(dsa->g)) {
+  // Fully checking for invalid DSA groups is expensive, so security and
+  // correctness of the signature scheme depend on how |dsa| was computed. I.e.
+  // we leave "assurance of domain parameter validity" from FIPS 186-4 to the
+  // caller. However, we check bounds on all values to avoid DoS vectors even
+  // when domain parameters are invalid. In particular, signing will infinite
+  // loop if |g| is zero.
+  if (BN_is_negative(dsa->p) || BN_is_negative(dsa->q) || BN_is_zero(dsa->p) ||
+      BN_is_zero(dsa->q) || !BN_is_odd(dsa->p) || !BN_is_odd(dsa->q) ||
+      // |q| must be a prime divisor of |p - 1|, which implies |q < p|.
+      BN_cmp(dsa->q, dsa->p) >= 0 ||
+      // |g| is in the multiplicative group of |p|.
+      BN_is_negative(dsa->g) || BN_is_zero(dsa->g) ||
+      BN_cmp(dsa->g, dsa->p) >= 0) {
     OPENSSL_PUT_ERROR(DSA, DSA_R_INVALID_PARAMETERS);
     return 0;
   }
@@ -97,6 +107,25 @@ int dsa_check_parameters(const DSA *dsa) {
     return 0;
   }
 
+  if (dsa->pub_key != NULL) {
+    // The public key is also in the multiplicative group of |p|.
+    if (BN_is_negative(dsa->pub_key) || BN_is_zero(dsa->pub_key) ||
+        BN_cmp(dsa->pub_key, dsa->p) >= 0) {
+      OPENSSL_PUT_ERROR(DSA, DSA_R_INVALID_PARAMETERS);
+      return 0;
+    }
+  }
+
+  if (dsa->priv_key != NULL) {
+    // The private key is a non-zero element of the scalar field, determined by
+    // |q|.
+    if (BN_is_negative(dsa->priv_key) || BN_is_zero(dsa->priv_key) ||
+        BN_cmp(dsa->priv_key, dsa->q) >= 0) {
+      OPENSSL_PUT_ERROR(DSA, DSA_R_INVALID_PARAMETERS);
+      return 0;
+    }
+  }
+
   return 1;
 }
 
@@ -162,7 +191,7 @@ DSA *DSA_parse_public_key(CBS *cbs) {
     OPENSSL_PUT_ERROR(DSA, DSA_R_DECODE_ERROR);
     goto err;
   }
-  if (!dsa_check_parameters(ret)) {
+  if (!dsa_check_key(ret)) {
     goto err;
   }
   return ret;
@@ -200,7 +229,7 @@ DSA *DSA_parse_parameters(CBS *cbs) {
     OPENSSL_PUT_ERROR(DSA, DSA_R_DECODE_ERROR);
     goto err;
   }
-  if (!dsa_check_parameters(ret)) {
+  if (!dsa_check_key(ret)) {
     goto err;
   }
   return ret;
@@ -251,9 +280,10 @@ DSA *DSA_parse_private_key(CBS *cbs) {
     OPENSSL_PUT_ERROR(DSA, DSA_R_DECODE_ERROR);
     goto err;
   }
-  if (!dsa_check_parameters(ret)) {
+  if (!dsa_check_key(ret)) {
     goto err;
   }
+
   return ret;
 
 err:

crypto/dsa/dsa_test.cc

@@ -69,6 +69,7 @@
 #include <openssl/bn.h>
 #include <openssl/crypto.h>
 #include <openssl/err.h>
+#include <openssl/pem.h>
 #include <openssl/span.h>
 
 #include "../test/test_util.h"
@@ -169,7 +170,7 @@ static const uint8_t fips_sig_bad_r[] = {
     0xdc, 0xd8, 0xc8,
 };
 
-static bssl::UniquePtr<DSA> GetFIPSDSA(void) {
+static bssl::UniquePtr<DSA> GetFIPSDSAGroup(void) {
   bssl::UniquePtr<DSA> dsa(DSA_new());
   if (!dsa) {
     return nullptr;
@@ -184,6 +185,14 @@ static bssl::UniquePtr<DSA> GetFIPSDSA(void) {
   p.release();
   q.release();
   g.release();
+  return dsa;
+}
+
+static bssl::UniquePtr<DSA> GetFIPSDSA(void) {
+  bssl::UniquePtr<DSA> dsa = GetFIPSDSAGroup();
+  if (!dsa) {
+    return nullptr;
+  }
   bssl::UniquePtr<BIGNUM> pub_key(BN_bin2bn(fips_y, sizeof(fips_y), nullptr));
   bssl::UniquePtr<BIGNUM> priv_key(BN_bin2bn(fips_x, sizeof(fips_x), nullptr));
   if (!pub_key || !priv_key ||
@@ -259,3 +268,71 @@ TEST(DSATest, InvalidGroup) {
   EXPECT_EQ(ERR_LIB_DSA, ERR_GET_LIB(err));
   EXPECT_EQ(DSA_R_INVALID_PARAMETERS, ERR_GET_REASON(err));
 }
+
+// Signing and verifying should cleanly fail when the DSA object is empty.
+TEST(DSATest, MissingParameters) {
+  bssl::UniquePtr<DSA> dsa(DSA_new());
+  ASSERT_TRUE(dsa);
+  EXPECT_EQ(-1, DSA_verify(0, fips_digest, sizeof(fips_digest), fips_sig,
+                           sizeof(fips_sig), dsa.get()));
+
+  std::vector<uint8_t> sig(DSA_size(dsa.get()));
+  unsigned sig_len;
+  EXPECT_FALSE(DSA_sign(0, fips_digest, sizeof(fips_digest), sig.data(),
+                        &sig_len, dsa.get()));
+}
+
+// Verifying should cleanly fail when the public key is missing.
+TEST(DSATest, MissingPublic) {
+  bssl::UniquePtr<DSA> dsa = GetFIPSDSAGroup();
+  ASSERT_TRUE(dsa);
+  EXPECT_EQ(-1, DSA_verify(0, fips_digest, sizeof(fips_digest), fips_sig,
+                           sizeof(fips_sig), dsa.get()));
+}
+
+// Signing should cleanly fail when the private key is missing.
+TEST(DSATest, MissingPrivate) {
+  bssl::UniquePtr<DSA> dsa = GetFIPSDSAGroup();
+  ASSERT_TRUE(dsa);
+
+  std::vector<uint8_t> sig(DSA_size(dsa.get()));
+  unsigned sig_len;
+  EXPECT_FALSE(DSA_sign(0, fips_digest, sizeof(fips_digest), sig.data(),
+                        &sig_len, dsa.get()));
+}
+
+// A zero private key is invalid and can cause signing to loop forever.
+TEST(DSATest, ZeroPrivateKey) {
+  bssl::UniquePtr<DSA> dsa = GetFIPSDSA();
+  ASSERT_TRUE(dsa);
+  BN_zero(dsa->priv_key);
+
+  static const uint8_t kZeroDigest[32] = {0};
+  std::vector<uint8_t> sig(DSA_size(dsa.get()));
+  unsigned sig_len;
+  EXPECT_FALSE(DSA_sign(0, kZeroDigest, sizeof(kZeroDigest), sig.data(),
+                        &sig_len, dsa.get()));
+}
+
+// If the "field" is actually a ring and the "generator" of the multiplicative
+// subgroup is actually nilpotent with low degree, DSA signing never completes.
+// Test that we give up in the infinite loop.
+TEST(DSATest, NilpotentGenerator) {
+  static const char kPEM[] = R"(
+-----BEGIN DSA PRIVATE KEY-----
+MGECAQACFQHH+MnFXh4NNlZiV/zUVb5a5ib3kwIVAOP8ZOKvDwabKzEr/moq3y1z
+E3vJAhUAl/2Ylx9fWbzHdh1URsc/c6IM/TECAQECFCsjU4AZRcuks45g1NMOUeCB
+Epvg
+-----END DSA PRIVATE KEY-----
+)";
+  bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(kPEM, sizeof(kPEM)));
+  ASSERT_TRUE(bio);
+  bssl::UniquePtr<DSA> dsa(
+      PEM_read_bio_DSAPrivateKey(bio.get(), nullptr, nullptr, nullptr));
+  ASSERT_TRUE(dsa);
+
+  std::vector<uint8_t> sig(DSA_size(dsa.get()));
+  unsigned sig_len;
+  EXPECT_FALSE(DSA_sign(0, fips_digest, sizeof(fips_digest), sig.data(),
+                        &sig_len, dsa.get()));
+}

crypto/dsa/internal.h

@@ -22,9 +22,9 @@ extern "C" {
 #endif
 
 
-// dsa_check_parameters checks that |dsa|'s group is within DoS bounds. It
-// returns one on success and zero on error.
-int dsa_check_parameters(const DSA *dsa);
+// 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);
 
 
 #if defined(__cplusplus)

crypto/err/dsa.errordata

@@ -6,3 +6,4 @@ DSA,107,INVALID_PARAMETERS
 DSA,101,MISSING_PARAMETERS
 DSA,102,MODULUS_TOO_LARGE
 DSA,103,NEED_NEW_SETUP_VALUES
+DSA,108,TOO_MANY_ITERATIONS

crypto/err/ecdsa.errordata

@@ -5,3 +5,4 @@ ECDSA,101,MISSING_PARAMETERS
 ECDSA,102,NEED_NEW_SETUP_VALUES
 ECDSA,103,NOT_IMPLEMENTED
 ECDSA,104,RANDOM_NUMBER_GENERATION_FAILED
+ECDSA,106,TOO_MANY_ITERATIONS

crypto/evp_extra/evp_tests.txt

@@ -128,6 +128,16 @@ PrivateKey = P-256-ExplicitParameters-CofactorTwo
 Input = 308201610201003081ec06072a8648ce3d02013081e0020101302c06072a8648ce3d0101022100ffffffff00000001000000000000000000000000ffffffffffffffffffffffff30440420ffffffff00000001000000000000000000000000fffffffffffffffffffffffc04205ac635d8aa3a93e7b3ebbd55769886bc651d06b0cc53b0f63bce3c3e27d2604b0441046b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c2964fe342e2fe1a7f9b8ee7eb4a7c0f9e162bce33576b315ececbb6406837bf51f5022100ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551020102046d306b02010104208a872fb62893c4d1ffc5b9f0f91758069f8352e08fa05a49f8db926cb5728725a144034200042c150f429ce70f216c252cf5e062ce1f639cd5d165c7f89424072c27197d78b33b920e95cdb664e990dcf0cfea0d94e2a8e6af9d0e58056e653104925b9fe6c9
 Error = UNKNOWN_GROUP
 
+# A zero ECDSA key, with the optional public key encoded.
+PrivateKey = P-256-Zero
+Input = 3047020100301306072a8648ce3d020106082a8648ce3d030107042d302b02010104200000000000000000000000000000000000000000000000000000000000000000a10403020000
+Error = INVALID_PRIVATE_KEY
+
+# A zero ECDSA key, with the optional public key omitted.
+PrivateKey = P-256-Zero-NoPublic
+Input = 3041020100301306072a8648ce3d020106082a8648ce3d0301070427302502010104200000000000000000000000000000000000000000000000000000000000000000
+Error = INVALID_PRIVATE_KEY
+
 # The public half of the same key encoded as a PublicKey.
 PublicKey = P-256-SPKI
 Type = EC
@@ -159,6 +169,11 @@ Input = 308202650201003082023906072a8648ce3804013082022c02820101009e12fab3de1221
 PKCS8VersionOut = 2
 Error = UNSUPPORTED_ALGORITHM
 
+# An invalid zero DSA private key.
+PrivateKey = DSA-1024-Zero
+Input = 308202450201003082023906072a8648ce3804013082022c02820101009e12fab3de12213501dd82aa10ca2d101d2d4ebfef4d2a3f8daa0fe0cedad8d6af85616aa2f3252c0a2b5a6db09e6f14900e0ddb8311876dd8f9669525f99ed65949e184d5064793271169a228680b95ec12f59a8e20b21f2b58eb2a2012d35bde2ee351822fe8f32d0a330565dcce5c672b7259c14b2433d0b5b2ca2b2db0ab626e8f13f47fe0345d904e7294bb038e9ce21a9e580b83356278706cfe768436c69de149ccff98b4aab8cb4f6385c9f102ce59346eaeef27e0ad222d53d6e89cc8cde5776dd00057b03f2d88ab3cedbafd7b585f0b7f7835e17a3728bbf25ea62572f245dc111f3ce39cb6ffacc31b0a2790e7bde90224ea9b09315362af3d2b022100f381dcf53ebf724f8b2e5ca82c010fb4b5eda9358d0fd88ed278589488b54fc3028201000c402a725dcc3a62e02bf4cf43cd17f4a493591220223669cf4193edab423ad08dfb552e308a6a57a5ffbc7cd0fb2087f81f8df0cb08ab2133287d2b6968714a94f633c940845a48a3e16708dde761cc6a8eab2d84db21b6ea5b07681493cc9c31fbc368b243f6ddf8c932a8b4038f44e7b15ca876344a147859f2b43b39458668ad5e0a1a9a669546dd2812e3b3617a0aef99d58e3bb4cc87fd94225e01d2dcc469a77268146c51918f18e8b4d70aa1f0c7623bcc52cf3731d38641b2d2830b7eecb2f09552ff137d046e494e7f33c3590002b16d1b97d936fda28f90c3ed3ca35338168ac16f77c3c57adc2e8f7c6c2256e41a5f65450590dbb5bcf06d66610403020100
+Error = INVALID_PARAMETERS
+
 # A DSA public key.
 PublicKey = DSA-1024-SPKI
 Type = DSA