Skip to content

Commit

Permalink
Implement DoH for validation queries (#7178)
Browse files Browse the repository at this point in the history
Fixes: #7141
  • Loading branch information
jsha authored Dec 11, 2023
1 parent 44587c1 commit c21b376
Show file tree
Hide file tree
Showing 17 changed files with 195 additions and 40 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/boulder-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
matrix:
# Add additional docker image tags here and all tests will be run with the additional image.
BOULDER_TOOLS_TAG:
- go1.21.5_2023-12-05
- go1.21.5_2023-12-07
# Tests command definitions. Use the entire "docker compose" command you want to run.
tests:
# Run ./test.sh --help for a description of each of the flags.
Expand Down
86 changes: 77 additions & 9 deletions bdns/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package bdns

import (
"context"
"crypto/tls"
"encoding/base64"
"errors"
"fmt"
"io"
"net"
"net/http"
"strconv"
"strings"
"sync"
Expand All @@ -15,6 +18,7 @@ import (
"github.com/miekg/dns"
"github.com/prometheus/client_golang/prometheus"

"github.com/letsencrypt/boulder/features"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/metrics"
)
Expand Down Expand Up @@ -175,19 +179,36 @@ type exchanger interface {

// New constructs a new DNS resolver object that utilizes the
// provided list of DNS servers for resolution.
//
// `tlsConfig` is the configuration used for outbound DoH queries,
// if applicable.
func New(
readTimeout time.Duration,
servers ServerProvider,
stats prometheus.Registerer,
clk clock.Clock,
maxTries int,
log blog.Logger,
tlsConfig *tls.Config,
) Client {
dnsClient := new(dns.Client)

// Set timeout for underlying net.Conn
dnsClient.ReadTimeout = readTimeout
dnsClient.Net = "udp"
var client exchanger
if features.Enabled(features.DOH) {
client = &dohExchanger{
clk: clk,
hc: http.Client{
Timeout: readTimeout,
Transport: &http.Transport{
TLSClientConfig: tlsConfig,
},
},
}
} else {
client = &dns.Client{
// Set timeout for underlying net.Conn
ReadTimeout: readTimeout,
Net: "udp",
}
}

queryTime := prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Expand Down Expand Up @@ -220,9 +241,8 @@ func New(
[]string{"qtype", "resolver"},
)
stats.MustRegister(queryTime, totalLookupTime, timeoutCounter, idMismatchCounter)

return &impl{
dnsClient: dnsClient,
dnsClient: client,
servers: servers,
allowRestrictedAddresses: false,
maxTries: maxTries,
Expand All @@ -244,8 +264,10 @@ func NewTest(
stats prometheus.Registerer,
clk clock.Clock,
maxTries int,
log blog.Logger) Client {
resolver := New(readTimeout, servers, stats, clk, maxTries, log)
log blog.Logger,
tlsConfig *tls.Config,
) Client {
resolver := New(readTimeout, servers, stats, clk, maxTries, log, tlsConfig)
resolver.(*impl).allowRestrictedAddresses = true
return resolver
}
Expand Down Expand Up @@ -619,3 +641,49 @@ func logDNSError(
underlying)
}
}

type dohExchanger struct {
clk clock.Clock
hc http.Client
}

// Exchange sends a DoH query to the provided DoH server and returns the response.
func (d *dohExchanger) Exchange(query *dns.Msg, server string) (*dns.Msg, time.Duration, error) {
q, err := query.Pack()
if err != nil {
return nil, 0, err
}

// The default Unbound URL template
url := fmt.Sprintf("https://%s/dns-query", server)
req, err := http.NewRequest("POST", url, strings.NewReader(string(q)))
if err != nil {
return nil, 0, err
}
req.Header.Set("Content-Type", "application/dns-message")
req.Header.Set("Accept", "application/dns-message")

start := d.clk.Now()
resp, err := d.hc.Do(req)
if err != nil {
return nil, d.clk.Since(start), err
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return nil, d.clk.Since(start), fmt.Errorf("doh: http status %d", resp.StatusCode)
}

b, err := io.ReadAll(resp.Body)
if err != nil {
return nil, d.clk.Since(start), fmt.Errorf("doh: reading response body: %w", err)
}

response := new(dns.Msg)
err = response.Unpack(b)
if err != nil {
return nil, d.clk.Since(start), fmt.Errorf("doh: unpacking response: %w", err)
}

return response, d.clk.Since(start), nil
}
22 changes: 11 additions & 11 deletions bdns/dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func TestDNSNoServers(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)

_, err = obj.LookupHost(context.Background(), "letsencrypt.org")
test.AssertError(t, err, "No servers")
Expand All @@ -264,7 +264,7 @@ func TestDNSOneServer(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)

_, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org")

Expand All @@ -275,7 +275,7 @@ func TestDNSDuplicateServers(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr, dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)

_, err = obj.LookupHost(context.Background(), "cps.letsencrypt.org")

Expand All @@ -286,7 +286,7 @@ func TestDNSServFail(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)
bad := "servfail.com"

_, err = obj.LookupTXT(context.Background(), bad)
Expand All @@ -304,7 +304,7 @@ func TestDNSLookupTXT(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)

a, err := obj.LookupTXT(context.Background(), "letsencrypt.org")
t.Logf("A: %v", a)
Expand All @@ -321,7 +321,7 @@ func TestDNSLookupHost(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)

ip, err := obj.LookupHost(context.Background(), "servfail.com")
t.Logf("servfail.com - IP: %s, Err: %s", ip, err)
Expand Down Expand Up @@ -389,7 +389,7 @@ func TestDNSNXDOMAIN(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)

hostname := "nxdomain.letsencrypt.org"
_, err = obj.LookupHost(context.Background(), hostname)
Expand All @@ -405,7 +405,7 @@ func TestDNSLookupCAA(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock())
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, blog.UseMock(), nil)
removeIDExp := regexp.MustCompile(" id: [[:digit:]]+")

caas, resp, err := obj.LookupCAA(context.Background(), "bracewel.net")
Expand Down Expand Up @@ -639,7 +639,7 @@ func TestRetry(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

testClient := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), tc.maxTries, blog.UseMock())
testClient := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), tc.maxTries, blog.UseMock(), nil)
dr := testClient.(*impl)
dr.dnsClient = tc.te
_, err = dr.LookupTXT(context.Background(), "example.com")
Expand Down Expand Up @@ -670,7 +670,7 @@ func TestRetry(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")

testClient := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, blog.UseMock())
testClient := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 3, blog.UseMock(), nil)
dr := testClient.(*impl)
dr.dnsClient = &testExchanger{errs: []error{isTempErr, isTempErr, nil}}
ctx, cancel := context.WithCancel(context.Background())
Expand Down Expand Up @@ -774,7 +774,7 @@ func TestRotateServerOnErr(t *testing.T) {
fmt.Println(staticProvider.servers)

maxTries := 5
client := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), maxTries, blog.UseMock())
client := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), maxTries, blog.UseMock(), nil)

// Configure a mock exchanger that will always return a retryable error for
// servers A and B. This will force server "[2606:4700:4700::1111]:53" to do
Expand Down
11 changes: 8 additions & 3 deletions bdns/servers.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ type dynamicProvider struct {
// service is the service name to look up SRV records for within the domain.
// If this field is left unspecified 'dns' will be used as the service name.
service string
// proto is the IP protocol (tcp or udp) to look up SRV records for.
proto string
// domain is the name to look up SRV records within.
domain string
// A map of IP addresses (results of A record lookups for SRV Targets) to
Expand Down Expand Up @@ -174,7 +176,9 @@ var _ ServerProvider = &dynamicProvider{}
// at refresh intervals and uses the resulting IP/port combos to populate the
// list returned by Addrs. The update process ignores the Priority and Weight
// attributes of the SRV records.
func StartDynamicProvider(c *cmd.DNSProvider, refresh time.Duration) (*dynamicProvider, error) {
//
// `proto` is the IP protocol (tcp or udp) to look up SRV records for.
func StartDynamicProvider(c *cmd.DNSProvider, refresh time.Duration, proto string) (*dynamicProvider, error) {
if c.SRVLookup.Domain == "" {
return nil, fmt.Errorf("'domain' cannot be empty")
}
Expand All @@ -200,6 +204,7 @@ func StartDynamicProvider(c *cmd.DNSProvider, refresh time.Duration) (*dynamicPr
dp := dynamicProvider{
dnsAuthority: dnsAuthority,
service: service,
proto: proto,
domain: c.SRVLookup.Domain,
addrs: make(map[string][]uint16),
cancel: make(chan interface{}),
Expand Down Expand Up @@ -263,9 +268,9 @@ func (dp *dynamicProvider) update() error {
}

// RFC 2782 formatted SRV record being queried e.g. "_service._proto.name."
record := fmt.Sprintf("_%s._udp.%s.", dp.service, dp.domain)
record := fmt.Sprintf("_%s._%s.%s.", dp.service, dp.proto, dp.domain)

_, srvs, err := resolver.LookupSRV(ctx, dp.service, "udp", dp.domain)
_, srvs, err := resolver.LookupSRV(ctx, dp.service, dp.proto, dp.domain)
if err != nil {
return fmt.Errorf("during SRV lookup of %q: %w", record, err)
}
Expand Down
18 changes: 12 additions & 6 deletions cmd/boulder-va/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,17 @@ func main() {
}

var servers bdns.ServerProvider
servers, err = bdns.StartDynamicProvider(c.VA.DNSProvider, 60*time.Second)
proto := "udp"
if features.Enabled(features.DOH) {
proto = "tcp"
}
servers, err = bdns.StartDynamicProvider(c.VA.DNSProvider, 60*time.Second, proto)
cmd.FailOnError(err, "Couldn't start dynamic DNS server resolver")
defer servers.Stop()

tlsConfig, err := c.VA.TLS.Load(scope)
cmd.FailOnError(err, "tlsConfig config")

var resolver bdns.Client
if !c.VA.DNSAllowLoopbackAddresses {
resolver = bdns.New(
Expand All @@ -97,20 +104,19 @@ func main() {
scope,
clk,
dnsTries,
logger)
logger,
tlsConfig)
} else {
resolver = bdns.NewTest(
c.VA.DNSTimeout.Duration,
servers,
scope,
clk,
dnsTries,
logger)
logger,
tlsConfig)
}

tlsConfig, err := c.VA.TLS.Load(scope)
cmd.FailOnError(err, "tlsConfig config")

var remotes []va.RemoteVA
if len(c.VA.RemoteVAs) > 0 {
for _, rva := range c.VA.RemoteVAs {
Expand Down
5 changes: 3 additions & 2 deletions features/featureflag_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,15 @@ const (
// compliant truncated SHA256 Subject Key Identifier in end-entity
// certificates.
SHA256SubjectKeyIdentifier

// DOH enables DNS-over-HTTPS queries for validation
DOH
)

// List of features and their default value, protected by fMu
var features = map[FeatureFlag]bool{
unused: false,
DOH: false,
CAAValidationMethods: false,
CAAAccountURI: false,
EnforceMultiVA: false,
Expand Down
2 changes: 1 addition & 1 deletion test/boulder-tools/install-go.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.28.0
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.2.0
go install github.com/rubenv/sql-migrate/sql-migrate@v1.1.2
go install golang.org/x/tools/cmd/stringer@latest
go install github.com/letsencrypt/pebble/cmd/pebble-challtestsrv@master
go install github.com/letsencrypt/pebble/v2/cmd/pebble-challtestsrv@66511d8
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3
go install honnef.co/go/tools/cmd/staticcheck@2023.1.5

Expand Down
5 changes: 3 additions & 2 deletions test/config-next/va-remote-a.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"dnsProvider": {
"dnsAuthority": "consul.service.consul",
"srvLookup": {
"service": "dns",
"service": "doh",
"domain": "service.consul"
}
},
Expand Down Expand Up @@ -33,7 +33,8 @@
}
},
"features": {
"CAAAfterValidation": true
"CAAAfterValidation": true,
"DOH": true
},
"accountURIPrefixes": [
"http://boulder.service.consul:4000/acme/reg/",
Expand Down
Loading

0 comments on commit c21b376

Please sign in to comment.