8000 feat(bep): T128 — Hold the proxy's keys in the TPM by minimal-foundry[bot] · Pull Request #3343 · gominimal/minimal · GitHub
[go: up one dir, main page]

Skip to content

feat(bep): T128 — Hold the proxy's keys in the TPM - #3343

Merged
norrietaylor merged 19 commits into
mainfrom
agent/3324-hold-the-proxy-s-keys-in-the-tpm
Nov 13, 2026
Merged

norrietaylor merged 19 commits into
mainfrom
agent/3324-hold-the-proxy-s-keys-in-the-tpm

Conversation

@minimal-foundry
Copy link
Copy Markdown
Contributor

T128 — Hold the proxy's keys in the TPM

slice: S18
kind: task
covers: BEP-088
size: L
est: 420 code, 400 tests
executor: agent
difficulty: unknown ground
model: strongest
diagnostics: a TPM key the store cannot create or use returns a key-store error naming the role and the TPM's response code, which the start refusals of T130 print to the person who started bep
observability: bep logs one line for each key it generates or loads from the TPM, with the role, the key's fingerprint and store=tpm, through the existing tracing subscriber
layers:
- store:
- crates/bep/src/tpm.rs (new): a key store that generates each proxy key under the TPM's storage key as a P-256 key with fixedTPM, fixedParent and noDA, with an authorization value of at least 128 bits from the OS CSPRNG that the proxy writes to a file only its user can read, keeps the key file and the authorization file in ~/.minimal/bep/tpm/, loads both at start, and implements HeldKey (agree through the TPM's ECDH) and CaKey (sign through the TPM's ECDSA), through the interface T127's record names
- crates/bep/src/lib.rs: declares the tpm module for Linux targets
- crates/bep/Cargo.toml: the TPM dependency T127's record names, under the Linux target
- Cargo.lock: the TPM dependency
test: cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization
- proof:
- crates/bep/tests/key_store_tpm_integration.rs (new): runs against the TPM that BEP_TPM_TCTI names, and returns early without it, as the _root_integration proofs do
test (deferred: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization
produces:
- crates/bep/src/tpm.rs
consumes:
- docs/spikes/2026-11-13-bep-tpm-interface.md
proof: Against swtpm, the proxy's three keys are generated in the TPM, sign and agree there, are refused for duplication, and load again with only their key and authorization files.
depends on:
- blocked by #3323

Proof

The bep crate has a Linux key store, bep::tpm::TpmStore. It generates each of the three proxy keys in the TPM as a P-256 key with fixedTPM, fixedParent, sensitiveDataOrigin, userWithAuth and noDA. Each key has its own 256-bit authorization value in a 0600 file. The sealing key agrees (tpm2_ecdhzgen) and the CA keys sign (tpm2_sign) in the TPM. A new store loads each key again from only its .key and .auth files in ~/.minimal/bep/tpm/. The integration test shows this against swtpm when BEP_TPM_TCTI is set. Nobody ran it against swtpm on this box, so the TPM half of the proof is not yet observed.

Notes from the agent: Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no TPM and no tpm2-tools, so I did not run it. The store was never run against a real TPM. The tpm2-tools argument shapes come from the spike record and the tools' documentation, and the tpm lane must confirm them: '-G ecc256:null:null' for tpm2_create, '-p file:-' (auth on stdin), 'tpm2_sign -d -f plain', and the TPM2B_ECC_POINT format for tpm2_ecdhzgen -u/-o. The parsers accept DER or raw r||s signatures, and TPM2B or bare-coordinate Z points. Cargo.toml and Cargo.lock are not changed. The T127 record (docs/spikes/2026-11-13-bep-tpm-interface.md) chose tpm2-tools as child processes, which is a runtime package and not a crate. Design choices for the next tasks: (1) The sealing key has only 'decrypt', and the CA keys have only 'sign'. probe() uses the key's own operation, as the keychain store does. (2) A failure with a TPM response code (ErrorCode 0x...) is KeyError::Refused, with the code in the detail. A tool or TCTI failure is KeyError::Store. A missing tpm2-tools names the tpm2-tools package. T129 owns the full sorting from spike finding 6. (3) The key file is the TPM2B_PUBLIC followed by the TPM2B_PRIVATE that tpm2_create wrote. find() and generate() check the TPM-reported attributes …[truncated]

Tests

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify

min task run agent-verify passed ([tasks.verify]).

Deferred

The plan marked these test lines deferred: the executor lacks the lane, so nothing here ran them and they still owe evidence.

line lane
cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization tpm

Closes 8000 #3324

A key store that generates each proxy key in the TPM through tpm2-tools,
as P-256 keys with fixedTPM, fixedParent and noDA, each with its own
authorization value in a file only the proxy's user can read.
The test runs against the TPM that BEP_TPM_TCTI names, and returns early
without it. It checks custody attributes, sign and agree in the TPM,
reload from the key and authorization files, and a wrong authorization.
@coderabbitai
coderabbitai Bot commented Nov 12, 2026 •
Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 335143d5-f6b1-4c45-9c2f-2addde8673d2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@minimal-foundry
minimal-foundry Bot commented Nov 12, 2026 •
Copy link
Copy Markdown
Contributor Author

Reviewed: 3 findings, none blocking at 23d5b558a460.

Address the comments on the diff.

Clobber: none: the diff only adds files and one cfg(target_os = "linux") line.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The TPM store conforms to Gatehouse §6.10 line 837. Each proxy key is a P-256 key in the TPM with fixedTPM, fixedParent and noDA. It signs and agrees only in the TPM. Each key has its own 256-bit authorization value, which goes to tpm2-tools on stdin and lives in a 0600 file. Nobody ran the swtpm proof. C1 requires it to pass in the bep-tpm-integration job at this head. If it fails, fix the tpm2-tools call in tpm.rs and not the test. The store choice and the TPM failure sorting are filed residuals (#3322, #3326).

Ruled at ee851be against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 4 findings, 4 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

@norrietaylor
Copy link
Copy Markdown
Member

/revise arch note on thread PRRT_kwDOPdfcxM623xGN (crates/bep/tests/key_store_tpm_integration.rs line 112): the noDA probe does not prove the root key has noDA.

Evidence: the probe uses the signing key, and that key carries noDA itself. So the test passes even when the root key has no noDA (Gatehouse §6.10 line 837: the helper's keys are created with dictionary-attack protection off).

Do:

  1. In key_store_tpm_integration.rs, restore the root key's authorization file and run the noDA probe against the root key, not the signing key.
  2. In each refusal the test asserts above that probe, assert the TPM response code 0x000009a2, in place of a match on any 0x code.
  3. Do not relax the swtpm proof at line 30. It must still pass in the bep-tpm-integration job.

The probe used the signing key, which has noDA itself, so it passed even
when the root key lacked noDA. Restore the root key's authorization file
and probe the root key. Pin TPM_RC_BAD_AUTH (0x000009a2) in each refusal.
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 1 of 10 on a /revise note, pushed as 044f75ad153f.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👍 valid — crates/bep/tests/key_store_tpm_integration.rs: The test now restores the root key's authorization file and probes the root key, not the signing key. Each wrong-authorization refusal must also end with 0x000009a2 (TPM_RC_BAD_AUTH), which the TPM returns only for a noDA object.

Against swtpm, the proxy's three keys are generated in the TPM, sign and agree there, carry fixedTPM and fixedParent so the TPM refuses to duplicate them, and load again from their key and authorization files alone. The integration test now also shows that the root key itself has noDA: five wrong authorizations each get TPM_RC_BAD_AUTH (0x000009a2), and after that the root key signs again with its own authorization file.

Notes from the agent: Revise round 1: addressed the /revise note in crates/bep/tests/key_store_tpm_integration.rs only. The test saves the root key's authorization file, asserts that each refusal detail ends with 0x000009a2, restores the file, then probes the root key and checks one signature. The swtpm proof at line 30 is not relaxed. Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no swtpm or tpm2-tools, so I did not run it. The new assertion depends on swtpm returning the session-1 BAD_AUTH code 0x000009a2 for a noDA object, as the unit test of response_code in tpm.rs already expects. cargo clippy -p bep --all-targets is clean.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The TPM key store conforms to Gatehouse §6.10 line 837 and to BEP-088. The new test now proves noDA on the key that receives the wrong authorization values. It asserts TPM_RC_BAD_AUTH, and then it uses that same key again. One condition is open. Nobody ran the TPM proof against a TPM. C1 requires the bep-tpm-integration job to pass on this head. If the job fails, fix the tpm2-tools arguments in tpm.rs. Do not change the assertions.

Ruled at 044f75a against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 4 findings, 4 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

Comment thread crates/bep/src/tpm.rs Resolved
A tool that exits before it reads standard input closed the pipe, and
the broken pipe returned early. The stderr with the response code was
lost and the child was never reaped. Ignore the broken pipe and wait.

(T128)
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 2 of 10 on the runtime's review findings, pushed as e701890c093a.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👎 invalid — crates/bep/tests/key_store_tpm_integration.rs:110: No code change is needed for this. The 0x000009a2 assertion and the line 120 probe stay as they are. This box has no swtpm or tpm2-tools, so I cannot run the job here. The tpm lane (bep-tpm-integration) still owes that run on the new head.
  • 👍 valid — crates/bep/src/tpm.rs:448: run_once now ignores BrokenPipe on the stdin write and always waits for the child, so it reaps the child and keeps the stderr response code. The unit test a_tool_that_exits_before_its_input_reports_its_response_code pins this.

Against swtpm, the proxy's three keys are generated in the TPM, sign and agree there, are refused for duplication, and load again with only their key and authorization files. The integration test in crates/bep/tests/key_store_tpm_integration.rs checks this. This box has no swtpm and no tpm2-tools, so this round did not run it. A tpm2-tools command that fails before it reads its input now still reports the TPM response code.

Notes from the agent: Revise round 2. In run_once, a broken pipe on the stdin write no longer returns early. The code waits for the child, reaps it, and reads the response code from stderr. A different write error is reported only when the tool exits with success. A unit test pins this with a 4 MiB input to sh that exits at once. Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no swtpm and no tpm2-tools, so I did not run it. The bep-tpm-integration job must still show it passes on this head. The assertions at lines 110 and 120 are not changed.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The compare changes only TpmStore::run_once. A tool that closes its input now keeps its TPM response code, and the path still fails closed, because each caller reads the tool's output file after it. Gatehouse §6.10 line 837 still governs, and the store conforms to it. C1 stays open from the earlier rulings. The tpm lane must run the swtpm proof and record the pass.

Ruled at e701890 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 4 findings, 4 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

Comment thread crates/bep/src/tpm.rs Resolved
Comment thread crates/bep/src/tpm.rs Resolved
A tpm2-tools command that blocks on the TPM held the key's lock for
ever. It now stops after 120 seconds with a key-store error. A killed
proxy left .work-<pid>-<n> directories behind; the store now removes
those whose process is gone.
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 3 of 10 on the runtime's review findings, pushed as d80142b9c9e1.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👍 valid — crates/bep/tests/key_store_tpm_integration.rs:32: The finding is correct: nobody ran this proof against swtpm. This box has no TPM, so I cannot run it. The tpm lane (bep-tpm-integration, BEP_TPM_TCTI set) must run it on this head and record the pass. No code changed for this thread.
  • 👍 valid — crates/bep/src/tpm.rs:451: run_once now polls try_wait against a 120 s deadline. The stdin write and the stderr read are on scoped threads. At the deadline it kills the child and returns a KeyError. A new test pins this: a_tool_that_runs_past_its_deadline_is_stopped.
  • 👍 valid — crates/bep/src/tpm.rs:399: work_dir now sweeps .work-- entries whose process is gone (no /proc/). Live processes keep theirs. A new test pins this: a_stale_work_directory_is_swept_when_the_store_opens_a_key.

Against swtpm, the proxy's three keys are generated in the TPM, sign and agree there, are refused for duplication, and load again with only their key and authorization files. This box has no TPM, so the swtpm run is still owed by the tpm lane. In this round, a tpm2-tools command that blocks is now stopped after 120 seconds with a key-store error. Stale work directories that a killed proxy left behind are now removed.

Notes from the agent: Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no TPM and no swtpm. I did not run it. The sweep removes a .work-- directory only when /proc/ does not exist, so a second live proxy keeps its directories. The new timeout message is a key-store error detail: " did not finish within 120 seconds, so the proxy stopped it".

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The TPM key store conforms to Gatehouse §6.10 line 837, as the earlier ruling on this PR found. The new deadline and the work-directory sweep fail closed and do not change key custody. The TPM proof leg is still not run at this head, so it is open. Run just test-tpm for this head and record the result, as C1 says.

Ruled at d80142b against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 4 findings, 4 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

Comment thread crates/bep/src/tpm.rs Resolved
Comment thread crates/bep/src/tpm.rs Resolved
A failed tpm2_createprimary, or a load of the new key that the TPM
refuses, now returns KeyError::Store, as KeyStore::generate_in promises.
agree removes the ECDH output file on every return path.
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 4 of 10 on the runtime's review findings, pushed as b33a59069344.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👎 invalid — crates/bep/tests/key_store_tpm_integration.rs:32: The finding is true, but this round cannot act on it. This box has no swtpm and no tpm2-tools, so just test-tpm cannot run here. The plan defers this test to the tpm lane. That lane must run it on the new head and record the result. The runner writes the PR body, not the coder.
  • 👍 valid — crates/bep/src/tpm.rs: generate_files now maps a tpm2_createprimary failure, and a TPM refusal when the new key loads, to KeyError::Store, as generate_in promises.
  • 👍 valid — crates/bep/src/tpm.rs:837: agree now holds z.point in a RemoveOnDrop guard. The file goes on every return path, including a read error.

No change to the proof this round. The code gives the proxy's three keys to the TPM: they are generated there, sign and agree there, are refused for duplication, and load again from only their key and authorization files. Nobody saw this against swtpm at this head. The tpm lane must run the deferred integration test to show it.

Notes from the agent: Revise round 4. A failed key generation now always returns KeyError::Store. This includes a tpm2_createprimary failure and a TPM refusal when the new key loads. agree now removes z.point on every return path through a drop guard. cargo clippy -p bep --all-targets -- -D warnings is clean. Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no swtpm and no tpm2-tools, so I did not run it or just test-tpm.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The TPM key store conforms to Gatehouse §6.10 line 837 and to BEP-088. The latest push makes every failed generation a store error and removes the ECDH output file on every path. Both changes are correct. Nobody ran the TPM proof on this head, and that leg is open. Run just test-tpm against swtpm and record the green result, as C1 states. The process-identity binding to a dedicated proxy user is filed as gominimal/inbox#1184.

Ruled at b33a590 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 3 findings, 3 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

Comment thread crates/bep/src/tpm.rs Resolved
relabel renames the auth file before the key file. A proxy that dies
between them left the key without its auth file. The store now moves the
orphan auth file back when it next looks at the key.
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 5 of 10 on the runtime's review findings, pushed as b3f712cb9649.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👎 invalid — crates/bep/tests/key_store_tpm_integration.rs:32: The plan defers this proof to the tpm lane. This box has no swtpm and no tpm2-tools, so just test-tpm cannot run here, and I will not report a pass that I did not see. The test is unchanged. The tpm lane must run it on this head and record the result.
  • 👍 valid — crates/bep/src/tpm.rs:958: Added recover_auth. find and relabel call it, and it moves the one orphan auth file (an auth file with no key file beside it) back next to a key that has no auth file. Test: an_interrupted_move_gets_its_authorization_file_back.

The swtpm proof in crates/bep/tests/key_store_tpm_integration.rs is still written but has not run. When the tpm lane runs it, it will show that the proxy's three keys are generated in the TPM, sign and agree there, are refused for duplication, and load again from only their key and authorization files. In this round, a proxy that dies in the middle of a key move gets its authorization file back on the next start.

Notes from the agent: Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no swtpm and no tpm2-tools, so neither that test nor just test-tpm ran. This round added TpmStore::recover_auth and the unit test an_interrupted_move_gets_its_authorization_file_back. find and relabel call recover_auth. All tpm:: unit tests pass (7 of 7), and cargo clippy -p bep --all-targets is clean.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The new commit adds recover_auth. It puts back an authorization file that an interrupted relabel left in another slot. It acts only on exactly one orphan, and it returns KeyError::Store on every I/O failure, so it fails closed. It does not change key custody, so Gatehouse §6.10 line 837 still holds. The TPM proof is still deferred and never ran against swtpm. Run it on the tpm lane and record the result (C1).

Ruled at b3f712c against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 7 of 10 on a /revise note, pushed as 5cc0514dbc74.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👍 valid — crates/bep/src/tpm.rs: Removed TpmKey::duplicate and its doc comment from crates/bep/src/tpm.rs.
  • 👍 valid — crates/bep/tests/key_store_tpm_integration.rs: Removed the key.duplicate() block. The asserted fixedTPM and fixedParent bits remain the proof.
  • 👍 valid — crates/bep/tests/key_store_tpm_integration.rs: The module doc now says that the fixedTPM and fixedParent bits the TPM reports are what stop duplication.

Against swtpm, the proxy's three keys are generated in the TPM with fixedTPM, fixedParent and noDA, which the proof reads back from the TPM's public area. They sign and agree in the TPM, and load again from their key and authorization files alone. The fixedTPM and fixedParent bits are what stop duplication, and the proof no longer makes a tpm2_duplicate call that the TPM refuses for an unrelated reason.

Notes from the agent: This round removes TpmKey::duplicate and the duplicate block in the TPM proof. It restores the module doc to say that the fixedTPM and fixedParent bits stop duplication. The commit reverts 2783114, with the doc text rewrapped. All other assertions stay, including the noDA root-key probe and 0x000009a2. cargo clippy -p bep --all-targets is clean. Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization was not run, because this box has no TPM.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The code at this head is the same as at b3f712cb, and it conforms to Gatehouse §6.10 line 837 for BEP-088. The revert removes the tpm2_duplicate probe correctly. A password session is refused for any key, so that probe did not test the custody bits. One leg stays open. Nobody ran the TPM proof against swtpm, and §6.10 needs keys that the TPM will not export. Get a green just test-tpm run on this head and record it, as C1 says.

Ruled at 5cc0514 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 3 findings, 3 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

Comment thread crates/bep/src/tpm.rs Resolved
A proxy killed inside write_private leaves <file>.tmp-<pid> with an
authorization value in it. The sweep now removes such files when their
process is gone.
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 8 of 10 on the runtime's review findings, pushed as 6110ebba71b8.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Applying loadouts: default Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-c

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👎 invalid — crates/bep/tests/key_store_tpm_integration.rs:33: No code change can satisfy this. The finding asks for a run of just test-tpm against swtpm in ci-linux-native and the result in the PR body. This box has no swtpm or tpm2-tools, and I cannot edit the PR body. The plan defers this proof to the tpm lane. The assertions stay as they are.
  • 👍 valid — crates/bep/src/tpm.rs:726: sweep_work_dirs now also removes .tmp- files whose process is gone. A unit test covers a stale file, a live file and a final file.

Against swtpm, the proxy's three keys are generated in the TPM, sign and agree there, and carry fixedTPM and fixedParent in the public area the TPM reports. They load again with only their key and authorization files. On this box a person can see the key template test pass. The swtpm proof in crates/bep/tests/key_store_tpm_integration.rs still needs the tpm lane (just test-tpm) to run.

Notes from the agent: Revise round 8. tpm.rs: sweep_work_dirs now also removes .tmp- files whose process is gone, and a new unit test a_stale_temporary_auth_file_is_swept_when_the_store_opens_a_key covers it. Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no swtpm and no tpm2-tools, so I did not run it or just test-tpm.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The new commit sweeps stale <file>.tmp-<pid> auth files with the stale work directories. That conforms, and it keeps an auth value from lingering after a killed proxy. The TPM store conforms to Gatehouse §6.10 line 837 for BEP-088. The TPM proof still did not run. Run it on the tpm lane at this head, or file an issue for it, as C1 says.

Ruled at 6110ebb against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 3 findings, 3 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

@bryan-minimal bryan-minimal left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read at 6110ebb against BEP-088. The lab ran b3f712c (prepush green, the swtpm proof ran and passed);
since then the branch merged main and changed bep only in sweep_work_dirs, which I read on this head. The design follows the spec and the PR touches only bep. Three things to change before it merges,
the first a real security bug.

1. tpm2-tools reads the auth as a C string, so about 1 key in 256 ends up with an empty auth. new_auth
makes 32 raw CSPRNG bytes and every command passes them as -p file:- on stdin. In tpm2-tools
(lib/tpm2_auth_util.c), get_auth_for_file_param strips trailing \n/\r bytes and hands the buffer to
handle_password, which falls through to handle_str_password: snprintf("%s", …). The auth the TPM
stores therefore stops at the first 0x00 byte: in ~1/256 keys the first byte is zero and the auth is empty
(usable by any member of tss, which is what the module doc says cannot happen); in ~6% it is shorter than
the 128 bits BEP-088 requires; an auth ending in 0x0a is shortened too. Creating and using the key go
through the same parsing, so every test passes, and the tests check the file's length, not what the TPM
got. The integration test has the matching 1-in-256 flake: its "wrong" auth is [0u8; 32], which parses to
the empty auth and matches a real auth that starts with 0x00. Fix: write hex:<64 hex chars> on stdin
(handle_password accepts the hex: prefix inside file content, and the buffer holds it), and pin in a
unit test that the stdin bytes are hex.

2. The ECDH shared secret is written to persistent disk under $HOME. agree has tpm2_ecdhzgen -o
write Z to <store>/.work-<pid>-<n>/z.point, reads it into Zeroizing, and RemoveOnDrop unlinks it.
An unlink does not erase it from a journaled filesystem or an SSD, and a SIGKILL between the write and the
unlink leaves it until the next sweep. Z is what decrypts a sealed value: it is the secret the TPM was
meant to keep off disk. Keep the per-call output files on tmpfs (a 0700 directory under $XDG_RUNTIME_DIR,
falling back to a memfd//dev/shm with a clear error); the key blobs and the auth file can stay where
they are.

3. The global flush on a TCTI without a resource manager races the other keys. run calls
tpm2_flushcontext -t, -l and -s after every command whenever tcti != "device:/dev/tpmrm0". That
flushes every transient object and session in the TPM, and the three keys each have their own mutex, so a
sealing agree can run while a signing sign is mid-command and flush its loaded object or HMAC session.
The proof is single-threaded and never sees it; on a real host device:/dev/tpm0 would also flush other
programs' objects. Either one store-wide lock around command plus flush when there is no resource manager,
or refuse any TCTI but a resource manager and swtpm.

Smaller:

  • The PR body says the keys "are refused for duplication"; the test asserts the fixedTPM and
    fixedParent bits, which imply it. Say that.
  • find and probe map every response code to Refused, including a set owner auth and the transient
    retry/testing/memory codes, and the start path treats Refused as a key to replace. Already #3326 (T129);
    a note, not a new ask.
  • sweep_work_dirs keeps a .work-<pid> or .tmp-<pid> whose pid now belongs to another process;
    harmless, worth a comment.
  • with_context reloads and retries on every TPM error, TPM_RC_BAD_AUTH included, so a bad auth reaches
    the TPM twice; harmless with noDA, but the retry could skip the auth codes.
  • tpm2-tools is now a runtime dependency and nothing records it for packaging.
  • The PR body still says the swtpm run is "not yet observed"; the CI job on this head ran it (1 test,
    2.8 s), so C1 has its evidence.

Checked and fine: the template bits (fixedTPM, fixedParent, sensitiveDataOrigin, userWithAuth,
noDA, not restricted) and check_public re-checking them on every load; the 0x000009a2 assertion
(file auth opens an HMAC session as session 1, so noDA gives TPM_RC_BAD_AUTH); the auth never in argv or
logs, files 0600 in a 0700 directory by atomic rename; -f plain DER for der_signature; no silent
fallback (a missing tool or device is KeyError::Store); tests on tempfile, the integration test #[ignore]
behind BEP_TPM_TCTI, nothing under $HOME.

tpm2-tools reads a raw -p file:- value as a C string, so an auth with a
zero byte reached the TPM short or empty. The tools now get the hex: form.

The ECDH shared secret goes to a 0700 directory under $XDG_RUNTIME_DIR,
or /dev/shm, never to the key directory on disk.

Without a resource manager, one process-wide lock now covers each command
and its global flush, so a flush cannot remove another key's object.
An authorization failure is not retried after a reload.

The integration test's wrong auth now differs from the real one in
every byte.
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 9 of 10 on a review requesting changes, pushed as 6ed77acc7a3a.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-config v0.0.1 (/workbench/cr

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👎 invalid — crates/bep/tests/key_store_tpm_integration.rs:33: The swtpm proof did run. The review on 6110ebb says the CI tpm job ran it on that head and it passed (1 test, 2.8 s), and the lab ran it on b3f712c. This round changes tpm.rs again, so the tpm lane re-runs the test at the new head. No code change is needed for this thread.

Against swtpm, the proxy's three keys are generated in the TPM. They sign and agree there, and carry fixedTPM and fixedParent in the public area the TPM reports, so the TPM never duplicates them. They load again with only their key and authorization files. In this round, every authorization value reaches the TPM whole as hex, and the ECDH shared secret no longer touches the disk.

Notes from the agent: Review item 1: create, sign and ECDH now write hex:<64 hex digits> on stdin through auth_input. The auth file keeps the raw 32 bytes. A unit test pins that the stdin bytes are the hex: prefix and hex digits, and decode to the value. The integration test's wrong auth is now the real auth XOR 0xa5, so it can never equal it. That the TPM accepts the hex: form inside file:- content comes from the reviewer's reading of tpm2_auth_util.c. Only the swtpm run on the tpm lane can show it. Review item 2: agree writes z.point to a per-call .work-<pid>-<n> directory (0700) under $XDG_RUNTIME_DIR/minimal-bep-tpm, or /dev/shm/minimal-bep-tpm-<uid> as the fallback. The store checks that this directory is a real directory, owned by the user, with mode 0700. If neither location exists, agree returns a key-store error that names XDG_RUNTIME_DIR. The file is removed right after the read, and the directory goes away with the call. The code does not check that the file system is tmpfs. Review item 3: a process-wide lock now covers each command and its flushes when the TCTI is not device:/dev/tpmrm0. Smaller items: with_context no longer reloads and retries on TPM_RC_BAD_AUTH or TPM_RC_AUTH_FAIL. sweep_work_dirs now has a comment on reused pids. These items are for the PR body, which the runner writes: the keys are fixed to the TPM by the fixedTPM and fixedParent bits the test asserts. The CI swtpm run passed (1 test, 2.8 s). tpm2-tools is a runtime dependency that packaging must record, and nothing in this task's layers records it. Response-code mapping in find and …[truncated]

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

The compare conforms to Gatehouse §6.10 line 837. The authorization value now reaches the TPM whole in the hex: form. The ECDH shared secret stays in a private directory in memory, and the store refuses when it has no such directory. The TPM-lane proof is not run, so it is open. C1 asks you to run it on this head and record the result. That leg also confirms the new hex: and -o tool assumptions.

Ruled at 6ed77ac against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict COMMENT — 3 findings, 3 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

Comment thread crates/bep/src/tpm.rs Resolved
Comment thread crates/bep/src/tpm.rs Resolved
secret_root now checks the file system type with statfs. A runtime
directory on the disk falls back to /dev/shm, and the store refuses when
neither is in memory.
@minimal-foundry
Copy link
Copy Markdown
Contributor Author

Round 10 of 10 on the runtime's review findings, pushed as 23d5b558a460.

line exit tail
cargo nextest run -p bep tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization 0 warning: unknown fields in task runtime-implement-test-0: network help: do you need to update to a newer version of minimal? Running task runtime-implement-test-0 in box runtime-implement-test-0-1... Compiling paths v0.0.1 (/workbench/crates/paths) Compiling client-config v0.0.1 (/workbench/cr

Verify min task run agent-verify passed.

Review threads the coder judged:

  • 👎 invalid — crates/bep/tests/key_store_tpm_integration.rs:33: The test is correct as written. The plan defers it to the tpm lane, and this box has no swtpm and no tpm2-tools. The tpm lane must run it with BEP_TPM_TCTI set to swtpm and record the result on the PR. That run also checks that -p file:- accepts the hex: value. A code change in this round cannot sup
  • 👍 valid — crates/bep/src/tpm.rs:217: secret_root now uses $XDG_RUNTIME_DIR or /dev/shm only when statfs reports tmpfs or ramfs. A runtime dir on the disk falls back to /dev/shm. If neither is in memory, agree returns a store error. The unit test covers both cases.
  • 👎 invalid — crates/bep/src/tpm.rs:788: Every caller generates into an empty slot. generate runs only after find returns None, and replace calls remove_in on New and Old before generate_in(New). So no wanted key is in the slot to overwrite or delete. The order is deliberate: auth first, then key. A crash between the renames leaves an auth

Against swtpm, the proxy's three keys are generated in the TPM, sign and agree there, carry fixedTPM and fixedParent in the public area the TPM reports, and load again with only their key and authorization files. This box has no TPM and no tpm2-tools, so the swtpm proof did not run here. The unit tests show the key template and that the ECDH secret goes only to a tmpfs or ramfs directory.

Notes from the agent: Revise round 10. Fixed the secret_root finding: secret_root_in now takes an in_memory predicate, and secret_root passes a statfs check for tmpfs or ramfs. A runtime directory on the disk falls back to /dev/shm. When neither is in memory, agree returns a store error. RAMFS_MAGIC is a local constant because libc does not export it. Deferred (lane: tpm): cargo nextest run -p bep --run-ignored ignored-only a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization. This box has no swtpm and no tpm2-tools, so I did not run it.

@minimal-arch minimal-arch Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling

BEP-088 still conforms to Gatehouse §6.10 line 837. The new commit keeps the ECDH shared secret only on tmpfs or ramfs. It refuses when neither directory is in memory, and that fails closed. One condition stays open. The deferred swtpm proof must pass in ci-linux-native on this head (C1).

Ruled at 23d5b55 against gominimal/arch@15b603e6afb8 by claude-opus-5-5.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — advisory review

Verdict CO 10BC8 MMENT — 3 findings, 3 inline.

Panel: 3 of 3 personas ran (correctness, security, supply-chain).

@norrietaylor norrietaylor left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Babysitter review of 23d5b558 against T128's plan block (#3324, proof as amended to match arch ruling 1241) and its test lines.

Conforms

  • Layers: crates/bep/src/tpm.rs (new), crates/bep/src/lib.rs, crates/bep/tests/key_store_tpm_integration.rs (new). No file outside the layers. No crate dependency, per T127's record choosing the tpm2-tools commands.
  • Template and custody: fixedTPM, fixedParent, sensitiveDataOrigin, userWithAuth, noDA. The proof reads them from the public area the TPM reports, which is the amended proof's evidence that the TPM never duplicates the keys.
  • Authorization: CSPRNG value, passed to tpm2-tools on stdin in the hex: form through -p file:- (lines 144-153), never in argv. Auth files are 0600 in a 0700 directory under ~/.minimal/bep/tpm/, and stale .work-* and .tmp-<pid> files are swept when the store opens.
  • ECDH secrets are written only under a directory that statfs reports as tmpfs or ramfs (secret_root), and are removed on every return path.
  • Observability: one info line per key loaded or generated with role, fingerprint and store = "tpm".
  • Test lines: tpm_key_template_is_fixed_exempt_from_lockout_and_needs_authorization exited 0. The deferred tpm line ran against swtpm at this head: a_tpm_key_is_fixed_exempt_from_lockout_and_needs_its_authorization PASS, 1 run, 0 skipped (run 43470339191).

Deferred

  • PRRT_kwDOPdfcxM63MwDD (LOW): the crash window between the two writes in generate_files. Recorded in gominimal/inbox#1256, because T128 is at its revise cap.

No blocking findings.

@norrietaylor
Copy link
Copy Markdown
Member

@bryan-minimal, the three changes from your review 5894914451 (at 6110ebba) are in at 23d5b558. Could you re-review?

  1. Auth read as a C string. Every command now writes the authorization value on stdin as hex:<64 hex chars> through -p file:- (auth_input, crates/bep/src/tpm.rs lines 144-153; round 9, 0e740836). an_authorization_value_reaches_the_tools_whole_in_hex (line 1293) pins the stdin bytes as hex, including a value with 0x00, 0x0a and 0xff. The integration test's wrong [0u8; 32] auth is now 32 zero bytes in hex, so it no longer collides with the empty auth.
  2. ECDH secret on disk. agree writes Z only under secret_root, a 0700 directory under $XDG_RUNTIME_DIR or /dev/shm, and only when statfs reports tmpfs or ramfs. It refuses when neither is in memory (lines 179-235, 960+; round 10, 23d5b558).
  3. Global flush race. With no resource manager, one store-wide NO_RESOURCE_MANAGER mutex (line 453) is held across each command and its tpm2_flushcontext calls (lines 578-589), so one key's flush cannot hit another key mid-command.

Smaller notes:

CI is green, arch passes (ruling 1257, C1 met by that run), and every thread is resolved.

@mitodrummer mitodrummer left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

@norrietaylor
norrietaylor dismissed bryan-minimal’s stale review November 13, 2026 21:22

Dismissed on the owner's decision. All three required changes are in at 23d5b55: hex auth on stdin with a pin test (round 9, 0e74083), the ECDH secret only on tmpfs/ramfs (round 10), and a store-wide NO_RESOURCE_MANAGER lock around each command and flush. Mapping: #3343 (comment). mitodrummer approved this head.

@norrietaylor
norrietaylor added this pull request to the merge queue Nov 13, 2026
Merged via the queue into main with commit 25021af Nov 13, 2026
39 checks passed
@norrietaylor
norrietaylor deleted the agent/3324-hold-the-proxy-s-keys-in-the-tpm branch November 13, 2026 22:01
9757
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

T128 — Hold the proxy's keys in the TPM

3 participants

0