* Make request.Request smaller
This makes the request struct smaller and removes the pointer to the do
boolean (tri-bool) as size == 0 will indicate if we have cached it.
Family can be a int8 because it only carries 3 values, Size itself is
just a uint16 under the covers.
This is a more comprehensive fix than #3292Closes#3292
Signed-off-by: Miek Gieben <miek@miek.nl>
* cache: fix test
this now needs a valid response writter
Signed-off-by: Miek Gieben <miek@miek.nl>
miekg/dns recently became more aggressive in setting TC; anything that's
chopped of a response results in TC. Amend the tests. Disputable if
these TC checks still add something.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Fix EDNS0 compliance
Do SizeAndDo in the server (ScrubWriter) and remove all uses of this
from the plugins. 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 there. The *rewrite* plugin used
this to add custom EDNS0 option codes that the server needs to
understand.
This also needs a new release of miekg/dns because it triggered a
race-condition that was basicly there forever.
See:
* https://github.com/miekg/dns/issues/857
* https://github.com/miekg/dns/pull/859
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>
Signed-off-by: Miek Gieben <miek@miek.nl>
* typos in comments
Signed-off-by: Miek Gieben <miek@miek.nl>
* fix truncation bug
* Generate records with generic RRs
* Remove SoundCloud from test name
* Comment for binary-search -1 adjustment
Explain why the binary search may have exited with
a reply size that is too large by one record.
* Refactor to remove sub variable
patch suggested by miek removes unnecessary sub
variable for removing a single line from the
reply.Extra length.
* server: actually scrub response
Did all the worked, hooked it up wrongly :(
This also needs test, but those are hard(er) because we only receive
packets after they have been decoded; i.e. we never see the wirefmt.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Add tests
Add a test for checking is compression pointers are set in the packet.
This also adds an undocumented 'large' feature to the erratic plugin to
send large responses that should be compressed.
Commenting the Scrub out in server results in:
=== RUN TestCompressScrub
--- FAIL: TestCompressScrub (0.00s)
compression_scrub_test.go:41: Expected returned packet to be < 512, got 839
FAIL
exit status 1
FAIL github.com/coredns/coredns/test 0.036s
Actually checking the size might be easier, but lets be thorough here
and check the pointers them selves.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Fix tests
Signed-off-by: Miek Gieben <miek@miek.nl>
* plugin erratic: fix e.large
always put an rr in the reply, fix e.large in erractic and add test to
check for it.
Signed-off-by: Miek Gieben <miek@miek.nl>
Every plugin needs to deal with EDNS0 and should call Scrub to make a
message fit the client's buffer. Move this functionality into the server
and wrapping the ResponseWriter into a ScrubWriter that handles these
bits for us. Result:
Less code and faster, because multiple chained plugins could all be
calling scrub and SizeAndDo - now there is just one place.
Most tests in file/* and dnssec/* needed adjusting because in those unit
tests you don't see OPT RRs anymore. The DNSSEC signer was also looking
at the returned OPT RR to see if it needed to sign - as those are now
added by the server (and thus later), this needed to change slightly.
Scrub itself still exist (for backward compat reasons), but has been
made a noop. Scrub has been renamed to scrub as it should not be used by
external plugins.
Fixes: #2010
Signed-off-by: Miek Gieben <miek@miek.nl>
* Add regression test for broken DNS truncation
With the latest changes to the implementation, request.Scrub() returns
invalid response messages which are larger than the advertised buffer
size. This commit adds a failing test for that.
* Add benchmark for response truncation
* request: re-adjust size for edns0 RR
Cherry-picked the test and benchmark commits from #1962 and make the
failing test not fail, by adding minimal fix in request.go.
This makes size 12 bytes smaller if we need to add back the OPT RR in
the message.
Closes: #1962
Signed-off-by: Miek Gieben <miek@miek.nl>
* Scrub: correctly check for EDNS0, not only Do
Make Scrub check for EDNS0, not only if the Do bit is set.
Signed-off-by: Miek Gieben <miek@miek.nl>
* request: add LocalIP
Fix TODO that was added: Add LocalIP and test the Clear() method.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Move to Errorf
PR feedback and move to Errorf instead Fatalf.
Signed-off-by: Miek Gieben <miek@miek.nl>
* request.Scrub: test for rl==size case
Make a test case for the new break statement in Scrub and also
account for the OPT record that may get re-added in SizeAndDo() -
otherwise we may break clients that expect this.
* Fix comment
Use binary search to find the minimal message size, that contains whole
RRs and fits the client's buffer. This is better then just setting
entire sections to `nil`. Extend the tests to test for additional and
answer section truncation. In the first case we *don't* set the TC bit.
This function now also set Compression to true.
* Fix truncation of messages longer than permitted by the client
CoreDNS currently doesn't respect the maximum response size advertised
by the client and returns the full answer on a message with the TC bit
set. This breaks client implementations which rely on DNS servers
respecting the advertised size limit, for example the Ruby stdlib
client. It also has negative network performance implications, as large
messages will be split up into multiple UDP packets, even though the
client will discard the truncated response anyway.
While RFC 2181 permits the response of partial RRSets, finding the
correct number of records fitting into the advertised response size is
non-trivial. As clients should ignore truncated messages, this change
simply removes the full RRSet on truncated messages.
* Remove incorrect etcd test assertion
If a client requests a TXT record larger than its advertised buffer
size, a DNS server should _not_ respond with the answer, but truncate
the message and set the TC bit, so that the client can retry using TCP.
* Rename middleware to plugin
first pass; mostly used 'sed', few spots where I manually changed
text.
This still builds a coredns binary.
* fmt error
* Rename AddMiddleware to AddPlugin
* Readd AddMiddleware to remain backwards compat
Check for a nil message and if we have a question section. Request is
usually called with an external Msg that already saw validation checks,
but we may also call it from message we create of our own, that may or
may not adhire to this. Just be more robust in this case.
This PR reverts a previous commit that was applied to master.
Move all (almost all) Go files in middleware into their
own packages. This makes for better naming and discoverability.
Lot of changes elsewhere to make this change.
The middleware.State was renamed to request.Request which is better,
but still does not cover all use-cases. It was also moved out middleware
because it is used by `dnsserver` as well.
A pkg/dnsutil packages was added for shared, handy, dns util functions.
All normalize functions are now put in normalize.go
