Skip to content

Commit

Permalink
Fix EDNS0 compliance
Browse files Browse the repository at this point in the history
Do SizeAndDo in the server and remove all uses of this from the plugins
and do it in the server in the ScrubWiriter. Also *always* do it. This
is to get into compliance  for https://dnsflagday.net/.

The pkg/edns0 now exports the EDNS0 options we understand; this is
exported to allow plugins add things here. This should be documented
still. We don't echo back options we don't understand.

Note I can't test it because of: miekg/dns#857

Running a test instance and pointing the https://ednscomp.isc.org/ednscomp
to it shows the tests are now fixed:

~~~
EDNS Compliance Tester
Checking: 'miek.nl' as at 2018-12-01T17:53:15Z

miek.nl. @147.75.204.203 (drone.coredns.io.): dns=ok edns=ok edns1=ok edns@512=ok ednsopt=ok edns1opt=ok do=ok ednsflags=ok docookie=ok edns512tcp=ok optlist=ok
miek.nl. @2604:1380:2002:a000::1 (drone.coredns.io.): dns=ok edns=ok edns1=ok edns@512=ok ednsopt=ok edns1opt=ok do=ok ednsflags=ok docookie=ok edns512tcp=ok optlist=ok

All Ok
Codes
ok - test passed.
~~~

Signed-off-by: Miek Gieben <miek@miek.nl>
  • Loading branch information
miekg committed Dec 1, 2018
1 parent 4c86e54 commit a5bed2a
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 10 deletions.
1 change: 0 additions & 1 deletion plugin/backend_lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,6 @@ func BackendError(b ServiceBackend, zone string, rcode int, state request.Reques
m.Authoritative, m.RecursionAvailable = true, true
m.Ns, _ = SOA(b, zone, state, opt)

state.SizeAndDo(m)
state.W.WriteMsg(m)
// Return success as the rcode to signal we have written to the client.
return dns.RcodeSuccess, err
Expand Down
1 change: 0 additions & 1 deletion plugin/chaos/chaos.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ func (c Chaos) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
}
m.Answer = []dns.RR{&dns.TXT{Hdr: hdr, Txt: []string{trim(hostname)}}}
}
state.SizeAndDo(m)
w.WriteMsg(m)
return 0, nil
}
Expand Down
1 change: 0 additions & 1 deletion plugin/dnssec/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ func (d Dnssec) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
if qname == z {
resp := d.getDNSKEY(state, z, do, server)
resp.Authoritative = true
state.SizeAndDo(resp)
w.WriteMsg(resp)
return dns.RcodeSuccess, nil
}
Expand Down
1 change: 0 additions & 1 deletion plugin/erratic/erratic.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ func (e *Erratic) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
time.Sleep(e.duration)
}

state.SizeAndDo(m)
w.WriteMsg(m)

return 0, nil
Expand Down
1 change: 0 additions & 1 deletion plugin/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
m := new(dns.Msg)
m.SetReply(r)
m.Authoritative, m.RecursionAvailable = true, true
state.SizeAndDo(m)
w.WriteMsg(m)

log.Infof("Notify from %s for %s: checking transfer", state.IP(), zone)
Expand Down
1 change: 0 additions & 1 deletion plugin/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func (l Logger) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
} else {
answer := new(dns.Msg)
answer.SetRcode(r, rc)
state.SizeAndDo(answer)

vars.Report(ctx, state, vars.Dropped, rcode.ToString(rc), answer.Len(), time.Now())

Expand Down
11 changes: 11 additions & 0 deletions plugin/pkg/edns/edns.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ import (
"github.com/miekg/dns"
)

// Supported is the list of supported EDNS0 options. This is export so that plugins can add (or remove) options.
// The list is indexed on the EDNS0 option code used. It is expected this map is only read from the serving Go routine, no
// writes occur, except during the server setup.
var Supported = map[uint16]struct{}{
dns.EDNS0NSID: struct{}{},
dns.EDNS0EXPIRE: struct{}{},
dns.EDNS0COOKIE: struct{}{},
dns.EDNS0TCPKEEPALIVE: struct{}{},
dns.EDNS0PADDING: struct{}{},
}

// Version checks the EDNS version in the request. If error
// is nil everything is OK and we can invoke the plugin. If non-nil, the
// returned Msg is valid to be returned to the client (and should). For some
Expand Down
1 change: 0 additions & 1 deletion plugin/whoami/whoami.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ func (wh Whoami) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)

a.Extra = []dns.RR{rr, srv}

state.SizeAndDo(a)
w.WriteMsg(a)

return 0, nil
Expand Down
24 changes: 21 additions & 3 deletions request/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (r *Request) Size() int {
// SizeAndDo adds an OPT record that the reflects the intent from request.
// The returned bool indicated if an record was found and normalised.
func (r *Request) SizeAndDo(m *dns.Msg) bool {
o := r.Req.IsEdns0() // TODO(miek): speed this up
o := r.Req.IsEdns0()
if o == nil {
return false
}
Expand All @@ -208,6 +208,16 @@ func (r *Request) SizeAndDo(m *dns.Msg) bool {
mo.SetUDPSize(o.UDPSize())
mo.Hdr.Ttl &= 0xff00 // clear flags

if len(o.Option) > 0 {
var supOpts []dns.EDNS0
for _, opt := range o.Option {
if _, ok := edns.Supported[opt.Option()]; ok {
supOpts = append(supOpts, opt)
}
}
o.Option = supOpts
}

if odo {
mo.SetDo()
}
Expand All @@ -219,6 +229,16 @@ func (r *Request) SizeAndDo(m *dns.Msg) bool {
o.SetVersion(0)
o.Hdr.Ttl &= 0xff00 // clear flags

if len(o.Option) > 0 {
var supOpts []dns.EDNS0
for _, opt := range o.Option {
if _, ok := edns.Supported[opt.Option()]; ok {
supOpts = append(supOpts, opt)
}
}
o.Option = supOpts
}

if odo {
o.SetDo()
}
Expand Down Expand Up @@ -288,7 +308,6 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
}

if rl <= size {
r.SizeAndDo(reply)
return reply
}

Expand Down Expand Up @@ -324,7 +343,6 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
// this extra m-1 step does make it fit in the client's buffer however.
}

r.SizeAndDo(reply)
reply.Truncated = true
return reply
}
Expand Down
2 changes: 2 additions & 0 deletions request/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ func NewScrubWriter(req *dns.Msg, w dns.ResponseWriter) *ScrubWriter { return &S
// scrub on the message m and will then write it to the client.
func (s *ScrubWriter) WriteMsg(m *dns.Msg) error {
state := Request{Req: s.req, W: s.ResponseWriter}

n := state.Scrub(m)
state.SizeAndDo(n)
return s.ResponseWriter.WriteMsg(n)
}

0 comments on commit a5bed2a

Please sign in to comment.