From 38802c218482a7cdaa379fd8bfc400cff9d18604 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 27 Oct 2020 16:33:08 -0700 Subject: [PATCH] Add a way to set a default supervisor TLS cert for when SNI won't work - Setting a Secret in the supervisor's namespace with a special name will cause it to get picked up and served as the supervisor's TLS cert for any request which does not have a matching SNI cert. - This is especially useful for when there is no DNS record for an issuer and the user will be accessing it via IP address. This is not how we would expect it to be used in production, but it might be useful for other cases. - Includes a new integration test - Also suppress all of the warnings about ignoring the error returned by Close() in lines like `defer x.Close()` to make GoLand happier --- cmd/local-user-authenticator/main.go | 4 +- cmd/local-user-authenticator/main_test.go | 6 +- cmd/pinniped-supervisor/main.go | 14 ++- internal/certauthority/certauthority.go | 6 +- internal/certauthority/certauthority_test.go | 3 +- internal/controller/apicerts/certs_manager.go | 1 + .../supervisorconfig/tls_cert_observer.go | 52 +++++++---- .../tls_cert_observer_test.go | 90 ++++++++++++++----- .../provider/dynamic_tls_cert_provider.go | 15 ++++ test/integration/supervisor_discovery_test.go | 59 ++++++++++-- 10 files changed, 192 insertions(+), 58 deletions(-) diff --git a/cmd/local-user-authenticator/main.go b/cmd/local-user-authenticator/main.go index f774500c..f226066f 100644 --- a/cmd/local-user-authenticator/main.go +++ b/cmd/local-user-authenticator/main.go @@ -109,7 +109,7 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { if err != nil { return } - defer req.Body.Close() + defer func() { _ = req.Body.Close() }() secret, err := w.secretInformer.Lister().Secrets(namespace).Get(username) notFound := k8serrors.IsNotFound(err) @@ -379,7 +379,7 @@ func run() error { if err != nil { return fmt.Errorf("cannot create listener: %w", err) } - defer l.Close() + defer func() { _ = l.Close() }() err = startWebhook(ctx, l, dynamicCertProvider, kubeInformers.Core().V1().Secrets()) if err != nil { diff --git a/cmd/local-user-authenticator/main_test.go b/cmd/local-user-authenticator/main_test.go index 3db5a542..07771971 100644 --- a/cmd/local-user-authenticator/main_test.go +++ b/cmd/local-user-authenticator/main_test.go @@ -106,7 +106,7 @@ func TestWebhook(t *testing.T) { l, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) - defer l.Close() + defer func() { _ = l.Close() }() require.NoError(t, w.start(ctx, l)) client := newClient(caBundle, serverName) @@ -412,7 +412,7 @@ func TestWebhook(t *testing.T) { Body: body, }) require.NoError(t, err) - defer rsp.Body.Close() + defer func() { _ = rsp.Body.Close() }() require.Equal(t, test.wantStatus, rsp.StatusCode) @@ -470,7 +470,7 @@ func newCertProvider(t *testing.T) (dynamiccert.Provider, []byte, string) { ca, err := certauthority.New(pkix.Name{CommonName: serverName + " CA"}, time.Hour*24) require.NoError(t, err) - cert, err := ca.Issue(pkix.Name{CommonName: serverName}, []string{serverName}, time.Hour*24) + cert, err := ca.Issue(pkix.Name{CommonName: serverName}, []string{serverName}, nil, time.Hour*24) require.NoError(t, err) certPEM, keyPEM, err := certauthority.ToPEM(cert) diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index 94a924bd..55e35adc 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -201,7 +201,7 @@ func run(serverInstallationNamespace string, cfg *supervisor.Config) error { if err != nil { return fmt.Errorf("cannot create listener: %w", err) } - defer httpListener.Close() + defer func() { _ = httpListener.Close() }() start(ctx, httpListener, oidProvidersManager) //nolint: gosec // Intentionally binding to all network interfaces. @@ -209,14 +209,22 @@ func run(serverInstallationNamespace string, cfg *supervisor.Config) error { MinVersion: tls.VersionTLS12, // Allow v1.2 because clients like the default `curl` on MacOS don't support 1.3 yet. GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { cert := dynamicTLSCertProvider.GetTLSCert(strings.ToLower(info.ServerName)) - klog.InfoS("GetCertificate called for port 443", "info.ServerName", info.ServerName, "foundCert", cert != nil) + defaultCert := dynamicTLSCertProvider.GetDefaultTLSCert() + klog.InfoS("GetCertificate called for port 443", + "info.ServerName", info.ServerName, + "foundSNICert", cert != nil, + "foundDefaultCert", defaultCert != nil, + ) + if cert == nil { + cert = defaultCert + } return cert, nil }, }) if err != nil { return fmt.Errorf("cannot create listener: %w", err) } - defer httpsListener.Close() + defer func() { _ = httpsListener.Close() }() start(ctx, httpsListener, oidProvidersManager) klog.InfoS("supervisor is ready", diff --git a/internal/certauthority/certauthority.go b/internal/certauthority/certauthority.go index 9e5f77dd..ba3e6492 100644 --- a/internal/certauthority/certauthority.go +++ b/internal/certauthority/certauthority.go @@ -17,6 +17,7 @@ import ( "fmt" "io" "math/big" + "net" "time" ) @@ -136,7 +137,7 @@ func (c *CA) Bundle() []byte { } // Issue a new server certificate for the given identity and duration. -func (c *CA) Issue(subject pkix.Name, dnsNames []string, ttl time.Duration) (*tls.Certificate, error) { +func (c *CA) Issue(subject pkix.Name, dnsNames []string, ips []net.IP, ttl time.Duration) (*tls.Certificate, error) { // Choose a random 128 bit serial number. serialNumber, err := randomSerial(c.env.serialRNG) if err != nil { @@ -171,6 +172,7 @@ func (c *CA) Issue(subject pkix.Name, dnsNames []string, ttl time.Duration) (*tl BasicConstraintsValid: true, IsCA: false, DNSNames: dnsNames, + IPAddresses: ips, } certBytes, err := x509.CreateCertificate(rand.Reader, &template, caCert, &privateKey.PublicKey, c.signer) if err != nil { @@ -194,7 +196,7 @@ func (c *CA) Issue(subject pkix.Name, dnsNames []string, ttl time.Duration) (*tl // IssuePEM issues a new server certificate for the given identity and duration, returning it as a pair of // PEM-formatted byte slices for the certificate and private key. func (c *CA) IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error) { - return toPEM(c.Issue(subject, dnsNames, ttl)) + return toPEM(c.Issue(subject, dnsNames, nil, ttl)) } func toPEM(cert *tls.Certificate, err error) ([]byte, []byte, error) { diff --git a/internal/certauthority/certauthority_test.go b/internal/certauthority/certauthority_test.go index 72737991..f2dc5b33 100644 --- a/internal/certauthority/certauthority_test.go +++ b/internal/certauthority/certauthority_test.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "io/ioutil" + "net" "strings" "testing" "time" @@ -282,7 +283,7 @@ func TestIssue(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - got, err := tt.ca.Issue(pkix.Name{CommonName: "Test Server"}, []string{"example.com"}, 10*time.Minute) + got, err := tt.ca.Issue(pkix.Name{CommonName: "Test Server"}, []string{"example.com"}, []net.IP{net.IPv4(1, 2, 3, 4)}, 10*time.Minute) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) require.Nil(t, got) diff --git a/internal/controller/apicerts/certs_manager.go b/internal/controller/apicerts/certs_manager.go index 541eaf62..c0be7873 100644 --- a/internal/controller/apicerts/certs_manager.go +++ b/internal/controller/apicerts/certs_manager.go @@ -103,6 +103,7 @@ func (c *certsManagerController) Sync(ctx controllerlib.Context) error { aggregatedAPIServerTLSCert, err := aggregatedAPIServerCA.Issue( pkix.Name{CommonName: serviceEndpoint}, []string{serviceEndpoint}, + nil, c.certDuration, ) if err != nil { diff --git a/internal/controller/supervisorconfig/tls_cert_observer.go b/internal/controller/supervisorconfig/tls_cert_observer.go index 6a4ce599..e29930c6 100644 --- a/internal/controller/supervisorconfig/tls_cert_observer.go +++ b/internal/controller/supervisorconfig/tls_cert_observer.go @@ -18,18 +18,21 @@ import ( "go.pinniped.dev/internal/controllerlib" ) +const SecretNameForDefaultTLSCertificate = "default-tls-certificate" //nolint:gosec // this is not a hardcoded credential + type tlsCertObserverController struct { - issuerHostToTLSCertMapSetter IssuerHostToTLSCertMapSetter - oidcProviderConfigInformer v1alpha1.OIDCProviderConfigInformer - secretInformer corev1informers.SecretInformer + issuerTLSCertSetter IssuerTLSCertSetter + oidcProviderConfigInformer v1alpha1.OIDCProviderConfigInformer + secretInformer corev1informers.SecretInformer } -type IssuerHostToTLSCertMapSetter interface { +type IssuerTLSCertSetter interface { SetIssuerHostToTLSCertMap(issuerHostToTLSCertMap map[string]*tls.Certificate) + SetDefaultTLSCert(certificate *tls.Certificate) } func NewTLSCertObserverController( - issuerHostToTLSCertMapSetter IssuerHostToTLSCertMapSetter, + issuerTLSCertSetter IssuerTLSCertSetter, secretInformer corev1informers.SecretInformer, oidcProviderConfigInformer v1alpha1.OIDCProviderConfigInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, @@ -38,9 +41,9 @@ func NewTLSCertObserverController( controllerlib.Config{ Name: "tls-certs-observer-controller", Syncer: &tlsCertObserverController{ - issuerHostToTLSCertMapSetter: issuerHostToTLSCertMapSetter, - oidcProviderConfigInformer: oidcProviderConfigInformer, - secretInformer: secretInformer, + issuerTLSCertSetter: issuerTLSCertSetter, + oidcProviderConfigInformer: oidcProviderConfigInformer, + secretInformer: secretInformer, }, }, withInformer( @@ -74,26 +77,41 @@ func (c *tlsCertObserverController) Sync(ctx controllerlib.Context) error { klog.InfoS("tlsCertObserverController Sync found an invalid issuer URL", "namespace", ns, "issuer", provider.Spec.Issuer) continue } - tlsSecret, err := c.secretInformer.Lister().Secrets(ns).Get(secretName) + certFromSecret, err := c.certFromSecret(ns, secretName) if err != nil { - klog.InfoS("tlsCertObserverController Sync could not find TLS cert secret", "namespace", ns, "secretName", secretName) - continue - } - certFromSecret, err := tls.X509KeyPair(tlsSecret.Data["tls.crt"], tlsSecret.Data["tls.key"]) - if err != nil { - klog.InfoS("tlsCertObserverController Sync found a TLS secret with Data in an unexpected format", "namespace", ns, "secretName", secretName) continue } // Lowercase the host part of the URL because hostnames should be treated as case-insensitive. - issuerHostToTLSCertMap[lowercaseHostWithoutPort(issuerURL)] = &certFromSecret + issuerHostToTLSCertMap[lowercaseHostWithoutPort(issuerURL)] = certFromSecret } klog.InfoS("tlsCertObserverController Sync updated the TLS cert cache", "issuerHostCount", len(issuerHostToTLSCertMap)) - c.issuerHostToTLSCertMapSetter.SetIssuerHostToTLSCertMap(issuerHostToTLSCertMap) + c.issuerTLSCertSetter.SetIssuerHostToTLSCertMap(issuerHostToTLSCertMap) + + defaultCert, err := c.certFromSecret(ns, SecretNameForDefaultTLSCertificate) + if err != nil { + c.issuerTLSCertSetter.SetDefaultTLSCert(nil) + } else { + c.issuerTLSCertSetter.SetDefaultTLSCert(defaultCert) + } return nil } +func (c *tlsCertObserverController) certFromSecret(ns string, secretName string) (*tls.Certificate, error) { + tlsSecret, err := c.secretInformer.Lister().Secrets(ns).Get(secretName) + if err != nil { + klog.InfoS("tlsCertObserverController Sync could not find TLS cert secret", "namespace", ns, "secretName", secretName) + return nil, err + } + certFromSecret, err := tls.X509KeyPair(tlsSecret.Data["tls.crt"], tlsSecret.Data["tls.key"]) + if err != nil { + klog.InfoS("tlsCertObserverController Sync found a TLS secret with Data in an unexpected format", "namespace", ns, "secretName", secretName) + return nil, err + } + return &certFromSecret, nil +} + func lowercaseHostWithoutPort(issuerURL *url.URL) string { lowercaseHost := strings.ToLower(issuerURL.Host) colonSegments := strings.Split(lowercaseHost, ":") diff --git a/internal/controller/supervisorconfig/tls_cert_observer_test.go b/internal/controller/supervisorconfig/tls_cert_observer_test.go index c82449e7..633bad04 100644 --- a/internal/controller/supervisorconfig/tls_cert_observer_test.go +++ b/internal/controller/supervisorconfig/tls_cert_observer_test.go @@ -96,31 +96,38 @@ func TestTLSCertObserverControllerInformerFilters(t *testing.T) { }, spec.Parallel(), spec.Report(report.Terminal{})) } -type fakeIssuerHostToTLSCertMapSetter struct { +type fakeIssuerTLSCertSetter struct { setIssuerHostToTLSCertMapWasCalled bool + setDefaultTLSCertWasCalled bool issuerHostToTLSCertMapReceived map[string]*tls.Certificate + setDefaultTLSCertReceived *tls.Certificate } -func (f *fakeIssuerHostToTLSCertMapSetter) SetIssuerHostToTLSCertMap(issuerHostToTLSCertMap map[string]*tls.Certificate) { +func (f *fakeIssuerTLSCertSetter) SetIssuerHostToTLSCertMap(issuerHostToTLSCertMap map[string]*tls.Certificate) { f.setIssuerHostToTLSCertMapWasCalled = true f.issuerHostToTLSCertMapReceived = issuerHostToTLSCertMap } +func (f *fakeIssuerTLSCertSetter) SetDefaultTLSCert(certificate *tls.Certificate) { + f.setDefaultTLSCertWasCalled = true + f.setDefaultTLSCertReceived = certificate +} + func TestTLSCertObserverControllerSync(t *testing.T) { spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { const installedInNamespace = "some-namespace" var ( - r *require.Assertions - subject controllerlib.Controller - pinnipedInformerClient *pinnipedfake.Clientset - kubeInformerClient *kubernetesfake.Clientset - pinnipedInformers pinnipedinformers.SharedInformerFactory - kubeInformers kubeinformers.SharedInformerFactory - timeoutContext context.Context - timeoutContextCancel context.CancelFunc - syncContext *controllerlib.Context - issuerHostToTLSCertSetter *fakeIssuerHostToTLSCertMapSetter + r *require.Assertions + subject controllerlib.Controller + pinnipedInformerClient *pinnipedfake.Clientset + kubeInformerClient *kubernetesfake.Clientset + pinnipedInformers pinnipedinformers.SharedInformerFactory + kubeInformers kubeinformers.SharedInformerFactory + timeoutContext context.Context + timeoutContextCancel context.CancelFunc + syncContext *controllerlib.Context + issuerTLSCertSetter *fakeIssuerTLSCertSetter ) // Defer starting the informers until the last possible moment so that the @@ -128,7 +135,7 @@ func TestTLSCertObserverControllerSync(t *testing.T) { var startInformersAndController = func() { // Set this at the last second to allow for injection of server override. subject = NewTLSCertObserverController( - issuerHostToTLSCertSetter, + issuerTLSCertSetter, kubeInformers.Core().V1().Secrets(), pinnipedInformers.Config().V1alpha1().OIDCProviderConfigs(), controllerlib.WithInformer, @@ -165,7 +172,7 @@ func TestTLSCertObserverControllerSync(t *testing.T) { kubeInformers = kubeinformers.NewSharedInformerFactory(kubeInformerClient, 0) pinnipedInformerClient = pinnipedfake.NewSimpleClientset() pinnipedInformers = pinnipedinformers.NewSharedInformerFactory(pinnipedInformerClient, 0) - issuerHostToTLSCertSetter = &fakeIssuerHostToTLSCertMapSetter{} + issuerTLSCertSetter = &fakeIssuerTLSCertSetter{} unrelatedSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -181,13 +188,15 @@ func TestTLSCertObserverControllerSync(t *testing.T) { }) when("there are no OIDCProviderConfigs and no TLS Secrets yet", func() { - it("sets the issuerHostToTLSCertSetter's map to be empty", func() { + it("sets the issuerTLSCertSetter's map to be empty", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - r.True(issuerHostToTLSCertSetter.setIssuerHostToTLSCertMapWasCalled) - r.Empty(issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived) + r.True(issuerTLSCertSetter.setIssuerHostToTLSCertMapWasCalled) + r.Empty(issuerTLSCertSetter.issuerHostToTLSCertMapReceived) + r.True(issuerTLSCertSetter.setDefaultTLSCertWasCalled) + r.Nil(issuerTLSCertSetter.setDefaultTLSCertReceived) }) }) @@ -293,24 +302,61 @@ func TestTLSCertObserverControllerSync(t *testing.T) { r.NoError(kubeInformerClient.Tracker().Add(badTLSSecret)) }) - it("updates the issuerHostToTLSCertSetter's map to include only the issuers that had valid certs", func() { + it("updates the issuerTLSCertSetter's map to include only the issuers that had valid certs", func() { startInformersAndController() r.NoError(controllerlib.TestSync(t, subject, *syncContext)) - r.True(issuerHostToTLSCertSetter.setIssuerHostToTLSCertMapWasCalled) - r.Len(issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived, 2) + r.True(issuerTLSCertSetter.setDefaultTLSCertWasCalled) + r.Nil(issuerTLSCertSetter.setDefaultTLSCertReceived) + + r.True(issuerTLSCertSetter.setIssuerHostToTLSCertMapWasCalled) + r.Len(issuerTLSCertSetter.issuerHostToTLSCertMapReceived, 2) // They keys in the map should be lower case and should not include the port numbers, because // TLS SNI says that SNI hostnames must be DNS names (not ports) and must be case insensitive. // See https://tools.ietf.org/html/rfc3546#section-3.1 - actualCertificate1 := issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived["www.issuer-with-good-secret1.com"] + actualCertificate1 := issuerTLSCertSetter.issuerHostToTLSCertMapReceived["www.issuer-with-good-secret1.com"] r.NotNil(actualCertificate1) // The actual cert should match the one from the test fixture that was put into the secret. r.Equal(expectedCertificate1, *actualCertificate1) - actualCertificate2 := issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived["www.issuer-with-good-secret2.com"] + actualCertificate2 := issuerTLSCertSetter.issuerHostToTLSCertMapReceived["www.issuer-with-good-secret2.com"] r.NotNil(actualCertificate2) r.Equal(expectedCertificate2, *actualCertificate2) }) + + when("there is also a default TLS cert secret called default-tls-certificate", func() { + var ( + expectedDefaultCertificate tls.Certificate + ) + + it.Before(func() { + var err error + testCrt := readTestFile("testdata/test3.crt") + r.NotEmpty(testCrt) + testKey := readTestFile("testdata/test3.key") + r.NotEmpty(testKey) + expectedDefaultCertificate, err = tls.X509KeyPair(testCrt, testKey) + r.NoError(err) + defaultTLSCertSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "default-tls-certificate", Namespace: installedInNamespace}, + Data: map[string][]byte{"tls.crt": testCrt, "tls.key": testKey}, + } + r.NoError(kubeInformerClient.Tracker().Add(defaultTLSCertSecret)) + }) + + it("updates the issuerTLSCertSetter's map as before but also updates the default certificate", func() { + startInformersAndController() + r.NoError(controllerlib.TestSync(t, subject, *syncContext)) + + r.True(issuerTLSCertSetter.setDefaultTLSCertWasCalled) + actualDefaultCertificate := issuerTLSCertSetter.setDefaultTLSCertReceived + r.NotNil(actualDefaultCertificate) + r.Equal(expectedDefaultCertificate, *actualDefaultCertificate) + + r.True(issuerTLSCertSetter.setIssuerHostToTLSCertMapWasCalled) + r.Len(issuerTLSCertSetter.issuerHostToTLSCertMapReceived, 2) + }) + }) }) }, spec.Parallel(), spec.Report(report.Terminal{})) } diff --git a/internal/oidc/provider/dynamic_tls_cert_provider.go b/internal/oidc/provider/dynamic_tls_cert_provider.go index 51e3c157..7c48ad9c 100644 --- a/internal/oidc/provider/dynamic_tls_cert_provider.go +++ b/internal/oidc/provider/dynamic_tls_cert_provider.go @@ -10,11 +10,14 @@ import ( type DynamicTLSCertProvider interface { SetIssuerHostToTLSCertMap(issuerToJWKSMap map[string]*tls.Certificate) + SetDefaultTLSCert(certificate *tls.Certificate) GetTLSCert(lowercaseIssuerHostName string) *tls.Certificate + GetDefaultTLSCert() *tls.Certificate } type dynamicTLSCertProvider struct { issuerHostToTLSCertMap map[string]*tls.Certificate + defaultCert *tls.Certificate mutex sync.RWMutex } @@ -30,8 +33,20 @@ func (p *dynamicTLSCertProvider) SetIssuerHostToTLSCertMap(issuerHostToTLSCertMa p.issuerHostToTLSCertMap = issuerHostToTLSCertMap } +func (p *dynamicTLSCertProvider) SetDefaultTLSCert(certificate *tls.Certificate) { + p.mutex.Lock() // acquire a write lock + defer p.mutex.Unlock() + p.defaultCert = certificate +} + func (p *dynamicTLSCertProvider) GetTLSCert(issuerHostName string) *tls.Certificate { p.mutex.RLock() // acquire a read lock defer p.mutex.RUnlock() return p.issuerHostToTLSCertMap[issuerHostName] } + +func (p *dynamicTLSCertProvider) GetDefaultTLSCert() *tls.Certificate { + p.mutex.RLock() // acquire a read lock + defer p.mutex.RUnlock() + return p.defaultCert +} diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index ebed769a..38c088ff 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -31,6 +31,49 @@ import ( "go.pinniped.dev/test/library" ) +func TestSupervisorTLSTerminationWithDefaultCerts(t *testing.T) { + env := library.IntegrationEnv(t) + pinnipedClient := library.NewPinnipedClientset(t) + kubeClient := library.NewClientset(t) + + ns := env.SupervisorNamespace + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + temporarilyRemoveAllOIDCProviderConfigs(ctx, t, ns, pinnipedClient) + + scheme := "https" + address := env.SupervisorHTTPSAddress // hostname and port for direct access to the supervisor's port 443 + + hostAndPortSegments := strings.Split(address, ":") + hostname := hostAndPortSegments[0] + port := "443" + if len(hostAndPortSegments) > 1 { + port = hostAndPortSegments[1] + } + ips, err := net.DefaultResolver.LookupIP(ctx, "ip4", hostname) + require.NoError(t, err) + ip := ips[0] + ipAsString := ip.String() + ipWithPort := ipAsString + ":" + port + + issuerUsingIPAddress := fmt.Sprintf("%s://%s/issuer1", scheme, ipWithPort) + + // Create an OIDCProviderConfig without an sniCertificateSecretName. + oidcProviderConfig1 := library.CreateTestOIDCProvider(ctx, t, issuerUsingIPAddress, "") + requireStatus(t, pinnipedClient, oidcProviderConfig1.Namespace, oidcProviderConfig1.Name, v1alpha1.SuccessOIDCProviderStatus) + + // There is no default TLS cert and the sniCertificateSecretName was not set, so the endpoints should fail with TLS errors. + requireEndpointHasTLSErrorBecauseCertificatesAreNotReady(t, issuerUsingIPAddress) + + // Create a Secret at the special name which represents the default TLS cert. + specialNameForDefaultTLSCertSecret := "default-tls-certificate" //nolint:gosec // this is not a hardcoded credential + ca := createTLSCertificateSecret(ctx, t, ns, "cert-hostname-doesnt-matter", []net.IP{ip}, specialNameForDefaultTLSCertSecret, kubeClient) + + // Now that the Secret exists, we should be able to access the endpoints by IP address using the CA. + _ = requireDiscoveryEndpointsAreWorking(t, scheme, ipWithPort, string(ca.Bundle()), issuerUsingIPAddress, nil) +} + func TestSupervisorTLSTerminationWithSNI(t *testing.T) { env := library.IntegrationEnv(t) pinnipedClient := library.NewPinnipedClientset(t) @@ -57,7 +100,7 @@ func TestSupervisorTLSTerminationWithSNI(t *testing.T) { requireEndpointHasTLSErrorBecauseCertificatesAreNotReady(t, issuer1) // Create the Secret. - ca1 := createSNICertificateSecret(ctx, t, ns, hostname1, sniCertificateSecretName1, kubeClient) + ca1 := createTLSCertificateSecret(ctx, t, ns, hostname1, nil, sniCertificateSecretName1, kubeClient) // Now that the Secret exists, we should be able to access the endpoints by hostname using the CA. _ = requireDiscoveryEndpointsAreWorking(t, scheme, address, string(ca1.Bundle()), issuer1, nil) @@ -74,7 +117,7 @@ func TestSupervisorTLSTerminationWithSNI(t *testing.T) { requireEndpointHasTLSErrorBecauseCertificatesAreNotReady(t, issuer1) // Create a Secret at the updated name. - ca1update := createSNICertificateSecret(ctx, t, ns, hostname1, sniCertificateSecretName1update, kubeClient) + ca1update := createTLSCertificateSecret(ctx, t, ns, hostname1, nil, sniCertificateSecretName1update, kubeClient) // Now that the Secret exists at the new name, we should be able to access the endpoints by hostname using the CA. _ = requireDiscoveryEndpointsAreWorking(t, scheme, address, string(ca1update.Bundle()), issuer1, nil) @@ -90,7 +133,7 @@ func TestSupervisorTLSTerminationWithSNI(t *testing.T) { requireStatus(t, pinnipedClient, oidcProviderConfig2.Namespace, oidcProviderConfig2.Name, v1alpha1.SuccessOIDCProviderStatus) // Create the Secret. - ca2 := createSNICertificateSecret(ctx, t, ns, hostname2, sniCertificateSecretName2, kubeClient) + ca2 := createTLSCertificateSecret(ctx, t, ns, hostname2, nil, sniCertificateSecretName2, kubeClient) // Now that the Secret exists, we should be able to access the endpoints by hostname using the CA. _ = requireDiscoveryEndpointsAreWorking(t, scheme, hostname2+":"+hostnamePort2, string(ca2.Bundle()), issuer2, map[string]string{ @@ -195,13 +238,13 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { } } -func createSNICertificateSecret(ctx context.Context, t *testing.T, ns string, hostname string, sniCertificateSecretName string, kubeClient kubernetes.Interface) *certauthority.CA { +func createTLSCertificateSecret(ctx context.Context, t *testing.T, ns string, hostname string, ips []net.IP, secretName string, kubeClient kubernetes.Interface) *certauthority.CA { // Create a CA. ca, err := certauthority.New(pkix.Name{CommonName: "Acme Corp"}, 1000*time.Hour) require.NoError(t, err) // Using the CA, create a TLS server cert. - tlsCert, err := ca.Issue(pkix.Name{CommonName: hostname}, []string{hostname}, 1000*time.Hour) + tlsCert, err := ca.Issue(pkix.Name{CommonName: hostname}, []string{hostname}, ips, 1000*time.Hour) require.NoError(t, err) // Write the serving cert to the SNI secret. @@ -210,7 +253,7 @@ func createSNICertificateSecret(ctx context.Context, t *testing.T, ns string, ho secret := corev1.Secret{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Name: sniCertificateSecretName, + Name: secretName, Namespace: ns, }, StringData: map[string]string{ @@ -226,7 +269,7 @@ func createSNICertificateSecret(ctx context.Context, t *testing.T, ns string, ho t.Helper() deleteCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - err := kubeClient.CoreV1().Secrets(ns).Delete(deleteCtx, sniCertificateSecretName, metav1.DeleteOptions{}) + err := kubeClient.CoreV1().Secrets(ns).Delete(deleteCtx, secretName, metav1.DeleteOptions{}) require.NoError(t, err) }) @@ -308,7 +351,7 @@ func requireEndpointHasTLSErrorBecauseCertificatesAreNotReady(t *testing.T, url return err != nil && strings.Contains(err.Error(), "tls: unrecognized name") }, 10*time.Second, 200*time.Millisecond) require.Error(t, err) - require.EqualError(t, err, `Get "https://localhost:12344/issuer1": remote error: tls: unrecognized name`) + require.EqualError(t, err, fmt.Sprintf(`Get "%s": remote error: tls: unrecognized name`, url)) } func requireCreatingOIDCProviderConfigCausesDiscoveryEndpointsToAppear(