test: GatewayInvalidTLSConfiguration conformance (#338)

* test: GatewayInvalidTLSConfiguration conformance Signed-off-by: Mattia Lavacca <[email protected]> * chore: explictly return values Signed-off-by: Mattia Lavacca <[email protected]> --------- Signed-off-by: Mattia Lavacca <[email protected]>
Kong · Jun 17, 2024 · e268b8b · e268b8b
1 parent 3a55bad
commit e268b8b
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,