va/rva: Validate user-agent for http-01 and DoH requests (#8114)

Plumb the userAgent field, used to set http-01 User-Agent headers, from
va/rva configuration through to where User-Agent headers can be set for
DoH queries. Use integration tests to validate that the User-Agent is
set for http-01 challenges, dns-01 challenges over DoH, and CAA checks
over DoH.

Fixes #7963.
This commit is contained in:
Samantha Frank 2025-04-15 16:31:08 -04:00 committed by GitHub
parent d800055fe6
commit 7a3feb2ceb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 140 additions and 39 deletions

View File

@ -147,6 +147,7 @@ func New(
stats prometheus.Registerer,
clk clock.Clock,
maxTries int,
userAgent string,
log blog.Logger,
tlsConfig *tls.Config,
) Client {
@ -167,6 +168,7 @@ func New(
Timeout: readTimeout,
Transport: transport,
},
userAgent: userAgent,
}
} else {
client = &dns.Client{
@ -230,10 +232,11 @@ func NewTest(
stats prometheus.Registerer,
clk clock.Clock,
maxTries int,
userAgent string,
log blog.Logger,
tlsConfig *tls.Config,
) Client {
resolver := New(readTimeout, servers, stats, clk, maxTries, log, tlsConfig)
resolver := New(readTimeout, servers, stats, clk, maxTries, userAgent, log, tlsConfig)
resolver.(*impl).allowRestrictedAddresses = true
return resolver
}
@ -636,8 +639,9 @@ func logDNSError(
}
type dohExchanger struct {
clk clock.Clock
hc http.Client
clk clock.Clock
hc http.Client
userAgent string
}
// Exchange sends a DoH query to the provided DoH server and returns the response.
@ -655,6 +659,9 @@ func (d *dohExchanger) Exchange(query *dns.Msg, server string) (*dns.Msg, time.D
}
req.Header.Set("Content-Type", "application/dns-message")
req.Header.Set("Accept", "application/dns-message")
if len(d.userAgent) > 0 {
req.Header.Set("User-Agent", d.userAgent)
}
start := d.clk.Now()
resp, err := d.hc.Do(req)

View File

@ -252,7 +252,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(), nil)
obj := NewTest(time.Hour, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), nil)
_, resolvers, err := obj.LookupHost(context.Background(), "letsencrypt.org")
test.AssertEquals(t, len(resolvers), 0)
@ -269,7 +269,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(), nil)
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), nil)
_, resolvers, err := obj.LookupHost(context.Background(), "cps.letsencrypt.org")
test.AssertEquals(t, len(resolvers), 2)
@ -282,7 +282,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(), nil)
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), nil)
_, resolvers, err := obj.LookupHost(context.Background(), "cps.letsencrypt.org")
test.AssertEquals(t, len(resolvers), 2)
@ -295,7 +295,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(), nil)
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), nil)
bad := "servfail.com"
_, _, err = obj.LookupTXT(context.Background(), bad)
@ -313,7 +313,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(), nil)
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)
@ -330,7 +330,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(), nil)
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), nil)
ip, resolvers, err := obj.LookupHost(context.Background(), "servfail.com")
t.Logf("servfail.com - IP: %s, Err: %s", ip, err)
@ -416,7 +416,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(), nil)
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), nil)
hostname := "nxdomain.letsencrypt.org"
_, _, err = obj.LookupHost(context.Background(), hostname)
@ -432,7 +432,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(), nil)
obj := NewTest(time.Second*10, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 1, "", blog.UseMock(), nil)
removeIDExp := regexp.MustCompile(" id: [[:digit:]]+")
caas, resp, resolvers, err := obj.LookupCAA(context.Background(), "bracewel.net")
@ -673,7 +673,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(), nil)
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")
@ -704,7 +704,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(), nil)
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())
@ -808,7 +808,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(), nil)
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
@ -878,7 +878,7 @@ func TestDOHMetric(t *testing.T) {
staticProvider, err := NewStaticProvider([]string{dnsLoopbackAddr})
test.AssertNotError(t, err, "Got error creating StaticProvider")
testClient := NewTest(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, blog.UseMock(), nil)
testClient := NewTest(time.Second*11, staticProvider, metrics.NoopRegisterer, clock.NewFake(), 0, "", blog.UseMock(), nil)
resolver := testClient.(*impl)
resolver.dnsClient = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: tempError(true)}}

View File

@ -106,6 +106,7 @@ func main() {
scope,
clk,
c.VA.DNSTries,
c.VA.UserAgent,
logger,
tlsConfig)
} else {
@ -115,6 +116,7 @@ func main() {
scope,
clk,
c.VA.DNSTries,
c.VA.UserAgent,
logger,
tlsConfig)
}

View File

@ -115,6 +115,7 @@ func main() {
scope,
clk,
c.RVA.DNSTries,
c.RVA.UserAgent,
logger,
tlsConfig)
} else {
@ -124,6 +125,7 @@ func main() {
scope,
clk,
c.RVA.DNSTries,
c.RVA.UserAgent,
logger,
tlsConfig)
}

2
go.mod
View File

@ -16,7 +16,7 @@ require (
github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1
github.com/jmhodges/clock v1.2.0
github.com/letsencrypt/borp v0.0.0-20240620175310-a78493c6e2bd
github.com/letsencrypt/challtestsrv v1.3.2
github.com/letsencrypt/challtestsrv v1.3.3
github.com/letsencrypt/pkcs11key/v4 v4.0.0
github.com/letsencrypt/validator/v10 v10.0.0-20230215210743-a0c7dfc17158
github.com/miekg/dns v1.1.61

4
go.sum
View File

@ -160,8 +160,8 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
github.com/letsencrypt/borp v0.0.0-20240620175310-a78493c6e2bd h1:3c+LdlAOEcW1qmG8gtkMCyAEoslmj6XCmniB+926kMM=
github.com/letsencrypt/borp v0.0.0-20240620175310-a78493c6e2bd/go.mod h1:gMSMCNKhxox/ccR923EJsIvHeVVYfCABGbirqa0EwuM=
github.com/letsencrypt/challtestsrv v1.3.2 h1:pIDLBCLXR3B1DLmOmkkqg29qVa7DDozBnsOpL9PxmAY=
github.com/letsencrypt/challtestsrv v1.3.2/go.mod h1:Ur4e4FvELUXLGhkMztHOsPIsvGxD/kzSJninOrkM+zc=
github.com/letsencrypt/challtestsrv v1.3.3 h1:ki02PH84fo6IOe/A+zt1/kfRBp2JrtauEaa5xwjg4/Q=
github.com/letsencrypt/challtestsrv v1.3.3/go.mod h1:Ur4e4FvELUXLGhkMztHOsPIsvGxD/kzSJninOrkM+zc=
github.com/letsencrypt/pkcs11key/v4 v4.0.0 h1:qLc/OznH7xMr5ARJgkZCCWk+EomQkiNTOoOF5LAgagc=
github.com/letsencrypt/pkcs11key/v4 v4.0.0/go.mod h1:EFUvBDay26dErnNb70Nd0/VW3tJiIbETBPTl9ATXQag=
github.com/letsencrypt/validator/v10 v10.0.0-20230215210743-a0c7dfc17158 h1:HGFsIltYMUiB5eoFSowFzSoXkocM2k9ctmJ57QMGjys=

View File

@ -223,6 +223,7 @@ type HTTPRequest struct {
Host string `json:"Host"`
HTTPS bool `json:"HTTPS"`
ServerName string `json:"ServerName"`
UserAgent string `json:"UserAgent"`
}
// HTTPRequestHistory fetches the challenge server's HTTP request history for
@ -406,6 +407,7 @@ type DNSRequest struct {
Qtype uint16 `json:"Qtype"`
Qclass uint16 `json:"Qclass"`
} `json:"Question"`
UserAgent string `json:"UserAgent"`
}
// DNSRequestHistory returns the history of DNS requests made to the challenge

View File

@ -7,15 +7,29 @@ import (
"crypto/elliptic"
"crypto/rand"
"database/sql"
"os"
"slices"
"sort"
"strings"
"testing"
"time"
"github.com/eggsampler/acme/v3"
challtestsrvclient "github.com/letsencrypt/boulder/test/chall-test-srv-client"
"github.com/letsencrypt/boulder/test/vars"
"github.com/miekg/dns"
)
var expectedUserAgents = []string{"boulder", "remoteva-a", "remoteva-b", "remoteva-c"}
func collectUserAgentsFromDNSRequests(requests []challtestsrvclient.DNSRequest) []string {
userAgents := make([]string, len(requests))
for i, request := range requests {
userAgents[i] = request.UserAgent
}
return userAgents
}
func TestMPICTLSALPN01(t *testing.T) {
t.Parallel()
@ -78,14 +92,27 @@ func TestMPICTLSALPN01(t *testing.T) {
t.Fatal(err)
}
caaCount := 0
var caaEvents []challtestsrvclient.DNSRequest
for _, event := range dnsEvents {
if event.Question.Qtype == dns.TypeCAA {
caaCount++
caaEvents = append(caaEvents, event)
}
}
if caaCount != 4 {
t.Fatalf("expected 4 CAA validation events (VA=1 RVAs=3), got %d", caaCount)
if len(caaEvents) != 4 {
t.Fatalf("expected 4 CAA validation events (VA=1 RVAs=3), got %d", len(caaEvents))
}
if os.Getenv("BOULDER_CONFIG_DIR") == "test/config-next" {
// We can only check the user-agent for DNS requests if the DOH
// feature-flag is enabled.
//
// TODO(#8120): Remove this once the DoH feature flag has been defaulted
// to true.
gotUserAgents := collectUserAgentsFromDNSRequests(caaEvents)
for _, ua := range expectedUserAgents {
if !slices.Contains(gotUserAgents, ua) {
t.Errorf("expected a query from User-Agent %q but did not get one (got %+v).", ua, gotUserAgents)
}
}
}
}
@ -135,14 +162,27 @@ func TestMPICDNS01(t *testing.T) {
t.Fatal(err)
}
validationCount := 0
var validationEvents []challtestsrvclient.DNSRequest
for _, event := range challDomainDNSEvents {
if event.Question.Qtype == dns.TypeTXT && event.Question.Name == "_acme-challenge."+domain+"." {
validationCount++
validationEvents = append(validationEvents, event)
}
}
if validationCount != 4 {
t.Errorf("expected 4 validation events (VA=1 RVAs=3), got %d", validationCount)
if len(validationEvents) != 4 {
t.Errorf("expected 4 validation events (VA=1 RVAs=3), got %d", len(validationEvents))
}
if os.Getenv("BOULDER_CONFIG_DIR") == "test/config-next" {
// We can only check the user-agent for DNS requests if the DOH
// feature-flag is enabled.
//
// TODO(#8120): Remove this once the DoH feature flag has been defaulted
// to true.
gotUserAgents := collectUserAgentsFromDNSRequests(validationEvents)
for _, ua := range expectedUserAgents {
if !slices.Contains(gotUserAgents, ua) {
t.Errorf("expected a query from User-Agent %q but did not get one (got %+v).", ua, gotUserAgents)
}
}
}
domainDNSEvents, err := testSrvClient.DNSRequestHistory(domain)
@ -150,14 +190,27 @@ func TestMPICDNS01(t *testing.T) {
t.Fatal(err)
}
caaCount := 0
var caaEvents []challtestsrvclient.DNSRequest
for _, event := range domainDNSEvents {
if event.Question.Qtype == dns.TypeCAA {
caaCount++
caaEvents = append(caaEvents, event)
}
}
if caaCount != 4 {
t.Fatalf("expected 4 CAA validation events (VA=1 RVAs=3), got %d", caaCount)
if len(caaEvents) != 4 {
t.Errorf("expected 4 CAA validation events (VA=1 RVAs=3), got %d", len(caaEvents))
}
if os.Getenv("BOULDER_CONFIG_DIR") == "test/config-next" {
// We can only check the user-agent for DNS requests if the DOH
// feature-flag is enabled.
//
// TODO(#8120): Remove this once the DoH feature flag has been defaulted
// to true.
gotUserAgents := collectUserAgentsFromDNSRequests(caaEvents)
for _, ua := range expectedUserAgents {
if !slices.Contains(gotUserAgents, ua) {
t.Errorf("expected a query from User-Agent %q but did not get one (got %+v).", ua, gotUserAgents)
}
}
}
}
@ -217,19 +270,44 @@ func TestMPICHTTP01(t *testing.T) {
t.Errorf("expected 4 validation events (VA=1 RVAs=3), got %d", validationCount)
}
sort.Slice(validationEvents, func(i, j int) bool {
return validationEvents[i].UserAgent < validationEvents[j].UserAgent
})
for i, event := range validationEvents {
if event.UserAgent != expectedUserAgents[i] {
t.Errorf("expected user agent %s, got %s", expectedUserAgents[i], event.UserAgent)
}
}
dnsEvents, err := testSrvClient.DNSRequestHistory(domain)
if err != nil {
t.Fatal(err)
}
caaCount := 0
var caaEvents []challtestsrvclient.DNSRequest
for _, event := range dnsEvents {
if event.Question.Qtype == dns.TypeCAA {
caaCount++
caaEvents = append(caaEvents, event)
}
}
if caaCount != 4 {
t.Fatalf("expected 4 CAA validation events (VA=1 RVAs=3), got %d", caaCount)
if len(caaEvents) != 4 {
t.Errorf("expected 4 CAA validation events (VA=1 RVAs=3), got %d", len(caaEvents))
}
if os.Getenv("BOULDER_CONFIG_DIR") == "test/config-next" {
// We can only check the user-agent for DNS requests if the DOH
// feature-flag is enabled.
//
// TODO(#8120): Remove this once the DoH feature flag has been defaulted
// to true.
sort.Slice(caaEvents, func(i, j int) bool {
return caaEvents[i].UserAgent < caaEvents[j].UserAgent
})
for i, event := range caaEvents {
if event.UserAgent != expectedUserAgents[i] {
t.Errorf("expected user agent %s, got %s", expectedUserAgents[i], event.UserAgent)
}
}
}
}

View File

@ -10,6 +10,8 @@ import (
// Common contains all of the shared fields for a VA and a Remote VA (RVA).
type Common struct {
cmd.ServiceConfig
// UserAgent is the "User-Agent" header sent during http-01 challenges and
// DoH queries.
UserAgent string
IssuerDomain string

View File

@ -101,6 +101,7 @@ func TestDNSValidationNoServer(t *testing.T) {
metrics.NoopRegisterer,
clock.New(),
1,
"",
log,
nil)

View File

@ -195,7 +195,7 @@ func (s *ChallSrv) dohHandler(w http.ResponseWriter, r *http.Request) {
return
}
s.dnsHandlerInner(&dnsToHTTPWriter{w}, msg)
s.dnsHandlerInner(&dnsToHTTPWriter{w}, msg, r.Header.Get("User-Agent"))
}
// dnsHandler is a miekg/dns handler that can process a dns.Msg request and
@ -204,10 +204,10 @@ func (s *ChallSrv) dohHandler(w http.ResponseWriter, r *http.Request) {
// DNS data. A host that is aliased by a CNAME record will follow that alias
// one level and return the requested record types for that alias' target
func (s *ChallSrv) dnsHandler(w dns.ResponseWriter, r *dns.Msg) {
s.dnsHandlerInner(w, r)
s.dnsHandlerInner(w, r, "")
}
func (s *ChallSrv) dnsHandlerInner(w writeMsg, r *dns.Msg) {
func (s *ChallSrv) dnsHandlerInner(w writeMsg, r *dns.Msg, userAgent string) {
m := new(dns.Msg)
m.SetReply(r)
m.Compress = false
@ -215,7 +215,8 @@ func (s *ChallSrv) dnsHandlerInner(w writeMsg, r *dns.Msg) {
// For each question, add answers based on the type of question
for _, q := range r.Question {
s.AddRequestEvent(DNSRequestEvent{
Question: q,
Question: q,
UserAgent: userAgent,
})
// If there is a ServFail mock set then ignore the question and set the

View File

@ -38,6 +38,8 @@ type HTTPRequestEvent struct {
// The ServerName from the ClientHello. May be empty if there was no SNI or if
// the request was not HTTPS
ServerName string
// The User-Agent header from the request
UserAgent string
}
// HTTPRequestEvents always have type HTTPRequestEventType
@ -59,6 +61,9 @@ func (e HTTPRequestEvent) Key() string {
type DNSRequestEvent struct {
// The DNS question received.
Question dns.Question
// The User-Agent header from the request, may be empty
// if the request was not over DoH.
UserAgent string
}
// DNSRequestEvents always have type DNSRequestEventType

View File

@ -128,6 +128,7 @@ func (s *ChallSrv) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Host: r.Host,
HTTPS: r.TLS != nil,
ServerName: serverName,
UserAgent: r.Header.Get("User-Agent"),
})
// If the request was not over HTTPS and we have a redirect, serve it.

2
vendor/modules.txt vendored
View File

@ -207,7 +207,7 @@ github.com/jmhodges/clock
# github.com/letsencrypt/borp v0.0.0-20240620175310-a78493c6e2bd
## explicit; go 1.20
github.com/letsencrypt/borp
# github.com/letsencrypt/challtestsrv v1.3.2
# github.com/letsencrypt/challtestsrv v1.3.3
## explicit; go 1.13
github.com/letsencrypt/challtestsrv
# github.com/letsencrypt/pkcs11key/v4 v4.0.0