Skip to content

subtree: import libbitcoinpqc v0.3.2 signer stack scrub - #268

Merged
kiwidream merged 3 commits into
1.x.xfrom
kiwidream/rg-260-libbitcoinpqc-stack-scrub
Nov 2, 2026
Merged

kiwidream merged 3 commits into
1.x.xfrom
kiwidream/rg-260-libbitcoinpqc-stack-scrub

Conversation

@kiwidream

@kiwidream kiwidream commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Summary

Refs #260. This PR targets the v1.0.1 release line (1.x.x). 1.x.x is frozen for v1.0.1 (#181), so a maintainer must lift the freeze before merging, and the v1.0.1 release notes in #181 need this entry.

Imports upstream release v0.3.2; the operator chose it for v1.0.1.

  • Release: v0.3.2. The tag object 81709ebe34dff96e7da6215f6d1cb8b296c2ecf1 is a signed annotated tag ("v0.3.2 release"); git verify-tag reports a good signature from key 289EA3EC2F1939A24984840ED26CFC05586D371E.
  • Its peeled commit is 1965d0da1eb4d9e196613b8f3a5d0d866a221c53, the squash merge of Release qbit-libbitcoinpqc 0.3.2 qbit-libbitcoinpqc#17 onto upstream main (db10f2ed1e). Its tree, 4cb1705232c5, is the tree tested below.
  • The branch was rebuilt on the current 1.x.x (02a465d09c), so the PR goes straight from v0.3.0 to v0.3.2. The earlier v0.3.1 import is gone from the history.
  • test/lint/libbitcoinpqc-subtree-check.sh against the real remote reports GOOD: split 1965d0da1e, tree 4cb1705232c5.

The leak being fixed (v0.3.0). crypto_sign_signature and crypto_sign_seed_keypair in sphincsplus/ref/sign.c copy SK_SEED into a stack spx_ctx, and nothing wipes it. The SHA-2 message schedule, which can be reversed to SK_SEED, SK_PRF and derived seeds, also stays on the stack these calls release: keygen, sign and secret-key validation.

What v0.3.2 contains, relative to v0.3.0

v0.3.2 was cut from upstream main, so it contains everything on main plus the #17 release commits:

Upstream Change Reaches qbit's build?
#5 Zero SLH-DSA secret-key scratch before keygen and sign return Yes
#6 Burn the released stack once per secret-key call: 16 KiB optimized, 288 KiB unoptimized (#17 raised it from 208 KiB) Yes, 16 KiB
#14 secure_zero_local stops wrapping len; UBSan CI The loop fix, yes
#10 BAD_KEY for signing with an inconsistent key, OUT_OF_MEMORY (-6) for allocation failures Yes; see below
#4 Test-helper-only SHA backend observers in sha2.{c,h} and test_helpers.c No: they exist only with BITCOINPQC_ENABLE_TEST_HELPERS
#13, #2, #16 ASan smoke setting, GitHub rulesets, 0.3.1 metadata forward-port No: CI, settings and docs
#17 Version 0.3.2; Rust maps -6 to PqcError::Other(-6) with no new enum variant; provenance doc Version and lockfiles
(v0.3.0 downstream patch) crossbeam-epoch 0.9.20 (RUSTSEC-2026-0204), upstream since v0.3.1 Yes; the vendored tree now equals the tag

API changes and how qbit handles them

Upstream #17 records these codes against v0.3.1. No function declaration changes, and #17 reports that the exported symbols match v0.3.1 and that cargo-semver-checks finds no semver change.

Change qbit
bitcoin_pqc_sign/_with_stats with a secret key that fails validation: BAD_SIGNATURE (-3) → BAD_KEY (-2); slh_dsa_sign: -1 → -101 (SLH_DSA_SIGN_ERROR_BAD_KEY) CPQCKey::Sign (src/crypto/pqc.cpp) checks slh_dsa_sign(...) != 0, and CPQCKey::Set already rejects such keys. No change.
Allocation failures in keygen and sign: BAD_ARG/BAD_KEY → OUT_OF_MEMORY (-6); slh_dsa_keygen: -1 → -102 MakeNewKey and DerivePQCKey check slh_dsa_keygen(...) != 0. No change.
slh_dsa_sign with rejected arguments now zeroes sig and sets *siglen = 0 qbit never passes those arguments.
sign_with_stats leaves stats unchanged when it fails before signing starts qbit calls it only in pqc_low_wots_cap_sign_failure_cleans_output, with a valid key, so signing starts and the test is unaffected.
Verify Unchanged. CPQCPubKey::Verify checks slh_dsa_verify(...) == 0.

I checked every qbit caller (src/crypto/pqc.{h,cpp}, src/test/pqc_tests.cpp, src/test/fuzz/pqc.cpp, src/test/fuzz/p2mr_script.cpp, the ci/libbitcoinpqc/*.c harnesses and contrib/testgen). Each one either compares against success or asserts a code that did not change, so no caller or test needed a fix.

What this PR does

  • 651554e474 imports upstream v0.3.2 into src/libbitcoinpqc as one squash-merge-shaped commit, Squashed 'src/libbitcoinpqc/' changes from ac72d1ffa0..1965d0da1e, with git-subtree-dir: src/libbitcoinpqc and git-subtree-split: 1965d0da1eb4d9e196613b8f3a5d0d866a221c53. It was made with git read-tree --prefix=src/libbitcoinpqc/ -u, because the signing preview cannot display git subtree pull --squash's whole-tree squash. git merge-tree --write-tree --merge-base=<v0.3.0 tree> 02a465d09c:src/libbitcoinpqc 1965d0da1e^{tree} gives the same tree, 4cb1705232c5, so this matches what the update script produces and absorbs qbit's v0.3.0 lockfile patch.
  • 9a707a8f67 pins v0.3.2 and adds the stack-residue test:
    • every pin (table below), and both contrib/testgen lockfiles move to qbit-libbitcoinpqc 0.3.2, without which the conformance job's cargo fetch --locked fails;
    • the crossbeam Cargo.lock exception stays removed from test/lint/libbitcoinpqc-subtree-check.sh and ci/scanners/run-scanners.py, with its 3 tests in test_run_scanners.py;
    • doc/subtrees/libbitcoinpqc.md documents the new codes and corrects the stack note: an -O0 in APPEND_CFLAGS lands before the forced -O2 on the compile line (a configure with -DAPPEND_CFLAGS=-O0 gives C_FLAGS = -O0 -O2 -g ...), so every qbit build uses the 16 KiB burn;
    • pqc_tests/pqc_secret_key_calls_scrub_released_stack, with the CI fix below. It runs CPQCKey::MakeNewKey, CPQCKey::Set, CPQCKey::Sign and DerivePQCKey on a painted thread stack that the test owns. It requires that no SK_SEED or SK_PRF copy remains, that the longest zeroed run below the caller is at least 12 KiB, and that at most 64 bytes were written below that run. It skips itself on Windows and under ASan, HWASan, MSan, SafeStack and TSan.
  • 61e2a2fda0 rewrites the release-notes fragment doc/release-notes-268.md for v0.3.0 → v0.3.2.

Pin locations

Location Value
src/libbitcoinpqc import, git-subtree-split 1965d0da1e…
contrib/devtools/update-libbitcoinpqc-subtree.sh DEFAULT_REMOTE_REF v0.3.2
test/lint/libbitcoinpqc-subtree-check.sh REMOTE_REF and usage text v0.3.2
ci/scanners/run-scanners.py LIBBITCOINPQC_UPSTREAM_TAG v0.3.2
test/fuzz/shared_target_mappings.json upstream.tag / peeled_commit v0.3.2 / 1965d0da1e…
doc/subtrees/libbitcoinpqc.md (tag, peeled commit, expected split) v0.3.2 / 1965d0da1e…
contrib/devtools/README.md (default tag, example) v0.3.2
contrib/testgen/{p2mr_v1_oracle,p2mr_checksigpqc_vectors}/Cargo.lock qbit-libbitcoinpqc 0.3.2

CI fix (first pushed as beb5a0c648, now part of 9a707a8f67)

Full Validation run 41101389372 failed pqc_tests/pqc_secret_key_calls_scrub_released_stack on no IPC, i686, DEBUG and TSan, depends, no gui. Each job had 4 failures, all on the zeroed-run check (zeroed run from the deepest write below the caller is 0 bytes, once 8 bytes under TSan). Neither job found an SK_SEED or SK_PRF copy on the stack. Every other job passed.

  • Cause (i686). The burn function is position-independent. After reserving its array, it loads its GOT pointer with call/pop, which pushes a return address just below the bytes it zeroes. So the deepest write is 8 bytes below the zeroed run, not its start.
  • Cause (TSan). __tsan_func_entry, one __tsan_write1 per store in the burn loop, and __tsan_func_exit all run below the burn's frame. Runtime frames therefore always lie below the zeroed window.
  • Change (test only). The check measures the longest zeroed run below the caller (still at least 12 KiB) and allows at most 64 bytes written below it. Under TSan the inspection is skipped, as under ASan, HWASan, MSan and SafeStack. The SK_SEED/SK_PRF checks are unchanged.

Complement: #270

#270 clears the stack on the qbit side. Its non-inlined *Impl functions plus a 32 KiB BurnReleasedStack() run after each CPQCKey call, and the subtree stays at v0.3.0.

If both land, the burns run one after another, not nested. Peak stack is about max(17 KiB, Impl frame + 32 KiB).

Consensus, compatibility, cost

  • No consensus change.
    • A C harness (slh_dsa_keygen, slh_dsa_sign, slh_dsa_verify only) produced byte-identical output when linked against v0.3.0, v0.3.1 and v0.3.2 static libraries built from their tag trees with qbit's flags: 4 keypairs and 16 deterministic signatures, 59,264 bytes, sha256 efff43644daf042d…. Each signature verifies, and a one-bit tamper does not.
    • Upstream Accept legacy Dogecoin-style AuxPoW payloads at the mining RPC boundary #17 reports the same for a wider harness: 8 keypairs, 96 signatures and 2,968 verify codes are byte-identical across v0.3.0, v0.3.1 and v0.3.2, with clang and gcc.
    • pqc_bounded30_known_answer_vector, the P2MR v1 vectors and conformance cases in script_p2mr_tests, and pqc_sigcache_tests pass unmodified. The P2MR v1 oracle, built against v0.3.2, verifies all 78 corpus cases.
  • Cost. v0.3.2's production code differs from v0.3.1 only in error mapping and the unoptimized burn size, so the v0.3.1 measurements still apply (clang -O3 Release, shared host, 3 interleaved runs of 20 iterations): sign about +10% (144–147 ms → 158–164 ms), keygen and validate about +12%, verify about +1%.
  • Stack. Every qbit build uses the 16 KiB burn, so each keygen, validation or sign call needs about 17 KiB of free stack. qbit's signing threads keep the platform default stack, 512 KiB or more on the release platforms.

Testing

All builds and tests ran on minidev-alex1 in a host lane (flock, 8 GB memory scope, -j5), against the tree that was committed, before signing.

  • Built locally: cmake --build build -j5 (RelWithDebInfo, wallet, tests, no GUI). The only warnings are two pre-existing -Wunused-function warnings in the vendored sha2_armv8_sha.c, which has not changed since v0.3.1.
  • Focused suites: test_qbit --run_test=pqc_tests,pqc_sigcache_tests,script_p2mr_tests,p2mr_datapqchash_tests,descriptor_p2mr_tests,psbt_p2mr_tests,key_io_p2mr_tests,wallet_p2mr_signing_tests,wallet_p2mr_parallel_signing_tests,wallet_p2mr_basic_tests,wallet_p2mr_batch_reservation_tests,wallet_p2mr_crypted_provider_tests,wallet_p2mr_encryption_tests. 260 test cases and 49,633 assertions passed.
  • Negative control. With BITCOIN_PQC_BURN_STACK_BYTES changed to 4 KiB in the imported secure_zero.h, the residue test fails with 8 of 56 assertions: for all 4 calls, the longest zeroed run is 4.3–4.7 KiB, and 8.0–8.3 KiB were written below it. With the file reverted to the tag's blob, the test passes with 56 of 56.
  • Full unit suite build/bin/test_qbit: 1296 of 1297 cases passed, plus 1 that passed with a warning. The warning is the pre-existing script_assets_test skip, because DIR_UNIT_TEST_DATA is unset. 20,014,787 assertions passed.
  • Clang 21 -fsyntax-only -Wthread-safety -Werror with the build's flags, with and without -fsanitize=thread, and clang-tidy 21 with src/.clang-tidy: clean on src/test/pqc_tests.cpp, the only C++ file changed outside the subtree.
  • Residue harness (slh_dsa_keygen, slh_dsa_secret_key_validate and slh_dsa_sign on a painted stack), built from the imported tree with qbit's subtree flags:
    • x86_64: zeroed run 16,360–16,385 bytes, starting at the deepest write. The old and new checks both pass.
    • i686: zeroed run 16,329–16,385 bytes, with 8 bytes written below it. The old check fails; the new check passes.
    • TSan: zeroed run 16,042–16,234 bytes, with 86–270 bytes written below it, so the window check fails as expected and qbit's test skips under TSan.
    • Every configuration: no SK_SEED or SK_PRF copy anywhere on the stack.
    • 4 KiB-burn mutants on x86_64 and i686: zeroed run 4.0–4.7 KiB with 7.6–8.4 KiB written below it. Both checks fail, as they should.
  • P2MR v1 conformance, as in the CI job, with qbit-libbitcoinpqc 0.3.2:
    • cargo fetch --locked --target x86_64-unknown-linux-gnu succeeds for both testgen manifests.
    • cargo fmt --check passes, and cargo test --locked --offline for p2mr_v1_oracle: 35 passed.
    • verify --release on the committed head: 78 cases passed (34 accepted, 44 rejected), manifest_sha256 d30f2fb354ee…, oracle commit 61e2a2fda0.
    • contrib/devtools/check-p2mr-v1-corpus-reproducible.sh: OK.
  • python3 ci/scanners/test_run_scanners.py: 10 tests passed. test/lint/lint-libbitcoinpqc-vectors.py: passed. KAT regeneration is skipped, as on v0.3.0, because the tag has no generator sources.
  • test/lint/libbitcoinpqc-subtree-check.sh against the real remote and tag: GOOD. Before the tag existed, I checked the same comparisons by hand: the staged subtree tree equals release/v0.3.2-prep's tree, and the tree-equality check against the 3-way merge-tree result above also passed. The script itself needs a committed import, and no commit could be signed before the tag existed.
  • Local /code-review medium on the full diff against origin/1.x.x. 10 findings; none needed a change in this PR.
    • 9 are in the vendored library, which this PR imports verbatim. They are upstream follow-ups, mostly the ones the v0.3.1 review deferred: per-compression SHA-2 wipes in the AVX2 path and the generic finalize, which verify also pays (about 1% measured on v0.3.1); the FORS worker-thread burn on a default-size thread in -O0 builds with test knobs (qbit forbids the knobs); MSVC /Od getting the 16 KiB burn; _impl helpers not marked noinline; exported *_unburned symbols; duplicated secure-zero helpers; slh_dsa_sign now zeroing a full SLH_DSA_SIGNATURE_SIZE of sig on rejected arguments, as its header documents (qbit always passes a full PQC_SIG_SIZE buffer); and Rust reporting -6 as PqcError::Other(-6) (upstream's semver choice for 0.3.x).
    • 1 is on the test: it is not skipped for -O0 UBSan builds. That does not apply to qbit, because the subtree is compiled at -O2 in every build type, and UBSan runs in CI only together with ASan, which skips the test.
  • Not run:
    • Functional tests: no RPC, P2P or wallet behaviour changed.
    • test/fuzz/test_qbit_fuzz_coverage.py and the CTest that reads shared_target_mappings.json: both need a coverage-instrumented fuzz binary, which this host does not build. The mapping change is only the tag and peeled commit, and the JSON parses.
    • lint-shell.py, lint-python.py, lint-spelling.py: their tools are not installed on this host.

Target Branch

  • This PR targets a maintainer-requested release branch: 1.x.x (v1.0.1). It is frozen per release: prepare v1.0.1 #181; a maintainer must lift the freeze before merging.

Risk / Review Notes

  • Consensus, script, crypto, wallet, P2P, release, CI, or security-sensitive behavior changed.

Notes: this updates vendored consensus crypto. Keys, signatures and verify results are byte-identical across v0.3.0, v0.3.1 and v0.3.2, so there is no consensus validity change. The library's error codes for an inconsistent signing key and for allocation failures change, and qbit treats both as failures exactly as before. Signing is about 10% slower and needs about 17 KiB of stack per call. The nightly scanner pin, the lint check and the testgen lockfiles also change.

Docs / Process Impact

  • I updated public docs: doc/subtrees/libbitcoinpqc.md (pin, removed exception, stack requirement, new codes), contrib/devtools/README.md, test/lint/README.md, and the release-notes fragment doc/release-notes-268.md.

libbitcoinpqc Subtree Checklist (if src/libbitcoinpqc changed)

  • Source commit is reachable from an immutable release tag in Qbit-Org/qbit-libbitcoinpqc: v0.3.2, which peels to 1965d0da1e.
  • qbit imports the tagged upstream tree directly, without pruning or a curated subtree branch. The tree equals 1965d0da1e^{tree}.
  • Subtree import/update was performed with contrib/devtools/update-libbitcoinpqc-subtree.sh: equivalent, as a single squash-merge-shaped commit. The tool's whole-tree squash commit cannot be shown in the signing preview, so its result tree was reproduced and checked with git merge-tree.
  • test/lint/libbitcoinpqc-subtree-check.sh passes locally against the real remote: GOOD.
  • Any default tag change in contrib/devtools/update-libbitcoinpqc-subtree.sh is intentional and matches doc/subtrees/libbitcoinpqc.md.

For the maintainer

  1. Lift the 1.x.x freeze (release: prepare v1.0.1 #181) before merging.
  2. Add this entry to the v1.0.1 release notes in release: prepare v1.0.1 #181. The fragment is doc/release-notes-268.md.
  3. Merge with "Rebase and merge", or keep the default squash message (this repository builds it from the commit messages). Either way, the import commit's git-subtree-dir and git-subtree-split trailers stay at the start of a line, where test/lint/libbitcoinpqc-subtree-check.sh and the nightly scanner look for them. If a squash message replaces them with this PR description, the subtree check fails after merge.

🤖 Generated with Claude Code

kiwidream added a commit that referenced this pull request Oct 30, 2026
Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kiwidream
kiwidream force-pushed the kiwidream/rg-260-libbitcoinpqc-stack-scrub branch from ba6435b to 18943b7 Compare October 30, 2026 16:56
kiwidream added a commit that referenced this pull request Oct 30, 2026
Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kiwidream kiwidream changed the title [blocked on upstream tag] subtree: import libbitcoinpqc v0.4.0 signer stack scrub [blocked on upstream v0.3.1 tag] subtree: import libbitcoinpqc v0.3.1 signer stack scrub Oct 30, 2026
@kiwidream kiwidream changed the title [blocked on upstream v0.3.1 tag] subtree: import libbitcoinpqc v0.3.1 signer stack scrub subtree: import libbitcoinpqc v0.3.1 signer stack scrub Oct 30, 2026
@kiwidream
kiwidream marked this pull request as ready for review October 30, 2026 17:46
@kiwidream

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T15:57:38.246678Z 61e2a2f Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 18943b770b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kiwidream

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: beb5a0c648

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kiwidream and others added 3 commits November 2, 2026 10:53
1965d0da1e Release qbit-libbitcoinpqc 0.3.2 (#17)
db10f2ed1e Forward-port 0.3.1 release metadata and provenance doc to main (#16)
edcea6cb89 Make the UBSan CI steps pass and require Linux CI on main (#14)
5fdfda2f85 Set allocator_may_return_null in the ASan release smoke (#13)
d3c6a6f0ea Prove requested SHA backend implementations execute (#4)
70008b9362 Report an inconsistent secret key from sign as BAD_KEY and allocation failure as OUT_OF_MEMORY (#10)
d165da6a15 Burn released stack once per secret-key call instead of per SHA-2 block (#6)
85cda2cfa8 Zero SLH-DSA secret-key scratch before keygen/sign return (#5)
f55b86b609 Add minimal GitHub ruleset configs (#2)
d5700dad0b Add trusted CI runner label

Imported from the upstream tag v0.3.2 (signed annotated tag 81709ebe34,
peeled 1965d0da1e, tree 4cb1705), the squash merge of
Qbit-Org/qbit-libbitcoinpqc#17 onto main db10f2ed1e. git merge-tree
from the v0.3.0 tree with qbit's subtree and this tree produces the
same tree, so this is what contrib/devtools/update-libbitcoinpqc-subtree.sh
produces, and qbit's downstream v0.3.0 crossbeam-epoch lockfile patch is
part of the tag. It is committed as one squash-merge-shaped commit, the
shape test/lint/libbitcoinpqc-subtree-check.sh reads, because the
signing preview cannot display the tool's whole-tree squash commit.

git-subtree-dir: src/libbitcoinpqc
git-subtree-split: 1965d0da1eb4d9e196613b8f3a5d0d866a221c53

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Point every libbitcoinpqc pin at the signed v0.3.2 tag (peeled
1965d0da1e, imported in the previous commit): the subtree helper
default, the subtree check, the nightly scanner and the fuzz target
mapping, plus the docs that name the tag.

v0.3.2 is v0.3.0 plus the secret-key scratch zeroization (#5), the
once-per-call stack burn (#6), the non-wrapping secure_zero_local loop
(#14), BAD_KEY for signing with an inconsistent secret key and a new
OUT_OF_MEMORY (-6) code for allocation failures (#10), and the
RUSTSEC-2026-0204 crossbeam-epoch 0.9.20 lockfile fix that qbit applied
downstream on v0.3.0, so the vendored tree now equals the tagged tree.
Drop the inert lockfile exception from the subtree check and the
scanner. qbit's callers treat any nonzero result as a failure and no
qbit test asserts a code that changed, so no caller changes. Keys,
signatures and verification results are unchanged.

The release bumps the crate to 0.3.2, so also move the
qbit-libbitcoinpqc path-dependency entry in the two contrib/testgen
lockfiles to 0.3.2; otherwise the P2MR oracle job's
cargo fetch --locked refuses the stale lockfiles.

Correct the runbook's stack note: the forced RelWithDebInfo C flags
follow CMAKE_C_FLAGS on the compile line, so an -O0 in APPEND_CFLAGS
does not select the unoptimized 288 KiB burn, and every qbit build
burns 16 KiB.

Add pqc_secret_key_calls_scrub_released_stack. It runs keygen, import
validation, signing and HKDF derivation through CPQCKey on a painted
thread stack, then requires that no SK_SEED or SK_PRF copy remains and
that the longest zeroed run below the caller is at least 12 KiB with at
most 64 bytes written below it, so no frame of the call ran below the
burn. The 64 bytes cover the burn's own prologue on i686, where
position-independent code pushes a return address below the array to
load its GOT pointer. The inspection is skipped under TSan, whose
runtime calls run below every instrumented frame, as under ASan,
HWASan, MSan and SafeStack. A 4 KiB burn fails the test.

Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kiwidream
kiwidream force-pushed the kiwidream/rg-260-libbitcoinpqc-stack-scrub branch from beb5a0c to 61e2a2f Compare November 2, 2026 15:54
@kiwidream kiwidream changed the title subtree: import libbitcoinpqc v0.3.1 signer stack scrub subtree: import libbitcoinpqc v0.3.2 signer stack scrub Nov 2, 2026
@kiwidream

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 61e2a2fda0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kiwidream
kiwidream merged commit e374ba2 into 1.x.x Nov 2, 2026
36 checks passed
kiwidream added a commit that referenced this pull request Nov 2, 2026
The comment above BurnReleasedStack() said that libbitcoinpqc returns
from key generation, secret-key validation and signing without clearing
its stack frames. The vendored v0.3.2 (#268) zeroes its secret-key
scratch and burns 16 KiB of released stack before those calls return.

Say so, and say what qbit's burn is still for. It clears DerivePQCKey's
own HKDF/HMAC state, which holds master-seed material in a frame the
library cannot reach, and it adds margin below the library's burn for
the other operations. Also update the measurement: with qbit's burn
compiled out, the operations reach at most about 17.3 KiB below their
caller (x86-64 GCC -O2), most of it the library's burn.

Comment only; no behaviour or constants change.

Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream added a commit that referenced this pull request Nov 2, 2026
Fold the release-notes fragments of #264, #265, #266 and #268 into
doc/release-notes-1.0.1.md and delete them, and write the #263 entry
from its pull request. #263's new P2MR stack-byte standardness rule is
also listed under the behavior changes.

Correct claims these merges made stale:
- the fuzz-evidence limitation said later changes touch no fuzzed code;
  #263 and #265 change the interpreter that p2mr_script runs, and #268
  replaces the library that pqc and p2mr_script call, so fresh evidence
  on the release source is now required before tagging;
- the #162 entry said data signatures are unchanged (#265 caches them);
- the no-measurement line now points to #268's library cost figures;
- the #252 entry now notes the later #264 and #263 profile edits.

Also correct the #183 entry, which said every other encryptwallet
failure returns -16; some return -4 or -8.

Record the deferred items under Known limitations: the consensus
stack-byte cap (#257), uncached failed PQC signature checks (#259), and
the remaining AuxPoW limits and open review notes from #266 (#258).

Refs #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kiwidream kiwidream mentioned this pull request Nov 2, 2026
9 of 28 tasks
kiwidream added a commit that referenced this pull request Nov 2, 2026
Address the independent review of the v1.0.1 notes:
- record that DerivePQCKey still leaves HD seed material and HKDF state
  in its own stack frame, which #268's library wipe does not reach and
  #270 would clear, and qualify the summary and the #268 entry;
- qualify #163's retry claim and record its pre-existing serial-fallback
  re-signing as a known limitation;
- say how v1.0.0 receivers handle headers messages cut short for size,
  matching the Known limitations entry;
- list every installed Linux binary in the upgrade steps;
- say that docs.qbit.org is already served by the earlier Pages workflow
  and only the versioned site from #148 is undeployed;
- give the nightly fuzz mutation phase the Known limitations entry its
  pointer refers to, and credit #164 for the Qt shutdown-test fix.

Refs #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream added a commit that referenced this pull request Nov 2, 2026
Release notes:
- add an Updated RPCs section: bumpfee rejects a replacement whose
  signer changed anything but input signatures, and revalidates the
  replacement at commit time (#167, which shipped the unmerged #146);
  send entries can omit vout, address and label (#150); list the new
  bumpfee failures under the behavior changes as well;
- restate #268's cost from the release review's measurements, with the
  SHA backends they cover;
- add Known limitations for per-descriptor PQC signature counters, the
  qbit-qt Send dialog signing while it prepares, and the platforms on
  which the libbitcoinpqc stack-wipe depth is verified.

doc/consensus/p2mr-v1.md: correct the non-normative section 9 sentence
from #264, which implied wallet-wide per-key counting. Wallet counters
enforce the 2^30 contract per descriptor and per wallet copy. No
normative text changes, but the specification bytes do, so the P2MR v1
consensus review must cite a commit that contains this change.

share/examples/qbit.conf: show the -fallbackfee default from #151, as
contrib/devtools/gen-qbit-conf.sh generates it. The generator's other
difference, the lock debug category, appears only in Debug builds and
is left out.

Refs #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream added a commit that referenced this pull request Nov 3, 2026
* crypto: clear stack released by PQC secret-key operations

libbitcoinpqc v0.3.0 returns from slh_dsa_keygen, slh_dsa_sign and
slh_dsa_secret_key_validate without clearing the stack frames that held
secret-key material, and DerivePQCKey kept the HD master seed and HMAC
state in its own frame. Each operation now runs in a non-inlined
function, after which the public CPQCKey and DerivePQCKey functions
overwrite PQC_STACK_BURN_BYTES (32 KiB) of the released stack, on success
and failure paths.

In the integrated (optimized) library build the library calls use at
most about 12 KiB of stack. The burn is sized from that measurement and
kept small, so the operations still fit in a 128 KiB thread stack, the
musl default. Tests check that the burn covers all stack the operations
write, that no secret key, master seed or HKDF state remains in the
released stack, and that the operations run on a 128 KiB thread.

Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: copy the released stack from a small frame in pqc_tests

SnapshotReleasedStack() copied the released stack through a 192 KiB
local array. With -fstack-clash-protection the compiler probes such a
frame as it allocates it, and on most targets the probes store to it:
Clang on x86 stores zeros every 4 KiB, GCC on AArch64 every 64 KiB, and
GCC on 32-bit ARM stores a register value every 4 KiB. The checks read
those stores as stack written below the burned region, or as a burned
region cut into pieces, and failed on correct builds. GCC on x86 probes
with `or $0`, which leaves memory unchanged.

The copy now runs in a leaf function with a small frame, which reads the
region below it into a buffer allocated before the stack is painted.

Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: let the pqc_tests stack probes run cleanly under Valgrind

The released-stack probes read the stack below the stack pointer, which
Valgrind's Memcheck reports as invalid reads, and the copy includes bytes
of the copying function's own frame, which Memcheck treats as undefined,
so the scans of the copy reported uninitialised-value errors.

When <valgrind/memcheck.h> is available, SnapshotReleasedStack() now
disables address-error reporting for the range it reads and marks the
copy defined afterwards. Without the header the client requests are
compiled out and nothing changes.

Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: check the library's burn directly in the scrub test

With qbit's BurnReleasedStack() on top of libbitcoinpqc v0.3.2, the
deepest zeroed run below a CPQCKey or DerivePQCKey call is qbit's
32 KiB burn rather than the library's 16 KiB one. That changes two
things in pqc_secret_key_calls_scrub_released_stack.

First, the burn's memory_cleanse() and memset() frames write below its
array, past the 64 bytes the test allowed: 38 bytes at -O2 on x86-64
and 64 on i686, and at Clang -O0 78 bytes on x86-64 and 76 on i686, so
the "no IPC, i686, DEBUG" job would fail. The CPQCKey and DerivePQCKey
runs now allow 128 bytes.

Second, qbit's burn hides the library's, so those runs no longer catch
a regression in the library's burn. The test now also calls
slh_dsa_keygen, slh_dsa_secret_key_validate and slh_dsa_sign directly
on a painted stack and holds them to the original checks: the 64-byte
limit, a zeroed run of at least 12 KiB, and no SK_SEED or SK_PRF copy.
With BITCOIN_PQC_BURN_STACK_BYTES set to 4 KiB, the direct runs fail
(longest zeroed run about 4.6 KiB, about 8 KiB written below it) while
the CPQCKey runs still pass.

Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* crypto: update the stack-burn comment for libbitcoinpqc v0.3.2

The comment above BurnReleasedStack() said that libbitcoinpqc returns
from key generation, secret-key validation and signing without clearing
its stack frames. The vendored v0.3.2 (#268) zeroes its secret-key
scratch and burns 16 KiB of released stack before those calls return.

Say so, and say what qbit's burn is still for. It clears DerivePQCKey's
own HKDF/HMAC state, which holds master-seed material in a frame the
library cannot reach, and it adds margin below the library's burn for
the other operations. Also update the measurement: with qbit's burn
compiled out, the operations reach at most about 17.3 KiB below their
caller (x86-64 GCC -O2), most of it the library's burn.

Comment only; no behaviour or constants change.

Refs #260

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Alexander Miller <alex@swaplabs.xyz>
kiwidream added a commit that referenced this pull request Nov 4, 2026
Fold the release-notes fragments of #264, #265, #266 and #268 into
doc/release-notes-1.0.1.md and delete them, and write the #263 entry
from its pull request. #263's new P2MR stack-byte standardness rule is
also listed under the behavior changes.

Correct claims these merges made stale:
- the fuzz-evidence limitation said later changes touch no fuzzed code;
  #263 and #265 change the interpreter that p2mr_script runs, and #268
  replaces the library that pqc and p2mr_script call, so fresh evidence
  on the release source is now required before tagging;
- the #162 entry said data signatures are unchanged (#265 caches them);
- the no-measurement line now points to #268's library cost figures;
- the #252 entry now notes the later #264 and #263 profile edits.

Also correct the #183 entry, which said every other encryptwallet
failure returns -16; some return -4 or -8.

Record the deferred items under Known limitations: the consensus
stack-byte cap (#257), uncached failed PQC signature checks (#259), and
the remaining AuxPoW limits and open review notes from #266 (#258).

Refs #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream added a commit that referenced this pull request Nov 4, 2026
Address the independent review of the v1.0.1 notes:
- record that DerivePQCKey still leaves HD seed material and HKDF state
  in its own stack frame, which #268's library wipe does not reach and
  #270 would clear, and qualify the summary and the #268 entry;
- qualify #163's retry claim and record its pre-existing serial-fallback
  re-signing as a known limitation;
- say how v1.0.0 receivers handle headers messages cut short for size,
  matching the Known limitations entry;
- list every installed Linux binary in the upgrade steps;
- say that docs.qbit.org is already served by the earlier Pages workflow
  and only the versioned site from #148 is undeployed;
- give the nightly fuzz mutation phase the Known limitations entry its
  pointer refers to, and credit #164 for the Qt shutdown-test fix.

Refs #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream added a commit that referenced this pull request Nov 4, 2026
Release notes:
- add an Updated RPCs section: bumpfee rejects a replacement whose
  signer changed anything but input signatures, and revalidates the
  replacement at commit time (#167, which shipped the unmerged #146);
  send entries can omit vout, address and label (#150); list the new
  bumpfee failures under the behavior changes as well;
- restate #268's cost from the release review's measurements, with the
  SHA backends they cover;
- add Known limitations for per-descriptor PQC signature counters, the
  qbit-qt Send dialog signing while it prepares, and the platforms on
  which the libbitcoinpqc stack-wipe depth is verified.

doc/consensus/p2mr-v1.md: correct the non-normative section 9 sentence
from #264, which implied wallet-wide per-key counting. Wallet counters
enforce the 2^30 contract per descriptor and per wallet copy. No
normative text changes, but the specification bytes do, so the P2MR v1
consensus review must cite a commit that contains this change.

share/examples/qbit.conf: show the -fallbackfee default from #151, as
contrib/devtools/gen-qbit-conf.sh generates it. The generator's other
difference, the lock debug category, appears only in Debug builds and
is left out.

Refs #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream added a commit that referenced this pull request Nov 4, 2026
…e v1.0.1 notes

The notes predated #270, #277 and #289, and the 2026-11-04 review of the
v1.0.1 change set found that they left out the deferred wallet and IPC
findings.

- #270: a normal entry replaces the DerivePQCKey known limitation and the
  qualifiers in the summary and #268's entry. The 17 KiB figure is scoped
  to the library, and qbit's calls need at least 32 KiB for the new wipe.
- #277: fold in release-notes-277.md. How to Upgrade gains the guidance
  for wallets encrypted by v1.0.0 that may still hold unencrypted PQC keys,
  and a downgrade statement.
- #289: fold in release-notes-278.md as a new Mining interface section, and
  add the IPC client requirement under Behavior changes.
- Known limitations add the deferred findings #279-#296 and #304, each with
  what users should do. The #143 entry now says which paths resume a pending
  keypool refill.
- The fuzz-anchor entry and the fuzz-evidence caveat cover #270 and #342.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream added a commit that referenced this pull request Nov 4, 2026
Release notes:
- fallback fee (#307): the floor is only this node's minimum relay fee,
  -mintxfee and mempool minimum. Peers with a higher minimum, or a
  congested network, may not relay or mine a fallback-rate send until its
  fee is bumped. Note the bump cost for P2MR, -fallbackfee=0 for services
  that prefer failure, and the 60-hour estimate staleness;
- #268 cost (#311): production mode pins the library's SHA-2 to its
  portable code on every platform, so SHA-NI, ARMv8 SHA and CommonCrypto
  are never used; AVX2 still speeds up keygen and signing. Restate the
  measured figures for that backend, drop the SHA-NI and test-knob-only
  scalar figures, list what was not measured, and remove the CommonCrypto
  clause from the stack-wipe limitation;
- low-work headers sync (#309) cannot complete from any peer through a
  dense AuxPoW range and retries at each announcement;
- the -15 encryptwallet wording, the libexec binaries in the upgrade
  steps (#339), the #163 planner wording (#330), the #337 docs deploy,
  and drop "reviewed" from the summary (#315).

doc/user/bitcoin-core-differences.md (#330): add the 1 MiB P2MR stack-byte
policy limit and its reject reason, and correct the data-signature charge
to 3,683 with the testnet4 and regtest legacy cases.

doc/subtrees/libbitcoinpqc.md (#333): direct library calls need about
17 KiB of stack; qbit's CPQCKey and DerivePQCKey calls need at least
32 KiB plus frames for the #270 wipe.

Remove the v1.0.0 fragments release-notes-95, -102, -128 and -132 (#340),
which shipped at v1.0.0 and are referenced nowhere.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant