Skip to content

Commit

Permalink
Fix custom resolver implementation for Windows (#255)
Browse files Browse the repository at this point in the history
Pebble accepts a `--dnsserver` flag, that allows to select a custom
DNS resolver to use when validating the ACME challenges. This option
is critical to make the certbot integration tests work in particular.

Current implementation is to set the `net.DefaultResolver` on a new
`net.Resolver` instance with a custom `Dial` property to consume the
value from `--dnsserver`. This allows to transparently resolve any
host using this custom DNS resolver from a pure-Go implementation of
the DNS resolution API.

Sadly this approach does not work on Windows, and the `--dnsserver`
has no effect on how resolution is done, and it is always the OS
mechanism that is used.

One can reveal this behavior by running the following piece of code on
Linux and on Windows:

```go
// test.go
package main

import (
	"context"
	"fmt"
	"net"
)

func main() {
	resolver := &net.Resolver{
		PreferGo: true,
		Dial: func(ctx context.Context, _, _ string) (net.Conn, error) {
			d := net.Dialer{}
			return d.DialContext(ctx, "udp", "4.3.2.1:404")
		},
	}
	net.DefaultResolver = resolver
	ctx, cancelfunc := context.WithTimeout(context.Background(), 30)
	defer cancelfunc()
	_, err := resolver.LookupTXT(ctx, "stupid.entry.net")

	fmt.Printf("Error is: %s\n", err)
}
```

This piece of code tries to resolve a non-existent domain on a
non-existent DNS server as IP `4.3.2.1:404`.

On Linux, you will get the following error:

```bash
Error is: lookup stupid.entry.net on 127.0.0.53:53: dial udp 4.3.2.1:404: i/o timeout
```

That indicates that the system tried to reach the non-existent DNS server,
and get back a timeout exception on it.

However on Windows you will get:
```bash
Error is: lookup stupid.entry.net: dnsquery: DNS name does not exist.
```

This indicates that the system ignored the dummy DNS server address,
contacted the OS DNS resolver, that responded that the DNS name does
not exist.

One can see also the reason for this behavior on Windows on the `net`
godoc, https://godoc.org/net, in particular this line in the module
introduction:

```
On Windows, the resolver always uses C library functions, such as GetAddrInfo and DnsQuery.
```

In fact, the pure Go DNS resolver is not applicable on Windows, the OS
DNS resolver will be used, ignoring any customization.

Several relevant discussions, in particular a proposal (not developed yet)
to make the pure Go DNS resolver available on Windows:

* golang/go#29621
* golang/go#33097
* golang/go#33086

To fix this, this PR makes Pebble switch to a different logic:
* if `-dnsserver` is not set, use the default API to resolve the names
* if `-dnsserver` is set, use a dedicated DNS client, to avoid to use
the OS one both on Linux and Windows

The DNS client is https://github.com/miekg/dns, a highly used and
supported DNS library.

With these modifications, integrations tests on Certbot are running
correctly both on Linux and Windows.
  • Loading branch information
adferrand authored and jsha committed Jul 29, 2019
1 parent ae250d5 commit ce13973
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 37 deletions.
18 changes: 1 addition & 17 deletions cmd/pebble/main.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package main

import (
"context"
"flag"
"log"
"net"
"net/http"
"os"
"strconv"
Expand Down Expand Up @@ -55,10 +53,6 @@ func main() {
err := cmd.ReadConfigFile(*configFile, &c)
cmd.FailOnError(err, "Reading JSON config file into config structure")

if len(*resolverAddress) > 0 {
setupCustomDNSResolver(*resolverAddress)
}

alternateRoots := 0
alternateRootsVal := os.Getenv("PEBBLE_ALTERNATE_ROOTS")
if val, err := strconv.ParseInt(alternateRootsVal, 10, 0); err == nil && val >= 0 {
Expand All @@ -67,7 +61,7 @@ func main() {

db := db.NewMemoryStore()
ca := ca.New(logger, db, c.Pebble.OCSPResponderURL, alternateRoots)
va := va.New(logger, c.Pebble.HTTPPort, c.Pebble.TLSPort, *strictMode)
va := va.New(logger, c.Pebble.HTTPPort, c.Pebble.TLSPort, *strictMode, *resolverAddress)

wfeImpl := wfe.New(logger, db, va, ca, *strictMode)
muxHandler := wfeImpl.Handler()
Expand Down Expand Up @@ -103,13 +97,3 @@ func main() {
muxHandler)
cmd.FailOnError(err, "Calling ListenAndServeTLS()")
}

func setupCustomDNSResolver(dnsResolverAddress string) {
net.DefaultResolver = &net.Resolver{
PreferGo: true,
Dial: func(ctx context.Context, _, _ string) (net.Conn, error) {
d := net.Dialer{}
return d.DialContext(ctx, "udp", dnsResolverAddress)
},
}
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module github.com/letsencrypt/pebble

require (
github.com/letsencrypt/challtestsrv v1.1.0
github.com/miekg/dns v1.1.15
golang.org/x/net v0.0.0-20181207154023-610586996380 // indirect
golang.org/x/sys v0.0.0-20181206074257-70b957f3b65e // indirect
gopkg.in/square/go-jose.v2 v2.1.9
Expand Down
148 changes: 128 additions & 20 deletions va/va.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"strings"
"time"

"github.com/miekg/dns"

"github.com/letsencrypt/challtestsrv"
"github.com/letsencrypt/pebble/acme"
"github.com/letsencrypt/pebble/core"
Expand Down Expand Up @@ -93,28 +95,38 @@ type vaTask struct {
}

type VAImpl struct {
log *log.Logger
httpPort int
tlsPort int
tasks chan *vaTask
sleep bool
sleepTime int
alwaysValid bool
strict bool
log *log.Logger
httpPort int
tlsPort int
tasks chan *vaTask
sleep bool
sleepTime int
alwaysValid bool
strict bool
customResolverAddr string
dnsClient *dns.Client
}

func New(
log *log.Logger,
httpPort, tlsPort int,
strict bool) *VAImpl {
strict bool, customResolverAddr string) *VAImpl {
va := &VAImpl{
log: log,
httpPort: httpPort,
tlsPort: tlsPort,
tasks: make(chan *vaTask, taskQueueSize),
sleep: true,
sleepTime: defaultSleepTime,
strict: strict,
log: log,
httpPort: httpPort,
tlsPort: tlsPort,
tasks: make(chan *vaTask, taskQueueSize),
sleep: true,
sleepTime: defaultSleepTime,
strict: strict,
customResolverAddr: customResolverAddr,
}

if customResolverAddr != "" {
va.log.Printf("Using custom DNS resolver for ACME challenges: %s", customResolverAddr)
va.dnsClient = new(dns.Client)
} else {
va.log.Print("Using system DNS resolver for ACME challenges")
}

// Read the PEBBLE_VA_NOSLEEP environment variable string
Expand Down Expand Up @@ -299,10 +311,7 @@ func (va VAImpl) validateDNS01(task *vaTask) *core.ValidationRecord {
ValidatedAt: time.Now(),
}

ctx, cancelfunc := context.WithTimeout(context.Background(), validationTimeout)
defer cancelfunc()

txts, err := net.DefaultResolver.LookupTXT(ctx, challengeSubdomain)
txts, err := va.getTXTEntry(challengeSubdomain)
if err != nil {
result.Error = acme.UnauthorizedProblem(fmt.Sprintf("Error retrieving TXT records for DNS challenge (%q)", err))
return result
Expand Down Expand Up @@ -490,6 +499,18 @@ func (va VAImpl) fetchHTTP(identifier string, token string) ([]byte, string, *ac
httpRequest.Header.Set("User-Agent", userAgent())
httpRequest.Header.Set("Accept", "*/*")

addrs, err := va.resolveIP(identifier)

if err != nil {
return nil, url.String(), acme.MalformedProblem(
fmt.Sprintf("Error occurred while resolving URL %q: %q", url.String(), err))
}

if len(addrs) == 0 {
return nil, url.String(), acme.MalformedProblem(
fmt.Sprintf("Could not resolve URL %q", url.String()))
}

transport := &http.Transport{
// We don't expect to make multiple requests to a client, so close
// connection immediately.
Expand All @@ -501,6 +522,12 @@ func (va VAImpl) fetchHTTP(identifier string, token string) ([]byte, string, *ac
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},

// Control specifically which IP will be used for this request
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
dialer := &net.Dialer{}
return dialer.DialContext(ctx, network, net.JoinHostPort(addrs[0], portString))
},
}

client := &http.Client{
Expand Down Expand Up @@ -534,6 +561,87 @@ func (va VAImpl) fetchHTTP(identifier string, token string) ([]byte, string, *ac
return body, url.String(), nil
}

// getTXTEntry fetches TXT entries for the given domain name using the recursive resolver located at
// `va.customResolverAddr`, or the default system resolver if no custom resolver addr is specified
func (va VAImpl) getTXTEntry(name string) ([]string, error) {
ctx, cancelfunc := context.WithTimeout(context.Background(), validationTimeout)
defer cancelfunc()

if va.customResolverAddr == "" {
return net.DefaultResolver.LookupTXT(ctx, name)
}

var txts []string
message := new(dns.Msg)
message.SetQuestion(dns.Fqdn(name), dns.TypeTXT)
in, _, err := va.dnsClient.ExchangeContext(ctx, message, va.customResolverAddr)

if err != nil {
return nil, err
}

if in.Rcode != dns.RcodeSuccess {
return nil, fmt.Errorf("DNS lookup for %q returned an unsuccessful response: %q", name, in.Rcode)
}

for _, record := range in.Answer {
if t, ok := record.(*dns.TXT); ok {
txts = append(txts, t.Txt...)
}
}

return txts, nil
}

// resolveIP find all IPs for the given domain name using the recursive resolver located at
// `va.customResolverAddr`, or the default system resolver if no custom resolver addr is specified
func (va VAImpl) resolveIP(name string) ([]string, error) {
ctx, cancelfunc := context.WithTimeout(context.Background(), validationTimeout)
defer cancelfunc()

if va.customResolverAddr == "" {
return net.DefaultResolver.LookupHost(ctx, name)
}

// Check if the given name is not already an IP. If it is the case, just return it untouched.
addrs := []string{}
parsed := net.ParseIP(name)
if parsed != nil {
addrs = append(addrs, name)
return addrs, nil
}

messageAAAA := new(dns.Msg)
messageAAAA.SetQuestion(dns.Fqdn(name), dns.TypeAAAA)
inAAAA, _, err := va.dnsClient.ExchangeContext(ctx, messageAAAA, va.customResolverAddr)

if err != nil {
return nil, err
}

for _, record := range inAAAA.Answer {
if t, ok := record.(*dns.AAAA); ok {
addrs = append(addrs, t.AAAA.String())
}
}

messageA := new(dns.Msg)
messageA.SetQuestion(dns.Fqdn(name), dns.TypeA)
inA, _, err := va.dnsClient.ExchangeContext(ctx, messageA, va.customResolverAddr)

if err != nil {
return nil, err
}

for _, record := range inA.Answer {
if t, ok := record.(*dns.A); ok {
addrs = append(addrs, t.A.String())
}
}

return addrs, nil
}

// reverseaddr function is borrowed from net/dnsclient.go[0] and the Go std library.
// [0]: https://golang.org/src/net/dnsclient.go
func reverseaddr(addr string) string {
Expand Down

0 comments on commit ce13973

Please sign in to comment.