From 813bd39114c25b6e394a538c312f4564227c84f7 Mon Sep 17 00:00:00 2001 From: Tom Thorogood Date: Fri, 4 Jan 2019 03:09:37 +1030 Subject: [PATCH] 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 --- defaults.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++--- labels_test.go | 16 ++++++----- msg.go | 58 ++++++++++++++++------------------------ msg_generate.go | 4 +-- msg_helpers.go | 4 +-- zmsg.go | 64 ++++++++++++++++++++++---------------------- 6 files changed, 135 insertions(+), 82 deletions(-) diff --git a/defaults.go b/defaults.go index c9240fb2..a5dab628 100644 --- a/defaults.go +++ b/defaults.go @@ -164,11 +164,74 @@ func (dns *Msg) IsEdns0() *OPT { // the number of labels. When false is returned the number of labels is not // defined. Also note that this function is extremely liberal; almost any // string is a valid domain name as the DNS is 8 bit protocol. It checks if each -// label fits in 63 characters, but there is no length check for the entire -// string s. I.e. a domain name longer than 255 characters is considered valid. +// label fits in 63 characters and that the entire name will fit into the 255 +// octet wire format limit. func IsDomainName(s string) (labels int, ok bool) { - _, labels, err := packDomainName(s, nil, 0, compressionMap{}, false) - return labels, err == nil + // XXX: The logic in this function was copied from packDomainName and + // should be kept in sync with that function. + + const lenmsg = 256 + + if len(s) == 0 { // Ok, for instance when dealing with update RR without any rdata. + return 0, false + } + + if !strings.HasSuffix(s, ".") { + s += "." + } + + // Each dot ends a segment of the name. Except for escaped dots (\.), which + // are normal dots. + + var ( + off int + begin int + wasDot bool + ) + for i := 0; i < len(s); i++ { + switch s[i] { + case '\\': + if off+1 > lenmsg { + return labels, false + } + + // check for \DDD + if i+3 < len(s) && isDigit(s[i+1]) && isDigit(s[i+2]) && isDigit(s[i+3]) { + i += 3 + begin += 3 + } else { + i++ + begin++ + } + + wasDot = false + case '.': + if wasDot { + // two dots back to back is not legal + return labels, false + } + wasDot = true + + labelLen := i - begin + if labelLen >= 1<<6 { // top two bits of length must be clear + return labels, false + } + + // off can already (we're in a loop) be bigger than lenmsg + // this happens when a name isn't fully qualified + off += 1 + labelLen + if off > lenmsg { + return labels, false + } + + labels++ + begin = i + 1 + default: + wasDot = false + } + } + + return labels, true } // IsSubDomain checks if child is indeed a child of the parent. If child and parent diff --git a/labels_test.go b/labels_test.go index d9bb556d..386df4e9 100644 --- a/labels_test.go +++ b/labels_test.go @@ -155,13 +155,15 @@ func TestIsDomainName(t *testing.T) { lab int } names := map[string]*ret{ - "..": {false, 1}, - "@.": {true, 1}, - "www.example.com": {true, 3}, - "www.e%ample.com": {true, 3}, - "www.example.com.": {true, 3}, - "mi\\k.nl.": {true, 2}, - "mi\\k.nl": {true, 2}, + "..": {false, 1}, + "@.": {true, 1}, + "www.example.com": {true, 3}, + "www.e%ample.com": {true, 3}, + "www.example.com.": {true, 3}, + "mi\\k.nl.": {true, 2}, + "mi\\k.nl": {true, 2}, + longestDomain: {true, 4}, + longestUnprintableDomain: {true, 4}, } for d, ok := range names { l, k := IsDomainName(d) diff --git a/msg.go b/msg.go index 7e8d1956..dd93cc56 100644 --- a/msg.go +++ b/msg.go @@ -231,29 +231,21 @@ func (m compressionMap) find(s string) (int, bool) { // map needs to hold a mapping between domain names and offsets // pointing into msg. func PackDomainName(s string, msg []byte, off int, compression map[string]int, compress bool) (off1 int, err error) { - off1, _, err = packDomainName(s, msg, off, compressionMap{ext: compression}, compress) - return + return packDomainName(s, msg, off, compressionMap{ext: compression}, compress) } -func packDomainName(s string, msg []byte, off int, compression compressionMap, compress bool) (off1 int, labels int, err error) { - // special case if msg == nil - lenmsg := 256 - if msg != nil { - lenmsg = len(msg) - } +func packDomainName(s string, msg []byte, off int, compression compressionMap, compress bool) (off1 int, err error) { + // XXX: A logical copy of this function exists in IsDomainName and + // should be kept in sync with this function. ls := len(s) if ls == 0 { // Ok, for instance when dealing with update RR without any rdata. - return off, 0, nil + return off, nil } - // If not fully qualified, error out, but only if msg != nil #ugly + // If not fully qualified, error out. if s[ls-1] != '.' { - if msg != nil { - return lenmsg, 0, ErrFqdn - } - s += "." - ls++ + return len(msg), ErrFqdn } // Each dot ends a segment of the name. @@ -283,8 +275,8 @@ loop: switch c { case '\\': - if off+1 > lenmsg { - return lenmsg, labels, ErrBuf + if off+1 > len(msg) { + return len(msg), ErrBuf } if bs == nil { @@ -307,19 +299,19 @@ loop: case '.': if wasDot { // two dots back to back is not legal - return lenmsg, labels, ErrRdata + return len(msg), ErrRdata } wasDot = true labelLen := i - begin if labelLen >= 1<<6 { // top two bits of length must be clear - return lenmsg, labels, ErrRdata + return len(msg), ErrRdata } // off can already (we're in a loop) be bigger than len(msg) // this happens when a name isn't fully qualified - if off+1+labelLen > lenmsg { - return lenmsg, labels, ErrBuf + if off+1+labelLen > len(msg) { + return len(msg), ErrBuf } // Don't try to compress '.' @@ -344,18 +336,15 @@ loop: } // The following is covered by the length check above. - if msg != nil { - msg[off] = byte(labelLen) + msg[off] = byte(labelLen) - if bs == nil { - copy(msg[off+1:], s[begin:i]) - } else { - copy(msg[off+1:], bs[begin:i]) - } + if bs == nil { + copy(msg[off+1:], s[begin:i]) + } else { + copy(msg[off+1:], bs[begin:i]) } off += 1 + labelLen - labels++ begin = i + 1 compBegin = begin + compOff default: @@ -365,22 +354,21 @@ loop: // Root label is special if isRootLabel(s, bs, 0, ls) { - return off, labels, nil + return off, nil } // If we did compression and we find something add the pointer here if pointer != -1 { // We have two bytes (14 bits) to put the pointer in - // if msg == nil, we will never do compression binary.BigEndian.PutUint16(msg[off:], uint16(pointer^0xC000)) - return off + 2, labels, nil + return off + 2, nil } - if msg != nil && off < lenmsg { + if off < len(msg) { msg[off] = 0 } - return off + 1, labels, nil + return off + 1, nil } // isRootLabel returns whether s or bs, from off to end, is the root @@ -1131,7 +1119,7 @@ func (dns *Msg) CopyTo(r1 *Msg) *Msg { } func (q *Question) pack(msg []byte, off int, compression compressionMap, compress bool) (int, error) { - off, _, err := packDomainName(q.Name, msg, off, compression, compress) + off, err := packDomainName(q.Name, msg, off, compression, compress) if err != nil { return off, err } diff --git a/msg_generate.go b/msg_generate.go index 93490ca3..fa232f10 100644 --- a/msg_generate.go +++ b/msg_generate.go @@ -115,9 +115,9 @@ return headerEnd, off, err switch { case st.Tag(i) == `dns:"-"`: // ignored case st.Tag(i) == `dns:"cdomain-name"`: - o("off, _, err = packDomainName(rr.%s, msg, off, compression, compress)\n") + o("off, err = packDomainName(rr.%s, msg, off, compression, compress)\n") case st.Tag(i) == `dns:"domain-name"`: - o("off, _, err = packDomainName(rr.%s, msg, off, compression, false)\n") + o("off, err = packDomainName(rr.%s, msg, off, compression, false)\n") case st.Tag(i) == `dns:"a"`: o("off, err = packDataA(rr.%s, msg, off)\n") case st.Tag(i) == `dns:"aaaa"`: diff --git a/msg_helpers.go b/msg_helpers.go index 202f4c68..c761d10f 100644 --- a/msg_helpers.go +++ b/msg_helpers.go @@ -106,7 +106,7 @@ func (hdr RR_Header) pack(msg []byte, off int, compression compressionMap, compr return off, off, nil } - off, _, err := packDomainName(hdr.Name, msg, off, compression, compress) + off, err := packDomainName(hdr.Name, msg, off, compression, compress) if err != nil { return off, len(msg), err } @@ -640,7 +640,7 @@ func unpackDataDomainNames(msg []byte, off, end int) ([]string, int, error) { func packDataDomainNames(names []string, msg []byte, off int, compression compressionMap, compress bool) (int, error) { var err error for j := 0; j < len(names); j++ { - off, _, err = packDomainName(names[j], msg, off, compression, compress) + off, err = packDomainName(names[j], msg, off, compression, compress) if err != nil { return len(msg), err } diff --git a/zmsg.go b/zmsg.go index a45f33bf..44268461 100644 --- a/zmsg.go +++ b/zmsg.go @@ -37,7 +37,7 @@ func (rr *AFSDB) pack(msg []byte, off int, compression compressionMap, compress if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Hostname, msg, off, compression, false) + off, err = packDomainName(rr.Hostname, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -161,7 +161,7 @@ func (rr *CNAME) pack(msg []byte, off int, compression compressionMap, compress if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Target, msg, off, compression, compress) + off, err = packDomainName(rr.Target, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -229,7 +229,7 @@ func (rr *DNAME) pack(msg []byte, off int, compression compressionMap, compress if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Target, msg, off, compression, false) + off, err = packDomainName(rr.Target, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -433,7 +433,7 @@ func (rr *KX) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Exchanger, msg, off, compression, false) + off, err = packDomainName(rr.Exchanger, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -517,7 +517,7 @@ func (rr *LP) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Fqdn, msg, off, compression, false) + off, err = packDomainName(rr.Fqdn, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -529,7 +529,7 @@ func (rr *MB) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mb, msg, off, compression, compress) + off, err = packDomainName(rr.Mb, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -541,7 +541,7 @@ func (rr *MD) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Md, msg, off, compression, compress) + off, err = packDomainName(rr.Md, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -553,7 +553,7 @@ func (rr *MF) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mf, msg, off, compression, compress) + off, err = packDomainName(rr.Mf, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -565,7 +565,7 @@ func (rr *MG) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mg, msg, off, compression, compress) + off, err = packDomainName(rr.Mg, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -577,11 +577,11 @@ func (rr *MINFO) pack(msg []byte, off int, compression compressionMap, compress if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Rmail, msg, off, compression, compress) + off, err = packDomainName(rr.Rmail, msg, off, compression, compress) if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Email, msg, off, compression, compress) + off, err = packDomainName(rr.Email, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -593,7 +593,7 @@ func (rr *MR) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mr, msg, off, compression, compress) + off, err = packDomainName(rr.Mr, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -609,7 +609,7 @@ func (rr *MX) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mx, msg, off, compression, compress) + off, err = packDomainName(rr.Mx, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -641,7 +641,7 @@ func (rr *NAPTR) pack(msg []byte, off int, compression compressionMap, compress if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Replacement, msg, off, compression, false) + off, err = packDomainName(rr.Replacement, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -693,7 +693,7 @@ func (rr *NS) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Ns, msg, off, compression, compress) + off, err = packDomainName(rr.Ns, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -705,7 +705,7 @@ func (rr *NSAPPTR) pack(msg []byte, off int, compression compressionMap, compres if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Ptr, msg, off, compression, false) + off, err = packDomainName(rr.Ptr, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -717,7 +717,7 @@ func (rr *NSEC) pack(msg []byte, off int, compression compressionMap, compress b if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.NextDomain, msg, off, compression, false) + off, err = packDomainName(rr.NextDomain, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -843,7 +843,7 @@ func (rr *PTR) pack(msg []byte, off int, compression compressionMap, compress bo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Ptr, msg, off, compression, compress) + off, err = packDomainName(rr.Ptr, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -859,11 +859,11 @@ func (rr *PX) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Map822, msg, off, compression, false) + off, err = packDomainName(rr.Map822, msg, off, compression, false) if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mapx400, msg, off, compression, false) + off, err = packDomainName(rr.Mapx400, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -911,11 +911,11 @@ func (rr *RP) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mbox, msg, off, compression, false) + off, err = packDomainName(rr.Mbox, msg, off, compression, false) if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Txt, msg, off, compression, false) + off, err = packDomainName(rr.Txt, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -955,7 +955,7 @@ func (rr *RRSIG) pack(msg []byte, off int, compression compressionMap, compress if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.SignerName, msg, off, compression, false) + off, err = packDomainName(rr.SignerName, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -975,7 +975,7 @@ func (rr *RT) pack(msg []byte, off int, compression compressionMap, compress boo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Host, msg, off, compression, false) + off, err = packDomainName(rr.Host, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -1015,7 +1015,7 @@ func (rr *SIG) pack(msg []byte, off int, compression compressionMap, compress bo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.SignerName, msg, off, compression, false) + off, err = packDomainName(rr.SignerName, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -1055,11 +1055,11 @@ func (rr *SOA) pack(msg []byte, off int, compression compressionMap, compress bo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Ns, msg, off, compression, compress) + off, err = packDomainName(rr.Ns, msg, off, compression, compress) if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Mbox, msg, off, compression, compress) + off, err = packDomainName(rr.Mbox, msg, off, compression, compress) if err != nil { return headerEnd, off, err } @@ -1115,7 +1115,7 @@ func (rr *SRV) pack(msg []byte, off int, compression compressionMap, compress bo if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Target, msg, off, compression, false) + off, err = packDomainName(rr.Target, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -1171,11 +1171,11 @@ func (rr *TALINK) pack(msg []byte, off int, compression compressionMap, compress if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.PreviousName, msg, off, compression, false) + off, err = packDomainName(rr.PreviousName, msg, off, compression, false) if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.NextName, msg, off, compression, false) + off, err = packDomainName(rr.NextName, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -1187,7 +1187,7 @@ func (rr *TKEY) pack(msg []byte, off int, compression compressionMap, compress b if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Algorithm, msg, off, compression, false) + off, err = packDomainName(rr.Algorithm, msg, off, compression, false) if err != nil { return headerEnd, off, err } @@ -1255,7 +1255,7 @@ func (rr *TSIG) pack(msg []byte, off int, compression compressionMap, compress b if err != nil { return headerEnd, off, err } - off, _, err = packDomainName(rr.Algorithm, msg, off, compression, false) + off, err = packDomainName(rr.Algorithm, msg, off, compression, false) if err != nil { return headerEnd, off, err }