Commit Graph

306 Commits

Author SHA1 Message Date
Kian-Meng Ang 0089167cae
Fix typos (#1413)
Found via `codespell -L ede,ans,te,crasher`
2023-01-14 08:19:09 +01:00
Miek Gieben 69924a02cf
Make tsigGenerateProvider/TsigVerifyProvider public (#1382)
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>
2022-06-21 10:37:36 +02:00
Tom Thorogood 33e64002b6
Support TsigProvider for Server and Transfer (#1331)
Automatically submitted.
2022-02-05 00:23:49 +00:00
R+ 2543d8bb2d
Close the UDPConn before returning if `setUDPSocketOptions` went wrong (#1227)
Detail: https://github.com/miekg/dns/issues/1226
2021-02-25 09:24:39 +01:00
Andrey Meshkov 67bd57debd
Send DNS query in one packet when using TCP/TLS (#1219)
* 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
2021-02-13 19:49:02 +01:00
Tom Thorogood fa528cceb7
Fix PacketConnReader typo in DecorateReader docs (#1189) 2020-10-25 02:26:19 +10:30
Tom Thorogood 0e1c4e69dd
Support generic net.PacketConn's for the Server (#1174)
* 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.
2020-10-25 02:23:01 +10:30
Brian Shea 034f791cf8
answer queries with no matching handler with RcodeRefused (#1151)
* 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>
2020-08-20 08:41:45 +02:00
chantra d89f1e3d4b Reply with NOTIMPL when Opcode is not supported (#982)
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.
2019-06-17 16:13:02 +01:00
Yaroslav Kolomiiets 1545072057 ignore Z flag in queries, clear Z flag in automatic replies (#976) 2019-05-23 20:54:24 +01:00
Tom Thorogood 357af3038a Always return UDP buffers to pool (#958) 2019-04-30 07:12:45 +01:00
Tom Thorogood 834f456fff Simplify TCP reading (#935)
* 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
2019-03-11 10:59:25 +00:00
Tom Thorogood 337216f9a7 Use net.Buffers for writing TCP message (#934) 2019-03-10 13:46:14 +00:00
Tom Thorogood 1a5555c783 Split Server.serve into separate TCP and UDP methods (#933)
* Split Server.serve into separate TCP and UDP methods

* Merge reject cases in Server.serveDNS

* Inline Server.disposeBuffer method
2019-03-10 12:52:08 +00:00
Tom Thorogood 53b8a87e14 Correct Close() check in Server.serve (#932)
This was changed in ec3443f85d, but I
missed this function. Apparently no one noticed.
2019-03-10 11:59:36 +00:00
Miek Gieben eef2495fa3
Move srv.Handling selection to init() (#931)
Move this code to the server's init function to get it out of the
hotpath.

Signed-off-by: Miek Gieben <miek@miek.nl>
2019-03-10 11:14:57 +00:00
Miek Gieben 284bad20d8
Manually revert go workers (98a1ef45) (#928)
Manually revert the worker model.

Signed-off-by: Miek Gieben <miek@miek.nl>
2019-03-09 13:34:22 +00:00
JINMEI Tatuya e838e1e3ce corrected default value of Server.MsgAcceptFunc as documented (#920)
the description says DefaultMsgAcceptFunc but actually defaultMsgAcceptFunc
was used.
2019-03-07 07:02:29 +00:00
Tom Thorogood 513c1ff221 Simplify and unify various returns (#893) 2019-01-04 10:19:42 +00:00
Tom Thorogood 5b818eed53 Use a value reciever for defaultReader (#888)
As defaultReader contains a single pointer, we can pass it by value and
avoid an allocation when it's put inside an interface{}.
2019-01-04 08:09:23 +00:00
Miek Gieben fef7963e99 remove newlines
Signed-off-by: Miek Gieben <miek@miek.nl>
2018-11-28 22:32:50 +00:00
Miek Gieben ab67d69d9b review
Signed-off-by: Miek Gieben <miek@miek.nl>
2018-11-28 22:25:28 +00:00
Miek Gieben 74dbfccc11 Code Review
Signed-off-by: Miek Gieben <miek@miek.nl>
2018-11-28 18:42:48 +00:00
Miek Gieben 2c18e7259a Add MsgAcceptFunc in server
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>
2018-11-28 18:42:48 +00:00
Tom Thorogood ec3443f85d Fix TCP connection tracking memory leak (#808)
* 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
2018-11-03 09:44:07 +00:00
Tom Thorogood a3f088363b Hold srv.lock while calling SetReadDeadline (#780)
* Hold srv.lock while calling SetReadDeadline

* Only hold the read lock in readTCP and readUDP
2018-10-09 18:46:15 +01:00
Tom Thorogood 36ffedf7d0
Avoid overriding aLongTimeAgo read deadline (#774)
* 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
2018-10-06 01:46:28 +09:30
Tom Thorogood 008c8ca764 Explicitly panic after (*response).Close (#769)
* Explicitly panic after (*response).Close

* Prefix panics with package name

* Harden TestResponseAfterClose by comparing panic message
2018-10-04 07:39:21 +01:00
Tom Thorogood 7f61c6631b
Fix dominikh/go-tools nits (#758)
* Remove unused functions and consts

* Address gosimple nits

* Address staticcheck nits

This excludes several that were intentional or weren't actual errors.

* Reduce size of lex struct

This reduces the size of the lex struct by 8 bytes from:
  lex.token string: 0-16 (size 16, align 8)
  lex.tokenUpper string: 16-32 (size 16, align 8)
  lex.length int: 32-40 (size 8, align 8)
  lex.err bool: 40-41 (size 1, align 1)
  lex.value uint8: 41-42 (size 1, align 1)
  padding: 42-48 (size 6, align 0)
  lex.line int: 48-56 (size 8, align 8)
  lex.column int: 56-64 (size 8, align 8)
  lex.torc uint16: 64-66 (size 2, align 2)
  padding: 66-72 (size 6, align 0)
  lex.comment string: 72-88 (size 16, align 8)
to:
  lex.token string: 0-16 (size 16, align 8)
  lex.tokenUpper string: 16-32 (size 16, align 8)
  lex.length int: 32-40 (size 8, align 8)
  lex.err bool: 40-41 (size 1, align 1)
  lex.value uint8: 41-42 (size 1, align 1)
  lex.torc uint16: 42-44 (size 2, align 2)
  padding: 44-48 (size 4, align 0)
  lex.line int: 48-56 (size 8, align 8)
  lex.column int: 56-64 (size 8, align 8)
  lex.comment string: 64-80 (size 16, align 8)

* Reduce size of response struct

This reduces the size of the response struct by 8 bytes from:
  response.msg []byte: 0-24 (size 24, align 8)
  response.hijacked bool: 24-25 (size 1, align 1)
  padding: 25-32 (size 7, align 0)
  response.tsigStatus error: 32-48 (size 16, align 8)
  response.tsigTimersOnly bool: 48-49 (size 1, align 1)
  padding: 49-56 (size 7, align 0)
  response.tsigRequestMAC string: 56-72 (size 16, align 8)
  response.tsigSecret map[string]string: 72-80 (size 8, align 8)
  response.udp *net.UDPConn: 80-88 (size 8, align 8)
  response.tcp net.Conn: 88-104 (size 16, align 8)
  response.udpSession *github.com/tmthrgd/dns.SessionUDP: 104-112 (size 8, align 8)
  response.writer github.com/tmthrgd/dns.Writer: 112-128 (size 16, align 8)
  response.wg *sync.WaitGroup: 128-136 (size 8, align 8)
to:
  response.msg []byte: 0-24 (size 24, align 8)
  response.hijacked bool: 24-25 (size 1, align 1)
  response.tsigTimersOnly bool: 25-26 (size 1, align 1)
  padding: 26-32 (size 6, align 0)
  response.tsigStatus error: 32-48 (size 16, align 8)
  response.tsigRequestMAC string: 48-64 (size 16, align 8)
  response.tsigSecret map[string]string: 64-72 (size 8, align 8)
  response.udp *net.UDPConn: 72-80 (size 8, align 8)
  response.tcp net.Conn: 80-96 (size 16, align 8)
  response.udpSession *github.com/tmthrgd/dns.SessionUDP: 96-104 (size 8, align 8)
  response.writer github.com/tmthrgd/dns.Writer: 104-120 (size 16, align 8)
  response.wg *sync.WaitGroup: 120-128 (size 8, align 8)
2018-09-27 04:02:05 +09:30
Tom Thorogood 60d113313c Move ServeMux into seperate file (#753)
This reduces the clutter in server.go.
2018-09-26 10:20:48 +01:00
Daniel Selifonov ab16005053 Bugfix for miekg/dns#748 (#749)
* 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.
2018-09-26 09:19:35 +01:00
chantra 833bf76c28 [tls] Carry TLS state within (possibly) response writer (#728)
* [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`
2018-09-22 18:34:55 +01:00
Tom Thorogood b0dc93d276
Make Shutdown wait for connections to terminate gracefully (#717)
* 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
2018-09-13 23:06:28 +09:30
Tom Thorogood e875a31a5c
Add SO_REUSEPORT support (#736)
* 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
2018-09-10 20:12:54 +09:30
Tom Thorogood bf6da3a5bd Reduce UDP server memory usage (#735)
* 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.
2018-09-08 17:15:17 +01:00
Tom Thorogood 4bda1db839 Improve documentation of maxWorkersCount const (#726)
While I'm here, rename it to maxIdleWorkersCount so it's purpose is
clearer.

Updates #722
Updates #725
2018-08-16 17:10:20 +01:00
Tom Thorogood c9b812d1d9 Remove redundant parenthesis (#727)
* 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.
2018-08-16 17:05:27 +01:00
Tom Thorogood 208cd1e89e Simplify unlocking dance in *Serve functions (#719) 2018-08-10 22:50:48 +01:00
Tom Thorogood b559d43c31 Abstract shutdown checking into seperate function (#716) 2018-07-28 13:47:30 +01:00
Uladzimir Trehubenka 621df0907e Make MaxTCPQueries configurable (#673) 2018-05-14 20:12:20 +01:00
Uladzimir Trehubenka 98a1ef4565 Use workers instead spawning goroutines for each incoming DNS request (#664)
* Use workers instead spawning goroutines for each incoming DNS request

* Replace count (int) with inUse (bool)
2018-05-09 16:44:32 +01:00
Tom Thorogood 01d59357d4 Cleanup serve function (reland) (#667)
* 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.
2018-04-06 16:09:55 +01:00
Miek Gieben 7fdfb0141b Revert "Cleanup serve function (#653)"
This reverts commit d174bbf0a5.
2018-04-01 12:27:36 +01:00
Tom Thorogood d174bbf0a5 Cleanup serve function (#653)
* 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.
2018-03-30 14:50:27 +01:00
Miek Gieben 43913f2f4f
Fix for CVE-2017-15133 TCP DOS (#631)
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.
2018-01-25 10:36:19 +00:00
Tom Thorogood f5ac34d755 Fix TCP Shutdown 'use of closed network connection' (#623)
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.
2018-01-10 13:37:59 +00:00
Tom Thorogood ddd8477be2 Ignore malformed UDP datagrams without headers (#622)
Ignore malformed UDP datagrams with incomplete DNS headers
2018-01-10 07:51:00 +00:00
Tom Thorogood 69d25e845f Fixes #613 & #619 (#621)
* 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
2018-01-09 13:57:26 +00:00
Miek Gieben e2db8456df
Revert "Fixes #613 (#617)" (#620)
This reverts commit ac8cd7878c.
2018-01-09 08:03:27 +00:00
Twitch ac8cd7878c Fixes #613 (#617)
* Fixes #613

* use net.Error interface for detecting temporary errors
2018-01-06 16:01:19 +00:00