Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC 2136: Dynamic updates #1513

Open
2 of 5 tasks
miekg opened this issue Nov 17, 2023 · 3 comments
Open
2 of 5 tasks

RFC 2136: Dynamic updates #1513

miekg opened this issue Nov 17, 2023 · 3 comments

Comments

@miekg
Copy link
Owner

miekg commented Nov 17, 2023

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)

  • Parsing with NewRR into RR with rdlenght=0
  • Packing msg with rdlength=0 RR
  • Unpacking TestNoRdataUnpack - off-by-one error in unpack/pack somewhere
  • Making directly from struct - guessing this works
  • Setting Rdlength to zero yourself - guessing this works
@miekg
Copy link
Owner Author

miekg commented Nov 17, 2023

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
Copy link
Owner Author

miekg commented Nov 17, 2023

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
Copy link
Owner Author

miekg commented Nov 17, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant