open-SECURITY-{SOC_TYPE_II}-integration-fix#8
Merged
Conversation
… in AuditEntry.from_dict, raising AuditError instead of crashing downstream
…-ladder point_mul in ephemeral_signer
…t JSONL lines instead of silently skipping them
…ng to TSA request failures
…hically validates TSA-signed messageImprint
…ape pow (no branchy extended-Euclidean)
…ic, no throwaway privkey derivation
…id) instead of full-file scan
…A responses whose echoed nonce doesn't match the request, closing a replay gap (also includes concurrently-landed CMS signature verification in verify()/finding F-1)
…ignerInfo signature over the TSA TimeStampToken, not just the hash RFC3161TimestampAuthority.verify() (and stamp(), via a self-check) previously accepted any TimeStampResp whose granted-status PKIStatusInfo and TSTInfo messageImprint hash matched the data, with no check that anyone actually signed the response. A malicious/compromised TSA or on-path attacker with no private key could forge such bytes and have them accepted as a genuine third-party attestation. verify() now decodes the CMS SignedData, resolves the embedded signer certificate (matching issuerAndSerialNumber when present), and verifies the SignerInfo's RSA/ECDSA signature over the (re-tagged) signedAttrs -- checking the signedAttrs messageDigest attribute against the actual eContent digest first. stamp() rejects any TSA response that fails this full verification instead of persisting it. Also fixes a latent bug this change surfaced: verify()'s TSTInfo decode used a strict asn1Spec, which pyasn1 cannot reliably parse when a `nonce` field is present without the earlier optional `accuracy`/`ordering` fields (a case already worked around elsewhere via schemaless decoding) -- switched to the same schemaless positional extraction so genuine nonce-echoing TSA responses verify correctly. Adds pyasn1-modules/cryptography to the `timestamp` extra, a shared DER/CMS test-fixture builder (tests/_rfc3161_test_support.py), and regression tests covering: genuine signed response accepted; correct-hash-but-unsigned rejected; corrupted-signature rejected; signature/cert keypair mismatch rejected.
verify_trade_flow computed chain_valid via all(v is True for v in [commitment_valid, execution_valid, settlement_valid] if v is not None). When an order_id has zero phases recorded (bogus/never-created order_id), all three locals stay None, the filtered list is empty, and Python's all([]) vacuously returns True — making quantum_safe=True for a trade flow with no cryptographic evidence whatsoever. Fix: require at least one phase to actually be present before chain_valid can be True. Empty flows now fail closed (chain_valid=False, quantum_safe=False). Adds regression test reproducing the exact scenario: a nonexistent order_id against an empty AuditLog.
chain_valid previously filtered None phase results out of the all() check, so a flow missing e.g. the commitment phase could still be certified quantum_safe=True as long as the phases that were present (execution, settlement) were internally self-consistent. Now requires all three phases to be explicitly True.
verify_static()/verify_signature() and every *_valid check in verify.py only validated a signature against the pubkey embedded in that same envelope, never against a known, authorised identity. Anyone could mint a fresh keypair, self-sign a fabricated commitment/execution/settlement flow for any order_id, and have AuditVerifier.verify_trade_flow report quantum_safe=True / chain_valid=True — a tautology, not authorization. Add AccountKeyRegistry (aether_protocol_c/identity.py): an out-of-band registry of authorised account pubkeys, populated at onboarding, never derived from the audit log itself. AuditVerifier.verify_trade_flow and detect_tampering now accept a registry and gate each phase's *_valid (and therefore chain_valid/quantum_safe) on the embedded pubkey being a registered signer for the account/order scope; omitting the registry fails closed rather than silently certifying an unauthenticated flow. Regression tests reproduce the breaker's exact attacker scenario (self-signed forged flow reported quantum_safe without a registry, and rejected once a registry is populated with only the legitimate key).
AetherAI3
commented
Jul 15, 2026
| return INFINITY | ||
| if p.y == 0: | ||
| return INFINITY | ||
| lam = (3 * p.x * p.x + A) * _modinv(2 * p.y, P) % P |
Owner
Author
There was a problem hiding this comment.
random numbers label it dude
AetherAI3
commented
Jul 15, 2026
Owner
Author
There was a problem hiding this comment.
label this better simplify organize in sections / border
AetherAI3
commented
Jul 15, 2026
| bytes(seed_bytes), | ||
| hashlib.sha256, | ||
| ).digest() | ||
| ) |
Owner
Author
There was a problem hiding this comment.
label properly / border pls
…review - _point_double: name the 3/2 slope-formula literals (curve tangent-line derivative, not tunable), per review comment. - ephemeral_signer.py __init__: bracket the private-key-derivation block with start/end markers, per review comment. - identity.py: section-border comments (pubkey format / errors / registry, mutation vs query methods), per review comment.
AuditLog._append() had no lock despite check_same_thread=False signalling multi-thread use is expected. Concurrent appends could race on the read-modify-write of self._line_count, causing duplicate jsonl_line/offset index rows (INSERT OR REPLACE silently overwriting an earlier commitment's index entry with no error) and corrupting get_trade_flow()'s offset-based lookups. Added a threading.RLock guarding the whole append/rotate critical section. Regression test spawns 40 threads calling append_commitment concurrently and asserts every JSONL line has a distinct index row.
QuantumExecutionVerifier only checked commitment_sig dict-equality to link execution to commitment, never comparing the execution's actual fill terms (filled_qty/fill_price) against the commitment's authorised trade_details (qty/price). A validly-signed execution attestation could report arbitrarily different fill terms than what was committed to and still pass every check (signature, quantum binding, nonce, seed independence, chain linkage). Adds QuantumExecutionVerifier.verify_matches_commitment_terms(), wired into AuditVerifier.verify_trade_flow and detect_tampering, plus a regression test reproducing the exact commit-10@50/execute-10000@5000 scenario, verified to fail against pre-fix code.
…t authentication fix broker_settlement_sig was a bare, free-form string never independently authenticated. verify_chain only checked it was hashed consistently into flow_merkle_hash, and verify_trade_flow never checked its provenance against any registry -- so a compromised/dishonest holder of an already-authorised settlement-phase key could fabricate any broker acknowledgement (including an empty string) and have verify_chain/verify_trade_flow/DisputeProofGenerator all certify the flow as fully verified with zero proof any broker acknowledged it. Fix: broker_settlement_sig must now be accompanied by a broker_signature envelope -- a real ECDSA signature over build_broker_attestation(order_id, commitment_sig, execution_sig, broker_sig) -- verified by verify_chain, folded into flow_merkle_hash, and checked by AuditVerifier.verify_trade_flow/detect_tampering against a registered broker pubkey (AccountKeyRegistry scope "broker:<account_id>"), mirroring the existing identity-binding pattern used for the commitment/execution/settlement signers. Adds regression tests reproducing the exact fabricated-ack and unregistered-broker-key scenarios; both fail against the pre-fix verify_chain/verify_trade_flow and pass post-fix.
verify_matches_commitment_terms only cross-checked fill_symbol/fill_side against trade_details when authorised_symbol/authorised_side were not both None. Since trade_details is a free-form dict at commitment-creation time with no schema requiring symbol/side, a commitment omitting those keys skipped the symbol/side check entirely -- letting an execution report a different instrument and/or opposite side while still passing qty/price bounds. Now missing authorised symbol/side fails closed instead of being treated as an unconstrained wildcard.
…/or bounds-check bypass Mutant flipped the range guard `if not (1 <= r < N and 1 <= s < N)` to use `or` instead of `and`, so a signature with only one of r/s out of [1, N) was no longer rejected. No existing test exercised r or s individually out of range against an otherwise-valid counterpart.
…s-code narrowing Mutant narrowed the accepted PKIStatusInfo.status set from (0, 1) to (0,), silently rejecting valid RFC 3161 "grantedWithMods" (status=1) TSA responses. No existing fixture varied status, so the narrowing went undetected. Adds a status-code test helper param and tests for both the accepted (1) and rejected (2) status values.
…heck inversion Mutant inverted `_check_identity`'s guard from `if not registry.is_authorized(...)` to `if registry.is_authorized(...)`, silently suppressing COMMITMENT_UNAUTHORIZED_KEY/EXECUTION_UNAUTHORIZED_KEY/ SETTLEMENT_UNAUTHORIZED_KEY issues. The existing forged-flow regression test still passed because it also triggers an unrelated BROKER_UNAUTHORIZED_KEY issue via a separate code path, masking the break. Adds a test that registers the broker key but leaves the phase-level signing key unauthorized, isolating the phase-level identity check.
…der settlement checks
/simplify pass (reuse/simplification/efficiency/altitude review) over the
sweep-1 + sweep-2 hardening diff:
- identity.py: add AccountKeyRegistry.is_broker_authorized(), replacing
two inline f"broker:{scope}" derivations in verify.py with one chokepoint.
- timestamp_authority.py: factor _decode_tst_info() out of
_extract_tst_info_nonce()/verify() -- both were independently decoding
the same TimeStampResp -> SignedData -> TSTInfo chain.
- audit.py get_trade_flow(): batch the 3 per-phase get_by_id() calls into
one indexed SQL query + one file open (was 3 round-trips). NOTE: kept
this on the SQLite-indexed path rather than reverting to
read_by_order_id() -- that helper full-scans the JSONL via read_all()
and would reintroduce the exact regression F-23 fixed earlier.
- settlement.py verify_chain(): compare the merkle hash before running
the ECDSA broker-signature verify, so an already-mismatched settlement
short-circuits before paying for the crypto verify.
- ephemeral_signer.py: collapse the zero-privkey guard from a while-loop
with a retry counter to a single deterministic re-hash (~2^-256 event,
never reachable/testable -- a loop and counter added review surface
for a state that can't occur).
- crypto.py: attempted to remove verify_signature()'s try/except as
"duplicate" of verify_static()'s own -- reverted after
test_verify_signature_unexpected_error_logging.py (which monkeypatches
verify_static itself, bypassing its internal handling) proved it's a
real second fail-closed layer, not redundant.
137/137 tests green (pytest -q).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Multi-agent audit + adversarial-review + fix pass on
aether_protocol_c/as part of positioning Protocol-C for SOC2 Type II. Adapted LOOP-01 (backend/security audit) for this crypto/attestation library, ran every finding through a LOOP-11-style adversarial review (skeptical security-architect persona, cite-evidence-or-reject), then applied fixes with regression tests on this branch. Closed with 3 rounds of LOOP-17 breaker/builder sparring (independent red-team attacks against the hardened surface, builder fixes each confirmed hit).27 of 28 raw findings survived adversarial review; all fixed with a permanent regression test per fix.
Key fixes
verify()comparedsha256(data)against a caller-suppliedmessage_imprintfield instead of parsing the TSA's signed response — now DER/CMS-decodes the realTimeStampResp/TSTInfoand checks the TSA-attested hash. Found again independently by the LOOP-17 breaker as a "vacuous truth" bypass and further hardened (nonce-replay check, CMSSignerInfosignature check).https://(opt-in-onlyhttp) and resolved hosts are checked against private/loopback/link-local/metadata-endpoint ranges before any request.ephemeral_signer.py:_modinvmoved to fixed-shape Fermat exponentiation;_point_mulmoved to a constant-iteration Montgomery-ladder schedule (was skipping work on zero scalar bits).destroy()now actually zeroes private key material via a mutable bytearray instead of just dropping the reference._read_json, TSA response body) that could exhaust memory on attacker-controlled input size._rebuild_index()now raisesAuditErroron corrupt JSONL instead of silently skipping tampered lines; TSA request failures get structured logging instead of a bare except;verify_signature()no longer swallows unexpected exceptions.AuditEntry.from_dictnow validatesdata/signature/quantum_proofshapes and raisesAuditErrorinstead of crashing downstream.Adversarial sparring (LOOP-17, 3 rounds)
Independent breaker/builder rounds against the post-fix surface found and closed: a residual vacuous-truth path in verification, a partial-completeness gap, and a missing-identity-binding issue. All three closed with root-cause fixes + reproduction-based regression tests, not just symptom patches.
Test plan
python -m pytest -q→ 124 passed🤖 Generated with multi-agent workflow orchestration (LOOP-01 backend audit + LOOP-11 adversarial review + LOOP-12 regression backfill + LOOP-17 breaker/builder sparring), reviewed and pushed by Claude Code.