diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1146170d..e1d4daf5 100644 --- a/.github/workflows/ci.yml +++ b/.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/ diff --git a/internal/legacy/ca_bundle_windows.go b/internal/legacy/ca_bundle_windows.go new file mode 100644 index 00000000..8043a547 --- /dev/null +++ b/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 + } + } +} diff --git a/internal/legacy/ca_bundle_windows_test.go b/internal/legacy/ca_bundle_windows_test.go new file mode 100644 index 00000000..b07cdd41 --- /dev/null +++ b/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) + } + } +} diff --git a/internal/legacy/cert_store_windows_test.go b/internal/legacy/cert_store_windows_test.go index 499b9660..548083c0 100644 --- a/internal/legacy/cert_store_windows_test.go +++ b/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) - }) -} diff --git a/internal/legacy/php_manager_windows.go b/internal/legacy/php_manager_windows.go index e1afb8df..4ade4d72 100644 --- a/internal/legacy/php_manager_windows.go +++ b/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()), } }