miekg / dns

DNS library in Go
https://miek.nl/2014/august/16/go-dns-package
BSD 3-Clause "New" or "Revised" License
7.99k stars 1.13k forks source link

RFC 2136: Dynamic updates #1513

Closed miekg closed 3 months ago

miekg commented 10 months ago

https://datatracker.ietf.org/doc/html/rfc2136

2.4.3 - RRset Does Not Exist

....

For this prerequisite, a requestor adds to the section a single RR whose NAME and TYPE are equal to that of the RRset whose nonexistence is required. The RDLENGTH of this record is zero (0), and RDATA field is therefore empty. CLASS must be specified as NONE in order to distinguish this condition from a valid RR whose RDLENGTH is naturally zero (0) (for example, the NULL RR). TTL must be specified as zero (0).

These are normal RRs where the RDATA is cut off, CLASS is not important here.

This issue will detail how to make those and where this lib currently fails to do so (or for which RR types)

miekg commented 10 months ago

this little test shows that all defined RRs parse correctly and give back an RR with RDLENGTH set to 0

package dns

import (
    "sort"
    "testing"
)

func TestUpdate2NoRdata(t *testing.T) {
    ex := "example.org. IN "

    types := sort.IntSlice{}
    for i := range TypeToString {
        types = append(types, int(i))
    }
    types.Sort()

    for _, ty := range types {
        if uint16(ty) == TypeNone || uint16(ty) == TypeReserved {
            continue
        }
        name := TypeToString[uint16(ty)]
        str := ex + name
        rr, err := NewRR(str)
        if err != nil {
            t.Fatalf("Failed to parse %s: %s", str, err)
        }
        t.Logf("%s => %#v", str, rr)
    }
}
miekg commented 10 months ago

packing also works

func TestUpdate2NoRdataPack(t *testing.T) {
    types := sort.IntSlice{}
    for i := range TypeToString {
        types = append(types, int(i))
    }
    types.Sort()

    for _, ty := range types {
        if uint16(ty) == TypeNone || uint16(ty) == TypeReserved {
            continue
        }
        m := new(Msg)
        if _, ok := TypeToRR[uint16(ty)]; !ok {
            t.Logf("no new function for %s", TypeToString[uint16(ty)])
            continue
        }

        rr := TypeToRR[uint16(ty)]()
        rr.Header().Name = "example.org."
        rr.Header().Rrtype = uint16(ty)
        m.Answer = []RR{rr}
        t.Logf("msg: %s\n", m)
        _, err := m.Pack()
        if err != nil {
            t.Fatalf("error packing expected msg: %v", err)
        }
    }
}

With the few RR that don't have an actual type:

    update2_test.go:44: no new function for NXT
    update2_test.go:44: no new function for UNSPEC
    update2_test.go:44: no new function for ATMA
    update2_test.go:44: no new function for ISDN
    update2_test.go:44: no new function for IXFR
    update2_test.go:44: no new function for AXFR
    update2_test.go:44: no new function for MAILB
    update2_test.go:44: no new function for MAILA

(and maybe a few other that I forgot)

miekg commented 10 months ago

Adding TestNoRdataUnpack things get indeed a bit fishy:

diff --git msg_helpers.go msg_helpers.go
index acec21f7..270be228 100644
--- msg_helpers.go
+++ msg_helpers.go
@@ -202,6 +202,11 @@ func packUint16(i uint16, msg []byte, off int) (off1 int, err error) {
 }

 func unpackUint32(msg []byte, off int) (i uint32, off1 int, err error) {
+       // for dynamic update purposes this uint32 may not be there
+       println(off, len(msg))
+       if off+1 == len(msg) {
+               return 0, off, nil
+       }
        if off+4 > len(msg) {
                return 0, len(msg), &Error{err: "overflow unpacking uint32"}

this prints:

41 43
    update2_test.go:72: failed to unpack RR with zero rdata: SOA: dns: overflow unpacking uint32

so off=41 and len(msg) = 43, this is ineed a off-by-one, because we should have reached the end of the message. Happens only for RR's that have an longer uint (uint32) as the first rdata field.

This hints in a problem with the packing.

Making that a off+2 == check, results in:

41 43
    update2_test.go:72: failed to unpack RR with zero rdata: SOA: dns: bad rdlength