From 6cdfe36c246c0c76328627679bee49ab9ba4d90e Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Thu, 16 Apr 2026 16:02:09 -0700 Subject: [PATCH 01/10] ci: add gh-pages workflow for API documentation Generate HTML API docs from header comments using the existing doc.go tool (inherited from BoringSSL) and publish to gh-pages. Docs are deployed to the /api/ subdirectory to allow doxygen or other doc generators to coexist in future PRs. Workflow only triggers on changes to public headers, the doc generator, its config, or the workflow itself. Add missing preamble comments to 6 headers (base64.h, dsa.h, kdf.h, lhash.h, opensslconf.h, pkcs8.h) so the doc generator produces descriptions for all headers in the index page. --- .github/workflows/api-docs.yml | 33 +++++++++++++++++++++++++++++++++ include/openssl/base64.h | 4 ++++ include/openssl/dsa.h | 4 ++++ include/openssl/kdf.h | 4 ++++ include/openssl/lhash.h | 4 ++++ include/openssl/opensslconf.h | 3 +++ include/openssl/pkcs8.h | 3 +++ 7 files changed, 55 insertions(+) create mode 100644 .github/workflows/api-docs.yml diff --git a/.github/workflows/api-docs.yml b/.github/workflows/api-docs.yml new file mode 100644 index 00000000000..199e30db362 --- /dev/null +++ b/.github/workflows/api-docs.yml @@ -0,0 +1,33 @@ +name: API Documentation +on: + push: + branches: [main] + paths: + - 'include/openssl/**' + - 'util/doc.go' + - 'util/doc.config' + - 'util/doc.css' + - '.github/workflows/api-docs.yml' + +permissions: + contents: write + +jobs: + publish-api-docs: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v4 + with: + go-version: ">=1.22" + - name: Generate API documentation + run: | + mkdir -p api-output + cd util + go run doc.go --config doc.config --out ../api-output + - name: Deploy to gh-pages + uses: peaceiris/actions-gh-pages@v4 + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + publish_dir: ./api-output + destination_dir: api diff --git a/include/openssl/base64.h b/include/openssl/base64.h index cb2ff5f0ed2..e7a8592465f 100644 --- a/include/openssl/base64.h +++ b/include/openssl/base64.h @@ -10,6 +10,10 @@ extern "C" { #endif + +// Base64 encoding and decoding. + + /** * @file * @brief base64 encoding and decoding functions. diff --git a/include/openssl/dsa.h b/include/openssl/dsa.h index 2e0b3087fca..41d7c81d125 100644 --- a/include/openssl/dsa.h +++ b/include/openssl/dsa.h @@ -15,6 +15,10 @@ extern "C" { #endif + +// Digital Signature Algorithm (deprecated). + + #define OPENSSL_DSA_MAX_MODULUS_BITS 10000 diff --git a/include/openssl/kdf.h b/include/openssl/kdf.h index 68750ca56d5..7345abcec92 100644 --- a/include/openssl/kdf.h +++ b/include/openssl/kdf.h @@ -10,6 +10,10 @@ extern "C" { #endif + +// Key derivation functions. + + // CRYPTO_tls1_prf calculates |out_len| bytes of the TLS PRF, using |digest|, // and writes them to |out|. It returns one on success and zero on error. // TLS 1.2: https://datatracker.ietf.org/doc/html/rfc5246#section-5 diff --git a/include/openssl/lhash.h b/include/openssl/lhash.h index da720bc3b81..438abe4117e 100644 --- a/include/openssl/lhash.h +++ b/include/openssl/lhash.h @@ -10,6 +10,10 @@ extern "C" { #endif + +// Hash table implementation for internal use. + + typedef struct lhash_st _LHASH; // lhash is an internal library and not exported for use outside BoringSSL. This diff --git a/include/openssl/opensslconf.h b/include/openssl/opensslconf.h index f475cdea5a8..11377da6cd7 100644 --- a/include/openssl/opensslconf.h +++ b/include/openssl/opensslconf.h @@ -13,6 +13,9 @@ extern "C" { #endif +// OpenSSL compatibility configuration flags. + + #define OPENSSL_NO_ASYNC #define OPENSSL_NO_BLAKE2 #define OPENSSL_NO_BUF_FREELISTS diff --git a/include/openssl/pkcs8.h b/include/openssl/pkcs8.h index e395c26d9ab..8e3bbe34015 100644 --- a/include/openssl/pkcs8.h +++ b/include/openssl/pkcs8.h @@ -15,6 +15,9 @@ extern "C" { #endif +// PKCS#8 private key encryption and decryption. + + // PKCS8_encrypt serializes and encrypts a PKCS8_PRIV_KEY_INFO with PBES1 or // PBES2 as defined in PKCS #5. Only pbeWithSHAAnd128BitRC4, // pbeWithSHAAnd3-KeyTripleDES-CBC and pbeWithSHA1And40BitRC2, defined in PKCS From e3de53b3eae618ad2888b7e9858b787a44ab6fcb Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 20 Apr 2026 16:27:43 -0700 Subject: [PATCH 02/10] revert: doxygen changes to [base64.h](https://github.com/aws/aws-lc/pull/2908) --- include/openssl/base64.h | 303 ++++++++++----------------------------- 1 file changed, 73 insertions(+), 230 deletions(-) diff --git a/include/openssl/base64.h b/include/openssl/base64.h index e7a8592465f..859ba56a059 100644 --- a/include/openssl/base64.h +++ b/include/openssl/base64.h @@ -11,286 +11,129 @@ extern "C" { #endif -// Base64 encoding and decoding. +// base64 functions. +// +// For historical reasons, these functions have the EVP_ prefix but just do +// base64 encoding and decoding. Note that BoringSSL is a cryptography library, +// so these functions are implemented with side channel protections, at a +// performance cost. For other base64 uses, use a general-purpose base64 +// implementation. -/** - * @file - * @brief base64 encoding and decoding functions. - * - * @details - * For historical reasons, these functions have the `EVP_` prefix but just do - * base64 encoding and decoding. Note that BoringSSL is a cryptography library, - * so these functions are implemented with side channel protections, at a - * performance cost. For other base64 uses, use a general-purpose base64 - * implementation. - */ +// Encoding - -/** - * @name Encoding Functions - * - * @{ - */ - -/** - * @brief Base64 encodes bytes from `src` into `dst`. - * - * @details - * Base64 encodes `src_len` bytes from `src` and writes the - * result to `dst` with a trailing `NULL`. - * - * @param [out] dst Base64 encoded value - * @param [in] src Bytes to encode - * @param [in] src_len Number of bytes to encode - * - * @return The number of bytes written, not including the trailing `NULL` - */ +// EVP_EncodeBlock encodes |src_len| bytes from |src| and writes the +// result to |dst| with a trailing NUL. It returns the number of bytes +// written, not including this trailing NUL. OPENSSL_EXPORT size_t EVP_EncodeBlock(uint8_t *dst, const uint8_t *src, size_t src_len); -/** - * @brief Calculates the number of bytes needed to encode an input using Base64. - * - * @details - * Sets `*out_len` to the number of bytes that will be needed to call - * #EVP_EncodeBlock on an input of length `len`. `*out_len` includes the final - * `NULL` that #EVP_EncodeBlock writes. - * - * @param [out] out_len Number of bytes needed to Base64 encode a source - * @param [in] len Source length - * - * @retval 1 Success - * @retval 0 Error - */ +// EVP_EncodedLength sets |*out_len| to the number of bytes that will be needed +// to call |EVP_EncodeBlock| on an input of length |len|. This includes the +// final NUL that |EVP_EncodeBlock| writes. It returns one on success or zero +// on error. OPENSSL_EXPORT int EVP_EncodedLength(size_t *out_len, size_t len); -/** @} Encoding Functions */ +// Decoding -/** - * @name Decoding Functions - * - * @{ - */ - -/** - * @brief Sets the maximum number of bytes need to store a decoded Base64 input. - * - * @param [out] out_len Maximum number of bytes to store decoded input - * @param [in] len Base64 formatted input length - * - * @retval 1 Success - * @retval 0 `len` is not a valid length for a base64-encoded string - */ +// EVP_DecodedLength sets |*out_len| to the maximum number of bytes that will +// be needed to call |EVP_DecodeBase64| on an input of length |len|. It returns +// one on success or zero if |len| is not a valid length for a base64-encoded +// string. OPENSSL_EXPORT int EVP_DecodedLength(size_t *out_len, size_t len); -/** - * @brief Decodes specified number of Base64 formatted bytes to output buffer. - * - * @details - * Decodes `in_len` bytes from base64 and writes `*out_len` bytes to `out`. If - * `*out_len` doesn't have enough bytes for the maximum output size, the - * operation fails. - * - * @param [out] out Decoded output - * @param [out] out_len Number of bytes written to `out` - * @param [in] max_out Length of `out` - * @param [in] in Base64 input buffer - * @param [in] in_len Number of bytes to decode from `in` - * - * @retval 1 Success - * @retval 0 Error - */ +// EVP_DecodeBase64 decodes |in_len| bytes from base64 and writes +// |*out_len| bytes to |out|. |max_out| is the size of the output +// buffer. If it is not enough for the maximum output size, the +// operation fails. It returns one on success or zero on error. OPENSSL_EXPORT int EVP_DecodeBase64(uint8_t *out, size_t *out_len, size_t max_out, const uint8_t *in, size_t in_len); -/** @} Decoding Functions */ -/** - * @name Deprecated Functions - * - * OpenSSL provides a streaming base64 implementation, however its behavior is - * very specific to PEM. It is also very lenient of invalid input. Use of any of - * these functions is thus deprecated. - * - * @{ - */ +// Deprecated functions. +// +// OpenSSL provides a streaming base64 implementation, however its behavior is +// very specific to PEM. It is also very lenient of invalid input. Use of any of +// these functions is thus deprecated. -/** - * @brief Returns a newly-allocated EVP_ENCODE_CTX. - * - * @details - * The caller must release the result with #EVP_ENCODE_CTX_free when - * done. - * - * @returns Pointer to newly allocated EVP_ENCODE_CTX or `NULL` on error - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_ENCODE_CTX_new returns a newly-allocated |EVP_ENCODE_CTX| or NULL on +// error. The caller must release the result with |EVP_ENCODE_CTX_free| when +// done. OPENSSL_EXPORT EVP_ENCODE_CTX *EVP_ENCODE_CTX_new(void); -/** - * @brief Releases memory associated with `ctx`. - * - * @param [in] ctx Pointer to deallocate - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_ENCODE_CTX_free releases memory associated with |ctx|. OPENSSL_EXPORT void EVP_ENCODE_CTX_free(EVP_ENCODE_CTX *ctx); -/** - * @brief Initialises `*ctx`. - * - * @details - * This is typically stack allocated, for an encoding operation. - * - * @param [in,out] ctx Context to initialize - * - * @warning The encoding operation breaks its output with newlines every 64 characters of output (48 characters of input). Use #EVP_EncodeBlock to encode raw base64. - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_EncodeInit initialises |*ctx|, which is typically stack +// allocated, for an encoding operation. +// +// NOTE: The encoding operation breaks its output with newlines every +// 64 characters of output (48 characters of input). Use +// EVP_EncodeBlock to encode raw base64. OPENSSL_EXPORT void EVP_EncodeInit(EVP_ENCODE_CTX *ctx); -/** - * @brief Encodes bytes from `in` to Base64 and writes the value to `out`. - * - * @details - * Some state may be contained in `ctx` so #EVP_EncodeFinal must be used to - * flush it before using the encoded data. - * - * @param [in,out] ctx Encoding context - * @param [out] out Base64 encoded value - * @param [out] out_len Number of bytes written - * @param [in] in Input buffer - * @param [in] in_len Number of bytes to encode - * - * @retval 1 Success - * @retval 0 Failure or `in_len` was 0 - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_EncodeUpdate encodes |in_len| bytes from |in| and writes an encoded +// version of them to |out| and sets |*out_len| to the number of bytes written. +// Some state may be contained in |ctx| so |EVP_EncodeFinal| must be used to +// flush it before using the encoded data. OPENSSL_EXPORT int EVP_EncodeUpdate(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len, const uint8_t *in, size_t in_len); -/** - * @brief Flushes any remaining output bytes from `ctx` to `out`. - * - * @param [in,out] ctx Encoding context - * @param [out] out Output buffer - * @param [out] out_len Number of bytes written to buffer - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_EncodeFinal flushes any remaining output bytes from |ctx| to |out| and +// sets |*out_len| to the number of bytes written. OPENSSL_EXPORT void EVP_EncodeFinal(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len); -/** - * @brief Initialises `*ctx` for a decoding operation. - * - * @details - * This is typically stack allocated. - * - * @param [in,out] ctx Context to initialize - * - * @todo davidben: This isn't a straight-up base64 decode either. Document and/or fix exactly what's going on here; maximum line length and such. - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_DecodeInit initialises |*ctx|, which is typically stack allocated, for +// a decoding operation. +// +// TODO(davidben): This isn't a straight-up base64 decode either. Document +// and/or fix exactly what's going on here; maximum line length and such. OPENSSL_EXPORT void EVP_DecodeInit(EVP_ENCODE_CTX *ctx); -/** - * @brief Decodes bytes from `in` and writes the decoded data to `out`. - * - * @details - * Some state may be contained in `ctx` so #EVP_DecodeFinal must be used to - * flush it before using the decoded data. - * - * @param [in,out] ctx Decoding context - * @param [out] out Decoded bytes - * @param [out] out_len Number of bytes written - * @param [in] in Input buffer - * @param [in] in_len Number of bytes to decode - * - * @retval -1 Error - * @retval 0 The line was short (i.e., it was the last line) - * @retval 1 A full line of input was processed - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_DecodeUpdate decodes |in_len| bytes from |in| and writes the decoded +// data to |out| and sets |*out_len| to the number of bytes written. Some state +// may be contained in |ctx| so |EVP_DecodeFinal| must be used to flush it +// before using the encoded data. +// +// It returns -1 on error, one if a full line of input was processed and zero +// if the line was short (i.e. it was the last line). OPENSSL_EXPORT int EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len, const uint8_t *in, size_t in_len); -/** - * @brief Flushes any remaining output bytes from `ctx` to `out`. - * - * @param [in,out] ctx Decoding context - * @param [out] out Output buffer - * @param [out] out_len Number of bytes written to buffer - * - * @retval 1 Success - * @retval -1 Error - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_DecodeFinal flushes any remaining output bytes from |ctx| to |out| and +// sets |*out_len| to the number of bytes written. It returns one on success +// and minus one on error. OPENSSL_EXPORT int EVP_DecodeFinal(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len); -/** - * @brief Decodes Base64 bytes from `src` and writes value to `dst`. - * - * @param [out] dst Decoded bytes - * @param [in] src Base64 encoded bytes - * @param [in] src_len Number of bytes to decode - * - * @returns Number of bytes written or -1 on error - * - * @warning `EVP_DecodeBlock`'s return value does not take padding into account. It also strips leading whitespace and trailing whitespace and minuses. - * - * @deprecated OpenSSL provides a streaming base64 implementation, however its behavior is very specific to PEM. It is also very lenient of invalid input. Use of any of these functions is thus deprecated. - */ +// EVP_DecodeBlock encodes |src_len| bytes from |src| and writes the result to +// |dst|. It returns the number of bytes written or -1 on error. +// +// WARNING: EVP_DecodeBlock's return value does not take padding into +// account. It also strips leading whitespace and trailing +// whitespace and minuses. OPENSSL_EXPORT int EVP_DecodeBlock(uint8_t *dst, const uint8_t *src, size_t src_len); -/** @} Deprecated Functions */ -/** - * @struct evp_encode_ctx_st - * Encoding Context - */ struct evp_encode_ctx_st { - /** - * @brief Number of valid bytes - * - * @details - * When encoding, `data` will be filled and encoded as a lump. When decoding, - * only the first four bytes of `data` will be used. - */ + // data_used indicates the number of bytes of |data| that are valid. When + // encoding, |data| will be filled and encoded as a lump. When decoding, only + // the first four bytes of |data| will be used. unsigned data_used; - - /** - * @brief Encoded or decoded data. - */ uint8_t data[48]; - /** - * @brief Indicates that the end of the base64 data has been seen. - * - * @details - * Only used when decoding. Only whitespace can follow. - */ + // eof_seen indicates that the end of the base64 data has been seen when + // decoding. Only whitespace can follow. char eof_seen; - /** - * @brief indicates that invalid base64 data was found. - * - * @details - * This will gitcause all future calls to fail. - */ + // error_encountered indicates that invalid base64 data was found. This will + // cause all future calls to fail. char error_encountered; }; From bdf127b14ac0ab06b601cf599d95747809536156 Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 20 Apr 2026 17:02:27 -0700 Subject: [PATCH 03/10] swap out gha for pages publishing --- .github/workflows/api-docs.yml | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/.github/workflows/api-docs.yml b/.github/workflows/api-docs.yml index 199e30db362..76a0eb7fc69 100644 --- a/.github/workflows/api-docs.yml +++ b/.github/workflows/api-docs.yml @@ -1,5 +1,6 @@ name: API Documentation on: + workflow_dispatch: push: branches: [main] paths: @@ -10,11 +11,20 @@ on: - '.github/workflows/api-docs.yml' permissions: - contents: write + contents: read + pages: write + id-token: write + +concurrency: + group: pages + cancel-in-progress: false jobs: publish-api-docs: runs-on: ubuntu-latest + environment: + name: github-pages + url: ${{ steps.deployment.outputs.page_url }} steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v4 @@ -24,10 +34,11 @@ jobs: run: | mkdir -p api-output cd util - go run doc.go --config doc.config --out ../api-output - - name: Deploy to gh-pages - uses: peaceiris/actions-gh-pages@v4 + go run doc.go --config doc.config --out ../pages-output + - name: Upload pages artifact + uses: actions/upload-pages-artifact@v3 with: - github_token: ${{ secrets.GITHUB_TOKEN }} - publish_dir: ./api-output - destination_dir: api + path: ./pages-output + - name: Deploy to GitHub Pages + id: deployment + uses: actions/deploy-pages@v4 From c9cbc308b44ee58e47fc3f5ea7cbf4cd60f2473b Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Tue, 21 Apr 2026 09:33:15 -0700 Subject: [PATCH 04/10] incorrect dir --- .github/workflows/api-docs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/api-docs.yml b/.github/workflows/api-docs.yml index 76a0eb7fc69..9e589db2ee0 100644 --- a/.github/workflows/api-docs.yml +++ b/.github/workflows/api-docs.yml @@ -32,7 +32,7 @@ jobs: go-version: ">=1.22" - name: Generate API documentation run: | - mkdir -p api-output + mkdir -p pages-output cd util go run doc.go --config doc.config --out ../pages-output - name: Upload pages artifact From 492997aeab4244ad1c8578ca80df26faf7531171 Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 10 Aug 2026 18:44:42 +0000 Subject: [PATCH 05/10] Add vmclock backend for VM UBE detection, preferring it over SysGenID Add support for the non-proprietary vmclock interface (/dev/vmclock0) for VM UBE detection, preferred over /dev/sysgenid when available and falling back to it otherwise (V2082724891). SysGenID is retained, not removed. The generation counter widens to uint64_t and vmclock is read via the spec's seqlock protocol with bounded retries. Test wiring gains a per-backend flag so single-backend builds only touch their own stand-in file; CI builds each backend separately. --- BUILDING.md | 29 ++- CMakeLists.txt | 26 ++- crypto/fipsmodule/rand/rand_test.cc | 2 +- crypto/rand_extra/urandom_test.cc | 3 +- crypto/test/gtest_main.cc | 7 +- crypto/ube/internal.h | 2 +- crypto/ube/ube.c | 10 +- crypto/ube/ube_test.cc | 86 ++++++-- crypto/ube/vm_ube_detect.c | 284 ++++++++++++++++++++++---- crypto/ube/vm_ube_detect.h | 54 +++-- crypto/ube/vm_ube_detect_test.cc | 296 ++++++++++++++++++++++++++-- crypto/ube/vmclock_abi.h | 82 ++++++++ ssl/test/bssl_shim.cc | 7 +- ssl/test/handshaker.cc | 7 +- tests/ci/run_fips_tests.sh | 18 +- tests/ci/run_posix_tests.sh | 27 ++- 16 files changed, 846 insertions(+), 94 deletions(-) create mode 100644 crypto/ube/vmclock_abi.h diff --git a/BUILDING.md b/BUILDING.md index 1f8793f8b89..a76f081c58e 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -229,15 +229,38 @@ More information on this can be found in [INCORPORATING.md](/INCORPORATING.md). # VM UBE Detection A VMM can snapshot, clone, and restore VMs. AWS-LC supports VM UBE-type -uniqueness breaking event detection on Linux using SysGenID -(https://lkml.org/lkml/2021/3/8/677). This mechanism is used for security -hardening. If a SysGenID interface is not found, then the mechanism is ignored. +uniqueness breaking event detection on Linux. This mechanism is used for +security hardening. Two backends are supported: + +- **vmclock** (preferred) via `/dev/vmclock0`, following the + [vmclock specification](https://uapi-group.org/specifications/specs/vmclock/). +- **SysGenID** (fallback) via `/dev/sysgenid` + (https://lkml.org/lkml/2021/3/8/677). + +At initialization AWS-LC prefers vmclock: if `/dev/vmclock0` is present and +usable it is used, otherwise AWS-LC falls back to `/dev/sysgenid`. If neither +interface is found, the mechanism is ignored. ## VM UBE Prerequisites VM snapshots taken on active hosts can potentially be unsafe to use. See "Snapshot Safety Prerequisites" here: https://lkml.org/lkml/2021/3/8/677 +## Testing VM UBE Detection + +The device nodes are not generally available in build/test environments, so +each backend can be pointed at a regular file that stands in for its device +node: + +- `-DTEST_SYSGENID_PATH=` exercises the SysGenID backend. +- `-DTEST_VMCLOCK_PATH=` exercises the vmclock backend. + +Setting either option enables VM UBE test mode. Because vmclock is preferred at +runtime, enabling both in a single build only exercises vmclock; to exercise the +SysGenID path (including the vmclock-to-SysGenID fallback), build with +`-DTEST_SYSGENID_PATH` alone. The CI scripts under `tests/ci/` build each backend +in a separate configuration for this reason. + # FIPS Mode For more details on building AWS-LC in FIPS mode, see the [crypto/fipsmodule/FIPS.md](crypto/fipsmodule/FIPS.md). diff --git a/CMakeLists.txt b/CMakeLists.txt index 5d90afcaa0e..25b7694b1d6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -313,11 +313,31 @@ install(DIRECTORY include/openssl PATTERN "*.in" EXCLUDE ) -if (TEST_SYSGENID_PATH) +# VM UBE (Uniqueness Breaking Event) detection can be exercised in tests by +# pointing a backend at a regular file that stands in for its device node. +# TEST_SYSGENID_PATH and TEST_VMCLOCK_PATH are independent: setting either one +# enables VM UBE test mode (AWSLC_VM_UBE_TESTING) and additionally sets a +# per-backend flag (AWSLC_TEST_SYSGENID / AWSLC_TEST_VMCLOCK) so that only the +# backend actually under test has its stand-in file created at startup. A build +# that enables only one backend must NOT try to create the other's file, which +# would point at the real /dev node and fail. +# +# Because vmclock is preferred over sysgenid at runtime, enabling both in a +# single build only exercises vmclock. To exercise the sysgenid path (including +# the vmclock->sysgenid fallback), build with TEST_SYSGENID_PATH alone. +if (TEST_SYSGENID_PATH OR TEST_VMCLOCK_PATH) message(STATUS "Setting AWSLC_VM_UBE_TESTING=1") add_definitions(-DAWSLC_VM_UBE_TESTING=1) - message(STATUS "Setting AWSLC_SYSGENID_PATH=${TEST_SYSGENID_PATH}") - add_definitions(-DAWSLC_SYSGENID_PATH=\"${TEST_SYSGENID_PATH}\") + if (TEST_SYSGENID_PATH) + message(STATUS "Setting AWSLC_SYSGENID_PATH=${TEST_SYSGENID_PATH}") + add_definitions(-DAWSLC_SYSGENID_PATH=\"${TEST_SYSGENID_PATH}\") + add_definitions(-DAWSLC_TEST_SYSGENID=1) + endif() + if (TEST_VMCLOCK_PATH) + message(STATUS "Setting AWSLC_VMCLOCK_PATH=${TEST_VMCLOCK_PATH}") + add_definitions(-DAWSLC_VMCLOCK_PATH=\"${TEST_VMCLOCK_PATH}\") + add_definitions(-DAWSLC_TEST_VMCLOCK=1) + endif() endif() if(NOT DISABLE_PERL) diff --git a/crypto/fipsmodule/rand/rand_test.cc b/crypto/fipsmodule/rand/rand_test.cc index 1e3f5826721..2b4b970a883 100644 --- a/crypto/fipsmodule/rand/rand_test.cc +++ b/crypto/fipsmodule/rand/rand_test.cc @@ -233,7 +233,7 @@ TEST_F(randTest, UbeDetectionMocked) { MockedUbeDetection( [](uint64_t gn) { - set_vm_ube_generation_number_FOR_TESTING(static_cast(gn)); + set_vm_ube_generation_number_FOR_TESTING(gn); } ); } diff --git a/crypto/rand_extra/urandom_test.cc b/crypto/rand_extra/urandom_test.cc index ca0fe5169dd..a5373fc8190 100644 --- a/crypto/rand_extra/urandom_test.cc +++ b/crypto/rand_extra/urandom_test.cc @@ -288,7 +288,8 @@ static void GetTrace(std::vector *out_trace, unsigned flags, break; } - if (filename != CRYPTO_get_sysgenid_path()) { + if (filename != CRYPTO_get_sysgenid_path() && + filename != CRYPTO_get_vmclock_path()) { out_trace->push_back(Event::Open(filename)); } diff --git a/crypto/test/gtest_main.cc b/crypto/test/gtest_main.cc index 8a415b769af..281caa97f34 100644 --- a/crypto/test/gtest_main.cc +++ b/crypto/test/gtest_main.cc @@ -15,11 +15,16 @@ int main(int argc, char **argv) { -#if defined(OPENSSL_LINUX) && defined(AWSLC_VM_UBE_TESTING) +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_SYSGENID) if (1 != HAZMAT_init_sysgenid_file()) { abort(); } #endif +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_VMCLOCK) + if (1 != HAZMAT_init_vmclock_file()) { + abort(); + } +#endif testing::InitGoogleTest(&argc, argv); bssl::SetupGoogleTest(); diff --git a/crypto/ube/internal.h b/crypto/ube/internal.h index 15dbf44a196..3f107d4f9de 100644 --- a/crypto/ube/internal.h +++ b/crypto/ube/internal.h @@ -44,7 +44,7 @@ OPENSSL_EXPORT void set_fork_ube_generation_number_FOR_TESTING(uint64_t fork_gn) // vm_ube detection. // |allow_mocked_ube_detection_FOR_TESTING| must have been invoked (once // per-process) to allow mocking the vm_ube generation number. -OPENSSL_EXPORT void set_vm_ube_generation_number_FOR_TESTING(uint32_t vm_ube_gn); +OPENSSL_EXPORT void set_vm_ube_generation_number_FOR_TESTING(uint64_t vm_ube_gn); // allow_mocked_ube_detection_FOR_TESTING allows mocking UBE detection even // though real detection is not available. This function must be called in diff --git a/crypto/ube/ube.c b/crypto/ube/ube.c index 4ef0892d5c9..6ddb51f42d0 100644 --- a/crypto/ube/ube.c +++ b/crypto/ube/ube.c @@ -52,14 +52,14 @@ void set_fork_ube_generation_number_FOR_TESTING(uint64_t fork_gn) { CRYPTO_STATIC_MUTEX_unlock_write(&ube_testing_lock); } -static uint32_t override_vm_ube_generation_number = 0; -void set_vm_ube_generation_number_FOR_TESTING(uint32_t vm_ube_gn) { +static uint64_t override_vm_ube_generation_number = 0; +void set_vm_ube_generation_number_FOR_TESTING(uint64_t vm_ube_gn) { CRYPTO_STATIC_MUTEX_lock_write(&ube_testing_lock); override_vm_ube_generation_number = vm_ube_gn; CRYPTO_STATIC_MUTEX_unlock_write(&ube_testing_lock); } -static int get_vm_ube_generation_number(uint32_t *gn) { +static int get_vm_ube_generation_number(uint64_t *gn) { if (allow_mocked_detection == 1) { CRYPTO_STATIC_MUTEX_lock_read(&ube_testing_lock); *gn = override_vm_ube_generation_number; @@ -97,7 +97,7 @@ static int get_fork_generation_number(uint64_t *gn) { struct ube_state { uint64_t generation_number; uint64_t cached_fork_gn; - uint32_t cached_vm_ube_gn; + uint64_t cached_vm_ube_gn; }; static struct ube_state ube_global_state = { 0, 0, 0 }; @@ -106,7 +106,7 @@ static struct ube_state ube_global_state = { 0, 0, 0 }; struct detection_gn { #define NUMBER_OF_DETECTION_GENERATION_NUMBERS 2 uint64_t current_fork_gn; - uint32_t current_vm_ube_gn; + uint64_t current_vm_ube_gn; }; // set_ube_detection_unavailable_once is the single mutation point of diff --git a/crypto/ube/ube_test.cc b/crypto/ube/ube_test.cc index 1a8e9b56e49..9c52f568e34 100644 --- a/crypto/ube/ube_test.cc +++ b/crypto/ube/ube_test.cc @@ -54,19 +54,19 @@ TEST_F(ubeGenerationNumberTest, BasicTests) { } static void MockedDetectionMethodTest( - std::function set_method_generation_number) { + std::function set_method_generation_number) { uint64_t generation_number = 0; uint64_t cached_generation_number = 0; - uint32_t mocked_generation_number = 0; + uint64_t mocked_generation_number = 0; uint8_t initial_mocked_generation_number[4] = {0}; ASSERT_TRUE(RAND_bytes(initial_mocked_generation_number, 4)); mocked_generation_number = - ((uint32_t)initial_mocked_generation_number[0] << 24) | - ((uint32_t)initial_mocked_generation_number[1] << 16) | - ((uint32_t)initial_mocked_generation_number[2] << 8) | - ((uint32_t)initial_mocked_generation_number[3]); + ((uint64_t)initial_mocked_generation_number[0] << 24) | + ((uint64_t)initial_mocked_generation_number[1] << 16) | + ((uint64_t)initial_mocked_generation_number[2] << 8) | + ((uint64_t)initial_mocked_generation_number[3]); // Testing that UBE generation number is incremented when: // mocked_generation_number + 1 @@ -133,32 +133,92 @@ TEST_F(ubeGenerationNumberTest, MockedDetectionMethodTests) { allowMockedUbe(); MockedDetectionMethodTest( - [](uint32_t gn) { - set_fork_ube_generation_number_FOR_TESTING(static_cast(gn)); + [](uint64_t gn) { + set_fork_ube_generation_number_FOR_TESTING(gn); } ); MockedDetectionMethodTest( - [](uint32_t gn) { + [](uint64_t gn) { set_vm_ube_generation_number_FOR_TESTING(gn); } ); MockedDetectionMethodTest( - [](uint32_t gn) { - set_fork_ube_generation_number_FOR_TESTING(static_cast(gn)); + [](uint64_t gn) { + set_fork_ube_generation_number_FOR_TESTING(gn); set_vm_ube_generation_number_FOR_TESTING(gn); } ); MockedDetectionMethodTest( - [](uint32_t gn) { - set_fork_ube_generation_number_FOR_TESTING(static_cast(gn)); + [](uint64_t gn) { + set_fork_ube_generation_number_FOR_TESTING(gn); set_vm_ube_generation_number_FOR_TESTING(gn + 1); } ); } +// Exercises the vm_ube generation number across the full 64-bit range. vmclock +// exposes a 64-bit vm_generation_counter (unlike the legacy 32-bit sysgenid), +// so the orchestration layer must detect changes in the high 32 bits and in +// values that exceed 2^32. |MockedDetectionMethodTest| above only covers a +// 32-bit-range value, so this guards the widening end-to-end. +TEST_F(ubeGenerationNumberTest, MockedVmUbe64BitValues) { + allowMockedUbe(); + + // A sequence of distinct 64-bit values. Consecutive entries differ only in + // the high 32 bits, only in the low 32 bits, or wrap across the 2^32 + // boundary -- each transition must be detected as exactly one UBE. + const uint64_t values[] = { + 0x0000000000000001ULL, + 0x0000000100000001ULL, // high half changed, low half identical + 0x0000000100000002ULL, // low half changed, high half identical + 0x00000000FFFFFFFFULL, // drop below 2^32 + 0x0000000100000000ULL, // cross the 2^32 boundary + 0xFFFFFFFFFFFFFFFFULL, // all bits set + 0x8000000000000000ULL, // high bit only + }; + + uint64_t generation_number = 0; + set_vm_ube_generation_number_FOR_TESTING(values[0]); + ASSERT_TRUE(CRYPTO_get_ube_generation_number(&generation_number)); + + for (size_t i = 1; i < sizeof(values) / sizeof(values[0]); i++) { + uint64_t before = generation_number; + + // Changing the mocked vm_ube generation number must bump the UBE + // generation number exactly once. + set_vm_ube_generation_number_FOR_TESTING(values[i]); + generation_number = 0; + ASSERT_TRUE(CRYPTO_get_ube_generation_number(&generation_number)); + ASSERT_EQ(generation_number, before + 1) << "at index " << i; + + // Stable when the value does not change. + uint64_t stable = 0; + ASSERT_TRUE(CRYPTO_get_ube_generation_number(&stable)); + ASSERT_EQ(stable, generation_number) << "instability at index " << i; + } +} + +// A change confined entirely to the high 32 bits of the vm_ube generation +// number must still be detected. A 32-bit-truncating implementation would miss +// this (both values alias to 0 in the low 32 bits) and fail to reseed. +TEST_F(ubeGenerationNumberTest, MockedVmUbeHighBitsOnlyChange) { + allowMockedUbe(); + + uint64_t generation_number = 0; + set_vm_ube_generation_number_FOR_TESTING(0x0000000000000000ULL + 0x100000000ULL); + ASSERT_TRUE(CRYPTO_get_ube_generation_number(&generation_number)); + + uint64_t before = generation_number; + // Low 32 bits stay 0; only the high 32 bits differ. + set_vm_ube_generation_number_FOR_TESTING(0x200000000ULL); + generation_number = 0; + ASSERT_TRUE(CRYPTO_get_ube_generation_number(&generation_number)); + ASSERT_EQ(generation_number, before + 1); +} + TEST_F(ubeGenerationNumberTest, ExpectedSupportTests) { uint64_t generation_number = 0; // Operating systems where we expect UBE detection to be enabled. diff --git a/crypto/ube/vm_ube_detect.c b/crypto/ube/vm_ube_detect.c index 9e7aa36902c..5a6a12c89f4 100644 --- a/crypto/ube/vm_ube_detect.c +++ b/crypto/ube/vm_ube_detect.c @@ -7,89 +7,254 @@ #if defined(OPENSSL_LINUX) #include +// This file is compiled only for Linux, where the toolchains we support +// (GCC/Clang) always provide working, lock-free C11 atomics. We use +// directly for the acquire fence in the vmclock seqlock reader +// rather than the tree's |CRYPTO_atomic_*| refcount helpers, which do not +// expose a general-purpose fence. +#include #include +#include #include #include #include #include "../internal.h" +#include "vmclock_abi.h" -// VM UBE state -#define VM_UBE_STATE_FAILED_INITIALISE 0x00 +// VM UBE state. A backend either initializes successfully or VM UBE detection +// is unavailable ("not supported"). There is deliberately no hard-failure +// state: an inaccessible or invalid device degrades to NOT_SUPPORTED so that +// the independent fork detection in ube.c keeps working (see do_vm_ube_init). #define VM_UBE_STATE_SUCCESS_INITIALISE 0x01 #define VM_UBE_STATE_NOT_SUPPORTED 0x02 +// VM UBE backend type +#define VM_UBE_BACKEND_NONE 0x00 +#define VM_UBE_BACKEND_VMCLOCK 0x01 +#define VM_UBE_BACKEND_SYSGENID 0x02 + +// Result of attempting to initialize a single detection backend. +#define VM_UBE_BACKEND_NOT_PRESENT 0x00 // device file does not exist +#define VM_UBE_BACKEND_INITIALISED 0x01 // device present and usable +#define VM_UBE_BACKEND_UNAVAILABLE 0x02 // device present but not usable by us + // (e.g. EACCES, mmap failure, or the + // device is not a valid vmclock) + +// Upper bound on seqlock read retries. The vmclock seqcount only advances +// while the VMM is mid-update, which is momentary, so a handful of retries is +// always sufficient in practice. The bound exists purely so that a wedged or +// corrupt |seq_count| (e.g. one stuck at an odd value) cannot spin this +// function -- and therefore |RAND_bytes| -- forever. +#define VMCLOCK_SEQLOCK_MAX_RETRIES 1024 + static CRYPTO_once_t vm_ube_init = CRYPTO_ONCE_INIT; static int vm_ube_state = 0; +static int vm_ube_backend = VM_UBE_BACKEND_NONE; // SysGenID generation number pointer static volatile uint32_t *sgn_addr = NULL; -static void do_sysgenid_init(void) { - vm_ube_state = VM_UBE_STATE_NOT_SUPPORTED; - sgn_addr = NULL; +// vmclock mapped region +static volatile struct vmclock_abi *vmclock_addr = NULL; +// try_vmclock_init attempts to initialize the vmclock backend. It returns one +// of the VM_UBE_BACKEND_* result codes. A non-present device, or a device that +// is present but not usable by this process (open/mmap failure such as EACCES, +// bad magic, or missing generation-counter flag), both leave the caller free to +// try the next backend and ultimately degrade to "not supported". +static int try_vmclock_init(void) { struct stat buff; - if (stat(CRYPTO_get_sysgenid_path(), &buff) != 0) { - return; + if (stat(CRYPTO_get_vmclock_path(), &buff) != 0) { + return VM_UBE_BACKEND_NOT_PRESENT; } - - vm_ube_state = VM_UBE_STATE_FAILED_INITIALISE; - int fd_sgn = open(CRYPTO_get_sysgenid_path(), O_RDONLY); - if (fd_sgn == -1) { - return; + // The device node exists but may not be usable by this process. A common + // case is /dev/vmclock0 being root-only (crw-------): an unprivileged process + // will get EACCES here. That is not an error -- we simply cannot use vmclock, + // so report it as unavailable and let detection fall through / degrade. + int fd = open(CRYPTO_get_vmclock_path(), O_RDONLY); + if (fd == -1) { + return VM_UBE_BACKEND_UNAVAILABLE; } - void *addr = mmap(NULL, sizeof(uint32_t), PROT_READ, MAP_SHARED, fd_sgn, 0); + void *addr = mmap(NULL, sizeof(struct vmclock_abi), PROT_READ, MAP_SHARED, + fd, 0); + close(fd); - // Can close file descriptor now per - // https://man7.org/linux/man-pages/man2/mmap.2.html: "After the mmap() call - // has returned, the file descriptor, fd, can be closed immediately without - // invalidating the mapping.". We have initialised vm_ube_state without errors - // and this function is only executed once. Therefore, try to close file - // descriptor but don't error if it fails. */ - close(fd_sgn); + if (addr == MAP_FAILED) { + return VM_UBE_BACKEND_UNAVAILABLE; + } + + volatile struct vmclock_abi *vmc = (volatile struct vmclock_abi *)addr; + + // |magic| is a constant field (never touched by the seqlock), so it is safe + // to read directly. On a big-endian host this comparison fails and we treat + // the device as unavailable; see the note in vmclock_abi.h. + if (vmc->magic != VMCLOCK_MAGIC) { + munmap(addr, sizeof(struct vmclock_abi)); + return VM_UBE_BACKEND_UNAVAILABLE; + } + + // |flags| lives in the seqlock-protected region, but this runs once at + // init from |CRYPTO_once| against a freshly mapped device, so a concurrent + // VMM update racing this single read is not a concern. Even if |flags| were + // read torn, the only consequence is mis-detecting the feature bit, which + // fails closed to the next backend -- never a wrong generation number. + uint64_t flags = vmc->flags; + if (!(flags & VMCLOCK_FLAG_VM_GEN_COUNTER_PRESENT)) { + munmap(addr, sizeof(struct vmclock_abi)); + return VM_UBE_BACKEND_UNAVAILABLE; + } + + vmclock_addr = vmc; + return VM_UBE_BACKEND_INITIALISED; +} + +static int try_sysgenid_init(void) { + struct stat buff; + if (stat(CRYPTO_get_sysgenid_path(), &buff) != 0) { + return VM_UBE_BACKEND_NOT_PRESENT; + } + + int fd = open(CRYPTO_get_sysgenid_path(), O_RDONLY); + if (fd == -1) { + return VM_UBE_BACKEND_UNAVAILABLE; + } + + void *addr = mmap(NULL, sizeof(uint32_t), PROT_READ, MAP_SHARED, fd, 0); + close(fd); if (addr == MAP_FAILED) { - return; + return VM_UBE_BACKEND_UNAVAILABLE; } - // sgn_addr will now point at the mapped memory and any 4-byte read from - // this pointer will correspond to the sgn managed by the VMM. sgn_addr = addr; - vm_ube_state = VM_UBE_STATE_SUCCESS_INITIALISE; + return VM_UBE_BACKEND_INITIALISED; } -static uint32_t vm_ube_read_sysgenid_gn(void) { - if (vm_ube_state == VM_UBE_STATE_SUCCESS_INITIALISE) { - return *sgn_addr; +static void do_vm_ube_init(void) { + vm_ube_state = VM_UBE_STATE_NOT_SUPPORTED; + vm_ube_backend = VM_UBE_BACKEND_NONE; + sgn_addr = NULL; + vmclock_addr = NULL; + + // Try vmclock first (preferred). Crucially, if vmclock is present but not + // usable by us -- e.g. |open|/|mmap| fails (EACCES on a root-only device), + // or the VMM exposes the device without the generation-counter flag -- we + // must still fall through to sysgenid. On a host that carries both devices + // during the sysgenid -> vmclock transition, letting a vmclock hiccup disable + // detection outright would silently drop UBE reseeding, which is the whole + // reason this code exists. + if (try_vmclock_init() == VM_UBE_BACKEND_INITIALISED) { + vm_ube_backend = VM_UBE_BACKEND_VMCLOCK; + vm_ube_state = VM_UBE_STATE_SUCCESS_INITIALISE; + return; + } + + if (try_sysgenid_init() == VM_UBE_BACKEND_INITIALISED) { + vm_ube_backend = VM_UBE_BACKEND_SYSGENID; + vm_ube_state = VM_UBE_STATE_SUCCESS_INITIALISE; + return; + } + + // No backend initialized. Whether a device was entirely absent + // (NOT_PRESENT) or was present but not usable by this process (UNAVAILABLE), + // VM UBE detection is simply not available here -- degrade to "not + // supported". This is intentionally NOT a hard failure: a hard failure + // propagates up through ube.c and disables *all* UBE detection (including the + // independent fork detection) and forces the DRBG to reseed on every request. + // A common trigger is an unprivileged process on a host where /dev/vmclock0 + // is root-only; that process must still get fork detection and normal reseed + // behaviour. + vm_ube_state = VM_UBE_STATE_NOT_SUPPORTED; +} + +#if defined(AWSLC_VM_UBE_TESTING) +// HAZMAT_reinit_vm_ube_FOR_TESTING re-runs backend initialization against the +// current on-disk state of the stand-in device file(s). It exists so tests can +// observe |do_vm_ube_init|'s behaviour for a device that is present but not +// usable at init time (e.g. corrupt contents or EACCES) -- something the normal +// once-per-process |CRYPTO_once| path, already completed against a valid file, +// cannot exercise. It must only be called from a single-threaded test context. +void HAZMAT_reinit_vm_ube_FOR_TESTING(void) { + if (vmclock_addr != NULL) { + munmap((void *)vmclock_addr, sizeof(struct vmclock_abi)); + vmclock_addr = NULL; + } + if (sgn_addr != NULL) { + munmap((void *)sgn_addr, sizeof(uint32_t)); + sgn_addr = NULL; } + do_vm_ube_init(); +} +#endif + +// vm_ube_read_vmclock_gn reads the vmclock generation counter using the +// seqlock protocol described in the vmclock specification. On success it writes +// the value to |*out| and returns 1. It returns 0 if it cannot obtain a +// consistent read within |VMCLOCK_SEQLOCK_MAX_RETRIES| attempts. +static int vm_ube_read_vmclock_gn(uint64_t *out) { + for (size_t i = 0; i < VMCLOCK_SEQLOCK_MAX_RETRIES; i++) { + uint32_t seq = vmclock_addr->seq_count & ~1u; + // Acquire fence pairs with the VMM's release fence: it ensures the + // |seq_count| read is not reordered after the |vm_generation_counter| read. + atomic_thread_fence(memory_order_acquire); + + uint64_t value = vmclock_addr->vm_generation_counter; + + // Acquire fence ensures the second |seq_count| read is not reordered before + // the |vm_generation_counter| read. + atomic_thread_fence(memory_order_acquire); + if (seq == vmclock_addr->seq_count) { + *out = value; + return 1; + } + } + return 0; +} +static int vm_ube_read_sysgenid_gn(uint64_t *out) { + *out = (uint64_t)*sgn_addr; + return 1; +} + +// vm_ube_read_generation reads the active backend's generation number into +// |*out|. Returns 1 on success and 0 on failure. +static int vm_ube_read_generation(uint64_t *out) { + if (vm_ube_backend == VM_UBE_BACKEND_VMCLOCK) { + return vm_ube_read_vmclock_gn(out); + } + if (vm_ube_backend == VM_UBE_BACKEND_SYSGENID) { + return vm_ube_read_sysgenid_gn(out); + } return 0; } -int CRYPTO_get_vm_ube_generation(uint32_t *vm_ube_generation_number) { - CRYPTO_once(&vm_ube_init, do_sysgenid_init); +int CRYPTO_get_vm_ube_generation(uint64_t *vm_ube_generation_number) { + CRYPTO_once(&vm_ube_init, do_vm_ube_init); switch (vm_ube_state) { case VM_UBE_STATE_NOT_SUPPORTED: *vm_ube_generation_number = 0; return 1; case VM_UBE_STATE_SUCCESS_INITIALISE: - *vm_ube_generation_number = vm_ube_read_sysgenid_gn(); + if (vm_ube_read_generation(vm_ube_generation_number) != 1) { + // A backend that initialized successfully but now cannot produce a + // consistent read (e.g. a wedged vmclock seqlock) is treated as a + // failure so the caller reseeds conservatively rather than trusting a + // stale or torn value. + *vm_ube_generation_number = 0; + return 0; + } return 1; - case VM_UBE_STATE_FAILED_INITIALISE: - *vm_ube_generation_number = 0; - return 0; default: - // No other state should be possible. abort(); } } int CRYPTO_get_vm_ube_active(void) { - CRYPTO_once(&vm_ube_init, do_sysgenid_init); + CRYPTO_once(&vm_ube_init, do_vm_ube_init); if (vm_ube_state == VM_UBE_STATE_SUCCESS_INITIALISE) { return 1; @@ -99,7 +264,7 @@ int CRYPTO_get_vm_ube_active(void) { } int CRYPTO_get_vm_ube_supported(void) { - CRYPTO_once(&vm_ube_init, do_sysgenid_init); + CRYPTO_once(&vm_ube_init, do_vm_ube_init); if (vm_ube_state == VM_UBE_STATE_NOT_SUPPORTED) { return 0; @@ -110,7 +275,7 @@ int CRYPTO_get_vm_ube_supported(void) { #else // !defined(OPENSSL_LINUX) -int CRYPTO_get_vm_ube_generation(uint32_t *vm_ube_generation_number) { +int CRYPTO_get_vm_ube_generation(uint64_t *vm_ube_generation_number) { *vm_ube_generation_number = 0; return 1; } @@ -125,7 +290,11 @@ const char* CRYPTO_get_sysgenid_path(void) { return AWSLC_SYSGENID_PATH; } -#if defined(OPENSSL_LINUX) && defined(AWSLC_VM_UBE_TESTING) +const char* CRYPTO_get_vmclock_path(void) { + return AWSLC_VMCLOCK_PATH; +} + +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_SYSGENID) int HAZMAT_init_sysgenid_file(void) { int fd_sgn = open(CRYPTO_get_sysgenid_path(), O_CREAT | O_RDWR, S_IRWXU | S_IRGRP | S_IROTH); @@ -155,3 +324,42 @@ int HAZMAT_init_sysgenid_file(void) { return 1; } #endif + +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_VMCLOCK) +int HAZMAT_init_vmclock_file(void) { + int fd = open(CRYPTO_get_vmclock_path(), O_CREAT | O_RDWR, + S_IRWXU | S_IRGRP | S_IROTH); + if (fd == -1) { + return 0; + } + + if (0 != lseek(fd, 0, SEEK_SET)) { + close(fd); + return 0; + } + + // Always write a valid vmclock structure at the start of the file. + struct vmclock_abi vmc; + memset(&vmc, 0, sizeof(vmc)); + vmc.magic = VMCLOCK_MAGIC; + vmc.size = sizeof(struct vmclock_abi); + vmc.version = 1; + vmc.flags = VMCLOCK_FLAG_VM_GEN_COUNTER_PRESENT; + vmc.seq_count = 0; + vmc.vm_generation_counter = 0; + + if ((ssize_t)sizeof(vmc) != write(fd, &vmc, sizeof(vmc))) { + close(fd); + return 0; + } + + if (0 != fsync(fd)) { + close(fd); + return 0; + } + + close(fd); + + return 1; +} +#endif diff --git a/crypto/ube/vm_ube_detect.h b/crypto/ube/vm_ube_detect.h index 3fe1febc755..1af97a63dcc 100644 --- a/crypto/ube/vm_ube_detect.h +++ b/crypto/ube/vm_ube_detect.h @@ -14,6 +14,10 @@ extern "C" { #define AWSLC_SYSGENID_PATH "/dev/sysgenid" #endif +#if !defined(AWSLC_VMCLOCK_PATH) + #define AWSLC_VMCLOCK_PATH "/dev/vmclock0" +#endif + // VM UBE-type uniqueness breaking event (ube detection). // // CRYPTO_get_vm_ube_generation provides the VM UBE generation number for @@ -22,36 +26,60 @@ extern "C" { // space and then again in a subsequently resumed snapshot/VM, the resumed // address space will observe a greater value. // -// We use SysGenID to detect resumed snapshot/VM events. See -// https://lkml.org/lkml/2021/3/8/677 for details about how SysGenID works. -// We make light use of the SysGenId capabilities and only use the following -// supported functions on the device: |open| and |mmap|. +// Two detection mechanisms are supported: +// 1. vmclock — Uses /dev/vmclock0 (preferred). See +// https://uapi-group.org/specifications/specs/vmclock/ for details. +// 2. SysGenID — Uses /dev/sysgenid (fallback). See +// https://lkml.org/lkml/2021/3/8/677 for details. +// +// vmclock is preferred when available. If neither is available, the function +// reports that VM UBE detection is not supported. // // |CRYPTO_get_vm_ube_generation| returns 0 only when the filesystem -// presents SysGenID interface (default is `/dev/sysgenid`) but we are -// is unable to initialize its use. Otherwise, it returns 1. +// presents a VM UBE interface but we are unable to initialize its use. +// Otherwise, it returns 1. OPENSSL_EXPORT int CRYPTO_get_vm_ube_generation( - uint32_t *vm_ube_generation_number); + uint64_t *vm_ube_generation_number); -// CRYPTO_get_vm_ube_active returns 1 if the file system presents the SysGenID -// interface and the library has successfully initialized its use. Otherwise, -// it returns 0. +// CRYPTO_get_vm_ube_active returns 1 if the file system presents a VM UBE +// interface (vmclock or SysGenID) and the library has successfully initialized +// its use. Otherwise, it returns 0. OPENSSL_EXPORT int CRYPTO_get_vm_ube_active(void); -// CRYPTO_get_vm_ube_supported returns 1 if the file system presents the -// SysGenID interface. Otherwise, it returns 0. +// CRYPTO_get_vm_ube_supported returns 1 if the file system presents a VM UBE +// interface (vmclock or SysGenID). Otherwise, it returns 0. OPENSSL_EXPORT int CRYPTO_get_vm_ube_supported(void); // CRYPTO_get_sysgenid_path returns the path used for the SysGenId interface. OPENSSL_EXPORT const char *CRYPTO_get_sysgenid_path(void); -#if defined(OPENSSL_LINUX) && defined(AWSLC_VM_UBE_TESTING) +// CRYPTO_get_vmclock_path returns the path used for the vmclock interface. +OPENSSL_EXPORT const char *CRYPTO_get_vmclock_path(void); + +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_SYSGENID) // HAZMAT_init_sysgenid_file should only be used for testing. It creates and // initializes the sysgenid path indicated by AWSLC_SYSGENID_PATH. // On success, it returns 1. Otherwise, returns 0. OPENSSL_EXPORT int HAZMAT_init_sysgenid_file(void); #endif +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_VMCLOCK) +// HAZMAT_init_vmclock_file should only be used for testing. It creates and +// initializes the vmclock path indicated by AWSLC_VMCLOCK_PATH. +// On success, it returns 1. Otherwise, returns 0. +OPENSSL_EXPORT int HAZMAT_init_vmclock_file(void); +#endif + +#if defined(OPENSSL_LINUX) && defined(AWSLC_VM_UBE_TESTING) +// HAZMAT_reinit_vm_ube_FOR_TESTING should only be used for testing. It unmaps +// any active backend mapping and re-runs VM UBE backend initialization against +// the current on-disk state of the stand-in device file(s). This lets tests +// exercise initialization outcomes (e.g. a device that is present but corrupt +// or inaccessible) that the once-per-process init path cannot otherwise reach. +// It must only be called from a single-threaded test context. +OPENSSL_EXPORT void HAZMAT_reinit_vm_ube_FOR_TESTING(void); +#endif + #ifdef __cplusplus } #endif diff --git a/crypto/ube/vm_ube_detect_test.cc b/crypto/ube/vm_ube_detect_test.cc index 96825028d9c..5c3b8595086 100644 --- a/crypto/ube/vm_ube_detect_test.cc +++ b/crypto/ube/vm_ube_detect_test.cc @@ -11,9 +11,18 @@ #include #include #include +#include +#include + +#include +#include + +#include "vmclock_abi.h" #define NUMBER_OF_TEST_VALUES 5 +#if defined(AWSLC_TEST_SYSGENID) +// Test helper for sysgenid backend typedef struct sgn_test_s { void *addr; } sgn_test_s; @@ -59,7 +68,271 @@ static int set_sgn(const sgn_test_s* sgn_test, uint32_t val) { } return 1; } +#endif // defined(AWSLC_TEST_SYSGENID) + +#if defined(AWSLC_TEST_VMCLOCK) +// Test helper for vmclock backend +typedef struct vmclock_test_s { + struct vmclock_abi *addr; +} vmclock_test_s; + +static int init_vmclock_test(vmclock_test_s* vmc_test) { + const int fd = open(CRYPTO_get_vmclock_path(), O_RDWR); + if (fd == -1) { + return 0; + } + + void* addr = mmap(nullptr, sizeof(struct vmclock_abi), PROT_WRITE | PROT_READ, + MAP_SHARED, fd, 0); + close(fd); + + if (addr == MAP_FAILED) { + return 0; + } + + vmc_test->addr = static_cast(addr); + return 1; +} + +static int set_vmclock_generation(vmclock_test_s* vmc_test, uint64_t val) { + // Use seqlock protocol: increment seq_count to odd, write, increment to even + vmc_test->addr->seq_count++; + __atomic_thread_fence(__ATOMIC_RELEASE); + vmc_test->addr->vm_generation_counter = val; + __atomic_thread_fence(__ATOMIC_RELEASE); + vmc_test->addr->seq_count++; + if (0 != msync(vmc_test->addr, sizeof(struct vmclock_abi), MS_SYNC)) { + return 0; + } + return 1; +} + +// set_vmclock_seq_count forces |seq_count| to an arbitrary value. A test uses +// this to leave the seqlock "held" (odd value), simulating a VMM that is +// perpetually mid-update or a corrupt mapping. +static int set_vmclock_seq_count(vmclock_test_s* vmc_test, uint32_t seq) { + vmc_test->addr->seq_count = seq; + if (0 != msync(vmc_test->addr, sizeof(struct vmclock_abi), MS_SYNC)) { + return 0; + } + return 1; +} + +TEST(VmUbeGenerationTest, DISABLED_VmclockRetrievalTesting) { + vmclock_test_s vmc_test; + ASSERT_TRUE(init_vmclock_test(&vmc_test)); + + EXPECT_EQ(1, CRYPTO_get_vm_ube_supported()); + EXPECT_EQ(1, CRYPTO_get_vm_ube_active()); + + uint64_t current_vm_ube_gen_num = 0; + ASSERT_TRUE(set_vmclock_generation(&vmc_test, 42)); + ASSERT_TRUE(CRYPTO_get_vm_ube_generation(¤t_vm_ube_gen_num)); + ASSERT_EQ((uint64_t)42, current_vm_ube_gen_num); + + // Test values that exercise the full 64-bit range + uint64_t test_vmclock_values[] = { + 0x03, + 0x100000003ULL, // > 32-bit + 0xFFFFFFFFULL, // 32-bit max + 0x100000000ULL, // Just above 32-bit max + 0xFFFFFFFFFFFFFFFFULL // 64-bit max + }; + + for (size_t i = 0; i < sizeof(test_vmclock_values) / sizeof(test_vmclock_values[0]); i++) { + ASSERT_TRUE(set_vmclock_generation(&vmc_test, test_vmclock_values[i])); + ASSERT_TRUE(CRYPTO_get_vm_ube_generation(¤t_vm_ube_gen_num)); + EXPECT_EQ(test_vmclock_values[i], current_vm_ube_gen_num); + } +} + +// A wedged seqlock (stuck at an odd value, i.e. a write perpetually "in +// progress") must not spin the reader forever. |CRYPTO_get_vm_ube_generation| +// must give up after a bounded number of retries and report failure (return 0), +// which the DRBG layer treats as "reseed conservatively". Once the seqlock is +// released (even value again), reads must succeed. +TEST(VmUbeGenerationTest, DISABLED_VmclockSeqlockWedged) { + vmclock_test_s vmc_test; + ASSERT_TRUE(init_vmclock_test(&vmc_test)); + + // Establish a known-good baseline. + uint64_t gen = 0; + ASSERT_TRUE(set_vmclock_generation(&vmc_test, 100)); + ASSERT_TRUE(CRYPTO_get_vm_ube_generation(&gen)); + ASSERT_EQ((uint64_t)100, gen); + + // Wedge the seqlock at an odd value. The generation counter underneath is + // irrelevant; the reader must never observe a consistent seq_count. + ASSERT_TRUE(set_vmclock_seq_count(&vmc_test, 0x7FFFFFFF)); + + // The read must terminate (bounded retries) and report failure rather than + // hang. This test hanging *is* the failure signal for the unbounded-loop bug. + gen = 0xdeadbeef; + ASSERT_EQ(0, CRYPTO_get_vm_ube_generation(&gen)); + ASSERT_EQ((uint64_t)0, gen); + + // Release the seqlock. Reset seq_count to an even value first: the seqlock + // was left odd (0x7FFFFFFF), and set_vmclock_generation() applies two + // increments, which from an odd start would land back on odd (still "held"). + ASSERT_TRUE(set_vmclock_seq_count(&vmc_test, 0)); + ASSERT_TRUE(set_vmclock_generation(&vmc_test, 101)); + ASSERT_TRUE(CRYPTO_get_vm_ube_generation(&gen)); + ASSERT_EQ((uint64_t)101, gen); +} + +// Hammer the reader while a background thread continuously updates the +// generation counter through the seqlock write protocol, writing the 64-bit +// value in two 32-bit halves so that a naive (non-seqlock) reader could observe +// a torn value. The reader must only ever return one of the two whole values +// that were actually written -- never a mix of their halves -- or fail cleanly. +TEST(VmUbeGenerationTest, DISABLED_VmclockConcurrentTornRead) { + vmclock_test_s vmc_test; + ASSERT_TRUE(init_vmclock_test(&vmc_test)); + + // Two values whose 32-bit halves are distinct bit patterns. Any torn read + // that mixes a low half from one write with a high half from the other + // yields a value equal to neither kValueA nor kValueB (e.g. 0xAAAAAAAA55555555 + // or 0x55555555AAAAAAAA). + const uint64_t kValueA = 0xAAAAAAAAAAAAAAAAULL; + const uint64_t kValueB = 0x5555555555555555ULL; + + // Start from a whole value. + ASSERT_TRUE(set_vmclock_generation(&vmc_test, kValueA)); + + std::atomic stop(false); + volatile struct vmclock_abi *abi = vmc_test.addr; + + std::thread writer([&]() { + bool toggle = false; + while (!stop.load(std::memory_order_relaxed)) { + uint64_t next = toggle ? kValueA : kValueB; + toggle = !toggle; + + volatile uint32_t *lo = + reinterpret_cast(&abi->vm_generation_counter); + volatile uint32_t *hi = lo + 1; + + // Enter the write section: make seq_count odd. + abi->seq_count++; + __atomic_thread_fence(__ATOMIC_RELEASE); + + // Write the two halves separately, leaving a window where the 64-bit + // value is a torn mix of the previous and next values. + *lo = (uint32_t)(next & 0xFFFFFFFFULL); + *hi = (uint32_t)(next >> 32); + + __atomic_thread_fence(__ATOMIC_RELEASE); + // Leave the write section: make seq_count even. + abi->seq_count++; + } + }); + + // Read many times. Every successful read must be a whole value; a failed + // read (0 return) is acceptable (writer happened to keep the lock held past + // the retry bound) but must never be a torn value. We record any torn read + // and stop, but must NOT return before join()ing the writer -- letting a + // std::thread destruct while joinable calls std::terminate() and would mask + // the real failure. + // 20k iterations is enough to land in the writer's torn window many times + // over (the writer flips continuously) while keeping runtime low even on a + // loaded CI host, where each read may spin up to the seqlock retry bound. + size_t whole_reads = 0; + bool torn_read = false; + uint64_t torn_value = 0; + for (size_t i = 0; i < 20000 && !torn_read; i++) { + uint64_t gen = 0; + if (CRYPTO_get_vm_ube_generation(&gen) == 1) { + if (gen != kValueA && gen != kValueB) { + torn_read = true; + torn_value = gen; + break; + } + whole_reads++; + } + } + + stop.store(true, std::memory_order_relaxed); + writer.join(); + + EXPECT_FALSE(torn_read) << "torn read observed: 0x" << std::hex << torn_value; + // Sanity: we should have gotten at least some consistent reads. + EXPECT_GT(whole_reads, (size_t)0); + + // Restore a clean, released state for any subsequent readers. + ASSERT_TRUE(set_vmclock_generation(&vmc_test, kValueA)); +} +// Regression test for graceful degradation when the vmclock device is present +// but not a valid vmclock (here: corrupt magic). Initialization must treat the +// backend as unavailable and degrade to "not supported" -- returning success +// with generation 0 -- NOT a hard failure. A hard failure would propagate +// through ube.c and disable all UBE detection (including fork detection) and +// force the DRBG to reseed on every request. This is uid-independent: it does +// not rely on file permissions, so it behaves identically as root or non-root. +TEST(VmUbeGenerationTest, DISABLED_VmclockPresentButInvalidDegradesGracefully) { + vmclock_test_s vmc_test; + ASSERT_TRUE(init_vmclock_test(&vmc_test)); + + // Corrupt the magic so the device no longer looks like a vmclock. + const uint32_t kOriginalMagic = vmc_test.addr->magic; + vmc_test.addr->magic = ~kOriginalMagic; + ASSERT_EQ(0, msync(vmc_test.addr, sizeof(struct vmclock_abi), MS_SYNC)); + + HAZMAT_reinit_vm_ube_FOR_TESTING(); + + // Present-but-invalid must degrade to "not supported", not hard failure. + EXPECT_EQ(0, CRYPTO_get_vm_ube_supported()); + EXPECT_EQ(0, CRYPTO_get_vm_ube_active()); + + // The generation query still "succeeds" (returns 1) with generation 0. This + // is the contract that keeps ube.c from disabling all UBE detection. + uint64_t gen = 0xdeadbeef; + EXPECT_EQ(1, CRYPTO_get_vm_ube_generation(&gen)); + EXPECT_EQ((uint64_t)0, gen); + + // Restore a valid device and re-init so later readers see a clean vmclock. + vmc_test.addr->magic = kOriginalMagic; + ASSERT_EQ(0, msync(vmc_test.addr, sizeof(struct vmclock_abi), MS_SYNC)); + HAZMAT_reinit_vm_ube_FOR_TESTING(); + EXPECT_EQ(1, CRYPTO_get_vm_ube_supported()); +} + +// Regression test for the exact production bug: on a host where /dev/vmclock0 +// exists but is not readable by the process (e.g. root-only crw-------), an +// unprivileged process must degrade gracefully rather than hard-fail. We +// reproduce the EACCES by removing read permission from the stand-in file. +// +// This only reproduces EACCES for a non-root process: root bypasses file +// permission checks, so the test is skipped when running as root rather than +// producing a misleading result. +TEST(VmUbeGenerationTest, DISABLED_VmclockInaccessibleDegradesGracefully) { + if (geteuid() == 0) { + GTEST_SKIP() << "root bypasses file permissions; cannot reproduce EACCES"; + } + + const char *path = CRYPTO_get_vmclock_path(); + + // Make the stand-in file unreadable so open(O_RDONLY) fails with EACCES, + // mirroring a root-only /dev/vmclock0 seen by an unprivileged process. + ASSERT_EQ(0, chmod(path, 0)); + + HAZMAT_reinit_vm_ube_FOR_TESTING(); + + // Inaccessible device must degrade to "not supported", not hard failure. + EXPECT_EQ(0, CRYPTO_get_vm_ube_supported()); + EXPECT_EQ(0, CRYPTO_get_vm_ube_active()); + uint64_t gen = 0xdeadbeef; + EXPECT_EQ(1, CRYPTO_get_vm_ube_generation(&gen)); + EXPECT_EQ((uint64_t)0, gen); + + // Restore readability and re-init so later readers see a usable vmclock. + ASSERT_EQ(0, chmod(path, S_IRWXU | S_IRGRP | S_IROTH)); + HAZMAT_reinit_vm_ube_FOR_TESTING(); + EXPECT_EQ(1, CRYPTO_get_vm_ube_supported()); +} +#endif // defined(AWSLC_TEST_VMCLOCK) + +#if defined(AWSLC_TEST_SYSGENID) TEST(VmUbeGenerationTest, DISABLED_SysGenIDretrievalTesting) { sgn_test_s sgn_test; ASSERT_TRUE(init_sgn_test(&sgn_test)); @@ -69,10 +342,10 @@ TEST(VmUbeGenerationTest, DISABLED_SysGenIDretrievalTesting) { EXPECT_EQ(1, CRYPTO_get_vm_ube_supported()); EXPECT_EQ(1, CRYPTO_get_vm_ube_active()); - uint32_t current_vm_ube_gen_num = 0; + uint64_t current_vm_ube_gen_num = 0; ASSERT_TRUE(set_sgn(&sgn_test, 7)); ASSERT_TRUE(CRYPTO_get_vm_ube_generation(¤t_vm_ube_gen_num)); - ASSERT_EQ((uint32_t) 7, current_vm_ube_gen_num); + ASSERT_EQ((uint64_t) 7, current_vm_ube_gen_num); uint32_t test_sysgenid_values[NUMBER_OF_TEST_VALUES] = { 0x03, // 2^0 + 2 @@ -83,33 +356,34 @@ TEST(VmUbeGenerationTest, DISABLED_SysGenIDretrievalTesting) { }; for (size_t i = 0; i < NUMBER_OF_TEST_VALUES; i++) { - // Exercise all bytes of the 32-bit generation number. uint32_t new_sysgenid_value_hint = test_sysgenid_values[i]; ASSERT_TRUE(set_sgn(&sgn_test, new_sysgenid_value_hint)); ASSERT_TRUE(CRYPTO_get_vm_ube_generation(¤t_vm_ube_gen_num)); - EXPECT_EQ(new_sysgenid_value_hint, current_vm_ube_gen_num); + EXPECT_EQ((uint64_t)new_sysgenid_value_hint, current_vm_ube_gen_num); } } +#endif // defined(AWSLC_TEST_SYSGENID) + #elif defined(OPENSSL_LINUX) TEST(VmUbeGenerationTest, SysGenIDretrievalLinux) { - uint32_t current_vm_ube_gen_num = 0xffffffff; + uint64_t current_vm_ube_gen_num = 0xffffffffffffffff; ASSERT_TRUE(CRYPTO_get_vm_ube_generation(¤t_vm_ube_gen_num)); if (CRYPTO_get_vm_ube_supported()) { ASSERT_TRUE(CRYPTO_get_vm_ube_active()); - // If we're on a system where the SysGenId is available, we won't - // know what sgn value to expect, but we assume it's not 0xffffffff - ASSERT_NE(0xffffffff, current_vm_ube_gen_num); + // If we're on a system where a VM UBE interface is available, we won't + // know what value to expect, but we assume it's not 0xffffffffffffffff + ASSERT_NE((uint64_t)0xffffffffffffffff, current_vm_ube_gen_num); } else { ASSERT_FALSE(CRYPTO_get_vm_ube_active()); - ASSERT_EQ((uint32_t) 0, current_vm_ube_gen_num); + ASSERT_EQ((uint64_t) 0, current_vm_ube_gen_num); } } #else TEST(VmUbeGenerationTest, SysGenIDretrievalNonLinux) { ASSERT_FALSE(CRYPTO_get_vm_ube_supported()); ASSERT_FALSE(CRYPTO_get_vm_ube_active()); - uint32_t current_vm_ube_gen_num = 0xffffffff; + uint64_t current_vm_ube_gen_num = 0xffffffffffffffff; ASSERT_TRUE(CRYPTO_get_vm_ube_generation(¤t_vm_ube_gen_num)); - ASSERT_EQ((uint32_t) 0, current_vm_ube_gen_num); + ASSERT_EQ((uint64_t) 0, current_vm_ube_gen_num); } #endif // defined(OPENSSL_LINUX) diff --git a/crypto/ube/vmclock_abi.h b/crypto/ube/vmclock_abi.h new file mode 100644 index 00000000000..4df6ca42ba4 --- /dev/null +++ b/crypto/ube/vmclock_abi.h @@ -0,0 +1,82 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + +#ifndef HEADER_VMCLOCK_ABI +#define HEADER_VMCLOCK_ABI + +#include + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +// This mirrors the Linux kernel's vmclock ABI (struct vmclock_abi in +// include/uapi/linux/vmclock-abi.h). See +// https://uapi-group.org/specifications/specs/vmclock/ for the specification. +// +// The on-device representation is little-endian. We read the fields natively +// (see vm_ube_detect.c), so on a big-endian host the |magic| comparison will +// fail and we fall through to another detection backend. This is intentional: +// we would rather disable vmclock on big-endian than byte-swap an interface we +// cannot exercise there. +// +// The field layout is defined so that every member is naturally aligned; the +// kernel struct is not packed and neither is this. The OPENSSL_STATIC_ASSERTs +// below pin the size and the offsets we actually dereference so that an +// accidental edit to this struct fails the build instead of silently shifting +// |vm_generation_counter|. + +#define VMCLOCK_MAGIC 0x4b4c4356 /* "VCLK" */ + +#define VMCLOCK_FLAG_VM_GEN_COUNTER_PRESENT (1ULL << 8) + +struct vmclock_abi { + /* Constant fields */ + uint32_t magic; + uint32_t size; + uint16_t version; + uint8_t counter_id; + uint8_t time_type; + + /* Non-constant fields protected by seqcount lock */ + uint32_t seq_count; + uint64_t disruption_marker; + uint64_t flags; + uint8_t pad[2]; + uint8_t clock_status; + uint8_t leap_second_smearing_hint; + uint16_t tai_offset_sec; + uint8_t leap_indicator; + uint8_t counter_period_shift; + uint64_t counter_value; + uint64_t counter_period_frac_sec; + uint64_t counter_period_esterror_rate_frac_sec; + uint64_t counter_period_maxerror_rate_frac_sec; + uint64_t time_sec; + uint64_t time_frac_sec; + uint64_t time_esterror_nanosec; + uint64_t time_maxerror_nanosec; + uint64_t vm_generation_counter; +}; + +// Pin the ABI layout. These values come from the kernel's vmclock-abi.h; if a +// change to |struct vmclock_abi| moves any of them, the build must fail. +OPENSSL_STATIC_ASSERT(sizeof(struct vmclock_abi) == 112, + vmclock_abi_unexpected_size); +OPENSSL_STATIC_ASSERT(offsetof(struct vmclock_abi, magic) == 0, + vmclock_abi_unexpected_magic_offset); +OPENSSL_STATIC_ASSERT(offsetof(struct vmclock_abi, seq_count) == 12, + vmclock_abi_unexpected_seq_count_offset); +OPENSSL_STATIC_ASSERT(offsetof(struct vmclock_abi, flags) == 24, + vmclock_abi_unexpected_flags_offset); +OPENSSL_STATIC_ASSERT(offsetof(struct vmclock_abi, vm_generation_counter) == 104, + vmclock_abi_unexpected_vm_generation_counter_offset); + +#ifdef __cplusplus +} +#endif + +#endif /* HEADER_VMCLOCK_ABI */ diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 8bbda4ee2ce..629ffabaca6 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -1422,11 +1422,16 @@ class StderrDelimiter { }; int main(int argc, char **argv) { -#if defined(OPENSSL_LINUX) && defined(AWSLC_VM_UBE_TESTING) +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_SYSGENID) if (1 != HAZMAT_init_sysgenid_file()) { abort(); } #endif +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_VMCLOCK) + if (1 != HAZMAT_init_vmclock_file()) { + abort(); + } +#endif // To distinguish ASan's output from ours, add a trailing message to stderr. // Anything following this line will be considered an error. diff --git a/ssl/test/handshaker.cc b/ssl/test/handshaker.cc index 0bbc443979c..3fedbb93432 100644 --- a/ssl/test/handshaker.cc +++ b/ssl/test/handshaker.cc @@ -218,11 +218,16 @@ int SignalError() { } // namespace int main(int argc, char **argv) { -#if defined(OPENSSL_LINUX) && defined(AWSLC_VM_UBE_TESTING) +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_SYSGENID) if (1 != HAZMAT_init_sysgenid_file()) { abort(); } #endif +#if defined(OPENSSL_LINUX) && defined(AWSLC_TEST_VMCLOCK) + if (1 != HAZMAT_init_vmclock_file()) { + abort(); + } +#endif TestConfig initial_config, resume_config, retry_config; if (!ParseConfig(argc - 1, argv + 1, /*is_shim=*/false, &initial_config, diff --git a/tests/ci/run_fips_tests.sh b/tests/ci/run_fips_tests.sh index 79a1a615b14..c8b62d68985 100755 --- a/tests/ci/run_fips_tests.sh +++ b/tests/ci/run_fips_tests.sh @@ -61,10 +61,26 @@ if static_linux_supported || static_openbsd_supported; then echo "Testing AWS-LC static library in FIPS Release mode with FIPS entropy source method CPU Jitter." fips_build_and_test -DCMAKE_BUILD_TYPE=Release -DENABLE_FIPS_ENTROPY_CPU_JITTER=ON - echo "Testing AWS-LC static library in FIPS Debug with SysGenId." + # VM UBE detection has two backends (vmclock preferred, SysGenID fallback). + # They are tested in separate builds because enabling both would never + # exercise the SysGenID path at runtime. The DISABLED_ value round-trip and + # seqlock tests mutate the shared stand-in device file, so they are run in a + # dedicated single-process invocation rather than interleaved with the suite. + echo "Testing AWS-LC static library in FIPS Debug with VM UBE detection (vmclock backend)." + TEST_VMCLOCK_PATH=$(mktemp) + dd if=/dev/zero of="${TEST_VMCLOCK_PATH}" bs=1 count=4096 + fips_build_and_test -DTEST_VMCLOCK_PATH="${TEST_VMCLOCK_PATH}" + "${BUILD_ROOT}/crypto/crypto_test" \ + --gtest_also_run_disabled_tests \ + --gtest_filter='VmUbeGenerationTest.DISABLED_Vmclock*' + + echo "Testing AWS-LC static library in FIPS Debug with VM UBE detection (SysGenId backend)." TEST_SYSGENID_PATH=$(mktemp) dd if=/dev/zero of="${TEST_SYSGENID_PATH}" bs=1 count=4096 fips_build_and_test -DTEST_SYSGENID_PATH="${TEST_SYSGENID_PATH}" + "${BUILD_ROOT}/crypto/crypto_test" \ + --gtest_also_run_disabled_tests \ + --gtest_filter='VmUbeGenerationTest.DISABLED_SysGenID*' fi # The AL2 version of Clang does not have all of the required artifacts for address sanitizer, see P45594051 diff --git a/tests/ci/run_posix_tests.sh b/tests/ci/run_posix_tests.sh index 62bff6f8597..7f3538809aa 100755 --- a/tests/ci/run_posix_tests.sh +++ b/tests/ci/run_posix_tests.sh @@ -26,10 +26,35 @@ build_and_test -DOPENSSL_NO_ASM=1 -DCMAKE_BUILD_TYPE=Release echo "Testing building shared lib." build_and_test -DBUILD_SHARED_LIBS=1 -DCMAKE_BUILD_TYPE=Release -echo "Testing with a SysGenId." +# VM UBE (Uniqueness Breaking Event) detection has two backends: vmclock +# (preferred) and SysGenID (fallback). They are tested in separate builds +# because vmclock is preferred at runtime -- enabling both in one build would +# never exercise the SysGenID path. Each build points the backend under test at +# a regular file standing in for its /dev node. +# +# The value round-trip and seqlock tests are DISABLED_ by default: they mutate +# the shared stand-in file that every RAND_bytes call in the suite reads, so +# they cannot run interleaved with other tests. We run them explicitly in a +# dedicated, single-process invocation (nothing else calling RAND_bytes +# concurrently) via --gtest_also_run_disabled_tests. + +echo "Testing with VM UBE detection (vmclock backend)." +TEST_VMCLOCK_PATH=$(mktemp) +dd if=/dev/zero of="${TEST_VMCLOCK_PATH}" bs=1 count=4096 +build_and_test -DTEST_VMCLOCK_PATH="${TEST_VMCLOCK_PATH}" +echo "Running device-mutating vmclock tests in isolation." +"${BUILD_ROOT}/crypto/crypto_test" \ + --gtest_also_run_disabled_tests \ + --gtest_filter='VmUbeGenerationTest.DISABLED_Vmclock*' + +echo "Testing with VM UBE detection (SysGenId backend / vmclock fallback)." TEST_SYSGENID_PATH=$(mktemp) dd if=/dev/zero of="${TEST_SYSGENID_PATH}" bs=1 count=4096 build_and_test -DTEST_SYSGENID_PATH="${TEST_SYSGENID_PATH}" +echo "Running device-mutating SysGenId tests in isolation." +"${BUILD_ROOT}/crypto/crypto_test" \ + --gtest_also_run_disabled_tests \ + --gtest_filter='VmUbeGenerationTest.DISABLED_SysGenID*' echo "Testing with pre-generated assembly code." build_and_test -DDISABLE_PERL=ON From 7ace893765bc8db17e514227b6ef6d623df4b3a0 Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 10 Aug 2026 18:52:52 +0000 Subject: [PATCH 06/10] updates from main --- .github/workflows/api-docs.yml | 41 +++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/.github/workflows/api-docs.yml b/.github/workflows/api-docs.yml index 9e589db2ee0..27303b491b1 100644 --- a/.github/workflows/api-docs.yml +++ b/.github/workflows/api-docs.yml @@ -9,6 +9,13 @@ on: - 'util/doc.config' - 'util/doc.css' - '.github/workflows/api-docs.yml' + pull_request: + paths: + - 'include/openssl/**' + - 'util/doc.go' + - 'util/doc.config' + - 'util/doc.css' + - '.github/workflows/api-docs.yml' permissions: contents: read @@ -20,25 +27,43 @@ concurrency: cancel-in-progress: false jobs: + validate-api-docs: + concurrency: + group: api-docs-validate-${{ github.ref }} + cancel-in-progress: true + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v6 + with: + go-version: ">=1.22" + - name: Generate API documentation + run: go run util/doc.go + publish-api-docs: + if: github.event_name == 'push' + needs: validate-api-docs runs-on: ubuntu-latest + permissions: + contents: read + pages: write + id-token: write environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v4 + - uses: actions/checkout@v7 + - uses: actions/setup-go@v6 with: go-version: ">=1.22" - name: Generate API documentation - run: | - mkdir -p pages-output - cd util - go run doc.go --config doc.config --out ../pages-output + run: go run util/doc.go - name: Upload pages artifact - uses: actions/upload-pages-artifact@v3 + uses: actions/upload-pages-artifact@v5 with: path: ./pages-output - name: Deploy to GitHub Pages id: deployment - uses: actions/deploy-pages@v4 + uses: actions/deploy-pages@v5 From 9bccb1688a97244556c4942f0036fef5bfde0749 Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 10 Aug 2026 19:06:32 +0000 Subject: [PATCH 07/10] Register CRYPTO_get_vmclock_path in libcrypto symbol registry New exported symbol added by the vmclock backend. Registered as AWS_LC_1.0 PRIVATE alongside the other CRYPTO_get_vm_ube_* symbols and the .map regenerated via util/generate_version_script. --- crypto/libcrypto.map | 1 + crypto/libcrypto.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/crypto/libcrypto.map b/crypto/libcrypto.map index faa04d04f76..a36e7950419 100644 --- a/crypto/libcrypto.map +++ b/crypto/libcrypto.map @@ -663,6 +663,7 @@ AWS_LC_1.0 { CRYPTO_get_vm_ube_active; CRYPTO_get_vm_ube_generation; CRYPTO_get_vm_ube_supported; + CRYPTO_get_vmclock_path; CRYPTO_has_asm; CRYPTO_has_broken_NEON; CRYPTO_is_ARMv8_DIT_capable_for_testing; diff --git a/crypto/libcrypto.txt b/crypto/libcrypto.txt index bc90099f02d..f2790627e3f 100644 --- a/crypto/libcrypto.txt +++ b/crypto/libcrypto.txt @@ -656,6 +656,7 @@ CRYPTO_get_ube_generation_number AWS_LC_1.0 PRIVATE CRYPTO_get_vm_ube_active AWS_LC_1.0 PRIVATE CRYPTO_get_vm_ube_generation AWS_LC_1.0 PRIVATE CRYPTO_get_vm_ube_supported AWS_LC_1.0 PRIVATE +CRYPTO_get_vmclock_path AWS_LC_1.0 PRIVATE CRYPTO_has_asm AWS_LC_1.0 PUBLIC CRYPTO_has_broken_NEON AWS_LC_1.0 PUBLIC CRYPTO_is_ARMv8_DIT_capable_for_testing AWS_LC_1.0 PRIVATE From 724ec4f188dfd65ac37d5dd0dc68415cb91e5370 Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 10 Aug 2026 19:52:46 +0000 Subject: [PATCH 08/10] Distinguish transient seqlock read failures from permanent init failures A wedged/contended vmclock seqlock makes the bounded seqlock reader give up and report failure. Previously CRYPTO_get_vm_ube_generation returned 0 for this, and ube.c mapped any 0 to ube_failed() -- which is CRYPTO_once-guarded and irreversibly disables ALL UBE detection (fork included) for the entire process. So a single momentary read failure permanently stopped reseeding. Make CRYPTO_get_vm_ube_generation tri-state: 1 success, 0 permanent init failure, -1 transient read failure. ube.c now treats -1 as "reseed conservatively for this call" (return 0 to the DRBG) without calling ube_failed(); only a permanent 0 disables detection. DRBG consumers are unchanged -- they still reseed on a 0 return from CRYPTO_get_ube_generation_number. --- crypto/ube/ube.c | 44 +++++++++++++++++++++++++++++--- crypto/ube/vm_ube_detect.c | 14 ++++++---- crypto/ube/vm_ube_detect.h | 15 ++++++++--- crypto/ube/vm_ube_detect_test.cc | 9 ++++--- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/crypto/ube/ube.c b/crypto/ube/ube.c index 6ddb51f42d0..32bce61a8ed 100644 --- a/crypto/ube/ube.c +++ b/crypto/ube/ube.c @@ -132,6 +132,10 @@ static void ube_state_initialize(void) { int ret_vm_ube_gn = get_vm_ube_generation_number( &(ube_global_state.cached_vm_ube_gn)); + // Only a permanent failure (0) disables detection. A transient VM UBE read + // failure (-1) here just leaves the cached generation number at 0; the next + // successful read will differ and be treated as a UBE, forcing a reseed -- + // conservative and correct, without disabling detection for the process. if (ret_fork_gn == 0 || ret_vm_ube_gn == 0) { ube_failed(); } @@ -154,8 +158,14 @@ static void ube_update_state(struct detection_gn *current_detection_gn) { // ube_get_detection_generation_numbers loads the current detection generation // numbers into |current_detection_gn|. // -// Returns 1 on success and 0 otherwise. The 0 return value means that a -// detection method we expected to be available, is in fact not. +// Returns tri-state: +// 1 Success. +// 0 Permanent failure: a detection method we expected to be available is in +// fact not. The caller must disable detection (|ube_failed|). +// -1 Transient failure: a method initialized successfully but could not +// produce a consistent read this call (e.g. a momentarily wedged vmclock +// seqlock). The caller must reseed conservatively for this call but must +// NOT disable detection. static int ube_get_detection_generation_numbers( struct detection_gn *current_detection_gn) { @@ -169,10 +179,19 @@ static int ube_get_detection_generation_numbers( int ret_vm_ube_gn = get_vm_ube_generation_number( &(current_detection_gn->current_vm_ube_gn)); + // A permanent failure of any method takes precedence: detection is no longer + // trustworthy and must be disabled by the caller. if (ret_detect_gn == 0 || ret_vm_ube_gn == 0) { return 0; } + // Otherwise, a transient VM UBE read failure (-1) means we could not read a + // consistent value this call. Signal a conservative reseed without disabling + // detection. + if (ret_vm_ube_gn == -1) { + return -1; + } + return 1; } @@ -223,10 +242,20 @@ int CRYPTO_get_ube_generation_number(uint64_t *current_generation_number) { // Each individual detection method will have their own concurrency controls // if needed. - if (ube_get_detection_generation_numbers(¤t_detection_gn) != 1) { + int ret_gn = ube_get_detection_generation_numbers(¤t_detection_gn); + if (ret_gn == 0) { + // Permanent failure: a detection method we expected is gone. Disable + // detection for the process and force a conservative reseed. ube_failed(); return 0; } + if (ret_gn == -1) { + // Transient read failure (e.g. a momentarily wedged vmclock seqlock). Force + // a conservative reseed for this call, but do NOT disable detection -- + // |ube_failed| is CRYPTO_once-guarded and would irreversibly turn off all + // UBE detection (including fork) for the entire process. + return 0; + } CRYPTO_STATIC_MUTEX_lock_read(&ube_lock); if (ube_is_detected(¤t_detection_gn) == 0) { // No UBE detected, so just grab UBE generation number from the state. @@ -246,11 +275,18 @@ int CRYPTO_get_ube_generation_number(uint64_t *current_generation_number) { // that had the first entry. CRYPTO_STATIC_MUTEX_lock_write(&ube_lock); - if (ube_get_detection_generation_numbers(¤t_detection_gn) != 1) { + ret_gn = ube_get_detection_generation_numbers(¤t_detection_gn); + if (ret_gn == 0) { ube_failed(); CRYPTO_STATIC_MUTEX_unlock_write(&ube_lock); return 0; } + if (ret_gn == -1) { + // Transient read failure: reseed conservatively without disabling + // detection (see the first call site above). + CRYPTO_STATIC_MUTEX_unlock_write(&ube_lock); + return 0; + } if (ube_is_detected(¤t_detection_gn) == 0) { // Another thread already updated the global state. Just load the UBE // generation number instead. diff --git a/crypto/ube/vm_ube_detect.c b/crypto/ube/vm_ube_detect.c index 5a6a12c89f4..fe0d4496677 100644 --- a/crypto/ube/vm_ube_detect.c +++ b/crypto/ube/vm_ube_detect.c @@ -240,12 +240,16 @@ int CRYPTO_get_vm_ube_generation(uint64_t *vm_ube_generation_number) { return 1; case VM_UBE_STATE_SUCCESS_INITIALISE: if (vm_ube_read_generation(vm_ube_generation_number) != 1) { - // A backend that initialized successfully but now cannot produce a - // consistent read (e.g. a wedged vmclock seqlock) is treated as a - // failure so the caller reseeds conservatively rather than trusting a - // stale or torn value. + // A backend that initialized successfully but cannot produce a + // consistent read this call (e.g. a momentarily wedged vmclock + // seqlock) is a *transient* failure -- distinct from a permanent + // initialization failure. Return -1 so the UBE layer forces a + // conservative reseed for this call without irreversibly disabling all + // UBE detection for the process (see ube.c). The value is zeroed so a + // caller that ignores the distinction still never sees a stale/torn + // number. *vm_ube_generation_number = 0; - return 0; + return -1; } return 1; default: diff --git a/crypto/ube/vm_ube_detect.h b/crypto/ube/vm_ube_detect.h index 1af97a63dcc..d841c6676f7 100644 --- a/crypto/ube/vm_ube_detect.h +++ b/crypto/ube/vm_ube_detect.h @@ -35,9 +35,18 @@ extern "C" { // vmclock is preferred when available. If neither is available, the function // reports that VM UBE detection is not supported. // -// |CRYPTO_get_vm_ube_generation| returns 0 only when the filesystem -// presents a VM UBE interface but we are unable to initialize its use. -// Otherwise, it returns 1. +// Return values are tri-state: +// 1 Success. |*vm_ube_generation_number| holds the current generation +// number, or 0 if no VM UBE interface is present (not supported). +// 0 Permanent failure: a VM UBE interface is present but could not be +// initialized. (Not currently returned on Linux -- an uninitializable +// device degrades to "not supported" -- but reserved for callers that +// must distinguish it.) +// -1 Transient failure: a backend initialized successfully but could not +// produce a consistent read this call (e.g. a momentarily wedged vmclock +// seqlock). |*vm_ube_generation_number| is set to 0. Callers must treat +// this as "reseed conservatively for this call" and retry later, NOT as a +// permanent loss of detection. OPENSSL_EXPORT int CRYPTO_get_vm_ube_generation( uint64_t *vm_ube_generation_number); diff --git a/crypto/ube/vm_ube_detect_test.cc b/crypto/ube/vm_ube_detect_test.cc index 5c3b8595086..2b8c0ee98f7 100644 --- a/crypto/ube/vm_ube_detect_test.cc +++ b/crypto/ube/vm_ube_detect_test.cc @@ -165,10 +165,13 @@ TEST(VmUbeGenerationTest, DISABLED_VmclockSeqlockWedged) { // irrelevant; the reader must never observe a consistent seq_count. ASSERT_TRUE(set_vmclock_seq_count(&vmc_test, 0x7FFFFFFF)); - // The read must terminate (bounded retries) and report failure rather than - // hang. This test hanging *is* the failure signal for the unbounded-loop bug. + // The read must terminate (bounded retries) and report a *transient* failure + // (-1) rather than hang. -1 (not 0) is essential: the UBE layer maps a 0 + // return to permanent, process-wide disabling of all detection, whereas -1 + // means "reseed conservatively this call, retry later". This test hanging + // *is* the failure signal for the unbounded-loop bug. gen = 0xdeadbeef; - ASSERT_EQ(0, CRYPTO_get_vm_ube_generation(&gen)); + ASSERT_EQ(-1, CRYPTO_get_vm_ube_generation(&gen)); ASSERT_EQ((uint64_t)0, gen); // Release the seqlock. Reset seq_count to an even value first: the seqlock From ec7ea3ad696e327b292ec347a7e8e7b370bae159 Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 10 Aug 2026 20:42:19 +0000 Subject: [PATCH 09/10] Fix legacy gcc 4.1 build: gate stdatomic.h behind C11 The legacy build (tests/ci/run_legacy_build.sh) compiles with gcc 4.1 under -std=gnu99, which predates C11 and the __atomic builtins, so the unconditional include failed with "stdatomic.h: No such file or directory". Wrap the seqlock acquire fence in vm_ube_acquire_fence(), gated on the same C11 check the tree already uses for refcounting (crypto/internal.h): use atomic_thread_fence(memory_order_acquire) when C11 atomics are available, and fall back to __sync_synchronize() -- a full barrier available since gcc 4.1 -- otherwise. A full barrier is stronger than the acquire we need, so it is always correct. --- crypto/ube/vm_ube_detect.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/crypto/ube/vm_ube_detect.c b/crypto/ube/vm_ube_detect.c index fe0d4496677..12ac64e4114 100644 --- a/crypto/ube/vm_ube_detect.c +++ b/crypto/ube/vm_ube_detect.c @@ -7,12 +7,6 @@ #if defined(OPENSSL_LINUX) #include -// This file is compiled only for Linux, where the toolchains we support -// (GCC/Clang) always provide working, lock-free C11 atomics. We use -// directly for the acquire fence in the vmclock seqlock reader -// rather than the tree's |CRYPTO_atomic_*| refcount helpers, which do not -// expose a general-purpose fence. -#include #include #include #include @@ -22,6 +16,25 @@ #include "../internal.h" #include "vmclock_abi.h" +// vm_ube_acquire_fence is an acquire memory barrier for the vmclock seqlock +// reader. We cannot unconditionally use C11 : the legacy build +// (tests/ci/run_legacy_build.sh) compiles this file with gcc 4.1 under +// -std=gnu99, which predates both C11 atomics and __atomic builtins. Mirror the +// tree's C11 gating (see crypto/internal.h) and fall back to __sync_synchronize +// -- a full barrier available since gcc 4.1 -- when C11 atomics are absent. A +// full barrier is stronger than the acquire we need, so it is always correct. +#if !defined(__STDC_NO_ATOMICS__) && defined(__STDC_VERSION__) && \ + __STDC_VERSION__ >= 201112L +#include +static void vm_ube_acquire_fence(void) { + atomic_thread_fence(memory_order_acquire); +} +#else +static void vm_ube_acquire_fence(void) { + __sync_synchronize(); +} +#endif + // VM UBE state. A backend either initializes successfully or VM UBE detection // is unavailable ("not supported"). There is deliberately no hard-failure // state: an inaccessible or invalid device degrades to NOT_SUPPORTED so that @@ -199,13 +212,13 @@ static int vm_ube_read_vmclock_gn(uint64_t *out) { uint32_t seq = vmclock_addr->seq_count & ~1u; // Acquire fence pairs with the VMM's release fence: it ensures the // |seq_count| read is not reordered after the |vm_generation_counter| read. - atomic_thread_fence(memory_order_acquire); + vm_ube_acquire_fence(); uint64_t value = vmclock_addr->vm_generation_counter; // Acquire fence ensures the second |seq_count| read is not reordered before // the |vm_generation_counter| read. - atomic_thread_fence(memory_order_acquire); + vm_ube_acquire_fence(); if (seq == vmclock_addr->seq_count) { *out = value; return 1; From cb63369942b5a34199c2ac28cb2c91c258d048a8 Mon Sep 17 00:00:00 2001 From: Doug Chapman Date: Mon, 10 Aug 2026 22:05:49 +0000 Subject: [PATCH 10/10] Fix vmclock test-file init clobbering generation value across processes HAZMAT_init_vmclock_file unconditionally rewrote the stand-in device file at every process startup, resetting vm_generation_counter to 0. all_tests.go runs several crypto_test processes concurrently against the same file, so a process starting up would reset the counter in the middle of another process's VmUbeGenerationTest -- which had just written e.g. 42 and was about to read it back -- producing a consistent read of 0. This surfaced as a flaky "Expected 42, Which is 0" on slower/older CI hosts (centos:7, ubuntu:16.04, fedora:31) where startup timing overlapped differently; it was a cross-process data race, not a compiler issue. Only initialize the file when it does not already contain a valid vmclock (magic not set), mirroring HAZMAT_init_sysgenid_file's populate-once intent. Gate on the magic rather than emptiness because CI pre-fills the file zeroed (dd), so it is never empty and vmclock additionally requires magic to be set. --- crypto/ube/vm_ube_detect.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/crypto/ube/vm_ube_detect.c b/crypto/ube/vm_ube_detect.c index 12ac64e4114..4235a42e968 100644 --- a/crypto/ube/vm_ube_detect.c +++ b/crypto/ube/vm_ube_detect.c @@ -350,12 +350,34 @@ int HAZMAT_init_vmclock_file(void) { return 0; } + // Only initialize the file if it does not already contain a valid vmclock + // (magic not yet set); otherwise leave it untouched. This is required for + // correctness under the test runner: all_tests.go launches several + // crypto_test processes concurrently against the same stand-in file, and + // every process calls this at startup. Unconditionally rewriting the struct + // would reset |vm_generation_counter| to 0 in the middle of another process's + // VmUbeGenerationTest, which had just written a value and was about to read + // it back -- yielding a consistent read of the wrong (0) value. + // + // Note this differs from HAZMAT_init_sysgenid_file's empty-file check: the CI + // pre-creates the stand-in file zero-filled (dd), so it is never empty. A + // zero sysgenid value is valid, but vmclock additionally needs |magic| set, + // so we gate on the magic instead. Once any process has written the magic, + // later-starting processes see it and leave the file (and its generation + // counter) alone. + uint32_t existing_magic = 0; + if ((ssize_t)sizeof(existing_magic) == + read(fd, &existing_magic, sizeof(existing_magic)) && + existing_magic == VMCLOCK_MAGIC) { + close(fd); + return 1; + } + if (0 != lseek(fd, 0, SEEK_SET)) { close(fd); return 0; } - // Always write a valid vmclock structure at the start of the file. struct vmclock_abi vmc; memset(&vmc, 0, sizeof(vmc)); vmc.magic = VMCLOCK_MAGIC;