Merge branch 'main' into add-ssl-security-callback

Author: WillChilds-Klein

.github/workflows/aws-lc-rs.yml

@@ -310,8 +310,11 @@ jobs:
         run: |
           set -ex
           # Wine binfmt allows the kernel to transparently run .exe files through
-          # Wine. This is needed for the FIPS build, which runs fips_empty_main.exe
-          # at build time to capture the integrity hash.
+          # Wine. The FIPS build itself no longer needs Wine (inject_hash.go
+          # patches the integrity hash directly into crypto.dll using the
+          # linker map), but we still need Wine to run the FIPS sanity test
+          # below, which loads the cross-built DLL so the .CRT$XCU initializer
+          # can trigger the power-on self-test.
           #
           # Ubuntu 24.04's wine64 (9.0) does not properly execute .CRT$XCU
           # initializers in cross-compiled DLLs, which prevents the FIPS

CMakeLists.txt

@@ -217,13 +217,14 @@ if(GENERATE_RUST_BINDINGS)
   message(STATUS "Found bindgen-cli: ${BINDGEN_EXECUTABLE} (version ${BINDGEN_VERSION})")
   message(STATUS "Rust bindings target version: ${RUST_BINDINGS_TARGET_VERSION}")
 
-  # Check for rustfmt (required for --formatter rustfmt option)
+  # Check for rustfmt (optional: used to format the generated bindings if available)
   find_program(RUSTFMT_EXECUTABLE NAMES rustfmt)
-  if(NOT RUSTFMT_EXECUTABLE)
-    message(FATAL_ERROR "GENERATE_RUST_BINDINGS requires rustfmt but it was not found. "
-                        "Install it with: rustup component add rustfmt")
+  if(RUSTFMT_EXECUTABLE)
+    message(STATUS "Found rustfmt: ${RUSTFMT_EXECUTABLE}")
+  else()
+    message(STATUS "rustfmt not found; generated Rust bindings will not be formatted. "
+                   "Install it with: rustup component add rustfmt")
   endif()
-  message(STATUS "Found rustfmt: ${RUSTFMT_EXECUTABLE}")
 endif()
 
 if(DISABLE_CPU_JITTER_ENTROPY)

cmake/rust_bindings.cmake

@@ -165,8 +165,12 @@ function(generate_rust_bindings)
     "--with-derive-eq"
     "--generate" "functions,types,vars,methods,constructors,destructors"
     "--rust-target" "${RUST_BINDINGS_TARGET_VERSION}"
-    "--formatter" "rustfmt"
   )
+  # Use rustfmt as the formatter only if it was discovered. Otherwise fall back
+  # to bindgen's default ("prettyplease") so the build doesn't require rustfmt.
+  if(RUSTFMT_EXECUTABLE)
+    list(APPEND _bindgen_args "--formatter" "rustfmt")
+  endif()
   # Add symbol prefix if specified. See module-level comment for why we use
   # --prefix-link-name instead of including the prefix symbols header.
   if(ARG_PREFIX AND NOT ARG_PREFIX STREQUAL "")

crypto/CMakeLists.txt

@@ -638,34 +638,22 @@ if(FIPS_SHARED)
   # Rewrite libcrypto.so, libcrypto.dylib, or crypto.dll to inject the correct module
   # hash value. For now we support the FIPS build only on Linux, macOS, iOS, and Windows.
   if(MSVC)
-    # On Windows we use capture_hash.go: build crypto.dll with a placeholder
-    # hash, then run fips_empty_main.exe (which triggers the integrity check
-    # and prints the correct hash), then patch the placeholder in crypto.dll.
-    #
-    # The fips_integrity target (marked ALL) ensures the hash injection runs
-    # before 'install' copies crypto.dll. Without this, Cargo builds (which
-    # run 'cmake --build --target install') would skip the hash injection
-    # because fips_empty_main is not in crypto's dependency chain.
+    # On Windows, inject_hash.go parses the linker map file and the PE to locate
+    # module boundaries, computes the integrity hash, and patches it directly
+    # into the DLL. This matches the Linux/Apple approach and does not require
+    # running a binary, enabling cross-compilation without Wine.
+    set(CRYPTO_MAP_FILE "${CMAKE_CURRENT_BINARY_DIR}/fips_crypto.map")
     build_libcrypto(NAME crypto MODULE_SOURCE $<TARGET_OBJECTS:fipsmodule> SET_OUTPUT_NAME)
-
-    add_executable(fips_empty_main fipsmodule/fips_empty_main.c)
-    target_link_libraries(fips_empty_main PUBLIC crypto)
-    target_add_awslc_include_paths(TARGET fips_empty_main SCOPE PRIVATE)
+    target_link_options(crypto PRIVATE "/MAP:${CRYPTO_MAP_FILE}")
 
     add_custom_command(
-      OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/fips_hash_injected.stamp
+      TARGET crypto POST_BUILD
       COMMAND ${GO_EXECUTABLE} run
-        ${AWSLC_SOURCE_DIR}/util/fipstools/capture_hash/capture_hash.go
-        -in-executable $<TARGET_FILE:fips_empty_main>
-        -patch-dll $<TARGET_FILE:crypto>
-      COMMAND ${CMAKE_COMMAND} -E touch ${CMAKE_CURRENT_BINARY_DIR}/fips_hash_injected.stamp
-      DEPENDS fips_empty_main crypto
-        ${AWSLC_SOURCE_DIR}/util/fipstools/capture_hash/capture_hash.go
+      ${AWSLC_SOURCE_DIR}/util/fipstools/inject_hash/inject_hash.go
+      -o $<TARGET_FILE:crypto> -in-object $<TARGET_FILE:crypto>
+      -map ${CRYPTO_MAP_FILE} -windows
       WORKING_DIRECTORY ${AWSLC_SOURCE_DIR}
     )
-    add_custom_target(fips_integrity ALL
-      DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/fips_hash_injected.stamp
-    )
   else()
     # On Apple and Linux platforms inject_hash.go can parse libcrypto and inject
     # the hash directly into the final library.

crypto/bio/pair.c

@@ -42,6 +42,10 @@ static int bio_new(BIO *bio) {
   return 1;
 }
 
+// bio_destroy_pair unlinks the two halves of a BIO pair. Concurrent calls to
+// BIO_free on both halves from separate threads are the caller's responsibility
+// to synchronize. As in OpenSSL, the BIO pair API does not provide internal
+// locking.
 static void bio_destroy_pair(BIO *bio) {
   struct bio_bio_st *b = bio->ptr;
   BIO *peer_bio;

crypto/fipsmodule/CMakeLists.txt

@@ -619,26 +619,30 @@ elseif(FIPS_SHARED)
     separate_arguments(FIPS_MARKER_C_FLAGS NATIVE_COMMAND "${CMAKE_C_FLAGS}")
     add_custom_command(
       OUTPUT fips_msvc_start.obj
-      COMMAND ${CMAKE_C_COMPILER} ${FIPS_MARKER_C_FLAGS} -w /nologo /c /DAWSLC_FIPS_SHARED_START /Fo:fips_msvc_start.obj ${CMAKE_CURRENT_SOURCE_DIR}/fips_shared_library_marker.c
+      COMMAND ${CMAKE_C_COMPILER} ${FIPS_MARKER_C_FLAGS} -w /nologo /c /DAWSLC_FIPS_SHARED_START /Fofips_msvc_start.obj ${CMAKE_CURRENT_SOURCE_DIR}/fips_shared_library_marker.c
       DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/fips_shared_library_marker.c
     )
     add_custom_command(
       OUTPUT fips_msvc_end.obj
-      COMMAND ${CMAKE_C_COMPILER} ${FIPS_MARKER_C_FLAGS} -w /nologo /c /DAWSLC_FIPS_SHARED_END /Fo:fips_msvc_end.obj ${CMAKE_CURRENT_SOURCE_DIR}/fips_shared_library_marker.c
+      COMMAND ${CMAKE_C_COMPILER} ${FIPS_MARKER_C_FLAGS} -w /nologo /c /DAWSLC_FIPS_SHARED_END /Fofips_msvc_end.obj ${CMAKE_CURRENT_SOURCE_DIR}/fips_shared_library_marker.c
       DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/fips_shared_library_marker.c
     )
 
-    if(CMAKE_AR)
-      set(MSVC_LIB "${CMAKE_AR}")
-    else()
-      get_filename_component(MSVC_BIN ${CMAKE_LINKER} DIRECTORY)
-      set(MSVC_LIB "${MSVC_BIN}/lib.exe")
+    get_filename_component(MSVC_BIN ${CMAKE_LINKER} DIRECTORY)
+    find_program(MSVC_LIB NAMES lib lib.exe llvm-lib llvm-lib.exe
+                 HINTS ${MSVC_BIN} NO_DEFAULT_PATH)
+    if(NOT MSVC_LIB)
+      find_program(MSVC_LIB NAMES lib lib.exe llvm-lib llvm-lib.exe)
+    endif()
+    if(NOT MSVC_LIB)
+      message(FATAL_ERROR "Could not find lib.exe or llvm-lib for creating bcm.lib")
     endif()
 
+    file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/bcm_objects.rsp"
+         CONTENT "\"$<JOIN:$<TARGET_OBJECTS:bcm_library>,\"\n\">\"")
     add_custom_command(
       OUTPUT ${BCM_NAME}
-      COMMAND ${MSVC_LIB} /nologo fips_msvc_start.obj $<TARGET_OBJECTS:bcm_library> fips_msvc_end.obj /OUT:${BCM_NAME}
-      COMMAND_EXPAND_LISTS
+      COMMAND ${MSVC_LIB} /nologo fips_msvc_start.obj @bcm_objects.rsp fips_msvc_end.obj /OUT:${BCM_NAME}
       DEPENDS fips_msvc_start.obj fips_msvc_end.obj bcm_library
       WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
     )

crypto/fipsmodule/FIPS.md

@@ -173,20 +173,10 @@ The Shared Windows FIPS integrity test differs in two key ways:
 2. How the correct integrity hash is calculated
 
 Microsoft Visual C compiler (MSVC) does not support linker scripts that add symbols to mark the start and end of the text and rodata sections, as is done on Linux. Instead, `fips_shared_library_marker.c` is compiled twice to generate two object files that contain start/end functions and variables. MSVC `pragma` segment definitions are used to place the markers in specific sections (e.g. `.fipstx$a`). This particular name format uses [Portable Executable Grouped Sections](https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#grouped-sections-object-only) to control what section the code is placed in and the order within the section. With the start and end markers placed at `$a` and `$z` respectively, BCM puts everything in the `$b` section. When the final crypto.dll is built, all the code is in the `.fipstx` section, all data is in `.fipsda`, all constants are in `.fipsco`, all uninitialized items in `.fipsbs`, and everything is in the correct order.
-The process to generate the expected integrity fingerprint is also different from Linux. We use a single-DLL capture-and-patch approach: build `crypto.dll` once with a placeholder hash, run it to compute the real hash, then binary-patch the placeholder directly in the DLL. This avoids building two separate DLLs whose linker output may differ (e.g. mandatory ASLR on ARM64 causes different ADRP immediates, and `lld-link` used by clang-cl is not guaranteed to produce byte-identical output across two independent link operations).
-
-1. Build the required object files once: `bcm.obj` from `bcm.c` and the start/end object files
-    1. `bcm.obj` places the power-on self tests in the `.CRT$XCU` section which is run automatically by the Windows Common Runtime library (CRT) startup code
-2. Use MSVC's `lib.exe` (or `llvm-lib` for clang-cl) to combine the start/end object files with `bcm.obj` to create the static library `bcm.lib`.
-    1. MSVC does not support combining multiple object files into another object file like the Apple build.
-3. Build `fipsmodule` which contains the placeholder integrity hash
-4. Build `crypto.dll` with `bcm.lib` and `fipsmodule`
-5. Build the small application `fips_empty_main.exe` and link it with `crypto.dll`
-6. `capture_hash.go` runs `fips_empty_main.exe`
-    1. The CRT runs all functions in the `.CRT$XC*` sections in order starting with `.CRT$XCA`
-    2. The BCM power-on tests are in `.CRT$XCU` and are run after all other Windows initialization is complete
-    3. BCM calculates the correct integrity value which will not match the placeholder value. Before aborting the process the correct value is printed
-    4. `capture_hash.go` reads the correct integrity value and binary-patches the 32-byte placeholder directly in `crypto.dll`
+The process to generate the expected integrity fingerprint follows the same approach as Linux and Apple, using `inject_hash.go` to patch the hash directly into the final binary:
+
+1. Build `crypto.dll` from `bcm.c`, the start/end marker objects, and the rest of `fipsmodule`, using the `/MAP:` linker flag to produce a linker map file
+2. `inject_hash.go -windows` parses the linker map file and the PE to locate the FIPS module boundaries via the marker symbols, computes the integrity hash over the module text and rodata, and patches the correct value directly into `crypto.dll`
 
 ### Linux Static build
 

crypto/fipsmodule/fips_empty_main.c

@@ -33,7 +33,8 @@ static int nc_match_single(GENERAL_NAME *sub, GENERAL_NAME *gen,
 static int nc_dn(X509_NAME *sub, X509_NAME *nm);
 static int nc_dns(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *dns,
                   int excluding);
-static int nc_email(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *eml);
+static int nc_email(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *eml,
+                    int excluding);
 static int nc_uri(const ASN1_IA5STRING *uri, const ASN1_IA5STRING *base);
 static int nc_ip(const ASN1_OCTET_STRING *ip, const ASN1_OCTET_STRING *base);
 
@@ -477,7 +478,7 @@ static int nc_match_single(GENERAL_NAME *gen, GENERAL_NAME *base,
       return nc_dns(gen->d.dNSName, base->d.dNSName, excluding);
 
     case GEN_EMAIL:
-      return nc_email(gen->d.rfc822Name, base->d.rfc822Name);
+      return nc_email(gen->d.rfc822Name, base->d.rfc822Name, excluding);
 
     case GEN_URI:
       return nc_uri(gen->d.uniformResourceIdentifier,
@@ -614,48 +615,120 @@ static int nc_dns(const ASN1_IA5STRING *dns, const ASN1_IA5STRING *base,
   return X509_V_OK;
 }
 
-static int nc_email(const ASN1_IA5STRING *eml, const ASN1_IA5STRING *base) {
+// Returns 1 if |cbs| contains only characters valid in an RFC 5321 Sec.4.1.2
+// local-part atom (atext per RFC 5322 Sec.3.2.3). RFC 5322 allows quoted
+// local-parts which may contain '@' characters. Rather than parsing
+// quoted-strings, we reject local-parts containing non-atext characters.
+static int is_valid_rfc822_local_part(const CBS *cbs) {
+  if (CBS_len(cbs) == 0) {
+    return 0;
+  }
+  for (size_t i = 0; i < CBS_len(cbs); i++) {
+    uint8_t c = CBS_data(cbs)[i];
+    if (!(OPENSSL_isalnum(c) || c == '!' || c == '#' || c == '$' ||
+          c == '%' || c == '&' || c == '\'' || c == '*' || c == '+' ||
+          c == '-' || c == '/' || c == '=' || c == '?' || c == '^' ||
+          c == '_' || c == '`' || c == '{' || c == '|' || c == '}' ||
+          c == '~' || c == '.')) {
+      return 0;
+    }
+  }
+  return 1;
+}
+
+// Returns 1 if |cbs| contains only characters valid in a DNS domain label
+// per RFC 1034 Sec.3.5 (alphanumeric, hyphen, dot).
+static int is_valid_rfc822_domain(const CBS *cbs) {
+  if (CBS_len(cbs) == 0) {
+    return 0;
+  }
+  for (size_t i = 0; i < CBS_len(cbs); i++) {
+    uint8_t c = CBS_data(cbs)[i];
+    if (!(OPENSSL_isalnum(c) || c == '-' || c == '.')) {
+      return 0;
+    }
+  }
+  return 1;
+}
+
+static int nc_email(const ASN1_IA5STRING *eml, const ASN1_IA5STRING *base,
+                    int excluding) {
   CBS eml_cbs, base_cbs;
   CBS_init(&eml_cbs, eml->data, eml->length);
   CBS_init(&base_cbs, base->data, base->length);
 
-  // TODO(davidben): In OpenSSL 1.1.1, this switched from the first '@' to the
-  // last one. Match them here, or perhaps do an actual parse. Looks like
-  // multiple '@'s may be allowed in quoted strings.
-  CBS eml_local, base_local;
-  if (!CBS_get_until_first(&eml_cbs, &eml_local, '@')) {
+  CBS eml_local;
+  if (!CBS_get_until_first(&eml_cbs, &eml_local, '@') ||
+      !CBS_skip(&eml_cbs, 1)) {
+    return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
+  }
+  CBS eml_domain = eml_cbs;
+
+  // Reject subject emails with multiple '@'. This catches both
+  // "a@b"@evil.example and a@b@evil.example.
+  CBS unused;
+  if (CBS_get_until_first(&eml_cbs, &unused, '@')) {
+    return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
+  }
+
+  // Reject subject emails with characters outside RFC 5321 atext in the
+  // local-part (rejects quoted local-parts like "user"@example.com) or
+  // invalid domain characters.
+  if (!is_valid_rfc822_local_part(&eml_local) ||
+      !is_valid_rfc822_domain(&eml_domain)) {
     return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
   }
+
+  CBS base_local;
   int base_has_at = CBS_get_until_first(&base_cbs, &base_local, '@');
 
-  // Special case: initial '.' is RHS match
-  if (!base_has_at && starts_with(&base_cbs, '.')) {
-    if (has_suffix_case(&eml_cbs, &base_cbs)) {
-      return X509_V_OK;
+  if (base_has_at) {
+    // "@example.com" is not a valid constraint per RFC 5280 Sec.4.2.1.10.
+    if (CBS_len(&base_local) == 0) {
+      return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
     }
-    return X509_V_ERR_PERMITTED_VIOLATION;
+    CBS_skip(&base_cbs, 1);  // skip past '@'
+    CBS base_domain = base_cbs;
+
+    // Reject constraints with multiple '@'.
+    if (CBS_get_until_first(&base_cbs, &unused, '@')) {
+      return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
+    }
+    if (!is_valid_rfc822_local_part(&base_local) ||
+        !is_valid_rfc822_domain(&base_domain)) {
+      return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
+    }
+
+    // For excluded subtrees, compare the local-part case-insensitively.
+    // RFC 5321 and RFC 5322 conflict on case sensitivity; for security we
+    // err on being more strict when checking exclusions.
+    int local_match = excluding
+        ? equal_case(&base_local, &eml_local)
+        : CBS_mem_equal(&base_local, CBS_data(&eml_local),
+                        CBS_len(&eml_local));
+    if (!local_match) {
+      return X509_V_ERR_PERMITTED_VIOLATION;
+    }
+    if (!equal_case(&base_domain, &eml_domain)) {
+      return X509_V_ERR_PERMITTED_VIOLATION;
+    }
+    return X509_V_OK;
   }
 
-  // If we have anything before '@' match local part
-  if (base_has_at) {
-    // TODO(davidben): This interprets a constraint of "@example.com" as
-    // "example.com", which is not part of RFC5280.
-    if (CBS_len(&base_local) > 0) {
-      // Case sensitive match of local part
-      if (!CBS_mem_equal(&base_local, CBS_data(&eml_local),
-                         CBS_len(&eml_local))) {
-        return X509_V_ERR_PERMITTED_VIOLATION;
-      }
+  if (!is_valid_rfc822_domain(&base_cbs)) {
+    return X509_V_ERR_UNSUPPORTED_NAME_SYNTAX;
+  }
+
+  // Special case: initial '.' is RHS match
+  if (starts_with(&base_cbs, '.')) {
+    if (has_suffix_case(&eml_domain, &base_cbs)) {
+      return X509_V_OK;
     }
-    // Position base after '@'
-    assert(starts_with(&base_cbs, '@'));
-    CBS_skip(&base_cbs, 1);
+    return X509_V_ERR_PERMITTED_VIOLATION;
   }
 
-  // Just have hostname left to match: case insensitive
-  assert(starts_with(&eml_cbs, '@'));
-  CBS_skip(&eml_cbs, 1);
-  if (!equal_case(&base_cbs, &eml_cbs)) {
+  // Domain-only constraint: case insensitive match
+  if (!equal_case(&base_cbs, &eml_domain)) {
     return X509_V_ERR_PERMITTED_VIOLATION;
   }
 

crypto/x509/v3_ncons.c

@@ -2773,16 +2773,34 @@ TEST(X509Test, NameConstraints) {
       {GEN_EMAIL, "foo@example.com", "foo@EXAMPLE.COM", X509_V_OK,
        X509_V_ERR_EXCLUDED_VIOLATION},
       {GEN_EMAIL, "foo@example.com", "FOO@example.com",
-       X509_V_ERR_PERMITTED_VIOLATION, X509_V_OK},
+       X509_V_ERR_PERMITTED_VIOLATION, X509_V_ERR_EXCLUDED_VIOLATION},
       {GEN_EMAIL, "foo@example.com", "bar@example.com",
        X509_V_ERR_PERMITTED_VIOLATION, X509_V_OK},
-      // OpenSSL ignores a stray leading @.
-      {GEN_EMAIL, "foo@example.com", "@example.com", X509_V_OK,
-       X509_V_ERR_EXCLUDED_VIOLATION},
-      {GEN_EMAIL, "foo@example.com", "@EXAMPLE.COM", X509_V_OK,
-       X509_V_ERR_EXCLUDED_VIOLATION},
+      // "@example.com" is not a valid constraint per RFC 5280 Sec.4.2.1.10.
+      {GEN_EMAIL, "foo@example.com", "@example.com",
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX,
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
+      {GEN_EMAIL, "foo@example.com", "@EXAMPLE.COM",
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX,
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
       {GEN_EMAIL, "foo@bar.example.com", "@example.com",
-       X509_V_ERR_PERMITTED_VIOLATION, X509_V_OK},
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX,
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
+
+      // Reject subject emails with quoted local-parts.
+      // A quoted local-part containing '@' would cause the parser to split
+      // at the wrong '@', bypassing name constraints.
+      {GEN_EMAIL, "\"a@b\"@evil.example", ".evil.example",
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX,
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
+      // Reject subject emails with multiple '@' signs.
+      {GEN_EMAIL, "a@b@evil.example", ".evil.example",
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX,
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
+      // Reject constraints with multiple '@' signs.
+      {GEN_EMAIL, "foo@example.com", "a@b@example.com",
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX,
+       X509_V_ERR_UNSUPPORTED_NAME_SYNTAX},
 
       // Basic syntax check.
       {GEN_URI, "not-a-url", "not-a-url", X509_V_ERR_UNSUPPORTED_NAME_SYNTAX,