From 9b94696b115d2d1394388e2b15c8ff05e5273cdf Mon Sep 17 00:00:00 2001 From: Ben Kaplan <77549469+kaplanben@users.noreply.github.com> Date: Thu, 1 Dec 2022 16:07:13 +0200 Subject: [PATCH] plugin/edns: remove truncating of question section on bad EDNS version (#5787) * plugin/edns: remove truncating of question section on bad EDNS version EDNS requests of "Unknown Version" removed the query section altogether. Not sure why since this is not require (see [link](https://kb.isc.org/docs/edns-compatibility-dig-queries) This cause issues with DNS solutions that uses this information (initial queried name, type and class) in order to route the response to the right client (e.g. PDNS). The change here is to keep the inital query section as is. Signed-off-by: Ben Kaplan * adding tests for edns0 version check Signed-off-by: Ben Kaplan * adding tests for non-edns0 version check Signed-off-by: Ben Kaplan Signed-off-by: Ben Kaplan --- plugin/pkg/edns/edns.go | 5 +---- plugin/pkg/edns/edns_test.go | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/plugin/pkg/edns/edns.go b/plugin/pkg/edns/edns.go index 31f57ea9b..cd8639915 100644 --- a/plugin/pkg/edns/edns.go +++ b/plugin/pkg/edns/edns.go @@ -36,8 +36,7 @@ func SupportedOption(option uint16) bool { // 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 -// reason this response should not contain a question RR in the question section. +// returned Msg is valid to be returned to the client (and should). func Version(req *dns.Msg) (*dns.Msg, error) { opt := req.IsEdns0() if opt == nil { @@ -48,8 +47,6 @@ func Version(req *dns.Msg) (*dns.Msg, error) { } m := new(dns.Msg) m.SetReply(req) - // zero out question section, wtf. - m.Question = nil o := new(dns.OPT) o.Hdr.Name = "." diff --git a/plugin/pkg/edns/edns_test.go b/plugin/pkg/edns/edns_test.go index a775b50f1..1976779bd 100644 --- a/plugin/pkg/edns/edns_test.go +++ b/plugin/pkg/edns/edns_test.go @@ -10,20 +10,32 @@ func TestVersion(t *testing.T) { m := ednsMsg() m.Extra[0].(*dns.OPT).SetVersion(2) - _, err := Version(m) + r, err := Version(m) if err == nil { t.Errorf("Expected wrong version, but got OK") } + if r.Question == nil { + t.Errorf("Expected question section, but got nil") + } + if r.Rcode != dns.RcodeBadVers { + t.Errorf("Expected Rcode to be of BADVER (16), but got %d", r.Rcode) + } + if r.Extra == nil { + t.Errorf("Expected OPT section, but got nil") + } } func TestVersionNoEdns(t *testing.T) { m := ednsMsg() m.Extra = nil - _, err := Version(m) + r, err := Version(m) if err != nil { t.Errorf("Expected no error, but got one: %s", err) } + if r != nil { + t.Errorf("Expected nil since not an EDNS0 request, but did not got nil") + } } func ednsMsg() *dns.Msg {