From 5cb1c0b0e2f12ed1bf62076397cb571f9656af1b Mon Sep 17 00:00:00 2001 From: folbrich Date: Sat, 20 Jul 2019 16:02:00 -0600 Subject: [PATCH] Add/strip padding according to RFC7830 and RFC8467 --- README.md | 5 +-- dnsclient.go | 4 +- dnslistener.go | 9 ++++ dohclient.go | 4 ++ dohlistener.go | 4 ++ dotclient.go | 3 ++ dotlistener_test.go | 50 ++++++++++++++++++++++ padding.go | 102 ++++++++++++++++++++++++++++++++++++++++++++ padding_test.go | 66 ++++++++++++++++++++++++++++ 9 files changed, 242 insertions(+), 5 deletions(-) create mode 100644 padding.go create mode 100644 padding_test.go diff --git a/README.md b/README.md index 5cbef3f..ea84860 100644 --- a/README.md +++ b/README.md @@ -15,10 +15,6 @@ Features: - Routing of queries based on query type, query name, or client IP - Written in Go - Platform independent -TODO: - -- Dot and DoH listeners should support padding as per [RFC7830](https://tools.ietf.org/html/rfc7830) and [RFC8467](https://tools.ietf.org/html/rfc8467) - Note: **RouteDNS is under active development and interfaces as well as configuration options are likely going to change** ## Installation @@ -386,4 +382,5 @@ The client is configured to act as local DNS resolver, handling all queries from - DNS-over-TLS RFC - [https://tools.ietf.org/html/rfc7858](https://tools.ietf.org/html/rfc7858) - DNS-over-HTTPS RFC - [https://tools.ietf.org/html/rfc8484](https://tools.ietf.org/html/rfc8484) +- EDNS0 padding [RFC7830](https://tools.ietf.org/html/rfc7830) and [RFC8467](https://tools.ietf.org/html/rfc8467) - GoDoc for the rdns library - [https://godoc.org/github.com/folbricht/routedns](https://godoc.org/github.com/folbricht/routedns) diff --git a/dnsclient.go b/dnsclient.go index 97b3438..12b48c0 100644 --- a/dnsclient.go +++ b/dnsclient.go @@ -29,7 +29,6 @@ func NewDNSClient(endpoint, net string) *DNSClient { endpoint: endpoint, pipeline: NewPipeline(endpoint, client), } - } // Resolve a DNS query. @@ -40,6 +39,9 @@ func (d *DNSClient) Resolve(q *dns.Msg, ci ClientInfo) (*dns.Msg, error) { "resolver": d.endpoint, "protocol": d.net, }).Debug("querying upstream resolver") + + // Remove padding before sending over the wire in plain + stripPadding(q) return d.pipeline.Resolve(q) } diff --git a/dnslistener.go b/dnslistener.go index 5707870..e9a25d4 100644 --- a/dnslistener.go +++ b/dnslistener.go @@ -46,6 +46,7 @@ func listenHandler(protocol, addr string, r Resolver) dns.HandlerFunc { case *net.UDPAddr: ci.SourceIP = addr.IP } + log := Log.WithFields(logrus.Fields{"client": ci.SourceIP, "qname": qName(req), "protocol": protocol, "addr": addr}) log.Debug("received query") log.WithField("resolver", r.String()).Trace("forwarding query to resolver") @@ -55,6 +56,14 @@ func listenHandler(protocol, addr string, r Resolver) dns.HandlerFunc { a = new(dns.Msg) a.SetRcode(req, dns.RcodeServerFailure) } + + // If the client asked via DoT and EDNS0 is enabled, the response should be padded for extra security. + // See rfc7830 and rfc8467. + if protocol == "dot" { + padAnswer(req, a) + } else { + stripPadding(a) + } w.WriteMsg(a) } } diff --git a/dohclient.go b/dohclient.go index 8457f3a..2b8b15d 100644 --- a/dohclient.go +++ b/dohclient.go @@ -86,6 +86,10 @@ func (d *DoHClient) Resolve(q *dns.Msg, ci ClientInfo) (*dns.Msg, error) { "method": d.opt.Method, }) log.Debug("querying upstream resolver") + + // Add padding before sending the query over HTTPS + padQuery(q) + switch d.opt.Method { case "POST": return d.ResolvePOST(q) diff --git a/dohlistener.go b/dohlistener.go index cfea2d7..e515e61 100644 --- a/dohlistener.go +++ b/dohlistener.go @@ -119,6 +119,10 @@ func (s DoHListener) parseAndRespond(b []byte, w http.ResponseWriter, r *http.Re a = new(dns.Msg) a.SetRcode(q, dns.RcodeServerFailure) } + + // Pad the packet according to rfc8467 and rfc7830 + padAnswer(q, a) + out, err := a.Pack() if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/dotclient.go b/dotclient.go index 6a9caa9..907f7c6 100644 --- a/dotclient.go +++ b/dotclient.go @@ -41,6 +41,9 @@ func (d *DoTClient) Resolve(q *dns.Msg, ci ClientInfo) (*dns.Msg, error) { "resolver": d.endpoint, "protocol": "dot", }).Debug("querying upstream resolver") + + // Add padding to the query before sending over TLS + padQuery(q) return d.pipeline.Resolve(q) } diff --git a/dotlistener_test.go b/dotlistener_test.go index d74a280..edbc02b 100644 --- a/dotlistener_test.go +++ b/dotlistener_test.go @@ -78,6 +78,56 @@ func TestDoTListenerMutual(t *testing.T) { require.Equal(t, 1, upstream.HitCount()) } +func TestDoTListenerPadding(t *testing.T) { + // Define a listener that does not respond with padding + upstream := NewDNSClient("8.8.8.8:53", "udp") + + // Find a free port for the listener + addr, err := getLnAddress() + require.NoError(t, err) + + // Create the listener + tlsServerConfig, err := TLSServerConfig("", "testdata/server.crt", "testdata/server.key", false) + require.NoError(t, err) + + s := NewDoTListener(addr, DoTListenerOptions{TLSConfig: tlsServerConfig}, upstream) + go func() { + err := s.Start() + require.NoError(t, err) + }() + defer s.Stop() + time.Sleep(time.Second) + + // Make a client talking to the listener. Need to trust the issue of the server certificate. + tlsConfig, err := TLSClientConfig("testdata/ca.crt", "", "") + require.NoError(t, err) + c := NewDoTClient(addr, DoTClientOptions{TLSConfig: tlsConfig}) + + // Send a query with the EDNS0 option set. This should cause the listener to add padding in the response. + q := new(dns.Msg) + q.SetQuestion("google.com.", dns.TypeA) + q.SetEdns0(4096, false) + a, err := c.Resolve(q, ClientInfo{}) + require.NoError(t, err) + edns0 := a.IsEdns0() + require.NotNil(t, edns0, "expected EDNS0 option in response") + var foundPadding bool + for _, opt := range edns0.Option { + if opt.Option() == dns.EDNS0PADDING { + foundPadding = true + } + } + require.True(t, foundPadding, "expected padding in response") + + // Send a query without the EDNS0 option. The response should not have an EDNS0 record. + q = new(dns.Msg) + q.SetQuestion("google.com.", dns.TypeA) + a, err = c.Resolve(q, ClientInfo{}) + require.NoError(t, err) + edns0 = a.IsEdns0() + require.Nil(t, edns0, "unexpected EDNS0 option in response") +} + func getLnAddress() (string, error) { l, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { diff --git a/padding.go b/padding.go new file mode 100644 index 0000000..16db9f6 --- /dev/null +++ b/padding.go @@ -0,0 +1,102 @@ +package rdns + +import "github.com/miekg/dns" + +// QueryPaddingBlockSize is used to pad queries sent over DoT and DoH according to rfc8467 +const QueryPaddingBlockSize = 128 + +// ResponsePaddingBlockSize is used to pad responses over DoT and DoH according to rfc8467 +const ResponsePaddingBlockSize = 468 + +// Fixed buffers to draw on for padding (rather than allocate every time) +var respPadBuf [ResponsePaddingBlockSize]byte +var queryPadBuf [QueryPaddingBlockSize]byte + +// Add padding to an answer before it's sent back over DoH or DoT accodring to rfc8467. +// Don't call this for un-encrypted responses as they should not be padded. +func padAnswer(q, a *dns.Msg) { + edns0q := q.IsEdns0() + if edns0q == nil { // Don't pad if the client does not support EDNS0 + return + } + + // Add an OPT record to the answer if there isn't one already + edns0a := a.IsEdns0() + if edns0a == nil { + a.SetEdns0(edns0q.UDPSize(), edns0q.Do()) + edns0a = a.IsEdns0() + } + + // If the answer has padding, grab that and truncate it before re-calculating the length + var paddingOpt *dns.EDNS0_PADDING + for _, opt := range edns0a.Option { + if opt.Option() == dns.EDNS0PADDING { + paddingOpt = opt.(*dns.EDNS0_PADDING) + paddingOpt.Padding = nil + } + } + + // Add the padding option if there isn't one already + if paddingOpt == nil { + paddingOpt = new(dns.EDNS0_PADDING) + edns0a.Option = append(edns0a.Option, paddingOpt) + } + + // Calculate the desired padding length + len := a.Len() + padLen := ResponsePaddingBlockSize - len%ResponsePaddingBlockSize + + // If padding would make the packet larger than the request EDNS0 allows, we need + // to truncate it. + if len+padLen > int(edns0q.UDPSize()) { + padLen = int(edns0q.UDPSize()) - len + if padLen < 0 { // Still doesn't fit? Give up on padding + padLen = 0 + } + } + paddingOpt.Padding = respPadBuf[0:padLen] +} + +// Adds padding to a query that is to be sent over DoH or DoT. Padding length is according to rfc8467. +// This should not be used for plain (unencrypted) DNS. +func padQuery(q *dns.Msg) { + edns0q := q.IsEdns0() + if edns0q == nil { // Don't pad if the client does not support EDNS0 + return + } + + // If the query has padding, grab that and truncate it before re-calculating the length + var paddingOpt *dns.EDNS0_PADDING + for _, opt := range edns0q.Option { + if opt.Option() == dns.EDNS0PADDING { + paddingOpt = opt.(*dns.EDNS0_PADDING) + paddingOpt.Padding = nil + } + } + + // Add the padding option if there isn't one already + if paddingOpt == nil { + paddingOpt = new(dns.EDNS0_PADDING) + edns0q.Option = append(edns0q.Option, paddingOpt) + } + + // Calculate the desired padding length + len := q.Len() + padLen := QueryPaddingBlockSize - len%QueryPaddingBlockSize + paddingOpt.Padding = respPadBuf[0:padLen] +} + +// Remove padding from a query or response. Typically needed when sending a response that was received +// via TLS over a plan connection. +func stripPadding(m *dns.Msg) { + edns0 := m.IsEdns0() + if edns0 == nil { // Nothing to do here + return + } + for i, opt := range edns0.Option { + if opt.Option() == dns.EDNS0PADDING { + edns0.Option = append(edns0.Option[:i], edns0.Option[i+1:]...) + } + } + +} diff --git a/padding_test.go b/padding_test.go new file mode 100644 index 0000000..fbe9cf1 --- /dev/null +++ b/padding_test.go @@ -0,0 +1,66 @@ +package rdns + +import ( + "testing" + + "github.com/miekg/dns" + "github.com/stretchr/testify/require" +) + +func TestAnswerPadding(t *testing.T) { + q := new(dns.Msg) + q.SetQuestion("google.com.", dns.TypeA) + + a := new(dns.Msg) + a.SetReply(q) + + // No EDNS0 in the query, there should be none in the answer either + padAnswer(q, a) + edns0 := a.IsEdns0() + require.Nil(t, edns0, "unexpected EDNS0 option in response") + + // With EDNS0 in the query now, should see padding in the response + q.SetEdns0(4096, false) + a.SetReply(q) + padAnswer(q, a) + edns0 = a.IsEdns0() + require.NotNil(t, edns0, "missing EDNS0 in response") + require.Zero(t, a.Len()%ResponsePaddingBlockSize, "response not padded to the correct length") + + // Use padding on packet that needs to be smaller than the usual padded size + maxSize := ResponsePaddingBlockSize - 10 + q.SetEdns0(uint16(maxSize), false) + a.SetReply(q) + padAnswer(q, a) + edns0 = a.IsEdns0() + require.NotNil(t, edns0, "missing EDNS0 in response") + require.Equal(t, maxSize, a.Len(), "not padded to the correct length") +} + +func TestQueryPadding(t *testing.T) { + q := new(dns.Msg) + q.SetQuestion("google.com.", dns.TypeA) + + // No padding should be added when there's no EDNS0 in the query + padQuery(q) + edns0 := q.IsEdns0() + require.Nil(t, edns0, "unexpected EDNS0 option in query") + + // Now with EDNS0, the query should be padded to the right size + q.SetEdns0(4096, false) + padQuery(q) + edns0 = q.IsEdns0() + require.NotNil(t, edns0, "missing EDNS0 in query") + require.Zero(t, q.Len()%QueryPaddingBlockSize, "query not padded to the correct length") +} + +func TestStripPadding(t *testing.T) { + q := new(dns.Msg) + q.SetQuestion("google.com.", dns.TypeA) + q.SetEdns0(4096, false) + len1 := q.Len() + padQuery(q) + stripPadding(q) + len2 := q.Len() + require.Equal(t, len1, len2, "padding not stripped off correctly") +}