[pjcdawkins]

On Windows, the CA file given to the embedded PHP is the shipped curl bundle plus the trusted roots from the Windows ROOT store. That let PHP trust the compromised eMudhra intermediates (EM DV/OV/EV TLS CA - G3B/G3C, and their Sectigo cross-signs).

Windows distrusts those intermediates through its auto-updated disallowed certificate list (disallowedcertstl.cab). It doesn't distrust their roots, and it installs them on demand. The Disallowed store contains no certificates for them, so filtering on it would not help. PHP's OpenSSL can't read that list, so it still trusted the intermediates through the roots.

This leaves out three roots by SHA-256 fingerprint:

• AAA Certificate Services
• emSign Root TLS CA - G3
• emSign ECC Root CA - G3, which cross-signs emSign Root TLS CA - G3

The shipped curl bundle (2026-10-20) does not include them either. As a result, PHP on Windows no longer trusts other chains under these roots that Windows still accepts, such as legacy Sectigo cross-signs via AAA.

Checked on Windows 11:

• TestWindowsCABundleDistrustedRoots fails without the change (all three roots in the store end up in the bundle) and passes with it.
• With the new bundle, PHP (curl and streams) fails on every test host at *.we-have-a-problem.com. It succeeds when the three roots are added back.

🤖 Generated with Claude Code

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 2 minor points

🔍 Full review · 2 files reviewed

🔵 Minor points

• internal/legacy/ca_bundle_windows.go:74 — The distrust check runs only on certificates from the Windows ROOT store, after shipped[fingerprint] has already returned. caBundle copies caCert into the bundle without filtering it. The update-php workflow updates the shipped curl bundle automatically (CACERT_DATE/CACERT_SHA256). If a later curl bundle carries one of these roots, it goes into the bundle unnoticed, even though distrustedRoots says it is never trusted. Its comment says "The shipped bundle no longer has these roots either", but nothing checks that.
• internal/legacy/ca_bundle_windows_test.go:81 — On any machine whose ROOT store doesn't yet hold these roots, TestWindowsCABundleDistrustedRoots passes without testing anything. Windows installs them on demand, so a fresh CI runner usually won't have them. The test only logs inStore and doesn't skip, so removing the distrustedRoots check would still pass there. Nothing in the test exercises the filter itself, for example by calling a function that takes the store certificates as input.

Verification

• The new condition skips a store certificate when its SHA-256 matches one of the three hex fingerprints. hex.EncodeToString gives lowercase output, and the map keys are lowercase, so the lookup matches.
• The test's fingerprints are the same three values as in distrustedRoots, and it decodes them into [32]byte the same way certFingerprints keys certificates.
• The AAA Certificate Services and emSign ECC Root CA - G3 fingerprints match the published SHA-256 values for those roots.

The PR adds TestWindowsCABundleDistrustedRoots. It only runs on Windows, and it only catches a regression when the host's ROOT store already holds one of these roots. No test covers the filter with injected certificates.

Review details

• Commit: 2537942
• Model: claude-opus-5-5

---

Review 1 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · ⚪ 1 nitpick

🔁 Incremental · 2 files reviewed

⚪ Nitpick

• internal/legacy/ca_bundle_windows.go:97 — The rewritten usableTLSRoot doc comment wasn't re-wrapped. Line 97 runs to about 119 columns, while the lines around it wrap near 80.

Verification

• usableTLSRoot now rejects a fingerprint in distrustedRoots before any other check, and systemRootsPEM sends every store certificate it doesn't skip as shipped through that function.
• systemRootsPEM still skips shipped certificates before the distrust check, so TestWindowsCABundleDistrustedRoots now also asserts that none of the three roots is in caCert.
• TestWindowsUsableTLSRoot adds a temporary entry to the package-level distrustedRoots map and removes it in t.Cleanup; no test in internal/legacy calls t.Parallel, so nothing races on the map.

TestWindowsUsableTLSRoot now tests the distrust filter directly, using a generated certificate. TestWindowsCABundleDistrustedRoots checks the shipped bundle and the built bundle. Both run in the windows-cert-store job in ci.yml (go test -run TestWindows ./internal/legacy/ on windows-latest).

Review details

• Commits: 2537942..c4923d9
• Model: claude-opus-5-5

---

Review 2 of 10 for this pull request · View the full run