Fix #1582 by not double-decoding the ca.crt field in external TLS secrets for the impersonation proxy

This commit is contained in:
Joshua Casey 2023-08-08 20:17:21 -05:00
parent f24f82b25b
commit 1707995378
4 changed files with 101 additions and 46 deletions

View File

@ -730,24 +730,18 @@ func (c *impersonatorConfigController) readExternalTLSSecret(externalTLSSecretNa
return nil, err
}
base64EncodedCaCert := secretFromInformer.Data[caCrtKey]
if caCertPEM, ok := secretFromInformer.Data[caCrtKey]; ok && len(caCertPEM) > 0 {
plog.Info(fmt.Sprintf("found a %s field in the externally provided TLS secret for the impersonation proxy", caCrtKey),
"secretName", externalTLSSecretName,
"caCertPEM", caCertPEM)
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)
block, _ := pem.Decode(caCertPEM)
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 caCertPEM, nil
}
return nil, nil

View File

@ -1356,7 +1356,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
when("the externally provided TLS secret has a ca.crt field", func() {
it.Before(func() {
addSecretToTrackers(mTLSClientCertCASecret, kubeInformerClient)
externalTLSSecret.Data["ca.crt"] = []byte(base64.StdEncoding.EncodeToString(externalCA.Bundle()))
externalTLSSecret.Data["ca.crt"] = externalCA.Bundle()
addSecretToTrackers(externalTLSSecret, kubeInformerClient)
addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
@ -1386,42 +1386,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
})
})
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")))
externalTLSSecret.Data["ca.crt"] = []byte("hello")
addSecretToTrackers(externalTLSSecret, kubeInformerClient)
addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},

View File

@ -1793,6 +1793,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
"external-tls-cert-secret-name",
corev1.SecretTypeTLS,
map[string]string{
"ca.crt": string(externallyProvidedTLSServingCertPEM),
v1.TLSCertKey: string(externallyProvidedTLSServingCertPEM),
v1.TLSPrivateKeyKey: string(externallyProvidedTLSServingKeyPEM),
})
@ -1847,6 +1848,76 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
}, 2*time.Minute, 500*time.Millisecond)
})
t.Run("using externally provided TLS serving cert with byte arrays", func(t *testing.T) {
var externallyProvidedCA *certauthority.CA
externallyProvidedCA, err = certauthority.New("Impersonation Proxy Integration Test CA", 1*time.Hour)
require.NoError(t, err)
var externallyProvidedTLSServingCertPEM, externallyProvidedTLSServingKeyPEM []byte
externallyProvidedTLSServingCertPEM, externallyProvidedTLSServingKeyPEM, err = externallyProvidedCA.IssueServerCertPEM([]string{proxyServiceEndpoint}, nil, 1*time.Hour)
require.NoError(t, err)
externallyProvidedTLSServingCertSecret := testlib.CreateTestSecretBytes(
t,
env.ConciergeNamespace,
"external-tls-cert-secret-name-integration-tests",
corev1.SecretTypeTLS,
map[string][]byte{
"ca.crt": externallyProvidedCA.Bundle(),
v1.TLSCertKey: externallyProvidedTLSServingCertPEM,
v1.TLSPrivateKeyKey: externallyProvidedTLSServingKeyPEM,
})
_, originalInternallyGeneratedCAPEM := performImpersonatorDiscoveryURL(ctx, t, env, adminConciergeClient)
t.Cleanup(func() {
// Remove the TLS block from the CredentialIssuer, which should revert the ImpersonationProxy to using an
// internally generated TLS serving cert derived from the original CA.
updateCredentialIssuer(ctx, t, env, adminConciergeClient, conciergev1alpha.CredentialIssuerSpec{
ImpersonationProxy: &conciergev1alpha.ImpersonationProxySpec{
Mode: conciergev1alpha.ImpersonationProxyModeEnabled,
ExternalEndpoint: proxyServiceEndpoint,
Service: conciergev1alpha.ImpersonationProxyServiceSpec{
Type: conciergev1alpha.ImpersonationProxyServiceTypeClusterIP,
},
},
})
// Wait for the CredentialIssuer's impersonation proxy frontend strategy to be updated to the original CA bundle
testlib.RequireEventuallyWithoutError(t, func() (bool, error) {
_, impersonationProxyCACertPEM = performImpersonatorDiscoveryURL(ctx, t, env, adminConciergeClient)
return bytes.Equal(impersonationProxyCACertPEM, originalInternallyGeneratedCAPEM), nil
}, 2*time.Minute, 500*time.Millisecond)
})
updateCredentialIssuer(ctx, t, env, adminConciergeClient, conciergev1alpha.CredentialIssuerSpec{
ImpersonationProxy: &conciergev1alpha.ImpersonationProxySpec{
Mode: conciergev1alpha.ImpersonationProxyModeEnabled,
ExternalEndpoint: proxyServiceEndpoint,
Service: conciergev1alpha.ImpersonationProxyServiceSpec{
Type: conciergev1alpha.ImpersonationProxyServiceTypeClusterIP,
},
TLS: &conciergev1alpha.ImpersonationProxyTLSSpec{
CertificateAuthorityData: base64.StdEncoding.EncodeToString(externallyProvidedCA.Bundle()),
SecretName: externallyProvidedTLSServingCertSecret.Name,
},
},
})
// Wait for the CredentialIssuer's impersonation proxy frontend strategy to be updated with the right CA bundle
testlib.RequireEventuallyWithoutError(t, func() (bool, error) {
_, impersonationProxyCACertPEM = performImpersonatorDiscoveryURL(ctx, t, env, adminConciergeClient)
return bytes.Equal(impersonationProxyCACertPEM, externallyProvidedCA.Bundle()), nil
}, 2*time.Minute, 500*time.Millisecond)
// Do a login via performImpersonatorDiscovery
testlib.RequireEventuallyWithoutError(t, func() (bool, error) {
_, newImpersonationProxyCACertPEM := performImpersonatorDiscovery(ctx, t, env, adminClient, adminConciergeClient, refreshCredential)
return bytes.Equal(newImpersonationProxyCACertPEM, externallyProvidedCA.Bundle()), err
}, 2*time.Minute, 500*time.Millisecond)
})
t.Run("manually disabling the impersonation proxy feature", func(t *testing.T) {
// Update configuration to force the proxy to disabled mode
updateCredentialIssuer(ctx, t, env, adminConciergeClient, conciergev1alpha.CredentialIssuerSpec{

View File

@ -364,6 +364,28 @@ func CreateTestSecret(t *testing.T, namespace string, baseName string, secretTyp
return created
}
func CreateTestSecretBytes(t *testing.T, namespace string, baseName string, secretType corev1.SecretType, data map[string][]byte) *corev1.Secret {
t.Helper()
client := NewKubernetesClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
created, err := client.CoreV1().Secrets(namespace).Create(ctx, &corev1.Secret{
ObjectMeta: testObjectMeta(t, baseName),
Type: secretType,
Data: data,
}, metav1.CreateOptions{})
require.NoError(t, err)
t.Cleanup(func() {
t.Logf("cleaning up test Secret %s/%s", created.Namespace, created.Name)
err := client.CoreV1().Secrets(namespace).Delete(context.Background(), created.Name, metav1.DeleteOptions{})
require.NoError(t, err)
})
t.Logf("created test Secret %s", created.Name)
return created
}
func CreateClientCredsSecret(t *testing.T, clientID string, clientSecret string) *corev1.Secret {
t.Helper()
env := IntegrationEnv(t)