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
This commit is contained in:
Ryan Richard
2020-10-27 16:33:08 -07:00
parent 1f1b6c884e
commit 38802c2184
10 changed files with 192 additions and 58 deletions
@@ -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 {
@@ -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, ":")
@@ -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{}))
}