Add DNS wildcard tests to ACME test suite (#20486)

* Refactor setting local addresses

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Validate wildcard domains in ACME test suite

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add locking to DNS resolver

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Better removal semantics for records

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

builtin/logical/pki/dnstest/server.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -21,6 +22,7 @@ type TestServer struct {
 	network string
 	startup *docker.Service
 
+	lock       sync.Mutex
 	serial     int
 	forwarders []string
 	domains    []string
@@ -170,6 +172,9 @@ func (ts *TestServer) buildZoneFile(target string) string {
 }
 
 func (ts *TestServer) PushConfig() {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
 	contents := docker.NewBuildContext()
 	cfgPath := "/etc/bind/named.conf.options"
 	namedCfg := ts.buildNamedConf()
@@ -248,6 +253,9 @@ func (ts *TestServer) GetRemoteAddr() string {
 }
 
 func (ts *TestServer) AddDomain(domain string) {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
 	for _, existing := range ts.domains {
 		if existing == domain {
 			return
@@ -258,6 +266,9 @@ func (ts *TestServer) AddDomain(domain string) {
 }
 
 func (ts *TestServer) AddRecord(domain string, record string, value string) {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
 	foundDomain := false
 	for _, existing := range ts.domains {
 		if strings.HasSuffix(domain, existing) {
@@ -277,7 +288,8 @@ func (ts *TestServer) AddRecord(domain string, record string, value string) {
 	if values, present := ts.records[domain][record]; present {
 		for _, candidate := range values {
 			if candidate == value {
-				break
+				// Already present; skip adding.
+				return
 			}
 		}
 	}
@@ -285,7 +297,92 @@ func (ts *TestServer) AddRecord(domain string, record string, value string) {
 	ts.records[domain][record] = append(ts.records[domain][record], value)
 }
 
+func (ts *TestServer) RemoveRecord(domain string, record string, value string) {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
+	foundDomain := false
+	for _, existing := range ts.domains {
+		if strings.HasSuffix(domain, existing) {
+			foundDomain = true
+			break
+		}
+	}
+	if !foundDomain {
+		// Not found.
+		return
+	}
+
+	value = strings.TrimSpace(value)
+	if _, present := ts.records[domain]; !present {
+		// Not found.
+		return
+	}
+
+	var remaining []string
+	if values, present := ts.records[domain][record]; present {
+		for _, candidate := range values {
+			if candidate != value {
+				remaining = append(remaining, candidate)
+			}
+		}
+	}
+
+	ts.records[domain][record] = remaining
+}
+
+func (ts *TestServer) RemoveRecordsOfTypeForDomain(domain string, record string) {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
+	foundDomain := false
+	for _, existing := range ts.domains {
+		if strings.HasSuffix(domain, existing) {
+			foundDomain = true
+			break
+		}
+	}
+	if !foundDomain {
+		// Not found.
+		return
+	}
+
+	if _, present := ts.records[domain]; !present {
+		// Not found.
+		return
+	}
+
+	delete(ts.records[domain], record)
+}
+
+func (ts *TestServer) RemoveRecordsForDomain(domain string) {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
+	foundDomain := false
+	for _, existing := range ts.domains {
+		if strings.HasSuffix(domain, existing) {
+			foundDomain = true
+			break
+		}
+	}
+	if !foundDomain {
+		// Not found.
+		return
+	}
+
+	if _, present := ts.records[domain]; !present {
+		// Not found.
+		return
+	}
+
+	ts.records[domain] = map[string][]string{}
+}
+
 func (ts *TestServer) RemoveAllRecords() {
+	ts.lock.Lock()
+	defer ts.lock.Unlock()
+
 	ts.records = map[string]map[string][]string{}
 }
 

builtin/logical/pkiext/pkiext_binary/acme_test.go

@@ -12,7 +12,6 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"crypto/x509/pkix"
-	"fmt"
 	"net"
 	"net/http"
 	"path"
@@ -34,8 +33,9 @@ func Test_ACME(t *testing.T) {
 	defer cluster.Cleanup()
 
 	tc := map[string]func(t *testing.T, cluster *VaultPkiCluster){
-		"certbot":      SubtestACMECertbot,
+		"certbot":       SubtestACMECertbot,
-		"acme ip sans": SubTestACMEIPAndDNS,
+		"acme ip sans":  SubtestACMEIPAndDNS,
+		"acme wildcard": SubtestACMEWildcardDNS,
 	}
 
 	// Wrap the tests within an outer group, so that we run all tests
@@ -89,7 +89,7 @@ func SubtestACMECertbot(t *testing.T, cluster *VaultPkiCluster) {
 	require.Contains(t, networks, vaultNetwork, "expected to contain vault network")
 
 	ipAddr := networks[vaultNetwork]
-	hostname := "acme-client.dadgarcorp.com"
+	hostname := "certbot-acme-client.dadgarcorp.com"
 
 	err = pki.AddHostname(hostname, ipAddr)
 	require.NoError(t, err, "failed to update vault host files")
@@ -150,15 +150,14 @@ func SubtestACMECertbot(t *testing.T, cluster *VaultPkiCluster) {
 	require.NotEqual(t, 0, retcode, "expected non-zero retcode double revoke command result")
 }
 
-func SubTestACMEIPAndDNS(t *testing.T, cluster *VaultPkiCluster) {
+func SubtestACMEIPAndDNS(t *testing.T, cluster *VaultPkiCluster) {
 	pki, err := cluster.CreateAcmeMount("pki-ip-dns-sans")
 	require.NoError(t, err, "failed setting up acme mount")
 
 	// Since we interact with ACME from outside the container network the ACME
 	// configuration needs to be updated to use the host port and not the internal
 	// docker ip.
-	basePath := fmt.Sprintf("https://%s/v1/%s", pki.GetActiveContainerHostPort(), pki.mount)
+	basePath, err := pki.UpdateClusterConfigLocalAddr()
-	err = pki.UpdateClusterConfig(map[string]interface{}{"path": basePath})
 	require.NoError(t, err, "failed updating cluster config")
 
 	logConsumer, logStdout, logStderr := getDockerLog(t)
@@ -220,7 +219,43 @@ func SubTestACMEIPAndDNS(t *testing.T, cluster *VaultPkiCluster) {
 		IPAddresses: []net.IP{net.ParseIP(ipAddr)},
 	}
 
-	acmeCert := doAcmeValidationWithGoLibrary(t, directoryUrl, acmeOrderIdentifiers, runner, nginxContainerId, challengeFolder, cr)
+	provisioningFunc := func(acmeClient *acme.Client, auths []*acme.Authorization) []*acme.Challenge {
+		// For each http-01 challenge, generate the file to place underneath the nginx challenge folder
+		acmeCtx := hDocker.NewBuildContext()
+		var challengesToAccept []*acme.Challenge
+		for _, auth := range auths {
+			for _, challenge := range auth.Challenges {
+				if challenge.Status != acme.StatusPending {
+					t.Logf("ignoring challenge not in status pending: %v", challenge)
+					continue
+				}
+
+				if challenge.Type == "http-01" {
+					challengeBody, err := acmeClient.HTTP01ChallengeResponse(challenge.Token)
+					require.NoError(t, err, "failed generating challenge response")
+
+					challengePath := acmeClient.HTTP01ChallengePath(challenge.Token)
+					require.NoError(t, err, "failed generating challenge path")
+
+					challengeFile := path.Base(challengePath)
+
+					acmeCtx[challengeFile] = hDocker.PathContentsFromString(challengeBody)
+
+					challengesToAccept = append(challengesToAccept, challenge)
+				}
+			}
+		}
+
+		require.GreaterOrEqual(t, len(challengesToAccept), 1, "Need at least one challenge, got none")
+
+		// Copy all challenges within the nginx container
+		err = runner.CopyTo(nginxContainerId, challengeFolder, acmeCtx)
+		require.NoError(t, err, "failed copying challenges to container")
+
+		return challengesToAccept
+	}
+
+	acmeCert := doAcmeValidationWithGoLibrary(t, directoryUrl, acmeOrderIdentifiers, cr, provisioningFunc)
 
 	require.Len(t, acmeCert.IPAddresses, 1, "expected only a single ip address in cert")
 	require.Equal(t, ipAddr, acmeCert.IPAddresses[0].String())
@@ -243,7 +278,7 @@ func SubTestACMEIPAndDNS(t *testing.T, cluster *VaultPkiCluster) {
 		IPAddresses: []net.IP{net.ParseIP(ipAddr)},
 	}
 
-	acmeCert = doAcmeValidationWithGoLibrary(t, directoryUrl, acmeOrderIdentifiers, runner, nginxContainerId, challengeFolder, cr)
+	acmeCert = doAcmeValidationWithGoLibrary(t, directoryUrl, acmeOrderIdentifiers, cr, provisioningFunc)
 
 	require.Len(t, acmeCert.IPAddresses, 1, "expected only a single ip address in cert")
 	require.Equal(t, ipAddr, acmeCert.IPAddresses[0].String())
@@ -251,7 +286,9 @@ func SubTestACMEIPAndDNS(t *testing.T, cluster *VaultPkiCluster) {
 	require.Equal(t, "", acmeCert.Subject.CommonName)
 }
 
-func doAcmeValidationWithGoLibrary(t *testing.T, directoryUrl string, acmeOrderIdentifiers []acme.AuthzID, runner *hDocker.Runner, nginxContainerId string, challengeFolder string, cr *x509.CertificateRequest) *x509.Certificate {
+type acmeGoValidatorProvisionerFunc func(acmeClient *acme.Client, auths []*acme.Authorization) []*acme.Challenge
+
+func doAcmeValidationWithGoLibrary(t *testing.T, directoryUrl string, acmeOrderIdentifiers []acme.AuthzID, cr *x509.CertificateRequest, provisioningFunc acmeGoValidatorProvisionerFunc) *x509.Certificate {
 	// Since we are contacting Vault through the host ip/port, the certificate will not validate properly
 	tr := &http.Transport{
 		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
@@ -287,36 +324,9 @@ func doAcmeValidationWithGoLibrary(t *testing.T, directoryUrl string, acmeOrderI
 		auths = append(auths, authorization)
 	}
 
-	// For each http-01 challenge, generate the file to place underneath the nginx challenge folder
+	// Handle the validation using the external validation mechanism.
-	acmeCtx := hDocker.NewBuildContext()
+	challengesToAccept := provisioningFunc(acmeClient, auths)
-	var challengesToAccept []*acme.Challenge
+	require.NotEmpty(t, challengesToAccept, "provisioning function failed to return any challenges to accept")
-	for _, auth := range auths {
-		for _, challenge := range auth.Challenges {
-			if challenge.Status != acme.StatusPending {
-				t.Logf("ignoring challenge not in status pending: %v", challenge)
-				continue
-			}
-			if challenge.Type == "http-01" {
-				challengeBody, err := acmeClient.HTTP01ChallengeResponse(challenge.Token)
-				require.NoError(t, err, "failed generating challenge response")
-
-				challengePath := acmeClient.HTTP01ChallengePath(challenge.Token)
-				require.NoError(t, err, "failed generating challenge path")
-
-				challengeFile := path.Base(challengePath)
-
-				acmeCtx[challengeFile] = hDocker.PathContentsFromString(challengeBody)
-
-				challengesToAccept = append(challengesToAccept, challenge)
-			}
-		}
-	}
-
-	require.GreaterOrEqual(t, len(challengesToAccept), 1, "Need at least one challenge, got none")
-
-	// Copy all challenges within the nginx container
-	err = runner.CopyTo(nginxContainerId, challengeFolder, acmeCtx)
-	require.NoError(t, err, "failed copying challenges to container")
 
 	// Tell the ACME server, that they can now validate those challenges.
 	for _, challenge := range challengesToAccept {
@@ -342,6 +352,80 @@ func doAcmeValidationWithGoLibrary(t *testing.T, directoryUrl string, acmeOrderI
 	return acmeCert
 }
 
+func SubtestACMEWildcardDNS(t *testing.T, cluster *VaultPkiCluster) {
+	pki, err := cluster.CreateAcmeMount("pki-dns-wildcards")
+	require.NoError(t, err, "failed setting up acme mount")
+
+	// Since we interact with ACME from outside the container network the ACME
+	// configuration needs to be updated to use the host port and not the internal
+	// docker ip.
+	basePath, err := pki.UpdateClusterConfigLocalAddr()
+	require.NoError(t, err, "failed updating cluster config")
+
+	hostname := "go-lang-wildcard-client.dadgarcorp.com"
+	wildcard := "*." + hostname
+
+	// Do validation without a role first.
+	directoryUrl := basePath + "/acme/directory"
+	acmeOrderIdentifiers := []acme.AuthzID{
+		{Type: "dns", Value: hostname},
+		{Type: "dns", Value: wildcard},
+	}
+	cr := &x509.CertificateRequest{
+		Subject:  pkix.Name{CommonName: wildcard},
+		DNSNames: []string{hostname, wildcard},
+	}
+
+	provisioningFunc := func(acmeClient *acme.Client, auths []*acme.Authorization) []*acme.Challenge {
+		// For each dns-01 challenge, place the record in the associated DNS resolver.
+		var challengesToAccept []*acme.Challenge
+		for _, auth := range auths {
+			for _, challenge := range auth.Challenges {
+				if challenge.Status != acme.StatusPending {
+					t.Logf("ignoring challenge not in status pending: %v", challenge)
+					continue
+				}
+
+				if challenge.Type == "dns-01" {
+					challengeBody, err := acmeClient.DNS01ChallengeRecord(challenge.Token)
+					require.NoError(t, err, "failed generating challenge response")
+
+					err = pki.AddDNSRecord("_acme-challenge."+auth.Identifier.Value, "TXT", challengeBody)
+					require.NoError(t, err, "failed setting DNS record")
+
+					challengesToAccept = append(challengesToAccept, challenge)
+				}
+			}
+		}
+
+		require.GreaterOrEqual(t, len(challengesToAccept), 1, "Need at least one challenge, got none")
+		return challengesToAccept
+	}
+
+	acmeCert := doAcmeValidationWithGoLibrary(t, directoryUrl, acmeOrderIdentifiers, cr, provisioningFunc)
+	require.Contains(t, acmeCert.DNSNames, hostname)
+	require.Contains(t, acmeCert.DNSNames, wildcard)
+	require.Equal(t, wildcard, acmeCert.Subject.CommonName)
+	pki.RemoveDNSRecordsForDomain(hostname)
+
+	// Redo validation with a role this time.
+	err = pki.UpdateRole("wildcard", map[string]interface{}{
+		"key_type":                    "any",
+		"allowed_domains":             "go-lang-wildcard-client.dadgarcorp.com",
+		"allow_subdomains":            true,
+		"allow_bare_domains":          true,
+		"allow_wildcard_certificates": true,
+	})
+	require.NoError(t, err, "failed creating role wildcard")
+	directoryUrl = basePath + "/roles/wildcard/acme/directory"
+
+	acmeCert = doAcmeValidationWithGoLibrary(t, directoryUrl, acmeOrderIdentifiers, cr, provisioningFunc)
+	require.Contains(t, acmeCert.DNSNames, hostname)
+	require.Contains(t, acmeCert.DNSNames, wildcard)
+	require.Equal(t, wildcard, acmeCert.Subject.CommonName)
+	pki.RemoveDNSRecordsForDomain(hostname)
+}
+
 func getDockerLog(t *testing.T) (func(s string), *pkiext.LogConsumerWriter, *pkiext.LogConsumerWriter) {
 	logConsumer := func(s string) {
 		t.Logf(s)

builtin/logical/pkiext/pkiext_binary/pki_cluster.go

@@ -120,6 +120,33 @@ func (vpc *VaultPkiCluster) AddDNSRecord(hostname, recordType, ip string) error
 	return nil
 }
 
+func (vpc *VaultPkiCluster) RemoveDNSRecord(domain string, record string, value string) error {
+	if vpc.Dns == nil {
+		return fmt.Errorf("no DNS server was provisioned on this cluster group; unable to remove specific record")
+	}
+
+	vpc.Dns.RemoveRecord(domain, record, value)
+	return nil
+}
+
+func (vpc *VaultPkiCluster) RemoveDNSRecordsOfTypeForDomain(domain string, record string) error {
+	if vpc.Dns == nil {
+		return fmt.Errorf("no DNS server was provisioned on this cluster group; unable to remove all records of type")
+	}
+
+	vpc.Dns.RemoveRecordsOfTypeForDomain(domain, record)
+	return nil
+}
+
+func (vpc *VaultPkiCluster) RemoveDNSRecordsForDomain(domain string) error {
+	if vpc.Dns == nil {
+		return fmt.Errorf("no DNS server was provisioned on this cluster group; unable to remove records for domain")
+	}
+
+	vpc.Dns.RemoveRecordsForDomain(domain)
+	return nil
+}
+
 func (vpc *VaultPkiCluster) RemoveAllDNSRecords() error {
 	if vpc.Dns == nil {
 		return fmt.Errorf("no DNS server was provisioned on this cluster group; unable to remove all records")

builtin/logical/pkiext/pkiext_binary/pki_mount.go

@@ -5,6 +5,7 @@ package pkiext_binary
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/hashicorp/vault/api"
 )
@@ -26,6 +27,13 @@ func (vpm *VaultPkiMount) UpdateClusterConfig(config map[string]interface{}) err
 	return err
 }
 
+func (vpm *VaultPkiMount) UpdateClusterConfigLocalAddr() (string, error) {
+	basePath := fmt.Sprintf("https://%s/v1/%s", vpm.GetActiveContainerHostPort(), vpm.mount)
+	return basePath, vpm.UpdateClusterConfig(map[string]interface{}{
+		"path": basePath,
+	})
+}
+
 func (vpm *VaultPkiMount) UpdateAcmeConfig(enable bool, config map[string]interface{}) error {
 	defaults := map[string]interface{}{
 		"enabled": enable,