diff --git a/command/agent/config.go b/command/agent/config.go index ac936ca114..e2bb2e3849 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -104,6 +104,11 @@ type DNSConfig struct { // whose health checks are in any non-passing state. By // default, only nodes in a critical state are excluded. OnlyPassing bool `mapstructure:"only_passing"` + + // DisableCompression 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. + DisableCompression bool `mapstructure:"disable_compression"` } // Telemetry is the telemetry configuration for the server @@ -1300,6 +1305,9 @@ func MergeConfig(a, b *Config) *Config { if b.DNSConfig.OnlyPassing { result.DNSConfig.OnlyPassing = true } + if b.DNSConfig.DisableCompression { + result.DNSConfig.DisableCompression = true + } if b.CheckUpdateIntervalRaw != "" || b.CheckUpdateInterval != 0 { result.CheckUpdateInterval = b.CheckUpdateInterval } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 00e5a0dfb3..354dfeca6a 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -608,6 +608,17 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: %#v", config) } + // DNS disable compression + input = `{"dns_config": {"disable_compression": true}}` + config, err = DecodeConfig(bytes.NewReader([]byte(input))) + if err != nil { + t.Fatalf("err: %s", err) + } + + if !config.DNSConfig.DisableCompression { + t.Fatalf("bad: %#v", config) + } + // CheckUpdateInterval input = `{"check_update_interval": "10m"}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) @@ -1369,10 +1380,11 @@ func TestMergeConfig(t *testing.T) { DataDir: "/tmp/bar", DNSRecursors: []string{"127.0.0.2:1001"}, DNSConfig: DNSConfig{ - AllowStale: false, - EnableTruncate: true, - MaxStale: 30 * time.Second, - NodeTTL: 10 * time.Second, + AllowStale: false, + EnableTruncate: true, + DisableCompression: true, + MaxStale: 30 * time.Second, + NodeTTL: 10 * time.Second, ServiceTTL: map[string]time.Duration{ "api": 10 * time.Second, }, diff --git a/command/agent/dns.go b/command/agent/dns.go index 32606abbad..62fdd87a9c 100644 --- a/command/agent/dns.go +++ b/command/agent/dns.go @@ -180,7 +180,7 @@ func (d *DNSServer) handlePtr(resp dns.ResponseWriter, req *dns.Msg) { // Setup the message response m := new(dns.Msg) m.SetReply(req) - m.Compress = true + m.Compress = !d.config.DisableCompression m.Authoritative = true m.RecursionAvailable = (len(d.recursors) > 0) @@ -250,7 +250,7 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) { // Setup the message response m := new(dns.Msg) m.SetReply(req) - m.Compress = true + m.Compress = !d.config.DisableCompression m.Authoritative = true m.RecursionAvailable = (len(d.recursors) > 0) @@ -495,7 +495,7 @@ func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qTy } // trimUDPAnswers makes sure a UDP response is not longer than allowed by RFC -// 1035. Enforce an arbitrary limit that can be further ratcheted down by +// 1035. Enforce an arbitrary limit that can be further ratcheted down by // config, and then make sure the response doesn't exceed 512 bytes. func trimUDPAnswers(config *DNSConfig, resp *dns.Msg) (trimmed bool) { numAnswers := len(resp.Answer) @@ -506,10 +506,17 @@ func trimUDPAnswers(config *DNSConfig, resp *dns.Msg) (trimmed bool) { resp.Answer = resp.Answer[:maxAnswers] } - // This enforces the hard limit of 512 bytes per the RFC. + // This enforces the hard limit of 512 bytes per the RFC. Note that we + // temporarily switch to uncompressed so that we limit to a response + // that will not exceed 512 bytes uncompressed, which is more + // conservative and will allow our responses to be compliant even if + // some downstream server uncompresses them. + compress := resp.Compress + resp.Compress = false for len(resp.Answer) > 0 && resp.Len() > 512 { resp.Answer = resp.Answer[:len(resp.Answer)-1] } + resp.Compress = compress return len(resp.Answer) < numAnswers } @@ -788,7 +795,11 @@ func (d *DNSServer) handleRecurse(resp dns.ResponseWriter, req *dns.Msg) { for _, recursor := range d.recursors { r, rtt, err = c.Exchange(req, recursor) if err == nil { - r.Compress = true + // Compress the response; we don't know if the incoming + // response was compressed or not, so by not compressing + // we might generate an invalid packet on the way out. + r.Compress = !d.config.DisableCompression + // Forward the response d.logger.Printf("[DEBUG] dns: recurse RTT for %v (%v)", q, rtt) if err := resp.WriteMsg(r); err != nil { @@ -804,7 +815,7 @@ func (d *DNSServer) handleRecurse(resp dns.ResponseWriter, req *dns.Msg) { q, resp.RemoteAddr().String(), resp.RemoteAddr().Network()) m := &dns.Msg{} m.SetReply(req) - m.Compress = true + m.Compress = !d.config.DisableCompression m.RecursionAvailable = true m.SetRcode(req, dns.RcodeServerFailure) resp.WriteMsg(m) diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index 1182d9172e..2ef693ac5d 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -3200,3 +3200,209 @@ func TestDNS_PreparedQuery_AgentSource(t *testing.T) { } } } + +func TestDNS_Compression_trimUDPAnswers(t *testing.T) { + config := &DefaultConfig().DNSConfig + + m := dns.Msg{} + trimUDPAnswers(config, &m) + if m.Compress { + t.Fatalf("compression should be off") + } + + // The trim function temporarily turns off compression, so we need to + // make sure the setting gets restored properly. + m.Compress = true + trimUDPAnswers(config, &m) + if !m.Compress { + t.Fatalf("compression should be on") + } +} + +func TestDNS_Compression_Query(t *testing.T) { + dir, srv := makeDNSServer(t) + defer os.RemoveAll(dir) + defer srv.agent.Shutdown() + + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") + + // Register a node with a service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + Service: &structs.NodeService{ + Service: "db", + Tags: []string{"master"}, + Port: 12345, + }, + } + + var out struct{} + if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + // Register an equivalent prepared query. + var id string + { + args := &structs.PreparedQueryRequest{ + Datacenter: "dc1", + Op: structs.PreparedQueryCreate, + Query: &structs.PreparedQuery{ + Service: structs.ServiceQuery{ + Service: "db", + }, + }, + } + if err := srv.agent.RPC("PreparedQuery.Apply", args, &id); err != nil { + t.Fatalf("err: %v", err) + } + } + + // Look up the service directly and via prepared query. + questions := []string{ + "db.service.consul.", + id + ".query.consul.", + } + for _, question := range questions { + m := new(dns.Msg) + m.SetQuestion(question, dns.TypeSRV) + + addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS) + conn, err := dns.Dial("udp", addr.String()) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Do a manual exchange with compression on (the default). + srv.config.DisableCompression = false + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + p := make([]byte, dns.MaxMsgSize) + compressed, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Disable compression and try again. + srv.config.DisableCompression = true + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + unc, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // We can't see the compressed status given the DNS API, so we + // just make sure the message is smaller to see if it's + // respecting the flag. + if compressed == 0 || unc == 0 || compressed >= unc { + t.Fatalf("'%s' doesn't look compressed: %d vs. %d", question, compressed, unc) + } + } +} + +func TestDNS_Compression_ReverseLookup(t *testing.T) { + dir, srv := makeDNSServer(t) + defer os.RemoveAll(dir) + defer srv.agent.Shutdown() + + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") + + // Register node. + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo2", + Address: "127.0.0.2", + } + var out struct{} + if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + + m := new(dns.Msg) + m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) + + addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS) + conn, err := dns.Dial("udp", addr.String()) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Do a manual exchange with compression on (the default). + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + p := make([]byte, dns.MaxMsgSize) + compressed, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Disable compression and try again. + srv.config.DisableCompression = true + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + unc, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // We can't see the compressed status given the DNS API, so we just make + // sure the message is smaller to see if it's respecting the flag. + if compressed == 0 || unc == 0 || compressed >= unc { + t.Fatalf("doesn't look compressed: %d vs. %d", compressed, unc) + } +} + +func TestDNS_Compression_Recurse(t *testing.T) { + recursor := makeRecursor(t, []dns.RR{dnsA("apple.com", "1.2.3.4")}) + defer recursor.Shutdown() + + dir, srv := makeDNSServerConfig(t, func(c *Config) { + c.DNSRecursor = recursor.Addr + }, nil) + defer os.RemoveAll(dir) + defer srv.agent.Shutdown() + + m := new(dns.Msg) + m.SetQuestion("apple.com.", dns.TypeANY) + + addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS) + conn, err := dns.Dial("udp", addr.String()) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Do a manual exchange with compression on (the default). + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + p := make([]byte, dns.MaxMsgSize) + compressed, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Disable compression and try again. + srv.config.DisableCompression = true + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + unc, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // We can't see the compressed status given the DNS API, so we just make + // sure the message is smaller to see if it's respecting the flag. + if compressed == 0 || unc == 0 || compressed >= unc { + t.Fatalf("doesn't look compressed: %d vs. %d", compressed, unc) + } +} diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 354c69c1db..ff5e017e4c 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -516,6 +516,10 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass are considered. For example, if a node has a health check that is critical then all services on that node will be excluded because they are also considered critical. + * `disable_compression` If + set to true, DNS responses will not be compressed. Compression was added and enabled by default + in Consul 0.7. + * `udp_answer_limit` - Limit the number of resource records contained in the answer section of a UDP-based DNS