diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index f94cc70f8..c956a9635 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -598,7 +598,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", - Message: "spec.tls.certificateAuthorityData is invalid: no certificates found", + Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`, ObservedGeneration: 1234, }, }, diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index 15ca58aaf..1486e4cfd 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -1394,7 +1394,7 @@ func TestController(t *testing.T) { buildGitHubConnectionValidUnknown(t), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), - buildTLSConfigurationValidFalse(t, "spec.githubAPI.tls.certificateAuthorityData is invalid: no certificates found"), + buildTLSConfigurationValidFalse(t, `spec.githubAPI.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 4 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`), }, }, }, @@ -1404,7 +1404,7 @@ func TestController(t *testing.T) { buildLogForUpdatingClaimsValidTrue("some-idp-name"), buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), - buildLogForUpdatingTLSConfigurationValid("some-idp-name", "False", "InvalidTLSConfig", "spec.githubAPI.tls.certificateAuthorityData is invalid: no certificates found"), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "False", "InvalidTLSConfig", `spec.githubAPI.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 4 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")`), buildLogForUpdatingGitHubConnectionValidUnknown("some-idp-name"), buildLogForUpdatingPhase("some-idp-name", "Error"), }, diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index 3dde9d235..77269be21 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -527,7 +527,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", - Message: "spec.tls.certificateAuthorityData is invalid: no certificates found", + Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`, ObservedGeneration: 1234, }, }, diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index ed5968185..f63d01e28 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -431,11 +431,11 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"ClientCredentialsSecretValid","status":"True","reason":"Success","message":"loaded client credentials"}`, - `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found"}`, - `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found"}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")"}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","status":"False","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")"}`, `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"test-namespace","name":"test-name","type":"AdditionalAuthorizeParametersValid","status":"True","reason":"Success","message":"additionalAuthorizeParameters parameter names are allowed"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found","error":"OIDCIdentityProvider has a failing condition"}`, - `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no certificates found","error":"OIDCIdentityProvider has a failing condition"}`, + `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"OIDCDiscoverySucceeded","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")","error":"OIDCIdentityProvider has a failing condition"}`, + `{"level":"error","timestamp":"2099-08-08T13:57:36.123456Z","logger":"oidc-upstream-observer","caller":"oidcupstreamwatcher/oidc_upstream_watcher.go:$oidcupstreamwatcher.(*oidcWatcherController).validateUpstream","message":"found failing condition","namespace":"test-namespace","name":"test-name","type":"TLSConfigurationValid","reason":"InvalidTLSConfig","message":"spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")","error":"OIDCIdentityProvider has a failing condition"}`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []idpv1alpha1.OIDCIdentityProvider{{ @@ -447,9 +447,9 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { {Type: "ClientCredentialsSecretValid", Status: "True", LastTransitionTime: now, Reason: "Success", Message: "loaded client credentials"}, {Type: "OIDCDiscoverySucceeded", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", - Message: `spec.tls.certificateAuthorityData is invalid: no certificates found`}, + Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`}, {Type: "TLSConfigurationValid", Status: "False", LastTransitionTime: now, Reason: "InvalidTLSConfig", - Message: "spec.tls.certificateAuthorityData is invalid: no certificates found"}, + Message: `spec.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 28 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`}, }, }, }}, diff --git a/internal/controller/tlsconfigutil/tls_config_util.go b/internal/controller/tlsconfigutil/tls_config_util.go index 14d1e26dd..f41bb9d7c 100644 --- a/internal/controller/tlsconfigutil/tls_config_util.go +++ b/internal/controller/tlsconfigutil/tls_config_util.go @@ -7,6 +7,7 @@ import ( "crypto/x509" "encoding/base64" "fmt" + "strings" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -83,7 +84,7 @@ func TLSSpecForConcierge(source *authenticationv1alpha1.TLSSpec) *TLSSpec { // or as a reference to a kubernetes secret or configmap using the CertificateAuthorityDataSource, and returns // - a condition of type TLSConfigurationValid based on the validity of the ca bundle, // - a pem encoded ca bundle -// - a X509 cert pool with the ca bundle +// - a X509 cert pool with the ca bundle. func ValidateTLSConfig( tlsSpec *TLSSpec, conditionPrefix string, @@ -134,6 +135,7 @@ func getCertPool( var err error caBundle := tlsSpec.CertificateAuthorityData + caBundleLength := len(caBundle) field := fmt.Sprintf("%s.%s", conditionPrefix, "certificateAuthorityData") // the ca data supplied inline in the CRDs is expected to be base64 encoded. // However, the ca data read from kubernetes secrets or config map will not be base64 encoded. @@ -146,6 +148,7 @@ func getCertPool( // this will be used to report in the condition status in case an invalid TLS condition is encountered. field = fmt.Sprintf("%s.%s", conditionPrefix, "certificateAuthorityDataSource") caBundle, err = readCABundleFromSource(tlsSpec.CertificateAuthorityDataSource, namespace, secretInformer, configMapInformer) + caBundleLength = len(caBundle) if err != nil { return nil, nil, fmt.Errorf("%s is invalid: %s", field, err.Error()) } @@ -167,7 +170,14 @@ func getCertPool( ca := x509.NewCertPool() ok := ca.AppendCertsFromPEM(bundleBytes) if !ok { - return nil, nil, fmt.Errorf("%s is invalid: no certificates found", field) + if decodeRequired { + return nil, nil, fmt.Errorf("%s is invalid: no base64-encoded PEM certificates found in %d bytes of data (PEM certificates must begin with \"-----BEGIN CERTIFICATE-----\")", + field, caBundleLength) + } + namespacedName := fmt.Sprintf("%s/%s", namespace, tlsSpec.CertificateAuthorityDataSource.Name) + + return nil, nil, fmt.Errorf(`%s is invalid: key %q with %d bytes of data in %s %q is not a PEM-encoded certificate (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`, + field, tlsSpec.CertificateAuthorityDataSource.Key, caBundleLength, strings.ToLower(tlsSpec.CertificateAuthorityDataSource.Kind), namespacedName) } return ca, bundleBytes, nil diff --git a/internal/controller/tlsconfigutil/tls_config_util_test.go b/internal/controller/tlsconfigutil/tls_config_util_test.go index cff871cb2..f7ba30ac3 100644 --- a/internal/controller/tlsconfigutil/tls_config_util_test.go +++ b/internal/controller/tlsconfigutil/tls_config_util_test.go @@ -84,7 +84,7 @@ func TestValidateTLSConfig(t *testing.T) { Type: typeTLSConfigurationValid, Status: metav1.ConditionFalse, Reason: ReasonInvalidTLSConfig, - Message: "spec.foo.tls.certificateAuthorityData is invalid: no certificates found", + Message: `spec.foo.tls.certificateAuthorityData is invalid: no base64-encoded PEM certificates found in 88 bytes of data (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`, }, }, { @@ -265,6 +265,35 @@ func TestValidateTLSConfig(t *testing.T) { Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in secret "awesome-namespace/awesome-secret"`, }, }, + { + name: "should return invalid condition when a secret has the configured key but the value is not a cert", + tlsSpec: &TLSSpec{ + CertificateAuthorityDataSource: &caBundleSource{ + Kind: "Secret", + Name: "awesome-secret", + Key: "ca-bundle", + }, + }, + namespace: "awesome-namespace", + k8sObjects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-secret", + Namespace: "awesome-namespace", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "ca-bundle": []byte("this is not a certificate"), + }, + }, + }, + expectedCondition: &metav1.Condition{ + Type: typeTLSConfigurationValid, + Status: metav1.ConditionFalse, + Reason: ReasonInvalidTLSConfig, + Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" with 25 bytes of data in secret "awesome-namespace/awesome-secret" is not a PEM-encoded certificate (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`, + }, + }, { name: "should return invalid condition when a configmap does not have the configured key", tlsSpec: &TLSSpec{ @@ -321,6 +350,34 @@ func TestValidateTLSConfig(t *testing.T) { Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" has empty value in configmap "awesome-namespace/awesome-configmap"`, }, }, + { + name: "should return invalid condition when a configmap has the configured key but its value not a cert", + tlsSpec: &TLSSpec{ + CertificateAuthorityDataSource: &caBundleSource{ + Kind: "ConfigMap", + Name: "awesome-configmap", + Key: "ca-bundle", + }, + }, + namespace: "awesome-namespace", + k8sObjects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awesome-configmap", + Namespace: "awesome-namespace", + }, + Data: map[string]string{ + "ca-bundle": "this is not a cert", + }, + }, + }, + expectedCondition: &metav1.Condition{ + Type: typeTLSConfigurationValid, + Status: metav1.ConditionFalse, + Reason: ReasonInvalidTLSConfig, + Message: `spec.foo.tls.certificateAuthorityDataSource is invalid: key "ca-bundle" with 18 bytes of data in configmap "awesome-namespace/awesome-configmap" is not a PEM-encoded certificate (PEM certificates must begin with "-----BEGIN CERTIFICATE-----")`, + }, + }, { name: "should return ca bundle from kubernetes configMap", tlsSpec: &TLSSpec{