From e43b91f633a2112a9b8227bac2661b5dc80f87d6 Mon Sep 17 00:00:00 2001 From: Lucas Clemente Date: Fri, 9 Jun 2017 16:06:18 +0200 Subject: [PATCH] Fix encryption of stream data This commit splits up handling of the crypto stream and the other streams in the framer, crypto setup, and the packer. - Crypto stream data is handled separately and should never be sent unencrypted or FW-secure. Fixes #544. - Non-crypto stream data is only sent with FW encryption on the server and only with non-FW or FW encryption on the client. Fixes #611. The crypto stream is current excluded from flow control (#657), but that shouldn't be an issue in practice for now. --- handshake/crypto_setup_client.go | 5 +- handshake/crypto_setup_client_test.go | 23 +++++ handshake/crypto_setup_server.go | 15 +-- handshake/crypto_setup_server_test.go | 28 ++--- handshake/interface.go | 1 + packet_packer.go | 57 ++++++----- packet_packer_test.go | 141 +++++++++++++------------- session.go | 6 +- session_test.go | 18 +--- stream_framer.go | 24 ++++- 10 files changed, 183 insertions(+), 135 deletions(-) diff --git a/handshake/crypto_setup_client.go b/handshake/crypto_setup_client.go index 76ae41f7..0b85a8c2 100644 --- a/handshake/crypto_setup_client.go +++ b/handshake/crypto_setup_client.go @@ -332,7 +332,6 @@ func (h *cryptoSetupClient) Open(dst, src []byte, packetNumber protocol.PacketNu func (h *cryptoSetupClient) GetSealer() (protocol.EncryptionLevel, Sealer) { h.mutex.RLock() defer h.mutex.RUnlock() - if h.forwardSecureAEAD != nil { return protocol.EncryptionForwardSecure, h.sealForwardSecure } else if h.secureAEAD != nil { @@ -342,6 +341,10 @@ func (h *cryptoSetupClient) GetSealer() (protocol.EncryptionLevel, Sealer) { } } +func (h *cryptoSetupClient) GetSealerForCryptoStream() (protocol.EncryptionLevel, Sealer) { + return protocol.EncryptionUnencrypted, h.sealUnencrypted +} + func (h *cryptoSetupClient) GetSealerWithEncryptionLevel(encLevel protocol.EncryptionLevel) (Sealer, error) { h.mutex.RLock() defer h.mutex.RUnlock() diff --git a/handshake/crypto_setup_client_test.go b/handshake/crypto_setup_client_test.go index 706ddfc0..b2dd51bf 100644 --- a/handshake/crypto_setup_client_test.go +++ b/handshake/crypto_setup_client_test.go @@ -686,6 +686,13 @@ var _ = Describe("Client Crypto Setup", func() { Expect(d).To(Equal(foobarFNVSigned)) }) + It("is used for the crypto stream", func() { + enc, seal := cs.GetSealerForCryptoStream() + Expect(enc).To(Equal(protocol.EncryptionUnencrypted)) + d := seal(nil, []byte("foobar"), 0, []byte{}) + Expect(d).To(Equal(foobarFNVSigned)) + }) + It("is accepted initially", func() { d, enc, err := cs.Open(nil, foobarFNVSigned, 0, []byte{}) Expect(err).ToNot(HaveOccurred()) @@ -744,6 +751,14 @@ var _ = Describe("Client Crypto Setup", func() { Expect(err).To(MatchError("authentication failed")) Expect(enc).To(Equal(protocol.EncryptionUnspecified)) }) + + It("is not used for the crypto stream", func() { + doCompleteREJ() + enc, seal := cs.GetSealerForCryptoStream() + Expect(enc).To(Equal(protocol.EncryptionUnencrypted)) + d := seal(nil, []byte("foobar"), 0, []byte{}) + Expect(d).To(Equal(foobarFNVSigned)) + }) }) Context("forward-secure encryption", func() { @@ -757,6 +772,14 @@ var _ = Describe("Client Crypto Setup", func() { d := seal(nil, []byte("foobar"), 0, []byte{}) Expect(d).To(Equal([]byte("foobar forward sec"))) }) + + It("is not used for the crypto stream", func() { + doSHLO() + enc, seal := cs.GetSealerForCryptoStream() + Expect(enc).To(Equal(protocol.EncryptionUnencrypted)) + d := seal(nil, []byte("foobar"), 0, []byte{}) + Expect(d).To(Equal(foobarFNVSigned)) + }) }) Context("forcing encryption levels", func() { diff --git a/handshake/crypto_setup_server.go b/handshake/crypto_setup_server.go index 68d50a16..5814d86d 100644 --- a/handshake/crypto_setup_server.go +++ b/handshake/crypto_setup_server.go @@ -214,12 +214,16 @@ func (h *cryptoSetupServer) Open(dst, src []byte, packetNumber protocol.PacketNu func (h *cryptoSetupServer) GetSealer() (protocol.EncryptionLevel, Sealer) { h.mutex.RLock() defer h.mutex.RUnlock() - - if h.forwardSecureAEAD != nil && h.sentSHLO { + if h.forwardSecureAEAD != nil { return protocol.EncryptionForwardSecure, h.sealForwardSecure - } else if h.secureAEAD != nil { - // secureAEAD and forwardSecureAEAD are created at the same time (when receiving the CHLO) - // make sure that the SHLO isn't sent forward-secure + } + return protocol.EncryptionUnencrypted, h.sealUnencrypted +} + +func (h *cryptoSetupServer) GetSealerForCryptoStream() (protocol.EncryptionLevel, Sealer) { + h.mutex.RLock() + defer h.mutex.RUnlock() + if h.secureAEAD != nil { return protocol.EncryptionSecure, h.sealSecure } return protocol.EncryptionUnencrypted, h.sealUnencrypted @@ -251,7 +255,6 @@ func (h *cryptoSetupServer) sealUnencrypted(dst, src []byte, packetNumber protoc } func (h *cryptoSetupServer) sealSecure(dst, src []byte, packetNumber protocol.PacketNumber, associatedData []byte) []byte { - h.sentSHLO = true return h.secureAEAD.Seal(dst, src, packetNumber, associatedData) } diff --git a/handshake/crypto_setup_server_test.go b/handshake/crypto_setup_server_test.go index ea5b04dd..85718596 100644 --- a/handshake/crypto_setup_server_test.go +++ b/handshake/crypto_setup_server_test.go @@ -551,6 +551,13 @@ var _ = Describe("Server Crypto Setup", func() { Expect(d).To(Equal(foobarServerFNVSigned)) }) + It("is used for crypto stream", func() { + enc, seal := cs.GetSealerForCryptoStream() + Expect(enc).To(Equal(protocol.EncryptionUnencrypted)) + d := seal(nil, []byte("foobar"), 0, []byte{}) + Expect(d).To(Equal(foobarServerFNVSigned)) + }) + It("is accepted initially", func() { d, enc, err := cs.Open(nil, foobarClientFNVSigned, 0, []byte{}) Expect(err).ToNot(HaveOccurred()) @@ -595,14 +602,6 @@ var _ = Describe("Server Crypto Setup", func() { }) Context("initial encryption", func() { - It("is used after CHLO", func() { - doCHLO() - enc, seal := cs.GetSealer() - Expect(enc).To(Equal(protocol.EncryptionSecure)) - d := seal(nil, []byte("foobar"), 0, []byte{}) - Expect(d).To(Equal([]byte("foobar normal sec"))) - }) - It("is accepted after CHLO", func() { doCHLO() d, enc, err := cs.Open(nil, []byte("encrypted"), 0, []byte{}) @@ -619,15 +618,20 @@ var _ = Describe("Server Crypto Setup", func() { Expect(err).To(MatchError("authentication failed")) Expect(enc).To(Equal(protocol.EncryptionUnspecified)) }) + + It("is used for crypto stream", func() { + doCHLO() + enc, seal := cs.GetSealerForCryptoStream() + Expect(enc).To(Equal(protocol.EncryptionSecure)) + d := seal(nil, []byte("foobar"), 0, []byte{}) + Expect(d).To(Equal([]byte("foobar normal sec"))) + }) }) Context("forward secure encryption", func() { - It("is used after sending out one packet with initial encryption", func() { + It("is used after the CHLO", func() { doCHLO() enc, seal := cs.GetSealer() - Expect(enc).To(Equal(protocol.EncryptionSecure)) - _ = seal(nil, []byte("SHLO"), 0, []byte{}) - enc, seal = cs.GetSealer() Expect(enc).To(Equal(protocol.EncryptionForwardSecure)) d := seal(nil, []byte("foobar"), 0, []byte{}) Expect(d).To(Equal([]byte("foobar forward sec"))) diff --git a/handshake/interface.go b/handshake/interface.go index 67fb4062..751aae1e 100644 --- a/handshake/interface.go +++ b/handshake/interface.go @@ -15,6 +15,7 @@ type CryptoSetup interface { GetSealer() (protocol.EncryptionLevel, Sealer) GetSealerWithEncryptionLevel(protocol.EncryptionLevel) (Sealer, error) + GetSealerForCryptoStream() (protocol.EncryptionLevel, Sealer) } // TransportParameters are parameters sent to the peer during the handshake diff --git a/packet_packer.go b/packet_packer.go index 6e547500..72a55bf6 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -9,7 +9,6 @@ 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 { @@ -24,18 +23,21 @@ type packetPacker struct { perspective protocol.Perspective version protocol.VersionNumber cryptoSetup handshake.CryptoSetup - // as long as packets are not sent with forward-secure encryption, we limit the MaxPacketSize such that they can be retransmitted as a whole - isForwardSecure bool packetNumberGenerator *packetNumberGenerator + connectionParameters handshake.ConnectionParametersManager + streamFramer *streamFramer - connectionParameters handshake.ConnectionParametersManager - - streamFramer *streamFramer controlFrames []frames.Frame } -func newPacketPacker(connectionID protocol.ConnectionID, cryptoSetup handshake.CryptoSetup, connectionParameters handshake.ConnectionParametersManager, streamFramer *streamFramer, perspective protocol.Perspective, version protocol.VersionNumber) *packetPacker { +func newPacketPacker(connectionID protocol.ConnectionID, + cryptoSetup handshake.CryptoSetup, + connectionParameters handshake.ConnectionParametersManager, + streamFramer *streamFramer, + perspective protocol.Perspective, + version protocol.VersionNumber, +) *packetPacker { return &packetPacker{ cryptoSetup: cryptoSetup, connectionID: connectionID, @@ -63,7 +65,6 @@ func (p *packetPacker) RetransmitNonForwardSecurePacket(stopWaitingFrame *frames if stopWaitingFrame == nil { return nil, errors.New("PacketPacker BUG: Handshake retransmissions must contain a StopWaitingFrame") } - return p.packPacket(stopWaitingFrame, 0, packet) } @@ -78,10 +79,12 @@ func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, con func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, leastUnacked protocol.PacketNumber, handshakePacketToRetransmit *ackhandler.Packet) (*packedPacket, error) { // handshakePacketToRetransmit is only set for handshake retransmissions isHandshakeRetransmission := (handshakePacketToRetransmit != nil) + isCryptoStreamFrame := p.streamFramer.HasCryptoStreamFrame() var sealer handshake.Sealer var encLevel protocol.EncryptionLevel + // TODO(#656): Only do this for the crypto stream if isHandshakeRetransmission { var err error encLevel = handshakePacketToRetransmit.EncryptionLevel @@ -89,6 +92,8 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, lea if err != nil { return nil, err } + } else if isCryptoStreamFrame { + encLevel, sealer = p.cryptoSetup.GetSealerForCryptoStream() } else { encLevel, sealer = p.cryptoSetup.GetSealer() } @@ -125,12 +130,12 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, lea } } else if isConnectionClose { payloadFrames = []frames.Frame{p.controlFrames[0]} + } else if isCryptoStreamFrame { + maxLen := protocol.MaxFrameAndPublicHeaderSize - protocol.NonForwardSecurePacketSizeReduction - publicHeaderLength + payloadFrames = []frames.Frame{p.streamFramer.PopCryptoStreamFrame(maxLen)} } else { maxSize := protocol.MaxFrameAndPublicHeaderSize - publicHeaderLength - if !p.isForwardSecure { - maxSize -= protocol.NonForwardSecurePacketSizeReduction - } - payloadFrames, err = p.composeNextPacket(stopWaitingFrame, maxSize) + payloadFrames, err = p.composeNextPacket(stopWaitingFrame, maxSize, p.canSendData(encLevel)) if err != nil { return nil, err } @@ -156,11 +161,7 @@ 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 @@ -175,13 +176,9 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, lea _ = sealer(raw[payloadStartIndex:payloadStartIndex], raw[payloadStartIndex:], publicHeader.PacketNumber, raw[:payloadStartIndex]) raw = raw[0 : buffer.Len()+12] - if hasNonCryptoStreamData && encLevel <= protocol.EncryptionUnencrypted { - return nil, qerr.AttemptToSendUnencryptedStreamData - } - num := p.packetNumberGenerator.Pop() if num != publicHeader.PacketNumber { - return nil, errors.New("PacketPacker BUG: Peeked and Popped packet numbers do not match.") + return nil, errors.New("packetPacker BUG: Peeked and Popped packet numbers do not match") } return &packedPacket{ @@ -192,7 +189,8 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, lea }, nil } -func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFrame, maxFrameSize protocol.ByteCount) ([]frames.Frame, error) { +func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFrame, + maxFrameSize protocol.ByteCount, canSendStreamFrames bool) ([]frames.Frame, error) { var payloadLength protocol.ByteCount var payloadFrames []frames.Frame @@ -220,6 +218,10 @@ func (p *packetPacker) composeNextPacket(stopWaitingFrame *frames.StopWaitingFra return nil, fmt.Errorf("Packet Packer BUG: packet payload (%d) too large (%d)", payloadLength, maxFrameSize) } + if !canSendStreamFrames { + return payloadFrames, nil + } + // temporarily increase the maxFrameSize by 2 bytes // this leads to a properly sized packet in all cases, since we do all the packet length calculations with StreamFrames that have the DataLen set // however, for the last StreamFrame in the packet, we can omit the DataLen, thus saving 2 bytes and yielding a packet of exactly the correct size @@ -246,10 +248,6 @@ func (p *packetPacker) QueueControlFrameForNextPacket(f frames.Frame) { p.controlFrames = append(p.controlFrames, f) } -func (p *packetPacker) SetForwardSecure() { - p.isForwardSecure = true -} - func (p *packetPacker) getPublicHeader(leastUnacked protocol.PacketNumber, encLevel protocol.EncryptionLevel) *PublicHeader { pnum := p.packetNumberGenerator.Peek() packetNumberLen := protocol.GetPacketNumberLengthForPublicHeader(pnum, leastUnacked) @@ -270,3 +268,10 @@ func (p *packetPacker) getPublicHeader(leastUnacked protocol.PacketNumber, encLe return publicHeader } + +func (p *packetPacker) canSendData(encLevel protocol.EncryptionLevel) bool { + if p.perspective == protocol.PerspectiveClient { + return encLevel >= protocol.EncryptionSecure + } + return encLevel == protocol.EncryptionForwardSecure +} diff --git a/packet_packer_test.go b/packet_packer_test.go index f0369cd9..39c9709a 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -8,15 +8,15 @@ import ( "github.com/lucas-clemente/quic-go/handshake" "github.com/lucas-clemente/quic-go/internal/mocks" "github.com/lucas-clemente/quic-go/protocol" - "github.com/lucas-clemente/quic-go/qerr" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) type mockCryptoSetup struct { - handleErr error - divNonce []byte - encLevelSeal protocol.EncryptionLevel + handleErr error + divNonce []byte + encLevelSeal protocol.EncryptionLevel + encLevelSealCrypto protocol.EncryptionLevel } func (m *mockCryptoSetup) HandleCryptoStream() error { @@ -30,6 +30,11 @@ func (m *mockCryptoSetup) GetSealer() (protocol.EncryptionLevel, handshake.Seale return append(src, bytes.Repeat([]byte{0}, 12)...) } } +func (m *mockCryptoSetup) GetSealerForCryptoStream() (protocol.EncryptionLevel, handshake.Sealer) { + return m.encLevelSealCrypto, func(dst, src []byte, packetNumber protocol.PacketNumber, associatedData []byte) []byte { + return append(src, bytes.Repeat([]byte{0}, 12)...) + } +} func (m *mockCryptoSetup) GetSealerWithEncryptionLevel(protocol.EncryptionLevel) (handshake.Sealer, error) { return func(dst, src []byte, packetNumber protocol.PacketNumber, associatedData []byte) []byte { return append(src, bytes.Repeat([]byte{0}, 12)...) @@ -46,13 +51,19 @@ var _ = Describe("Packet packer", func() { publicHeaderLen protocol.ByteCount maxFrameSize protocol.ByteCount streamFramer *streamFramer + cryptoStream *stream ) BeforeEach(func() { mockCpm := mocks.NewMockConnectionParametersManager(mockCtrl) mockCpm.EXPECT().TruncateConnectionID().Return(false).AnyTimes() - streamFramer = newStreamFramer(newStreamsMap(nil, protocol.PerspectiveServer, nil), nil) + cryptoStream = &stream{} + + streamsMap := newStreamsMap(nil, protocol.PerspectiveServer, nil) + streamsMap.streams[1] = cryptoStream + streamsMap.openStreams = []protocol.StreamID{1} + streamFramer = newStreamFramer(streamsMap, nil) packer = &packetPacker{ cryptoSetup: &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure}, @@ -65,7 +76,6 @@ var _ = Describe("Packet packer", func() { publicHeaderLen = 1 + 8 + 2 // 1 flag byte, 8 connection ID, 2 packet number maxFrameSize = protocol.MaxFrameAndPublicHeaderSize - publicHeaderLen packer.version = protocol.VersionWhatever - packer.isForwardSecure = true }) It("returns nil when no packet is queued", func() { @@ -90,7 +100,7 @@ var _ = Describe("Packet packer", func() { }) It("stores the encryption level a packet was sealed with", func() { - packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionSecure + packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionForwardSecure f := &frames.StreamFrame{ StreamID: 5, Data: []byte("foobar"), @@ -98,7 +108,7 @@ var _ = Describe("Packet packer", func() { streamFramer.AddFrameForRetransmission(f) p, err := packer.PackPacket(nil, []frames.Frame{}, 0) Expect(err).ToNot(HaveOccurred()) - Expect(p.encryptionLevel).To(Equal(protocol.EncryptionSecure)) + Expect(p.encryptionLevel).To(Equal(protocol.EncryptionForwardSecure)) }) Context("diversificaton nonces", func() { @@ -107,40 +117,27 @@ var _ = Describe("Packet packer", func() { BeforeEach(func() { nonce = bytes.Repeat([]byte{'e'}, 32) packer.cryptoSetup.(*mockCryptoSetup).divNonce = nonce - f := &frames.StreamFrame{ - StreamID: 1, - Data: []byte{0xDE, 0xCA, 0xFB, 0xAD}, - } - streamFramer.AddFrameForRetransmission(f) }) It("doesn't include a div nonce, when sending a packet with initial encryption", func() { - packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionUnencrypted - p, err := packer.PackPacket(nil, []frames.Frame{}, 0) - Expect(err).ToNot(HaveOccurred()) - Expect(p.raw).ToNot(ContainSubstring(string(nonce))) + ph := packer.getPublicHeader(1, protocol.EncryptionUnencrypted) + Expect(ph.DiversificationNonce).To(BeEmpty()) }) It("includes a div nonce, when sending a packet with secure encryption", func() { - packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionSecure - p, err := packer.PackPacket(nil, []frames.Frame{}, 0) - Expect(err).ToNot(HaveOccurred()) - Expect(p.raw).To(ContainSubstring(string(nonce))) + ph := packer.getPublicHeader(1, protocol.EncryptionSecure) + Expect(ph.DiversificationNonce).To(Equal(nonce)) }) It("doesn't include a div nonce, when sending a packet with forward-secure encryption", func() { - packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionUnencrypted - p, err := packer.PackPacket(nil, []frames.Frame{}, 0) - Expect(err).ToNot(HaveOccurred()) - Expect(p.raw).ToNot(ContainSubstring(string(nonce))) + ph := packer.getPublicHeader(1, protocol.EncryptionForwardSecure) + Expect(ph.DiversificationNonce).To(BeEmpty()) }) It("doesn't send a div nonce as a client", func() { packer.perspective = protocol.PerspectiveClient - packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionSecure - p, err := packer.PackPacket(nil, []frames.Frame{}, 0) - Expect(err).ToNot(HaveOccurred()) - Expect(p.raw).ToNot(ContainSubstring(string(nonce))) + ph := packer.getPublicHeader(1, protocol.EncryptionSecure) + Expect(ph.DiversificationNonce).To(BeEmpty()) }) }) @@ -260,10 +257,10 @@ var _ = Describe("Packet packer", func() { controlFrames = append(controlFrames, f) } packer.controlFrames = controlFrames - payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize, false) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(maxFramesPerPacket)) - payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize, false) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(BeEmpty()) }) @@ -279,10 +276,10 @@ var _ = Describe("Packet packer", func() { controlFrames = append(controlFrames, blockedFrame) } packer.controlFrames = controlFrames - payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize, false) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(maxFramesPerPacket)) - payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize, false) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(10)) }) @@ -316,11 +313,11 @@ var _ = Describe("Packet packer", func() { maxStreamFrameDataLen := maxFrameSize - minLength f.Data = bytes.Repeat([]byte{'f'}, int(maxStreamFrameDataLen)) streamFramer.AddFrameForRetransmission(f) - payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(1)) Expect(payloadFrames[0].(*frames.StreamFrame).DataLenPresent).To(BeFalse()) - payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(BeEmpty()) }) @@ -350,18 +347,6 @@ var _ = Describe("Packet packer", func() { Expect(p.frames[0].(*frames.StreamFrame).DataLenPresent).To(BeFalse()) }) - It("packs smaller packets when it is not yet forward-secure", func() { - packer.isForwardSecure = false - f := &frames.StreamFrame{ - StreamID: 3, - Data: bytes.Repeat([]byte{'f'}, int(protocol.MaxPacketSize)), - } - streamFramer.AddFrameForRetransmission(f) - p, err := packer.PackPacket(nil, nil, 0) - Expect(err).ToNot(HaveOccurred()) - Expect(p.raw).To(HaveLen(int(protocol.MaxPacketSize - protocol.NonForwardSecurePacketSizeReduction))) - }) - It("packs multiple small stream frames into single packet", func() { f1 := &frames.StreamFrame{ StreamID: 5, @@ -403,17 +388,17 @@ var _ = Describe("Packet packer", func() { maxStreamFrameDataLen := protocol.MaxFrameAndPublicHeaderSize - publicHeaderLen - minLength f.Data = bytes.Repeat([]byte{'f'}, int(maxStreamFrameDataLen)+200) streamFramer.AddFrameForRetransmission(f) - payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(1)) Expect(payloadFrames[0].(*frames.StreamFrame).DataLenPresent).To(BeFalse()) Expect(payloadFrames[0].(*frames.StreamFrame).Data).To(HaveLen(int(maxStreamFrameDataLen))) - payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(1)) Expect(payloadFrames[0].(*frames.StreamFrame).Data).To(HaveLen(200)) Expect(payloadFrames[0].(*frames.StreamFrame).DataLenPresent).To(BeFalse()) - payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(BeEmpty()) }) @@ -476,10 +461,10 @@ var _ = Describe("Packet packer", func() { f.Data = bytes.Repeat([]byte{'f'}, int(protocol.MaxFrameAndPublicHeaderSize-publicHeaderLen-minLength+2)) // + 2 since MinceLength is 1 bigger than the actual StreamFrame header streamFramer.AddFrameForRetransmission(f) - payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err := packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(1)) - payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize) + payloadFrames, err = packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(payloadFrames).To(HaveLen(1)) }) @@ -491,11 +476,13 @@ var _ = Describe("Packet packer", func() { Data: []byte("foobar"), } streamFramer.AddFrameForRetransmission(f) - _, err := packer.PackPacket(nil, nil, 0) - Expect(err).To(MatchError(qerr.AttemptToSendUnencryptedStreamData)) + p, err := packer.PackPacket(nil, nil, 0) + Expect(err).NotTo(HaveOccurred()) + Expect(p).To(BeNil()) }) - It("sends encrypted, non forward-secure, stream data on a data stream", func() { + It("sends non forward-secure data as the client", func() { + packer.perspective = protocol.PerspectiveClient packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionSecure f := &frames.StreamFrame{ StreamID: 5, @@ -508,30 +495,46 @@ var _ = Describe("Packet packer", func() { Expect(p.frames[0]).To(Equal(f)) }) - It("sends unencrypted stream data on the crypto stream", func() { - packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionUnencrypted + It("does not send non forward-secure data as the server", func() { + packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionSecure f := &frames.StreamFrame{ - StreamID: 1, + StreamID: 5, Data: []byte("foobar"), } streamFramer.AddFrameForRetransmission(f) p, err := packer.PackPacket(nil, nil, 0) Expect(err).ToNot(HaveOccurred()) + Expect(p).To(BeNil()) + }) + + It("sends unencrypted stream data on the crypto stream", func() { + packer.cryptoSetup.(*mockCryptoSetup).encLevelSealCrypto = protocol.EncryptionUnencrypted + cryptoStream.dataForWriting = []byte("foobar") + 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)) + Expect(p.frames).To(HaveLen(1)) + Expect(p.frames[0]).To(Equal(&frames.StreamFrame{StreamID: 1, Data: []byte("foobar")})) }) 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) + packer.cryptoSetup.(*mockCryptoSetup).encLevelSealCrypto = protocol.EncryptionSecure + cryptoStream.dataForWriting = []byte("foobar") 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)) + Expect(p.frames).To(HaveLen(1)) + Expect(p.frames[0]).To(Equal(&frames.StreamFrame{StreamID: 1, Data: []byte("foobar")})) + }) + + It("does not pack stream frames if not allowed", func() { + packer.cryptoSetup.(*mockCryptoSetup).encLevelSeal = protocol.EncryptionUnencrypted + packer.QueueControlFrameForNextPacket(&frames.AckFrame{}) + streamFramer.AddFrameForRetransmission(&frames.StreamFrame{StreamID: 3, Data: []byte("foobar")}) + p, err := packer.PackPacket(nil, nil, 0) + Expect(err).ToNot(HaveOccurred()) + Expect(p.frames).To(HaveLen(1)) + Expect(func() { _ = p.frames[0].(*frames.AckFrame) }).NotTo(Panic()) }) }) @@ -544,7 +547,7 @@ var _ = Describe("Packet packer", func() { Data: bytes.Repeat([]byte{'f'}, length), } streamFramer.AddFrameForRetransmission(f) - _, err := packer.composeNextPacket(nil, maxFrameSize) + _, err := packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(packer.controlFrames[0]).To(Equal(&frames.BlockedFrame{StreamID: 5})) }) @@ -557,7 +560,7 @@ var _ = Describe("Packet packer", func() { Data: bytes.Repeat([]byte{'f'}, length), } streamFramer.AddFrameForRetransmission(f) - p, err := packer.composeNextPacket(nil, maxFrameSize) + p, err := packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(p).To(HaveLen(1)) Expect(p[0].(*frames.StreamFrame).DataLenPresent).To(BeFalse()) @@ -570,7 +573,7 @@ var _ = Describe("Packet packer", func() { Data: []byte("foobar"), } streamFramer.AddFrameForRetransmission(f) - _, err := packer.composeNextPacket(nil, maxFrameSize) + _, err := packer.composeNextPacket(nil, maxFrameSize, true) Expect(err).ToNot(HaveOccurred()) Expect(packer.controlFrames[0]).To(Equal(&frames.BlockedFrame{StreamID: 0})) }) diff --git a/session.go b/session.go index dfffccd4..ea04d7cb 100644 --- a/session.go +++ b/session.go @@ -219,7 +219,8 @@ func (s *session) setup( return nil, nil, err } - s.packer = newPacketPacker(s.connectionID, s.cryptoSetup, s.connectionParameters, s.streamFramer, s.perspective, s.version) + s.packer = newPacketPacker(s.connectionID, s.cryptoSetup, s.connectionParameters, s.streamFramer, + s.perspective, s.version) s.unpacker = &packetUnpacker{aead: s.cryptoSetup, version: s.version} return s, handshakeChan, nil @@ -278,9 +279,6 @@ runLoop: close(s.handshakeChan) close(s.handshakeCompleteChan) } else { - if l == protocol.EncryptionForwardSecure { - s.packer.SetForwardSecure() - } s.tryDecryptingQueuedPackets() s.handshakeChan <- handshakeEvent{encLevel: l} } diff --git a/session_test.go b/session_test.go index 67cfc54c..a16d10bd 100644 --- a/session_test.go +++ b/session_test.go @@ -914,7 +914,7 @@ var _ = Describe("Session", func() { It("informs the SentPacketHandler about sent packets", func() { sess.sentPacketHandler = newMockSentPacketHandler() sess.packer.packetNumberGenerator.next = 0x1337 + 9 - sess.packer.cryptoSetup = &mockCryptoSetup{encLevelSeal: protocol.EncryptionSecure} + sess.packer.cryptoSetup = &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure} f := &frames.StreamFrame{ StreamID: 5, @@ -929,7 +929,7 @@ var _ = Describe("Session", func() { sentPackets := sess.sentPacketHandler.(*mockSentPacketHandler).sentPackets Expect(sentPackets).To(HaveLen(1)) Expect(sentPackets[0].Frames).To(ContainElement(f)) - Expect(sentPackets[0].EncryptionLevel).To(Equal(protocol.EncryptionSecure)) + Expect(sentPackets[0].EncryptionLevel).To(Equal(protocol.EncryptionForwardSecure)) Expect(sentPackets[0].Length).To(BeEquivalentTo(len(mconn.written[0]))) }) }) @@ -994,10 +994,6 @@ var _ = Describe("Session", func() { }) Context("for packets after the handshake", func() { - BeforeEach(func() { - sess.packer.SetForwardSecure() - }) - It("sends a StreamFrame from a packet queued for retransmission", func() { f := frames.StreamFrame{ StreamID: 0x5, @@ -1252,16 +1248,6 @@ var _ = Describe("Session", func() { }) }) - It("tells the packetPacker when forward-secure encryption is used", func(done Done) { - go sess.run() - aeadChanged <- protocol.EncryptionSecure - Consistently(func() bool { return sess.packer.isForwardSecure }).Should(BeFalse()) - aeadChanged <- protocol.EncryptionForwardSecure - Eventually(func() bool { return sess.packer.isForwardSecure }).Should(BeTrue()) - Expect(sess.Close(nil)).To(Succeed()) - close(done) - }) - It("closes when crypto stream errors", func() { testErr := errors.New("crypto setup error") cryptoSetup.handleErr = testErr diff --git a/stream_framer.go b/stream_framer.go index 45c07220..bcc4b6e1 100644 --- a/stream_framer.go +++ b/stream_framer.go @@ -45,6 +45,28 @@ func (f *streamFramer) HasFramesForRetransmission() bool { return len(f.retransmissionQueue) > 0 } +func (f *streamFramer) HasCryptoStreamFrame() bool { + // TODO(#657): Flow control + cs, _ := f.streamsMap.GetOrOpenStream(1) + return cs.lenOfDataForWriting() > 0 +} + +// TODO(lclemente): This is somewhat duplicate with the normal path for generating frames. +// TODO(#657): Flow control +func (f *streamFramer) PopCryptoStreamFrame(maxLen protocol.ByteCount) *frames.StreamFrame { + if !f.HasCryptoStreamFrame() { + return nil + } + cs, _ := f.streamsMap.GetOrOpenStream(1) + frame := &frames.StreamFrame{ + StreamID: 1, + Offset: cs.writeOffset, + } + frameHeaderBytes, _ := frame.MinLength(protocol.VersionWhatever) // can never error + frame.Data = cs.getDataForWriting(maxLen - frameHeaderBytes) + return frame +} + func (f *streamFramer) maybePopFramesForRetransmission(maxLen protocol.ByteCount) (res []*frames.StreamFrame, currentLen protocol.ByteCount) { for len(f.retransmissionQueue) > 0 { frame := f.retransmissionQueue[0] @@ -76,7 +98,7 @@ func (f *streamFramer) maybePopNormalFrames(maxBytes protocol.ByteCount) (res [] var currentLen protocol.ByteCount fn := func(s *stream) (bool, error) { - if s == nil { + if s == nil || s.streamID == 1 /* crypto stream is handled separately */ { return true, nil }