From 15289b0e8b1a9d2cb0a7380ee5d25d81a4fda6f7 Mon Sep 17 00:00:00 2001 From: eaudetcobello <155978570+eaudetcobello@users.noreply.github.com> Date: Tue, 26 Mar 2024 11:46:57 -0400 Subject: [PATCH] Specify list of ExtraSANs for the kube-apiserver certs (#263) * Initial implementation * Remove unused field * Add tests and move long string into file * Review comments * Extend test suite * Rename test * Test Kubelet cert SAN for good measure * Remove dead code and add test for SeparateSANs * Clearer function name * Update test * Embed test CA file * Types * Change value in test * Fix types * Review comments * Revert hooks_join.go * Omit empty field * Gomega idiom * Review comments * Remove redundant length check * Review comments * Use net.ParseIP instead of string * Revert change --- src/k8s/api/v1/types.go | 15 +-- src/k8s/pkg/k8sd/app/hooks_bootstrap.go | 7 +- src/k8s/pkg/k8sd/pki/control_plane_test.go | 109 ++++++++++++++++----- src/k8s/pkg/k8sd/pki/testdata/ca.pem | 22 +++++ src/k8s/pkg/utils/certificate.go | 28 ++++++ src/k8s/pkg/utils/certificate_test.go | 26 +++++ 6 files changed, 173 insertions(+), 34 deletions(-) create mode 100644 src/k8s/pkg/k8sd/pki/testdata/ca.pem create mode 100644 src/k8s/pkg/utils/certificate.go create mode 100644 src/k8s/pkg/utils/certificate_test.go diff --git a/src/k8s/api/v1/types.go b/src/k8s/api/v1/types.go index 7fde7a440..b626453e5 100644 --- a/src/k8s/api/v1/types.go +++ b/src/k8s/api/v1/types.go @@ -16,13 +16,14 @@ type BootstrapConfig struct { // ServiceCIDR is the CIDR of the cluster services. ServiceCIDR string `yaml:"service-cidr"` // EnableRBAC determines if RBAC will be enabled; *bool to know true/false/unset. - EnableRBAC *bool `yaml:"enable-rbac"` - K8sDqlitePort int `yaml:"k8s-dqlite-port"` - Datastore string `yaml:"datastore"` - DatastoreURL string `yaml:"datastore-url,omitempty"` - DatastoreCACert string `yaml:"datastore-ca-crt,omitempty"` - DatastoreClientCert string `yaml:"datastore-client-crt,omitempty"` - DatastoreClientKey string `yaml:"datastore-client-key,omitempty"` + EnableRBAC *bool `yaml:"enable-rbac"` + K8sDqlitePort int `yaml:"k8s-dqlite-port"` + Datastore string `yaml:"datastore"` + DatastoreURL string `yaml:"datastore-url,omitempty"` + DatastoreCACert string `yaml:"datastore-ca-crt,omitempty"` + DatastoreClientCert string `yaml:"datastore-client-crt,omitempty"` + DatastoreClientKey string `yaml:"datastore-client-key,omitempty"` + ExtraSANs []string `yaml:"extrasans,omitempty"` } // SetDefaults sets the fields to default values. diff --git a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go index 21857fbf7..38e5acdf3 100644 --- a/src/k8s/pkg/k8sd/app/hooks_bootstrap.go +++ b/src/k8s/pkg/k8sd/app/hooks_bootstrap.go @@ -212,10 +212,15 @@ func onBootstrapControlPlane(s *state.State, initConfig map[string]string) error return fmt.Errorf("unsupported datastore %s, must be one of %v", cfg.APIServer.Datastore, setup.SupportedDatastores) } + extraIPs, extraNames := utils.SplitIPAndDNSSANs(bootstrapConfig.ExtraSANs) + + IPSANs := append(append([]net.IP{nodeIP}, serviceIPs...), extraIPs...) + // Certificates certificates := pki.NewControlPlanePKI(pki.ControlPlanePKIOpts{ Hostname: s.Name(), - IPSANs: append([]net.IP{nodeIP}, serviceIPs...), + IPSANs: IPSANs, + DNSSANs: extraNames, Years: 20, AllowSelfSignedCA: true, IncludeMachineAddressSANs: true, diff --git a/src/k8s/pkg/k8sd/pki/control_plane_test.go b/src/k8s/pkg/k8sd/pki/control_plane_test.go index 4b24a0564..7945b23c2 100644 --- a/src/k8s/pkg/k8sd/pki/control_plane_test.go +++ b/src/k8s/pkg/k8sd/pki/control_plane_test.go @@ -1,12 +1,24 @@ package pki_test import ( + "crypto/x509" + "encoding/pem" + "net" + "os" "testing" "github.com/canonical/k8s/pkg/k8sd/pki" . "github.com/onsi/gomega" ) +func mustReadTestData(t *testing.T, filename string) string { + data, err := os.ReadFile("testdata/" + filename) + if err != nil { + t.Fatal(err) + } + return string(data) +} + func TestControlPlaneCertificates(t *testing.T) { c := pki.NewControlPlanePKI(pki.ControlPlanePKIOpts{ Hostname: "h1", @@ -16,39 +28,84 @@ func TestControlPlaneCertificates(t *testing.T) { g := NewWithT(t) - g.Expect(c.CompleteCertificates()).To(BeNil()) - g.Expect(c.CompleteCertificates()).To(BeNil()) + g.Expect(c.CompleteCertificates()).To(Succeed()) + g.Expect(c.CompleteCertificates()).To(Succeed()) t.Run("MissingCAKey", func(t *testing.T) { c := pki.NewControlPlanePKI(pki.ControlPlanePKIOpts{ Hostname: "h1", Years: 10, }) - c.CACert = ` ------BEGIN CERTIFICATE----- -MIIDtTCCAp2gAwIBAgIQOPOTOjxvIVlC5ev8EzrnITANBgkqhkiG9w0BAQsFADAY -MRYwFAYDVQQDEw1rdWJlcm5ldGVzLWNhMB4XDTI0MDIwODAyNDYyOVoXDTM0MDIw -ODAyNDYyOVowGTEXMBUGA1UEAxMOa3ViZS1hcGlzZXJ2ZXIwggEiMA0GCSqGSIb3 -DQEBAQUAA4IBDwAwggEKAoIBAQCum3KkohfK+E4KCpauilnlxm0e6y+jzyOaRCHx -P/3iLqN5zN+s2SV+GJNNcT3vSVZ1YhcJKWNrs7QxK2qcq9OhHncmp9Vqu5BV9O+e -ys4bBlf08lHH0//wrAwXy71ueWXN2uWyFg4i2VSirbRxpXGIR751i4qVtutbSOPy -3Jjf07upq3zAMyvTx1YTZcwduwW2vrU1f48IZOTueS1eOz0YjCkWLueD2uhLLgRA -mcxq33pwTM9P0MaZGrrM2GeA+1Hyss5WtoEMkR6TPUWQmYcKFEZui9/JpLfbM8yu -6h6Ta7GeSccjtclHSGp9fge0IXErhYSmLNoQ7JP8fQeg0DpTAgMBAAGjgfkwgfYw -DgYDVR0PAQH/BAQDAgSwMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATAM -BgNVHRMBAf8EAjAAMB8GA1UdIwQYMBaAFJjD6HMwGRJQMOzNm919/ZaqdcUwMIGV -BgNVHREEgY0wgYqCCmt1YmVybmV0ZXOCEmt1YmVybmV0ZXMuZGVmYXVsdIIWa3Vi -ZXJuZXRlcy5kZWZhdWx0LnN2Y4Iea3ViZXJuZXRlcy5kZWZhdWx0LnN2Yy5jbHVz -dGVygiRrdWJlcm5ldGVzLmRlZmF1bHQuc3ZjLmNsdXN0ZXIubG9jYWyHBAqYtwGH -BH8AAAEwDQYJKoZIhvcNAQELBQADggEBADPWn//rPb0SmZ49WhIa6wc39Ryl7eGo -Q2H+RY9BMah/xge6fLgeTvFe+H6Vol9BVqm5XgD0BuD5yzKYI2aDq8Ikm4EMOxPl -7Gs9cqWMMF7Iiw+rYJY4vwzm+5kSCg6oxBx8GLYYkDpbFe8UAWKf/9QTghtoBEEw -JVBDECnQwJU4tb9ANmPbgxmCYLZjx2vmXQRlXpe6QS9nPmMSS9KkJMyLEEpgzIIA -aSprnA8WIeSaO/5wLMYS1lUWWzegz2LnKuJ5C5Q+XYkwIY/vFH7OSTnmvt+rHwhh -4Oj+ScJ0RKnGGcXQnctSvMogDoucw7Y2RjxKcJV8fEKV5ZIeTz0U+nE= ------END CERTIFICATE-----` + + c.CACert = mustReadTestData(t, "ca.pem") + + g := NewWithT(t) + g.Expect(c.CompleteCertificates()).ToNot(Succeed()) + }) + + t.Run("ApiServerCertSANs", func(t *testing.T) { + c := pki.NewControlPlanePKI(pki.ControlPlanePKIOpts{ + Hostname: "h1", + Years: 10, + AllowSelfSignedCA: true, + IPSANs: []net.IP{net.ParseIP("192.168.2.123")}, + DNSSANs: []string{"cluster.local"}, + }) g := NewWithT(t) - g.Expect(c.CompleteCertificates()).ToNot(BeNil()) + g.Expect(c.CompleteCertificates()).To(Succeed()) + + block, _ := pem.Decode([]byte(c.APIServerCert)) + g.Expect(block).ToNot(BeNil()) + + cert, _ := x509.ParseCertificate(block.Bytes) + g.Expect(cert).ToNot(BeNil()) + + t.Run("IPAddresses", func(t *testing.T) { + g := NewWithT(t) + expectedIPs := []net.IP{net.ParseIP("192.168.2.123").To4(), net.ParseIP("127.0.0.1").To4(), net.ParseIP("::1")} + + g.Expect(cert.IPAddresses).To(ConsistOf(expectedIPs)) + }) + + t.Run("DNSNames", func(t *testing.T) { + g := NewWithT(t) + expectedDNSNames := []string{"cluster.local", "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster", "kubernetes.default.svc.cluster.local"} + + g.Expect(cert.DNSNames).To(ConsistOf(expectedDNSNames)) + }) + }) + + t.Run("KubeletCertSANs", func(t *testing.T) { + c := pki.NewControlPlanePKI(pki.ControlPlanePKIOpts{ + Hostname: "h1", + Years: 10, + AllowSelfSignedCA: true, + IPSANs: []net.IP{net.ParseIP("192.168.2.123")}, + DNSSANs: []string{"cluster.local"}, + }) + + g := NewWithT(t) + g.Expect(c.CompleteCertificates()).To(Succeed()) + + block, _ := pem.Decode([]byte(c.KubeletCert)) + g.Expect(block).ToNot(BeNil()) + + cert, _ := x509.ParseCertificate(block.Bytes) + g.Expect(cert).ToNot(BeNil()) + + t.Run("IPAddresses", func(t *testing.T) { + g := NewWithT(t) + expectedIPs := []net.IP{net.ParseIP("192.168.2.123").To4(), net.ParseIP("127.0.0.1").To4(), net.ParseIP("::1")} + + g.Expect(cert.IPAddresses).To(ConsistOf(expectedIPs)) + }) + + t.Run("DNSNames", func(t *testing.T) { + g := NewWithT(t) + expectedDNSNames := []string{"h1", "cluster.local"} + + g.Expect(cert.DNSNames).To(ConsistOf(expectedDNSNames)) + }) }) } diff --git a/src/k8s/pkg/k8sd/pki/testdata/ca.pem b/src/k8s/pkg/k8sd/pki/testdata/ca.pem new file mode 100644 index 000000000..49b3b47f7 --- /dev/null +++ b/src/k8s/pkg/k8sd/pki/testdata/ca.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDkzCCAnugAwIBAgIUHjmfFK9cfwsJ9wVeay77DUxGfsQwDQYJKoZIhvcNAQEL +BQAwWTELMAkGA1UEBhMCVVMxDjAMBgNVBAgMBVN0YXRlMQ0wCwYDVQQHDARDaXR5 +MRUwEwYDVQQKDAxPcmdhbml6YXRpb24xFDASBgNVBAMMC2V4YW1wbGUuY29tMB4X +DTI0MDMyMTE5MTE1NVoXDTM0MDMxOTE5MTE1NVowWTELMAkGA1UEBhMCVVMxDjAM +BgNVBAgMBVN0YXRlMQ0wCwYDVQQHDARDaXR5MRUwEwYDVQQKDAxPcmdhbml6YXRp +b24xFDASBgNVBAMMC2V4YW1wbGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A +MIIBCgKCAQEAqiI/f/IWAk+2I3uxoTxB20RxrwUvPglAmsvXpkT40PbCHZ9pqI4I +GQoY5mR4bQMx4s3TQNIMGIIha9IvLXVgQb6WxZNc7lLWOg/VHAw+0tUkGnO2o89v +loRNJj2+ZcFu9UZQDLa/cr5pKGnFI4O3rR8DcQxt9rPtSY62ICLFwqU2Hw3fjyHI +FITKmTrZNccmcWKBuOfj4DkFaFT9+jZ72W8DHBXMjAm7qZC3ar9ZlzhHT8mI942i +LuNd0r47yrzga/kLCtjHDYXjBGBareIsfAZDJ+1WV9wVShL42brTwchZhBVcxY66 +by8PZJPD97c22zvVyCKIUGGcFKxvWb2fBQIDAQABo1MwUTAdBgNVHQ4EFgQU3LTT +fZ/8wUZhUj856yEniIkE6xwwHwYDVR0jBBgwFoAU3LTTfZ/8wUZhUj856yEniIkE +6xwwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEABRNgwMKqA5Y8 +7wfa+X3RsoG0BVOF/+GYCtyXBwH3lXzlOrkTbkL4e9rYGmPx67VCpsnCEAhipta3 +FqjGyZFhMsaaIDlhJjm+K7MTGA7aSfo6NIBmpPRKjIQFL2rhmqs1r7riafwvvDrU +CzhIi7rODCf7NAzoISU1EzowzKdKNgGYMNvIpv1pMd7p7WHQNK+W+gvQJZ93UpDY +o9fgMdo44Am9bsiiPi7LAWU5qzbdUErrgFslI+inwD3dOxIwBGfEfD0ngz2nF+Jh +S63GKldmH7KYVE4sdB2BvfgiraDTTHRIDNre930YIhVI+XLHIhtJ+BSpFO4w/idC +xjvgVUetag== +-----END CERTIFICATE----- diff --git a/src/k8s/pkg/utils/certificate.go b/src/k8s/pkg/utils/certificate.go new file mode 100644 index 000000000..60e60cc64 --- /dev/null +++ b/src/k8s/pkg/utils/certificate.go @@ -0,0 +1,28 @@ +package utils + +import ( + "log" + "net" +) + +// SplitIPAndDNSSANs splits a list of SANs into IP and DNS SANs +// Returns a list of IP addresses and a list of DNS names +func SplitIPAndDNSSANs(extraSANs []string) ([]net.IP, []string) { + var ipSANs []net.IP + var dnsSANs []string + + for _, san := range extraSANs { + if san == "" { + log.Println("Skipping empty SAN") + continue + } + + if ip := net.ParseIP(san); ip != nil { + ipSANs = append(ipSANs, ip) + } else { + dnsSANs = append(dnsSANs, san) + } + } + + return ipSANs, dnsSANs +} diff --git a/src/k8s/pkg/utils/certificate_test.go b/src/k8s/pkg/utils/certificate_test.go new file mode 100644 index 000000000..0400cdcf7 --- /dev/null +++ b/src/k8s/pkg/utils/certificate_test.go @@ -0,0 +1,26 @@ +package utils_test + +import ( + "testing" + + "github.com/canonical/k8s/pkg/utils" + + . "github.com/onsi/gomega" +) + +func TestSplitIPAndDNSSANs(t *testing.T) { + tests := []string{"192.168.0.1", "::1", "cluster.local", "kubernetes.svc.local", "", "2001:db8:0:1:1:1:1:1"} + + g := NewWithT(t) + gotIPs, gotDNSs := utils.SplitIPAndDNSSANs(tests) + + // Convert cert.IPAddresses to a slice of string representations + ips := make([]string, len(gotIPs)) + for i, ip := range gotIPs { + ips[i] = ip.String() + } + + g.Expect(ips).To(ConsistOf("192.168.0.1", "::1", "2001:db8:0:1:1:1:1:1")) + + g.Expect(gotDNSs).To(ConsistOf("cluster.local", "kubernetes.svc.local")) +}