ipn/ipnlocal: fix the path for writing cert files (#7203)

Fixes #7202.

Change-Id: I1f8e9c59d5e42e7df7a3fbbd82ae2b4293845916
Signed-off-by: M. J. Fromberger <[email protected]>

ipn/ipnlocal/cert.go

@@ -146,46 +146,56 @@ var errCertExpired = errors.New("cert expired")
 
 func (b *LocalBackend) getCertStore(dir string) certStore {
 	if hostinfo.GetEnvType() == hostinfo.Kubernetes && dir == "/tmp" {
-		return certStateStore{b.store}
+		return certStateStore{StateStore: b.store}
 	}
-	return certFileStore(dir)
+	return certFileStore{dir: dir}
 }
 
 // certFileStore implements certStore by storing the cert & key files in the named directory.
-type certFileStore string // dir
+type certFileStore struct {
+	dir string
+
+	// This field allows a test to override the CA root(s) for certificate
+	// verification. If nil the default system pool is used.
+	testRoots *x509.CertPool
+}
 
 func (f certFileStore) Read(domain string, now time.Time) (*TLSCertKeyPair, error) {
-	certPEM, err := os.ReadFile(keyFile(string(f), domain))
+	certPEM, err := os.ReadFile(certFile(f.dir, domain))
 	if err != nil {
 		if os.IsNotExist(err) {
 			return nil, ipn.ErrStateNotExist
 		}
 		return nil, err
 	}
-	keyPEM, err := os.ReadFile(certFile(string(f), domain))
+	keyPEM, err := os.ReadFile(keyFile(f.dir, domain))
 	if err != nil {
 		if os.IsNotExist(err) {
 			return nil, ipn.ErrStateNotExist
 		}
 		return nil, err
 	}
-	if !validCertPEM(domain, keyPEM, certPEM, now) {
+	if !validCertPEM(domain, keyPEM, certPEM, f.testRoots, now) {
 		return nil, errCertExpired
 	}
 	return &TLSCertKeyPair{CertPEM: certPEM, KeyPEM: keyPEM, Cached: true}, nil
 }
 
 func (f certFileStore) WriteCert(domain string, cert []byte) error {
-	return os.WriteFile(keyFile(string(f), domain), cert, 0644)
+	return os.WriteFile(certFile(f.dir, domain), cert, 0644)
 }
 
 func (f certFileStore) WriteKey(domain string, key []byte) error {
-	return os.WriteFile(keyFile(string(f), domain), key, 0600)
+	return os.WriteFile(keyFile(f.dir, domain), key, 0600)
 }
 
 // certStateStore implements certStore by storing the cert & key files in an ipn.StateStore.
 type certStateStore struct {
 	ipn.StateStore
+
+	// This field allows a test to override the CA root(s) for certificate
+	// verification. If nil the default system pool is used.
+	testRoots *x509.CertPool
 }
 
 func (s certStateStore) Read(domain string, now time.Time) (*TLSCertKeyPair, error) {
@@ -197,7 +207,7 @@ func (s certStateStore) Read(domain string, now time.Time) (*TLSCertKeyPair, err
 	if err != nil {
 		return nil, err
 	}
-	if !validCertPEM(domain, keyPEM, certPEM, now) {
+	if !validCertPEM(domain, keyPEM, certPEM, s.testRoots, now) {
 		return nil, errCertExpired
 	}
 	return &TLSCertKeyPair{CertPEM: certPEM, KeyPEM: keyPEM, Cached: true}, nil
@@ -458,7 +468,11 @@ func acmeKey(dir string) (crypto.Signer, error) {
 	return privKey, nil
 }
 
-func validCertPEM(domain string, keyPEM, certPEM []byte, now time.Time) bool {
+// validCertPEM reports whether the given certificate is valid for domain at now.
+//
+// If roots != nil, it is used instead of the system root pool. This is meant
+// to support testing, and production code should pass roots == nil.
+func validCertPEM(domain string, keyPEM, certPEM []byte, roots *x509.CertPool, now time.Time) bool {
 	if len(keyPEM) == 0 || len(certPEM) == 0 {
 		return false
 	}
@@ -466,6 +480,7 @@ func validCertPEM(domain string, keyPEM, certPEM []byte, now time.Time) bool {
 	if err != nil {
 		return false
 	}
+
 	var leaf *x509.Certificate
 	intermediates := x509.NewCertPool()
 	for i, certDER := range tlsCert.Certificate {
@@ -485,6 +500,7 @@ func validCertPEM(domain string, keyPEM, certPEM []byte, now time.Time) bool {
 	_, err = leaf.Verify(x509.VerifyOptions{
 		DNSName:       domain,
 		CurrentTime:   now,
+		Roots:         roots,
 		Intermediates: intermediates,
 	})
 	return err == nil

ipn/ipnlocal/cert_test.go

@@ -5,7 +5,15 @@
 
 package ipnlocal
 
-import "testing"
+import (
+	"crypto/x509"
+	"embed"
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"tailscale.com/ipn/store/mem"
+)
 
 func TestValidLookingCertDomain(t *testing.T) {
 	tests := []struct {
@@ -26,3 +34,69 @@ func TestValidLookingCertDomain(t *testing.T) {
 		}
 	}
 }
+
+//go:embed testdata/*
+var certTestFS embed.FS
+
+func TestCertStoreRoundTrip(t *testing.T) {
+	const testDomain = "example.com"
+
+	// Use a fixed verification timestamp so validity doesn't fall off when the
+	// cert expires. If you update the test data below, this may also need to be
+	// updated.
+	testNow := time.Date(2023, time.February, 10, 0, 0, 0, 0, time.UTC)
+
+	// To re-generate a root certificate and domain certificate for testing,
+	// use:
+	//
+	//   	go run filippo.io/mkcert@latest example.com
+	//
+	// The content is not important except to be structurally valid so we can be
+	// sure the round-trip succeeds.
+	testRoot, err := certTestFS.ReadFile("testdata/rootCA.pem")
+	if err != nil {
+		t.Fatal(err)
+	}
+	roots := x509.NewCertPool()
+	if !roots.AppendCertsFromPEM(testRoot) {
+		t.Fatal("Unable to add test CA to the cert pool")
+	}
+
+	testCert, err := certTestFS.ReadFile("testdata/example.com.pem")
+	if err != nil {
+		t.Fatal(err)
+	}
+	testKey, err := certTestFS.ReadFile("testdata/example.com-key.pem")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tests := []struct {
+		name  string
+		store certStore
+	}{
+		{"FileStore", certFileStore{dir: t.TempDir(), testRoots: roots}},
+		{"StateStore", certStateStore{StateStore: new(mem.Store), testRoots: roots}},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if err := test.store.WriteCert(testDomain, testCert); err != nil {
+				t.Fatalf("WriteCert: unexpected error: %v", err)
+			}
+			if err := test.store.WriteKey(testDomain, testKey); err != nil {
+				t.Fatalf("WriteKey: unexpected error: %v", err)
+			}
+
+			kp, err := test.store.Read(testDomain, testNow)
+			if err != nil {
+				t.Fatalf("Read: unexpected error: %v", err)
+			}
+			if diff := cmp.Diff(kp.CertPEM, testCert); diff != "" {
+				t.Errorf("Certificate (-got, +want):\n%s", diff)
+			}
+			if diff := cmp.Diff(kp.KeyPEM, testKey); diff != "" {
+				t.Errorf("Key (-got, +want):\n%s", diff)
+			}
+		})
+	}
+}

ipn/ipnlocal/testdata/example.com-key.pem

@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCejQaJrntrJSgE
+QtScyTU6TXOU+v1FdFjrsyHFK5mjV1C5pVQxnLn93GRshtIrGOLLrd3Wv2TVYZOX
+xH7f1ZLFbneDURCXbS+7nmsg+TLHRSRKfODbE3oYZj7NSJ163CCvwSJKTdmLpXbn
+ui9F04tyk0zxO4Wre4ukwf6xtse8G5zl2RJrueiVAiouTG/pJdIS08dGQa0GM1n9
+Aesa+TerlZcpRZR6X402yQqa8q/QqbIuzrlfDmgOb8sm6T8+JMtj3hEvnYdpMVOg
+w/XiTlX0v/YrB9sVQ9XnqGsqwTL0OMG0choMNKipwLi2n+XPSCIiRhi666zNNivE
+K1qaPS5RAgMBAAECggEAV9dAGQWPISR70CiKjLa5A60nbRHFQjackTE0c32daC6W
+7dOYGsh/DxOMm8fyJqhp9nhEYJa3MbUWxU27ER3NbA6wrhM6gvqeKG8zYRhPNrGq
+0o3vMdDPozb6cldZ0Fimz1jMO6h373NjtiyjxibWqkrLpRbaDtCq5EQKbMEcVa2D
+Xt5hxCOaCA3OZ/mAcGUNFmDNgNsGP/r6eXdI5pbqnUNMPkv/JsHl8h2HuyKUm4hf
+TRnXPAak6DkUod9QXYFKVBVPa5pjiO09e0aiMUvJ8vYd/6bNIsAKWLPa1PYuUE2l
+kg8Nik+P/XLzffKsLxiFKY0nCqrorM9K5q7baofGdQKBgQDPujjebFg6OKw6MS3S
+PESopvL//C/XgtgifcSSZCWzIZRVBVTbbJCGRtqFzF0XO4YRX3EOAyD/L7wYUPzO
++W3AU2W3/DVJYdcm2CASABbHNy0kk52LI0HHAssbFDgyB9XuuWP+vVZk7B5OmCAD
+Bppuj6Mnu03i282nKNJzvRiVnwKBgQDDZUXv22K8y7GkKw/ZW/wQP2zBNtFc15he
+1EOyUGHlXuQixnDSaqonkwec6IOlo7Sx/vwO/7+v4Jzc24Wq3DFAmMu/EYJgvI+m
+m3kpB4H7Xus4JqnhxqN7GB7zOdguCWZF1HLemZNZlVrUjG5mQ9cizzvvYptnQDLq
+FEJ1hddWDwKBgB+vy276Xfb7oCH8UH4KXXrQhK7RvEaGmgug3bRq/Gk3zRWvC4Ox
+KtagxkK0qtqZZNkPkwJNLeJfWLTo3beAyuIUlqabHVHFT/mH7FRymQbofsVekyCf
+TzBZV7wYuH3BPjv9IajBHwWkEvdwMyni/vmwhXXRF49schF2o6uuA6sHAoGBAL1J
+Xnb+EKjUq0JedPwcIBOdXb3PXQKT2QgEmZAkTrHlOxx1INa2fh/YT4ext9a+wE2u
+tn/RQeEfttY90z+yEASEAN0YGTWddYvxEW6t1z2stjGvQuN1ium0dEcrwkDW2jzL
+knwSSqx+A3/kiw6GqeMO3wEIhYOArdIVzkwLXJABAoGAOXLGhz5u5FWjF3zAeYme
+uHTU/3Z3jeI80PvShGrgAakPOBt3cIFpUaiOEslcqqgDUSGE3EnmkRqaEch+UapF
+ty6Zz7cKjXhQSWOjew1uUW2ANNEpsnYbmZOOnfvosd7jfHSVbL6KIhWmIdC6h0NP
+c/bJnTXEEVsWjLZTwYaq0Us=
+-----END PRIVATE KEY-----

ipn/ipnlocal/testdata/example.com.pem

@@ -0,0 +1,26 @@
+-----BEGIN CERTIFICATE-----
+MIIEcDCCAtigAwIBAgIRAPmUKRkyFAkVVxFblB/233cwDQYJKoZIhvcNAQELBQAw
+gZ8xHjAcBgNVBAoTFW1rY2VydCBkZXZlbG9wbWVudCBDQTE6MDgGA1UECwwxZnJv
+bWJlcmdlckBzdGFyZHVzdC5sb2NhbCAoTWljaGFlbCBKLiBGcm9tYmVyZ2VyKTFB
+MD8GA1UEAww4bWtjZXJ0IGZyb21iZXJnZXJAc3RhcmR1c3QubG9jYWwgKE1pY2hh
+ZWwgSi4gRnJvbWJlcmdlcikwHhcNMjMwMjA3MjAzNDE4WhcNMjUwNTA3MTkzNDE4
+WjBlMScwJQYDVQQKEx5ta2NlcnQgZGV2ZWxvcG1lbnQgY2VydGlmaWNhdGUxOjA4
+BgNVBAsMMWZyb21iZXJnZXJAc3RhcmR1c3QubG9jYWwgKE1pY2hhZWwgSi4gRnJv
+bWJlcmdlcikwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCejQaJrntr
+JSgEQtScyTU6TXOU+v1FdFjrsyHFK5mjV1C5pVQxnLn93GRshtIrGOLLrd3Wv2TV
+YZOXxH7f1ZLFbneDURCXbS+7nmsg+TLHRSRKfODbE3oYZj7NSJ163CCvwSJKTdmL
+pXbnui9F04tyk0zxO4Wre4ukwf6xtse8G5zl2RJrueiVAiouTG/pJdIS08dGQa0G
+M1n9Aesa+TerlZcpRZR6X402yQqa8q/QqbIuzrlfDmgOb8sm6T8+JMtj3hEvnYdp
+MVOgw/XiTlX0v/YrB9sVQ9XnqGsqwTL0OMG0choMNKipwLi2n+XPSCIiRhi666zN
+NivEK1qaPS5RAgMBAAGjYDBeMA4GA1UdDwEB/wQEAwIFoDATBgNVHSUEDDAKBggr
+BgEFBQcDATAfBgNVHSMEGDAWgBTXyq2jQVrnqQKL8fB9C4L0QJftwDAWBgNVHREE
+DzANggtleGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAYEAQWzpOaBkRR4M+WqB
+CsT4ARyM6WpZ+jpeSblCzPdlDRW+50G1HV7K930zayq4DwncPY/SqSn0Q31WuzZv
+bTWHkWa+MLPGYANHsusOmMR8Eh16G4+5+GGf8psWa0npAYO35cuNkyyCCc1LEB4M
+NrzCB2+KZ+SyOdfCCA5VzEKN3I8wvVLaYovi24Zjwv+0uETG92TlZmLQRhj8uPxN
+deeLM45aBkQZSYCbGMDVDK/XYKBkNLn3kxD/eZeXxxr41v4pH44+46FkYcYJzdn8
+ccAg5LRGieqTozhLiXARNK1vTy6kR1l/Az8DIx6GN4sP2/LMFYFijiiOCDKS1wWA
+xQgZeHt4GIuBym+Kd+Z5KXcP0AT+47Cby3+B10Kq8vHwjTELiF0UFeEYYMdynPAW
+pbEwVLhsfMsBqFtj3dsxHr8Kz3rnarOYzkaw7EMZnLAthb2CN7y5uGV9imQC5RMI
+/qZdRSuCYZ3A1E/WJkGbPY/YdPql/IE+LIAgKGFHZZNftBCo
+-----END CERTIFICATE-----

ipn/ipnlocal/testdata/rootCA.pem

@@ -0,0 +1,30 @@
+-----BEGIN CERTIFICATE-----
+MIIFEDCCA3igAwIBAgIRANf5NdPojIfj70wMfJVYUg8wDQYJKoZIhvcNAQELBQAw
+gZ8xHjAcBgNVBAoTFW1rY2VydCBkZXZlbG9wbWVudCBDQTE6MDgGA1UECwwxZnJv
+bWJlcmdlckBzdGFyZHVzdC5sb2NhbCAoTWljaGFlbCBKLiBGcm9tYmVyZ2VyKTFB
+MD8GA1UEAww4bWtjZXJ0IGZyb21iZXJnZXJAc3RhcmR1c3QubG9jYWwgKE1pY2hh
+ZWwgSi4gRnJvbWJlcmdlcikwHhcNMjMwMjA3MjAzNDE4WhcNMzMwMjA3MjAzNDE4
+WjCBnzEeMBwGA1UEChMVbWtjZXJ0IGRldmVsb3BtZW50IENBMTowOAYDVQQLDDFm
+cm9tYmVyZ2VyQHN0YXJkdXN0LmxvY2FsIChNaWNoYWVsIEouIEZyb21iZXJnZXIp
+MUEwPwYDVQQDDDhta2NlcnQgZnJvbWJlcmdlckBzdGFyZHVzdC5sb2NhbCAoTWlj
+aGFlbCBKLiBGcm9tYmVyZ2VyKTCCAaIwDQYJKoZIhvcNAQEBBQADggGPADCCAYoC
+ggGBAL5uXNnrZ6dgjcvK0Hc7ZNUIRYEWst9qbO0P9H7le08pJ6d9T2BUWruZtVjk
+Q12msv5/bVWHhVk8dZclI9FLXuMsIrocH8bsoP4wruPMyRyp6EedSKODN51fFSRv
+/jHbS5vzUVAWTYy9qYmd6qL0uhsHCZCCT6gfigamHPUFKM3sHDn5ZHWvySMwcyGl
+AicmPAIkBWqiCZAkB5+WM7+oyRLjmrIalfWIZYxW/rojGLwTfneHv6J5WjVQnpJB
+ayWCzCzaiXukK9MeBWeTOe8UfVN0Engd74/rjLWvjbfC+uZSr6RVkZvs2jANLwPF
+zgzBPHgRPfAhszU1NNAMjnNQ47+OMOTKRt7e6jYzhO5fyO1qVAAvGBqcfpj+JfDk
+cccaUMhUvdiGrhGf1V1tN/PislxvALirzcFipjD01isBKwn0fxRugzvJNrjEo8RA
+RvbcdeKcwex7M0o/Cd0+G2B13gZNOFvR33PmG7iTpp7IUrUKfQg28I83Sp8tMY3s
+ljJSawIDAQABo0UwQzAOBgNVHQ8BAf8EBAMCAgQwEgYDVR0TAQH/BAgwBgEB/wIB
+ADAdBgNVHQ4EFgQU18qto0Fa56kCi/HwfQuC9ECX7cAwDQYJKoZIhvcNAQELBQAD
+ggGBAAzs96LwZVOsRSlBdQqMo8oMAvs7HgnYbXt8SqaACLX3+kJ3cV/vrCE3iJrW
+ma4CiQbxS/HqsiZjota5m4lYeEevRnUDpXhp+7ugZTiz33Flm1RU99c9UYfQ+919
+ANPAKeqNpoPco/HF5Bz0ocepjcfKQrVZZNTj6noLs8o12FHBLO5976AcF9mqlNfh
+8/F0gDJXq6+x7VT5y8u0rY004XKPRe3CklRt8kpeMiP6mhRyyUehOaHeIbNx8ubi
+Pi44ByN/ueAnuRhF9zYtyZVZZOaSLysJge01tuPXF8rBXGruoJIv35xTTBa9BzaP
+YDOGbGn1ZnajdNagHqCba8vjTLDSpqMvgRj3TFrGHdETA2LDQat38uVxX8gxm68K
+va5Tyv7n+6BQ5YTpJjTPnmSJKaXZrrhdLPvG0OU2TxeEsvbcm5LFQofirOOw86Se
+vzF2cQ94mmHRZiEk0Av3NO0jF93ELDrBCuiccVyEKq6TknuvPQlutCXKDOYSEb8I
+MHctBg==
+-----END CERTIFICATE-----