From 3a357369b0a5e17e96a95d21ec885e19eacc65c6 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 7 May 2017 22:39:16 +0800 Subject: [PATCH] use a directed aeadChanged chan in the crypto setups --- handshake/crypto_setup_client.go | 4 +- handshake/crypto_setup_client_test.go | 60 ++++++++++++++------------- handshake/crypto_setup_server.go | 4 +- handshake/crypto_setup_server_test.go | 6 +-- 4 files changed, 39 insertions(+), 35 deletions(-) diff --git a/handshake/crypto_setup_client.go b/handshake/crypto_setup_client.go index 44da0433..341b39de 100644 --- a/handshake/crypto_setup_client.go +++ b/handshake/crypto_setup_client.go @@ -47,7 +47,7 @@ type cryptoSetupClient struct { nullAEAD crypto.AEAD secureAEAD crypto.AEAD forwardSecureAEAD crypto.AEAD - aeadChanged chan protocol.EncryptionLevel + aeadChanged chan<- protocol.EncryptionLevel connectionParameters ConnectionParametersManager } @@ -68,7 +68,7 @@ func NewCryptoSetupClient( cryptoStream io.ReadWriter, tlsConfig *tls.Config, connectionParameters ConnectionParametersManager, - aeadChanged chan protocol.EncryptionLevel, + aeadChanged chan<- protocol.EncryptionLevel, negotiatedVersions []protocol.VersionNumber, ) (CryptoSetup, error) { return &cryptoSetupClient{ diff --git a/handshake/crypto_setup_client_test.go b/handshake/crypto_setup_client_test.go index e28ab12f..33e0de86 100644 --- a/handshake/crypto_setup_client_test.go +++ b/handshake/crypto_setup_client_test.go @@ -72,11 +72,14 @@ func (m *mockCertManager) Verify(hostname string) error { } var _ = Describe("Client Crypto Setup", func() { - var cs *cryptoSetupClient - var certManager *mockCertManager - var stream *mockStream - var keyDerivationCalledWith *keyDerivationValues - var shloMap map[Tag][]byte + var ( + cs *cryptoSetupClient + certManager *mockCertManager + stream *mockStream + keyDerivationCalledWith *keyDerivationValues + shloMap map[Tag][]byte + aeadChanged chan protocol.EncryptionLevel + ) BeforeEach(func() { shloMap = map[Tag][]byte{ @@ -101,7 +104,8 @@ var _ = Describe("Client Crypto Setup", func() { stream = &mockStream{} certManager = &mockCertManager{} version := protocol.Version36 - csInt, err := NewCryptoSetupClient("hostname", 0, version, stream, nil, NewConnectionParamatersManager(protocol.PerspectiveClient, version), make(chan protocol.EncryptionLevel, 2), nil) + aeadChanged = make(chan protocol.EncryptionLevel, 2) + csInt, err := NewCryptoSetupClient("hostname", 0, version, stream, nil, NewConnectionParamatersManager(protocol.PerspectiveClient, version), aeadChanged, nil) Expect(err).ToNot(HaveOccurred()) cs = csInt.(*cryptoSetupClient) cs.certManager = certManager @@ -369,22 +373,22 @@ var _ = Describe("Client Crypto Setup", func() { cs.receivedSecurePacket = false err := cs.handleSHLOMessage(shloMap) Expect(err).To(MatchError(qerr.Error(qerr.CryptoEncryptionLevelIncorrect, "unencrypted SHLO message"))) - Expect(cs.aeadChanged).ToNot(Receive()) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).ToNot(Receive()) + Expect(aeadChanged).ToNot(BeClosed()) }) It("rejects SHLOs without a PUBS", func() { delete(shloMap, TagPUBS) err := cs.handleSHLOMessage(shloMap) Expect(err).To(MatchError(qerr.Error(qerr.CryptoMessageParameterNotFound, "PUBS"))) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).ToNot(BeClosed()) }) It("rejects SHLOs without a version list", func() { delete(shloMap, TagVER) err := cs.handleSHLOMessage(shloMap) Expect(err).To(MatchError(qerr.Error(qerr.InvalidCryptoMessageParameter, "server hello missing version list"))) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).ToNot(BeClosed()) }) It("accepts a SHLO after a version negotiation", func() { @@ -409,8 +413,8 @@ var _ = Describe("Client Crypto Setup", func() { err := cs.handleSHLOMessage(shloMap) Expect(err).ToNot(HaveOccurred()) Expect(cs.forwardSecureAEAD).ToNot(BeNil()) - Expect(cs.aeadChanged).To(Receive(Equal(protocol.EncryptionForwardSecure))) - Expect(cs.aeadChanged).To(BeClosed()) + Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionForwardSecure))) + Expect(aeadChanged).To(BeClosed()) }) It("reads the connection paramaters", func() { @@ -598,9 +602,9 @@ var _ = Describe("Client Crypto Setup", func() { Expect(keyDerivationCalledWith.cert).To(Equal(certManager.leafCert)) Expect(keyDerivationCalledWith.divNonce).To(Equal(cs.diversificationNonce)) Expect(keyDerivationCalledWith.pers).To(Equal(protocol.PerspectiveClient)) - Expect(cs.aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) - Expect(cs.aeadChanged).ToNot(Receive()) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) + Expect(aeadChanged).ToNot(Receive()) + Expect(aeadChanged).ToNot(BeClosed()) }) It("uses the server nonce, if the server sent one", func() { @@ -610,24 +614,24 @@ var _ = Describe("Client Crypto Setup", func() { Expect(err).ToNot(HaveOccurred()) Expect(cs.secureAEAD).ToNot(BeNil()) Expect(keyDerivationCalledWith.nonces).To(Equal(append(cs.nonc, cs.sno...))) - Expect(cs.aeadChanged).To(Receive()) - Expect(cs.aeadChanged).ToNot(Receive()) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).To(Receive()) + Expect(aeadChanged).ToNot(Receive()) + Expect(aeadChanged).ToNot(BeClosed()) }) It("doesn't create a secureAEAD if the certificate is not yet verified, even if it has all necessary values", func() { err := cs.maybeUpgradeCrypto() Expect(err).ToNot(HaveOccurred()) Expect(cs.secureAEAD).To(BeNil()) - Expect(cs.aeadChanged).ToNot(Receive()) + Expect(aeadChanged).ToNot(Receive()) cs.serverVerified = true // make sure we really had all necessary values before, and only serverVerified was missing err = cs.maybeUpgradeCrypto() Expect(err).ToNot(HaveOccurred()) Expect(cs.secureAEAD).ToNot(BeNil()) - Expect(cs.aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) - Expect(cs.aeadChanged).ToNot(Receive()) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) + Expect(aeadChanged).ToNot(Receive()) + Expect(aeadChanged).ToNot(BeClosed()) }) It("tries to escalate before reading a handshake message", func() { @@ -638,9 +642,9 @@ var _ = Describe("Client Crypto Setup", func() { // this is because the mockStream doesn't block if there's no data to read Expect(err).To(MatchError(qerr.HandshakeFailed)) Expect(cs.secureAEAD).ToNot(BeNil()) - Expect(cs.aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) - Expect(cs.aeadChanged).ToNot(Receive()) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) + Expect(aeadChanged).ToNot(Receive()) + Expect(aeadChanged).ToNot(BeClosed()) }) It("tries to escalate the crypto after receiving a diversification nonce", func() { @@ -650,9 +654,9 @@ var _ = Describe("Client Crypto Setup", func() { err := cs.SetDiversificationNonce([]byte("div")) Expect(err).ToNot(HaveOccurred()) Expect(cs.secureAEAD).ToNot(BeNil()) - Expect(cs.aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) - Expect(cs.aeadChanged).ToNot(Receive()) - Expect(cs.aeadChanged).ToNot(BeClosed()) + Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionSecure))) + Expect(aeadChanged).ToNot(Receive()) + Expect(aeadChanged).ToNot(BeClosed()) }) Context("null encryption", func() { diff --git a/handshake/crypto_setup_server.go b/handshake/crypto_setup_server.go index c046c123..7f5ef075 100644 --- a/handshake/crypto_setup_server.go +++ b/handshake/crypto_setup_server.go @@ -36,7 +36,7 @@ type cryptoSetupServer struct { receivedForwardSecurePacket bool sentSHLO bool receivedSecurePacket bool - aeadChanged chan protocol.EncryptionLevel + aeadChanged chan<- protocol.EncryptionLevel keyDerivation KeyDerivationFunction keyExchange KeyExchangeFunction @@ -64,7 +64,7 @@ func NewCryptoSetup( cryptoStream io.ReadWriter, connectionParametersManager ConnectionParametersManager, supportedVersions []protocol.VersionNumber, - aeadChanged chan protocol.EncryptionLevel, + aeadChanged chan<- protocol.EncryptionLevel, ) (CryptoSetup, error) { return &cryptoSetupServer{ connID: connID, diff --git a/handshake/crypto_setup_server_test.go b/handshake/crypto_setup_server_test.go index 6f2ab0d9..98d33226 100644 --- a/handshake/crypto_setup_server_test.go +++ b/handshake/crypto_setup_server_test.go @@ -595,9 +595,9 @@ var _ = Describe("Server Crypto Setup", func() { doCHLO() _, _, err := cs.Open(nil, []byte("forward secure encrypted"), 0, []byte{}) Expect(err).ToNot(HaveOccurred()) - Expect(cs.aeadChanged).To(Receive()) // consume the protocol.EncryptionSecure - Expect(cs.aeadChanged).To(Receive()) // consume the protocol.EncryptionForwardSecure - Expect(cs.aeadChanged).To(BeClosed()) + Expect(aeadChanged).To(Receive()) // consume the protocol.EncryptionSecure + Expect(aeadChanged).To(Receive()) // consume the protocol.EncryptionForwardSecure + Expect(aeadChanged).To(BeClosed()) }) })