From e1aa862e6a951d4dc9c0e89b6d42f7fea1c5aba7 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Thu, 8 Feb 2024 11:42:03 +0300 Subject: [PATCH] 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 --------- Co-authored-by: Matt Holt --- caddytest/integration/acme_test.go | 261 ++++++++++++++++++ .../acme_server_custom_challenges.txt | 65 +++++ .../acme_server_default_challenges.txt | 62 +++++ .../acme_server_multi_custom_challenges.txt | 66 +++++ modules/caddypki/acmeserver/acmeserver.go | 15 +- modules/caddypki/acmeserver/caddyfile.go | 7 +- modules/caddypki/acmeserver/challenges.go | 77 ++++++ 7 files changed, 549 insertions(+), 4 deletions(-) create mode 100644 caddytest/integration/acme_test.go create mode 100644 caddytest/integration/caddyfile_adapt/acme_server_custom_challenges.txt create mode 100644 caddytest/integration/caddyfile_adapt/acme_server_default_challenges.txt create mode 100644 caddytest/integration/caddyfile_adapt/acme_server_multi_custom_challenges.txt create mode 100644 modules/caddypki/acmeserver/challenges.go diff --git a/caddytest/integration/acme_test.go b/caddytest/integration/acme_test.go new file mode 100644 index 00000000000..00a3a6c4e96 --- /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:you@example.com"}, + 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:you@example.com"}, + 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 00000000000..2a7a5149243 --- /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 00000000000..26d345047c9 --- /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 00000000000..7fe3ca663db --- /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 8ac1b6c892e..e0588c527a8 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 51290eba612..864a94c534c 100644 --- a/modules/caddypki/acmeserver/caddyfile.go +++ b/modules/caddypki/acmeserver/caddyfile.go @@ -32,6 +32,7 @@ func init() { // ca // lifetime // resolvers +// 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 00000000000..728a7492813 --- /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 +}