Merge branch 'aws:main' into supportcallbacks

.github/workflows/integrations.yml

@@ -175,6 +175,21 @@ jobs:
       - name: Run strongswan build
         run: |
           ./tests/ci/integration/run_strongswan_integration.sh
+  openvpn:
+    if: github.repository_owner == 'aws'
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install OS Dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get -y --no-install-recommends install \
+          cmake gcc ninja-build golang libnl-3-dev libnl-genl-3-dev \
+          libcap-ng-dev liblz4-dev liblzo2-dev libpam-dev libcmocka-dev \
+          python3-docutils
+      - uses: actions/checkout@v4
+      - name: Run openvpn build
+        run: |
+          ./tests/ci/integration/run_openvpn_integration.sh
   libevent:
     if: github.repository_owner == 'aws'
     runs-on: ubuntu-latest

.github/workflows/opensslcomparison.yml

@@ -0,0 +1,27 @@
+name: OpenSSL CLI Comparison Tests
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  openssl_comparison_tests:
+    if: github.repository_owner == 'aws'
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      - name: Install OS Dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get -y --no-install-recommends install \
+            cmake gcc ninja-build golang make autoconf pkg-config openssl
+
+      - name: Make the script executable
+        run: chmod +x ./tests/ci/run_openssl_comparison_tests.sh
+
+      - name: Build AWS-LC & OpenSSL and Run Comparison Tests
+        run: |
+            ./tests/ci/run_openssl_comparison_tests.sh

BUILDING.md

@@ -62,13 +62,12 @@ Using Make (does not work on Windows):
     cmake -B build
     make -C build
 
-You usually don't need to run `cmake` again after changing `CMakeLists.txt`
-files because the build scripts will detect changes to them and rebuild
-themselves automatically.
+This produces a debug build by default. Optimisation isn't enabled, and debug
+assertions are included. Pass `-DCMAKE_BUILD_TYPE=Release` to `cmake` to
+configure a release build:
 
-Note that the default build flags in the top-level `CMakeLists.txt` are for
-debugging—optimisation isn't enabled. Pass `-DCMAKE_BUILD_TYPE=Release` to
-`cmake` to configure a release build.
+    cmake -GNinja -B build -DCMAKE_BUILD_TYPE=Release
+    ninja -C build
 
 If you want to cross-compile then there is an example toolchain file for 32-bit
 Intel in `util/`. Wipe out the build directory, run `cmake` like this:
@@ -87,6 +86,10 @@ remove some code that is especially large.
 See [CMake's documentation](https://cmake.org/cmake/help/v3.4/manual/cmake-variables.7.html)
 for other variables which may be used to configure the build.
 
+You usually don't need to run `cmake` again after changing `CMakeLists.txt`
+files because the build scripts will detect changes to them and rebuild
+themselves automatically.
+
 ### Building for Android
 
 It's possible to build BoringSSL with the Android NDK using CMake. Recent

crypto/CMakeLists.txt

@@ -298,6 +298,7 @@ if(GO_EXECUTABLE)
     err/engine.errordata
     err/evp.errordata
     err/hkdf.errordata
+    err/hmac.errordata
     err/obj.errordata
     err/ocsp.errordata
     err/pem.errordata

crypto/bytestring/ber.c

@@ -18,13 +18,10 @@
 #include <string.h>
 
 #include "internal.h"
-#include "../internal.h"
 
 
-// kMaxDepth is a just a sanity limit. The code should be such that the length
-// of the input being processes always decreases. None the less, a very large
-// input could otherwise cause the stack to overflow.
-static const uint32_t kMaxDepth = 2048;
+// kMaxDepth limits the recursion depth to avoid overflowing the stack.
+static const uint32_t kMaxDepth = 128;
 
 // is_string_type returns one if |tag| is a string type and zero otherwise. It
 // ignores the constructed bit.
@@ -56,13 +53,11 @@ static int is_string_type(CBS_ASN1_TAG tag) {
 // found. The value of |orig_in| is not changed. It returns one on success (i.e.
 // |*ber_found| was set) and zero on error.
 static int cbs_find_ber(const CBS *orig_in, int *ber_found, uint32_t depth) {
-  CBS in;
-
   if (depth > kMaxDepth) {
     return 0;
   }
 
-  CBS_init(&in, CBS_data(orig_in), CBS_len(orig_in));
+  CBS in = *orig_in;
   *ber_found = 0;
 
   while (CBS_len(&in) > 0) {
@@ -87,6 +82,10 @@ static int cbs_find_ber(const CBS *orig_in, int *ber_found, uint32_t depth) {
           !cbs_find_ber(&contents, ber_found, depth + 1)) {
         return 0;
       }
+      if (*ber_found) {
+        // We already found BER. No need to continue parsing.
+        return 1;
+      }
     }
   }
 

crypto/bytestring/bytestring_test.cc

@@ -679,6 +679,38 @@ TEST(CBSTest, BerConvert) {
       0xa0, 0x08, 0x04, 0x02, 0x00, 0x01, 0x04, 0x02, 0x02, 0x03,
   };
 
+  // kWrappedIndefBER contains indefinite-length SEQUENCE, wrapped
+  // and followed by valid DER. This tests that we correctly identify BER nested
+  // inside DER.
+  //
+  //  SEQUENCE {
+  //    SEQUENCE {
+  //      SEQUENCE indefinite {}
+  //    }
+  //    SEQUENCE {}
+  //  }
+  static const uint8_t kWrappedIndefBER[] = {0x30, 0x08, 0x30, 0x04, 0x30,
+                                             0x80, 0x00, 0x00, 0x30, 0x00};
+  static const uint8_t kWrappedIndefDER[] = {0x30, 0x06, 0x30, 0x02,
+                                             0x30, 0x00, 0x30, 0x00};
+
+  // kWrappedConstructedStringBER contains a constructed OCTET STRING, wrapped
+  // and followed by valid DER. This tests that we correctly identify BER nested
+  // inside DER.
+  //
+  //  SEQUENCE {
+  //    SEQUENCE {
+  //      [OCTET_STRING CONSTRUCTED] {
+  //        OCTET_STRING {}
+  //      }
+  //    }
+  //    SEQUENCE {}
+  //  }
+  static const uint8_t kWrappedConstructedStringBER[] = {
+      0x30, 0x08, 0x30, 0x04, 0x24, 0x02, 0x04, 0x00, 0x30, 0x00};
+  static const uint8_t kWrappedConstructedStringDER[] = {
+      0x30, 0x06, 0x30, 0x02, 0x04, 0x00, 0x30, 0x00};
+
   // kConstructedBitString contains a BER constructed BIT STRING. These are not
   // supported and thus are left unchanged.
   static const uint8_t kConstructedBitStringBER[] = {
@@ -695,6 +727,25 @@ TEST(CBSTest, BerConvert) {
                    kConstructedStringBER);
   ExpectBerConvert("kConstructedBitStringBER", kConstructedBitStringBER,
                    kConstructedBitStringBER);
+  ExpectBerConvert("kWrappedIndefBER", kWrappedIndefDER, kWrappedIndefBER);
+  ExpectBerConvert("kWrappedConstructedStringBER", kWrappedConstructedStringDER,
+                   kWrappedConstructedStringBER);
+
+  // indef_overflow is 200 levels deep of an indefinite-length-encoded SEQUENCE.
+  // This will exceed our recursion limits and fail to be converted.
+  std::vector<uint8_t> indef_overflow;
+  for (int i = 0; i < 200; i++) {
+    indef_overflow.push_back(0x30);
+    indef_overflow.push_back(0x80);
+  }
+  for (int i = 0; i < 200; i++) {
+    indef_overflow.push_back(0x00);
+    indef_overflow.push_back(0x00);
+  }
+  CBS in, out;
+  CBS_init(&in, indef_overflow.data(), indef_overflow.size());
+  uint8_t *storage;
+  ASSERT_FALSE(CBS_asn1_ber_to_der(&in, &out, &storage));
 }
 
 struct BERTest {

crypto/cipher_extra/asm/aes128gcmsiv-x86_64.pl

@@ -1263,14 +1263,15 @@ sub aesgcmsiv_dec {
 .L${labelPrefix}_dec_start:
     vzeroupper
     vmovdqa ($POL), $T
+    # The claimed tag is provided after the current calculated tag value.
+    # CTRBLKs is made from it.
+    vmovdqu 16($POL), $CTR
+    vpor OR_MASK(%rip), $CTR, $CTR      # CTR = [1]TAG[126...32][00..00]
     movq $POL, $secureBuffer
 
     leaq 32($secureBuffer), $secureBuffer
     leaq 32($Htbl), $Htbl
 
-    # make CTRBLKs from given tag.
-    vmovdqu ($CT,$LEN), $CTR
-    vpor OR_MASK(%rip), $CTR, $CTR      # CTR = [1]TAG[126...32][00..00]
     andq \$~15, $LEN
 
     # If less then 6 blocks, make singles

crypto/cipher_extra/e_aesgcmsiv.c

@@ -135,16 +135,16 @@ extern void aesgcmsiv_htable_polyval(const uint8_t htable[16 * 8],
                                      uint8_t in_out_poly[16]);
 
 // aes128gcmsiv_dec decrypts |in_len| & ~15 bytes from |out| and writes them to
-// |in|. (The full value of |in_len| is still used to find the authentication
-// tag appended to the ciphertext, however, so must not be pre-masked.)
+// |in|. |in| and |out| may be equal, but must not otherwise alias.
 //
-// |in| and |out| may be equal, but must not otherwise overlap.
+// |in_out_calculated_tag_and_scratch|, on entry, must contain:
+//    1. The current value of the calculated tag, which will be updated during
+//       decryption and written back to the beginning of this buffer on exit.
+//    2. The claimed tag, which is needed to derive counter values.
 //
-// While decrypting, it updates the POLYVAL value found at the beginning of
-// |in_out_calculated_tag_and_scratch| and writes the updated value back before
-// return. During executation, it may use the whole of this space for other
-// purposes. In order to decrypt and update the POLYVAL value, it uses the
-// expanded key from |key| and the table of powers in |htable|.
+// While decrypting, the whole of |in_out_calculated_tag_and_scratch| may be
+// used for other purposes. In order to decrypt and update the POLYVAL value, it
+// uses the expanded key from |key| and the table of powers in |htable|.
 extern void aes128gcmsiv_dec(const uint8_t *in, uint8_t *out,
                              uint8_t in_out_calculated_tag_and_scratch[16 * 8],
                              const uint8_t htable[16 * 6],
@@ -405,23 +405,19 @@ static int aead_aes_gcm_siv_asm_seal_scatter(
   return 1;
 }
 
-// TODO(martinkr): Add aead_aes_gcm_siv_asm_open_gather. N.B. aes128gcmsiv_dec
-// expects ciphertext and tag in a contiguous buffer.
-
-static int aead_aes_gcm_siv_asm_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
-                                     size_t *out_len, size_t max_out_len,
-                                     const uint8_t *nonce, size_t nonce_len,
-                                     const uint8_t *in, size_t in_len,
-                                     const uint8_t *ad, size_t ad_len) {
+static int aead_aes_gcm_siv_asm_open_gather(
+    const EVP_AEAD_CTX *ctx, uint8_t *out, const uint8_t *nonce,
+    size_t nonce_len, const uint8_t *in, size_t in_len, const uint8_t *in_tag,
+    size_t in_tag_len, const uint8_t *ad, size_t ad_len) {
   const uint64_t ad_len_64 = ad_len;
   if (ad_len_64 >= (UINT64_C(1) << 61)) {
     OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_TOO_LARGE);
     return 0;
   }
 
   const uint64_t in_len_64 = in_len;
-  if (in_len < EVP_AEAD_AES_GCM_SIV_TAG_LEN ||
-      in_len_64 > (UINT64_C(1) << 36) + AES_BLOCK_SIZE) {
+  if (in_len_64 > UINT64_C(1) << 36 ||
+      in_tag_len != EVP_AEAD_AES_GCM_SIV_TAG_LEN) {
     OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BAD_DECRYPT);
     return 0;
   }
@@ -436,13 +432,6 @@ static int aead_aes_gcm_siv_asm_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
     OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_ALIGNMENT_CHANGED);
     return 0;
   }
-  const size_t plaintext_len = in_len - EVP_AEAD_AES_GCM_SIV_TAG_LEN;
-  const uint8_t *const given_tag = in + plaintext_len;
-
-  if (max_out_len < plaintext_len) {
-    OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BUFFER_TOO_SMALL);
-    return 0;
-  }
 
   alignas(16) uint64_t record_auth_key[2];
   alignas(16) uint64_t record_enc_key[4];
@@ -475,27 +464,27 @@ static int aead_aes_gcm_siv_asm_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
   alignas(16) uint8_t htable[16 * 6];
   aesgcmsiv_htable6_init(htable, (const uint8_t *)record_auth_key);
 
+  // aes[128|256]gcmsiv_dec needs access to the claimed tag. So it's put into
+  // its scratch space.
+  memcpy(calculated_tag + 16, in_tag, EVP_AEAD_AES_GCM_SIV_TAG_LEN);
   if (gcm_siv_ctx->is_128_bit) {
-    aes128gcmsiv_dec(in, out, calculated_tag, htable, &expanded_key,
-                     plaintext_len);
+    aes128gcmsiv_dec(in, out, calculated_tag, htable, &expanded_key, in_len);
   } else {
-    aes256gcmsiv_dec(in, out, calculated_tag, htable, &expanded_key,
-                     plaintext_len);
+    aes256gcmsiv_dec(in, out, calculated_tag, htable, &expanded_key, in_len);
   }
 
-  if (plaintext_len & 15) {
+  if (in_len & 15) {
     aead_aes_gcm_siv_asm_crypt_last_block(gcm_siv_ctx->is_128_bit, out, in,
-                                          plaintext_len, given_tag,
-                                          &expanded_key);
+                                          in_len, in_tag, &expanded_key);
     OPENSSL_memset(scratch, 0, sizeof(scratch));
-    OPENSSL_memcpy(scratch, out + (plaintext_len & ~15), plaintext_len & 15);
+    OPENSSL_memcpy(scratch, out + (in_len & ~15), in_len & 15);
     aesgcmsiv_polyval_horner(calculated_tag, (const uint8_t *)record_auth_key,
                              scratch, 1);
   }
 
   uint8_t length_block[16];
   CRYPTO_store_u64_le(length_block, ad_len * 8);
-  CRYPTO_store_u64_le(length_block + 8, plaintext_len * 8);
+  CRYPTO_store_u64_le(length_block + 8, in_len * 8);
   aesgcmsiv_polyval_horner(calculated_tag, (const uint8_t *)record_auth_key,
                            length_block, 1);
 
@@ -511,13 +500,12 @@ static int aead_aes_gcm_siv_asm_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
     aes256gcmsiv_ecb_enc_block(calculated_tag, calculated_tag, &expanded_key);
   }
 
-  if (CRYPTO_memcmp(calculated_tag, given_tag, EVP_AEAD_AES_GCM_SIV_TAG_LEN) !=
+  if (CRYPTO_memcmp(calculated_tag, in_tag, EVP_AEAD_AES_GCM_SIV_TAG_LEN) !=
       0) {
     OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BAD_DECRYPT);
     return 0;
   }
 
-  *out_len = in_len - EVP_AEAD_AES_GCM_SIV_TAG_LEN;
   return 1;
 }
 
@@ -532,9 +520,9 @@ static const EVP_AEAD aead_aes_128_gcm_siv_asm = {
     aead_aes_gcm_siv_asm_init,
     NULL /* init_with_direction */,
     aead_aes_gcm_siv_asm_cleanup,
-    aead_aes_gcm_siv_asm_open,
+    NULL /* open */,
     aead_aes_gcm_siv_asm_seal_scatter,
-    NULL /* open_gather */,
+    aead_aes_gcm_siv_asm_open_gather,
     NULL /* get_iv */,
     NULL /* tag_len */,
     NULL /* serialize_state */,
@@ -552,9 +540,9 @@ static const EVP_AEAD aead_aes_256_gcm_siv_asm = {
     aead_aes_gcm_siv_asm_init,
     NULL /* init_with_direction */,
     aead_aes_gcm_siv_asm_cleanup,
-    aead_aes_gcm_siv_asm_open,
+    NULL /* open */,
     aead_aes_gcm_siv_asm_seal_scatter,
-    NULL /* open_gather */,
+    aead_aes_gcm_siv_asm_open_gather,
     NULL /* get_iv */,
     NULL /* tag_len */,
     NULL /* serialize_state */,
@@ -669,8 +657,8 @@ static void gcm_siv_polyval(
   }
 
   uint8_t length_block[16];
-  CRYPTO_store_u64_le(length_block, ad_len * 8);
-  CRYPTO_store_u64_le(length_block + 8, in_len * 8);
+  CRYPTO_store_u64_le(length_block, ((uint64_t) ad_len) * 8);
+  CRYPTO_store_u64_le(length_block + 8, ((uint64_t) in_len) * 8);
   CRYPTO_POLYVAL_update_blocks(&polyval_ctx, length_block,
                                sizeof(length_block));