Repository navigation
feat(windows): build curl with Schannel again - #4
Conversation
Go back to static-php-cli's default Schannel build of curl on Windows, removing the patch that switched it to OpenSSL. With no CA file, Schannel verifies certificates with Windows itself: the certificate store (including roots installed by an organization, and roots Windows downloads on demand), the disallowed certificate trust list, and revocation. curl on OpenSSL can only be given a copy of the store's roots, which misses Windows' distrust decisions, such as the eMudhra intermediates distrusted only through the disallowed CTL. The CLI previously had to pass a CA file, which Schannel uses instead of the store. The CLI will stop doing that, and will need this build. The SSL backend check now requires Schannel.
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 2 warnings · 🔵 1 minor point
🔍 Full review · 3 files reviewed
Verification
- After removing the patch step, nothing in the repository still refers to
scripts/patch-spc-windows-curl.php; grep finds nopatch-spcmatch. - The backend check compares
curl_version()['ssl_version']with the literalSchannelusing PowerShell's-ne, which ignores case. A build with an OpenSSL curl fails this step. - The order of the Windows job is unchanged apart from the removed patch step.
doctor,downloadandbuildstill run against the unmodified spc clone.
The only verification is the Check the SSL backend step in the build-windows-amd64 job of build-php.yml, which runs on pull requests and fails unless curl reports Schannel. Nothing tests how certificates are actually verified, and nothing ties a release to the CLI change.
Review details
- Commit: 5b5c02e
- Model: claude-opus-5-5
Review 1 of 10 for this pull request · View the full run
| Write-Host "openssl extension: $openssl" | ||
| if ($curl -notlike 'OpenSSL/*') { | ||
| throw "expected curl to be built against OpenSSL, got '$curl'" | ||
| if ($curl -ne 'Schannel') { |
There was a problem hiding this comment.
🟡 Warning — An automatic release can bring back Windows TLS failures for users behind corporate root certificates before the CLI is ready for it.
Nothing in the repository holds a Schannel build back until the CLI change lands. check-updates.yml runs every day on a cron schedule. When any upstream source changes, it calls this workflow with create_release: true. So after this merges, the next PHP or library update publishes a php-X.Y.Z-N release with a Schannel curl and no one has to act. Today the CLI passes a CA file. Given that file, Schannel checks against it alone, ignoring the Windows store, and refuses bundles over 1 MiB. Windows users behind an organization's root certificate would get cURL error 60 again, which is the problem #1 fixed. The PR description says such a release must not reach the CLI first, but only the order in which people merge and release enforces that.
There was a problem hiding this comment.
Agreed that nothing here holds a release back. A release only reaches the CLI when the CLI pins its PHP_BUILD and checksums, so the order is: merge this, release, then upsun/cli#259 pins that build (its Windows test requires Schannel). An automatic update-php PR opened in between must not be merged before #259. No change here.
🤖 Addressed by Claude Code
The README now says that either openssl.cafile or curl.cainfo makes PHP's curl pass a CA file, which Schannel uses instead of the store, and that the CLI makes HTTPS requests with curl only. The Windows job no longer checks out the repository: only the removed patch script needed it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Note
Reviewed — No new issues found · 1 still open
🔁 Incremental · 2 files reviewed
Outstanding from earlier reviews:
- 🟡 #4338712979 —
.github/workflows/build-php.yml:327: An automatic release can bring back Windows TLS failures for users behind corporate root certificates before the CLI is ready for it. — Still nothing in the repo holds back a release: check-updates.yml can publish a Schannel build with create_release: true, and only the pin order in upsun/cli#259 keeps it from reaching the CLI first.
Verification
- After the checkout step was removed, no remaining step in the Windows job (clone, setup-php, composer, doctor, download, build, SSL check, collect, upload) reads a file from the repository checkout.
- The README no longer says PHP streams need a CA file; it now says the CLI sets neither ini value and makes HTTPS requests with curl only.
- The README says ext/curl sets CURLOPT_CAINFO from openssl.cafile and falls back to curl.cainfo; this matches the extension's default-options code as I recall it (not re-read this pass).
This repository has no tests for its workflows. The Windows job's Check out the SSL backend step is the only check: it fails the build unless curl_version()['ssl_version'] is Schannel. The README change is documentation only and nothing checks it.
Review 2 of 10 for this pull request · View the full run
Build 2 of PHP 8.5.12 builds curl with Schannel on Windows (upsun/cli-php-builds#4). No source archives changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(legacy): let Schannel verify certificates on Windows The embedded PHP's curl is now built with Schannel (upsun/cli-php-builds), which verifies certificates with Windows: the certificate store (including roots installed for TLS inspection, and roots Windows downloads on demand), the disallowed certificate trust list, and revocation. The wrapper no longer writes a CA bundle (the shipped curl bundle plus the store's roots) or sets openssl.cafile: PHP passes that to curl, and Schannel would then verify against the file instead of the store. The bundle missed trust decisions Windows makes outside the ROOT store, such as the eMudhra intermediates distrusted only through the disallowed CTL. This removes the CA bundle code, the embedded curl.se bundle and its updates in update-php.sh, and file.WriteIfChanged, which only the bundle used. The Windows test now checks that curl is built with Schannel, that an installed root is trusted with best-effort revocation checks, and that a CA file would replace the store. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(legacy): make all HTTPS requests with curl, verified by Schannel on Windows PHP streams use OpenSSL, which cannot use the Windows certificate store, so the two remaining HTTPS requests made with them now use Guzzle and curl: - The activity log stream, with a new HttpStream class, which reads a response as it arrives through Guzzle's CurlMultiHandler, waiting for a limited time, as the non-blocking stream did. - The credential helper download. Config::getGuzzleOptions() replaces getStreamContextOptions(), and holds the TLS, proxy and user agent options shared by every HTTP client. With curl built against Schannel, it sets no CA file, so that Windows verifies certificates, and sets CURLSSLOPT_REVOKE_BEST_EFFORT: Schannel's default strict revocation checks reject roots without revocation information, such as many installed to inspect TLS traffic. The API client library's Connector only passes "verify" to Guzzle, so a subclass adds a "curl" option, applied to API requests with a middleware and to OAuth 2.0 token requests with the provider's HTTP client. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * build: rebuild PHP 8.5.12 (build 2) Build 2 of PHP 8.5.12 builds curl with Schannel on Windows (upsun/cli-php-builds#4). No source archives changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(legacy): check an HTTPS proxy's revocation on a best-effort basis too curl reads the TLS options for an HTTPS proxy from CURLOPT_PROXY_SSL_OPTIONS, so with Schannel a proxy whose certificate chains to a root without revocation information (as many TLS inspection roots are) failed the strict default check. It now gets CURLSSLOPT_REVOKE_BEST_EFFORT as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Goes back to static-php-cli's default Schannel build of curl on Windows, and removes the patch that switched it to OpenSSL (#1).
With no CA file, Schannel verifies certificates with Windows itself: the certificate store (including roots installed by an organization), the disallowed certificate trust list, and revocation. curl on OpenSSL can only be given a copy of the store's roots, which misses Windows' distrust decisions.
The CLI needs a matching change to stop passing a CA file, which Schannel uses instead of the store. A release from this must not reach the CLI before that change.
🤖 Generated with Claude Code