diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/BUILD b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/BUILD index 735d024d516..c8ee012fce2 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/BUILD @@ -57,10 +57,12 @@ go_test( "cert_key_test.go", "client_ca_test.go", "named_certificates_test.go", + "server_test.go", "tlsconfig_test.go", ], embed = [":go_default_library"], deps = [ + "//staging/src/k8s.io/client-go/util/cert:go_default_library", "//vendor/github.com/davecgh/go-spew/spew:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates.go index 5590dea5991..f4a7ed116e1 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates.go @@ -20,6 +20,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "net" "strings" v1 "k8s.io/api/core/v1" @@ -76,7 +77,10 @@ func getCertificateNames(cert *x509.Certificate) []string { var names []string cn := cert.Subject.CommonName - if cn == "*" || len(validation.IsDNS1123Subdomain(strings.TrimPrefix(cn, "*."))) == 0 { + cnIsIP := net.ParseIP(cn) != nil + cnIsValidDomain := cn == "*" || len(validation.IsDNS1123Subdomain(strings.TrimPrefix(cn, "*."))) == 0 + // don't use the CN if it is a valid IP because our IP serving detection may unexpectedly use it to terminate the connection. + if !cnIsIP && cnIsValidDomain { names = append(names, cn) } for _, san := range cert.DNSNames { diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates_test.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates_test.go index 443817cfd43..56848723337 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/named_certificates_test.go @@ -68,6 +68,20 @@ func TestBuiltNamedCertificates(t *testing.T) { "test.com": 0, }, }, + { + // ip as cns are ignored + certs: []namedtestCertSpec{ + { + testCertSpec: testCertSpec{ + host: "1.2.3.4", + names: []string{"test.com"}, + }, + }, + }, + expected: map[string]int{ + "test.com": 0, + }, + }, { // ips are ignored certs: []namedtestCertSpec{ diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/server_test.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/server_test.go new file mode 100644 index 00000000000..0503c2c10b1 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/server_test.go @@ -0,0 +1,218 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package dynamiccertificates + +import ( + "bytes" + "crypto/tls" + "crypto/x509" + "fmt" + "net" + "net/http" + "strings" + "testing" + "time" + + "k8s.io/client-go/util/cert" +) + +func TestServingCert(t *testing.T) { + tlsConfig := &tls.Config{ + // Can't use SSLv3 because of POODLE and BEAST + // Can't use TLSv1.0 because of POODLE and BEAST using CBC cipher + // Can't use TLSv1.1 because of RC4 cipher usage + MinVersion: tls.VersionTLS12, + // enable HTTP2 for go's 1.7 HTTP Server + NextProtos: []string{"h2", "http/1.1"}, + } + + defaultCertProvider, err := createTestTLSCerts(testCertSpec{ + host: "172.30.0.1", + ips: []string{"172.30.0.1"}, + names: []string{"kubernetes", "kubernetes.default.svc.cluster.local"}, + }, nil) + if err != nil { + t.Fatal(err) + } + + var sniCerts []SNICertKeyContentProvider + certSpecs := []testCertSpec{ + { + host: "172.30.0.1", + ips: []string{"172.30.0.1"}, + names: []string{"openshift", "openshift.default.svc.cluster.local"}, + }, + { + host: "127.0.0.1", + ips: []string{"127.0.0.1"}, + names: []string{"localhost"}, + }, + { + host: "2001:abcd:bcda::1", + ips: []string{"2001:abcd:bcda::1"}, + names: []string{"openshiftv6", "openshiftv6.default.svc.cluster.local"}, + }, + } + + for _, certSpec := range certSpecs { + names := append([]string{}, certSpec.ips...) + names = append(names, certSpec.names...) + certProvider, err := createTestTLSCerts(certSpec, names) + if err != nil { + t.Fatal(err) + } + sniCerts = append(sniCerts, certProvider) + } + + dynamicCertificateController := NewDynamicServingCertificateController( + *tlsConfig, + &nullCAContent{name: "client-ca"}, + defaultCertProvider, + sniCerts, + nil, // TODO see how to plumb an event recorder down in here. For now this results in simply klog messages. + ) + if err := dynamicCertificateController.RunOnce(); err != nil { + t.Fatal(err) + } + tlsConfig.GetConfigForClient = dynamicCertificateController.GetConfigForClient + tlsConfig.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) { return nil, fmt.Errorf("positive failure") } + + stopCh := make(chan struct{}) + defer close(stopCh) + server := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusOK) + }), + MaxHeaderBytes: 1 << 20, + TLSConfig: tlsConfig, + } + listener, _, err := createListener("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + apiPort := listener.Addr().String() + go func() { + t.Logf("listening on %s", listener.Addr().String()) + listener = tls.NewListener(listener, server.TLSConfig) + if err := server.ServeTLS(listener, "", ""); err != nil { + t.Error(err) + panic(err) + } + + <-stopCh + server.Close() + t.Logf("stopped listening on %s", listener.Addr().String()) + }() + + time.Sleep(1 * time.Second) + + expectedDefaultCertBytes, _ := defaultCertProvider.CurrentCertKeyContent() + expectedServiceCertBytes, _ := sniCerts[0].CurrentCertKeyContent() + expectedLocalhostCertBytes, _ := sniCerts[1].CurrentCertKeyContent() + expectedServiceV6CertBytes, _ := sniCerts[2].CurrentCertKeyContent() + + tests := []struct { + name string + serverName string + expected []byte + }{ + { + name: "default", // the no server name hits 127.0.0.1, which we hope matches by IP + serverName: "", + expected: []byte(strings.TrimSpace(string(expectedLocalhostCertBytes))), + }, + { + name: "invalid", + serverName: "not-marked", + expected: []byte(strings.TrimSpace(string(expectedDefaultCertBytes))), + }, + { + name: "service by dns", + serverName: "openshift", + expected: []byte(strings.TrimSpace(string(expectedServiceCertBytes))), + }, + { + name: "service v6 by dns", + serverName: "openshiftv6", + expected: []byte(strings.TrimSpace(string(expectedServiceV6CertBytes))), + }, + { + name: "localhost by dns", + serverName: "localhost", + expected: []byte(strings.TrimSpace(string(expectedLocalhostCertBytes))), + }, + { + name: "localhost by IP", + serverName: "127.0.0.1", // this can never actually happen, but let's see + expected: []byte(strings.TrimSpace(string(expectedLocalhostCertBytes))), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, actualDefaultCertBytes, err := cert.GetServingCertificates(apiPort, test.serverName) + if err != nil { + t.Fatal(err) + } + if e, a := test.expected, actualDefaultCertBytes[0]; !bytes.Equal(e, a) { + eCert, _ := cert.ParseCertsPEM(e) + aCert, _ := cert.ParseCertsPEM(a) + t.Fatalf("expected %v\n, got %v", GetHumanCertDetail(eCert[0]), GetHumanCertDetail(aCert[0])) + } + }) + } + +} + +func createListener(network, addr string) (net.Listener, int, error) { + if len(network) == 0 { + network = "tcp" + } + ln, err := net.Listen(network, addr) + if err != nil { + return nil, 0, fmt.Errorf("failed to listen on %v: %v", addr, err) + } + + // get port + tcpAddr, ok := ln.Addr().(*net.TCPAddr) + if !ok { + ln.Close() + return nil, 0, fmt.Errorf("invalid listen address: %q", ln.Addr().String()) + } + + return ln, tcpAddr.Port, nil +} + +type nullCAContent struct { + name string +} + +var _ CAContentProvider = &nullCAContent{} + +// Name is just an identifier +func (c *nullCAContent) Name() string { + return c.name +} + +// CurrentCABundleContent provides ca bundle byte content +func (c *nullCAContent) CurrentCABundleContent() (cabundle []byte) { + return nil +} + +func (c *nullCAContent) VerifyOptions() (x509.VerifyOptions, bool) { + return x509.VerifyOptions{}, false +} diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go index 78e094a5d81..23a9b2e429e 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go @@ -21,6 +21,7 @@ import ( "crypto/x509" "errors" "fmt" + "net" "sync/atomic" "time" @@ -94,7 +95,28 @@ func (c *DynamicServingCertificateController) GetConfigForClient(clientHello *tl return nil, errors.New("dynamiccertificates: unexpected config type") } - return tlsConfig.Clone(), nil + tlsConfigCopy := tlsConfig.Clone() + + // if the client set SNI information, just use our "normal" SNI flow + if len(clientHello.ServerName) > 0 { + return tlsConfigCopy, nil + } + + // if the client didn't set SNI, then we need to inspect the requested IP so that we can choose + // a certificate from our list if we specifically handle that IP. This can happen when an IP is specifically mapped by name. + host, _, err := net.SplitHostPort(clientHello.Conn.LocalAddr().String()) + if err != nil { + return tlsConfigCopy, nil + } + + ipCert, ok := tlsConfigCopy.NameToCertificate[host] + if !ok { + return tlsConfigCopy, nil + } + tlsConfigCopy.Certificates = []tls.Certificate{*ipCert} + tlsConfigCopy.NameToCertificate = nil + + return tlsConfigCopy, nil } // newTLSContent determines the next set of content for overriding the baseTLSConfig. diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/serving.go b/staging/src/k8s.io/apiserver/pkg/server/options/serving.go index e9241061000..f40de698283 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/serving.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/serving.go @@ -180,7 +180,9 @@ func (s *SecureServingOptions) AddFlags(fs *pflag.FlagSet) { fs.Var(cliflag.NewNamedCertKeyArray(&s.SNICertKeys), "tls-sni-cert-key", ""+ "A pair of x509 certificate and private key file paths, optionally suffixed with a list of "+ "domain patterns which are fully qualified domain names, possibly with prefixed wildcard "+ - "segments. If no domain patterns are provided, the names of the certificate are "+ + "segments. The domain patterns also allow IP addresses, but IPs should only be used if "+ + "the apiserver has visibility to the IP address requested by a client. "+ + "If no domain patterns are provided, the names of the certificate are "+ "extracted. Non-wildcard matches trump over wildcard matches, explicit domain patterns "+ "trump over extracted names. For multiple key/certificate pairs, use the "+ "--tls-sni-cert-key multiple times. "+