Update BoringSSL to f961de5c47ed265c3e758ec70dd15ece20809962 (#115)

Lukasa · web-flow · commit d9825fa541df · 2022-04-21T10:39:48.000+01:00
This patch also cleans up an include issue. The actual code change is in
`scripts/vendor_boringssl.sh`, the rest is just the update.
diff --git a/Package.swift b/Package.swift
@@ -20,7 +20,7 @@
 // Sources/CCryptoBoringSSL directory. The source repository is at
 // https://boringssl.googlesource.com/boringssl.
 //
-// BoringSSL Commit: 2fc6d38391cb76839c76b2a462619e7d69fd998d
+// BoringSSL Commit: f961de5c47ed265c3e758ec70dd15ece20809962
 
 import PackageDescription
 
diff --git a/Sources/CCryptoBoringSSL/crypto/asn1/tasn_dec.c b/Sources/CCryptoBoringSSL/crypto/asn1/tasn_dec.c
@@ -74,8 +74,6 @@
  */
 #define ASN1_MAX_CONSTRUCTED_NEST 30
 
-static int asn1_check_eoc(const unsigned char **in, long len);
-
 static int asn1_check_tlen(long *olen, int *otag, unsigned char *oclass,
                            char *cst, const unsigned char **in, long len,
                            int exptag, int expclass, char opt, ASN1_TLC *ctx);
@@ -373,13 +371,6 @@ static int asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in,
             if (!len)
                 break;
             q = p;
-            /* TODO(https://crbug.com/boringssl/455): Although we've removed
-             * indefinite-length support, this check is not quite a no-op.
-             * Reject [UNIVERSAL 0] in the tag parsers themselves. */
-            if (asn1_check_eoc(&p, len)) {
-                OPENSSL_PUT_ERROR(ASN1, ASN1_R_UNEXPECTED_EOC);
-                goto err;
-            }
             /*
              * This determines the OPTIONAL flag value. The field cannot be
              * omitted if it is the last of a SEQUENCE and there is still
@@ -592,13 +583,6 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val,
         while (len > 0) {
             ASN1_VALUE *skfield;
             const unsigned char *q = p;
-            /* TODO(https://crbug.com/boringssl/455): Although we've removed
-             * indefinite-length support, this check is not quite a no-op.
-             * Reject [UNIVERSAL 0] in the tag parsers themselves. */
-            if (asn1_check_eoc(&p, len)) {
-                OPENSSL_PUT_ERROR(ASN1, ASN1_R_UNEXPECTED_EOC);
-                goto err;
-            }
             skfield = NULL;
              if (!asn1_item_ex_d2i(&skfield, &p, len, ASN1_ITEM_ptr(tt->item),
                                    -1, 0, 0, ctx, depth)) {
@@ -868,21 +852,6 @@ static int asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
     return ret;
 }
 
-/* Check for ASN1 EOC and swallow it if found */
-
-static int asn1_check_eoc(const unsigned char **in, long len)
-{
-    const unsigned char *p;
-    if (len < 2)
-        return 0;
-    p = *in;
-    if (!p[0] && !p[1]) {
-        *in += 2;
-        return 1;
-    }
-    return 0;
-}
-
 /*
  * Check an ASN1 tag and length: a bit like ASN1_get_object but it handles
  * the ASN1_TLC cache and checks the expected tag.
diff --git a/Sources/CCryptoBoringSSL/crypto/bytestring/ber.c b/Sources/CCryptoBoringSSL/crypto/bytestring/ber.c
@@ -93,11 +93,14 @@ static int cbs_find_ber(const CBS *orig_in, int *ber_found, unsigned depth) {
   return 1;
 }
 
-// is_eoc returns true if |header_len| and |contents|, as returned by
-// |CBS_get_any_ber_asn1_element|, indicate an "end of contents" (EOC) value.
-static char is_eoc(size_t header_len, CBS *contents) {
-  return header_len == 2 && CBS_len(contents) == 2 &&
-         OPENSSL_memcmp(CBS_data(contents), "\x00\x00", 2) == 0;
+// cbs_get_eoc returns one if |cbs| begins with an "end of contents" (EOC) value
+// and zero otherwise. If an EOC was found, it advances |cbs| past it.
+static int cbs_get_eoc(CBS *cbs) {
+  if (CBS_len(cbs) >= 2 &&
+      CBS_data(cbs)[0] == 0 && CBS_data(cbs)[1] == 0) {
+    return CBS_skip(cbs, 2);
+  }
+  return 0;
 }
 
 // cbs_convert_ber reads BER data from |in| and writes DER data to |out|. If
@@ -116,21 +119,20 @@ static int cbs_convert_ber(CBS *in, CBB *out, unsigned string_tag,
   }
 
   while (CBS_len(in) > 0) {
+    if (looking_for_eoc && cbs_get_eoc(in)) {
+      return 1;
+    }
+
     CBS contents;
     unsigned tag, child_string_tag = string_tag;
     size_t header_len;
     int indefinite;
     CBB *out_contents, out_contents_storage;
-
     if (!CBS_get_any_ber_asn1_element(in, &contents, &tag, &header_len,
                                       /*out_ber_found=*/NULL, &indefinite)) {
       return 0;
     }
 
-    if (is_eoc(header_len, &contents)) {
-      return looking_for_eoc;
-    }
-
     if (string_tag != 0) {
       // This is part of a constructed string. All elements must match
       // |string_tag| up to the constructed bit and get appended to |out|
diff --git a/Sources/CCryptoBoringSSL/crypto/bytestring/cbs.c b/Sources/CCryptoBoringSSL/crypto/bytestring/cbs.c
@@ -279,6 +279,13 @@ static int parse_asn1_tag(CBS *cbs, unsigned *out) {
 
   tag |= tag_number;
 
+  // Tag [UNIVERSAL 0] is reserved for use by the encoding. Reject it here to
+  // avoid some ambiguity around ANY values and BER indefinite-length EOCs. See
+  // https://crbug.com/boringssl/455.
+  if ((tag & ~CBS_ASN1_CONSTRUCTED) == 0) {
+    return 0;
+  }
+
   *out = tag;
   return 1;
 }
diff --git a/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256-armv8-asm.ios.aarch64.S b/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256-armv8-asm.ios.aarch64.S
@@ -14,7 +14,7 @@
 #if defined(BORINGSSL_PREFIX)
 #include <CCryptoBoringSSL_boringssl_prefix_symbols_asm.h>
 #endif
-#include "openssl/arm_arch.h"
+#include "CCryptoBoringSSL_arm_arch.h"
 
 .text
 .align	5
diff --git a/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256-armv8-asm.linux.aarch64.S b/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256-armv8-asm.linux.aarch64.S
@@ -15,7 +15,7 @@
 #if defined(BORINGSSL_PREFIX)
 #include <CCryptoBoringSSL_boringssl_prefix_symbols_asm.h>
 #endif
-#include "openssl/arm_arch.h"
+#include "CCryptoBoringSSL_arm_arch.h"
 
 .text
 .align	5
diff --git a/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256_beeu-armv8-asm.ios.aarch64.S b/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256_beeu-armv8-asm.ios.aarch64.S
@@ -14,7 +14,7 @@
 #if defined(BORINGSSL_PREFIX)
 #include <CCryptoBoringSSL_boringssl_prefix_symbols_asm.h>
 #endif
-#include "openssl/arm_arch.h"
+#include "CCryptoBoringSSL_arm_arch.h"
 
 .text
 .globl	_beeu_mod_inverse_vartime
diff --git a/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256_beeu-armv8-asm.linux.aarch64.S b/Sources/CCryptoBoringSSL/crypto/fipsmodule/p256_beeu-armv8-asm.linux.aarch64.S
@@ -15,7 +15,7 @@
 #if defined(BORINGSSL_PREFIX)
 #include <CCryptoBoringSSL_boringssl_prefix_symbols_asm.h>
 #endif
-#include "openssl/arm_arch.h"
+#include "CCryptoBoringSSL_arm_arch.h"
 
 .text
 .globl	beeu_mod_inverse_vartime
diff --git a/Sources/CCryptoBoringSSL/crypto/internal.h b/Sources/CCryptoBoringSSL/crypto/internal.h
@@ -126,10 +126,18 @@
 #endif
 
 #if !defined(__cplusplus)
-#if defined(_MSC_VER)
+#if defined(_MSC_VER) && !defined(__clang__)
 #define alignas(x) __declspec(align(x))
 #define alignof __alignof
 #else
+// With the exception of MSVC, we require C11 to build the library. C11 is a
+// prerequisite for improved refcounting performance. All our supported C
+// compilers have long implemented C11 and made it default. The most likely
+// cause of pre-C11 modes is stale -std=c99 or -std=gnu99 flags in build
+// configuration. Such flags can be removed.
+#if __STDC_VERSION__ < 201112L
+#error "BoringSSL must be built in C11 mode or higher."
+#endif
 #include <stdalign.h>
 #endif
 #endif
diff --git a/Sources/CCryptoBoringSSL/hash.txt b/Sources/CCryptoBoringSSL/hash.txt
@@ -1 +1 @@
-This directory is derived from BoringSSL cloned from https://boringssl.googlesource.com/boringssl at revision 2fc6d38391cb76839c76b2a462619e7d69fd998d
+This directory is derived from BoringSSL cloned from https://boringssl.googlesource.com/boringssl at revision f961de5c47ed265c3e758ec70dd15ece20809962
diff --git a/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_bytestring.h b/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_bytestring.h
@@ -265,6 +265,10 @@ OPENSSL_EXPORT int CBS_get_any_asn1_element(CBS *cbs, CBS *out,
 // also true for empty elements so |*out_indefinite| should be checked). If
 // |out_ber_found| is not NULL then it is set to one if any case of invalid DER
 // but valid BER is found, and to zero otherwise.
+//
+// This function will not successfully parse an end-of-contents (EOC) as an
+// element. Callers parsing indefinite-length encoding must check for EOC
+// separately.
 OPENSSL_EXPORT int CBS_get_any_ber_asn1_element(CBS *cbs, CBS *out,
                                                 unsigned *out_tag,
                                                 size_t *out_header_len,
diff --git a/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_span.h b/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_span.h
@@ -99,12 +99,11 @@ class Span : private internal::SpanBase<const T> {
   // Heuristically test whether C is a container type that can be converted into
   // a Span by checking for data() and size() member functions.
   //
-  // TODO(davidben): Require C++14 support and switch to std::enable_if_t.
-  // Perhaps even C++17 now?
+  // TODO(davidben): Require C++17 support for std::is_convertible_v, etc.
   template <typename C>
-  using EnableIfContainer = typename std::enable_if<
+  using EnableIfContainer = std::enable_if_t<
       std::is_convertible<decltype(std::declval<C>().data()), T *>::value &&
-      std::is_integral<decltype(std::declval<C>().size())>::value>::type;
+      std::is_integral<decltype(std::declval<C>().size())>::value>;
 
  public:
   constexpr Span() : Span(nullptr, 0) {}
@@ -113,14 +112,12 @@ class Span : private internal::SpanBase<const T> {
   template <size_t N>
   constexpr Span(T (&array)[N]) : Span(array, N) {}
 
-  template <
-      typename C, typename = EnableIfContainer<C>,
-      typename = typename std::enable_if<std::is_const<T>::value, C>::type>
+  template <typename C, typename = EnableIfContainer<C>,
+            typename = std::enable_if_t<std::is_const<T>::value, C>>
   Span(const C &container) : data_(container.data()), size_(container.size()) {}
 
-  template <
-      typename C, typename = EnableIfContainer<C>,
-      typename = typename std::enable_if<!std::is_const<T>::value, C>::type>
+  template <typename C, typename = EnableIfContainer<C>,
+            typename = std::enable_if_t<!std::is_const<T>::value, C>>
   explicit Span(C &container)
       : data_(container.data()), size_(container.size()) {}
 
diff --git a/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_stack.h b/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_stack.h
@@ -443,16 +443,14 @@ namespace internal {
 
 // Stacks defined with |DEFINE_CONST_STACK_OF| are freed with |sk_free|.
 template <typename Stack>
-struct DeleterImpl<
-    Stack, typename std::enable_if<StackTraits<Stack>::kIsConst>::type> {
+struct DeleterImpl<Stack, std::enable_if_t<StackTraits<Stack>::kIsConst>> {
   static void Free(Stack *sk) { sk_free(reinterpret_cast<_STACK *>(sk)); }
 };
 
 // Stacks defined with |DEFINE_STACK_OF| are freed with |sk_pop_free| and the
 // corresponding type's deleter.
 template <typename Stack>
-struct DeleterImpl<
-    Stack, typename std::enable_if<!StackTraits<Stack>::kIsConst>::type> {
+struct DeleterImpl<Stack, std::enable_if_t<!StackTraits<Stack>::kIsConst>> {
   static void Free(Stack *sk) {
     // sk_FOO_pop_free is defined by macros and bound by name, so we cannot
     // access it from C++ here.
@@ -502,18 +500,17 @@ class StackIteratorImpl {
 };
 
 template <typename Stack>
-using StackIterator = typename std::enable_if<StackTraits<Stack>::kIsStack,
-                                              StackIteratorImpl<Stack>>::type;
+using StackIterator =
+    std::enable_if_t<StackTraits<Stack>::kIsStack, StackIteratorImpl<Stack>>;
 
 }  // namespace internal
 
 // PushToStack pushes |elem| to |sk|. It returns true on success and false on
 // allocation failure.
 template <typename Stack>
-inline
-    typename std::enable_if<!internal::StackTraits<Stack>::kIsConst, bool>::type
-    PushToStack(Stack *sk,
-                UniquePtr<typename internal::StackTraits<Stack>::Type> elem) {
+inline std::enable_if_t<!internal::StackTraits<Stack>::kIsConst, bool>
+PushToStack(Stack *sk,
+            UniquePtr<typename internal::StackTraits<Stack>::Type> elem) {
   if (!sk_push(reinterpret_cast<_STACK *>(sk), elem.get())) {
     return false;
   }
diff --git a/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_type_check.h b/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_type_check.h
@@ -71,7 +71,12 @@ extern "C" {
 // C11 defines the |_Static_assert| keyword and the |static_assert| macro in
 // assert.h. While the former is available at all versions in Clang and GCC, the
 // later depends on libc and, in glibc, depends on being built in C11 mode. We
-// do not require this, for now, so use |_Static_assert| directly.
+// require C11 mode to build the library but, for now, do not require it in
+// public headers. Use |_Static_assert| directly.
+//
+// TODO(davidben): In July 2022, if the C11 change has not been reverted, switch
+// all uses of this macro within the library to C11 |static_assert|. This macro
+// will only be necessary in public headers.
 #define OPENSSL_STATIC_ASSERT(cond, msg) _Static_assert(cond, msg)
 #endif
 
diff --git a/scripts/vendor-boringssl.sh b/scripts/vendor-boringssl.sh
@@ -271,7 +271,7 @@ echo "RENAMING header files"
     rmdir "include/openssl"
 
     # Now change the imports from "<openssl/X> to "<CCryptoBoringSSL_X>", apply the same prefix to the 'boringssl_prefix_symbols' headers.
-    find . -name "*.[ch]" -or -name "*.cc" -or -name "*.S" | xargs $sed -i -e 's+include <openssl/+include <CCryptoBoringSSL_+' -e 's+include <boringssl_prefix_symbols+include <CCryptoBoringSSL_boringssl_prefix_symbols+'
+    find . -name "*.[ch]" -or -name "*.cc" -or -name "*.S" | xargs $sed -i -e 's+include <openssl/+include <CCryptoBoringSSL_+' -e 's+include <boringssl_prefix_symbols+include <CCryptoBoringSSL_boringssl_prefix_symbols+' -e 's+include "openssl/+include "CCryptoBoringSSL_+'
 
     # Okay now we need to rename the headers adding the prefix "CCryptoBoringSSL_".
     pushd include

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-This directory is derived from BoringSSL cloned from https://boringssl.googlesource.com/boringssl at revision 2fc6d38391cb76839c76b2a462619e7d69fd998d`
	`1`	`+This directory is derived from BoringSSL cloned from https://boringssl.googlesource.com/boringssl at revision f961de5c47ed265c3e758ec70dd15ece20809962`