Skip to content

base red: the moved-acpmux peer test accepts Apple's expired clang leaf being refused before the OU rule - #29362

Merged
lawrencecchen merged 7 commits into
feat-cmux-nextfrom
fix/acpmux-peer-signature-at-signing-time
Oct 31, 2026
Merged

lawrencecchen merged 7 commits into
feat-cmux-nextfrom
fix/acpmux-peer-signature-at-signing-time

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Base red: moved_peer_the_kernel_signature_of_a_peer_whose_executable_is_gone_is_read_and_checked (Native/OptChat/optchat-chief/tests/acpmux_peer_moved.rs) fails on every fleet host. It failed 5 of 5 at feat 038bf00, on aws-m4pro-3 and -4 (macOS 27) and on -6 and -8 (macOS 26), with its CMS signature does not verify (signer status 4, certificate result -2147409654) (CSSMERR_TP_CERT_EXPIRED).

Cause: the test uses Xcode's clang as a stand-in peer. Apple signs clang with its own leaf, and with no secure timestamp and no signing time. While that leaf was valid, the chain verified and the leaf-OU rule refused clang, which the test expected. Apple's leaf for Xcode 26.6's clang expired on 2026-10-24. Security now has no time but now to check the chain at, and it refuses the signer as an invalid certificate (signer status 4) before the OU rule runs.

The product code does not change. The base check calls CMSDecoderCopySignerStatus with trust evaluation on, so Security checks the chain as of the signature's secure timestamp, else its signing time, else now (read in Apple's open-source Security code, not tested here).

The test now accepts either signature rule as the one that refuses clang. It still proves its purpose: the kernel signature of a peer whose executable is gone is read, the peer goes through the signature rules (not the unknown-executable refusal), and it is refused.

Takes back an earlier version of this PR. Its first commits added a check of the chain as of now plus a retry at the signing time. The security review (FIX at 82f51bd) showed that this was wrong. The base already uses the signature's own time, so the red I recorded came from that version's own seam. The retry was also looser than Security's code signing: it fell back to the key holder's signing time with no Apple timestamp rule. Those commits are taken back in 3afb061.

Coverage gap (said plainly): with Xcode 26.6 on the fleet, no test reaches the leaf-OU refusal now. The fixture tests stop at the CodeDirectory team rule, which runs first. A fixture of a valid Apple-signed binary's kernel signature would bring it back. That is a follow-up (owner: me), not done here.

Testing

Focused command (fleet, macOS):

cmux-ci run --class isolated --script scripts/ci/cmux-tui-rust-check.sh --ref SHA --arg optchat-chief --arg moved_peer
  • Red on the base: 5 of 5 runs at feat 038bf00 (steps 32985ab9, 3cfdf956, 367d52bc, aa89890f, 7d878a1d) fail at the old assertion with the message above.
  • Green at the head 3afb061: step aa62cbd64274581bed28c121 (aws-m4pro-6): all 7 moved_peer tests pass, including the end-to-end clang test; clippy and fmt pass.

Observed / Not run

Observed: prepush-gate.py found no new violations. Against the branch base, the PR changes only tests/acpmux_peer_moved.rs.
Not run: no test of the leaf-OU rule (see the coverage gap above). No test shows that our time-stamped acpmux still passes after its certificate expires; that comes from reading the base code, as above.

Changelog

none

…ill passes after it expires (red)

acpmux_peer_moved fails on every fleet host since 2026-10-24 (5 of 5 runs
at feat 038bf00, aws-m4pro-3, -4, -6, -8, macOS 26 and 27): the peer
check evaluates the CMS signer's certificate chain as of now, so it
refuses any binary whose signing certificate expired after it was signed
(CSSMERR_TP_CERT_EXPIRED for Xcode's clang). Security's code signing
checks a chain as of the signature's own time (its secure timestamp, else
its signing time).

This commit adds the seam code_signature::verify_at(signature, ours, now)
(verify is verify_at as of SystemTime::now()), and checks the CMS
signature first and then its chain as of now, which is the old behavior.
The tests:
- moved_peer_signed_while_its_certificate_was_valid_passes_after_it_expires:
  the nightly fixture (leaf expires 2031-04-16, signed and time-stamped
  2026-10-21) as of 2031-06-01 must pass; it fails here.
- moved_peer_a_signature_from_after_now_is_refused: as of 2026-01-01
  (before the leaf was issued) it is refused, and the later signing time
  is never used (a guard; it passes before and after the fix).
…s of the signature's own time once its certificate has expired

The moved-acpmux peer check evaluated the CMS signer's certificate chain
as of now, so a peer signed while its certificate was valid was refused
once that certificate expired: our own acpmux after its Developer ID
certificate expires, and Xcode's clang on every fleet host today
(CSSMERR_TP_CERT_EXPIRED, acpmux_peer_moved red 5 of 5 at feat 038bf00).

As Security's code signing does, a chain that does not verify as of now
is checked again as of the signature's own time: its secure timestamp (a
time-stamping authority's signed time) when it has one, else the signing
time the signer signed. A time after now is never used, and a signature
with no time keeps the refusal. Every other rule (cdhash, team ID,
identifier, Apple Root CA anchor, the leaf's OU) is unchanged.
A chain that does not verify now says whether the signature had no
secure timestamp or signing time, was dated after now, or also failed as
of its own time. The green run at 80807ce (step 56b55177172b414957392c9b)
still refused Xcode's clang with certificate result -67818
(errSecCertificateExpired) and did not say why; this names the case.
…gnature rule refusing Apple's clang

The diagnostic run at 043330a (step fe6ee97b8e6583951437f90c) shows
why Xcode's clang is still refused: "its CMS signature's certificate
chain does not verify as of now, and the signature has no secure
timestamp or signing time (certificate result -67818)". Apple signs
clang with no time, so once Apple's leaf expired (Xcode 26.6's did on
2026-10-24) there is no time to check its chain at, and the chain rule
refuses before the OU rule the test expected.

The test's purpose is unchanged and still checked: a peer whose
executable is gone has its kernel signature read and goes through the
signature rules (not the unknown-executable refusal), and is refused. It
now accepts either signature rule as the one that refuses Apple's clang.
The OU rule itself stays covered by the fixture tests. The peer check is
not loosened: an expired chain with no secure timestamp or signing time
is still refused, unlike Security's default for code.
@lawrencecchen lawrencecchen added the risk:high Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 3afb061: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr29362 -f ref=fix/acpmux-peer-signature-at-signing-time -f platform=macos -f backend_mode=local

Shipped in:

@github-actions github-actions Bot added risk:normal Risk tier (docs/ci/risk-tiers.md) and removed risk:high Risk tier (docs/ci/risk-tiers.md) labels Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus independent reviewer): FIX at 82f51bd

Read only. I read the diff, the PR body and Apple's open-source Security (OSX/libsecurity_cms/lib/CMSDecoder.cpp, OSX/libsecurity_smime/lib/cmssiginfo.c, cmssigdata.c, cert.c, tsaSupport.c, OSX/libsecurity_codesigning/lib/StaticCode.cpp, signer.cpp). I did not build or run anything. The shipped macOS 26/27 Security can differ from the open source; the points that depend on it are marked.

Findings

  1. The OU rule has no test now (must fix). Native/OptChat/optchat-chief/tests/acpmux_peer_moved.rs:221-240: the clang test was the only test that reached the leaf-OU refusal (src/code_signature.rs:102-108). Since Xcode 26.6's leaf expired, it always takes the chain refusal, so the OU branch is never run. moved_peer_another_team_or_identifier_is_refused (tests/acpmux_peer_moved.rs:96-110) stops at the CodeDirectory team rule (src/code_signature.rs:99-101), not the OU rule. The PR body says "The OU rule stays covered by the fixture tests"; I found no such test. The OU rule is the one that binds the self-declared CodeDirectory team to an Apple-issued certificate, so it is the main authorization rule here. Fix: commit clang's kernel signature as a fixture (as nightly-acpmux-signature.bin is), and assert verify_at(&clang_fixture, &apple, <a date inside its leaf's validity>) returns exactly the "signing certificate's team (OU)" error. This is deterministic and does not depend on the Xcode version installed on the host.

  2. The retry trusts any TSA chain and an attacker-chosen signing time (must fix, or explain in the PR). src/code_signature.rs:421-433, 540-556.

    • signer_time falls back to CMSDecoderCopySignerSigningTime. That value is a signed attribute, so the holder of the key picks it (signer.cpp:338-348 lets the signer set any kSecCodeSignerSigningTime). Revocation is never checked (SecPolicyCreateBasicX509, no revocation policy). So a leaked key of our team, with an expired leaf, can sign a new binary back-dated into the leaf's validity, and this path accepts it.
    • StaticCode.cpp (verifySignature) adds one more rule when a secure timestamp is present: if the signature chain ends at an Apple CA, the TSA chain must also end at an Apple CA (CMSDecoderCopySignerTimestampCertificates, then isAppleCA on the last certificate). This PR does not have that rule. The TSA token is checked by tsaSupport.c verifySigners against the system trust store with kSecPolicyAppleTimeStamping. That policy checks only the timestamping EKU and lets an expired TSA chain pass, so any public CA's timestamping certificate gives a time that is accepted.
    • Note: the null policy you pass is effectively not used. CMSDecoderCopySignerStatus already checked the token (with SecPolicyCreateAppleTimeStampingAndRevocationPolicies) and cached timestampTime, and SecCmsSignerInfoGetTimestampTimeWithPolicy returns the cached time first. A token that is present but does not verify makes the signer status kCMSSignerInvalidSignature, so it is refused before the retry. That part is correct.
    • Our acpmux is time-stamped (the fixture is), so the stricter rule costs nothing: retry only with a secure timestamp whose TSA chain ends at Apple Root CA (compare its last certificate with APPLE_ROOT_CA_SHA256, as the signer chain is compared), and remove the signing-time fallback. The PR already makes stricter choices than Security for code with no time, so this follows the same reasoning. Not a regression against base (see 3), but this PR now owns this rule explicitly and says it matches Security's code signing, so it must match it.
  3. The diagnosis in the PR body is not what base did (fix the description). At base, CMSDecoderCopySignerStatus(..., evaluateSecTrust = 1, ...) already checked the chain as of the signer's own time: SecCmsSignerInfoVerifyCertificate (cmssiginfo.c) uses the TSA time, else the signing time, else now, and CERT_VerifyCert (cert.c) calls SecTrustSetVerifyDate with it before CMSDecoderCopySignerStatus evaluates. So base would have accepted the nightly acpmux in 2031. The red at 7ea0b59 came from that commit's own chain_verifies(trust, absolute_time(now)), not from base behavior. The real cause of the base red is only clang: no timestamp and no signing time, so it was checked as of now and its leaf expired. Compared with base, the code now (a) also accepts a chain that verifies as of now when it does not verify as of the signer's time, and (b) refuses a signer time after now. The Changelog line ("Fixed: Chief brains keep working ... after its signing certificate expires") describes a failure that base did not have. (This depends on shipped Security being the same as the open source; the fleet result for clang agrees with it.)

  4. Q1, signature check without trust evaluation: correct. With evaluateSecTrust = 0 (src/code_signature.rs:510-529), kCMSSignerValid still requires SecCmsSignerInfoVerifyWithPolicy to pass: the content-type and message-digest attributes against the digest of the detached CodeDirectory, the signature over the signed attributes, and any timestamp token. Only the SecTrust evaluation is skipped (CMSDecoder.cpp:515-548). A rewritten CodeDirectory still fails. The test moved_peer_a_rewritten_code_directory_fails_its_cms_signature asserts only err.contains("CMS") (tests/acpmux_peer_moved.rs:143); the new chain refusals also contain "CMS", so make it assert "its CMS signature does not verify (signer status".

  5. Q3, other rules: no change. After a retry, SecTrustCopyCertificateChain reads the chain from the last evaluation, and the Apple Root CA anchor check and the leaf OU still run on it (src/code_signature.rs:558-582). The cdhash, identifier and CodeDirectory team checks are not changed. grep in Native/OptChat and cmux-tui finds no other code or test that matches the old "signer status ..., certificate result ..." text.

  6. Q4, FFI: correct. CFDateCreate(CFAllocatorRef, CFAbsoluteTime), CFErrorGetCode -> CFIndex (isize), SecTrustSetVerifyDate(SecTrustRef, CFDateRef) -> OSStatus, SecTrustEvaluateWithError(SecTrustRef, CFErrorRef *) -> bool (Rust bool is C _Bool), CMSDecoderCopySignerSigningTime(decoder, size_t, CFAbsoluteTime *) and CMSDecoderCopySignerTimestampWithPolicy(decoder, CFTypeRef, size_t, CFAbsoluteTime *) match the headers. The CFDate and the CFError both follow the create rule and are released by Cf; the trust retains the date. absolute_time uses the correct epoch offset (978307200 s). There are now up to two SecTrustEvaluateWithError calls; this path is not on a UI thread.

  7. Q5, tests: deterministic, but weak in two places. The 2031-06-01 and 2026-01-01 dates use fixed signatures and fixed dates, and BasicX509 needs no network, so they are deterministic.

    • moved_peer_a_signature_from_after_now_is_refused (tests/acpmux_peer_moved.rs:91-94) asserts only is_err(). Assert the "the signature is dated after now" message, so that it proves the rule its name gives.
    • moved_peer_signed_while_its_certificate_was_valid_passes_after_it_expires (:79-85) passes with the timestamp or with the signing time (both are 2026-10-21), so it does not prove the secure timestamp is used. If you make the change in finding 2, this test proves it.
    • The clang assertion (:229-240) is not vacuous: the chain refusal comes after CS_VALID, cdhash, identifier, CodeDirectory team and the CMS signer status, so the end-to-end property (the kernel signature of a peer whose executable is gone is read, checked and refused) is still proved. What it no longer proves is the OU rule (finding 1).

To ship

  • Add the OU fixture test (finding 1).
  • Retry only with an Apple-anchored secure timestamp, or write in the PR why the signing-time fallback and a non-Apple TSA are acceptable for this authorization check (finding 2).
  • Correct the PR body and the Changelog line (finding 3).
  • Tighten the two assertions (findings 4 and 7).

If the base red must go first, the test change alone (tests/acpmux_peer_moved.rs:221-240) fixes it, together with the OU fixture test.

…test changes (review FIX)

The security review of 82f51bd (FIX) is right. The base check calls
CMSDecoderCopySignerStatus with trust evaluation on, and Security already
evaluates the chain as of the signature's secure timestamp, else its
signing time, else now. The red recorded for this PR came from its own
seam, which checked the chain as of now; the base would accept our
time-stamped acpmux after its certificate expires. The retry also added a
fallback to a key holder's signing time with no Apple timestamp rule, which
is looser than Security's code signing.

So code_signature.rs is the base again, and the two verify_at tests go.
The only real base red stays fixed: Apple signs Xcode's clang with no
secure timestamp or signing time, so since its leaf expired (Xcode 26.6,
2026-10-24) Security refuses it as an invalid certificate (signer status 4)
before the OU rule. The end-to-end test now accepts either signature rule,
and still proves that a peer whose executable is gone has its kernel
signature read and is refused by a signature rule.
@github-actions github-actions Bot added risk:low Risk tier (docs/ci/risk-tiers.md) and removed risk:normal Risk tier (docs/ci/risk-tiers.md) labels Oct 31, 2026
@lawrencecchen lawrencecchen changed the title fix(optchat-chief): the acpmux peer check verifies a signer's chain as of the signature's own time once its certificate has expired base red: the moved-acpmux peer test accepts Apple's expired clang leaf being refused before the OU rule Oct 31, 2026
@blacksmith-sh

blacksmith-sh Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus independent reviewer): SHIP at 3afb061

Read only. I read the diff against the merge base, src/acpmux_peer.rs, src/code_signature.rs, the PR body and the earlier FIX review. I did not build or run anything.

Findings

  1. Scope: correct. Merge base 49b6d497969e. git diff --stat against it shows one file: Native/OptChat/optchat-chief/tests/acpmux_peer_moved.rs (+14 / -5). The diff of src/code_signature.rs against the base is empty, so the retry, verify_at and the signing-time fallback from the earlier commits are all gone. origin/feat-cmux-next has no new commits under Native/OptChat/optchat-chief since the merge base.

  2. The test fix: correct for the fleet red, and it still proves its purpose.

    • The base message its CMS signature does not verify (signer status 4, certificate result -2147409654) comes from cms_signer_team (src/code_signature.rs:445-449). Status 4 is kCMSSignerInvalidCert, and -2147409654 is CSSMERR_TP_CERT_EXPIRED. That agrees with an expired leaf on a signature that has no time of its own.
    • verify reaches cms_signer_team only after CS_VALID, not ad hoc, SHA-256 CodeDirectory, cdhash, identifier and CodeDirectory team all pass, and after the CMS decode and the signer count. So either accepted message proves that the kernel signature was read and went through those rules.
    • In acpmux_peer::check, the unknown-executable text (pid N's executable is unknown, or ... which is gone) is only the prefix of the error from moved. Every early return (uid, no ACPMUX_BIN, pid N runs X, not Y) contains neither substring, and the of_file/running_peer errors contain neither. So a match proves that check went past the path rule into moved -> verify. If a regression made verify accept clang, unwrap_err panics.
    • A forged CodeDirectory gives status 3 (kCMSSignerInvalidSignature), not 4, so the new alternative does not hide the signature-over-content check.
    • What the alternative can hide: a regression that makes the trust evaluation refuse every peer with status 4 for some other reason (a wrong policy, a wrong verify date) is caught by moved_peer_a_moved_acpmux_signed_by_our_team_as_our_acpmux_passes, which needs Ok. A regression that affects only signatures with no time (clang's kind) would pass this test. Optional tightening: also match certificate result -2147409654, so that only the expired-leaf cause is accepted. I do not require it, because Security can give a different TP code on another OS.
  3. The coverage gap statement: accurate, and acceptable here. No test reaches the refusal branch at src/code_signature.rs:89-94. moved_peer_another_team_or_identifier_is_refused stops at the CodeDirectory team or identifier rule (:82-88), and the nightly fixture test runs the OU comparison only on the equal path. The gap comes from Apple's leaf expiry on base, not from this PR, and the OU code does not change. A base red is priority zero, so deferring the fixture to a named follow-up is acceptable. Note for that follow-up: the fixture that the earlier review suggested used verify_at, which this PR removes. A fixture of clang's signature, or of any Apple signature with no secure timestamp, is now checked as of now, so it will give status 4 again. The fixture must have a secure timestamp inside its leaf's validity (or the test needs a separate seam), and its leaf OU must differ from its CodeDirectory team.

  4. Risk tier: correct. The PR has risk:low now (not risk:normal). It changes only a test, which is the low tier in the risk tiers doc. No product, auth, socket or signing code changes.

  5. Checks on this head: none caused by this PR. nightly-guards / workflow-guard-tests / release-notary and nightly-guards / Guard status are red on every recent feat-cmux-next PR I looked at (base red: name the suites whose windows stay open when the Swift suite is cut short (timed window census) #29381, chat-open bench: keep the click reply's clicked flag (follow-up of #28940) #29383, Fix Home dock Channels: the Chromium page gets the keyboard at its first open #29386, cmux NEXT: a remote chat attach cancelled while it waits its turn is never sent #29388, base red: webview shard 2 (stall log l10n, automation queue fixture, shared find chunk) #29389): a base finding. action export areas failed at checkout (fatal: early EOF, could not fetch ... from promisor remote): infrastructure, rerun the same SHA.

  6. PR body: accurate. It describes the take-back, the cause of the red and the gap correctly. The sentence "Our time-stamped acpmux still passes after its certificate expires" comes from reading Security's source (earlier review, finding 3). No test on this head checks it. It belongs under "Not run", not under the summary as an observed fact. This is a wording point only. It does not hold the merge.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 31, 2026
@cursor

cursor Bot commented Oct 31, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 19:29 Inactive
@lawrencecchen
lawrencecchen merged commit 23bdcfc into feat-cmux-next Oct 31, 2026
138 of 139 checks passed
@lawrencecchen
lawrencecchen deleted the fix/acpmux-peer-signature-at-signing-time branch October 31, 2026 21:02

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 80549145 Deployed Oct 31, 2026 by cmux-bot[bot] via cmux-tui tree preflight #91140
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant