feat: G2 code-signing verifier (Mode-1) with legacy G1 transition

[DeepDiver1975]

Description

Reimplements the verification half of OC\IntegrityCheck\Checker for the new
"G2" code-signing PKI (ECDSA-P384 primary, RSA-4096-PSS fallback), with CRL-based
revocation and a time-boxed transition path for legacy G1 signatures. The signing
half (writeAppSignature/writeCoreSignature, occ integrity:sign-*) is left
untouched — it is superseded by the standalone ocsign tool.

Verification now accepts the new signature.json v2 format
({v, alg, hashes, signature, certificates:{leaf, chain[]}}) as well as the legacy
format, validates the certificate chain against bundled trust anchors, verifies the
manifest signature over the canonical bytes M, enforces CRL revocation
(fail-closed), checks appId/CN identity, and produces the same structured
integrity diff the admin UI already consumes.

The new logic lives in focused, independently-tested collaborators under
lib/private/IntegrityCheck/Verifier/; Checker::verify() delegates to them,
preserving the public API, cache, result-shape contract, and all callers.
Mode-2 attestation is intentionally out of scope (blocked upstream on the
undecided bind(H,T) token encoding).

Components

• CanonicalManifest — byte-exact reconstruction of the signed manifest M
  (gated by shared golden vectors copied from the ocsign repo).
• SignatureEnvelope — v2 + legacy parser; extracts the verbatim hashes byte-span
  (no re-serialization, so what is verified equals what is diffed).
• ManifestVerifier — ecdsa-p384-sha384 (ASN.1 DER), rsa-pss-sha384 (salt 48),
  and legacy rsa-pss-sha1 (MGF1-SHA512, salt 0).
• AlgorithmAllowlist (§3) — allowlist + G1-only legacy gate with a hardcoded
  LEGACY_SUNSET.
• TrustStore + ChainValidator (§4.2) — only bundled roots are trust anchors;
  an envelope-supplied intermediate is independently re-verified to a bundled root
  before it may vouch for the leaf. Enforces leaf CA:FALSE / digitalSignature /
  codeSigning.
• CrlFetcher / CrlValidator / CrlProvider (§5) — HTTPS-only, no-redirect fetch;
  fetched-valid → bundled → fail-closed (CRL_UNAVAILABLE).
• AppIdResolver (§7) — ASCII-only case-fold, strict regex, exact-byte CN compare;
  XXE-safe XML parsing.
• IntegrityDiffer (§4.6) — legacy EXTRA_FILE/FILE_MISSING/INVALID_HASH shape.
• Verifier — the §4 Mode-1 pipeline + the §8 G1 dual-trust warn-and-allow branch.

resources/codesigning/ migrated to the new roots/ intermediates/ crl/ layout;
the legacy G1 anchor + frozen CRL are relocated so legacy apps keep verifying.
The G2 root/intermediate and the production leaf CRL are populated by the CA
ceremony (documented in resources/codesigning/README.md) — G2 verification is
inert until then, which is intentional and strictly safer than shipping a
placeholder anchor.

Related Issue

• Fixes <issue_link>

Motivation and Context

The legacy integrity check trusts a single RSA-PSS-SHA1 PKI whose intermediate
expires in 2026. This introduces the G2 PKI (stronger algorithms, revocation via a
published CRL, revoke-from-time) while keeping the legacy G1 chain trusted through a
time-boxed transition (LEGACY_SUNSET). Spec: spec-core-verifier.md §1–§11.

How Has This Been Tested?

• test environment: PHP 8.3, PHPUnit (tests/phpunit-autotest.xml), phpseclib3
  (vendored).
• Unit + integration: tests/lib/IntegrityCheck/ — 180 tests / 357 assertions,
  0 failures. Includes byte-exact golden-vector parity, real ECDSA + RSA-PSS
  signature verification, a security-critical rogue-intermediate rejection test,
  CRL fail-closed + wrong-issuer rejection, §8 before/after-sunset transition,
  revoked-leaf and revoked-intermediate cases, and Checker delegation/contract tests.
• Cross-implementation conformance: the golden manifest vectors are shared
  verbatim with the Go ocsign signer.
• Adversarial review: whole-branch + offensive-security review, which found and
  fixed an AIA-URL SSRF (phpseclib URL fetch disabled), a fail-open DI defect, and
  extended revocation to the full chain. The cryptographic core was confirmed sound
  against rogue-chain, algorithm-confusion, canonical-M dual-parse, CRL-bypass, XXE,
  and §8-abuse attacks.

Types of changes

• New feature (non-breaking change which adds functionality)
• Technical debt

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item

Notes for reviewers

• Commits are DCO signed-off and PGP-signed.
• Follow-ups (documented, non-blocking): (1) the CA ceremony must publish the G2
  root/intermediate and the production developers.crl; (2) intermediate revocation
  is checked against the available CRL, but a dedicated root-signed intermediate.crl
  source is ceremony-pending; (3) the mimetypelist.js exclusion could use a correct
  relative-path-keyed narrowing (an incorrect anchoring attempt was reverted).

🤖 Generated with Claude Code

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[kw-fscheuer]

Deep review of the G2 verifier. Overall this is strong, careful work: the decomposition into single-responsibility collaborators is clean, the security-critical chain logic (only bundled roots are anchors; envelope intermediates are independently re-verified to a bundled root before they may vouch for the leaf) is correct, XXE and AIA-SSRF are handled, and the fail-closed CRL posture matches spec §5. Findings verified against spec-core-verifier.md (§4/§5/§8) and the PKI design doc (§9).

Two items to fix before merge, one low-severity hardening note (inline), and one deferred implementation gap.

Blockers

1. Legacy-warn (§8 case 2) apps are blocked at install — violates "warn + allow"

lib/private/Installer.php (~line 352, not in this diff) treats any non-empty result as failure:

if ($integrityResult !== []) { /* throw, refuse install */ }

A valid, expired, pre-sunset G1 app returns ['LEGACY_ACCEPTED_WARN' => true], which is !== [], so the install is refused — the exact case §8 calls "the single warn-not-block exception." hasPassedCheck() was correctly taught about this marker (Checker.php:459-463), but the install gate was not. Fix: treat the LEGACY_ACCEPTED_WARN marker as a pass at this call site.

Lesser, same area (non-blocking): core/Command/Integrity/CheckApp.php:66 returns exit code 1 for the warn marker — arguably fine for a diagnostic command, but worth aligning for consistency; and the "warn" half of "warn + allow" is never surfaced visibly to the admin even when the app is allowed.

2. v2 parser accepts an arbitrary version number — see inline comment on SignatureEnvelope.php.

Note (not a blocker)

3. decodeHashesMap can return a non-array — see inline comment on CanonicalManifest.php.

Implementation gap (non-blocking, follow-up ticket)

4. Periodic full-instance re-verification is not implemented

Spec §6 / design §9 require periodic re-verification as "the safety net that makes revocation meaningful for already-installed apps." No TimedJob/cron invokes runInstanceVerification() — it runs only on upgrade, setup, manual rescan, and per-app install. Without it, a revoked leaf on an already-installed app keeps running until an admin upgrades or clicks Rescan. This likely pre-dates the PR (which reimplements verification, not scheduling), so a follow-up ticket rather than a blocker here.

Perf suggestion for when that job is designed: Verifier::verify() fetches the CRL once per app, so a full run is N sequential HTTPS fetches (up to ~15s each against an unreachable host). The CRL is instance-wide, not per-app — memoize the fetched-and-validated CRL per generation for the duration of a run to collapse N fetches to one.

Reviewed and explicitly NOT flagged

• CRL staleness: the signature-only CRL validation (no nextUpdate rejection) is correct per spec §5 / design §9 — a stale-but-signature-valid CRL is accepted by design so airgapped instances keep working off the bundled CRL. Fail-closed only on no valid CRL.
• Intermediate revocation being inert in production (developers.crl is signed by the intermediate and can't revoke its own issuer) traces to a spec-level §4/§2/§5 inconsistency, not this PR — tracked separately.

[kw-fscheuer]

v2 parser accepts an arbitrary version number. $version = $data['v'] ?? 2 accepts e.g. {"v": 99, "certificates": {...}} as a v2 envelope and processes it with v2 canonicalization rules. Harmless today because AlgorithmAllowlist still gates the algorithm, but a future v3 with different canonical-M construction would be silently mis-parsed as v2 (wrong signed-bytes reconstruction). Suggest rejecting unknown v values explicitly — accept only the versions this verifier understands, throw MissingSignatureException otherwise.

[DeepDiver1975]

Fixed in 53a5178. SignatureEnvelope::parse() now rejects any envelope whose v is not the known version 2 (throws MissingSignatureException), so a future v3 with different canonical-M rules can no longer be silently mis-parsed as v2. Added testErrorUnknownVersion (v: 99) as a regression test.

[kw-fscheuer]

decodeHashesMap can return a non-array. json_decode($rawM, true) returns null on malformed input, and the result flows into SignatureEnvelope's array $hashesMap constructor param — a TypeError rather than the intended MissingSignatureException. Realistic malformed input is already caught by the is_array($data['hashes']) guard in SignatureEnvelope::parse(), so this is only reachable via a defect in the brace-balanced scanner; the effect is that a clean MISSING_SIGNATURE reason code degrades into a generic EXCEPTION result. Low severity / defense-in-depth: a two-line if (!\is_array($decoded)) { throw new MissingSignatureException(...); } closes it and preserves the reason-code contract. Not attacker-reachable given the upstream guard.

[DeepDiver1975]

Fixed in 53a5178. CanonicalManifest::decodeHashesMap() now guards the json_decode result and throws MissingSignatureException when it is not an array, so a scanner defect degrades into the clean MISSING_SIGNATURE reason code instead of a TypeError/generic EXCEPTION. Added testDecodeHashesMapRejectsMalformed and testDecodeHashesMapRejectsScalar.

[DeepDiver1975]

Thanks for the deep review. Addressed in 53a5178:

Blocker 1 — legacy-warn (§8 case 2) apps blocked at install. Fixed the install gate in Installer.php: a verifyAppSignature result consisting solely of the LEGACY_ACCEPTED_WARN marker is now treated as a pass, matching the "warn + allow" contract that hasPassedCheck() already implements. I also aligned the lesser same-area point: core/Command/Integrity/CheckApp.php no longer returns exit 1 for the warn-only marker, and since it still prints the marker before deciding the exit code, the warning is now surfaced visibly to the admin while the app is allowed.

Blocker 2 — v2 parser accepts arbitrary version. Fixed — see the inline reply on SignatureEnvelope.php. Unknown v values are now rejected.

Note 3 — decodeHashesMap can return a non-array. Fixed — see the inline reply on CanonicalManifest.php.

Gap 4 — periodic full-instance re-verification. Agreed this pre-dates the PR (which reimplements verification, not scheduling) and is best tracked as a follow-up ticket rather than expanded here. The CRL memoization suggestion (collapse N per-app fetches to one instance-wide fetch per generation) is noted for when that TimedJob is designed.

Full tests/lib/IntegrityCheck/ + InstallerTest suite green (183 tests) with three new regression tests covering the two hardening fixes.

[DeepDiver1975]

Code Review — G2 code-signing verifier (Mode-1)

Reviewed the full diff. Engineering quality is high: clean decomposition into individually-tested collaborators, immutable value objects, fail-closed CRL handling, XXE-guarded XML parsing, and strong adversarial test coverage for the new G2 machinery. My review focuses on where the new behavior diverges from the old verifier, since that is where existing installations can break.

🔴 High — Potential regression: leaf X.509 constraints enforced on G1 certs that lack them

ChainValidator::enforceLeafConstraints() now hard-rejects any leaf missing CA:FALSE, keyUsage=digitalSignature, and extKeyUsage=codeSigning. The old verifier checked none of these.

The real production G1 certs appear to be extension-less:

openssl x509 -in resources/codesigning/core.crt -noout -text   # shipped core leaf
  Subject: CN=core        → NO X509v3 extensions section

openssl x509 -in tests/data/integritycheck/SomeApp.crt ...      # canonical legacy dev cert
  subject=CN=SomeApp      → NO X509v3 extensions section

Every G1 test fixture is freshly minted with these extensions (gen_g1_pki.sh), so the suite passes while masking the regression. Chain validation runs (Verifier.php:111) before the §8 legacy-warn branch (which only relaxes expiry, line 163), so a real extension-less G1 leaf fails with BadChainException and never reaches the transition path — defeating the stated goal of keeping legacy apps verifying.

Action: Confirm whether real-world G1 leaves in the field carry these extensions. If not, relax enforceLeafConstraints for the g1 anchor generation, and add an extension-less G1 leaf fixture through the full pipeline.

🔴 High — Uppercase legacy CNs now rejected by AppIdResolver

assertAppIdMatchesCn() validates the leaf CN against APPID_PATTERN = /^[a-z][a-z0-9_.-]{2,63}\z/ without folding it. The old code did a plain byte compare. The legacy dev cert has CN=SomeApp (uppercase) → fails isValidId($leafCn) → CnMismatchException. All fixtures use lowercase CNs (example-app, core), hiding this. Confirm real CN conventions and add an uppercase/short-CN fixture.

🟡 Medium — No CRL freshness check (stale-CRL revocation bypass)

CrlValidator::parseAndValidate() validates only the CRL signature; nothing checks thisUpdate/nextUpdate. On the G2 network path an attacker serving a validly-signed but outdated developers.crl can suppress a revocation. Fail-closed handles unavailability but not staleness. Consider enforcing nextUpdate >= now and/or preferring the newer of fetched-vs-bundled, or document as a known limitation.

🟡 Medium — Reflection into phpseclib internals is fragile

extractNotAfterFromLeaf() (Verifier.php:217) and extractSerialFromPem() (ChainValidator.php:199) reach into the private currentCert property via reflection. phpseclib exposes getCurrentCert() publicly, returning the same parsed array. Reflection couples verification correctness to an undocumented internal field name.

🟢 Low / Minor

• Anchor-generation ordering is filesystem-dependent. determineAnchorGeneration() returns the first root in scandir() order that validates the leaf; if a leaf could anchor to both g1 and g2, the choice (and thus CRL path + legacy-warn eligibility) depends on iteration order. Sort getRoots() deterministically.
• Redundant crypto work — the chain is validated multiple times (per candidate×root, then leaf, then per-root for anchor derivation). Acceptable given caching, but the anchor step could reuse the earlier result.
• IntegrityDiffer::diff() throws a raw \Exception on the "should never reach here" branch — pre-existing, now buried in a collaborator (no reason code).
• X509::disableURLFetch() mutates global phpseclib state and is never restored — safer direction, worth a note.
• Scrub internal "Task 12/Task 13" references in Checker/VerificationResult/OnDiskHasher docblocks before merge.

Tests

Coverage of the new G2 machinery is genuinely strong. The gap is the backward-compat surface: every G1 fixture is regenerated with modern extensions and lowercase CNs, so the two High findings sail through green. CheckerTest now fully mocks Verifier, so the end-to-end legacy cert path is no longer exercised anywhere. Add real extension-less / uppercase-CN G1 fixtures run through the full Verifier pipeline.

Summary

Architecturally excellent, crypto core sound. The blocking question before merge is G1 backward compatibility: the stricter leaf-constraint and CN validation will, on the evidence of the bundled core.crt and SomeApp.crt, reject real extension-less / uppercase-CN G1 certs that the current code accepts. Verify against certs in the field, relax the G1 path if needed, and add fixtures to lock it in.

🤖 Generated with Claude Code

[DeepDiver1975]

Addressed the blocking G1 backward-compat findings in e783347.

Both 🔴 High findings confirmed and fixed. Verified against the actual shipped certs: resources/codesigning/core.crt and tests/data/integritycheck/SomeApp.crt are both extension-less with a mixed-case CN, exactly as the review predicted, and every G1 fixture was minted with modern extensions + lowercase CN — masking the regression.

• Leaf X.509 constraints on G1 — ChainValidator now determines the anchor generation before enforcing the strict leaf profile (CA:FALSE, keyUsage=digitalSignature, extKeyUsage=codeSigning), and applies it to G2 leaves only. Extension-less G1 leaves validate again, matching the old verifier (which enforced none of these).
• Uppercase legacy CNs — AppIdResolver gained a $legacyG1 path that restores the old plain case-sensitive byte compare of the raw info.xml <id> vs leaf CN (no ASCII folding, no APPID_PATTERN gate). Verifier passes $chain->isG1() in.

Backward-compat fixture through the full pipeline. Added leaf-g1-legacy-noext.crt (extension-less, CN=SomeApp, non-expired) + app-tree-g1-legacy/, driven end-to-end through Verifier::verify() in LegacyTransitionTest, plus focused ChainValidator/AppIdResolver unit tests (extension-less-G1-passes, extension-less-rejected-for-G2, legacy uppercase accepted, G2-still-rejects-uppercase). All 189 IntegrityCheck tests pass; php-cs-fixer and Phan clean.

Security posture. The relaxations are scoped to the G1 anchor generation, which is reachable only by cryptographically chaining to the bundled, ownCloud-keyed G1 root — an attacker cannot force a malicious leaf onto the relaxed path. Core-mode CN binding, manifest-over-leaf verification, the algorithm allowlist, the legacy sunset, and CRL revocation all still apply. An adversarial security review of the change confirmed no trust-bypass or privilege-escalation.

🟢 Lows also addressed:

• Roots are now ordered deterministically by filename (no scandir dependence).
• Documented the intentional never-restored X509::disableURLFetch() global mutation.
• Scrubbed internal "Task 12/13" references from Checker/VerificationResult/OnDiskHasher/Verifier docblocks.

Deferred (follow-ups, not in this pass):

• The reviewer's "prefer G2 on ambiguity" tie-break: attempting it surfaced that determineAnchorGeneration loads verified intermediates as CAs, so phpseclib's single-level validateSignature() lets a single-generation leaf validate against both roots — making a naive generation-first sort mis-classify. Hardening the anchor derivation to a true per-root path build is needed before that tie-break is safe; tracked separately.
• The 🟡 Mediums (CRL freshness / nextUpdate, and replacing phpseclib currentCert reflection with getCurrentCert()) are left for a follow-up as they touch crypto-verification behavior beyond the backward-compat scope of this pass.