From 20101c63d8cb9d5d673bbd2b93f9dfd8344f06a2 Mon Sep 17 00:00:00 2001 From: Shulhan Date: Fri, 17 Jan 2025 00:09:21 +0700 Subject: lib/dns: detect invalid header earlier Previously, we unpack the header and then question without detecting whether the header itself is valid or not, for example the op-code, the response code. This cause the unpacking question return an error like label length overflow at index xxx One of the case is when someone sent random or HTTP request to DoT port. --- lib/dns/message.go | 12 ++++---- lib/dns/message_header.go | 21 +++++++++++-- lib/dns/message_header_test.go | 5 ++- lib/dns/message_test.go | 36 ++++++++++++++++++++++ .../testdata/message/UnpackHeaderQuestion_test.txt | 16 ++++++++++ 5 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 lib/dns/testdata/message/UnpackHeaderQuestion_test.txt diff --git a/lib/dns/message.go b/lib/dns/message.go index 02a237c0..058a159e 100644 --- a/lib/dns/message.go +++ b/lib/dns/message.go @@ -5,7 +5,6 @@ package dns import ( - "errors" "fmt" "net" "strconv" @@ -1072,15 +1071,16 @@ func (msg *Message) String() string { // packet. This method assume that message.packet already set to DNS raw // message. func (msg *Message) UnpackHeaderQuestion() (err error) { - if len(msg.packet) <= sectionHeaderSize { - return errors.New(`UnpackHeaderQuestion: missing question`) - } + var logp = `UnpackHeaderQuestion` - msg.Header.unpack(msg.packet) + err = msg.Header.unpack(msg.packet) + if err != nil { + return fmt.Errorf(`%s: %w`, logp, err) + } err = msg.Question.unpack(msg.packet[sectionHeaderSize:]) if err != nil { - return err + return fmt.Errorf(`%s: %w`, logp, err) } return nil diff --git a/lib/dns/message_header.go b/lib/dns/message_header.go index c3c6f9cb..aa505aec 100644 --- a/lib/dns/message_header.go +++ b/lib/dns/message_header.go @@ -5,6 +5,9 @@ package dns import ( + "errors" + "fmt" + libbytes "git.sr.ht/~shulhan/pakakeh.go/lib/bytes" ) @@ -161,19 +164,31 @@ func (hdr *MessageHeader) pack() []byte { } // unpack the DNS header section. -func (hdr *MessageHeader) unpack(packet []byte) { +func (hdr *MessageHeader) unpack(packet []byte) (err error) { + if len(packet) < sectionHeaderSize { + return errors.New(`header too small`) + } + hdr.Op = OpCode((packet[2] & headerMaskOpCode) >> 3) + if hdr.Op < 0 || hdr.Op > OpCodeStatus { + return fmt.Errorf(`unknown op code=%d`, hdr.Op) + } + hdr.RCode = ResponseCode(headerMaskRCode & packet[3]) + if hdr.RCode < 0 || hdr.RCode > RCodeRefused { + return fmt.Errorf(`unknown response code=%d`, hdr.RCode) + } + hdr.ID = libbytes.ReadUint16(packet, 0) hdr.IsQuery = packet[2]&headerIsResponse != headerIsResponse - hdr.Op = OpCode((packet[2] & headerMaskOpCode) >> 3) hdr.IsAA = packet[2]&headerIsAA == headerIsAA hdr.IsTC = packet[2]&headerIsTC == headerIsTC hdr.IsRD = packet[2]&headerIsRD == headerIsRD hdr.IsRA = packet[3]&headerIsRA == headerIsRA - hdr.RCode = ResponseCode(headerMaskRCode & packet[3]) hdr.QDCount = libbytes.ReadUint16(packet, 4) hdr.ANCount = libbytes.ReadUint16(packet, 6) hdr.NSCount = libbytes.ReadUint16(packet, 8) hdr.ARCount = libbytes.ReadUint16(packet, 10) + + return nil } diff --git a/lib/dns/message_header_test.go b/lib/dns/message_header_test.go index cb30ece1..95fbe113 100644 --- a/lib/dns/message_header_test.go +++ b/lib/dns/message_header_test.go @@ -262,7 +262,10 @@ func TestMessageHeader_unpack(t *testing.T) { for _, c = range cases { got = MessageHeader{} - got.unpack(c.packet) + var err = got.unpack(c.packet) + if err != nil { + t.Fatal(err) + } test.Assert(t, c.desc, c.hdr, got) } } diff --git a/lib/dns/message_test.go b/lib/dns/message_test.go index 8a1d47b2..a2f07b8b 100644 --- a/lib/dns/message_test.go +++ b/lib/dns/message_test.go @@ -1210,6 +1210,42 @@ func TestMessageSetResponseCode(t *testing.T) { } } +func TestMessage_UnpackHeaderQuestion(t *testing.T) { + var tdata *test.Data + var err error + tdata, err = test.LoadData(`testdata/message/UnpackHeaderQuestion_test.txt`) + if err != nil { + t.Fatal(err) + } + + var name string + var rawb []byte + var msg Message + for name, rawb = range tdata.Input { + t.Run(name, func(t *testing.T) { + msg.packet, err = hexdump.Parse(rawb, true) + if err != nil { + t.Fatal(err) + } + + err = msg.UnpackHeaderQuestion() + if err != nil { + var expError = tdata.Output[name+`Error`] + test.Assert(t, `error`, string(expError), + err.Error()) + return + } + + rawb, err = json.MarshalIndent(&msg, ``, ` `) + if err != nil { + t.Fatal(err) + } + test.Assert(t, `message`, string(tdata.Output[name]), + string(rawb)) + }) + } +} + func TestUnpackMessage(t *testing.T) { type testCase struct { exp *Message diff --git a/lib/dns/testdata/message/UnpackHeaderQuestion_test.txt b/lib/dns/testdata/message/UnpackHeaderQuestion_test.txt new file mode 100644 index 00000000..01dae739 --- /dev/null +++ b/lib/dns/testdata/message/UnpackHeaderQuestion_test.txt @@ -0,0 +1,16 @@ +Various invalid messages that we receive on our server. + +>>> invalidHeader001 +0000000 5420 2f20 4854 5450 2f31 2e31 0d0a 486f +0000010 7374 3a20 3139 342e 3233 332e 3638 2e31 +0000020 3834 3a35 330d 0a55 7365 722d 4167 656e +0000030 743a 204d 6f7a 696c 6c61 2f35 2e30 2028 +0000040 636f 6d70 6174 6962 6c65 3b20 4d6f 6461 +0000050 7453 6361 6e6e 6572 2f31 2e30 3b20 2b68 +0000060 7474 7073 3a2f 2f6d 6f64 6174 2e69 6f2f +0000070 290d 0a41 6363 6570 743a 202a 2f2a 0d0a +0000080 4163 6365 7074 2d45 6e63 6f64 696e 673a +0000090 2067 7a69 700d 0a0d 0a + +<<< invalidHeader001Error +UnpackHeaderQuestion: unknown op code=5 -- cgit v1.3