From 9c1ee5d5ca5abaaecdc2920269cbd2a71aa09ad2 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Tue, 10 Sep 2013 18:09:22 +0000 Subject: [PATCH] Update IsDomainName This new functions just compiles the domain to wire format, if that works, the name is deemed OK. It is also much less strict than the older code. Almost everything is allowed in the name, except two dots back to back (there is an explicit test for that). --- defaults.go | 37 +++----------------------------- dnssec.go | 3 +-- dnssec_test.go | 2 +- labels_test.go | 21 ++++++++++--------- msg.go | 57 +++++++++++++++++++++++++++++++++++++------------- parse_test.go | 5 +++-- 6 files changed, 62 insertions(+), 63 deletions(-) diff --git a/defaults.go b/defaults.go index e97ef309..f4b80341 100644 --- a/defaults.go +++ b/defaults.go @@ -160,40 +160,9 @@ func (dns *Msg) IsEdns0() *OPT { // Note that non fully qualified domain name is considered valid, in this case the // last label is counted in the number of labels. // When false is returned the number of labels is not defined. -func IsDomainName(s string) (int, bool) { - // use PackDomainName - if buf == nil { - buf = make([]byte, 256) - } - lenmsg, err := PackDomainName(s, buf, 0, nil, false) - if err != nil { - return 0, false - } - // There are no compression pointers, because the map was nil, so - // walk the binary name and count the length bits - this is the number - // of labels. - off := 0 - labels := 0 -Loop: - for { - if off >= lenmsg { - return labels, false - } - c := int(buf[off]) - switch c & 0xC0 { - case 0x00: - if c == 0x00 { - // end of name - break - } - if off+c > lenmsg { - return labels, false - } - labels++ - off += c - } - } - return labels, true +func IsDomainName(s string) (labels int, ok bool) { + _, labels, err := packDomainName(s, nil, 0, nil, false) + return labels, err == nil } // IsSubDomain checks if child is indeed a child of the parent. Both child and diff --git a/dnssec.go b/dnssec.go index af28ea17..fdb103bf 100644 --- a/dnssec.go +++ b/dnssec.go @@ -222,8 +222,7 @@ func (rr *RRSIG) Sign(k PrivateKey, rrset []RR) error { rr.OrigTtl = rrset[0].Header().Ttl rr.TypeCovered = rrset[0].Header().Rrtype rr.TypeCovered = rrset[0].Header().Rrtype - x := CountLabel(rrset[0].Header().Name) - rr.Labels = uint8(x) + rr.Labels = uint8(CountLabel(rrset[0].Header().Name)) if strings.HasPrefix(rrset[0].Header().Name, "*") { rr.Labels-- // wildcard, remove from label count diff --git a/dnssec_test.go b/dnssec_test.go index 9456744f..ac6ab765 100644 --- a/dnssec_test.go +++ b/dnssec_test.go @@ -173,7 +173,7 @@ func TestSignVerify(t *testing.T) { sig := new(RRSIG) sig.Hdr = RR_Header{"miek.nl.", TypeRRSIG, ClassINET, 14400, 0} sig.TypeCovered = soa.Hdr.Rrtype - sig.Labels, _, _ = IsDomainName(soa.Hdr.Name) + sig.Labels = uint8(CountLabel(soa.Hdr.Name)) sig.OrigTtl = soa.Hdr.Ttl sig.Expiration = 1296534305 // date -u '+%s' -d"2011-02-01 04:25:05" sig.Inception = 1293942305 // date -u '+%s' -d"2011-01-02 04:25:05" diff --git a/labels_test.go b/labels_test.go index 2e335a78..0493d830 100644 --- a/labels_test.go +++ b/labels_test.go @@ -95,20 +95,21 @@ domainLoop: func TestIsDomainName(t *testing.T) { type ret struct { ok bool - lab uint8 - l uint8 + lab int } names := map[string]*ret{ - "www.example.com": &ret{true, 3, 15}, - "www.example.com.": &ret{true, 3, 16}, - "mi\\k.nl.": &ret{true, 2, 8}, - "mi\\k.nl": &ret{true, 2, 7}, + "..": &ret{false, 1}, + "@.": &ret{true, 1}, + "www.example.com": &ret{true, 3}, + "www.example.com.": &ret{true, 3}, + "mi\\k.nl.": &ret{true, 2}, + "mi\\k.nl": &ret{true, 2}, } for d, ok := range names { - l1, l2, k := IsDomainName(d) - if ok.ok != k || ok.lab != l1 || ok.l != l2 { - t.Logf("Got %v %d %d for %s ", k, l1, l2, d) - t.Logf(" %v %d %d for %s ", ok.ok, ok.lab, ok.l, d) + l, k := IsDomainName(d) + if ok.ok != k || ok.lab != l { + t.Logf(" got %v %d for %s ", k, l, d) + t.Logf("have %v %d for %s ", ok.ok, ok.lab, d) t.Fail() } } diff --git a/msg.go b/msg.go index d5ccd821..e647d6fa 100644 --- a/msg.go +++ b/msg.go @@ -212,14 +212,31 @@ var RcodeToString = map[int]string{ // 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) { - lenmsg := len(msg) + off1, _, err = packDomainName(s, msg, off, compression, compress) + return +} + +func packDomainName(s string, msg []byte, off int, compression map[string]int, compress bool) (off1 int, labels int, err error) { + // special case if msg == nil + lenmsg := 256 + if msg != nil { + lenmsg = len(msg) + } ls := len(s) if ls == 0 { // Ok, for instance when dealing with update RR without any rdata. - return off, nil + return off, 0, nil } - // If not fully qualified, error out - if s[ls-1] != '.' { - return lenmsg, ErrFqdn + // If not fully qualified, error out, but only if msg == nil #ugly + switch { + case msg == nil: + if s[ls-1] != '.' { + s += "." + ls++ + } + case msg != nil: + if s[ls-1] != '.' { + return lenmsg, 0, ErrFqdn + } } // Each dot ends a segment of the name. // We trade each dot byte for a length byte. @@ -239,7 +256,7 @@ func PackDomainName(s string, msg []byte, off int, compression map[string]int, c } ls-- if off+1 > lenmsg { - return lenmsg, ErrBuf + return lenmsg, labels, ErrBuf } // check for \DDD if i+2 < ls && bs[i] >= '0' && bs[i] <= '9' && @@ -255,22 +272,30 @@ func PackDomainName(s string, msg []byte, off int, compression map[string]int, c } if bs[i] == '.' { + if i > 0 && bs[i-1] == '.' { + // two dots back to back is not legal + return lenmsg, labels, ErrRdata + } if i-begin >= 1<<6 { // top two bits of length must be clear - return lenmsg, ErrRdata + return lenmsg, labels, 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 > lenmsg { - return lenmsg, ErrBuf + return lenmsg, labels, ErrBuf + } + if msg != nil { + msg[off] = byte(i - begin) } - msg[off] = byte(i - begin) offset := off off++ for j := begin; j < i; j++ { if off+1 > lenmsg { - return lenmsg, ErrBuf + return lenmsg, labels, ErrBuf + } + if msg != nil { + msg[off] = bs[j] } - msg[off] = bs[j] off++ } // Dont try to compress '.' @@ -294,24 +319,28 @@ func PackDomainName(s string, msg []byte, off int, compression map[string]int, c } } } + labels++ begin = i + 1 } } // Root label is special if len(bs) == 1 && bs[0] == '.' { - return off, nil + return off, labels, 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 msg[nameoffset], msg[nameoffset+1] = packUint16(uint16(pointer ^ 0xC000)) off = nameoffset + 1 goto End } - msg[off] = 0 + if msg != nil { + msg[off] = 0 + } End: off++ - return off, nil + return off, labels, nil } // Unpack a domain name. diff --git a/parse_test.go b/parse_test.go index 2f23d4fa..99edc590 100644 --- a/parse_test.go +++ b/parse_test.go @@ -357,7 +357,8 @@ func TestParseFailure(t *testing.T) { "miek.nl. IN AAAA ::x", "miek.nl. IN MX a0 miek.nl.", "miek.nl aap IN MX mx.miek.nl.", - "miek.nl. IN CNAME ", + // "miek.nl. IN CNAME ", // actually valid nowadays, zero size rdata + "miek.nl. IN CNAME ..", "miek.nl. PA MX 10 miek.nl.", "miek.nl. ) IN MX 10 miek.nl.", } @@ -365,7 +366,7 @@ func TestParseFailure(t *testing.T) { for _, s := range tests { _, err := NewRR(s) if err == nil { - t.Log("Should have triggered an error") + t.Logf("Should have triggered an error: \"%s\"", s) t.Fail() } }