Add crl integraiton to tests (#17447)

* Add tests using client certificates Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Refactor Go TLS client tests Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add tests for CRLs Note that Delta CRL support isn't present in nginx or apache, so we lack a server-side test presently. Wget2 does appear to support it however, if we wanted to add a client-side OpenSSL test. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add checks for delta CRL with wget2 This ensures the delta CRL is properly formatted and accepted by OpenSSL. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Re-add missing test helpers Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Rename clientFullChain->clientWireChain Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
2025-10-30 02:02:43 +00:00 · 2022-11-28 10:32:22 -05:00
parent a4a23f794a
commit 48d98a8b4c
2 changed files with 290 additions and 52 deletions
--- a/builtin/logical/pkiext/nginx_test.go
+++ b/builtin/logical/pkiext/nginx_test.go
@@ -29,21 +29,24 @@ var (
 )

 const (
-	protectedFile   = `dadgarcorp-internal-protected`
-	unprotectedFile = `hello-world`
-	uniqueHostname  = `dadgarcorpvaultpkitestingnginxwgetcurlcontainersexample.com`
-	containerName   = `vault_pki_nginx_integration`
+	protectedFile    = `dadgarcorp-internal-protected`
+	unprotectedFile  = `hello-world`
+	failureIndicator = `THIS-TEST-SHOULD-FAIL`
+	uniqueHostname   = `dadgarcorpvaultpkitestingnginxwgetcurlcontainersexample.com`
+	containerName    = `vault_pki_nginx_integration`
 )

-func buildNginxContainer(t *testing.T, chain string, private string) (func(), string, int, string, string, int) {
+func buildNginxContainer(t *testing.T, root string, crl string, chain string, private string) (func(), string, int, string, string, int) {
 	containerfile := `
 FROM nginx:latest

 RUN mkdir /www /etc/nginx/ssl && rm /etc/nginx/conf.d/*.conf

 COPY testing.conf /etc/nginx/conf.d/
+COPY root.pem /etc/nginx/ssl/root.pem
 COPY fullchain.pem /etc/nginx/ssl/fullchain.pem
 COPY privkey.pem /etc/nginx/ssl/privkey.pem
+COPY crl.pem /etc/nginx/ssl/crl.pem
 COPY /data /www/data
 `

@@ -64,7 +67,26 @@ server {
 	ssl_certificate /etc/nginx/ssl/fullchain.pem;
 	ssl_certificate_key /etc/nginx/ssl/privkey.pem;

-    location / {
+	ssl_client_certificate /etc/nginx/ssl/root.pem;
+	ssl_crl /etc/nginx/ssl/crl.pem;
+	ssl_verify_client optional;
+
+	# Magic per: https://serverfault.com/questions/891603/nginx-reverse-proxy-with-optional-ssl-client-authentication
+	# Only necessary since we're too lazy to setup two different subdomains.
+	set $ssl_status 'open';
+	if ($request_uri ~ protected) {
+		set $ssl_status 'closed';
+	}
+
+	if ($ssl_client_verify != SUCCESS) {
+		set $ssl_status "$ssl_status-fail";
+	}
+
+	if ($ssl_status = "closed-fail") {
+		return 403;
+	}
+
+	location / {
 		root /www/data;
    }
 }
@@ -72,8 +94,10 @@ server {

 	bCtx := docker.NewBuildContext()
 	bCtx["testing.conf"] = docker.PathContentsFromString(siteConfig)
+	bCtx["root.pem"] = docker.PathContentsFromString(root)
 	bCtx["fullchain.pem"] = docker.PathContentsFromString(chain)
 	bCtx["privkey.pem"] = docker.PathContentsFromString(private)
+	bCtx["crl.pem"] = docker.PathContentsFromString(crl)
 	bCtx["/data/index.html"] = docker.PathContentsFromString(unprotectedFile)
 	bCtx["/data/protected.html"] = docker.PathContentsFromString(protectedFile)

@@ -152,7 +176,7 @@ func buildWgetCurlContainer(t *testing.T, network string) {
 	containerfile := `
 FROM ubuntu:latest

-RUN apt update && DEBIAN_FRONTEND="noninteractive" apt install -y curl wget
+RUN apt update && DEBIAN_FRONTEND="noninteractive" apt install -y curl wget wget2
 `

 	bCtx := docker.NewBuildContext()
@@ -207,7 +231,7 @@ func CheckWithClients(t *testing.T, network string, address string, url string,
 	ctx := context.Background()
 	ctr, _, _, err := cwRunner.Start(ctx, true, false)
 	if err != nil {
-		t.Fatalf("Could not start golang container for zlint: %s", err)
+		t.Fatalf("Could not start golang container for wget/curl checks: %s", err)
 	}

 	// Commands to run after potentially writing the certificate. We
@@ -227,6 +251,9 @@ func CheckWithClients(t *testing.T, network string, address string, url string,
 		// Copy the cert into the newly running container.
 		certCtx["client-cert.pem"] = docker.PathContentsFromString(certificate)
 		certCtx["client-privkey.pem"] = docker.PathContentsFromString(privatekey)
+
+		wgetCmd = []string{"wget", "--verbose", "--ca-certificate=/root.pem", "--certificate=/client-cert.pem", "--private-key=/client-privkey.pem", url}
+		curlCmd = []string{"curl", "--verbose", "--cacert", "/root.pem", "--cert", "/client-cert.pem", "--key", "/client-privkey.pem", url}
 	}
 	if err := cwRunner.CopyTo(ctr.ID, "/", certCtx); err != nil {
 		t.Fatalf("Could not copy certificate and key into container: %v", err)
@@ -251,11 +278,144 @@ func CheckWithClients(t *testing.T, network string, address string, url string,
 	}
 }

+func CheckDeltaCRL(t *testing.T, network string, address string, url string, rootCert string, crls string) {
+	// We assume the network doesn't change once assigned.
+	buildClientContainerOnce.Do(func() {
+		buildWgetCurlContainer(t, network)
+		builtNetwork = network
+	})
+
+	if builtNetwork != network {
+		t.Fatalf("failed assumption check: different built network (%v) vs run network (%v); must've changed while running tests", builtNetwork, network)
+	}
+
+	// Start our service with a random name to not conflict with other
+	// threads.
+	ctx := context.Background()
+	ctr, _, _, err := cwRunner.Start(ctx, true, false)
+	if err != nil {
+		t.Fatalf("Could not start golang container for wget2 delta CRL checks: %s", err)
+	}
+
+	// Commands to run after potentially writing the certificate. We
+	// might augment these if the certificate exists.
+	//
+	// We manually add the expected hostname to the local hosts file
+	// to avoid resolving it over the network and instead resolving it
+	// to this other container we just started (potentially in parallel
+	// with other containers).
+	hostPrimeCmd := []string{"sh", "-c", "echo '" + address + "	" + uniqueHostname + "' >> /etc/hosts"}
+	wgetCmd := []string{"wget2", "--verbose", "--ca-certificate=/root.pem", "--crl-file=/crls.pem", url}
+
+	certCtx := docker.NewBuildContext()
+	certCtx["root.pem"] = docker.PathContentsFromString(rootCert)
+	certCtx["crls.pem"] = docker.PathContentsFromString(crls)
+	if err := cwRunner.CopyTo(ctr.ID, "/", certCtx); err != nil {
+		t.Fatalf("Could not copy certificate and key into container: %v", err)
+	}
+
+	for index, cmd := range [][]string{hostPrimeCmd, wgetCmd} {
+		t.Logf("Running client connection command: %v", cmd)
+
+		stdout, stderr, retcode, err := cwRunner.RunCmdWithOutput(ctx, ctr.ID, cmd)
+		if err != nil {
+			t.Fatalf("Could not run command (%v) in container: %v", cmd, err)
+		}
+
+		if len(stderr) != 0 {
+			t.Logf("Got stderr from command (%v):\n%v\n", cmd, string(stderr))
+		}
+
+		if retcode != 0 && index == 0 {
+			t.Logf("Got stdout from command (%v):\n%v\n", cmd, string(stdout))
+			t.Fatalf("Got unexpected non-zero retcode from command (%v): %v\n", cmd, retcode)
+		}
+
+		if retcode == 0 && index == 1 {
+			t.Logf("Got stdout from command (%v):\n%v\n", cmd, string(stdout))
+			t.Fatalf("Got unexpected zero retcode from command; wanted this to fail (%v): %v\n", cmd, retcode)
+		}
+	}
+}
+
+func CheckWithGo(t *testing.T, rootCert string, clientCert string, clientChain []string, clientKey string, host string, port int, networkAddr string, networkPort int, url string, expected string, shouldFail bool) {
+	// Ensure we can connect with Go.
+	pool := x509.NewCertPool()
+	pool.AppendCertsFromPEM([]byte(rootCert))
+	tlsConfig := &tls.Config{
+		RootCAs: pool,
+	}
+
+	if clientCert != "" {
+		var clientTLSCert tls.Certificate
+		clientTLSCert.Certificate = append(clientTLSCert.Certificate, parseCert(t, clientCert).Raw)
+		clientTLSCert.PrivateKey = parseKey(t, clientKey)
+		for _, cert := range clientChain {
+			clientTLSCert.Certificate = append(clientTLSCert.Certificate, parseCert(t, cert).Raw)
+		}
+
+		tlsConfig.Certificates = append(tlsConfig.Certificates, clientTLSCert)
+	}
+
+	dialer := &net.Dialer{
+		Timeout:   30 * time.Second,
+		KeepAlive: 30 * time.Second,
+	}
+
+	transport := &http.Transport{
+		TLSClientConfig: tlsConfig,
+		DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+			if addr == host+":"+strconv.Itoa(port) {
+				// If we can't resolve our hostname, try
+				// accessing it via the docker protocol
+				// instead of via the returned service
+				// address.
+				if _, err := net.LookupHost(host); err != nil && strings.Contains(err.Error(), "no such host") {
+					addr = networkAddr + ":" + strconv.Itoa(networkPort)
+				}
+			}
+			return dialer.DialContext(ctx, network, addr)
+		},
+	}
+
+	client := &http.Client{Transport: transport}
+	clientResp, err := client.Get(url)
+	if err != nil {
+		if shouldFail {
+			return
+		}
+
+		t.Fatalf("failed to fetch url (%v): %v", url, err)
+	} else if shouldFail {
+		if clientResp.StatusCode == 200 {
+			t.Fatalf("expected failure to fetch url (%v): got response: %v", url, clientResp)
+		}
+
+		return
+	}
+
+	defer clientResp.Body.Close()
+	body, err := io.ReadAll(clientResp.Body)
+	if err != nil {
+		t.Fatalf("failed to get read response body: %v", err)
+	}
+	if !strings.Contains(string(body), expected) {
+		t.Fatalf("expected body to contain (%v) but was:\n%v", expected, string(body))
+	}
+}
+
 func RunNginxRootTest(t *testing.T, caKeyType string, caKeyBits int, caUsePSS bool, roleKeyType string, roleKeyBits int, roleUsePSS bool) {
 	b, s := pki.CreateBackendWithStorage(t)

 	testSuffix := fmt.Sprintf(" - %v %v %v - %v %v %v", caKeyType, caKeyType, caUsePSS, roleKeyType, roleKeyBits, roleUsePSS)

+	// Configure our mount to use auto-rotate, even though we don't have
+	// a periodic func.
+	_, err := pki.CBWrite(b, s, "config/crl", map[string]interface{}{
+		"auto_rebuild": true,
+		"enable_delta": true,
+	})
+
 	// Create a root and intermediate, setting the intermediate as default.
 	resp, err := pki.CBWrite(b, s, "root/generate/internal", map[string]interface{}{
 		"common_name":  "Root X1" + testSuffix,
@@ -265,6 +425,7 @@ func RunNginxRootTest(t *testing.T, caKeyType string, caKeyBits int, caUsePSS bo
 		"key_type":     caKeyType,
 		"key_bits":     caKeyBits,
 		"use_pss":      caUsePSS,
+		"issuer_name":  "root",
 	})
 	requireSuccessNonNilResponse(t, resp, err, "failed to create root cert")
 	rootCert := resp.Data["certificate"].(string)
@@ -287,8 +448,9 @@ func RunNginxRootTest(t *testing.T, caKeyType string, caKeyBits int, caUsePSS bo
 		"csr":          resp.Data["csr"],
 	})
 	requireSuccessNonNilResponse(t, resp, err, "failed to sign intermediate csr")
+	intCert := resp.Data["certificate"].(string)
 	resp, err = pki.CBWrite(b, s, "issuers/import/bundle", map[string]interface{}{
-		"pem_bundle": resp.Data["certificate"],
+		"pem_bundle": intCert,
 	})
 	requireSuccessNonNilResponse(t, resp, err, "failed to sign intermediate csr")
 	_, err = pki.CBWrite(b, s, "config/issuers", map[string]interface{}{
@@ -309,7 +471,7 @@ func RunNginxRootTest(t *testing.T, caKeyType string, caKeyBits int, caUsePSS bo
 		"ip_sans":     "127.0.0.1,::1",
 		"sans":        uniqueHostname + ",localhost,localhost4,localhost6,localhost.localdomain",
 	})
-	requireSuccessNonNilResponse(t, resp, err, "failed to create leaf cert")
+	requireSuccessNonNilResponse(t, resp, err, "failed to create server leaf cert")
 	leafCert := resp.Data["certificate"].(string)
 	leafPrivateKey := resp.Data["private_key"].(string) + "\n"
 	fullChain := leafCert + "\n"
@@ -317,7 +479,69 @@ func RunNginxRootTest(t *testing.T, caKeyType string, caKeyBits int, caUsePSS bo
 		fullChain += cert + "\n"
 	}

-	cleanup, host, port, networkName, networkAddr, networkPort := buildNginxContainer(t, fullChain, leafPrivateKey)
+	// Issue a client leaf certificate.
+	resp, err = pki.CBWrite(b, s, "issue/testing", map[string]interface{}{
+		"common_name": "testing.client.dadgarcorp.com",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "failed to create client leaf cert")
+	clientCert := resp.Data["certificate"].(string)
+	clientKey := resp.Data["private_key"].(string) + "\n"
+	clientWireChain := clientCert + "\n" + resp.Data["issuing_ca"].(string) + "\n"
+	clientTrustChain := resp.Data["issuing_ca"].(string) + "\n" + rootCert + "\n"
+	clientCAChain := resp.Data["ca_chain"].([]string)
+
+	// Issue a client leaf cert and revoke it, placing it on the main CRL
+	// via rotation.
+	resp, err = pki.CBWrite(b, s, "issue/testing", map[string]interface{}{
+		"common_name": "revoked-crl.client.dadgarcorp.com",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "failed to create revoked client leaf cert")
+	revokedCert := resp.Data["certificate"].(string)
+	revokedKey := resp.Data["private_key"].(string) + "\n"
+	// revokedFullChain := revokedCert + "\n" + resp.Data["issuing_ca"].(string) + "\n"
+	// revokedTrustChain := resp.Data["issuing_ca"].(string) + "\n" + rootCert + "\n"
+	revokedCAChain := resp.Data["ca_chain"].([]string)
+	_, err = pki.CBWrite(b, s, "revoke", map[string]interface{}{
+		"certificate": revokedCert,
+	})
+	require.NoError(t, err)
+	_, err = pki.CBRead(b, s, "crl/rotate")
+	require.NoError(t, err)
+
+	// Issue a client leaf cert and revoke it, placing it on the delta CRL
+	// via rotation.
+	/*resp, err = pki.CBWrite(b, s, "issue/testing", map[string]interface{}{
+	      "common_name": "revoked-delta-crl.client.dadgarcorp.com",
+	  })
+	  requireSuccessNonNilResponse(t, resp, err, "failed to create delta CRL revoked client leaf cert")
+	  deltaCert := resp.Data["certificate"].(string)
+	  deltaKey := resp.Data["private_key"].(string) + "\n"
+	  //deltaFullChain := deltaCert + "\n" + resp.Data["issuing_ca"].(string) + "\n"
+	  //deltaTrustChain := resp.Data["issuing_ca"].(string) + "\n" + rootCert + "\n"
+	  deltaCAChain := resp.Data["ca_chain"].([]string)
+	  _, err = pki.CBWrite(b, s, "revoke", map[string]interface{}{
+	      "certificate": deltaCert,
+	  })
+	  require.NoError(t, err)
+	  _, err = pki.CBRead(b, s, "crl/rotate-delta")
+	  require.NoError(t, err)*/
+
+	// Get the CRL and Delta CRLs.
+	resp, err = pki.CBRead(b, s, "issuer/root/crl")
+	require.NoError(t, err)
+	rootCRL := resp.Data["crl"].(string) + "\n"
+	resp, err = pki.CBRead(b, s, "issuer/default/crl")
+	require.NoError(t, err)
+	intCRL := resp.Data["crl"].(string) + "\n"
+
+	// No need to fetch root Delta CRL as we've not revoked anything on it.
+	resp, err = pki.CBRead(b, s, "issuer/default/crl/delta")
+	require.NoError(t, err)
+	deltaCRL := resp.Data["crl"].(string) + "\n"
+
+	crls := rootCRL + intCRL + deltaCRL
+
+	cleanup, host, port, networkName, networkAddr, networkPort := buildNginxContainer(t, rootCert, crls, fullChain, leafPrivateKey)
 	defer cleanup()

 	if host != "127.0.0.1" && host != "::1" && strings.HasPrefix(host, containerName) {
@@ -326,52 +550,43 @@ func RunNginxRootTest(t *testing.T, caKeyType string, caKeyBits int, caUsePSS bo
 		port = networkPort
 	}

-	localURL := "https://" + host + ":" + strconv.Itoa(port) + "/index.html"
-	containerURL := "https://" + uniqueHostname + ":" + strconv.Itoa(networkPort) + "/index.html"
+	localBase := "https://" + host + ":" + strconv.Itoa(port)
+	localURL := localBase + "/index.html"
+	localProtectedURL := localBase + "/protected.html"
+	containerBase := "https://" + uniqueHostname + ":" + strconv.Itoa(networkPort)
+	containerURL := containerBase + "/index.html"
+	containerProtectedURL := containerBase + "/protected.html"

-	t.Logf("Spawned nginx container:\nhost: %v\nport: %v\nnetworkName: %v\nnetworkAddr: %v\nnetworkPort: %v\nlocalURL: %v\ncontainerURL: %v\n", host, port, networkName, networkAddr, networkPort, localURL, containerURL)
+	t.Logf("Spawned nginx container:\nhost: %v\nport: %v\nnetworkName: %v\nnetworkAddr: %v\nnetworkPort: %v\nlocalURL: %v\ncontainerURL: %v\n", host, port, networkName, networkAddr, networkPort, localBase, containerBase)

-	// Ensure we can connect with Go.
-	pool := x509.NewCertPool()
-	pool.AppendCertsFromPEM([]byte(rootCert))
-	tlsConfig := &tls.Config{
-		RootCAs: pool,
-	}
-	dialer := &net.Dialer{
-		Timeout:   30 * time.Second,
-		KeepAlive: 30 * time.Second,
-	}
-	transport := &http.Transport{
-		TLSClientConfig: tlsConfig,
-		DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
-			if addr == host+":"+strconv.Itoa(port) {
-				// If we can't resolve our hostname, try
-				// accessing it via the docker protocol
-				// instead of via the returned service
-				// address.
-				if _, err := net.LookupHost(host); err != nil && strings.Contains(err.Error(), "no such host") {
-					addr = networkAddr + ":" + strconv.Itoa(networkPort)
-				}
-			}
-			return dialer.DialContext(ctx, network, addr)
-		},
-	}
-	client := &http.Client{Transport: transport}
-	clientResp, err := client.Get(localURL)
-	if err != nil {
-		t.Fatalf("failed to fetch url (%v): %v", localURL, err)
-	}
-	defer clientResp.Body.Close()
-	body, err := io.ReadAll(clientResp.Body)
-	if err != nil {
-		t.Fatalf("failed to get read response body: %v", err)
-	}
-	if !strings.Contains(string(body), unprotectedFile) {
-		t.Fatalf("expected body to contain (%v) but was:\n%v", unprotectedFile, string(body))
-	}
+	// Ensure we can connect with Go. We do our checks for revocation here,
+	// as this behavior is server-controlled and shouldn't matter based on
+	// client type.
+	CheckWithGo(t, rootCert, "", nil, "", host, port, networkAddr, networkPort, localURL, unprotectedFile, false)
+	CheckWithGo(t, rootCert, "", nil, "", host, port, networkAddr, networkPort, localProtectedURL, failureIndicator, true)
+	CheckWithGo(t, rootCert, clientCert, clientCAChain, clientKey, host, port, networkAddr, networkPort, localProtectedURL, protectedFile, false)
+	CheckWithGo(t, rootCert, revokedCert, revokedCAChain, revokedKey, host, port, networkAddr, networkPort, localProtectedURL, protectedFile, true)
+	// CheckWithGo(t, rootCert, deltaCert, deltaCAChain, deltaKey, host, port, networkAddr, networkPort, localProtectedURL, protectedFile, true)

 	// Ensure we can connect with wget/curl.
 	CheckWithClients(t, networkName, networkAddr, containerURL, rootCert, "", "")
+	CheckWithClients(t, networkName, networkAddr, containerProtectedURL, clientTrustChain, clientWireChain, clientKey)
+
+	// Ensure OpenSSL will validate the delta CRL by revoking our server leaf
+	// and then using it with wget2. This will land on the intermediate's
+	// Delta CRL.
+	_, err = pki.CBWrite(b, s, "revoke", map[string]interface{}{
+		"certificate": leafCert,
+	})
+	require.NoError(t, err)
+	_, err = pki.CBRead(b, s, "crl/rotate-delta")
+	require.NoError(t, err)
+	resp, err = pki.CBRead(b, s, "issuer/default/crl/delta")
+	require.NoError(t, err)
+	deltaCRL = resp.Data["crl"].(string) + "\n"
+	crls = rootCRL + intCRL + deltaCRL
+
+	CheckDeltaCRL(t, networkName, networkAddr, containerURL, rootCert, crls)
 }

 func Test_NginxRSAPure(t *testing.T) {
--- a/builtin/logical/pkiext/test_helpers.go
+++ b/builtin/logical/pkiext/test_helpers.go
@@ -1,10 +1,15 @@
 package pkiext

 import (
+	"crypto"
+	"crypto/x509"
+	"encoding/pem"
 	"fmt"
 	"testing"

+	"github.com/hashicorp/vault/sdk/helper/certutil"
 	"github.com/hashicorp/vault/sdk/logical"
+
 	"github.com/stretchr/testify/require"
 )

@@ -40,3 +45,21 @@ func requireSuccessNilResponse(t *testing.T, resp *logical.Response, err error,
 		require.Nilf(t, resp, msg, msgAndArgs...)
 	}
 }
+
+func parseCert(t *testing.T, pemCert string) *x509.Certificate {
+	block, _ := pem.Decode([]byte(pemCert))
+	require.NotNil(t, block, "failed to decode PEM block")
+
+	cert, err := x509.ParseCertificate(block.Bytes)
+	require.NoError(t, err)
+	return cert
+}
+
+func parseKey(t *testing.T, pemKey string) crypto.Signer {
+	block, _ := pem.Decode([]byte(pemKey))
+	require.NotNil(t, block, "failed to decode PEM block")
+
+	key, _, err := certutil.ParseDERKey(block.Bytes)
+	require.NoError(t, err)
+	return key
+}