Make it public as TsigGenerateWithProvider and update the docs a little.
And TsigVerifyWithProvider also - tweak those docs also a little.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Send DNS query in one packet when using TCP/TLS
* fix review comments
* Removed net.Buffers
* Added unit-tests for writing messages over TCP in one go
* Support generic net.PacketConn's for the Server
This commit adds support for listening on generic net.PacketConn's for
UDP DNS requests, previously *net.UDPConn was the only supported type.
In the event of a future v2 of this module, this should be streamlined.
* Eliminate wrapper functions around RunLocalXServerWithFinChan
* Eliminate RunLocalTCPServerWithTsig function
* Replace RunLocalTLSServer with a wrapper around RunLocalTCPServer
This reduces code duplication.
* Add net.PacketConn server tests
This provides coverage over nearly all of the newly added code (with
the unfortunate exception of (*response).RemoteAddr).
* Fix broken client_test.go tests
a433fbede4 was merged into master between this PR being opened and
being merged. This broke the CI tests in rather strange ways as the
code was being merged into master in a way that wasn't at all clear.
This commit fixes the two broken lines.
* answer queries with no matching handler with RcodeRefused
* update documentation
* mark HandleFailed deprecated
* add handleRefused and use it to answer requests matching no handler
* silence noise maker
Co-authored-by: Brian <brian@pop-os.localdomain>
One of the test from DNS Compliance testing validates that if the opcode
is not supported, a NOTIMPL rcode is returned.
e0884144dd/genreport.c (L293)
This diff makes the default acceptfunc support this case and reply with
NOTIMPL instead of FORMERR.
* Simplify Server.readTCP
This slightly alters the error behaviour, but it should not be
observable outside of a decorated reader. I don't believe the old
behaviour was either obvious, documented or correct.
* Simplify TCP reading in client Conn
This alters the error behaviour in possibly observable ways, though
this is quite subtle and may not actually be readily observable.
Conn.ReadMsgHeader should behave the same way and still returns
ErrShortRead for length being too short.
Conn.Read will no longer return ErrShortRead if the length == 0,
otherwise it should be largely similar.
* Remove redundant error check in Conn.ReadMsgHeader
Generalize the srv.Unsafe and make it pluggeable. Also add a default
accept function that allows to discard malformed DNS messages very early
on. Before we allocate and parse anything furher.
Also re-use the client's message when sending a reply.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Add test that srv.conns is empty in checkInProgressQueriesAtShutdownServer
* Track ResponseWriter Close without nil-ing tcp
* Remove LocalAddr and RemoteAddr panic after Close
This is no longer needed as the tcp field is no longer set to nil in
Close.
* Add more explicit WriteMsg panic after Close
Previously this would panic with `dns: Write called after Close` which
is obviously less clear.
* Panic if Hijack is called after Close
Previously this worked, but later calls to Write would panic. This is
more explicit.
* Return an error if Close called multiple times
Neither io.Closer, nor ResponseWriter, provide any guarantees about the
behaviour of multiple calls to Close. This was made explicit in
https://golang.org/cl/8575043 and in practice implementations differ
wildly.
This matches ShutdownContext which returns an error if called multiple
times.
* Check map len under lock in checkInProgressQueriesAtShutdownServer
* Correct error message in checkInProgressQueriesAtShutdownServer
* Remove panic-after-Close from Hijack
* Return errors, not panic, on Write after Close
* Avoid overriding aLongTimeAgo read deadline
The (*Server).readTCP and (*Server).readUDP methods both call
SetReadDeadline. If they race with ShutdownContext, they can override
the aLongTimeAgo read deadline that is set in ShutdownContext. That
will cause ShutdownContext to hang or timeout.
* Reword readTCP comment
* Bugfix for miekg/dns#748
w.msg was being prematurely cleared prior to use by TsigVerify
* Modified patch after feedback from tmthrgd
Added a disposeBuffer method to the server that's passed a response. This wipes the reference to and frees the buffer used to store the message after TSIG validation has occured, not before. Since the pool is an attribute of the server (and the logic refers to a server UDPSize attribute), it made sense to make this a function of the server rather than a function of the response.
* Added TestServerRoundtripTsig to server_test.go
This test generates a TSIG signed query, and makes sure that server TSIG validation does not produce an error.
* Fixed data races introduced by TestServerRoundtripTsig
* Simplified error signalling in TestServerRoundtripTsig
* RunLocalUDPServerWithFinChan variadic closure argument added
This (clever hack suggested by tmthrgd) allows specifying field values (like TsigSecret) on Server instances at test time without making the race detector grouchy, but is backwards compatible with existing invocations of RunLocalUDPServerWithFinChan.
* [tls] Carry TLS state within (possibly) response writer
This allows a server to make decision wether or not the link used to
connect to the DNS server is using TLS.
This can be used by the handler for instance to (but not limited to):
- log that the request was TLS vs TCP
- craft specific responsed knowing that the link is secured
- return custom answers based on client cert (if provided)
...
Fixes#711
* Address @tmthrgd comments:
- do not check whether w.tcp is nil
- create RR after setting txt value
* Address @miekg comments.
Attempt to make a TLS connection state specific test, it goes over
testing each individual server types (TLS, TCP, UDP) and validate that
tls.Connectionstate is only accessible when expected.
* ConnectionState() returns value instead of pointer
* * make ConnectionStater.ConnectionState() return a pointer again
* rename interface ConnectionState to ConnectionStater
* fix nits pointed by @tmthrgd
* @tmthrgd comment: Do not use concret type in `ConnectionState`
* Make Shutdown wait for connections to terminate gracefully
* Add graceful shutdown test files from #713
* Tidy up graceful shutdown tests
* Call t.Error directly in checkInProgressQueriesAtShutdownServer
* Remove timeout arguments from RunLocal*ServerWithFinChan
* Merge defers together in (*Server).serve
This removes the defer from the UDP path, in favour of directly
calling (*sync.WaitGroup).Done after (*Serve).serveDNS has
returned.
* Replace checkInProgressQueriesAtShutdownServer implementation
This performs dialing, writing and reading as three seperate steps.
* Add sleep after writing shutdown test messages
* Avoid race condition when setting server timeouts
Server timeouts cannot be set after the server has started without
triggering the race detector. The timeout's are not strictly needed, so
remove them.
* Use a sync.Cond for testShutdownNotify
Using a chan erroneously triggered the race detector, using a sync.Cond
avoids that problem.
* Remove TestShutdownUDPWithContext
This doesn't really add anything.
* Move shutdown and conn into (*Server).init
* Only log ResponseWriter.WriteMsg error once
* Test that ShutdownContext waits for the reply
* Remove stray newline from diff
* Rename err to ctxErr in ShutdownContext
* Reword testShutdownNotify comment
* Use strings.TrimSuffix in ListenAndServe for TLS
This replaces the if/else statements with something simpler.
Interestingly, the first pull request I submitted to this library was
to fix the tcp6-tls case way back in 4744e915eb.
* Add SO_REUSEPORT implementation
Fixes#654
* Rename Reuseport field to ReusePort
* Rename supportsReuseport to match ReusePort
* Rename listenUDP and listenTCP file to listen_*.go
* Clear the response.msg field after unpacking
The allocated buffer cannot be freed by the garbage collector while the
response is alive, by clearing msg here, the GC can collect the buffer
sooner.
* Use a sync.Pool for UDP message buffers
* Return UDP message buffer to pool in all paths
* Move udpPool.New closure out of (*Server).init
The closure used to capture the *Server which would cause a reference
loop and prevent it from ever being released by the garbage collector.
This also gives the closure a more obvious name in memory profiles:
github.com/miekg/dns.makeUDPBuffer.func1 rather than
github.com/miekg/dns.(*Server).init.func1.
* Remove redundant parenthesis
These were caught with:
gofmt -r '(a) -> a' -w *.go
This commit only includes the changes where the formatting makes the
ordering of operations clear.
* Remove more redundant parenthesis
These were caught with:
gofmt -r '(a) -> a' -w *.go
This commit includes the remaining changes where the formatting does not
make the ordering of operations as clear as the previous commit.
* Split central ServeDNS code out of (*Server).serve
* Add UDP and TCP specific (*Server).serve wrappers
* Move UDP serve functionality into serveUDPPacket
* Merge serve into serveTCPConn
* Cleanup serveTCPConn replacing goto with for
* defer Close in serveTCPConn
* Remove remoteAddr field from response struct
* Fix broken tsigSecret check in serveDNS
* Reorder serveDNS arguments
This makes it consistent with the ordering of arguments to
serveUDPPacket and serveTCPConn.
* Split central ServeDNS code out of (*Server).serve
* Add UDP and TCP specific (*Server).serve wrappers
* Move UDP serve functionality into serveUDPPacket
* Merge serve into serveTCPConn
* Cleanup serveTCPConn replacing goto with for
* defer Close in serveTCPConn
* Remove remoteAddr field from response struct
* Fix broken tsigSecret check in serveDNS
* Reorder serveDNS arguments
This makes it consistent with the ordering of arguments to
serveUDPPacket and serveTCPConn.
serveTCP calls reader.ReadTCP in the accept loop rather than in
the per-connection goroutine. If an attacker opens a connection
and leaves it idle, this will block the accept loop until the
connection times out (2s by default). During this time no other
incoming connections will succeed, preventing legitimate queries
from being answered.
This commit moves the call to reader.ReadTCP into the per-connection
goroutine. It also adds a missing call to Close whose absence allowed
file-descirptors to leak in select cases.
This attack and fix have no impact on serving UDP queries.
The check for srv.started being false is in the wrong place, it should
be after Accept not after ReadTCP. If Shutdown is called, serveTCP will
currently return a 'use of closed network connection' error, which is
undesired.
This commit mirrors the behaviour of serveUDP with respect to Shutdown.
* Do not reutrn ErrShortRead in readUDP
A read of zero bytes indicates a peer shutdown for TCP sockets -- and
thus returning ErrShortRead is fine in readTCP -- but not for UDP
sockets. For UDP sockets a read of zero bytes literally indicates a
zero-byte datagram, and is a valid return value not indicating an error.
Removing this case will cause readUDP to correctly return a zero-byte
message.
* Return non-temporary error from serveUDP loop
Fixes#613