From 91a2552fd37fcd4f85a8db62bb96b4fb723457c4 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Thu, 24 Aug 2023 18:53:19 +0200 Subject: [PATCH 1/2] dns/dnsmessage: compress all names while appending to a buffer --- dns/dnsmessage/message.go | 5 +-- dns/dnsmessage/message_test.go | 31 ++++++++++++++++--- .../fuzz/FuzzUnpackThenPack/80809ab2946d47dd | 2 ++ .../fuzz/FuzzUnpackThenPack/c9709b403534b1ab | 2 ++ 4 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd create mode 100644 dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab diff --git a/dns/dnsmessage/message.go b/dns/dnsmessage/message.go index 9ddf2c2292..608f9577c0 100644 --- a/dns/dnsmessage/message.go +++ b/dns/dnsmessage/message.go @@ -2001,13 +2001,14 @@ func (n *Name) pack(msg []byte, compression map[string]int, compressionOff int) // Miss. Add the suffix to the compression table if the // offset can be stored in the available 14 bytes. - if len(msg) <= int(^uint16(0)>>2) { + newPtr := len(msg) - compressionOff + if newPtr <= int(^uint16(0)>>2) { if nameAsStr == "" { // allocate n.Data on the heap once, to avoid allocating it // multiple times (for next labels). nameAsStr = string(n.Data[:n.Length]) } - compression[nameAsStr[i:]] = len(msg) - compressionOff + compression[string(n.Data[i:])] = newPtr } } } diff --git a/dns/dnsmessage/message_test.go b/dns/dnsmessage/message_test.go index ee42febbc2..a1c3c92b40 100644 --- a/dns/dnsmessage/message_test.go +++ b/dns/dnsmessage/message_test.go @@ -1820,24 +1820,20 @@ func TestBuilderNameCompressionWithNonZeroedName(t *testing.T) { if err := b.StartQuestions(); err != nil { t.Fatalf("b.StartQuestions() unexpected error: %v", err) } - name := MustNewName("go.dev.") if err := b.Question(Question{Name: name}); err != nil { t.Fatalf("b.Question() unexpected error: %v", err) } - // Character that is not part of the name (name.Data[:name.Length]), // shouldn't affect the compression algorithm. name.Data[name.Length] = '1' if err := b.Question(Question{Name: name}); err != nil { t.Fatalf("b.Question() unexpected error: %v", err) } - msg, err := b.Finish() if err != nil { t.Fatalf("b.Finish() unexpected error: %v", err) } - expect := []byte{ 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, // header 2, 'g', 'o', 3, 'd', 'e', 'v', 0, 0, 0, 0, 0, // question 1 @@ -1847,3 +1843,30 @@ func TestBuilderNameCompressionWithNonZeroedName(t *testing.T) { t.Fatalf("b.Finish() = %v, want: %v", msg, expect) } } + +func TestBuilderCompressionInAppendMode(t *testing.T) { + maxPtr := int(^uint16(0) >> 2) + b := NewBuilder(make([]byte, maxPtr, maxPtr+512), Header{}) + b.EnableCompression() + if err := b.StartQuestions(); err != nil { + t.Fatalf("b.StartQuestions() unexpected error: %v", err) + } + if err := b.Question(Question{Name: MustNewName("go.dev.")}); err != nil { + t.Fatalf("b.Question() unexpected error: %v", err) + } + if err := b.Question(Question{Name: MustNewName("go.dev.")}); err != nil { + t.Fatalf("b.Question() unexpected error: %v", err) + } + msg, err := b.Finish() + if err != nil { + t.Fatalf("b.Finish() unexpected error: %v", err) + } + expect := []byte{ + 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, // header + 2, 'g', 'o', 3, 'd', 'e', 'v', 0, 0, 0, 0, 0, // question 1 + 0xC0, 12, 0, 0, 0, 0, // question 2 + } + if !bytes.Equal(msg[maxPtr:], expect) { + t.Fatalf("msg[maxPtr:] = %v, want: %v", msg[maxPtr:], expect) + } +} diff --git a/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd b/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd new file mode 100644 index 0000000000..57fa02f1cd --- /dev/null +++ b/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0000\x00\x01\x00\x00\x00\x01\x00\x00\x000000\x01.\x0000000000\x00\x040000") diff --git a/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab b/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab new file mode 100644 index 0000000000..a24f172690 --- /dev/null +++ b/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0000\x00\x01\x00\x00\x00\x00\x00\x00\x01.\x000000") From eb23195734794ab2b211677e5e3616de5f0eb7be Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Fri, 25 Aug 2023 10:30:30 +0200 Subject: [PATCH 2/2] bytes -> bits typo --- dns/dnsmessage/message.go | 4 ++-- dns/dnsmessage/message_test.go | 4 ++++ .../testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd | 2 -- .../testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab | 2 -- 4 files changed, 6 insertions(+), 6 deletions(-) delete mode 100644 dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd delete mode 100644 dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab diff --git a/dns/dnsmessage/message.go b/dns/dnsmessage/message.go index 608f9577c0..0215a5dde3 100644 --- a/dns/dnsmessage/message.go +++ b/dns/dnsmessage/message.go @@ -2000,7 +2000,7 @@ func (n *Name) pack(msg []byte, compression map[string]int, compressionOff int) } // Miss. Add the suffix to the compression table if the - // offset can be stored in the available 14 bytes. + // offset can be stored in the available 14 bits. newPtr := len(msg) - compressionOff if newPtr <= int(^uint16(0)>>2) { if nameAsStr == "" { @@ -2008,7 +2008,7 @@ func (n *Name) pack(msg []byte, compression map[string]int, compressionOff int) // multiple times (for next labels). nameAsStr = string(n.Data[:n.Length]) } - compression[string(n.Data[i:])] = newPtr + compression[nameAsStr[i:]] = newPtr } } } diff --git a/dns/dnsmessage/message_test.go b/dns/dnsmessage/message_test.go index a1c3c92b40..23fb3d5748 100644 --- a/dns/dnsmessage/message_test.go +++ b/dns/dnsmessage/message_test.go @@ -1820,20 +1820,24 @@ func TestBuilderNameCompressionWithNonZeroedName(t *testing.T) { if err := b.StartQuestions(); err != nil { t.Fatalf("b.StartQuestions() unexpected error: %v", err) } + name := MustNewName("go.dev.") if err := b.Question(Question{Name: name}); err != nil { t.Fatalf("b.Question() unexpected error: %v", err) } + // Character that is not part of the name (name.Data[:name.Length]), // shouldn't affect the compression algorithm. name.Data[name.Length] = '1' if err := b.Question(Question{Name: name}); err != nil { t.Fatalf("b.Question() unexpected error: %v", err) } + msg, err := b.Finish() if err != nil { t.Fatalf("b.Finish() unexpected error: %v", err) } + expect := []byte{ 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, // header 2, 'g', 'o', 3, 'd', 'e', 'v', 0, 0, 0, 0, 0, // question 1 diff --git a/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd b/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd deleted file mode 100644 index 57fa02f1cd..0000000000 --- a/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/80809ab2946d47dd +++ /dev/null @@ -1,2 +0,0 @@ -go test fuzz v1 -[]byte("0000\x00\x01\x00\x00\x00\x01\x00\x00\x000000\x01.\x0000000000\x00\x040000") diff --git a/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab b/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab deleted file mode 100644 index a24f172690..0000000000 --- a/dns/dnsmessage/testdata/fuzz/FuzzUnpackThenPack/c9709b403534b1ab +++ /dev/null @@ -1,2 +0,0 @@ -go test fuzz v1 -[]byte("0000\x00\x01\x00\x00\x00\x00\x00\x00\x01.\x000000")