From 67c37c4339bb73618621da4393227a52612dfd5e Mon Sep 17 00:00:00 2001 From: Olivier Poitrey Date: Sun, 6 Feb 2022 05:09:59 +0100 Subject: [PATCH 1/4] Invalid NSEC/3 bitmap on non-zero buffer If the PackBuffer is used to encode an NSEC/3 record, the bitmap is xored with the content of the buffer instead of being zeroed first. The algorithm has been changed so it is able zero bytes without losing too much performance (around 2x slower). --- msg_helpers.go | 50 +++++++++++++++++++++------------- msg_helpers_test.go | 66 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 88 insertions(+), 28 deletions(-) diff --git a/msg_helpers.go b/msg_helpers.go index 10754c8b8..15271e800 100644 --- a/msg_helpers.go +++ b/msg_helpers.go @@ -558,29 +558,43 @@ func packDataNsec(bitmap []uint16, msg []byte, off int) (int, error) { if len(bitmap) == 0 { return off, nil } - var lastwindow, lastlength uint16 - for _, t := range bitmap { - window := t / 256 - length := (t-window*256)/8 + 1 - if window > lastwindow && lastlength != 0 { // New window, jump to the new offset - off += int(lastlength) + 2 - lastlength = 0 + + bi := 0 + for window := 0; window < 256; window++ { + if bi == len(bitmap) { + break } - if window < lastwindow || length < lastlength { - return len(msg), &Error{err: "nsec bits out of order"} + if int(bitmap[bi]>>8) != window { + continue } - if off+2+int(length) > len(msg) { - return len(msg), &Error{err: "overflow packing nsec"} + length := 0 + for i := 0; i < 32; i++ { + if bi == len(bitmap) { + break + } + _off := off + 2 + i + if int(bitmap[bi]>>8) == window && int(byte(bitmap[bi])/8) == i { + if _off >= len(msg) { + return off, &Error{err: "overflow packing nsec"} + } + msg[_off] = byte(0x80 >> (byte(bitmap[bi]) % 8)) + for j := length + 1; j < i; j++ { + msg[off+2+j] = 0 + } + length = i + bi++ + } + for ; bi < len(bitmap) && int(bitmap[bi]>>8) == window && int(byte(bitmap[bi])/8) == i; bi++ { + msg[_off] |= byte(0x80 >> (byte(bitmap[bi]) % 8)) + } } - // Setting the window # msg[off] = byte(window) - // Setting the octets length - msg[off+1] = byte(length) - // Setting the bit value for the type in the right octet - msg[off+1+int(length)] |= byte(1 << (7 - t%8)) - lastwindow, lastlength = window, length + msg[off+1] = byte(length + 1) + off += length + 3 + } + if bi != len(bitmap) { + return off, &Error{err: "nsec bits out of order"} } - off += int(lastlength) + 2 return off, nil } diff --git a/msg_helpers_test.go b/msg_helpers_test.go index bd41e245c..4a318c286 100644 --- a/msg_helpers_test.go +++ b/msg_helpers_test.go @@ -19,7 +19,8 @@ func TestPackDataNsec(t *testing.T) { tests := []struct { name string args args - want int + wantOff int + wantBytes []byte wantErr bool wantErrMsg string }{ @@ -44,14 +45,14 @@ func TestPackDataNsec(t *testing.T) { }, wantErr: true, wantErrMsg: "dns: overflow packing nsec", - want: 31, + wantOff: 48, }, { name: "disordered nsec bits", args: args{ bitmap: []uint16{ 8962, - 0, + 1, }, msg: []byte{ 48, 48, 48, 48, 0, 0, 0, 1, 0, 0, 0, 0, @@ -72,13 +73,13 @@ func TestPackDataNsec(t *testing.T) { }, wantErr: true, wantErrMsg: "dns: nsec bits out of order", - want: 155, + wantOff: 3, }, { name: "simple message with only one window", args: args{ bitmap: []uint16{ - 0, + 1, }, msg: []byte{ 48, 48, 48, 48, 0, 0, @@ -89,13 +90,33 @@ func TestPackDataNsec(t *testing.T) { }, off: 0, }, - wantErr: false, - want: 3, + wantErr: false, + wantOff: 3, + wantBytes: []byte{0, 1, 64}, + }, + { + name: "multiple types", + args: args{ + bitmap: []uint16{ + TypeNS, TypeSOA, TypeRRSIG, TypeDNSKEY, TypeNSEC3PARAM, + }, + msg: []byte{ + 48, 48, 48, 48, 0, 0, + 0, 1, 0, 0, 0, 0, + 0, 0, 50, 48, 48, 48, + 48, 48, 48, 0, 54, 48, + 48, 48, 48, 0, 19, 48, 48, + }, + off: 0, + }, + wantErr: false, + wantOff: 9, + wantBytes: []byte{0, 7, 34, 0, 0, 0, 0, 2, 144}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := packDataNsec(tt.args.bitmap, tt.args.msg, tt.args.off) + gotOff, err := packDataNsec(tt.args.bitmap, tt.args.msg, tt.args.off) if (err != nil) != tt.wantErr { t.Errorf("packDataNsec() error = %v, wantErr %v", err, tt.wantErr) return @@ -104,13 +125,38 @@ func TestPackDataNsec(t *testing.T) { t.Errorf("packDataNsec() error msg = %v, wantErrMsg %v", err.Error(), tt.wantErrMsg) return } - if got != tt.want { - t.Errorf("packDataNsec() = %v, want %v", got, tt.want) + if gotOff != tt.wantOff { + t.Errorf("packDataNsec() = %v, want off %v", gotOff, tt.wantOff) + } + if err == nil && tt.args.off < len(tt.args.msg) && gotOff < len(tt.args.msg) { + if want, got := tt.wantBytes, tt.args.msg[tt.args.off:gotOff]; !bytes.Equal(got, want) { + t.Errorf("packDataNsec() = %v, want bytes %v", got, want) + } } }) } } +func TestPackDataNsecDirtyBuffer(t *testing.T) { + zeroBuf := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0} + dirtyBuf := []byte{1, 2, 3, 4, 5, 6, 7, 8, 9} + off1, _ := packDataNsec([]uint16{TypeNS, TypeSOA, TypeRRSIG}, zeroBuf, 0) + off2, _ := packDataNsec([]uint16{TypeNS, TypeSOA, TypeRRSIG}, dirtyBuf, 0) + if off1 != off2 { + t.Errorf("off1 %v != off2 %v", off1, off2) + } + if !bytes.Equal(zeroBuf[:off1], dirtyBuf[:off2]) { + t.Errorf("dirty buffer differs from zero buffer: %v, %v", zeroBuf[:off1], dirtyBuf[:off2]) + } +} + +func BenchmarkPackDataNsec(b *testing.B) { + types := []uint16{TypeNS, TypeSOA, TypeRRSIG, TypeDNSKEY, TypeNSEC3PARAM} + buf := make([]byte, 100) + for n := 0; n < b.N; n++ { + packDataNsec(types, buf, 0) + } +} func TestUnpackString(t *testing.T) { msg := []byte("\x00abcdef\x0f\\\"ghi\x04mmm\x7f") msg[0] = byte(len(msg) - 1) From 09a259662fb8a28227ba2df70b8ad2ffb1168cf1 Mon Sep 17 00:00:00 2001 From: Olivier Poitrey Date: Mon, 7 Feb 2022 16:32:58 +0100 Subject: [PATCH 2/4] Add some comments + rename some vars to make algo clearer --- msg_helpers.go | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/msg_helpers.go b/msg_helpers.go index 15271e800..d7d4affec 100644 --- a/msg_helpers.go +++ b/msg_helpers.go @@ -554,45 +554,52 @@ func typeBitMapLen(bitmap []uint16) int { return l } -func packDataNsec(bitmap []uint16, msg []byte, off int) (int, error) { - if len(bitmap) == 0 { +func packDataNsec(types []uint16, msg []byte, off int) (int, error) { + if len(types) == 0 { return off, nil } - bi := 0 + ti := 0 // current type index for window := 0; window < 256; window++ { - if bi == len(bitmap) { + if ti == len(types) { + // All types are encoded break } - if int(bitmap[bi]>>8) != window { + if int(types[ti]>>8) != window { + // Next types high-order 0 bit does not fit in this window, skipping. continue } length := 0 for i := 0; i < 32; i++ { - if bi == len(bitmap) { + if ti == len(types) { + // No more types to encode break } - _off := off + 2 + i - if int(bitmap[bi]>>8) == window && int(byte(bitmap[bi])/8) == i { - if _off >= len(msg) { + bo := off + 2 + i // current bitmap offset + if int(types[ti]>>8) == window && int(byte(types[ti])/8) == i { + if bo >= len(msg) { return off, &Error{err: "overflow packing nsec"} } - msg[_off] = byte(0x80 >> (byte(bitmap[bi]) % 8)) + // Set current bitmap + msg[bo] = byte(0x80 >> (byte(types[ti]) % 8)) + // Zero previous empty bitmap blocks if any for j := length + 1; j < i; j++ { msg[off+2+j] = 0 } + // Push length of the bitmap to the highest lower-order 8bit block length = i - bi++ + ti++ } - for ; bi < len(bitmap) && int(bitmap[bi]>>8) == window && int(byte(bitmap[bi])/8) == i; bi++ { - msg[_off] |= byte(0x80 >> (byte(bitmap[bi]) % 8)) + for ; ti < len(types) && int(types[ti]>>8) == window && int(byte(types[ti])/8) == i; ti++ { + // Add additional types fitting in the same bitmap block if any + msg[bo] |= byte(0x80 >> (byte(types[ti]) % 8)) } } msg[off] = byte(window) msg[off+1] = byte(length + 1) off += length + 3 } - if bi != len(bitmap) { + if ti != len(types) { return off, &Error{err: "nsec bits out of order"} } return off, nil From e6e7d50239d92892e825acbd1f2d9da277d31eb6 Mon Sep 17 00:00:00 2001 From: Olivier Poitrey Date: Fri, 11 Feb 2022 21:21:52 +0100 Subject: [PATCH 3/4] Revert to previous algo with window length compute+0 on new window --- msg_helpers.go | 75 ++++++++++++++++++++++----------------------- msg_helpers_test.go | 23 ++++++++++---- 2 files changed, 53 insertions(+), 45 deletions(-) diff --git a/msg_helpers.go b/msg_helpers.go index d7d4affec..612c52f5e 100644 --- a/msg_helpers.go +++ b/msg_helpers.go @@ -554,54 +554,51 @@ func typeBitMapLen(bitmap []uint16) int { return l } -func packDataNsec(types []uint16, msg []byte, off int) (int, error) { - if len(types) == 0 { +func packDataNsec(bitmap []uint16, msg []byte, off int) (int, error) { + if len(bitmap) == 0 { return off, nil } - - ti := 0 // current type index - for window := 0; window < 256; window++ { - if ti == len(types) { - // All types are encoded - break - } - if int(types[ti]>>8) != window { - // Next types high-order 0 bit does not fit in this window, skipping. - continue - } - length := 0 - for i := 0; i < 32; i++ { - if ti == len(types) { - // No more types to encode - break + var lastwindow, lastlength uint16 + for i, t := range bitmap { + window := t / 256 + length := (t-window*256)/8 + 1 + if (window > lastwindow && lastlength != 0) || i == 0 { + if i > 0 { + // New window, jump to the new offset + off += int(lastlength) + 2 + lastlength = 0 } - bo := off + 2 + i // current bitmap offset - if int(types[ti]>>8) == window && int(byte(types[ti])/8) == i { - if bo >= len(msg) { - return off, &Error{err: "overflow packing nsec"} - } - // Set current bitmap - msg[bo] = byte(0x80 >> (byte(types[ti]) % 8)) - // Zero previous empty bitmap blocks if any - for j := length + 1; j < i; j++ { - msg[off+2+j] = 0 + + // Zero out window bitmap + windowLength := length + for j := i + 1; j < len(bitmap); j++ { + if bitmap[j]>>8 == window { + windowLength = bitmap[j]&0xff/8 + 1 } - // Push length of the bitmap to the highest lower-order 8bit block - length = i - ti++ } - for ; ti < len(types) && int(types[ti]>>8) == window && int(byte(types[ti])/8) == i; ti++ { - // Add additional types fitting in the same bitmap block if any - msg[bo] |= byte(0x80 >> (byte(types[ti]) % 8)) + lastOff := off + 1 + int(windowLength) + if lastOff > len(msg) { + return len(msg), &Error{err: "overflow packing nsec"} + } + for zoff := off + 1; zoff <= lastOff; zoff++ { + msg[zoff] = 0 } } + if window < lastwindow || length < lastlength { + return len(msg), &Error{err: "nsec bits out of order"} + } + if off+2+int(length) > len(msg) { + return len(msg), &Error{err: "overflow packing nsec"} + } + // Setting the window # msg[off] = byte(window) - msg[off+1] = byte(length + 1) - off += length + 3 - } - if ti != len(types) { - return off, &Error{err: "nsec bits out of order"} + // Setting the octets length + msg[off+1] = byte(length) + // Setting the bit value for the type in the right octet + msg[off+1+int(length)] |= byte(1 << (7 - t%8)) + lastwindow, lastlength = window, length } + off += int(lastlength) + 2 return off, nil } diff --git a/msg_helpers_test.go b/msg_helpers_test.go index 4a318c286..ac604646c 100644 --- a/msg_helpers_test.go +++ b/msg_helpers_test.go @@ -45,7 +45,7 @@ func TestPackDataNsec(t *testing.T) { }, wantErr: true, wantErrMsg: "dns: overflow packing nsec", - wantOff: 48, + wantOff: 31, }, { name: "disordered nsec bits", @@ -73,7 +73,7 @@ func TestPackDataNsec(t *testing.T) { }, wantErr: true, wantErrMsg: "dns: nsec bits out of order", - wantOff: 3, + wantOff: 155, }, { name: "simple message with only one window", @@ -151,10 +151,21 @@ func TestPackDataNsecDirtyBuffer(t *testing.T) { } func BenchmarkPackDataNsec(b *testing.B) { - types := []uint16{TypeNS, TypeSOA, TypeRRSIG, TypeDNSKEY, TypeNSEC3PARAM} - buf := make([]byte, 100) - for n := 0; n < b.N; n++ { - packDataNsec(types, buf, 0) + benches := []struct { + name string + types []uint16 + }{ + {"empty", nil}, + {"typical", []uint16{TypeNS, TypeSOA, TypeRRSIG, TypeDNSKEY, TypeNSEC3PARAM}}, + {"multiple_windows", []uint16{1, 300, 350, 10000, 20000}}, + } + for _, bb := range benches { + b.Run(bb.name, func(b *testing.B) { + buf := make([]byte, 100) + for n := 0; n < b.N; n++ { + packDataNsec(bb.types, buf, 0) + } + }) } } func TestUnpackString(t *testing.T) { From afe53a6e92816c4f208d3fdd242afd0c431cdcb9 Mon Sep 17 00:00:00 2001 From: Olivier Poitrey Date: Sat, 12 Mar 2022 08:48:47 -0800 Subject: [PATCH 4/4] Use typeBitMapLen to compute the bitmap length to zero --- msg_helpers.go | 36 ++++++++++++++---------------------- msg_helpers_test.go | 2 +- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/msg_helpers.go b/msg_helpers.go index 612c52f5e..b049028b7 100644 --- a/msg_helpers.go +++ b/msg_helpers.go @@ -558,31 +558,23 @@ func packDataNsec(bitmap []uint16, msg []byte, off int) (int, error) { if len(bitmap) == 0 { return off, nil } + if off > len(msg) { + return off, &Error{err: "overflow packing nsec"} + } + toZero := msg[off:] + if maxLen := typeBitMapLen(bitmap); maxLen < len(toZero) { + toZero = toZero[:maxLen] + } + for i := range toZero { + toZero[i] = 0 + } var lastwindow, lastlength uint16 - for i, t := range bitmap { + for _, t := range bitmap { window := t / 256 length := (t-window*256)/8 + 1 - if (window > lastwindow && lastlength != 0) || i == 0 { - if i > 0 { - // New window, jump to the new offset - off += int(lastlength) + 2 - lastlength = 0 - } - - // Zero out window bitmap - windowLength := length - for j := i + 1; j < len(bitmap); j++ { - if bitmap[j]>>8 == window { - windowLength = bitmap[j]&0xff/8 + 1 - } - } - lastOff := off + 1 + int(windowLength) - if lastOff > len(msg) { - return len(msg), &Error{err: "overflow packing nsec"} - } - for zoff := off + 1; zoff <= lastOff; zoff++ { - msg[zoff] = 0 - } + if window > lastwindow && lastlength != 0 { // New window, jump to the new offset + off += int(lastlength) + 2 + lastlength = 0 } if window < lastwindow || length < lastlength { return len(msg), &Error{err: "nsec bits out of order"} diff --git a/msg_helpers_test.go b/msg_helpers_test.go index ac604646c..1f17c57ce 100644 --- a/msg_helpers_test.go +++ b/msg_helpers_test.go @@ -45,7 +45,7 @@ func TestPackDataNsec(t *testing.T) { }, wantErr: true, wantErrMsg: "dns: overflow packing nsec", - wantOff: 31, + wantOff: 48, }, { name: "disordered nsec bits",