From 6c1eba584810b6b984731b2a5b7754b04eefe5f4 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 24 Mar 2018 08:36:52 +0000 Subject: [PATCH] generate the diversification nonce when creating the server crypto setup --- internal/handshake/crypto_setup_client.go | 4 -- internal/handshake/crypto_setup_server.go | 40 ++++++++----------- .../handshake/crypto_setup_server_test.go | 16 +------- internal/handshake/crypto_setup_tls.go | 4 -- internal/handshake/interface.go | 2 - packet_packer.go | 5 ++- packet_packer_test.go | 24 ++++------- session.go | 10 +++++ session_test.go | 2 + 9 files changed, 41 insertions(+), 66 deletions(-) diff --git a/internal/handshake/crypto_setup_client.go b/internal/handshake/crypto_setup_client.go index 532a2a81..ee553639 100644 --- a/internal/handshake/crypto_setup_client.go +++ b/internal/handshake/crypto_setup_client.go @@ -373,10 +373,6 @@ func (h *cryptoSetupClient) GetSealerWithEncryptionLevel(encLevel protocol.Encry return nil, errors.New("CryptoSetupClient: no encryption level specified") } -func (h *cryptoSetupClient) DiversificationNonce() []byte { - panic("not needed for cryptoSetupClient") -} - func (h *cryptoSetupClient) ConnectionState() ConnectionState { h.mutex.Lock() defer h.mutex.Unlock() diff --git a/internal/handshake/crypto_setup_server.go b/internal/handshake/crypto_setup_server.go index 4f87664d..ec4fbbab 100644 --- a/internal/handshake/crypto_setup_server.go +++ b/internal/handshake/crypto_setup_server.go @@ -73,6 +73,7 @@ func NewCryptoSetup( connID protocol.ConnectionID, remoteAddr net.Addr, version protocol.VersionNumber, + divNonce []byte, scfg *ServerConfig, params *TransportParameters, supportedVersions []protocol.VersionNumber, @@ -85,20 +86,21 @@ func NewCryptoSetup( return nil, err } return &cryptoSetupServer{ - cryptoStream: cryptoStream, - connID: connID, - remoteAddr: remoteAddr, - version: version, - supportedVersions: supportedVersions, - scfg: scfg, - keyDerivation: crypto.DeriveQuicCryptoAESKeys, - keyExchange: getEphermalKEX, - nullAEAD: nullAEAD, - params: params, - acceptSTKCallback: acceptSTK, - sentSHLO: make(chan struct{}), - paramsChan: paramsChan, - handshakeEvent: handshakeEvent, + cryptoStream: cryptoStream, + connID: connID, + remoteAddr: remoteAddr, + version: version, + supportedVersions: supportedVersions, + diversificationNonce: divNonce, + scfg: scfg, + keyDerivation: crypto.DeriveQuicCryptoAESKeys, + keyExchange: getEphermalKEX, + nullAEAD: nullAEAD, + params: params, + acceptSTKCallback: acceptSTK, + sentSHLO: make(chan struct{}), + paramsChan: paramsChan, + handshakeEvent: handshakeEvent, }, nil } @@ -364,11 +366,6 @@ func (h *cryptoSetupServer) handleCHLO(sni string, data []byte, cryptoData map[T return nil, err } - h.diversificationNonce = make([]byte, 32) - if _, err = rand.Read(h.diversificationNonce); err != nil { - return nil, err - } - clientNonce := cryptoData[TagNONC] err = h.validateClientNonce(clientNonce) if err != nil { @@ -450,11 +447,6 @@ func (h *cryptoSetupServer) handleCHLO(sni string, data []byte, cryptoData map[T return reply.Bytes(), nil } -// DiversificationNonce returns the diversification nonce -func (h *cryptoSetupServer) DiversificationNonce() []byte { - return h.diversificationNonce -} - func (h *cryptoSetupServer) ConnectionState() ConnectionState { h.mutex.Lock() defer h.mutex.Unlock() diff --git a/internal/handshake/crypto_setup_server_test.go b/internal/handshake/crypto_setup_server_test.go index 86f10884..bc4d90c9 100644 --- a/internal/handshake/crypto_setup_server_test.go +++ b/internal/handshake/crypto_setup_server_test.go @@ -164,6 +164,7 @@ var _ = Describe("Server Crypto Setup", func() { protocol.ConnectionID(42), remoteAddr, version, + make([]byte, 32), // div nonce scfg, &TransportParameters{IdleTimeout: protocol.DefaultIdleTimeout}, supportedVersions, @@ -184,21 +185,6 @@ var _ = Describe("Server Crypto Setup", func() { cs.cryptoStream = stream }) - Context("diversification nonce", func() { - BeforeEach(func() { - cs.secureAEAD = mockcrypto.NewMockAEAD(mockCtrl) - cs.receivedForwardSecurePacket = false - - Expect(cs.DiversificationNonce()).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)) - }) - }) - Context("when responding to client messages", func() { var cert []byte var xlct []byte diff --git a/internal/handshake/crypto_setup_tls.go b/internal/handshake/crypto_setup_tls.go index 5e4d90db..b9385ccf 100644 --- a/internal/handshake/crypto_setup_tls.go +++ b/internal/handshake/crypto_setup_tls.go @@ -157,10 +157,6 @@ func (h *cryptoSetupTLS) GetSealerForCryptoStream() (protocol.EncryptionLevel, S return protocol.EncryptionUnencrypted, h.nullAEAD } -func (h *cryptoSetupTLS) DiversificationNonce() []byte { - panic("diversification nonce not needed for TLS") -} - func (h *cryptoSetupTLS) ConnectionState() ConnectionState { h.mutex.Lock() defer h.mutex.Unlock() diff --git a/internal/handshake/interface.go b/internal/handshake/interface.go index 8bd2f5ab..caca3ef7 100644 --- a/internal/handshake/interface.go +++ b/internal/handshake/interface.go @@ -39,8 +39,6 @@ type MintTLS interface { type CryptoSetup interface { Open(dst, src []byte, packetNumber protocol.PacketNumber, associatedData []byte) ([]byte, protocol.EncryptionLevel, error) HandleCryptoStream() error - // TODO: clean up this interface - DiversificationNonce() []byte // only needed for cryptoSetupServer ConnectionState() ConnectionState GetSealer() (protocol.EncryptionLevel, Sealer) diff --git a/packet_packer.go b/packet_packer.go index 01079da3..0b872c01 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -44,6 +44,7 @@ type packetPacker struct { perspective protocol.Perspective version protocol.VersionNumber cryptoSetup handshake.CryptoSetup + divNonce []byte packetNumberGenerator *packetNumberGenerator getPacketNumberLen func(protocol.PacketNumber) protocol.PacketNumberLen @@ -64,6 +65,7 @@ func newPacketPacker(connectionID protocol.ConnectionID, initialPacketNumber protocol.PacketNumber, getPacketNumberLen func(protocol.PacketNumber) protocol.PacketNumberLen, remoteAddr net.Addr, // only used for determining the max packet size + divNonce []byte, cryptoSetup handshake.CryptoSetup, streamFramer streamFrameSource, perspective protocol.Perspective, @@ -84,6 +86,7 @@ func newPacketPacker(connectionID protocol.ConnectionID, } return &packetPacker{ cryptoSetup: cryptoSetup, + divNonce: divNonce, connectionID: connectionID, perspective: perspective, version: version, @@ -457,7 +460,7 @@ func (p *packetPacker) getHeader(encLevel protocol.EncryptionLevel) *wire.Header } if !p.version.UsesTLS() { if p.perspective == protocol.PerspectiveServer && encLevel == protocol.EncryptionSecure { - header.DiversificationNonce = p.cryptoSetup.DiversificationNonce() + header.DiversificationNonce = p.divNonce } if p.perspective == protocol.PerspectiveClient && encLevel != protocol.EncryptionForwardSecure { header.VersionFlag = true diff --git a/packet_packer_test.go b/packet_packer_test.go index df139b00..9226f859 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -26,7 +26,6 @@ var _ handshake.Sealer = &mockSealer{} type mockCryptoSetup struct { handleErr error - divNonce []byte encLevelSeal protocol.EncryptionLevel encLevelSealCrypto protocol.EncryptionLevel } @@ -48,7 +47,6 @@ func (m *mockCryptoSetup) GetSealerForCryptoStream() (protocol.EncryptionLevel, func (m *mockCryptoSetup) GetSealerWithEncryptionLevel(protocol.EncryptionLevel) (handshake.Sealer, error) { return &mockSealer{}, nil } -func (m *mockCryptoSetup) DiversificationNonce() []byte { return m.divNonce } func (m *mockCryptoSetup) ConnectionState() ConnectionState { panic("not implemented") } var _ = Describe("Packet packer", func() { @@ -58,6 +56,7 @@ var _ = Describe("Packet packer", func() { publicHeaderLen protocol.ByteCount maxFrameSize protocol.ByteCount mockStreamFramer *MockStreamFrameSource + divNonce []byte ) BeforeEach(func() { @@ -65,12 +64,14 @@ var _ = Describe("Packet packer", func() { mockSender := NewMockStreamSender(mockCtrl) mockSender.EXPECT().onHasStreamData(gomock.Any()).AnyTimes() mockStreamFramer = NewMockStreamFrameSource(mockCtrl) + divNonce = bytes.Repeat([]byte{'e'}, 32) packer = newPacketPacker( 0x1337, 1, func(protocol.PacketNumber) protocol.PacketNumberLen { return protocol.PacketNumberLen2 }, &net.TCPAddr{}, + divNonce, &mockCryptoSetup{encLevelSeal: protocol.EncryptionForwardSecure}, mockStreamFramer, protocol.PerspectiveServer, @@ -86,20 +87,20 @@ var _ = Describe("Packet packer", func() { Context("determining the maximum packet size", func() { It("uses the minimum initial size, if it can't determine if the remote address is IPv4 or IPv6", func() { remoteAddr := &net.TCPAddr{} - packer = newPacketPacker(0x1337, 1, nil, remoteAddr, nil, nil, protocol.PerspectiveServer, protocol.VersionWhatever) + packer = newPacketPacker(0x1337, 1, nil, remoteAddr, nil, nil, nil, protocol.PerspectiveServer, protocol.VersionWhatever) Expect(packer.maxPacketSize).To(BeEquivalentTo(protocol.MinInitialPacketSize)) }) It("uses the maximum IPv4 packet size, if the remote address is IPv4", func() { remoteAddr := &net.UDPAddr{IP: net.IPv4(11, 12, 13, 14), Port: 1337} - packer = newPacketPacker(0x1337, 1, nil, remoteAddr, nil, nil, protocol.PerspectiveServer, protocol.VersionWhatever) + packer = newPacketPacker(0x1337, 1, nil, remoteAddr, nil, nil, nil, protocol.PerspectiveServer, protocol.VersionWhatever) Expect(packer.maxPacketSize).To(BeEquivalentTo(protocol.MaxPacketSizeIPv4)) }) It("uses the maximum IPv6 packet size, if the remote address is IPv6", func() { ip := net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334") remoteAddr := &net.UDPAddr{IP: ip, Port: 1337} - packer = newPacketPacker(0x1337, 1, nil, remoteAddr, nil, nil, protocol.PerspectiveServer, protocol.VersionWhatever) + packer = newPacketPacker(0x1337, 1, nil, remoteAddr, nil, nil, nil, protocol.PerspectiveServer, protocol.VersionWhatever) Expect(packer.maxPacketSize).To(BeEquivalentTo(protocol.MaxPacketSizeIPv6)) }) }) @@ -178,13 +179,6 @@ var _ = Describe("Packet packer", func() { }) Context("diversificaton nonces", func() { - var nonce []byte - - BeforeEach(func() { - nonce = bytes.Repeat([]byte{'e'}, 32) - packer.cryptoSetup.(*mockCryptoSetup).divNonce = nonce - }) - It("doesn't include a div nonce, when sending a packet with initial encryption", func() { ph := packer.getHeader(protocol.EncryptionUnencrypted) Expect(ph.DiversificationNonce).To(BeEmpty()) @@ -192,7 +186,7 @@ var _ = Describe("Packet packer", func() { It("includes a div nonce, when sending a packet with secure encryption", func() { ph := packer.getHeader(protocol.EncryptionSecure) - Expect(ph.DiversificationNonce).To(Equal(nonce)) + Expect(ph.DiversificationNonce).To(Equal(divNonce)) }) It("doesn't include a div nonce, when sending a packet with forward-secure encryption", func() { @@ -666,8 +660,6 @@ var _ = Describe("Packet packer", func() { }) It("packs a retransmission for a packet sent with initial encryption", func() { - nonce := bytes.Repeat([]byte{'e'}, 32) - packer.cryptoSetup.(*mockCryptoSetup).divNonce = nonce packet := &ackhandler.Packet{ EncryptionLevel: protocol.EncryptionSecure, Frames: []wire.Frame{sf}, @@ -679,7 +671,7 @@ var _ = Describe("Packet packer", func() { Expect(p[0].encryptionLevel).To(Equal(protocol.EncryptionSecure)) // a packet sent by the server with initial encryption contains the SHLO // it needs to have a diversification nonce - Expect(p[0].raw).To(ContainSubstring(string(nonce))) + Expect(p[0].raw).To(ContainSubstring(string(divNonce))) }) It("includes the diversification nonce on packets sent with initial encryption", func() { diff --git a/session.go b/session.go index 9996e3d5..7befb33b 100644 --- a/session.go +++ b/session.go @@ -2,6 +2,7 @@ package quic import ( "context" + "crypto/rand" "crypto/tls" "errors" "fmt" @@ -167,11 +168,16 @@ func newSession( MaxStreams: uint32(s.config.MaxIncomingStreams), IdleTimeout: s.config.IdleTimeout, } + divNonce := make([]byte, 32) + if _, err := rand.Read(divNonce); err != nil { + return nil, err + } cs, err := newCryptoSetup( s.cryptoStream, s.connectionID, s.conn.RemoteAddr(), s.version, + divNonce, scfg, transportParams, s.config.Versions, @@ -190,6 +196,7 @@ func newSession( 1, s.sentPacketHandler.GetPacketNumberLen, s.RemoteAddr(), + divNonce, cs, s.streamFramer, s.perspective, @@ -252,6 +259,7 @@ var newClientSession = func( 1, s.sentPacketHandler.GetPacketNumberLen, s.RemoteAddr(), + nil, // no diversification nonce cs, s.streamFramer, s.perspective, @@ -295,6 +303,7 @@ func newTLSServerSession( initialPacketNumber, s.sentPacketHandler.GetPacketNumberLen, s.RemoteAddr(), + nil, // no diversification nonce cs, s.streamFramer, s.perspective, @@ -351,6 +360,7 @@ var newTLSClientSession = func( initialPacketNumber, s.sentPacketHandler.GetPacketNumberLen, s.RemoteAddr(), + nil, // no diversification nonce cs, s.streamFramer, s.perspective, diff --git a/session_test.go b/session_test.go index 4a21f589..329fcd18 100644 --- a/session_test.go +++ b/session_test.go @@ -96,6 +96,7 @@ var _ = Describe("Session", func() { _ protocol.ConnectionID, _ net.Addr, _ protocol.VersionNumber, + _ []byte, _ *handshake.ServerConfig, _ *handshake.TransportParameters, _ []protocol.VersionNumber, @@ -147,6 +148,7 @@ var _ = Describe("Session", func() { _ protocol.ConnectionID, _ net.Addr, _ protocol.VersionNumber, + _ []byte, _ *handshake.ServerConfig, _ *handshake.TransportParameters, _ []protocol.VersionNumber,