EDNS0 header should not be copied to response as it arrived

[pemensik]

What happened:

• EDNS0 header is copied back as it is. That is incorrect.

What you expected to happen:

• Only UDP size and DO bit is copied to response OPT header.
• Other extensions should be added to response only when the server understands them and wants to send data back to clients in it.

How to reproduce it (as minimally and precisely as possible):

• dig +ednsopt=NSID:6578616d706c65 @localhost -p 3053 - send query with EDNS0 extension

$ dig +ednsopt=NSID:6578616d706c65 @localhost -p 3053

; <<>> DiG 9.18.16 <<>> +ednsopt @localhost -p 3053
; (2 servers found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 1225
;; flags: qr aa rd ra ad; QUERY: 1, ANSWER: 13, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 1232
; COOKIE: 059cc5c1f6b85787 (echoed)
; NSID: 65 78 61 6d 70 6c 65 ("example")
;; QUESTION SECTION:
;.				IN	NS

;; ANSWER SECTION:
.			2420	IN	NS	c.root-servers.net.
.			2420	IN	NS	k.root-servers.net.
.			2420	IN	NS	j.root-servers.net.
.			2420	IN	NS	a.root-servers.net.
.			2420	IN	NS	l.root-servers.net.
.			2420	IN	NS	d.root-servers.net.
.			2420	IN	NS	g.root-servers.net.
.			2420	IN	NS	b.root-servers.net.
.			2420	IN	NS	h.root-servers.net.
.			2420	IN	NS	f.root-servers.net.
.			2420	IN	NS	i.root-servers.net.
.			2420	IN	NS	e.root-servers.net.
.			2420	IN	NS	m.root-servers.net.

;; Query time: 0 msec
;; SERVER: ::1#3053(localhost) (UDP)
;; WHEN: Sat Jul 01 13:42:53 CEST 2023
;; MSG SIZE  rcvd: 454

• EDNS0 is copied from the request without understanding each extension and reacting to it. ISC BIND9 and unbound send
• dig +ednsopt=NSID @localhost should not result in empty but present NSID.

Anything else we need to know?:

• Related to Create proper support for DNS cookies #6187
• I haven't found exact violation of EDNS0 RFC, but all good resolvers implement it that way. It might have been specified later in some summary RFC.
• Common public resolvers reply with their own information, not request value. For example dig +ednsopt=NSID @8.8.8.8
• Such behaviour should be expected from any EDNS extensions
• NSID RFC makes it clear NSID extension should not be forwarded to other servers.

Environment:

• the version of CoreDNS: master branch, commit 6e1263d
• Corefile:

.:3053 {
  cache
  forward . 9.9.9.9
  log
}

• logs, if applicable:

.:3053
CoreDNS-1.10.1
linux/amd64, go1.20.5, 6e1263d3
[INFO] [::1]:42478 - 58592 "NS IN . udp 48 false 1232" NOERROR qr,rd,ra,ad 420 0.007001289s
[INFO] [::1]:47058 - 1225 "NS IN . udp 51 false 1232" NOERROR qr,aa,rd,ra,ad 420 0.00014005s

• OS: Fedora 38
• Others:

[marka63]

Copying unknown options violates this section of RFC 6891.

   Any OPTION-CODE values not understood by a responder or requestor
   MUST be ignored.  Specifications of such options might wish to
   include some kind of signaled acknowledgement.  For example, an
   option specification might say that if a responder sees and supports
   option XYZ, it MUST include option XYZ in its response.

The intent is to allow EDNS options to be deployed by either the server or the client without prior agreement. It also means that an unknown EDNS option does not cause FORMERR or NOTIMP or REFUSED or BADVERS or no response.

[gcs278]

I notice this while experimenting with the https://gitlab.isc.org/isc-projects/DNS-Compliance-Testing tool.

I'll also explicitly point out, that it's the cache plugin that's copying the EDNS0 extensions.

I've been trying to understand some of the comments in #2357 for context of this change, and I'm struggling to understand the motivation of copying back the query's OPTs (minus bufsize and DO). I see it helped with testing (here it's checking the response, but it should be checking the request).

I do see there was some confusion whether CoreDNS is a "middlebox". And I think this might of fueled this interpretation. Seems like the consensus is that CoreDNS isn't a middlebox and OPT should be hop-by-hop as @pemensik is advocating for here.

It's possible the "understand" terminology was misinterpreted: in fc667b9:

The pkg/edns0 now exports the EDNS0 options we understand; this is exported to allow plugins add things there. The rewrite plugin used this to add custom EDNS0 option codes that the server needs to understand.

"Understand" seems a bit like a strong word here. It looks like we just hard coded in specific EDNS0 options that we want to copy from request to response, but we potentially do nothing useful with them, nor provide back valuable data using them (minus nsid plugin).

Please let me know if I am missing any key information about this design decision. I am WIP on #6415.

[marka63]

The EDNS bufsize MUST NOT be copied from the request. It tells the client the server's receive buffer size. Set it to an appropriate value for your server.

The DO bit is copied if your support RFC 4034 and RFC 4035 as that is where the copying is specified.