Skip to content

Commit

Permalink
Allow to control the number of A/AAAA Record returned by DNS
Browse files Browse the repository at this point in the history 
This allows to have randomized resource records (i.e. each
answer contains only one IP, but the IP changes every request) for
A, AAAA records.

It will fix hashicorp#3355 and
hashicorp#3937

See hashicorp#3937 (comment)
for details.

It basically add a new option called `a_record_limit` and will not
return more than a_record_limit when performing A, AAAA or ANY DNS
requests.

The existing `udp_answer_limit` option is still working but should
be considered as deprecated since it works only with DNS clients
not supporting EDNS.
  • Loading branch information
@pierresouchay
pierresouchay committed Mar 6, 2018
1 parent 257ad52 commit 0b7f620
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 12 deletions.
4 changes: 4 additions & 0 deletions agent/config/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
// DNS
DNSAddrs: dnsAddrs,
DNSAllowStale: b.boolVal(c.DNS.AllowStale),
DNSARecordLimit: b.intVal(c.DNS.ARecordLimit),
DNSDisableCompression: b.boolVal(c.DNS.DisableCompression),
DNSDomain: b.stringVal(c.DNSDomain),
DNSEnableTruncate: b.boolVal(c.DNS.EnableTruncate),
Expand Down Expand Up @@ -810,6 +811,9 @@ func (b *Builder) Validate(rt RuntimeConfig) error {
if rt.DNSUDPAnswerLimit < 0 {
return fmt.Errorf("dns_config.udp_answer_limit cannot be %d. Must be greater than or equal to zero", rt.DNSUDPAnswerLimit)
}
if rt.DNSARecordLimit < 0 {
return fmt.Errorf("dns_config.a_record_limit cannot be %d. Must be greater than or equal to zero", rt.DNSARecordLimit)
}
if err := structs.ValidateMetadata(rt.NodeMeta, false); err != nil {
return fmt.Errorf("node_meta invalid: %v", err)
}
Expand Down
1 change: 1 addition & 0 deletions agent/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ type CheckDefinition struct {

type DNS struct {
AllowStale *bool `json:"allow_stale,omitempty" hcl:"allow_stale" mapstructure:"allow_stale"`
ARecordLimit *int `json:"a_record_limit,omitempty" hcl:"a_record_limit" mapstructure:"a_record_limit"`
DisableCompression *bool `json:"disable_compression,omitempty" hcl:"disable_compression" mapstructure:"disable_compression"`
EnableTruncate *bool `json:"enable_truncate,omitempty" hcl:"enable_truncate" mapstructure:"enable_truncate"`
MaxStale *string `json:"max_stale,omitempty" hcl:"max_stale" mapstructure:"max_stale"`
Expand Down
1 change: 1 addition & 0 deletions agent/config/default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func DefaultSource() Source {
}
dns_config = {
allow_stale = true
a_record_limit = 0
udp_answer_limit = 3
max_stale = "87600h"
recursor_timeout = "2s"
Expand Down
36 changes: 24 additions & 12 deletions agent/config/runtime.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,25 @@ type RuntimeConfig struct {
// hcl: dns_config { allow_stale = (true|false) }
DNSAllowStale bool

// DNSARecordLimit is used to limit the maximum number of DNS Resource
// Records returned in the ANSWER section of a DNS response for A or AAAA
// records for both UDP and TCP queries.
//
// This is not normally useful and will be limited based on the querying
// protocol, however systems that implemented §6 Rule 9 in RFC3484
// may want to set this to `1` in order to subvert §6 Rule 9 and
// re-obtain the effect of randomized resource records (i.e. each
// answer contains only one IP, but the IP changes every request).
// RFC3484 sorts answers in a deterministic order, which defeats the
// purpose of randomized DNS responses. This RFC has been obsoleted
// by RFC6724 and restores the desired behavior of randomized
// responses, however a large number of Linux hosts using glibc(3)
// implemented §6 Rule 9 and may need this option (e.g. CentOS 5-6,
// Debian Squeeze, etc).
//
// hcl: dns_config { a_record_limit = int }
DNSARecordLimit int

// DNSDisableCompression is used to control whether DNS responses are
// compressed. In Consul 0.7 this was turned on by default and this
// config was added as an opt-out.
Expand Down Expand Up @@ -253,18 +272,11 @@ type RuntimeConfig struct {
DNSServiceTTL map[string]time.Duration

// DNSUDPAnswerLimit is used to limit the maximum number of DNS Resource
// Records returned in the ANSWER section of a DNS response. This is
// not normally useful and will be limited based on the querying
// protocol, however systems that implemented §6 Rule 9 in RFC3484
// may want to set this to `1` in order to subvert §6 Rule 9 and
// re-obtain the effect of randomized resource records (i.e. each
// answer contains only one IP, but the IP changes every request).
// RFC3484 sorts answers in a deterministic order, which defeats the
// purpose of randomized DNS responses. This RFC has been obsoleted
// by RFC6724 and restores the desired behavior of randomized
// responses, however a large number of Linux hosts using glibc(3)
// implemented §6 Rule 9 and may need this option (e.g. CentOS 5-6,
// Debian Squeeze, etc).
// Records returned in the ANSWER section of a DNS response for UDP
// responses without EDNS support (limited to 512 bytes).
// This parameter is deprecated, if you want to limit the number of
// records returned by A or AAAA questions, please use DNSARecordLimit
// instead.
//
// hcl: dns_config { udp_answer_limit = int }
DNSUDPAnswerLimit int
Expand Down
13 changes: 13 additions & 0 deletions agent/config/runtime_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1595,6 +1595,15 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
hcl: []string{`dns_config = { udp_answer_limit = -1 }`},
err: "dns_config.udp_answer_limit cannot be -1. Must be greater than or equal to zero",
},
{
desc: "dns_config.a_record_limit invalid",
args: []string{
`-data-dir=` + dataDir,
},
json: []string{`{ "dns_config": { "a_record_limit": -1 } }`},
hcl: []string{`dns_config = { a_record_limit = -1 }`},
err: "dns_config.a_record_limit cannot be -1. Must be greater than or equal to zero",
},
{
desc: "performance.raft_multiplier < 0",
args: []string{
Expand Down Expand Up @@ -2288,6 +2297,7 @@ func TestFullConfig(t *testing.T) {
"domain": "7W1xXSqd",
"dns_config": {
"allow_stale": true,
"a_record_limit": 29907,
"disable_compression": true,
"enable_truncate": true,
"max_stale": "29685s",
Expand Down Expand Up @@ -2723,6 +2733,7 @@ func TestFullConfig(t *testing.T) {
domain = "7W1xXSqd"
dns_config {
allow_stale = true
a_record_limit = 29907
disable_compression = true
enable_truncate = true
max_stale = "29685s"
Expand Down Expand Up @@ -3283,6 +3294,7 @@ func TestFullConfig(t *testing.T) {
CheckUpdateInterval: 16507 * time.Second,
ClientAddrs: []*net.IPAddr{ipAddr("93.83.18.19")},
DNSAddrs: []net.Addr{tcpAddr("93.95.95.81:7001"), udpAddr("93.95.95.81:7001")},
DNSARecordLimit: 29907,
DNSAllowStale: true,
DNSDisableCompression: true,
DNSDomain: "7W1xXSqd",
Expand Down Expand Up @@ -3959,6 +3971,7 @@ func TestSanitize(t *testing.T) {
"ConsulSerfWANProbeTimeout": "0s",
"ConsulSerfWANSuspicionMult": 0,
"ConsulServerHealthInterval": "0s",
"DNSARecordLimit": 0,
"DNSAddrs": [
"tcp://1.2.3.4:5678",
"udp://1.2.3.4:5678"
Expand Down
8 changes: 8 additions & 0 deletions agent/dns.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type dnsConfig struct {
SegmentName string
ServiceTTL map[string]time.Duration
UDPAnswerLimit int
ARecordLimit int
}

// DNSServer is used to wrap an Agent and expose various
Expand Down Expand Up @@ -94,6 +95,7 @@ func NewDNSServer(a *Agent) (*DNSServer, error) {
func GetDNSConfig(conf *config.RuntimeConfig) *dnsConfig {
return &dnsConfig{
AllowStale: conf.DNSAllowStale,
ARecordLimit: conf.DNSARecordLimit,
Datacenter: conf.Datacenter,
EnableTruncate: conf.DNSEnableTruncate,
MaxStale: conf.DNSMaxStale,
Expand Down Expand Up @@ -974,6 +976,7 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode
handled := make(map[string]struct{})
edns := req.IsEdns0() != nil

count := 0
for _, node := range nodes {
// Start with the translated address but use the service address,
// if specified.
Expand All @@ -999,6 +1002,11 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode
records := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns)
if records != nil {
resp.Answer = append(resp.Answer, records...)
count++
if count == d.config.ARecordLimit {
// We stop only if greater than 0 or we reached the limit
return
}
}
}
}
Expand Down
157 changes: 157 additions & 0 deletions agent/dns_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2997,6 +2997,163 @@ func testDNSServiceLookupResponseLimits(t *testing.T, answerLimit int, qType uin
return true, nil
}

func testDNSServiceLookupResponseARecordLimits(t *testing.T, generateNumNodes int, aRecordLimit int, qType uint16,
expectedResultsCount int, udpSize uint16, udpAnswerLimit int) (bool, error) {
a := NewTestAgent(t.Name(), `
node_name = "test-node"
dns_config {
a_record_limit = `+fmt.Sprintf("%d", aRecordLimit)+`
udp_answer_limit = `+fmt.Sprintf("%d", aRecordLimit)+`
}
`)
defer a.Shutdown()

for i := 0; i < generateNumNodes; i++ {
nodeAddress := fmt.Sprintf("127.0.0.%d", i+1)
if rand.Float64() < pctNodesWithIPv6 {
nodeAddress = fmt.Sprintf("fe80::%d", i+1)
}
args := &structs.RegisterRequest{
Datacenter: "dc1",
Node: fmt.Sprintf("foo%d", i),
Address: nodeAddress,
Service: &structs.NodeService{
Service: "api-tier",
Port: 8080,
},
}

var out struct{}
if err := a.RPC("Catalog.Register", args, &out); err != nil {
return false, fmt.Errorf("err: %v", err)
}
}
var id string
{
args := &structs.PreparedQueryRequest{
Datacenter: "dc1",
Op: structs.PreparedQueryCreate,
Query: &structs.PreparedQuery{
Name: "api-tier",
Service: structs.ServiceQuery{
Service: "api-tier",
},
},
}

if err := a.RPC("PreparedQuery.Apply", args, &id); err != nil {
return false, fmt.Errorf("err: %v", err)
}
}

// Look up the service directly and via prepared query.
questions := []string{
"api-tier.service.consul.",
"api-tier.query.consul.",
id + ".query.consul.",
}
for _, question := range questions {
m := new(dns.Msg)

m.SetQuestion(question, qType)
protocol := "tcp"
if udpSize > 0 {
protocol = "udp"
}
if udpSize > 512 {
m.SetEdns0(udpSize, true)
}
c := &dns.Client{Net: protocol, UDPSize: 8192}
in, _, err := c.Exchange(m, a.DNSAddr())
if err != nil {
return false, fmt.Errorf("err: %v", err)
}
if len(in.Answer) != expectedResultsCount {
return false, fmt.Errorf("%d/%d answers received for type %v for %s (%s)", len(in.Answer), expectedResultsCount, qType, question, protocol)
}
}

return true, nil
}

func TestDNS_ServiceLookup_ARecordLimits(t *testing.T) {
t.Parallel()
tests := []struct {
name string
aRecordLimit int
expectedAResults int
expectedAAAAResuls int
expectedSRVResults int
numNodesTotal int
udpSize uint16
udpAnswerLimit int
}{
// UDP + EDNS
{"udp-edns-1", 1, 1, 1, 30, 30, 8192, 3},
{"udp-edns-2", 2, 2, 1, 30, 30, 8192, 3},
{"udp-edns-3", 3, 3, 1, 30, 30, 8192, 3},
{"udp-edns-4", 4, 4, 1, 30, 30, 8192, 3},
{"udp-edns-5", 5, 5, 1, 30, 30, 8192, 3},
{"udp-edns-6", 6, 6, 1, 30, 30, 8192, 3},
{"udp-edns-max", 6, 3, 3, 3, 3, 8192, 3},
// All UDP without EDNS have a limit of 2 answers due to udpAnswerLimit
// Even SRV records are limit to 2 records
{"udp-limit-1", 1, 1, 1, 1, 1, 512, 2},
{"udp-limit-2", 2, 2, 2, 2, 2, 512, 2},
// AAAA results limited by size of payload
{"udp-limit-3", 3, 2, 2, 2, 2, 512, 2},
{"udp-limit-4", 4, 2, 2, 2, 2, 512, 2},
{"udp-limit-5", 5, 2, 2, 2, 2, 512, 2},
{"udp-limit-6", 6, 2, 2, 2, 2, 512, 2},
{"udp-limit-max", 6, 2, 2, 2, 2, 512, 2},
// All UDP without EDNS and no udpAnswerLimit
// Size of records is limited by UDP payload
{"udp-1", 1, 1, 1, 1, 1, 512, 0},
{"udp-2", 2, 2, 2, 2, 2, 512, 0},
{"udp-3", 3, 2, 2, 2, 2, 512, 0},
{"udp-4", 4, 2, 2, 2, 2, 512, 0},
{"udp-5", 5, 2, 2, 2, 2, 512, 0},
{"udp-6", 6, 2, 2, 2, 2, 512, 0},
// Only 3 A and 3 SRV records on 512 bytes
{"udp-max", 6, 2, 2, 2, 2, 512, 0},

{"tcp-1", 1, 1, 1, 30, 30, 0, 0},
{"tcp-2", 2, 2, 2, 30, 30, 0, 0},
{"tcp-3", 3, 3, 3, 30, 30, 0, 0},
{"tcp-4", 4, 4, 4, 30, 30, 0, 0},
{"tcp-5", 5, 5, 5, 30, 30, 0, 0},
{"tcp-6", 6, 6, 5, 30, 30, 0, 0},
{"tcp-max", 6, 2, 2, 2, 2, 0, 0},
}
for _, test := range tests {
test := test // capture loop var

queriesLimited := []uint16{
dns.TypeA,
dns.TypeAAAA,
dns.TypeANY,
}
// All those queries should have at max queriesLimited elements
for idx, qType := range queriesLimited {
t.Run(fmt.Sprintf("ARecordLimit %d qType: %d", idx, qType), func(t *testing.T) {
t.Parallel()
ok, err := testDNSServiceLookupResponseARecordLimits(t, test.numNodesTotal, test.aRecordLimit, qType, test.expectedAResults, test.udpSize, test.udpAnswerLimit)
if !ok {
t.Errorf("Expected lookup %s to pass: %v", test.name, err)
}
})
}
// No limits but the size of records for SRV records, since not subject to randomization issues
t.Run("SRV lookup limitARecord", func(t *testing.T) {
t.Parallel()
ok, err := testDNSServiceLookupResponseARecordLimits(t, test.expectedSRVResults, test.aRecordLimit, dns.TypeSRV, test.numNodesTotal, test.udpSize, test.udpAnswerLimit)
if !ok {
t.Errorf("Expected service SRV lookup %s to pass: %v", test.name, err)
}
})
}
}

func TestDNS_ServiceLookup_AnswerLimits(t *testing.T) {
t.Parallel()
// Build a matrix of config parameters (udpAnswerLimit), and the
Expand Down

0 comments on commit 0b7f620

Please sign in to comment.