Address PR feedback, especially to check that the CA bundle is some kind of valid cert

This commit is contained in:
Joshua Casey
2023-08-01 13:29:43 -05:00
parent 959f18b67b
commit dc61d132cf
39 changed files with 397 additions and 94 deletions

View File

@@ -328,7 +328,8 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre
func (c *impersonatorConfigController) ensureCAAndTLSSecrets(
ctx context.Context,
nameInfo *certNameInfo) ([]byte, error) {
nameInfo *certNameInfo,
) ([]byte, error) {
var (
impersonationCA *certauthority.CA
err error
@@ -349,7 +350,8 @@ func (c *impersonatorConfigController) ensureCAAndTLSSecrets(
func (c *impersonatorConfigController) evaluateExternallyProvidedTLSSecret(
ctx context.Context,
tlsSpec *v1alpha1.ImpersonationProxyTLSSpec) ([]byte, error) {
tlsSpec *v1alpha1.ImpersonationProxyTLSSpec,
) ([]byte, error) {
if tlsSpec.SecretName == "" {
return nil, fmt.Errorf("must provide impersonationSpec.TLS.secretName if impersonationSpec.TLS is provided")
}
@@ -378,6 +380,11 @@ func (c *impersonatorConfigController) evaluateExternallyProvidedTLSSecret(
return nil, fmt.Errorf("could not decode impersonationSpec.TLS.certificateAuthorityData: %w", err)
}
block, _ := pem.Decode(caBundle)
if block == nil {
return nil, fmt.Errorf("could not decode impersonationSpec.TLS.certificateAuthorityData: data is not a certificate")
}
c.infoLog.Info("the impersonation proxy will advertise its CA Bundle from impersonationSpec.TLS.CertificateAuthorityData",
"CertificateAuthorityData", caBundle)
}
@@ -723,7 +730,27 @@ func (c *impersonatorConfigController) readExternalTLSSecret(externalTLSSecretNa
return nil, err
}
return secretFromInformer.Data[caCrtKey], nil
base64EncodedCaCert := secretFromInformer.Data[caCrtKey]
if len(base64EncodedCaCert) > 0 {
var decodedCaCert []byte
decodedCaCert, err = base64.StdEncoding.DecodeString(string(secretFromInformer.Data[caCrtKey]))
if err != nil {
err = fmt.Errorf("unable to read provided ca.crt: %w", err)
plog.Error("error loading cert from externally provided TLS secret for the impersonation proxy", err)
return nil, err
}
block, _ := pem.Decode(decodedCaCert)
if block == nil {
plog.Warning("error loading cert from externally provided TLS secret for the impersonation proxy: data is not a certificate")
return nil, fmt.Errorf("unable to read provided ca.crt: data is not a certificate")
}
return decodedCaCert, nil
}
return nil, nil
}
func (c *impersonatorConfigController) ensureTLSSecret(ctx context.Context, nameInfo *certNameInfo, ca *certauthority.CA) error {

View File

@@ -1278,11 +1278,85 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
})
})
when("the CertificateAuthorityData is configured with invalid data", func() {
it.Before(func() {
addNodeWithRoleToTracker("worker", kubeAPIClient)
})
when("CertificateAuthorityData is not base64 encoded", func() {
it.Before(func() {
addSecretToTrackers(mTLSClientCertCASecret, kubeInformerClient)
addSecretToTrackers(externalTLSSecret, kubeInformerClient)
addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
Spec: v1alpha1.CredentialIssuerSpec{
ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
Mode: v1alpha1.ImpersonationProxyModeAuto,
ExternalEndpoint: localhostIP,
Service: v1alpha1.ImpersonationProxyServiceSpec{
Type: v1alpha1.ImpersonationProxyServiceTypeNone,
},
TLS: &v1alpha1.ImpersonationProxyTLSSpec{
CertificateAuthorityData: string(externalCA.Bundle()),
SecretName: externallyProvidedTLSSecretName,
},
},
},
}, pinnipedInformerClient, pinnipedAPIClient)
})
it("returns an error", func() {
startInformersAndController()
r.Error(runControllerSync(), "could not decode impersonationSpec.TLS.certificateAuthorityData: illegal base64 data at input byte 0")
r.Len(kubeAPIClient.Actions(), 1)
requireNodesListed(kubeAPIClient.Actions()[0])
requireCredentialIssuer(newErrorStrategy("could not decode impersonationSpec.TLS.certificateAuthorityData: illegal base64 data at input byte 0"))
requireMTLSClientCertProviderHasLoadedCerts([]byte{}, []byte{})
})
})
when("CertificateAuthorityData is not a cert", func() {
it.Before(func() {
addSecretToTrackers(mTLSClientCertCASecret, kubeInformerClient)
addSecretToTrackers(externalTLSSecret, kubeInformerClient)
addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
Spec: v1alpha1.CredentialIssuerSpec{
ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
Mode: v1alpha1.ImpersonationProxyModeAuto,
ExternalEndpoint: localhostIP,
Service: v1alpha1.ImpersonationProxyServiceSpec{
Type: v1alpha1.ImpersonationProxyServiceTypeNone,
},
TLS: &v1alpha1.ImpersonationProxyTLSSpec{
CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte("hello")),
SecretName: externallyProvidedTLSSecretName,
},
},
},
}, pinnipedInformerClient, pinnipedAPIClient)
})
it("returns an error", func() {
startInformersAndController()
r.Error(runControllerSync(), "could not decode impersonationSpec.TLS.certificateAuthorityData: data is not a certificate")
r.Len(kubeAPIClient.Actions(), 1)
requireNodesListed(kubeAPIClient.Actions()[0])
requireCredentialIssuer(newErrorStrategy("could not decode impersonationSpec.TLS.certificateAuthorityData: data is not a certificate"))
requireMTLSClientCertProviderHasLoadedCerts([]byte{}, []byte{})
})
})
})
when("the CertificateAuthorityData is not configured", func() {
it.Before(func() {
addNodeWithRoleToTracker("worker", kubeAPIClient)
})
when("the externally provided TLS secret has a ca.crt field", func() {
it.Before(func() {
addSecretToTrackers(mTLSClientCertCASecret, kubeInformerClient)
externalTLSSecret.Data["ca.crt"] = externalCA.Bundle()
externalTLSSecret.Data["ca.crt"] = []byte(base64.StdEncoding.EncodeToString(externalCA.Bundle()))
addSecretToTrackers(externalTLSSecret, kubeInformerClient)
addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
@@ -1299,7 +1373,6 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
},
},
}, pinnipedInformerClient, pinnipedAPIClient)
addNodeWithRoleToTracker("worker", kubeAPIClient)
})
it("will advertise ca.crt from the externally provided secret", func() {
@@ -1307,12 +1380,76 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
r.NoError(runControllerSync())
r.Len(kubeAPIClient.Actions(), 1)
requireNodesListed(kubeAPIClient.Actions()[0])
requireTLSServerIsRunning(externalTLSSecret.Data["ca.crt"], testServerAddr(), nil)
requireCredentialIssuer(newSuccessStrategy(localhostIP, externalTLSSecret.Data["ca.crt"]))
requireTLSServerIsRunning(externalCA.Bundle(), testServerAddr(), nil)
requireCredentialIssuer(newSuccessStrategy(localhostIP, externalCA.Bundle()))
requireMTLSClientCertProviderHasLoadedCerts(mTLSClientCertCACertPEM, mTLSClientCertCAPrivateKeyPEM)
})
})
when("the externally provided TLS secret has a ca.crt field that is not base64-encoded", func() {
it.Before(func() {
addSecretToTrackers(mTLSClientCertCASecret, kubeInformerClient)
externalTLSSecret.Data["ca.crt"] = []byte("hello")
addSecretToTrackers(externalTLSSecret, kubeInformerClient)
addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
Spec: v1alpha1.CredentialIssuerSpec{
ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
Mode: v1alpha1.ImpersonationProxyModeAuto,
ExternalEndpoint: localhostIP,
Service: v1alpha1.ImpersonationProxyServiceSpec{
Type: v1alpha1.ImpersonationProxyServiceTypeNone,
},
TLS: &v1alpha1.ImpersonationProxyTLSSpec{
SecretName: externallyProvidedTLSSecretName,
},
},
},
}, pinnipedInformerClient, pinnipedAPIClient)
})
it("returns an error", func() {
startInformersAndController()
r.Error(runControllerSync(), "could not load the externally provided TLS secret for the impersonation proxy: unable to read provided ca.crt: illegal base64 data at input byte 4")
r.Len(kubeAPIClient.Actions(), 1)
requireNodesListed(kubeAPIClient.Actions()[0])
requireCredentialIssuer(newErrorStrategy("could not load the externally provided TLS secret for the impersonation proxy: unable to read provided ca.crt: illegal base64 data at input byte 4"))
requireMTLSClientCertProviderHasLoadedCerts([]byte{}, []byte{})
})
})
when("the externally provided TLS secret has a ca.crt field that is not a valid cert", func() {
it.Before(func() {
addSecretToTrackers(mTLSClientCertCASecret, kubeInformerClient)
externalTLSSecret.Data["ca.crt"] = []byte(base64.StdEncoding.EncodeToString([]byte("hello")))
addSecretToTrackers(externalTLSSecret, kubeInformerClient)
addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
Spec: v1alpha1.CredentialIssuerSpec{
ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
Mode: v1alpha1.ImpersonationProxyModeAuto,
ExternalEndpoint: localhostIP,
Service: v1alpha1.ImpersonationProxyServiceSpec{
Type: v1alpha1.ImpersonationProxyServiceTypeNone,
},
TLS: &v1alpha1.ImpersonationProxyTLSSpec{
SecretName: externallyProvidedTLSSecretName,
},
},
},
}, pinnipedInformerClient, pinnipedAPIClient)
})
it("returns an error", func() {
startInformersAndController()
r.Error(runControllerSync(), "could not load the externally provided TLS secret for the impersonation proxy: unable to read provided ca.crt: data is not a certificate")
r.Len(kubeAPIClient.Actions(), 1)
requireNodesListed(kubeAPIClient.Actions()[0])
requireCredentialIssuer(newErrorStrategy("could not load the externally provided TLS secret for the impersonation proxy: unable to read provided ca.crt: data is not a certificate"))
requireMTLSClientCertProviderHasLoadedCerts([]byte{}, []byte{})
})
})
when("the externally provided TLS secret does not have a ca.crt field", func() {
it.Before(func() {
addSecretToTrackers(mTLSClientCertCASecret, kubeInformerClient)
@@ -1332,7 +1469,6 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
},
},
}, pinnipedInformerClient, pinnipedAPIClient)
addNodeWithRoleToTracker("worker", kubeAPIClient)
})
it("will advertise an empty CA bundle", func() {