From c01e16da713f2b0d629e58172404f2df2fc26939 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 21 Jun 2016 19:47:05 +0700 Subject: [PATCH] validate parsed ACK ranges from a QUIC 34 ACK frame ref #182 --- frames/ack_frame_new.go | 42 ++++++++++++ frames/ack_frame_new_test.go | 122 +++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) diff --git a/frames/ack_frame_new.go b/frames/ack_frame_new.go index 8db486d19..bc80288b0 100644 --- a/frames/ack_frame_new.go +++ b/frames/ack_frame_new.go @@ -122,6 +122,10 @@ func ParseAckFrameNew(r *bytes.Reader, version protocol.VersionNumber) (*AckFram } } + if !frame.validateAckRanges() { + return nil, errInvalidAckRanges + } + var numTimestampByte byte numTimestampByte, err = r.ReadByte() if err != nil { @@ -297,6 +301,44 @@ func (f *AckFrameNew) GetHighestInOrderPacketNumber() protocol.PacketNumber { return f.LargestObserved } +func (f *AckFrameNew) validateAckRanges() bool { + if len(f.AckRanges) == 0 { + return true + } + + // if there are missing packets, there will always be at least 2 ACK ranges + if len(f.AckRanges) == 1 { + return false + } + + if f.AckRanges[0].LastPacketNumber != f.LargestObserved { + return false + } + + // check the validity of every single ACK range + for _, ackRange := range f.AckRanges { + if ackRange.FirstPacketNumber > ackRange.LastPacketNumber { + return false + } + } + + // check the consistency for ACK with multiple NACK ranges + for i, ackRange := range f.AckRanges { + if i == 0 { + continue + } + lastAckRange := f.AckRanges[i-1] + if lastAckRange.FirstPacketNumber <= ackRange.FirstPacketNumber { + return false + } + if lastAckRange.FirstPacketNumber <= ackRange.LastPacketNumber+1 { + return false + } + } + + return true +} + // numWrittenNackRanges calculates the number of ACK blocks that are about to be written // this number is different from len(f.AckRanges) for the case of long gaps (> 255 packets) func (f *AckFrameNew) numWrittenNackRanges() uint64 { diff --git a/frames/ack_frame_new_test.go b/frames/ack_frame_new_test.go index 0984c567d..83279d9eb 100644 --- a/frames/ack_frame_new_test.go +++ b/frames/ack_frame_new_test.go @@ -67,6 +67,13 @@ var _ = Describe("AckFrame", func() { Expect(b.Len()).To(BeZero()) }) + It("rejects a frame with invalid ACK ranges", func() { + // like the test before, but increased the last ACK range, such that the FirstPacketNumber would be negative + b := bytes.NewReader([]byte{0x60, 0x18, 0x94, 0x1, 0x1, 0x3, 0x2, 0x15, 0x2, 0x1, 0x5c, 0xd5, 0x0, 0x0, 0x0, 0x95, 0x0}) + _, err := ParseAckFrameNew(b, 0) + Expect(err).To(MatchError(errInvalidAckRanges)) + }) + It("parses a frame with multiple single packets missing", func() { b := bytes.NewReader([]byte{0x60, 0x27, 0xda, 0x0, 0x6, 0x9, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x13, 0x2, 0x1, 0x71, 0x12, 0x3, 0x0, 0x0, 0x47, 0x2}) frame, err := ParseAckFrameNew(b, 0) @@ -393,6 +400,121 @@ var _ = Describe("AckFrame", func() { }) }) + Context("ACK range validator", func() { + It("accepts an ACK without NACK Ranges", func() { + ack := AckFrameNew{LargestObserved: 7} + Expect(ack.validateAckRanges()).To(BeTrue()) + }) + + It("rejects ACK ranges with a single range", func() { + ack := AckFrameNew{ + LargestObserved: 10, + AckRanges: []AckRange{AckRange{FirstPacketNumber: 1, LastPacketNumber: 10}}, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("rejects ACK ranges with LastPacketNumber of the first range unequal to LargestObserved", func() { + ack := AckFrameNew{ + LargestObserved: 10, + AckRanges: []AckRange{ + AckRange{FirstPacketNumber: 8, LastPacketNumber: 9}, + AckRange{FirstPacketNumber: 2, LastPacketNumber: 3}, + }, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("rejects ACK ranges with FirstPacketNumber greater than LastPacketNumber", func() { + ack := AckFrameNew{ + LargestObserved: 10, + AckRanges: []AckRange{ + AckRange{FirstPacketNumber: 8, LastPacketNumber: 10}, + AckRange{FirstPacketNumber: 4, LastPacketNumber: 3}, + }, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("rejects ACK ranges with FirstPacketNumber greater than LargestObserved", func() { + ack := AckFrameNew{ + LargestObserved: 5, + AckRanges: []AckRange{ + AckRange{FirstPacketNumber: 4, LastPacketNumber: 10}, + AckRange{FirstPacketNumber: 1, LastPacketNumber: 2}, + }, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("rejects ACK ranges in the wrong order", func() { + ack := AckFrameNew{ + LargestObserved: 7, + AckRanges: []AckRange{ + {FirstPacketNumber: 2, LastPacketNumber: 2}, + {FirstPacketNumber: 6, LastPacketNumber: 7}, + }, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("rejects with overlapping ACK ranges", func() { + ack := AckFrameNew{ + LargestObserved: 7, + AckRanges: []AckRange{ + {FirstPacketNumber: 5, LastPacketNumber: 7}, + {FirstPacketNumber: 2, LastPacketNumber: 5}, + }, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("rejects ACK ranges that are part of a larger ACK range", func() { + ack := AckFrameNew{ + LargestObserved: 7, + AckRanges: []AckRange{ + {FirstPacketNumber: 4, LastPacketNumber: 7}, + {FirstPacketNumber: 5, LastPacketNumber: 6}, + }, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("rejects with directly adjacent ACK ranges", func() { + ack := AckFrameNew{ + LargestObserved: 7, + AckRanges: []AckRange{ + {FirstPacketNumber: 5, LastPacketNumber: 7}, + {FirstPacketNumber: 2, LastPacketNumber: 4}, + }, + } + Expect(ack.validateAckRanges()).To(BeFalse()) + }) + + It("accepts an ACK with one lost packet", func() { + ack := AckFrameNew{ + LargestObserved: 10, + AckRanges: []AckRange{ + {FirstPacketNumber: 5, LastPacketNumber: 10}, + {FirstPacketNumber: 1, LastPacketNumber: 3}, + }, + } + Expect(ack.validateAckRanges()).To(BeTrue()) + }) + + It("accepts an ACK with multiple lost packets", func() { + ack := AckFrameNew{ + LargestObserved: 20, + AckRanges: []AckRange{ + {FirstPacketNumber: 15, LastPacketNumber: 20}, + {FirstPacketNumber: 10, LastPacketNumber: 12}, + {FirstPacketNumber: 1, LastPacketNumber: 3}, + }, + } + Expect(ack.validateAckRanges()).To(BeTrue()) + }) + }) + Context("highest in order packet number", func() { It("gets the hightest in order packet number for a simple ACK", func() { frame := &AckFrameNew{