Skip to content

feat(windows): build curl with Schannel again - #4

Merged
pjcdawkins merged 2 commits into
mainfrom
cli-222-windows-schannel
Oct 21, 2026
Merged

pjcdawkins merged 2 commits into
mainfrom
cli-222-windows-schannel

Conversation

@pjcdawkins

Copy link
Copy Markdown
Contributor

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

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.

@upsun-dispatch upsun-dispatch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 no patch-spc match.
  • The backend check compares curl_version()['ssl_version'] with the literal Schannel using 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, download and build still 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') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread README.md Outdated
Comment thread .github/workflows/build-php.yml
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>

@upsun-dispatch upsun-dispatch Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 details

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

@pjcdawkins
pjcdawkins merged commit db55e4d into main Oct 21, 2026
8 checks passed
@pjcdawkins
pjcdawkins deleted the cli-222-windows-schannel branch October 21, 2026 16:15
pjcdawkins added a commit to upsun/cli that referenced this pull request Oct 21, 2026
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>
pjcdawkins added a commit to upsun/cli that referenced this pull request Oct 21, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant