From 8c5e7818a03baf24c7f7be2befc8f73872bc53ac Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 1 Mar 2017 15:06:10 +0700 Subject: [PATCH] retransmit the diversification nonce in the packet carrying the SHLO The packet containing the SHLO is the only packet that is sent with initial encryption. If it is lost, we need to make sure that the diversification nonce is included in the PublicHeader, otherwise the client will not be able to derive the keys for the forward-secure encryption. --- handshake/crypto_setup_client.go | 2 +- handshake/crypto_setup_interface.go | 4 ++-- handshake/crypto_setup_server.go | 8 +++---- handshake/crypto_setup_server_test.go | 13 ++++++---- packet_packer.go | 3 ++- packet_packer_test.go | 34 ++++++++++++++++++++------- 6 files changed, 44 insertions(+), 20 deletions(-) diff --git a/handshake/crypto_setup_client.go b/handshake/crypto_setup_client.go index 50ca9227..2148ae53 100644 --- a/handshake/crypto_setup_client.go +++ b/handshake/crypto_setup_client.go @@ -331,7 +331,7 @@ func (h *cryptoSetupClient) SealWith(dst, src []byte, packetNumber protocol.Pack return nil, protocol.EncryptionUnspecified, errors.New("no encryption level specified") } -func (h *cryptoSetupClient) DiversificationNonce() []byte { +func (h *cryptoSetupClient) DiversificationNonce(bool) []byte { panic("not needed for cryptoSetupClient") } diff --git a/handshake/crypto_setup_interface.go b/handshake/crypto_setup_interface.go index 49952525..90e09bd4 100644 --- a/handshake/crypto_setup_interface.go +++ b/handshake/crypto_setup_interface.go @@ -12,6 +12,6 @@ type CryptoSetup interface { UnlockForSealing() HandshakeComplete() bool // TODO: clean up this interface - DiversificationNonce() []byte // only needed for cryptoSetupServer - SetDiversificationNonce([]byte) error // only needed for cryptoSetupClient + DiversificationNonce(force bool) []byte // only needed for cryptoSetupServer + SetDiversificationNonce([]byte) error // only needed for cryptoSetupClient } diff --git a/handshake/crypto_setup_server.go b/handshake/crypto_setup_server.go index 8bbd9262..4d3bf8e0 100644 --- a/handshake/crypto_setup_server.go +++ b/handshake/crypto_setup_server.go @@ -390,11 +390,11 @@ func (h *cryptoSetupServer) handleCHLO(sni string, data []byte, cryptoData map[T } // DiversificationNonce returns a diversification nonce if required in the next packet to be Seal'ed. See LockForSealing()! -func (h *cryptoSetupServer) DiversificationNonce() []byte { - if h.secureAEAD == nil || h.sentSHLO { - return nil +func (h *cryptoSetupServer) DiversificationNonce(force bool) []byte { + if force || (h.secureAEAD != nil && !h.sentSHLO) { + return h.diversificationNonce } - return h.diversificationNonce + return nil } func (h *cryptoSetupServer) SetDiversificationNonce(data []byte) error { diff --git a/handshake/crypto_setup_server_test.go b/handshake/crypto_setup_server_test.go index 89b22ee9..a81e29f8 100644 --- a/handshake/crypto_setup_server_test.go +++ b/handshake/crypto_setup_server_test.go @@ -185,23 +185,28 @@ var _ = Describe("Crypto setup", func() { cs.secureAEAD = &mockAEAD{} cs.receivedForwardSecurePacket = false - Expect(cs.DiversificationNonce()).To(BeEmpty()) + Expect(cs.DiversificationNonce(false)).To(BeEmpty()) // Div nonce is created after CHLO cs.handleCHLO("", nil, map[Tag][]byte{TagNONC: nonce32}) }) It("returns diversification nonces", func() { - Expect(cs.DiversificationNonce()).To(HaveLen(32)) + Expect(cs.DiversificationNonce(false)).To(HaveLen(32)) }) It("does not return nonce after sending the SHLO", func() { cs.sentSHLO = true - Expect(cs.DiversificationNonce()).To(BeEmpty()) + Expect(cs.DiversificationNonce(false)).To(BeEmpty()) + }) + + It("returns a nonce for a retransmission, even after sending the SHLO", func() { + cs.sentSHLO = true + Expect(cs.DiversificationNonce(true)).To(HaveLen(32)) }) It("does not return nonce for unencrypted packets", func() { cs.secureAEAD = nil - Expect(cs.DiversificationNonce()).To(BeEmpty()) + Expect(cs.DiversificationNonce(false)).To(BeEmpty()) }) }) diff --git a/packet_packer.go b/packet_packer.go index df8ce311..10161899 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -93,7 +93,8 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, lea } if p.perspective == protocol.PerspectiveServer { - responsePublicHeader.DiversificationNonce = p.cryptoSetup.DiversificationNonce() + force := isHandshakeRetransmission && (packetToRetransmit.EncryptionLevel == protocol.EncryptionSecure) + responsePublicHeader.DiversificationNonce = p.cryptoSetup.DiversificationNonce(force) } if p.perspective == protocol.PerspectiveClient && !p.cryptoSetup.HandshakeComplete() { diff --git a/packet_packer_test.go b/packet_packer_test.go index a1421080..453c3f76 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -12,9 +12,10 @@ import ( ) type mockCryptoSetup struct { - diversificationNonce []byte - handshakeComplete bool - encLevelSeal protocol.EncryptionLevel + divNonce []byte + forcedDivNonce bool + handshakeComplete bool + encLevelSeal protocol.EncryptionLevel } func (m *mockCryptoSetup) HandleCryptoStream() error { return nil } @@ -28,10 +29,13 @@ func (m *mockCryptoSetup) Seal(dst, src []byte, packetNumber protocol.PacketNumb func (m *mockCryptoSetup) SealWith(dst, src []byte, packetNumber protocol.PacketNumber, associatedData []byte, encLevel protocol.EncryptionLevel) ([]byte, protocol.EncryptionLevel, error) { return append(src, bytes.Repeat([]byte{0}, 12)...), encLevel, nil } -func (m *mockCryptoSetup) LockForSealing() {} -func (m *mockCryptoSetup) UnlockForSealing() {} -func (m *mockCryptoSetup) HandshakeComplete() bool { return m.handshakeComplete } -func (m *mockCryptoSetup) DiversificationNonce() []byte { return m.diversificationNonce } +func (m *mockCryptoSetup) LockForSealing() {} +func (m *mockCryptoSetup) UnlockForSealing() {} +func (m *mockCryptoSetup) HandshakeComplete() bool { return m.handshakeComplete } +func (m *mockCryptoSetup) DiversificationNonce(force bool) []byte { + m.forcedDivNonce = force + return m.divNonce +} func (m *mockCryptoSetup) SetDiversificationNonce([]byte) error { panic("not implemented") } var _ = Describe("Packet packer", func() { @@ -54,6 +58,7 @@ var _ = Describe("Packet packer", func() { packer = &packetPacker{ cryptoSetup: &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure}, connectionParameters: cpm, + connectionID: 0x1337, packetNumberGenerator: newPacketNumberGenerator(protocol.SkipPacketAveragePeriodLength), streamFramer: streamFramer, perspective: protocol.PerspectiveServer, @@ -99,7 +104,7 @@ var _ = Describe("Packet packer", func() { It("includes a diversification nonce, when acting as a server", func() { nonce := bytes.Repeat([]byte{'e'}, 32) - packer.cryptoSetup.(*mockCryptoSetup).diversificationNonce = nonce + packer.cryptoSetup.(*mockCryptoSetup).divNonce = nonce f := &frames.StreamFrame{ StreamID: 5, Data: []byte{0xDE, 0xCA, 0xFB, 0xAD}, @@ -588,6 +593,8 @@ var _ = Describe("Packet packer", func() { Expect(p.frames).To(ContainElement(sf)) Expect(p.frames).To(ContainElement(swf)) Expect(p.encryptionLevel).To(Equal(protocol.EncryptionUnencrypted)) + // unencrypted packets don't need a diversification nonce + Expect(packer.cryptoSetup.(*mockCryptoSetup).forcedDivNonce).To(BeFalse()) }) It("packs a retransmission for a packet sent with initial encryption", func() { @@ -602,6 +609,17 @@ var _ = Describe("Packet packer", func() { Expect(p.encryptionLevel).To(Equal(protocol.EncryptionSecure)) }) + It("includes the diversification nonce on packets sent with initial encryption", func() { + packet := &ackhandler.Packet{ + EncryptionLevel: protocol.EncryptionSecure, + Frames: []frames.Frame{sf}, + } + p, err := packer.RetransmitNonForwardSecurePacket(swf, packet) + Expect(err).ToNot(HaveOccurred()) + Expect(p.encryptionLevel).To(Equal(protocol.EncryptionSecure)) + Expect(packer.cryptoSetup.(*mockCryptoSetup).forcedDivNonce).To(BeTrue()) + }) + It("removes non-retransmittable frames", func() { wuf := &frames.WindowUpdateFrame{StreamID: 5, ByteOffset: 10} packet := &ackhandler.Packet{