Commit Graph

529 Commits

Author SHA1 Message Date
Miek Gieben fa589750ad
Merge pull request #842 from tmthrgd/compression-map-escaped
Put escaped names into compression map
2018-11-28 23:39:22 +00:00
Tom Thorogood 07ae768ab1
Put escaped names into compression map in PackDomainName 2018-11-29 09:49:18 +10:30
Miek Gieben 1c92765836
Merge pull request #830 from miekg/passfunc
Add MsgAcceptFunc
2018-11-28 23:07:35 +00:00
Miek Gieben f92da6fc6e Code review
Signed-off-by: Miek Gieben <miek@miek.nl>
2018-11-28 22:40:08 +00:00
Tom Thorogood 6aa28be819
Bail early from UnpackDomainName when name is too long (#839)
* Simplify maxDomainNameWireOctets checking in UnpackDomainName

* Don't return too long name in UnpackDomainName

* Simplify root domain return in UnpackDomainName

* Bail early from UnpackDomainName when name is too long

This drastically reduces the amount of garbage created
in UnpackDomainName for certain malicious names.

The wire formatted name
 "\x3Faaabbbcccdddeeefffggghhhiiijjjkkklllmmmnnnooopppqqqrrrssstttuuu\xC0\x00"
would previously generate 1936B of garbage (36112B since maxCompressionPointers
was raised) before returning the "too many compression pointers" error, while
it now generates just 384B of garbage.

* Change +1 budget comment to reflect spec

This better reflects what maxDomainNameWireOctets is actually measuring.

* Remove budget check from after loop in UnpackDomainName

This can never be tripped as budget is always checked immediately after
subtracting inside the loop.

* Improve UnpackDomainName documentation
2018-11-29 08:26:30 +10:30
Miek Gieben 091d66a39f
Merge pull request #818 from tmthrgd/comp-opt
Improve PackDomainName performance
2018-11-28 18:53:23 +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
Miek Gieben 6bf402f3c4
Fix "too many compression points" for valid message (#835)
* Increase the maximum number of allowed compression pointers

* Add a Pack+Unpack test case for many compression pointers

* Clarify maxCompressionPointers comment
2018-11-28 11:45:22 +00:00
Tom Thorogood 64a73613cd Use range loop in packBufferWithCompressionMap (#837)
* Use range loops in Msg.packBufferWithCompressionMap

* Remove rr set variables in Msg.packBufferWithCompressionMap

* Move Header var down in Msg.packBufferWithCompressionMap

* Move stripTsig comment into Msg.Unpack
2018-11-28 11:44:23 +00:00
Tom Thorogood d193d08243
Clarify maxCompressionPointers comment 2018-11-28 21:38:37 +10:30
Tom Thorogood c567cfc2bb
Increase the maximum number of allowed compression pointers 2018-11-28 19:52:41 +10:30
Tom Thorogood 7ae05cdcf8
Use map[string]struct{} for compression map in Len (#820)
* Use map[string]struct{} for compression map in Len

map[string]int requires 8 bytes per entry to store the unused position
information.

* Add MsgLength benchmark with more RRs
2018-11-28 08:02:08 +10:30
Tom Thorogood 34d23c00e1
Add bounds check comment to dddToByte 2018-11-28 07:42:44 +10:30
Tom Thorogood 03d7306558 Fix NotImp RCode string (#819)
* Fix NOTIMP typo in RcodeToString

RFC 6895 lists RCODE 4 as NotImp.

* Accept legacy NOTIMPL spelling in StringToRcode
2018-11-27 14:38:33 +00:00
Tom Thorogood e2f69345fd Avoid creating compression map for question only Msg (#823)
* Pass dns.Compress explicitly to packBufferWithCompressionMap

* Avoid creating compression map for question only Msg

This idea was inspired by:
  "Skip dname compression for replies with no answers."
 https://www.nlnetlabs.nl/bugs-script/show_bug.cgi?id=235

* Continue compressing multiple questions
2018-11-27 14:34:07 +00:00
Miek Gieben 1ff265a784
Remove ErrTruncated from the library (#815)
* Remove ErrTruncated from the library

ErrTruncated is removed. This (correctly) assume that a truncated
message will be fully formed. Any message that isn't fully formed will
return (most likely) an unpack error.

Any program using ErrTruncated will fail to compile when they update to
this version: this is by design: you're doing it wrong. For checking if
a message was truncated you should checked the msg.Truncated boolean;
assuming the unpack didn't fail.

Fixes #814

Signed-off-by: Miek Gieben <miek@miek.nl>

* Restore tests

Signed-off-by: Miek Gieben <miek@miek.nl>
2018-11-27 14:26:11 +00:00
Tom Thorogood 30d0133e57
Revert commits unrelated to PackDomainName
This reverts commit 2eeda8aabc,
                    8e6e188a87
                and 7bef528091.
2018-11-27 17:55:48 +10:30
Tom Thorogood f522504216
Eliminate roBs allocation from packDomainName
This allocation only occurred when s was escaped, but will no longer
occur.
2018-11-26 16:43:21 +10:30
Tom Thorogood 07ed56b1d6
Add isRootLabel helper for packDomainName
This handles the bs == nil case internally.
2018-11-26 16:38:15 +10:30
Tom Thorogood 149f3c884f
Move bs allocation above length check in packDomainName 2018-11-26 16:16:08 +10:30
Tom Thorogood 7f4b3bb806
Only copy once per \DDD in packDomainName
Previously the remainder of bs would be copied twice.
2018-11-26 16:13:32 +10:30
Tom Thorogood 6aa05940d5
Reset roBs even if compress is false in packDomainName
By only resetting roBs when compress is true, the compression map can
end up with inconsistent entries between compress being true and false.
2018-11-26 16:04:34 +10:30
Tom Thorogood 896cef4ce4
Replace bsFresh variable with bsDirty in packDomainName
This avoids needing to initialise it to true.
2018-11-26 16:00:37 +10:30
Tom Thorogood 5547fd63a0
Fix garbage after name in compression map
When packDomainName is called with an escaped domain name and compress
being true, bs wasn't be truncated to the correct length and would
include garbage that would be included in the compression map.
2018-11-26 15:53:29 +10:30
Tom Thorogood 260b5b401d
Only compute i-begin once in packDomainName 2018-11-26 15:09:47 +10:30
Tom Thorogood e5bc3b14fb
Use lenmsg rather than len(msg) in packDomainName
This is purely for consistency, they are always equal at this point.
2018-11-26 15:08:05 +10:30
Tom Thorogood 9358e95aef
Simplify final returns from packDomainName 2018-11-26 15:07:17 +10:30
Tom Thorogood 926752f160
Remove nameoffset variable from packDomainName
This is now always equal to off, so use that instead.
2018-11-26 15:05:51 +10:30
Tom Thorogood 03053758d4
Add whitespace to packDomainName 2018-11-26 15:04:35 +10:30
Tom Thorogood 4c43711692
Remove End goto in packDomainName 2018-11-26 15:03:49 +10:30
Tom Thorogood 36a30d2e58
Remove tainted zeroing from packDomainName
With the label copying now moved after the compression, the msg buffer
will no longer be tainted and need clearing.
2018-11-26 15:02:17 +10:30
Tom Thorogood 8995ae83e3
Move label copying below compression in packDomainName
When the dname is found in the compression map and compress is true,
this copy is as it will simply be overwritten later. This could provide
a very slight speedup.
2018-11-26 14:59:17 +10:30
Tom Thorogood ecef32b31b
Merge lenmsg checks in packDomainName 2018-11-26 14:53:33 +10:30
Tom Thorogood 3534784466
Reorder if-statements in packDomainName 2018-11-26 14:50:37 +10:30
Tom Thorogood bf8065a091
Simplify double dot check in packDomainName 2018-11-26 12:32:35 +10:30
Tom Thorogood 0125cf9d0c
Simplify initial fqdn check in packDomainName 2018-11-26 12:25:11 +10:30
Tom Thorogood 77d26d8088
Avoid allocating copy of s in packDomainName
In the more common case, where the domain name has no escaping, this
avoids an allocation.
2018-11-26 12:21:09 +10:30
Tom Thorogood c12f225763
Use a switch statement in packDomainName 2018-11-26 12:07:46 +10:30
Tom Thorogood 8e6e188a87
Use NextLabel in compressionLenHelper
This avoids the allocation of Split and should have slightly better
performance.
2018-11-26 12:02:04 +10:30
Tom Thorogood 3b3a5b7c6a
Replace another for loop with copy in packDomainName
This change is included as a separate commit because this loop is not as
trivial as the others.
2018-11-26 12:00:33 +10:30
Tom Thorogood 8d08c56229
Replace simple loops with copy in packDomainName 2018-11-26 11:59:38 +10:30
Tom Thorogood ab9dd29c1d
Hoist bounds check in dddToByte
This eliminates two of the bounds checks in dddToByte and
dddStringToByte.
2018-11-26 11:52:43 +10:30
chantra 1c9c9bf4c9 properly set extended rcode when packing (#791)
* properly set extended rcode when packing

When calling `SetExtendedRcode`, we expect to get the full extended
rcode, not the rcode after we shift 4 bytes right.

* fix extended rcode

* fix TestOPTTtl test

* set error messages in TestPackExtendedBadCookie

* Set Rcode with extended rcode

* |=

* Set extended RCODE field to 0 when RCODE is not an extended one.
+ unittests

* Force setting extended rcode if we have an OPT available.

* go fmt + @tmthrgd comments

* comments and nits

* reformat comment
2018-11-17 10:30:14 +10:30
Tom Thorogood 0d29b283ac
Optimise sprintX functions in types.go (#757)
* Simplify appendByte

* Add test case and benchmark for sprintName

* Add test case and benchmark for sprintTxtOctet

* Add test case and benchmark for sprintTxt

* Use strings.Builder for sprint* functions in types.go

* Use writeByte helper in unpackString

* Rename writeByte to writeEscapedByte

This better captures the purpose of this function.
2018-10-06 02:06:59 +09:30
Tom Thorogood 7ca2be95a9 NSEC type bitmap packing bug (#768)
* Add test case for NSEC after packing and unpacking

This is ported from:
https://gist.github.com/cesarkuroiwa/ebc2b4fb1103a7e88824865184f0c73c

* Clear msg data after pointer in packDomainName
2018-10-04 07:39:45 +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 f195b71879 Replace unpackTxtString with identical unpackString (#751)
These two functions were identical (sans-variable names) before I
optimized unpackString in 5debfeec63.

This will improve the performance of it's only caller unpackTxt and is
covered by the test and benchmark added in 5debfeec63.
2018-09-26 09:14:19 +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
Miek Gieben 3745b9737d
Small comment tweaks (#678)
Some post #668 comments nits.
2018-05-16 08:53:51 +01:00
Pierre Souchay 09649115c1 Fixed len computation when size just goes beyond 14 bits (#668)
* Fixed len computation when size just goes beyond 14 bits

* Added bouds checks around 14bits

* Len() always right including when around 14bits boudaries

* Avoid splitting into labels when not applicable

* Fixed comments

* Added comments in code

* Added new test cases

* Fixed computation of Len() for SRV and all kind of records

* Fixed Sign that was relying on non-copy for Unit tests

* Removed unused padding

* Fixed typo in PackBuffer() function

* Added comment about packBufferWithCompressionMap() for testing purposes
2018-05-16 07:20:13 +01:00