Upstream merge 2023 08 09

SANDBOXING.md

@@ -90,8 +90,10 @@ addition to the operations above.
 
 On Linux ARM platforms, BoringSSL depends on OS APIs to query CPU capabilities.
 32-bit and 64-bit ARM both depend on the `getauxval` function. 32-bit ARM, to
-work around bugs in older Android devices, may additionally read `/proc/cpuinfo`
-and `/proc/self/auxv`.
+work around bugs in older Android devices, may additionally read
+`/proc/cpuinfo`.
+
+On 64-bit Apple ARM platforms, BoringSSL needs to query `hw.optional.*` sysctls.
 
 If querying CPU capabilities fails, BoringSSL will still function, but may not
 perform as well.

crypto/CMakeLists.txt

@@ -431,6 +431,7 @@ add_library(
   rc4/rc4.c
   refcount_c11.c
   refcount_lock.c
+  refcount_win.c
   rsa_extra/rsa_asn1.c
   rsa_extra/rsassa_pss_asn1.c
   rsa_extra/rsa_print.c

crypto/fipsmodule/cpucap/cpu_aarch64_apple.c

@@ -86,6 +86,9 @@ void OPENSSL_cpuid_setup(void) {
   // available in macOS 12. For compatibility with macOS 11, we also support
   // the old names. The old names don't have values for features like FEAT_AES,
   // so instead we detect them statically above.
+  //
+  // If querying new sysctls, update the Chromium sandbox definition. See
+  // https://crrev.com/c/4415225.
   if (has_hw_feature("hw.optional.arm.FEAT_SHA512") ||
       has_hw_feature("hw.optional.armv8_2_sha512")) {
     OPENSSL_armcap_P |= ARMV8_SHA512;

crypto/fipsmodule/hkdf/hkdf.c

@@ -123,7 +123,7 @@ int HKDF_expand(uint8_t *out_key, size_t out_len, const EVP_MD *digest,
     }
 
     todo = digest_len;
-    if (done + todo > out_len) {
+    if (todo > out_len - done) {
       todo = out_len - done;
     }
     OPENSSL_memcpy(out_key + done, previous, todo);

crypto/fipsmodule/rand/rand.c

@@ -52,14 +52,14 @@
 // This might be a bit of a leap of faith, esp on Windows, but there's nothing
 // that we can do about it.)
 
-// When in FIPS mode we use the CPU Jitter entropy source to seed our DRBG.  
+// When in FIPS mode we use the CPU Jitter entropy source to seed our DRBG.
 // This entropy source is very slow and can incur a cost anywhere between 10-60ms
-// depending on configuration and CPU.  Increasing to 2^24 will amortize the 
-// penalty over more requests.  This is the same value used in OpenSSL 3.0  
+// depending on configuration and CPU.  Increasing to 2^24 will amortize the
+// penalty over more requests.  This is the same value used in OpenSSL 3.0
 // and meets the requirements defined in SP 800-90B for a max reseed of interval (2^48)
 //
 // CPU Jitter:  https://www.chronox.de/jent/doc/CPU-Jitter-NPTRNG.html
-// 
+//
 // kReseedInterval is the number of generate calls made to CTR-DRBG before
 // reseeding.
 
@@ -94,6 +94,9 @@ struct rand_thread_state {
   // calls is the number of generate calls made on |drbg| since it was last
   // (re)seeded. This is bound by |kReseedInterval|.
   unsigned calls;
+  // fork_unsafe_buffering is non-zero iff, when |drbg| was last (re)seeded,
+  // fork-unsafe buffering was enabled.
+  int fork_unsafe_buffering;
 
 #if defined(BORINGSSL_FIPS)
   // next and prev form a NULL-terminated, double-linked list of all states in
@@ -375,6 +378,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
   }
 
   const uint64_t fork_generation = CRYPTO_get_fork_generation();
+  const int fork_unsafe_buffering = rand_fork_unsafe_buffering_enabled();
 
   // Additional data is mixed into every CTR-DRBG call to protect, as best we
   // can, against forks & VM clones. We do not over-read this information and
@@ -389,7 +393,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
     // entropy is used. This can be expensive (one read per |RAND_bytes| call)
     // and so is disabled when we have fork detection, or if the application has
     // promised not to fork.
-    if (fork_generation != 0 || rand_fork_unsafe_buffering_enabled()) {
+    if (fork_generation != 0 || fork_unsafe_buffering) {
       OPENSSL_memset(additional_data, 0, sizeof(additional_data));
     } else if (!have_rdrand()) {
       // No alternative so block for OS entropy.
@@ -441,6 +445,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
     }
     state->calls = 0;
     state->fork_generation = fork_generation;
+    state->fork_unsafe_buffering = fork_unsafe_buffering;
 
 #if defined(BORINGSSL_FIPS)
     if (state != &stack_state) {
@@ -460,7 +465,14 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
   }
 
   if (state->calls >= kReseedInterval ||
-      state->fork_generation != fork_generation) {
+      // If we've forked since |state| was last seeded, reseed.
+      state->fork_generation != fork_generation ||
+      // If |state| was seeded from a state with different fork-safety
+      // preferences, reseed. Suppose |state| was fork-safe, then forked into
+      // two children, but each of the children never fork and disable fork
+      // safety. The children must reseed to avoid working from the same PRNG
+      // state.
+      state->fork_unsafe_buffering != fork_unsafe_buffering) {
     uint8_t seed[CTR_DRBG_ENTROPY_LEN];
     int want_additional_input;
     rand_get_seed(state, seed, &want_additional_input);
@@ -487,6 +499,7 @@ void RAND_bytes_with_additional_data(uint8_t *out, size_t out_len,
     }
     state->calls = 0;
     state->fork_generation = fork_generation;
+    state->fork_unsafe_buffering = fork_unsafe_buffering;
     OPENSSL_cleanse(seed, CTR_DRBG_ENTROPY_LEN);
     OPENSSL_cleanse(add_data_for_reseed, CTR_DRBG_ENTROPY_LEN);
   } else {

crypto/internal.h

@@ -494,6 +494,14 @@ OPENSSL_EXPORT void CRYPTO_once(CRYPTO_once_t *once, void (*init)(void));
 #define OPENSSL_C11_ATOMIC
 #endif
 
+// Older MSVC does not support C11 atomics, so we fallback to the Windows APIs.
+// This can be removed once we can rely on
+// https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/
+#if !defined(OPENSSL_C11_ATOMIC) && defined(OPENSSL_THREADS) && \
+    defined(OPENSSL_WINDOWS)
+#define OPENSSL_WINDOWS_ATOMIC
+#endif
+
 // CRYPTO_REFCOUNT_MAX is the value at which the reference count saturates.
 #define CRYPTO_REFCOUNT_MAX 0xffffffff
 

crypto/rand_extra/rand_test.cc

@@ -66,7 +66,7 @@ TEST(RandTest, NotObviouslyBroken) {
 
 #if !defined(OPENSSL_WINDOWS) && !defined(OPENSSL_IOS) && \
     !defined(OPENSSL_FUCHSIA) && !defined(BORINGSSL_UNSAFE_DETERMINISTIC_MODE)
-static bool ForkAndRand(bssl::Span<uint8_t> out) {
+static bool ForkAndRand(bssl::Span<uint8_t> out, bool fork_unsafe_buffering) {
   int pipefds[2];
   if (pipe(pipefds) < 0) {
     perror("pipe");
@@ -86,6 +86,9 @@ static bool ForkAndRand(bssl::Span<uint8_t> out) {
   if (child == 0) {
     // This is the child. Generate entropy and write it to the parent.
     close(pipefds[0]);
+    if (fork_unsafe_buffering) {
+      RAND_enable_fork_unsafe_buffering(-1);
+    }
     RAND_bytes(out.data(), out.size());
     while (!out.empty()) {
       ssize_t ret = write(pipefds[1], out.data(), out.size());
@@ -148,18 +151,27 @@ TEST(RandTest, Fork) {
   // intentionally uses smaller buffers than the others, to minimize the chance
   // of sneaking by with a large enough buffer that we've since reseeded from
   // the OS.
-  uint8_t buf1[16], buf2[16], buf3[16];
-  ASSERT_TRUE(ForkAndRand(buf1));
-  ASSERT_TRUE(ForkAndRand(buf2));
-  RAND_bytes(buf3, sizeof(buf3));
-
-  // All should be different.
-  EXPECT_NE(Bytes(buf1), Bytes(buf2));
-  EXPECT_NE(Bytes(buf2), Bytes(buf3));
-  EXPECT_NE(Bytes(buf1), Bytes(buf3));
-  EXPECT_NE(Bytes(buf1), Bytes(kZeros));
-  EXPECT_NE(Bytes(buf2), Bytes(kZeros));
-  EXPECT_NE(Bytes(buf3), Bytes(kZeros));
+  //
+  // All child processes should have different PRNGs, including the ones that
+  // disavow fork-safety. Although they are produced by fork, they themselves do
+  // not fork after that call.
+  uint8_t bufs[5][16];
+  ASSERT_TRUE(ForkAndRand(bufs[0], /*fork_unsafe_buffering=*/false));
+  ASSERT_TRUE(ForkAndRand(bufs[1], /*fork_unsafe_buffering=*/false));
+  ASSERT_TRUE(ForkAndRand(bufs[2], /*fork_unsafe_buffering=*/true));
+  ASSERT_TRUE(ForkAndRand(bufs[3], /*fork_unsafe_buffering=*/true));
+  RAND_bytes(bufs[4], sizeof(bufs[4]));
+
+  // All should be different and non-zero.
+  for (const auto &buf : bufs) {
+    EXPECT_NE(Bytes(buf), Bytes(kZeros));
+  }
+  for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(bufs); i++) {
+    for (size_t j = 0; j < i; j++) {
+      EXPECT_NE(Bytes(bufs[i]), Bytes(bufs[j]))
+          << "buffers " << i << " and " << j << " matched";
+    }
+  }
 }
 #endif  // !OPENSSL_WINDOWS && !OPENSSL_IOS &&
         // !OPENSSL_FUCHSIA && !BORINGSSL_UNSAFE_DETERMINISTIC_MODE

crypto/refcount_lock.c

@@ -19,7 +19,7 @@
 #include <openssl/type_check.h>
 
 
-#if !defined(OPENSSL_C11_ATOMIC)
+#if !defined(OPENSSL_C11_ATOMIC) && !defined(OPENSSL_WINDOWS_ATOMIC)
 
 OPENSSL_STATIC_ASSERT((CRYPTO_refcount_t)-1 == CRYPTO_REFCOUNT_MAX,
                       CRYPTO_REFCOUNT_MAX_is_incorrect)
@@ -50,4 +50,4 @@ int CRYPTO_refcount_dec_and_test_zero(CRYPTO_refcount_t *count) {
   return ret;
 }
 
-#endif  // OPENSSL_C11_ATOMIC
+#endif  // !OPENSSL_C11_ATOMIC && !OPENSSL_WINDOWS_ATOMICS

crypto/refcount_win.c

@@ -0,0 +1,89 @@
+/* Copyright (c) 2023, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+#include "internal.h"
+
+#if defined(OPENSSL_WINDOWS_ATOMIC)
+
+#include <windows.h>
+
+
+// See comment above the typedef of CRYPTO_refcount_t about these tests.
+static_assert(alignof(CRYPTO_refcount_t) == alignof(LONG),
+              "CRYPTO_refcount_t does not match LONG alignment");
+static_assert(sizeof(CRYPTO_refcount_t) == sizeof(LONG),
+              "CRYPTO_refcount_t does not match LONG size");
+
+static_assert((CRYPTO_refcount_t)-1 == CRYPTO_REFCOUNT_MAX,
+              "CRYPTO_REFCOUNT_MAX is incorrect");
+
+static uint32_t atomic_load_u32(volatile LONG *ptr) {
+  // This is not ideal because it still writes to a cacheline. MSVC is not able
+  // to optimize this to a true atomic read, and Windows does not provide an
+  // InterlockedLoad function.
+  //
+  // The Windows documentation [1] does say "Simple reads and writes to
+  // properly-aligned 32-bit variables are atomic operations", but this is not
+  // phrased in terms of the C11 and C++11 memory models, and indeed a read or
+  // write seems to produce slightly different code on MSVC than a sequentially
+  // consistent std::atomic::load in C++. Moreover, it is unclear if non-MSVC
+  // compilers on Windows provide the same guarantees. Thus we avoid relying on
+  // this and instead still use an interlocked function. This is still
+  // preferable a global mutex, and eventually this code will be replaced by
+  // [2]. Additionally, on clang-cl, we'll use the |OPENSSL_C11_ATOMIC| path.
+  //
+  // [1] https://learn.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access
+  // [2] https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/
+  return (uint32_t)InterlockedCompareExchange(ptr, 0, 0);
+}
+
+static int atomic_compare_exchange_u32(volatile LONG *ptr, uint32_t *expected32,
+                                       uint32_t desired) {
+  LONG expected = (LONG)*expected32;
+  LONG actual = InterlockedCompareExchange(ptr, (LONG)desired, expected);
+  *expected32 = (uint32_t)actual;
+  return actual == expected;
+}
+
+void CRYPTO_refcount_inc(CRYPTO_refcount_t *in_count) {
+  volatile LONG *count = (volatile LONG *)in_count;
+  uint32_t expected = atomic_load_u32(count);
+
+  while (expected != CRYPTO_REFCOUNT_MAX) {
+    const uint32_t new_value = expected + 1;
+    if (atomic_compare_exchange_u32(count, &expected, new_value)) {
+      break;
+    }
+  }
+}
+
+int CRYPTO_refcount_dec_and_test_zero(CRYPTO_refcount_t *in_count) {
+  volatile LONG *count = (volatile LONG *)in_count;
+  uint32_t expected = atomic_load_u32(count);
+
+  for (;;) {
+    if (expected == 0) {
+      abort();
+    } else if (expected == CRYPTO_REFCOUNT_MAX) {
+      return 0;
+    } else {
+      const uint32_t new_value = expected - 1;
+      if (atomic_compare_exchange_u32(count, &expected, new_value)) {
+        return new_value == 0;
+      }
+    }
+  }
+}
+
+#endif  // OPENSSL_WINDOWS_ATOMIC

crypto/stack/stack.c

@@ -57,7 +57,9 @@
 #include <openssl/stack.h>
 
 #include <assert.h>
+#include <limits.h>
 
+#include <openssl/err.h>
 #include <openssl/mem.h>
 
 #include "../internal.h"
@@ -161,6 +163,11 @@ size_t sk_insert(_STACK *sk, void *p, size_t where) {
     return 0;
   }
 
+  if (sk->num >= INT_MAX) {
+    OPENSSL_PUT_ERROR(CRYPTO, ERR_R_OVERFLOW);
+    return 0;
+  }
+
   if (sk->num_alloc <= sk->num + 1) {
     // Attempt to double the size of the array.
     size_t new_alloc = sk->num_alloc << 1;

include/openssl/stack.h

@@ -140,7 +140,8 @@ STACK_OF(SAMPLE) *sk_SAMPLE_new(sk_SAMPLE_cmp_func comp);
 // NULL on allocation failure.
 STACK_OF(SAMPLE) *sk_SAMPLE_new_null(void);
 
-// sk_SAMPLE_num returns the number of elements in |sk|.
+// sk_SAMPLE_num returns the number of elements in |sk|. It is safe to cast this
+// value to |int|. |sk| is guaranteed to have at most |INT_MAX| elements.
 size_t sk_SAMPLE_num(const STACK_OF(SAMPLE) *sk);
 
 // sk_SAMPLE_zero resets |sk| to the empty state but does nothing to free the

include/openssl/x509.h

@@ -2785,6 +2785,11 @@ OPENSSL_EXPORT void X509_STORE_set_verify(X509_STORE *ctx,
 OPENSSL_EXPORT void X509_STORE_CTX_set_verify(X509_STORE_CTX *ctx,
                                               X509_STORE_CTX_verify_fn verify);
 OPENSSL_EXPORT X509_STORE_CTX_verify_fn X509_STORE_get_verify(X509_STORE *ctx);
+
+// X509_STORE_set_verify_cb acts like |X509_STORE_CTX_set_verify_cb| but sets
+// the verify callback for any |X509_STORE_CTX| created from this |X509_STORE|
+//
+// Do not use this funciton. see |X509_STORE_CTX_set_verify_cb|.
 OPENSSL_EXPORT void X509_STORE_set_verify_cb(
     X509_STORE *ctx, X509_STORE_CTX_verify_cb verify_cb);
 #define X509_STORE_set_verify_cb_func(ctx, func) \
@@ -2935,8 +2940,27 @@ OPENSSL_EXPORT void X509_STORE_CTX_set_time(X509_STORE_CTX *ctx,
 OPENSSL_EXPORT void X509_STORE_CTX_set_time_posix(X509_STORE_CTX *ctx,
                                                   unsigned long flags,
                                                   int64_t t);
+
+// X509_STORE_CTX_set_verify_cb configures a callback function for |ctx| that is
+// called multiple times during |X509_verify_cert|. The callback returns zero to
+// fail verification and non-zero to proceed. Typically, it will return |ok|,
+// which preserves the default behavior. Returning one when |ok| is zero will
+// proceed past some error. The callback may inspect |ctx| and the error queue
+// to attempt to determine the current stage of certificate verification, but
+// this is often unreliable.
+//
+// WARNING: Do not use this function. It is extremely fragile and unpredictable.
+// This callback exposes implementation details of certificate verification,
+// which change as the library evolves. Attempting to use it for security checks
+// can introduce vulnerabilities if making incorrect assumptions about when the
+// callback is called. Additionally, overriding |ok| may leave |ctx| in an
+// inconsistent state and break invariants.
+//
+// Instead, customize certificate verification by configuring options on the
+// |X509_STORE_CTX| before verification, or applying additional checks after
+// |X509_verify_cert| completes successfully.
 OPENSSL_EXPORT void X509_STORE_CTX_set_verify_cb(
-    X509_STORE_CTX *ctx, int (*verify_cb)(int, X509_STORE_CTX *));
+    X509_STORE_CTX *ctx, int (*verify_cb)(int ok, X509_STORE_CTX *ctx));
 
 OPENSSL_EXPORT X509_VERIFY_PARAM *X509_STORE_CTX_get0_param(
     X509_STORE_CTX *ctx);