Add vmclock backend for VM UBE detection, preferring it over SysGenID - #3418
Draft
dougch wants to merge 11 commits into
Draft
Add vmclock backend for VM UBE detection, preferring it over SysGenID#3418dougch wants to merge 11 commits into
dougch wants to merge 11 commits into
Conversation
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.
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.
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.
Contributor
|
🔒 Security Review — View Report Please review before merging. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3418 +/- ##
==========================================
- Coverage 78.20% 78.19% -0.02%
==========================================
Files 695 695
Lines 124271 124348 +77
Branches 17265 17282 +17
==========================================
+ Hits 97186 97233 +47
- Misses 26161 26190 +29
- Partials 924 925 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
The legacy build (tests/ci/run_legacy_build.sh) compiles with gcc 4.1 under -std=gnu99, which predates C11 <stdatomic.h> 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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
AWS-LC detects VM Uniqueness Breaking Events (UBE, formerly Snapsafe) on Linux so the DRBG can reseed after a VM snapshot/clone/restore. Detection previously relied solely on the Amazon-proprietary
/dev/sysgenidinterface. This change adds support for the non-proprietary vmclock interface (/dev/vmclock0, see the vmclock specification) and prefers it when available, so the ecosystem can move off SysGenID.Behavior:
/dev/vmclock0is absent, or present but not usable by this process (EACCES on a root-only node, mmap failure, bad magic, or the generation-counter flag unset), we fall back to/dev/sysgenid. If neither is usable, VM UBE detection degrades to "not supported" rather than hard-failing — a hard failure would also disable the independent fork detection and force the DRBG to reseed on every request.RAND_bytesforever.uint32_ttouint64_tto match vmclock's counter width; the SysGenID path zero-extends.The new
crypto/ube/vmclock_abi.hmirrors the kernel's vmclock ABI and uses static asserts to pin the struct size and the offsets we dereference, so an accidental edit fails the build rather than silently shifting the counter field.Call-outs:
AWSLC_VM_UBE_TESTINGbuild guard now additionally sets a per-backend flag (AWSLC_TEST_SYSGENID/AWSLC_TEST_VMCLOCK) selected by-DTEST_SYSGENID_PATH/-DTEST_VMCLOCK_PATH, so a single-backend build only creates the stand-in file for the backend under test and never touches the real /dev node for the other. Because vmclock is preferred at runtime, exercising the SysGenID path (and the vmclock→SysGenID fallback) requires a-DTEST_SYSGENID_PATH-only build.DISABLED_by default: they mutate the shared stand-in device file that everyRAND_bytescall in the suite reads, so they cannot run interleaved. CI runs them in a dedicated single-process invocation via--gtest_also_run_disabled_tests.Testing:
run_posix_tests.shandrun_fips_tests.shnow build each backend in a separate configuration and run its isolated device-mutating tests. Locally, both the vmclock and SysGenID backend builds compile cleanly and all VM UBE / UBE tests pass, including the isolated seqlock, torn-read, and graceful-degradation cases.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.