diff --git a/controller/gateway/controller_reconciler_utils.go b/controller/gateway/controller_reconciler_utils.go index 8551b7be1..6a975d94d 100644 --- a/controller/gateway/controller_reconciler_utils.go +++ b/controller/gateway/controller_reconciler_utils.go @@ -24,6 +24,7 @@ import ( gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1" + "github.com/kong/gateway-operator/controller/pkg/secrets" operatorerrors "github.com/kong/gateway-operator/internal/errors" gwtypes "github.com/kong/gateway-operator/internal/types" "github.com/kong/gateway-operator/internal/utils/gatewayclass" @@ -875,17 +876,32 @@ func getSupportedKindsWithResolvedRefsCondition(ctx context.Context, c client.Cl secretNamespace = string(*certificateRef.Namespace) } + isReferenceGranted := true + // In case there is a cross-namespace reference, check if there is any referenceGrant allowing it. + if secretNamespace != gatewayNamespace { + referenceGrantList := &gatewayv1beta1.ReferenceGrantList{} + err = c.List(ctx, referenceGrantList, client.InNamespace(secretNamespace)) + if err != nil { + return nil, metav1.Condition{}, fmt.Errorf("failed to list ReferenceGrants: %w", err) + } + if !isSecretCrossReferenceGranted(gatewayv1.Namespace(gatewayNamespace), certificateRef.Name, referenceGrantList.Items) { + resolvedRefsCondition.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) + message = conditionMessage(message, fmt.Sprintf("Secret %s/%s reference not allowed by any referenceGrant", secretNamespace, certificateRef.Name)) + isReferenceGranted = false + } + } + var secretExists bool - if isValidGroupKind { + certificateSecret := &corev1.Secret{} + if isValidGroupKind && isReferenceGranted { // Get the secret and check it exists. - certificateSecret := &corev1.Secret{} err = c.Get(ctx, types.NamespacedName{ Namespace: secretNamespace, Name: string(certificateRef.Name), }, certificateSecret) if err != nil { if !k8serrors.IsNotFound(err) { - return + return nil, metav1.Condition{}, fmt.Errorf("failed to get Secret: %w", err) } resolvedRefsCondition.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) message = conditionMessage(message, fmt.Sprintf("Referenced secret %s/%s does not exist", secretNamespace, certificateRef.Name)) @@ -894,18 +910,11 @@ func getSupportedKindsWithResolvedRefsCondition(ctx context.Context, c client.Cl } } - if secretExists { - // In case there is a cross-namespace reference, check if there is any referenceGrant allowing it. - if secretNamespace != gatewayNamespace { - referenceGrantList := &gatewayv1beta1.ReferenceGrantList{} - err = c.List(ctx, referenceGrantList, client.InNamespace(secretNamespace)) - if err != nil { - return - } - if !isSecretCrossReferenceGranted(gatewayv1.Namespace(gatewayNamespace), certificateRef.Name, referenceGrantList.Items) { - resolvedRefsCondition.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) - message = conditionMessage(message, fmt.Sprintf("Secret %s/%s reference not allowed by any referenceGrant", secretNamespace, certificateRef.Name)) - } + if isReferenceGranted && secretExists { + // Check if the secret is a valid TLS secret. + if !secrets.IsTLSSecretValid(certificateSecret) { + resolvedRefsCondition.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) + message = conditionMessage(message, "Referenced secret does not contain a valid TLS certificate") } } } diff --git a/controller/gateway/controller_reconciler_utils_test.go b/controller/gateway/controller_reconciler_utils_test.go index 47528c611..e5194523b 100644 --- a/controller/gateway/controller_reconciler_utils_test.go +++ b/controller/gateway/controller_reconciler_utils_test.go @@ -21,6 +21,7 @@ import ( "github.com/kong/gateway-operator/modules/manager/scheme" "github.com/kong/gateway-operator/pkg/consts" k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes" + "github.com/kong/gateway-operator/test/helpers" ) func TestParseKongProxyListenEnv(t *testing.T) { @@ -856,6 +857,7 @@ func TestGatewayStatusNeedsUpdate(t *testing.T) { func TestGetSupportedKindsWithResolvedRefsCondition(t *testing.T) { var generation int64 = 1 + ca := helpers.CreateCA(t) testCases := []struct { name string @@ -950,6 +952,10 @@ func TestGetSupportedKindsWithResolvedRefsCondition(t *testing.T) { Name: "test-secret", Namespace: "default", }, + Data: map[string][]byte{ + "tls.crt": ca.CertPEM.Bytes(), + "tls.key": ca.KeyPEM.Bytes(), + }, }, }, expectedSupportedKinds: []gwtypes.RouteGroupKind{ @@ -986,6 +992,10 @@ func TestGetSupportedKindsWithResolvedRefsCondition(t *testing.T) { Name: "test-secret", Namespace: "default", }, + Data: map[string][]byte{ + "tls.crt": ca.CertPEM.Bytes(), + "tls.key": ca.KeyPEM.Bytes(), + }, }, }, expectedSupportedKinds: []gwtypes.RouteGroupKind{ @@ -1091,6 +1101,46 @@ func TestGetSupportedKindsWithResolvedRefsCondition(t *testing.T) { ObservedGeneration: generation, }, }, + { + name: "tls bad-formed, invalid cert and key", + gatewayNamespace: "default", + listener: gwtypes.Listener{ + Protocol: gatewayv1.HTTPSProtocolType, + TLS: &gatewayv1.GatewayTLSConfig{ + Mode: lo.ToPtr(gatewayv1.TLSModeTerminate), + CertificateRefs: []gatewayv1.SecretObjectReference{ + { + Name: "test-secret", + }, + }, + }, + }, + secrets: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "tls.crt": []byte("invalid-cert"), + "tls.key": []byte("invalid-key"), + }, + }, + }, + expectedSupportedKinds: []gwtypes.RouteGroupKind{ + { + Group: (*gwtypes.Group)(&gatewayv1.GroupVersion.Group), + Kind: "HTTPRoute", + }, + }, + expectedResolvedRefsCondition: metav1.Condition{ + Type: string(gatewayv1.ListenerConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(gatewayv1.ListenerReasonInvalidCertificateRef), + Message: "Referenced secret does not contain a valid TLS certificate.", + ObservedGeneration: generation, + }, + }, { name: "tls well-formed, with allowed cross-namespace reference", gatewayNamespace: "default", @@ -1112,6 +1162,10 @@ func TestGetSupportedKindsWithResolvedRefsCondition(t *testing.T) { Name: "test-secret", Namespace: "other-namespace", }, + Data: map[string][]byte{ + "tls.crt": ca.CertPEM.Bytes(), + "tls.key": ca.KeyPEM.Bytes(), + }, }, }, referenceGrants: []client.Object{ @@ -1172,6 +1226,10 @@ func TestGetSupportedKindsWithResolvedRefsCondition(t *testing.T) { Name: "test-secret", Namespace: "other-namespace", }, + Data: map[string][]byte{ + "tls.crt": ca.CertPEM.Bytes(), + "tls.key": ca.KeyPEM.Bytes(), + }, }, }, expectedSupportedKinds: []gwtypes.RouteGroupKind{ diff --git a/controller/pkg/secrets/cert.go b/controller/pkg/secrets/cert.go index 120aaf236..32db56899 100644 --- a/controller/pkg/secrets/cert.go +++ b/controller/pkg/secrets/cert.go @@ -147,6 +147,25 @@ func signCertificate(csr certificatesv1.CertificateSigningRequest, ca *corev1.Se return certBytes, nil } +// IsTLSSecretValid checks if a Secret contains a valid TLS certificate and key. +func IsTLSSecretValid(secret *corev1.Secret) bool { + var ok bool + var crt, key []byte + if crt, ok = secret.Data["tls.crt"]; !ok { + return false + } + if key, ok = secret.Data["tls.key"]; !ok { + return false + } + if p, _ := pem.Decode(crt); p == nil { + return false + } + if p, _ := pem.Decode(key); p == nil { + return false + } + return true +} + // EnsureCertificate creates a namespace/name Secret for subject signed by the CA in the // mtlsCASecretNamespace/mtlsCASecretName Secret, or does nothing if a namespace/name Secret is // already present. It returns a boolean indicating if it created a Secret and an error indicating diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index c0204630e..f283065c7 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -27,18 +27,12 @@ import ( ) var skippedTestsForExpressionsRouter = []string{ - // gateway - tests.GatewayInvalidTLSConfiguration.ShortName, - // TODO: remove the skip https://github.com/Kong/gateway-operator/issues/295 // This test is flaky. tests.HTTPRouteWeight.ShortName, } var skippedTestsForTraditionalCompatibleRouter = []string{ - // gateway - tests.GatewayInvalidTLSConfiguration.ShortName, - // httproute tests.HTTPRouteHeaderMatching.ShortName, tests.HTTPRouteInvalidBackendRefUnknownKind.ShortName,