mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 11:08:25 +00:00
Clear a connection's handshake slot when its whole handshake state leaves the machine
Taking the handshake state off a connection's control machine moved it out with Option::take, which writes only the None marker and leaves the old bytes in place. On promotion the machine lives on as the active peer's control machine, so its slot kept the session's two traffic keys for as long as the peer stayed connected; the cross-connection and stale-connection paths left the same copy behind. The slot is now cleared as the state leaves, as it already is when a completed handshake or a rekeyed session leaves its own slot. The three callers unwrap the result at once, which the clearing helper's documentation says can leave a second copy on the stack. That copy is a move like any other and is outside what the clearing covers; clearing in the one shared method still covers the heap slot for every caller. The security reference now lists this move among the cleared ones. The new residue test reads the emptied slot's bytes, which only means something in an optimised build: in a debug build the None written by Option::take carries uninitialised stack bytes either way. It is therefore release-only, and the release library-test step in GitHub CI and local CI, which only compiled the tests before, now also runs it by exact name and fails if it did not run.
This commit is contained in:
@@ -384,10 +384,18 @@ jobs:
|
||||
# Debug-only helpers (anything behind #[cfg(debug_assertions)]) vanish in
|
||||
# a release build, so a test calling one without the same gate breaks a
|
||||
# build no other job performs: every run above compiles the test target
|
||||
# in debug. Compile it in release too, without running it — the point is
|
||||
# that it builds at all. Mirrored in testing/ci-local.sh.
|
||||
- name: Compile the library tests in release mode
|
||||
run: cargo test --release --lib --no-run
|
||||
# in debug. Compile it in release too. Of what it builds, run only the
|
||||
# leg-slot residue test, which is release-only because only an optimised
|
||||
# build leaves the slot in a state worth measuring; the grep fails the
|
||||
# step if that test did not run, since a name filter that matches
|
||||
# nothing passes. Mirrored in testing/ci-local.sh.
|
||||
- name: Compile the library tests in release mode and run the leg-slot residue test
|
||||
shell: bash
|
||||
env:
|
||||
RESIDUE_TEST: peer::machine::tests::take_leg_leaves_no_session_keys_in_the_slot_it_empties
|
||||
run: |
|
||||
cargo test --release --lib -- --exact "$RESIDUE_TEST" | tee "$RUNNER_TEMP/release-lib-tests.log"
|
||||
grep -q '^test result: ok\. 1 passed;' "$RUNNER_TEMP/release-lib-tests.log"
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
# Job 2b – Unit tests (macOS)
|
||||
|
||||
@@ -123,14 +123,13 @@ reach:
|
||||
dropped. Each of those moves leaves behind, in a stack frame that is
|
||||
no longer in use, a copy of the node's long-term private key and,
|
||||
once the handshake has started, of its ephemeral key and chaining
|
||||
key. Two moves out of the slots on a connection's control machine
|
||||
key. Three moves out of the slots on a connection's control machine
|
||||
are cleared: when a completed handshake leaves the slot that held it,
|
||||
and when a session is taken out of its slot for a rekey, the slot is
|
||||
overwritten as the value leaves. Other moves are not. When a
|
||||
connection is promoted to an active peer, or reaped as stale, its
|
||||
whole handshake state is moved off its control machine, which leaves
|
||||
the session's two traffic keys, or an unfinished handshake's private
|
||||
keys, in the heap memory the machine occupies.
|
||||
when a session is taken out of its slot for a rekey, and when the
|
||||
whole handshake state leaves the machine because the connection is
|
||||
promoted to an active peer, resolved as one side of a
|
||||
cross-connection, or reaped as stale. In each case the slot is
|
||||
overwritten as the value leaves. Other moves are not.
|
||||
- **Loading the identity from a secret string.** Building the node's
|
||||
identity from its key file or from `node.identity.nsec` leaves
|
||||
copies of the private key, among them a whole intermediate identity,
|
||||
|
||||
+44
-1
@@ -594,7 +594,12 @@ impl PeerMachine {
|
||||
/// Take the handshake crypto carrier off the machine (promotion and
|
||||
/// teardown consume it by value).
|
||||
pub(crate) fn take_leg(&mut self) -> Option<HandshakeCrypto> {
|
||||
self.leg.take()
|
||||
// On promotion the machine lives on as the peer's control machine, so
|
||||
// the slot is cleared as the leg leaves, or it would keep the session's
|
||||
// traffic keys. Callers unwrap the result at once, which `take_cleared`
|
||||
// warns may leave a second stack copy; clearing here still covers every
|
||||
// caller's heap slot, and a stack copy is a move like any other.
|
||||
noise::take_cleared(&mut self.leg)
|
||||
}
|
||||
|
||||
/// Attach a handshake crypto carrier to the machine.
|
||||
@@ -3405,6 +3410,44 @@ mod tests {
|
||||
assert_eq!(decrypted, plaintext);
|
||||
}
|
||||
|
||||
/// Non-zero bytes left in a leg slot, read in place.
|
||||
#[cfg(not(debug_assertions))]
|
||||
fn leg_slot_nonzero_bytes(slot: &Option<HandshakeCrypto>) -> usize {
|
||||
let base = (slot as *const Option<HandshakeCrypto>).cast::<u8>();
|
||||
(0..std::mem::size_of::<Option<HandshakeCrypto>>())
|
||||
// SAFETY: `base` comes from a live reference, so every byte of the
|
||||
// slot is in bounds; volatile so the read is of memory as it is.
|
||||
.filter(|&i| unsafe { std::ptr::read_volatile(base.add(i)) } != 0)
|
||||
.count()
|
||||
}
|
||||
|
||||
/// Release only: in a debug build `Option::take` leaves stack garbage in
|
||||
/// the payload of the `None` it writes, so the count means nothing there.
|
||||
#[cfg(not(debug_assertions))]
|
||||
#[test]
|
||||
fn take_leg_leaves_no_session_keys_in_the_slot_it_empties() {
|
||||
let initiator = Identity::generate();
|
||||
let responder = Identity::generate();
|
||||
let responder_id = PeerIdentity::from_pubkey_full(responder.pubkey_full());
|
||||
let mut ini = outbound_leg(LinkId::new(1), responder_id, 1000);
|
||||
let mut res = Box::new(inbound_leg(LinkId::new(2), 1000));
|
||||
let msg1 = ini
|
||||
.start_handshake(initiator.keypair(), make_epoch(), 1100)
|
||||
.unwrap();
|
||||
res.receive_handshake_init(responder.keypair(), make_epoch(), &msg1, 1200)
|
||||
.unwrap();
|
||||
assert!(res.has_session(), "the responder's leg holds the session");
|
||||
assert!(leg_slot_nonzero_bytes(&res.leg) > 64);
|
||||
|
||||
let leg = res.take_leg().expect("the leg was attached");
|
||||
assert!(leg.noise_session.is_some(), "the session left with the leg");
|
||||
let left = leg_slot_nonzero_bytes(&res.leg);
|
||||
assert!(
|
||||
left <= 8,
|
||||
"{left} non-zero bytes of the session stayed in the emptied leg slot"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_connection_failure() {
|
||||
// `mark_failed` releases the leg's Noise handshake handle. The failure
|
||||
|
||||
+14
-6
@@ -676,16 +676,24 @@ run_tests() {
|
||||
# Debug-only helpers (anything behind #[cfg(debug_assertions)]) vanish in a
|
||||
# release build, so a test calling one without the same gate breaks a build
|
||||
# nothing here ever performs: every run above compiles the test target in
|
||||
# debug. Compile it in release too, without running it — the point is that
|
||||
# it builds at all. Mirrored in .github/workflows/ci.yml; check-ci-parity.sh
|
||||
# compares integration suites only and would not catch a stage added to one
|
||||
# runner and not the other.
|
||||
info "cargo test --release --lib --no-run"
|
||||
if cargo test --release --lib --no-run 2>&1; then
|
||||
# debug. Compile it in release too. Of what it builds, run only the leg-slot
|
||||
# residue test, which is release-only because only an optimised build
|
||||
# leaves the slot in a state worth measuring; the grep fails the stage if
|
||||
# that test did not run, since a name filter that matches nothing passes.
|
||||
# Mirrored in .github/workflows/ci.yml; check-ci-parity.sh compares
|
||||
# integration suites only and would not catch a stage added to one runner
|
||||
# and not the other.
|
||||
local residue_test="peer::machine::tests::take_leg_leaves_no_session_keys_in_the_slot_it_empties"
|
||||
local release_log
|
||||
release_log=$(mktemp "/tmp/ci-release-lib-tests.XXXXXX")
|
||||
info "cargo test --release --lib -- --exact $residue_test"
|
||||
if cargo test --release --lib -- --exact "$residue_test" 2>&1 | tee "$release_log" \
|
||||
&& grep -q '^test result: ok\. 1 passed;' "$release_log"; then
|
||||
record "release-test-compile" 0
|
||||
else
|
||||
record "release-test-compile" 1
|
||||
fi
|
||||
rm -f "$release_log"
|
||||
}
|
||||
|
||||
# ── Stage 3: Integration Tests ─────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user