Merge pull request #327 from stlaz/e2e_cryptorefactor

e2e: refactor TLS helpers
brancz · Dec 9, 2024 · ffd3bdd · ffd3bdd
2 parents ab5cad1 + 48dfc38
commit ffd3bdd
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 109 deletions.
diff --git a/test/e2e/identityheaders.go b/test/e2e/identityheaders.go
@@ -81,13 +81,24 @@ func testIdentityHeaders(client kubernetes.Interface) kubetest.TestSuite {
 					"identityheaders/insecure/clusterRole.yaml",
 					"identityheaders/insecure/clusterRoleBinding-client.yaml",
 					"identityheaders/insecure/clusterRoleBinding.yaml",
-					"identityheaders/insecure/deployment-proxy.yaml",
 					"identityheaders/insecure/deployment-upstream.yaml",
-					"identityheaders/insecure/service-proxy.yaml",
 					"identityheaders/insecure/service-upstream.yaml",
+					"identityheaders/insecure/deployment-proxy.yaml",
+					"identityheaders/insecure/service-proxy.yaml",
 					"identityheaders/insecure/serviceAccount.yaml",
 				),
 			),
+			When: kubetest.Actions(
+				kubetest.PodsAreReady(
+					client,
+					1,
+					"app=nginx",
+				),
+				kubetest.ServiceIsReady(
+					client,
+					"nginx",
+				),
+			),
 			Then: kubetest.Actions(
 				kubetest.PodIsCrashLoopBackOff(
 					client,
@@ -102,7 +113,6 @@ func testIdentityHeaders(client kubernetes.Interface) kubetest.TestSuite {
 					Verifies that the proxy is able to connect to the remote upstream service,
 					through a mTLS connection, when providing identity headers.
 				`,
-
 			Given: kubetest.Actions(
 				kubetest.CreateServerCerts(client, "nginx"),
 				kubetest.CreateClientCerts(client, "kube-rbac-proxy-client"),
@@ -114,13 +124,29 @@ func testIdentityHeaders(client kubernetes.Interface) kubetest.TestSuite {
 					"identityheaders/secure/clusterRoleBinding-client.yaml",
 					"identityheaders/secure/clusterRoleBinding.yaml",
 					"identityheaders/secure/configmap-nginx.yaml",
-					"identityheaders/secure/deployment-proxy.yaml",
 					"identityheaders/secure/deployment-upstream.yaml",
-					"identityheaders/secure/service-proxy.yaml",
 					"identityheaders/secure/service-upstream.yaml",
+					"identityheaders/secure/deployment-proxy.yaml",
+					"identityheaders/secure/service-proxy.yaml",
 					"identityheaders/secure/serviceAccount.yaml",
 				),
 			),
+			When: kubetest.Actions(
+				kubetest.PodsAreReady(
+					client,
+					1,
+					"app=nginx",
+				),
+				kubetest.ServiceIsReady(
+					client,
+					"nginx",
+				),
+				kubetest.PodsAreReady(
+					client,
+					1,
+					"app=kube-rbac-proxy",
+				),
+			),
 			Then: kubetest.Actions(
 				kubetest.ClientSucceeds(
 					client,

diff --git a/test/e2e/identityheaders/secure/configmap-nginx.yaml b/test/e2e/identityheaders/secure/configmap-nginx.yaml
@@ -9,10 +9,12 @@ data:
         server_name upstream-nginx;
 
         ssl_certificate /etc/nginx/certs/tls.crt;
-        ssl_certificate_key /etc/nginx/keys/tls.key;
+        ssl_certificate_key /etc/nginx/certs/tls.key;
         ssl_client_certificate /etc/nginx/client-ca/ca.crt;
         ssl_verify_client on;
 
+        error_log /dev/stderr debug;
+
         location /metrics {
             add_header Content-Type text/plain;
             return 200 'metrics endpoint reached\n';

diff --git a/test/e2e/identityheaders/secure/deployment-proxy.yaml b/test/e2e/identityheaders/secure/deployment-proxy.yaml
@@ -25,23 +25,17 @@ spec:
             - "--auth-header-groups-field-name=x-remote-groups"
             - "--auth-header-groups-field-separator=|"
             - "--tls-cert-file=/usr/local/etc/kube-rbac-proxy/server-certs/tls.crt"
-            - "--tls-private-key-file=/usr/local/etc/kube-rbac-proxy/server-keys/tls.key"
-            - "--upstream-ca-file=/usr/local/etc/kube-rbac-proxy/upstream-certs/ca.crt"
+            - "--tls-private-key-file=/usr/local/etc/kube-rbac-proxy/server-certs/tls.key"
+            - "--upstream-ca-file=/usr/local/etc/kube-rbac-proxy/upstream-trust/ca.crt"
             - "--upstream-client-cert-file=/usr/local/etc/kube-rbac-proxy/client-certs/tls.crt"
-            - "--upstream-client-key-file=/usr/local/etc/kube-rbac-proxy/client-keys/tls.key"
+            - "--upstream-client-key-file=/usr/local/etc/kube-rbac-proxy/client-certs/tls.key"
             - "--v=10"
           volumeMounts:
-            - name: kube-rbac-proxy-client-keys
-              mountPath: /usr/local/etc/kube-rbac-proxy/client-keys
-              readOnly: true
             - name: kube-rbac-proxy-client-certs
               mountPath: /usr/local/etc/kube-rbac-proxy/client-certs
               readOnly: true
-            - name: nginx-certs
-              mountPath: /usr/local/etc/kube-rbac-proxy/upstream-certs
-              readOnly: true
-            - name: kube-rbac-proxy-keys
-              mountPath: /usr/local/etc/kube-rbac-proxy/server-keys
+            - name: nginx-trust
+              mountPath: /usr/local/etc/kube-rbac-proxy/upstream-trust
               readOnly: true
             - name: kube-rbac-proxy-certs
               mountPath: /usr/local/etc/kube-rbac-proxy/server-certs
@@ -50,18 +44,12 @@ spec:
             - containerPort: 8443
               name: https
       volumes:
-        - name: kube-rbac-proxy-client-keys
-          secret:
-            secretName: kube-rbac-proxy-client-keys
         - name: kube-rbac-proxy-client-certs
-          configMap:
-            name: kube-rbac-proxy-client-certs
-        - name: nginx-certs
-          configMap:
-            name: nginx-certs
-        - name: kube-rbac-proxy-keys
           secret:
-            secretName: kube-rbac-proxy-keys
-        - name: kube-rbac-proxy-certs
+            secretName: kube-rbac-proxy-client-certs
+        - name: nginx-trust
           configMap:
-            name: kube-rbac-proxy-certs
+            name: nginx-trust
+        - name: kube-rbac-proxy-certs
+          secret:
+            secretName: kube-rbac-proxy-certs
diff --git a/test/e2e/identityheaders/secure/deployment-upstream.yaml b/test/e2e/identityheaders/secure/deployment-upstream.yaml
@@ -24,10 +24,7 @@ spec:
         - name: nginx-certs
           mountPath: "/etc/nginx/certs"
           readOnly: true
-        - name: nginx-keys
-          mountPath: "/etc/nginx/keys"
-          readOnly: true
-        - name: kube-rbac-proxy-client-certs
+        - name: kube-rbac-proxy-client-trust
           mountPath: "/etc/nginx/client-ca"
           readOnly: true
         - name: nginx-config
@@ -38,11 +35,8 @@ spec:
         configMap:
           name: nginx-config
       - name: nginx-certs
-        configMap:
-          name: nginx-certs
-      - name: nginx-keys
         secret:
-          secretName: nginx-keys
-      - name: kube-rbac-proxy-client-certs
+          secretName: nginx-certs
+      - name: kube-rbac-proxy-client-trust
         configMap:
-          name: kube-rbac-proxy-client-certs
+          name: kube-rbac-proxy-client-trust
diff --git a/test/kubetest/kubernetes.go b/test/kubetest/kubernetes.go
@@ -46,12 +46,7 @@ func CreateClientCerts(client kubernetes.Interface, name string) Action {
 	return createCerts(client, name, createSignedClientCert)
 }
 
-// Panic panics. It is used to stop the tests after setting up the manfiests.
-func Panic(_ *ScenarioContext) error {
-	panic("happy debugging")
-}
-
-func createCerts(client kubernetes.Interface, name string, createSignedCert certer) Action {
+func createCerts(client kubernetes.Interface, name string, createSignedCert createCertsFunc) Action {
 	return func(ctx *ScenarioContext) error {
 		caCert, caKey, err := createSelfSignedCA(fmt.Sprintf("%s-ca", name))
 		if err != nil {
@@ -62,14 +57,13 @@ func createCerts(client kubernetes.Interface, name string, createSignedCert cert
 			return err
 		}
 
-		configMapName := fmt.Sprintf("%s-certs", name)
+		configMapName := fmt.Sprintf("%s-trust", name)
 		_, err = client.CoreV1().ConfigMaps(ctx.Namespace).Create(context.TODO(), &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: configMapName,
 			},
 			Data: map[string]string{
-				"ca.crt":  string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caCert.Raw})),
-				"tls.crt": string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})),
+				"ca.crt": string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caCert.Raw})),
 			},
 		}, metav1.CreateOptions{})
 		if err != nil {
@@ -79,20 +73,20 @@ func createCerts(client kubernetes.Interface, name string, createSignedCert cert
 			return client.CoreV1().ConfigMaps(ctx.Namespace).Delete(context.TODO(), configMapName, metav1.DeleteOptions{})
 		})
 
-		secretName := fmt.Sprintf("%s-keys", name)
+		secretName := fmt.Sprintf("%s-certs", name)
 		_, err = client.CoreV1().Secrets(ctx.Namespace).Create(context.TODO(), &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: secretName,
 			},
 			Data: map[string][]byte{
+				"tls.crt": pem.EncodeToMemory(&pem.Block{
+					Type:  "CERTIFICATE",
+					Bytes: cert.Raw,
+				}),
 				"tls.key": []byte(pem.EncodeToMemory(&pem.Block{
 					Type:  "RSA PRIVATE KEY",
 					Bytes: x509.MarshalPKCS1PrivateKey(key),
 				})),
-				"ca.key": []byte(pem.EncodeToMemory(&pem.Block{
-					Type:  "RSA PRIVATE KEY",
-					Bytes: x509.MarshalPKCS1PrivateKey(caKey),
-				})),
 			},
 		}, metav1.CreateOptions{})
 		ctx.AddCleanUp(func() error {

diff --git a/test/kubetest/tls.go b/test/kubetest/tls.go
@@ -18,7 +18,6 @@ package kubetest
 import (
 	"crypto/rand"
 	"crypto/rsa"
-	"crypto/sha1"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"fmt"
@@ -33,7 +32,7 @@ var (
 	minimalRSAKeySize = 2048
 )
 
-type certer func(*x509.Certificate, *rsa.PrivateKey, string) (*x509.Certificate, *rsa.PrivateKey, error)
+type createCertsFunc func(*x509.Certificate, *rsa.PrivateKey, string) (*x509.Certificate, *rsa.PrivateKey, error)
 
 func createSignedClientCert(cacert *x509.Certificate, caPrivateKey *rsa.PrivateKey, name string) (*x509.Certificate, *rsa.PrivateKey, error) {
 	// Generate a private key.
@@ -42,34 +41,12 @@ func createSignedClientCert(cacert *x509.Certificate, caPrivateKey *rsa.PrivateK
 		return nil, nil, err
 	}
 
-	// Generate subject key id.
-	subjectKeyID := sha1.Sum(privateKey.PublicKey.N.Bytes())
-	authorityKeyID := cacert.SubjectKeyId
-
-	// Generate serial number with at least 20 bits of entropy.
-	serialNumber, err := generateSerialNumber()
+	template, err := certTemplate()
 	if err != nil {
-		return nil, nil, err
-	}
-
-	// Create certificate template.
-	template := &x509.Certificate{
-		Subject: pkix.Name{CommonName: name},
-
-		NotBefore: time.Now().Add(-1 * time.Second),
-		NotAfter:  time.Now().Add(year),
-
-		SerialNumber:   serialNumber,
-		SubjectKeyId:   subjectKeyID[:],
-		AuthorityKeyId: authorityKeyID,
-
-		SignatureAlgorithm: x509.SHA256WithRSA,
-
-		KeyUsage:    x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
-		ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
-
-		BasicConstraintsValid: true,
+		return nil, nil, fmt.Errorf("failed to generate cert template: %v", err)
 	}
+	template.Subject = pkix.Name{CommonName: name}
+	template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}
 
 	// Sign Certificate
 	derBytes, err := x509.CreateCertificate(
@@ -102,36 +79,13 @@ func createSignedServerCert(caCert *x509.Certificate, caPrivateKey *rsa.PrivateK
 		return nil, nil, err
 	}
 
-	// Generate subject key id.
-	subjectKeyID := sha1.Sum(privateKey.PublicKey.N.Bytes())
-	authorityKeyID := caCert.SubjectKeyId
-
-	// Generate serial number with at least 20 bits of entropy.
-	serialNumber, err := generateSerialNumber()
+	template, err := certTemplate()
 	if err != nil {
-		return nil, nil, err
-	}
-
-	// Create certificate template.
-	template := &x509.Certificate{
-		Subject: pkix.Name{CommonName: dnsName},
-
-		NotBefore: time.Now().Add(-1 * time.Second),
-		NotAfter:  time.Now().Add(year),
-
-		SerialNumber:   serialNumber,
-		SubjectKeyId:   subjectKeyID[:],
-		AuthorityKeyId: authorityKeyID,
-
-		SignatureAlgorithm: x509.SHA256WithRSA,
-
-		KeyUsage:    x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
-		ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-
-		DNSNames: []string{dnsName},
-
-		BasicConstraintsValid: true,
+		return nil, nil, fmt.Errorf("failed to generate cert template: %v", err)
 	}
+	template.Subject = pkix.Name{CommonName: dnsName}
+	template.DNSNames = []string{dnsName}
+	template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}
 
 	// Sign Certificate
 	derBytes, err := x509.CreateCertificate(
@@ -157,6 +111,30 @@ func createSignedServerCert(caCert *x509.Certificate, caPrivateKey *rsa.PrivateK
 	return certs[0], privateKey, nil
 }
 
+// certTemplate creates a basic cert template to use in tests. The caller must
+// add its own Subject and any extensions specific to their use.
+func certTemplate() (*x509.Certificate, error) {
+	// Generate serial number with at least 20 bits of entropy.
+	serialNumber, err := generateSerialNumber()
+	if err != nil {
+		return nil, err
+	}
+
+	return &x509.Certificate{
+		NotBefore: time.Now().Add(-1 * time.Second),
+		NotAfter:  time.Now().Add(year),
+
+		SerialNumber: serialNumber,
+
+		SignatureAlgorithm: x509.SHA256WithRSA,
+
+		KeyUsage:    x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+
+		BasicConstraintsValid: true,
+	}, nil
+}
+
 func createSelfSignedCA(name string) (*x509.Certificate, *rsa.PrivateKey, error) {
 	privateKey, err := rsa.GenerateKey(rand.Reader, minimalRSAKeySize)
 	if err != nil {