Accept pki-core certificate chains when building CA pools (WDY-2339)

[justsem]

Confirms and fixes the latent bug flagged in the WDY-2339 device-enrollment map.

The bug

pki-core emits certificates with trailing bytes after the outer ASN.1 SEQUENCE, so x509.ParseCertificate rejects them with trailing data. The repo already knows this — certs.ParseCertsFromPEM strips to the exact outer element, and certs.LeafCertificatePEM exists so ML-DSA chain certs are never sent in a handshake.

x509.CertPool.AppendCertsFromPEM has no such tolerance. It drops every certificate in such a chain and returns false.

Six call sites treated that false as fatal, and the failure is worse than an untrusted chain. The device stores the issued chain as its only trust material, so once it is enrolled against pki-core:

• brokerTLSConfig returns no valid CA certificates in chainPEM — the tunnel broker dial never happens.
• The cloud notification sender returns parse Wendy enrollment CA chain — same.

Two other sites ignored the boolean and silently degraded to system roots only. That divergence — one hard-fail, one silent pin loss, from the same input — is what the ticket flagged as "one of the two is wrong". Both were.

The fix

certs.AppendChainToPool(pool, chainPEM) int routes the append through the tolerant parser and returns a count instead of a boolean, so "empty chain" stays distinguishable from "ML-DSA chain". Every caller that puts a Wendy-issued chain into a CertPool now uses it: agent broker client, notification sender, telemetry flusher; CLI broker dialler, registry proxies, wendy-lite provider.

mtls/server.go and certs.BuildServerVerifyConnection are left alone — they already pair AppendCertsFromPEM with ParseCertsFromPEM and hand the parsed certs to the ML-DSA-aware verifier, so the pool there is only the classical fast path.

Tests

A chain re-encoded with trailing bytes reproduces the pki-core shape from an ordinary ECDSA cert, so no ML-DSA material is needed. The tests assert AppendCertsFromPEM refuses it (the premise — it fails loudly if a future Go release starts accepting it), AppendChainToPool accepts it, and brokerTLSConfig builds a config from it. Both new tests were confirmed to fail against the pre-fix code.

Full local suite: 83 packages pass with -race. The only failures are the pre-existing internal/agent/oci GPU-entitlement tests, which fail identically on a clean tree here (no NVIDIA/Pi device nodes).

Not in scope

The rest of WDY-2339 is untouched and still open: no ACME/EST client, IdentityFromCert still refuses SPIFFE identities (deferred to the WDY-2808 parser pass), and the trust anchor still arrives inline in the issuance response rather than pinned from the OS image.

[github-actions]

AI Security Review

Note

Automated security review from Claude. Apply, adapt, silence with // SECURITY: <reason>, or dismiss as needed.

Input coverage: 9/9 changed files; 12,431/12,431 bytes reviewed; diff SHA-256 a80d47f8a776cbabef35068f6b577e1eb4b4173901663cf8617f70b763946897; truncation: none.

Claude found security review findings for this PR.

💡 Info — Open LOW: wendy-lite provider silently trusts system roots when chain is empty or unparseable

go/internal/cli/providers/microwendy.go:733-739: AppendChainToPool's return count is ignored here, so an empty or malformed PemCertificateChain yields an empty pinning pool without any error.

Details

**Status:** Open
**Severity:** LOW
**Standards:** SOC2-CC6.1, ISO27001-A.8.24, NIST-SC-12
**Location:** `go/internal/cli/providers/microwendy.go:733-739`

Unlike the broker and notification call sites, `connectClient` discards the count returned by `certs.AppendChainToPool(rootCAs, certInfo.PemCertificateChain)`. If the chain is empty or unparseable, `rootCAs` is empty and the subsequent `ConnectWithMutualAuthentication` proceeds with a pool that pins nothing, potentially degrading trust silently. This preserves the pre-existing behavior (the old code also ignored the boolean), so it is not a regression, but the PR explicitly calls out that "silent pin loss" is the bug being fixed — this site was not brought in line with the others.

Remediation: check the returned count and return an error when it is `0` for a required chain, matching `brokerTLSConfig`/`connectionFor`.

💡 Info — Open LOW: Tolerant parser masks malformed-input errors as zero count

go/internal/shared/certs/mldsa.go:176-187: ParseCertsFromPEM discards parse errors, so callers cannot distinguish 'empty chain' from 'chain present but all entries corrupt/garbage'.

Details

**Status:** Open
**Severity:** LOW
**Standards:** SOC2-CC7.1, ISO27001-A.8.24, NIST-SI-10
**Location:** `go/internal/shared/certs/mldsa.go:176-187`

`AppendChainToPool` relies on `ParseCertsFromPEM`, which "skips what it cannot parse and never returns an error." A chain of legitimately non-empty but wholly corrupt PEM returns count `0`, indistinguishable from an empty string. Most call sites treat `0` as fatal, which is safe, but the wendy-lite path (see separate finding) and any future caller that treats `0` as merely 'no pinning' would silently accept corrupt trust material as absence of trust. Consider having `AppendChainToPool` also surface whether any PEM blocks were present-but-dropped so callers can fail closed on corruption vs. absence.

This is a hardening/clarity concern rather than an exploitable flaw given current call-site checks.

💡 Info — Open INFORMATIONAL: Trust anchor still delivered inline in issuance response

go/internal/agent/services/tunnel_broker_client.go:172-176: The PR notes the trust anchor arrives inline in the enrollment response rather than being pinned from the OS image (WDY-2339 remains open).

Details

**Status:** Open
**Severity:** INFORMATIONAL
**Standards:** SOC2-CC6.1, ISO27001-A.8.24
**Location:** `go/internal/agent/services/tunnel_broker_client.go:172-176`

As the PR body acknowledges under "Not in scope," the CA chain used to build these pools still arrives inline in the issuance response rather than being pinned from an immutable OS-image trust anchor. Until that is addressed, the security of these pinned pools depends entirely on the integrity of the enrollment channel. This is out of scope for this diff and tracked separately; recorded here for traceability.

💡 Info — Open INFORMATIONAL: Trailing-byte tolerance widens accepted certificate encodings

go/internal/shared/certs/mldsa.go:167-188: Accepting certificates with trailing bytes after the ASN.1 SEQUENCE is intentional but relaxes strictness relative to standard Go parsing.

Details

**Status:** Open
**Severity:** INFORMATIONAL
**Standards:** SOC2-CC6.1, ISO27001-A.8.24, NIST-SC-8
**Location:** `go/internal/shared/certs/mldsa.go:167-188`

The fix deliberately tolerates the non-conformant pki-core encoding (extra bytes after the outer SEQUENCE). Because `ParseCertsFromPEM` strips to the exact outer element and still runs `x509.ParseCertificate` on the trimmed DER, the certificate contents themselves are validated normally, so this does not bypass certificate verification. The residual risk is only that malformed-but-benign trailing data is silently accepted, which is the intended workaround. Recommend documenting the accepted deviation and, longer term, fixing pki-core issuance to emit strict DER so this tolerance can be removed.

Noted as informational; the actual chain verification via VerifyConnection/InsecureSkipVerify pinning is unchanged by this PR.

Compliance summary

The change routes CA-pool construction through a tolerant parser to fix a device-enrollment TLS failure and unify divergent error handling; certificate content validation is preserved. Residual concerns are minor: one call site (wendy-lite) still ignores the parsed count and can silently fall back to system roots (**SOC2-CC6.1**, **ISO27001-A.8.24**), and the tolerant parser cannot distinguish empty from corrupt input (**NIST-SI-10**). The inline trust-anchor delivery noted by the author remains an open supply-chain/trust-establishment gap tracked under WDY-2339. No PCI DSS or HIPAA data domains are touched.