prevent sending of unencrypted stream data on data streams

fixes #446

packet_packer.go

@@ -8,6 +8,7 @@ import (
 	"github.com/lucas-clemente/quic-go/frames"
 	"github.com/lucas-clemente/quic-go/handshake"
 	"github.com/lucas-clemente/quic-go/protocol"
+	"github.com/lucas-clemente/quic-go/qerr"
 )
 
 type packedPacket struct {
@@ -127,7 +128,11 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, lea
 
 	payloadStartIndex := buffer.Len()
 
+	var hasNonCryptoStreamData bool // does this frame contain any stream frame on a stream > 1
 	for _, frame := range payloadFrames {
+		if sf, ok := frame.(*frames.StreamFrame); ok && sf.StreamID != 1 {
+			hasNonCryptoStreamData = true
+		}
 		err := frame.Write(buffer, p.version)
 		if err != nil {
 			return nil, err
@@ -142,6 +147,10 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, lea
 	_, encryptionLevel := p.cryptoSetup.Seal(raw[payloadStartIndex:payloadStartIndex], raw[payloadStartIndex:], currentPacketNumber, raw[:payloadStartIndex])
 	raw = raw[0 : buffer.Len()+12]
 
+	if hasNonCryptoStreamData && encryptionLevel <= protocol.EncryptionUnencrypted {
+		return nil, qerr.AttemptToSendUnencryptedStreamData
+	}
+
 	num := p.packetNumberGenerator.Pop()
 	if num != currentPacketNumber {
 		return nil, errors.New("PacketPacker BUG: Peeked and Popped packet numbers do not match.")

packet_packer_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/lucas-clemente/quic-go/frames"
 	"github.com/lucas-clemente/quic-go/protocol"
+	"github.com/lucas-clemente/quic-go/qerr"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -46,7 +47,7 @@ var _ = Describe("Packet packer", func() {
 		streamFramer = newStreamFramer(newStreamsMap(nil, protocol.PerspectiveServer, cpm), fcm)
 
 		packer = &packetPacker{
-			cryptoSetup:           &mockCryptoSetup{},
+			cryptoSetup:           &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure},
 			connectionParameters:  cpm,
 			packetNumberGenerator: newPacketNumberGenerator(protocol.SkipPacketAveragePeriodLength),
 			streamFramer:          streamFramer,
@@ -429,6 +430,56 @@ var _ = Describe("Packet packer", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(payloadFrames).To(HaveLen(1))
 		})
+
+		It("refuses to send unencrypted stream data on a data stream", func() {
+			packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionUnencrypted
+			f := &frames.StreamFrame{
+				StreamID: 3,
+				Data:     []byte("foobar"),
+			}
+			streamFramer.AddFrameForRetransmission(f)
+			_, err := packer.PackPacket(nil, nil, 0)
+			Expect(err).To(MatchError(qerr.AttemptToSendUnencryptedStreamData))
+		})
+
+		It("sends encrypted, non forward-secure, stream data on a data stream", func() {
+			packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionSecure
+			f := &frames.StreamFrame{
+				StreamID: 5,
+				Data:     []byte("foobar"),
+			}
+			streamFramer.AddFrameForRetransmission(f)
+			p, err := packer.PackPacket(nil, nil, 0)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(p.encryptionLevel).To(Equal(protocol.EncryptionSecure))
+			Expect(p.frames[0]).To(Equal(f))
+		})
+
+		It("sends unencrypted stream data on the crypto stream", func() {
+			packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionUnencrypted
+			f := &frames.StreamFrame{
+				StreamID: 1,
+				Data:     []byte("foobar"),
+			}
+			streamFramer.AddFrameForRetransmission(f)
+			p, err := packer.PackPacket(nil, nil, 0)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(p.encryptionLevel).To(Equal(protocol.EncryptionUnencrypted))
+			Expect(p.frames[0]).To(Equal(f))
+		})
+
+		It("sends encrypted stream data on the crypto stream", func() {
+			packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionSecure
+			f := &frames.StreamFrame{
+				StreamID: 1,
+				Data:     []byte("foobar"),
+			}
+			streamFramer.AddFrameForRetransmission(f)
+			p, err := packer.PackPacket(nil, nil, 0)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(p.encryptionLevel).To(Equal(protocol.EncryptionSecure))
+			Expect(p.frames[0]).To(Equal(f))
+		})
 	})
 
 	Context("Blocked frames", func() {

session_test.go

@@ -803,6 +803,10 @@ var _ = Describe("Session", func() {
 	})
 
 	Context("retransmissions", func() {
+		BeforeEach(func() {
+			sess.packer.cryptoSetup = &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure}
+		})
+
 		It("sends a StreamFrame from a packet queued for retransmission", func() {
 			// a StopWaitingFrame is added, so make sure the packet number of the new package is higher than the packet number of the retransmitted packet
 			sess.packer.packetNumberGenerator.next = 0x1337 + 9
@@ -954,9 +958,41 @@ var _ = Describe("Session", func() {
 			sentPackets := sph.(*mockSentPacketHandler).sentPackets
 			Expect(sentPackets).To(BeEmpty())
 		})
+
+		It("retransmits RTO packets", func() {
+			// We simulate consistently low RTTs, so that the test works faster
+			n := protocol.PacketNumber(10)
+			for p := protocol.PacketNumber(1); p < n; p++ {
+				err := sess.sentPacketHandler.SentPacket(&ackhandler.Packet{PacketNumber: p, Length: 1})
+				Expect(err).NotTo(HaveOccurred())
+				time.Sleep(time.Microsecond)
+				ack := &frames.AckFrame{}
+				ack.LargestAcked = p
+				err = sess.sentPacketHandler.ReceivedAck(ack, p, time.Now())
+				Expect(err).NotTo(HaveOccurred())
+			}
+			sess.packer.packetNumberGenerator.next = n + 1
+			// Now, we send a single packet, and expect that it was retransmitted later
+			err := sess.sentPacketHandler.SentPacket(&ackhandler.Packet{
+				PacketNumber: n,
+				Length:       1,
+				Frames: []frames.Frame{&frames.StreamFrame{
+					Data: []byte("foobar"),
+				}},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			go sess.run()
+			sess.scheduleSending()
+			Eventually(func() [][]byte { return mconn.written }).ShouldNot(BeEmpty())
+			Expect(mconn.written[0]).To(ContainSubstring("foobar"))
+		})
 	})
 
 	Context("scheduling sending", func() {
+		BeforeEach(func() {
+			sess.packer.cryptoSetup = &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure}
+		})
+
 		It("sends after writing to a stream", func(done Done) {
 			Expect(sess.sendingScheduled).NotTo(Receive())
 			s, err := sess.GetOrOpenStream(3)
@@ -1199,34 +1235,6 @@ var _ = Describe("Session", func() {
 		close(done)
 	}, 0.5)
 
-	It("retransmits RTO packets", func() {
-		// We simulate consistently low RTTs, so that the test works faster
-		n := protocol.PacketNumber(10)
-		for p := protocol.PacketNumber(1); p < n; p++ {
-			err := sess.sentPacketHandler.SentPacket(&ackhandler.Packet{PacketNumber: p, Length: 1})
-			Expect(err).NotTo(HaveOccurred())
-			time.Sleep(time.Microsecond)
-			ack := &frames.AckFrame{}
-			ack.LargestAcked = p
-			err = sess.sentPacketHandler.ReceivedAck(ack, p, time.Now())
-			Expect(err).NotTo(HaveOccurred())
-		}
-		sess.packer.packetNumberGenerator.next = n + 1
-		// Now, we send a single packet, and expect that it was retransmitted later
-		err := sess.sentPacketHandler.SentPacket(&ackhandler.Packet{
-			PacketNumber: n,
-			Length:       1,
-			Frames: []frames.Frame{&frames.StreamFrame{
-				Data: []byte("foobar"),
-			}},
-		})
-		Expect(err).NotTo(HaveOccurred())
-		go sess.run()
-		sess.scheduleSending()
-		Eventually(func() [][]byte { return mconn.written }).ShouldNot(BeEmpty())
-		Expect(mconn.written[0]).To(ContainSubstring("foobar"))
-	})
-
 	Context("getting streams", func() {
 		It("returns a new stream", func() {
 			str, err := sess.GetOrOpenStream(11)