Repository: caddyserver/caddy (language: go)

Issue:
Allow configuring DNS resolvers used by the internal acme_server to verify DNS challenges
When the internal ACME server has to verify a `DNS-01` challenge, it goes through the system resolver using  `net.LookupTXT`. It would be nice to have the ability to configure a set of name servers / resolvers to use instead of the system resolver. This setting already exists for when Caddy is acting as an ACME *client* (see [here](https://caddyserver.com/docs/json/apps/tls/automation/policies/issuers/acme/#challenges/dns/resolvers)), so it stands to reason they might be useful on the server side, too.

In this case, it came up while I was trying to come up with a way to test the new DNS server plugin I'm writing.

I've had a quick look at how this would be implemented, and unfortunately it might not be entirely straight-forward. The smallstep library *does* allow overriding the resolvers it uses to verify challenges, but not for the method Caddy is using.

Caddy uses [`smallstep/certificates/acme/api.NewHandler`](https://pkg.go.dev/github.com/smallstep/certificates@v0.23.2/acme/api#NewHandler) [here](https://github.com/caddyserver/caddy/blob/b6fe5d4b41d07e70a502ed58d40e8b0e75067db5/modules/caddypki/acmeserver/acmeserver.go#L159):
```go
acmeRouterHandler := acmeAPI.NewHandler(acmeAPI.HandlerOptions{
	CA:     auth,
	DB:     acmeDB,                            // stores all the server state
	DNS:    ash.Host,                          // used for directory links
	Prefix: strings.Trim(ash.PathPrefix, "/"), // used for directory links
})
```
This handler will create a default [`Client` ](https://pkg.go.dev/github.com/smallstep/certificates@v0.23.2/acme#Client) (see [this file](https://github.com/smallstep/certificates/blob/3c76834807fb38ff4ca0ee3ab12c617fd808f7cf/acme/api/handler.go#L77)), which is implemented [here](https://github.com/smallstep/certificates/blob/3c76834807fb38ff4ca0ee3ab12c617fd808f7cf/acme/client.go) (as `client` in lowercase, somewhat confusing), and looks pretty simple:
```go
func (c *client) Get(url string) (*http.Response, error) {
	return c.http.Get(url)
}

func (c *client) LookupTxt(name string) ([]string, error) {
	return net.LookupTXT(name)
}

func (c *client) TLSDial(network, addr string, config *tls.Config) (*tls.Conn, error) {
	return tls.DialWithDialer(c.dialer, network, addr, config)
}
```
I think a new [`Client` ](https://pkg.go.dev/github.com/smallstep/certificates@v0.23.2/acme#Client) would need to be implemented, with a different `LookupTxt`.

Then, the `acme_server` module [`Handler.Provision`](https://github.com/caddyserver/caddy/blob/b6fe5d4b41d07e70a502ed58d40e8b0e75067db5/modules/caddypki/acmeserver/acmeserver.go#L92) would need to switch from using [`smallstep/certificates/acme/api.NewHandler`](https://pkg.go.dev/github.com/smallstep/certificates@v0.23.2/acme/api#NewHandler) (which is sort of half-deprecated anyway) to the recommended [`smallstep/certificates/acme/api.Route`](https://pkg.go.dev/github.com/smallstep/certificates@v0.23.2/acme/api#Route) (or maybe it's  [`smallstep/certificates/api.Route`](https://pkg.go.dev/github.com/smallstep/certificates@v0.23.2/api#Route)? very confusing!). If you check the docs you'll see that this method is configured differently: it "requires that all the acme components, authority, db, client, linker, and prerequisite checker [are] present in the context". [`smallstep/certificates/acme/NewContext`](https://pkg.go.dev/github.com/smallstep/certificates/acme@v0.23.2#NewContext) is provided to add these things into the context. But, from what I can tell, it's not a 1-to-1 translation from the [`HandlerOptions`](https://pkg.go.dev/github.com/smallstep/certificates@v0.23.2/acme/api#HandlerOptions) (the way it is configured currently) to the things you can put into the context.
Finally, it looks like this context needs to be glued onto every incoming `http.Request` somehow - I assume this is a common Go idiom.


Discussion / hints:
@maraino Just wanted to put this on your radar; I haven't spent much time looking into this so I'm not sure what the best approach is, maybe this is something that can be made configurable in Smallstep?
@francislavoie we allow to change the resolver, but I'm not sure if it is exposed, let me check
I've checked and the way we do it is changing the default resolver:
```go
net.DefaultResolver.PreferGo = true
net.DefaultResolver.Dial = func(ctx context.Context, network, address string) (net.Conn, error) {
	return net.Dial(network, resolver)
}
```
Hmm, that has side effects. I guess wrapping `acme.Client` would probably be okay.
Yes, any custom implementation of the client interface will do it:
```go
type Client interface {
	Get(url string) (*http.Response, error)
	LookupTxt(name string) ([]string, error)
	TLSDial(network, addr string, config *tls.Config) (*tls.Conn, error)
}
```

For example, for the `Get`, you will need an `http.Client`, which transport is using a `net.Dialer` with a custom resolver.

Fix patch:
diff --git a/listeners.go b/listeners.go
index 7c5401a65af..14a5c49919d 100644
--- a/listeners.go
+++ b/listeners.go
@@ -303,13 +303,19 @@ func IsUnixNetwork(netw string) bool {
 // Network addresses are distinct from URLs and do not
 // use URL syntax.
 func ParseNetworkAddress(addr string) (NetworkAddress, error) {
+	return ParseNetworkAddressWithDefaults(addr, "tcp", 0)
+}
+
+// ParseNetworkAddressWithDefaults is like ParseNetworkAddress but allows
+// the default network and port to be specified.
+func ParseNetworkAddressWithDefaults(addr, defaultNetwork string, defaultPort uint) (NetworkAddress, error) {
 	var host, port string
 	network, host, port, err := SplitNetworkAddress(addr)
 	if err != nil {
 		return NetworkAddress{}, err
 	}
 	if network == "" {
-		network = "tcp"
+		network = defaultNetwork
 	}
 	if IsUnixNetwork(network) {
 		return NetworkAddress{
@@ -318,7 +324,10 @@ func ParseNetworkAddress(addr string) (NetworkAddress, error) {
 		}, nil
 	}
 	var start, end uint64
-	if port != "" {
+	if port == "" {
+		start = uint64(defaultPort)
+		end = uint64(defaultPort)
+	} else {
 		before, after, found := strings.Cut(port, "-")
 		if !found {
 			after = before
diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go
index 30bd7b5753c..42fedb651bd 100644
--- a/modules/caddyhttp/reverseproxy/upstreams.go
+++ b/modules/caddyhttp/reverseproxy/upstreams.go
@@ -8,7 +8,6 @@ import (
 	"net"
 	"net/http"
 	"strconv"
-	"strings"
 	"sync"
 	"time"
 
@@ -471,16 +470,9 @@ type UpstreamResolver struct {
 // and ensures they're ready to be used.
 func (u *UpstreamResolver) ParseAddresses() error {
 	for _, v := range u.Addresses {
-		addr, err := caddy.ParseNetworkAddress(v)
+		addr, err := caddy.ParseNetworkAddressWithDefaults(v, "udp", 53)
 		if err != nil {
-			// If a port wasn't specified for the resolver,
-			// try defaulting to 53 and parse again
-			if strings.Contains(err.Error(), "missing port in address") {
-				addr, err = caddy.ParseNetworkAddress(v + ":53")
-			}
-			if err != nil {
-				return err
-			}
+			return err
 		}
 		if addr.PortRangeSize() != 1 {
 			return fmt.Errorf("resolver address must have exactly one address; cannot call %v", addr)
diff --git a/modules/caddypki/acmeserver/acmeserver.go b/modules/caddypki/acmeserver/acmeserver.go
index 6ecdfdc66b1..0f739ec319c 100644
--- a/modules/caddypki/acmeserver/acmeserver.go
+++ b/modules/caddypki/acmeserver/acmeserver.go
@@ -15,7 +15,10 @@
 package acmeserver
 
 import (
+	"context"
 	"fmt"
+	weakrand "math/rand"
+	"net"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -28,7 +31,7 @@ import (
 	"github.com/caddyserver/caddy/v2/modules/caddypki"
 	"github.com/go-chi/chi"
 	"github.com/smallstep/certificates/acme"
-	acmeAPI "github.com/smallstep/certificates/acme/api"
+	"github.com/smallstep/certificates/acme/api"
 	acmeNoSQL "github.com/smallstep/certificates/acme/db/nosql"
 	"github.com/smallstep/certificates/authority"
 	"github.com/smallstep/certificates/authority/provisioner"
@@ -76,8 +79,26 @@ type Handler struct {
 	// changed or removed in the future.
 	SignWithRoot bool `json:"sign_with_root,omitempty"`
 
+	// The addresses of DNS resolvers to use when looking up
+	// the TXT records for solving DNS challenges.
+	// It accepts [network addresses](/docs/conventions#network-addresses)
+	// with port range of only 1. If the host is an IP address,
+	// it will be dialed directly to resolve the upstream server.
+	// If the host is not an IP address, the addresses are resolved
+	// using the [name resolution convention](https://golang.org/pkg/net/#hdr-Name_Resolution)
+	// of the Go standard library. If the array contains more
+	// than 1 resolver address, one is chosen at random.
+	Resolvers []string `json:"resolvers,omitempty"`
+
+	logger    *zap.Logger
+	resolvers []caddy.NetworkAddress
+	ctx       caddy.Context
+
+	acmeDB        acme.DB
+	acmeAuth      *authority.Authority
+	acmeClient    acme.Client
+	acmeLinker    acme.Linker
 	acmeEndpoints http.Handler
-	logger        *zap.Logger
 }
 
 // CaddyModule returns the Caddy module information.
@@ -90,7 +111,9 @@ func (Handler) CaddyModule() caddy.ModuleInfo {
 
 // Provision sets up the ACME server handler.
 func (ash *Handler) Provision(ctx caddy.Context) error {
+	ash.ctx = ctx
 	ash.logger = ctx.Logger()
+
 	// set some defaults
 	if ash.CA == "" {
 		ash.CA = caddypki.DefaultCAID
@@ -142,31 +165,30 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 		DB: database,
 	}
 
-	auth, err := ca.NewAuthority(authorityConfig)
+	ash.acmeAuth, err = ca.NewAuthority(authorityConfig)
 	if err != nil {
 		return err
 	}
 
-	var acmeDB acme.DB
-	if authorityConfig.DB != nil {
-		acmeDB, err = acmeNoSQL.New(auth.GetDatabase().(nosql.DB))
-		if err != nil {
-			return fmt.Errorf("configuring ACME DB: %v", err)
-		}
+	ash.acmeDB, err = acmeNoSQL.New(ash.acmeAuth.GetDatabase().(nosql.DB))
+	if err != nil {
+		return fmt.Errorf("configuring ACME DB: %v", err)
 	}
 
-	// create the router for the ACME endpoints
-	acmeRouterHandler := acmeAPI.NewHandler(acmeAPI.HandlerOptions{
-		CA:     auth,
-		DB:     acmeDB,                            // stores all the server state
-		DNS:    ash.Host,                          // used for directory links
-		Prefix: strings.Trim(ash.PathPrefix, "/"), // used for directory links
-	})
+	ash.acmeClient, err = ash.makeClient()
+	if err != nil {
+		return err
+	}
+
+	ash.acmeLinker = acme.NewLinker(
+		ash.Host,
+		strings.Trim(ash.PathPrefix, "/"),
+	)
 
 	// extract its http.Handler so we can use it directly
 	r := chi.NewRouter()
 	r.Route(ash.PathPrefix, func(r chi.Router) {
-		acmeRouterHandler.Route(r)
+		api.Route(r)
 	})
 	ash.acmeEndpoints = r
 
@@ -175,6 +197,16 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 
 func (ash Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
 	if strings.HasPrefix(r.URL.Path, ash.PathPrefix) {
+		acmeCtx := acme.NewContext(
+			r.Context(),
+			ash.acmeDB,
+			ash.acmeClient,
+			ash.acmeLinker,
+			nil,
+		)
+		acmeCtx = authority.NewContext(acmeCtx, ash.acmeAuth)
+		r = r.WithContext(acmeCtx)
+
 		ash.acmeEndpoints.ServeHTTP(w, r)
 		return nil
 	}
@@ -227,6 +259,55 @@ func (ash Handler) openDatabase() (*db.AuthDB, error) {
 	return database.(databaseCloser).DB, err
 }
 
+// makeClient creates an ACME client which will use a custom
+// resolver instead of net.DefaultResolver.
+func (ash Handler) makeClient() (acme.Client, error) {
+	for _, v := range ash.Resolvers {
+		addr, err := caddy.ParseNetworkAddressWithDefaults(v, "udp", 53)
+		if err != nil {
+			return nil, err
+		}
+		if addr.PortRangeSize() != 1 {
+			return nil, fmt.Errorf("resolver address must have exactly one address; cannot call %v", addr)
+		}
+		ash.resolvers = append(ash.resolvers, addr)
+	}
+
+	var resolver *net.Resolver
+	if len(ash.resolvers) != 0 {
+		dialer := &net.Dialer{
+			Timeout: 2 * time.Second,
+		}
+		resolver = &net.Resolver{
+			PreferGo: true,
+			Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
+				//nolint:gosec
+				addr := ash.resolvers[weakrand.Intn(len(ash.resolvers))]
+				return dialer.DialContext(ctx, addr.Network, addr.JoinHostPort(0))
+			},
+		}
+	} else {
+		resolver = net.DefaultResolver
+	}
+
+	return resolverClient{
+		Client:   acme.NewClient(),
+		resolver: resolver,
+		ctx:      ash.ctx,
+	}, nil
+}
+
+type resolverClient struct {
+	acme.Client
+
+	resolver *net.Resolver
+	ctx      context.Context
+}
+
+func (c resolverClient) LookupTxt(name string) ([]string, error) {
+	return c.resolver.LookupTXT(c.ctx, name)
+}
+
 const defaultPathPrefix = "/acme/"
 
 var keyCleaner = regexp.MustCompile(`[^\w.-_]`)
diff --git a/modules/caddypki/acmeserver/caddyfile.go b/modules/caddypki/acmeserver/caddyfile.go
index ae2d8ef1184..3b52113b527 100644
--- a/modules/caddypki/acmeserver/caddyfile.go
+++ b/modules/caddypki/acmeserver/caddyfile.go
@@ -29,8 +29,9 @@ func init() {
 // parseACMEServer sets up an ACME server handler from Caddyfile tokens.
 //
 //	acme_server [<matcher>] {
-//	    ca <id>
-//	    lifetime <duration>
+//		ca        <id>
+//		lifetime  <duration>
+//		resolvers <addresses...>
 //	}
 func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
 	if !h.Next() {
@@ -74,6 +75,12 @@ func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 				}
 
 				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")
+				}
 			}
 		}
 	}


Test patch:
diff --git a/listeners_test.go b/listeners_test.go
index c5aa5273a07..5508a9f08a4 100644
--- a/listeners_test.go
+++ b/listeners_test.go
@@ -175,47 +175,57 @@ func TestJoinNetworkAddress(t *testing.T) {
 
 func TestParseNetworkAddress(t *testing.T) {
 	for i, tc := range []struct {
-		input      string
-		expectAddr NetworkAddress
-		expectErr  bool
+		input          string
+		defaultNetwork string
+		defaultPort    uint
+		expectAddr     NetworkAddress
+		expectErr      bool
 	}{
 		{
 			input:     "",
 			expectErr: true,
 		},
 		{
-			input: ":",
+			input:          ":",
+			defaultNetwork: "udp",
 			expectAddr: NetworkAddress{
-				Network: "tcp",
+				Network: "udp",
 			},
 		},
 		{
-			input: "[::]",
+			input:          "[::]",
+			defaultNetwork: "udp",
+			defaultPort:    53,
 			expectAddr: NetworkAddress{
-				Network: "tcp",
-				Host:    "::",
+				Network:   "udp",
+				Host:      "::",
+				StartPort: 53,
+				EndPort:   53,
 			},
 		},
 		{
-			input: ":1234",
+			input:          ":1234",
+			defaultNetwork: "udp",
 			expectAddr: NetworkAddress{
-				Network:   "tcp",
+				Network:   "udp",
 				Host:      "",
 				StartPort: 1234,
 				EndPort:   1234,
 			},
 		},
 		{
-			input: "tcp/:1234",
+			input:          "udp/:1234",
+			defaultNetwork: "udp",
 			expectAddr: NetworkAddress{
-				Network:   "tcp",
+				Network:   "udp",
 				Host:      "",
 				StartPort: 1234,
 				EndPort:   1234,
 			},
 		},
 		{
-			input: "tcp6/:1234",
+			input:          "tcp6/:1234",
+			defaultNetwork: "tcp",
 			expectAddr: NetworkAddress{
 				Network:   "tcp6",
 				Host:      "",
@@ -224,7 +234,8 @@ func TestParseNetworkAddress(t *testing.T) {
 			},
 		},
 		{
-			input: "tcp4/localhost:1234",
+			input:          "tcp4/localhost:1234",
+			defaultNetwork: "tcp",
 			expectAddr: NetworkAddress{
 				Network:   "tcp4",
 				Host:      "localhost",
@@ -233,14 +244,16 @@ func TestParseNetworkAddress(t *testing.T) {
 			},
 		},
 		{
-			input: "unix//foo/bar",
+			input:          "unix//foo/bar",
+			defaultNetwork: "tcp",
 			expectAddr: NetworkAddress{
 				Network: "unix",
 				Host:    "/foo/bar",
 			},
 		},
 		{
-			input: "localhost:1234-1234",
+			input:          "localhost:1234-1234",
+			defaultNetwork: "tcp",
 			expectAddr: NetworkAddress{
 				Network:   "tcp",
 				Host:      "localhost",
@@ -249,11 +262,13 @@ func TestParseNetworkAddress(t *testing.T) {
 			},
 		},
 		{
-			input:     "localhost:2-1",
-			expectErr: true,
+			input:          "localhost:2-1",
+			defaultNetwork: "tcp",
+			expectErr:      true,
 		},
 		{
-			input: "localhost:0",
+			input:          "localhost:0",
+			defaultNetwork: "tcp",
 			expectAddr: NetworkAddress{
 				Network:   "tcp",
 				Host:      "localhost",
@@ -262,11 +277,138 @@ func TestParseNetworkAddress(t *testing.T) {
 			},
 		},
 		{
-			input:     "localhost:1-999999999999",
+			input:          "localhost:1-999999999999",
+			defaultNetwork: "tcp",
+			expectErr:      true,
+		},
+	} {
+		actualAddr, err := ParseNetworkAddressWithDefaults(tc.input, tc.defaultNetwork, tc.defaultPort)
+		if tc.expectErr && err == nil {
+			t.Errorf("Test %d: Expected error but got: %v", i, err)
+		}
+		if !tc.expectErr && err != nil {
+			t.Errorf("Test %d: Expected no error but got: %v", i, err)
+		}
+
+		if actualAddr.Network != tc.expectAddr.Network {
+			t.Errorf("Test %d: Expected network '%v' but got '%v'", i, tc.expectAddr, actualAddr)
+		}
+		if !reflect.DeepEqual(tc.expectAddr, actualAddr) {
+			t.Errorf("Test %d: Expected addresses %v but got %v", i, tc.expectAddr, actualAddr)
+		}
+	}
+}
+
+func TestParseNetworkAddressWithDefaults(t *testing.T) {
+	for i, tc := range []struct {
+		input          string
+		defaultNetwork string
+		defaultPort    uint
+		expectAddr     NetworkAddress
+		expectErr      bool
+	}{
+		{
+			input:     "",
 			expectErr: true,
 		},
+		{
+			input:          ":",
+			defaultNetwork: "udp",
+			expectAddr: NetworkAddress{
+				Network: "udp",
+			},
+		},
+		{
+			input:          "[::]",
+			defaultNetwork: "udp",
+			defaultPort:    53,
+			expectAddr: NetworkAddress{
+				Network:   "udp",
+				Host:      "::",
+				StartPort: 53,
+				EndPort:   53,
+			},
+		},
+		{
+			input:          ":1234",
+			defaultNetwork: "udp",
+			expectAddr: NetworkAddress{
+				Network:   "udp",
+				Host:      "",
+				StartPort: 1234,
+				EndPort:   1234,
+			},
+		},
+		{
+			input:          "udp/:1234",
+			defaultNetwork: "udp",
+			expectAddr: NetworkAddress{
+				Network:   "udp",
+				Host:      "",
+				StartPort: 1234,
+				EndPort:   1234,
+			},
+		},
+		{
+			input:          "tcp6/:1234",
+			defaultNetwork: "tcp",
+			expectAddr: NetworkAddress{
+				Network:   "tcp6",
+				Host:      "",
+				StartPort: 1234,
+				EndPort:   1234,
+			},
+		},
+		{
+			input:          "tcp4/localhost:1234",
+			defaultNetwork: "tcp",
+			expectAddr: NetworkAddress{
+				Network:   "tcp4",
+				Host:      "localhost",
+				StartPort: 1234,
+				EndPort:   1234,
+			},
+		},
+		{
+			input:          "unix//foo/bar",
+			defaultNetwork: "tcp",
+			expectAddr: NetworkAddress{
+				Network: "unix",
+				Host:    "/foo/bar",
+			},
+		},
+		{
+			input:          "localhost:1234-1234",
+			defaultNetwork: "tcp",
+			expectAddr: NetworkAddress{
+				Network:   "tcp",
+				Host:      "localhost",
+				StartPort: 1234,
+				EndPort:   1234,
+			},
+		},
+		{
+			input:          "localhost:2-1",
+			defaultNetwork: "tcp",
+			expectErr:      true,
+		},
+		{
+			input:          "localhost:0",
+			defaultNetwork: "tcp",
+			expectAddr: NetworkAddress{
+				Network:   "tcp",
+				Host:      "localhost",
+				StartPort: 0,
+				EndPort:   0,
+			},
+		},
+		{
+			input:          "localhost:1-999999999999",
+			defaultNetwork: "tcp",
+			expectErr:      true,
+		},
 	} {
-		actualAddr, err := ParseNetworkAddress(tc.input)
+		actualAddr, err := ParseNetworkAddressWithDefaults(tc.input, tc.defaultNetwork, tc.defaultPort)
 		if tc.expectErr && err == nil {
 			t.Errorf("Test %d: Expected error but got: %v", i, err)
 		}