acmeserver: support specifying the allowed challenge types (#5794)

* acmeserver: support specifying the allowed challenge types

* add caddyfile adapt tests

* introduce basic acme_server test

* skip acme test on unsuitable environments

* skip integration tests of ACME

* documentation

* add negative-scenario test for mismatched allowed challenges

* a bit more docs

* fix tests for ACME challenges

* appease the linter

* skip ACME tests on s390x

* enable ACME challenge tests on all machines

* Apply suggestions from code review

Co-authored-by: Matt Holt <[email protected]>

---------

Co-authored-by: Matt Holt <[email protected]>

7 files changed, 549 insertions, 4 deletions

diff --git a/caddytest/integration/acme_test.go b/caddytest/integration/acme_test.go
new file mode 100644
index 000000000..00a3a6c4e
--- /dev/null
+++ b/caddytest/integration/acme_test.go
@@ -0,0 +1,261 @@
+package integration
+
+import (
+	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/tls"
+	"crypto/x509"
+	"fmt"
+	"net"
+	"net/http"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/caddytest"
+	"github.com/mholt/acmez"
+	"github.com/mholt/acmez/acme"
+	smallstepacme "github.com/smallstep/certificates/acme"
+	"go.uber.org/zap"
+)
+
+const acmeChallengePort = 8080
+
+// Test the basic functionality of Caddy's ACME server
+func TestACMEServerWithDefaults(t *testing.T) {
+	ctx := context.Background()
+	logger, err := zap.NewDevelopment()
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+	{
+		skip_install_trust
+		admin localhost:2999
+		http_port     9080
+		https_port    9443
+		local_certs
+	}
+	acme.localhost {
+		acme_server
+	}
+  `, "caddyfile")
+
+	datadir := caddy.AppDataDir()
+	rootCertsGlob := filepath.Join(datadir, "pki", "authorities", "local", "*.crt")
+	matches, err := filepath.Glob(rootCertsGlob)
+	if err != nil {
+		t.Errorf("could not find root certs: %s", err)
+		return
+	}
+	certPool := x509.NewCertPool()
+	for _, m := range matches {
+		certPem, err := os.ReadFile(m)
+		if err != nil {
+			t.Errorf("reading cert file '%s' error: %s", m, err)
+			return
+		}
+		if !certPool.AppendCertsFromPEM(certPem) {
+			t.Errorf("failed to append the cert: %s", m)
+			return
+		}
+	}
+
+	client := acmez.Client{
+		Client: &acme.Client{
+			Directory: "https://acme.localhost:9443/acme/local/directory",
+			HTTPClient: &http.Client{
+				Transport: &http.Transport{
+					TLSClientConfig: &tls.Config{
+						RootCAs: certPool,
+					},
+				},
+			},
+			Logger: logger,
+		},
+		ChallengeSolvers: map[string]acmez.Solver{
+			acme.ChallengeTypeHTTP01: &naiveHTTPSolver{logger: logger},
+		},
+	}
+
+	accountPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Errorf("generating account key: %v", err)
+	}
+	account := acme.Account{
+		Contact:              []string{"mailto:[email protected]"},
+		TermsOfServiceAgreed: true,
+		PrivateKey:           accountPrivateKey,
+	}
+	account, err = client.NewAccount(ctx, account)
+	if err != nil {
+		t.Errorf("new account: %v", err)
+		return
+	}
+
+	// Every certificate needs a key.
+	certPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Errorf("generating certificate key: %v", err)
+		return
+	}
+
+	certs, err := client.ObtainCertificate(ctx, account, certPrivateKey, []string{"localhost"})
+	if err != nil {
+		t.Errorf("obtaining certificate: %v", err)
+		return
+	}
+
+	// ACME servers should usually give you the entire certificate chain
+	// in PEM format, and sometimes even alternate chains! It's up to you
+	// which one(s) to store and use, but whatever you do, be sure to
+	// store the certificate and key somewhere safe and secure, i.e. don't
+	// lose them!
+	for _, cert := range certs {
+		t.Logf("Certificate %q:\n%s\n\n", cert.URL, cert.ChainPEM)
+	}
+}
+
+func TestACMEServerWithMismatchedChallenges(t *testing.T) {
+	ctx := context.Background()
+	logger := caddy.Log().Named("acmez")
+
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+	{
+		skip_install_trust
+		admin localhost:2999
+		http_port     9080
+		https_port    9443
+		local_certs
+	}
+	acme.localhost {
+		acme_server {
+			challenges tls-alpn-01
+		}
+	}
+  `, "caddyfile")
+
+	datadir := caddy.AppDataDir()
+	rootCertsGlob := filepath.Join(datadir, "pki", "authorities", "local", "*.crt")
+	matches, err := filepath.Glob(rootCertsGlob)
+	if err != nil {
+		t.Errorf("could not find root certs: %s", err)
+		return
+	}
+	certPool := x509.NewCertPool()
+	for _, m := range matches {
+		certPem, err := os.ReadFile(m)
+		if err != nil {
+			t.Errorf("reading cert file '%s' error: %s", m, err)
+			return
+		}
+		if !certPool.AppendCertsFromPEM(certPem) {
+			t.Errorf("failed to append the cert: %s", m)
+			return
+		}
+	}
+
+	client := acmez.Client{
+		Client: &acme.Client{
+			Directory: "https://acme.localhost:9443/acme/local/directory",
+			HTTPClient: &http.Client{
+				Transport: &http.Transport{
+					TLSClientConfig: &tls.Config{
+						RootCAs: certPool,
+					},
+				},
+			},
+			Logger: logger,
+		},
+		ChallengeSolvers: map[string]acmez.Solver{
+			acme.ChallengeTypeHTTP01: &naiveHTTPSolver{logger: logger},
+		},
+	}
+
+	accountPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Errorf("generating account key: %v", err)
+	}
+	account := acme.Account{
+		Contact:              []string{"mailto:[email protected]"},
+		TermsOfServiceAgreed: true,
+		PrivateKey:           accountPrivateKey,
+	}
+	account, err = client.NewAccount(ctx, account)
+	if err != nil {
+		t.Errorf("new account: %v", err)
+		return
+	}
+
+	// Every certificate needs a key.
+	certPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Errorf("generating certificate key: %v", err)
+		return
+	}
+
+	certs, err := client.ObtainCertificate(ctx, account, certPrivateKey, []string{"localhost"})
+	if len(certs) > 0 {
+		t.Errorf("expected '0' certificates, but received '%d'", len(certs))
+	}
+	if err == nil {
+		t.Error("expected errors, but received none")
+	}
+	const expectedErrMsg = "no solvers available for remaining challenges (configured=[http-01] offered=[tls-alpn-01] remaining=[tls-alpn-01])"
+	if !strings.Contains(err.Error(), expectedErrMsg) {
+		t.Errorf(`received error message does not match expectation: expected="%s" received="%s"`, expectedErrMsg, err.Error())
+	}
+}
+
+// naiveHTTPSolver is a no-op acmez.Solver for example purposes only.
+type naiveHTTPSolver struct {
+	srv    *http.Server
+	logger *zap.Logger
+}
+
+func (s *naiveHTTPSolver) Present(ctx context.Context, challenge acme.Challenge) error {
+	smallstepacme.InsecurePortHTTP01 = acmeChallengePort
+	s.srv = &http.Server{
+		Addr: fmt.Sprintf("localhost:%d", acmeChallengePort),
+		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			host, _, err := net.SplitHostPort(r.Host)
+			if err != nil {
+				host = r.Host
+			}
+			if r.Method == "GET" && r.URL.Path == challenge.HTTP01ResourcePath() && strings.EqualFold(host, challenge.Identifier.Value) {
+				w.Header().Add("Content-Type", "text/plain")
+				w.Write([]byte(challenge.KeyAuthorization))
+				r.Close = true
+				s.logger.Info("served key authentication",
+					zap.String("identifier", challenge.Identifier.Value),
+					zap.String("challenge", "http-01"),
+					zap.String("remote", r.RemoteAddr),
+				)
+			}
+		}),
+	}
+	l, err := net.Listen("tcp", fmt.Sprintf(":%d", acmeChallengePort))
+	if err != nil {
+		return err
+	}
+	s.logger.Info("present challenge", zap.Any("challenge", challenge))
+	go s.srv.Serve(l)
+	return nil
+}
+
+func (s naiveHTTPSolver) CleanUp(ctx context.Context, challenge acme.Challenge) error {
+	smallstepacme.InsecurePortHTTP01 = 0
+	s.logger.Info("cleanup", zap.Any("challenge", challenge))
+	if s.srv != nil {
+		s.srv.Close()
+	}
+	return nil
+}
diff --git a/caddytest/integration/caddyfile_adapt/acme_server_custom_challenges.txt b/caddytest/integration/caddyfile_adapt/acme_server_custom_challenges.txt
new file mode 100644
index 000000000..2a7a51492
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/acme_server_custom_challenges.txt
@@ -0,0 +1,65 @@
+{
+	pki {
+		ca custom-ca {
+			name "Custom CA"
+		}
+	}
+}
+
+acme.example.com {
+	acme_server {
+		ca custom-ca
+		challenges dns-01
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"acme.example.com"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"ca": "custom-ca",
+													"challenges": [
+														"dns-01"
+													],
+													"handler": "acme_server"
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		},
+		"pki": {
+			"certificate_authorities": {
+				"custom-ca": {
+					"name": "Custom CA"
+				}
+			}
+		}
+	}
+}
diff --git a/caddytest/integration/caddyfile_adapt/acme_server_default_challenges.txt b/caddytest/integration/caddyfile_adapt/acme_server_default_challenges.txt
new file mode 100644
index 000000000..26d345047
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/acme_server_default_challenges.txt
@@ -0,0 +1,62 @@
+{
+	pki {
+		ca custom-ca {
+			name "Custom CA"
+		}
+	}
+}
+
+acme.example.com {
+	acme_server {
+		ca custom-ca
+		challenges
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"acme.example.com"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"ca": "custom-ca",
+													"handler": "acme_server"
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		},
+		"pki": {
+			"certificate_authorities": {
+				"custom-ca": {
+					"name": "Custom CA"
+				}
+			}
+		}
+	}
+}
diff --git a/caddytest/integration/caddyfile_adapt/acme_server_multi_custom_challenges.txt b/caddytest/integration/caddyfile_adapt/acme_server_multi_custom_challenges.txt
new file mode 100644
index 000000000..7fe3ca663
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/acme_server_multi_custom_challenges.txt
@@ -0,0 +1,66 @@
+{
+	pki {
+		ca custom-ca {
+			name "Custom CA"
+		}
+	}
+}
+
+acme.example.com {
+	acme_server {
+		ca custom-ca
+		challenges dns-01 http-01
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"acme.example.com"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"ca": "custom-ca",
+													"challenges": [
+														"dns-01",
+														"http-01"
+													],
+													"handler": "acme_server"
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		},
+		"pki": {
+			"certificate_authorities": {
+				"custom-ca": {
+					"name": "Custom CA"
+				}
+			}
+		}
+	}
+}
diff --git a/modules/caddypki/acmeserver/acmeserver.go b/modules/caddypki/acmeserver/acmeserver.go
index 8ac1b6c89..e0588c527 100644
--- a/modules/caddypki/acmeserver/acmeserver.go
+++ b/modules/caddypki/acmeserver/acmeserver.go
@@ -91,6 +91,11 @@ type Handler struct {
 	// than 1 resolver address, one is chosen at random.
 	Resolvers []string `json:"resolvers,omitempty"`
 
+	// Specify the set of enabled ACME challenges. An empty or absent value
+	// means all challenges are enabled. Accepted values are:
+	// "http-01", "dns-01", "tls-alpn-01"
+	Challenges ACMEChallenges `json:"challenges,omitempty" `
+
 	logger    *zap.Logger
 	resolvers []caddy.NetworkAddress
 	ctx       caddy.Context
@@ -125,6 +130,11 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 	if ash.Lifetime == 0 {
 		ash.Lifetime = caddy.Duration(12 * time.Hour)
 	}
+	if len(ash.Challenges) > 0 {
+		if err := ash.Challenges.validate(); err != nil {
+			return err
+		}
+	}
 
 	// get a reference to the configured CA
 	appModule, err := ctx.App("pki")
@@ -153,8 +163,9 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 		AuthConfig: &authority.AuthConfig{
 			Provisioners: provisioner.List{
 				&provisioner.ACME{
-					Name: ash.CA,
-					Type: provisioner.TypeACME.String(),
+					Name:       ash.CA,
+					Challenges: ash.Challenges.toSmallstepType(),
+					Type:       provisioner.TypeACME.String(),
 					Claims: &provisioner.Claims{
 						MinTLSDur:     &provisioner.Duration{Duration: 5 * time.Minute},
 						MaxTLSDur:     &provisioner.Duration{Duration: 24 * time.Hour * 365},
diff --git a/modules/caddypki/acmeserver/caddyfile.go b/modules/caddypki/acmeserver/caddyfile.go
index 51290eba6..864a94c53 100644
--- a/modules/caddypki/acmeserver/caddyfile.go
+++ b/modules/caddypki/acmeserver/caddyfile.go
@@ -32,6 +32,7 @@ func init() {
 //		ca        <id>
 //		lifetime  <duration>
 //		resolvers <addresses...>
+//		challenges <challenges...>
 //	}
 func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
 	h.Next() // consume directive name
@@ -73,14 +74,16 @@ func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 			if d := time.Duration(ca.IntermediateLifetime); d > 0 && dur > d {
 				return nil, h.Errf("certificate lifetime (%s) exceeds intermediate certificate lifetime (%s)", dur, d)
 			}
-
 			acmeServer.Lifetime = caddy.Duration(dur)
-
 		case "resolvers":
 			acmeServer.Resolvers = h.RemainingArgs()
 			if len(acmeServer.Resolvers) == 0 {
 				return nil, h.Errf("must specify at least one resolver address")
 			}
+		case "challenges":
+			acmeServer.Challenges = append(acmeServer.Challenges, stringToChallenges(h.RemainingArgs())...)
+		default:
+			return nil, h.Errf("unrecognized ACME server directive: %s", h.Val())
 		}
 	}
 
diff --git a/modules/caddypki/acmeserver/challenges.go b/modules/caddypki/acmeserver/challenges.go
new file mode 100644
index 000000000..728a74928
--- /dev/null
+++ b/modules/caddypki/acmeserver/challenges.go
@@ -0,0 +1,77 @@
+package acmeserver
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+
+	"github.com/smallstep/certificates/authority/provisioner"
+)
+
+// ACMEChallenge is an opaque string that represents supported ACME challenges.
+type ACMEChallenge string
+
+const (
+	HTTP_01     ACMEChallenge = "http-01"
+	DNS_01      ACMEChallenge = "dns-01"
+	TLS_ALPN_01 ACMEChallenge = "tls-alpn-01"
+)
+
+// validate checks if the given challenge is supported.
+func (c ACMEChallenge) validate() error {
+	switch c {
+	case HTTP_01, DNS_01, TLS_ALPN_01:
+		return nil
+	default:
+		return fmt.Errorf("acme challenge %q is not supported", c)
+	}
+}
+
+// The unmarshaller first marshals the value into a string. Then it
+// trims any space around it and lowercase it for normaliztion. The
+// method does not and should not validate the value within accepted enums.
+func (c *ACMEChallenge) UnmarshalJSON(b []byte) error {
+	var s string
+	if err := json.Unmarshal(b, &s); err != nil {
+		return err
+	}
+	*c = ACMEChallenge(strings.ToLower(strings.TrimSpace(s)))
+	return nil
+}
+
+// String returns a string representation of the challenge.
+func (c ACMEChallenge) String() string {
+	return strings.ToLower(string(c))
+}
+
+// ACMEChallenges is a list of ACME challenges.
+type ACMEChallenges []ACMEChallenge
+
+// validate checks if the given challenges are supported.
+func (c ACMEChallenges) validate() error {
+	for _, ch := range c {
+		if err := ch.validate(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (c ACMEChallenges) toSmallstepType() []provisioner.ACMEChallenge {
+	if len(c) == 0 {
+		return nil
+	}
+	ac := make([]provisioner.ACMEChallenge, len(c))
+	for i, ch := range c {
+		ac[i] = provisioner.ACMEChallenge(ch)
+	}
+	return ac
+}
+
+func stringToChallenges(chs []string) ACMEChallenges {
+	challenges := make(ACMEChallenges, len(chs))
+	for i, ch := range chs {
+		challenges[i] = ACMEChallenge(ch)
+	}
+	return challenges
+}