feat(legacy): add the Windows certificate store to the CA bundle

.github/workflows/ci.yml

@@ -169,3 +169,9 @@ jobs:
         env:
           CLI_TEST_MODIFY_CERT_STORE: '1'
         run: GOEXPERIMENT=jsonv2 go test -v -count=1 -timeout 3m -run TestWindows ./internal/legacy/
+
+      # Reading the certificate store happens before every legacy command.
+      - name: Measure building the CA bundle
+        if: always()
+        shell: bash
+        run: GOEXPERIMENT=jsonv2 go test -run '^$' -bench BenchmarkWindows -benchmem ./internal/legacy/

internal/legacy/ca_bundle_windows.go

@@ -0,0 +1,142 @@
+package legacy
+
+import (
+	"bytes"
+	"crypto/sha256"
+	"crypto/x509"
+	"encoding/pem"
+	"errors"
+	"fmt"
+	"time"
+	"unsafe"
+
+	"golang.org/x/sys/windows"
+)
+
+// maxCAFileSize is the largest CA file curl will read on Windows, from
+// MAX_CAFILE_SIZE in its schannel_verify.c. A larger file fails every request.
+const maxCAFileSize = 1024 * 1024
+
+// caBundle returns the certificates the legacy CLI should trust: those shipped
+// with it, plus the roots installed locally on this machine.
+//
+// An organization which inspects TLS traffic installs its own root certificate
+// in the Windows store, and every other program on the machine then trusts it.
+// curl in the embedded PHP could read that store, but only while no CA file is
+// set, and one has to be set because the openssl extension cannot read the
+// store at all.
+//
+// Only the locally installed roots are added, not the whole store: Windows also
+// keeps the certificates it gets from Microsoft's root program there, which are
+// numerous enough to exceed the size curl will read. Leaving those out cannot
+// lose any trust the CLI had before, because before it trusted only the
+// certificates shipped with it.
+func caBundle() ([]byte, error) {
+	extra, err := localRootsPEM()
+	if err != nil {
+		return nil, err
+	}
+
+	bundle := make([]byte, 0, len(caCert)+len(extra)+1)
+	bundle = append(bundle, bytes.TrimRight(caCert, "\n")...)
+	bundle = append(bundle, '\n')
+	bundle = append(bundle, extra...)
+
+	if len(bundle) > maxCAFileSize {
+		// curl would refuse the file, so no request would work at all. The
+		// shipped certificates alone are what the CLI trusted before.
+		return caCert, nil
+	}
+	return bundle, nil
+}
+
+// localRootsPEM returns the roots this machine was told to trust, meaning those
+// in the store which are neither shipped with the CLI nor part of Microsoft's
+// root program.
+func localRootsPEM() ([]byte, error) {
+	shipped, err := certFingerprints(caCert)
+	if err != nil {
+		return nil, err
+	}
+	// AuthRoot holds Microsoft's root program, which the shipped certificates
+	// cover for the purposes of the CLI.
+	publicRoots, err := storeFingerprints("AuthRoot")
+	if err != nil {
+		return nil, err
+	}
+
+	var out bytes.Buffer
+	err = eachStoreCert("ROOT", func(der []byte) error {
+		fingerprint := sha256.Sum256(der)
+		if shipped[fingerprint] || publicRoots[fingerprint] {
+			return nil
+		}
+		cert, err := x509.ParseCertificate(der)
+		if err != nil {
+			return nil // Not a certificate this build of Go can read.
+		}
+		if now := time.Now(); now.Before(cert.NotBefore) || now.After(cert.NotAfter) {
+			return nil
+		}
+		return pem.Encode(&out, &pem.Block{Type: "CERTIFICATE", Bytes: der})
+	})
+	if err != nil {
+		return nil, err
+	}
+	return out.Bytes(), nil
+}
+
+// certFingerprints reads a PEM bundle and returns a fingerprint per certificate.
+func certFingerprints(bundle []byte) (map[[32]byte]bool, error) {
+	fingerprints := make(map[[32]byte]bool)
+	for rest := bundle; len(rest) > 0; {
+		var block *pem.Block
+		block, rest = pem.Decode(rest)
+		if block == nil {
+			break
+		}
+		if block.Type == "CERTIFICATE" {
+			fingerprints[sha256.Sum256(block.Bytes)] = true
+		}
+	}
+	if len(fingerprints) == 0 {
+		return nil, errors.New("no certificates found in the shipped CA bundle")
+	}
+	return fingerprints, nil
+}
+
+// storeFingerprints returns a fingerprint per certificate in a Windows store.
+func storeFingerprints(name string) (map[[32]byte]bool, error) {
+	fingerprints := make(map[[32]byte]bool)
+	err := eachStoreCert(name, func(der []byte) error {
+		fingerprints[sha256.Sum256(der)] = true
+		return nil
+	})
+	return fingerprints, err
+}
+
+// eachStoreCert calls fn with the encoded bytes of every certificate in a
+// Windows system store, as read by every program which verifies against it.
+func eachStoreCert(name string, fn func(der []byte) error) error {
+	store, err := windows.CertOpenSystemStore(0, windows.StringToUTF16Ptr(name))
+	if err != nil {
+		return fmt.Errorf("could not open the %s certificate store: %w", name, err)
+	}
+	defer windows.CertCloseStore(store, 0) //nolint:errcheck
+
+	var certContext *windows.CertContext
+	for {
+		certContext, err = windows.CertEnumCertificatesInStore(store, certContext)
+		if certContext == nil {
+			if err != nil && !errors.Is(err, windows.Errno(windows.CRYPT_E_NOT_FOUND)) {
+				return fmt.Errorf("could not read the %s certificate store: %w", name, err)
+			}
+			return nil
+		}
+		// The context belongs to the store, so the bytes are only borrowed.
+		if err := fn(unsafe.Slice(certContext.EncodedCert, certContext.Length)); err != nil {
+			windows.CertFreeCertificateContext(certContext) //nolint:errcheck
+			return err
+		}
+	}
+}

internal/legacy/ca_bundle_windows_test.go

@@ -0,0 +1,78 @@
+package legacy
+
+import (
+	"crypto/x509"
+	"encoding/pem"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestWindowsCABundle(t *testing.T) {
+	bundle, err := caBundle()
+	require.NoError(t, err)
+
+	shipped, err := certFingerprints(caCert)
+	require.NoError(t, err)
+	held, err := certFingerprints(bundle)
+	require.NoError(t, err)
+
+	// What the machine holds, to show how much of the store is left out, and
+	// why the whole store would not fit. This varies by machine, so it is
+	// reported rather than asserted.
+	root, err := storeFingerprints("ROOT")
+	require.NoError(t, err)
+	authRoot, err := storeFingerprints("AuthRoot")
+	require.NoError(t, err)
+	var inRootProgram int
+	for fingerprint := range root {
+		if authRoot[fingerprint] {
+			inRootProgram++
+		}
+	}
+	t.Logf("bundle: %d certificates in %d KB (%d shipped, %d added)",
+		len(held), len(bundle)/1024, len(shipped), len(held)-len(shipped))
+	t.Logf("store: %d roots, of which %d are in Microsoft's root program (AuthRoot holds %d)",
+		len(root), inRootProgram, len(authRoot))
+
+	assert.Less(t, len(bundle), maxCAFileSize, "expected curl to be willing to read the bundle")
+	for fingerprint := range shipped {
+		require.True(t, held[fingerprint], "expected every shipped certificate to still be trusted")
+	}
+	require.True(t, x509.NewCertPool().AppendCertsFromPEM(bundle),
+		"expected the bundle to be readable as a pool of trusted roots")
+	for rest := bundle; len(rest) > 0; {
+		var block *pem.Block
+		block, rest = pem.Decode(rest)
+		if block == nil {
+			require.Empty(t, rest, "expected the whole bundle to be readable")
+			break
+		}
+		_, err := x509.ParseCertificate(block.Bytes)
+		require.NoError(t, err, "expected every certificate in the bundle to be readable")
+	}
+}
+
+// BenchmarkWindowsCABundle measures reading the store and building the bundle,
+// which happens before every legacy command.
+func BenchmarkWindowsCABundle(b *testing.B) {
+	for b.Loop() {
+		if _, err := caBundle(); err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+// BenchmarkWindowsCAFile measures the whole of what a command pays: building
+// the bundle, and finding the cached file already up to date.
+func BenchmarkWindowsCAFile(b *testing.B) {
+	manager := &phpManagerPerOS{b.TempDir()}
+	require.NoError(b, manager.writeCAFile())
+
+	for b.Loop() {
+		if err := manager.writeCAFile(); err != nil {
+			b.Fatal(err)
+		}
+	}
+}

internal/legacy/cert_store_windows_test.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"crypto/rand"
 	"crypto/rsa"
+	"crypto/sha256"
 	"crypto/tls"
 	"crypto/x509"
 	"crypto/x509/pkix"
@@ -71,23 +72,16 @@ func TestWindowsCertStoreTrust(t *testing.T) {
 
 	installCAInRootStore(t, ca.certDER)
 
-	// Lay out the PHP binary and its bundled CA file the way the CLI does.
+	// Lay out the PHP binary and its CA file the way the CLI does. The CA file
+	// is written after the certificate is installed, so it includes it.
 	cacheDir := t.TempDir()
 	manager := newPHPManager(cacheDir)
 	require.NoError(t, manager.copy())
 	phpBin := manager.binPath()
-	bundledCAFile := filepath.Join(cacheDir, "cacert.pem")
-	require.FileExists(t, bundledCAFile)
-
-	// A bundle holding both the shipped certificates and the one from the
-	// store: what the wrapper would have to generate to keep organization
-	// certificates working.
-	mergedCAFile := filepath.Join(cacheDir, "merged.pem")
-	bundled, err := os.ReadFile(bundledCAFile)
-	require.NoError(t, err)
-	merged := append(bytes.TrimRight(bundled, "\n"), '\n')
-	merged = append(merged, ca.certPEM...)
-	require.NoError(t, os.WriteFile(mergedCAFile, merged, 0o600))
+
+	// The certificates shipped with the CLI, without those from the store.
+	shippedCAFile := filepath.Join(cacheDir, "shipped.pem")
+	require.NoError(t, os.WriteFile(shippedCAFile, caCert, 0o600))
 
 	cases := []struct {
 		name      string
@@ -100,17 +94,16 @@ func TestWindowsCertStoreTrust(t *testing.T) {
 			wantTrust: true,
 		},
 		{
-			// The wrapper passes this setting today, which is why an
-			// organization's certificate is not seen. Expect this
-			// expectation to change along with that.
-			name:      "a CA file is used instead of the store",
+			name:      "the settings the wrapper passes",
 			args:      settingArgs(manager.settings()),
-			wantTrust: false,
+			wantTrust: true,
 		},
 		{
-			name:      "a CA file which includes the store's certificate",
-			args:      []string{"-d", iniSetting("openssl.cafile", mergedCAFile)},
-			wantTrust: true,
+			// Setting a CA file stops curl reading the store, so this is what
+			// the wrapper used to do, and what issue #110 reported.
+			name:      "only the shipped certificates",
+			args:      []string{"-d", iniSetting("openssl.cafile", shippedCAFile)},
+			wantTrust: false,
 		},
 	}
 
@@ -126,9 +119,15 @@ func TestWindowsCertStoreTrust(t *testing.T) {
 		})
 	}
 
-	t.Run("the root store can be read from Go", func(t *testing.T) {
-		assert.True(t, rootStoreContains(t, ca.certDER),
-			"expected to find the installed certificate by enumerating the ROOT store")
+	t.Run("the installed certificate is one of the locally trusted roots", func(t *testing.T) {
+		// It is not in Microsoft's root program, so it must be picked up.
+		local, err := localRootsPEM()
+		require.NoError(t, err)
+		fingerprints, err := certFingerprints(local)
+		require.NoError(t, err)
+		assert.True(t, fingerprints[sha256.Sum256(ca.certDER)],
+			"expected the installed certificate to be added to the bundle")
+		t.Logf("%d locally trusted roots in %d bytes", len(fingerprints), len(local))
 	})
 }
 
@@ -335,18 +334,3 @@ func runPHPRequest(t *testing.T, phpBin, url string, extraArgs ...string) string
 
 	return strings.TrimSpace(string(output))
 }
-
-// rootStoreContains reports whether the trusted roots hold a certificate,
-// using the same API a merged CA bundle would need to read them.
-func rootStoreContains(t *testing.T, certDER []byte) bool {
-	t.Helper()
-
-	store := openRootStore(t)
-	defer func() {
-		assert.NoError(t, windows.CertCloseStore(store, 0))
-	}()
-
-	return eachCertInStore(t, store, func(_ *windows.CertContext, encoded []byte) bool {
-		return bytes.Equal(encoded, certDER)
-	})
-}

internal/legacy/php_manager_windows.go

@@ -18,17 +18,30 @@ func (m *phpManagerPerOS) copy() error {
 	if err := file.WriteIfNeeded(m.binPath(), phpCLI, 0o755); err != nil {
 		return err
 	}
-	// Write cacert.pem for OpenSSL CA bundle (Windows needs this explicitly).
-	return file.WriteIfNeeded(filepath.Join(m.cacheDir, "cacert.pem"), caCert, 0o644)
+	return m.writeCAFile()
 }
 
 func (m *phpManagerPerOS) binPath() string {
 	return filepath.Join(m.cacheDir, "php.exe")
 }
 
+// writeCAFile writes the CA bundle, which Windows needs to be given
+// explicitly, and which depends on the machine's certificate store.
+func (m *phpManagerPerOS) writeCAFile() error {
+	bundle, err := caBundle()
+	if err != nil {
+		return err
+	}
+	return file.WriteIfNeeded(m.caFilePath(), bundle, 0o644)
+}
+
+func (m *phpManagerPerOS) caFilePath() string {
+	return filepath.Join(m.cacheDir, "cacert.pem")
+}
+
 func (m *phpManagerPerOS) settings() []string {
 	return []string{
-		iniSetting("openssl.cafile", filepath.Join(m.cacheDir, "cacert.pem")),
+		iniSetting("openssl.cafile", m.caFilePath()),
 	}
 }