Commit Graph

543 Commits

Author SHA1 Message Date
Tom Thorogood d8ff986484 Use for range loops instead of manual for loops (#937)
* Use for range loops instead of manual loops

* Use for range loop in Msg.CopyTo

This is a separate commit as the change is slightly more than just
switching the loop style.

* Use for range loop in DNSKEY.publicKeyRSA

* Add explen comment to DNSKEY.publicKeyRSA
2019-03-18 07:06:44 +00:00
Tom Thorogood 513c1ff221 Simplify and unify various returns (#893) 2019-01-04 10:19:42 +00:00
Tom Thorogood 57ca5ae8f4 Use headerSize const instead of hardcoded 12 (#894) 2019-01-04 10:19:01 +00:00
Tom Thorogood 09499bd07f Use IsFqdn and Fqdn helper functions more (#892) 2019-01-04 08:13:00 +00:00
Tom Thorogood b955100a79
Move RR header packing out of generated code (#885) 2019-01-04 10:09:14 +10:30
Tom Thorogood 813bd39114 Stop using packDomainName in IsDomainName (#873)
* Fork packDomainName for IsDomainName

* Eliminate msg buffer from packDomainName2

* Eliminate compression code from packDomainName2

* Remove off argument and return from packDomainName2

* Remove bs buffer from packDomainName2

* Merge packDomainName2 into IsDomainName

* Eliminate root label special case from IsDomainName

* Remove ls variable from IsDomainName

* Fixup comments in IsDomainName

* Remove msg == nil special cases from packDomainName

* Eliminate lenmsg variable from packDomainName

* Eliminate label counting from packDomainName

* Change off length check in IsDomainName

* Fix IsDomainName for escaped names

* Use strings.HasSuffix for IsFqdn

* Revert "Use strings.HasSuffix for IsFqdn"

I'll submit this as a seperate PR.

This reverts commit 80bf8c83700d121ea45edac0f00db52817498166.

* Cross reference IsDomainName and packDomainName

* Correct IsDomainName max length comment
2019-01-03 17:39:37 +01:00
Tom Thorogood 57b81e0614 Use an interface method for unpacking records (#884)
* Use an interface method for unpacking records

* Eliminate err var declaration from unpack functions

* Remove pointless r.Data assignment in PrivateRR.unpack
2019-01-03 17:35:32 +01:00
Tom Thorogood ff7d445081 Avoid setting the Rdlength field when packing records (#859)
* Avoid setting the Rdlength field when packing

The offset of the end of the header is now returned from the RR.pack
method, with the RDLENGTH record field being written in packRR.

To maintain compatability with callers of PackRR who might be relying
on this old behaviour, PackRR will now set rr.Header().Rdlength for
external callers. Care must be taken by callers to ensure this won't
cause a data-race.

* Prevent panic if TestClientLocalAddress fails

This came up during testing of the previous change.

* Change style of overflow check in packRR
2018-12-02 08:23:35 +00:00
Tom Thorogood 470f08e191
Reduce compression memory use with map[string]uint16 (#852)
* Reduce compression memory use with map[string]uint16

map[string]uint16 uses 25% less memory per-entry than a map[string]int
(16+2)/(16+8) = 0.75. All entries in the compression map are bound by
maxCompressionOffset which is 14-bits and fits within a uint16.

* Add PackMsg benchmark with more RRs

* Add a comment to the compressionMap struct
2018-12-02 08:50:51 +10:30
Tom Thorogood 8d24af5fb5 Fix compressed length calculations for escaped names (#850)
* Fix compressed length calculations for escaped names

* Add Len benchmark for escaped name

* Fix length with escaping after compression point

* Avoid calling escapedNameLen multiple times in domainNameLen

* Use regular quotes for question in TestMsgCompressLengthEscaped
2018-11-30 07:50:49 +00:00
Tom Thorogood 778aa4f83d
Properly calculate compressed message lengths (#833)
* Remove fullSize return from compressionLenSearch

This wasn't used anywhere but TestCompressionLenSearch, and was very
wrong.

* Add generated compressedLen functions and use them

This replaces the confusing and complicated compressionLenSlice
function.

* Use compressedLenWithCompressionMap even for uncompressed

This leaves the len() functions unused and they'll soon be removed.

This also fixes the off-by-one error of compressedLen when a (Q)NAME
is ".".

* Use Len helper instead of RR.len private method

* Merge len and compressedLen functions

* Merge compressedLen helper into Msg.Len

* Remove compress bool from compressedLenWithCompressionMap

* Merge map insertion into compressionLenSearch

This eliminates the need to loop over the domain name twice when we're
compressing the name.

* Use compressedNameLen for NSEC.NextDomain

This was a mistake.

* Remove compress from RR.len

* Add test case for multiple questions length

* Add test case for MINFO and SOA compression

These are the only RRs with multiple compressible names within the same
RR, and they were previously broken.

* Rename compressedNameLen to domainNameLen

It also handles the length of uncompressed domain names.

* Use off directly instead of len(s[:off])

* Move initial maxCompressionOffset check out of compressionLenMapInsert

This should allow us to avoid the call overhead of
compressionLenMapInsert in certain limited cases and may result in a
slight performance increase.

compressionLenMapInsert still has a maxCompressionOffset check inside
the for loop.

* Rename compressedLenWithCompressionMap to msgLenWithCompressionMap

This better reflects that it also calculates the uncompressed length.

* Merge TestMsgCompressMINFO with TestMsgCompressSOA

They're both testing the same thing.

* Remove compressionLenMapInsert

compressionLenSearch does everything compressionLenMapInsert did anyway.

* Only call compressionLenSearch in one place in domainNameLen

* Split if statement in domainNameLen

The last two commits worsened the performance of domainNameLen
noticably, this change restores it's original performance.

name                            old time/op    new time/op    delta
MsgLength-12                       550ns ±13%     510ns ±21%    ~     (p=0.050 n=10+10)
MsgLengthNoCompression-12         26.9ns ± 2%    27.0ns ± 1%    ~     (p=0.198 n=9+10)
MsgLengthPack-12                  2.30µs ±12%    2.26µs ±16%    ~     (p=0.739 n=10+10)
MsgLengthMassive-12               32.9µs ± 7%    32.0µs ±10%    ~     (p=0.243 n=9+10)
MsgLengthOnlyQuestion-12          9.60ns ± 1%    9.20ns ± 1%  -4.16%  (p=0.000 n=9+9)

* Remove stray newline from TestMsgCompressionMultipleQuestions

* Remove stray newline in length_test.go

This was introduced when resolving merge conflicts.
2018-11-30 10:03:41 +10:30
Tom Thorogood 2c039114d2 Use a table lookup for escaping unprintable bytes (#846) 2018-11-29 19:57:48 +00:00
Tom Thorogood c0747f060e Reduce allocations in UnpackDomainName by better sizing slice (#844)
* Reduce allocations in UnpackDomainName by better sizing slice

The maximum size of a domain name in presentation format is bounded by
the maximum length of a name in wire octet form and the maximum length
of a label. As s doesn't escape from UnpackDomainName, we can safely
give it the maximum capacity and it will never need to grow.

* Benchmark UnpackDomainName with lonest names possible

* Rename BenchmarkUnpackDomainNameLongestEscaped to match

* Improve maxDomainNamePresentationLength comment

* Further improve maxDomainNamePresentationLength comment
2018-11-29 19:55:51 +00:00
Tom Thorogood b1bf6f1b9b Simplify unprintable handling in UnpackDomainName (#845)
* Avoid the for loop for unprintable octets in UnpackDomainName

* Unify unprintable checking with writeTXTStringByte

These constants are identical.
2018-11-29 07:32:13 +00:00
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