From 73b9be78466931d24597fa107df9675c28a0b962 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Mon, 23 Mar 2015 15:49:48 +0700 Subject: Improve handling of extensions for session resumption --- crypto/TestResult.xml | 1985 +++++++++++++++++++++++++++ crypto/src/crypto/tls/AbstractTlsServer.cs | 4 + crypto/src/crypto/tls/DtlsClientProtocol.cs | 215 +-- crypto/src/crypto/tls/DtlsProtocol.cs | 28 +- crypto/src/crypto/tls/DtlsServerProtocol.cs | 107 +- crypto/src/crypto/tls/TlsClientProtocol.cs | 81 +- crypto/src/crypto/tls/TlsProtocol.cs | 30 +- crypto/src/crypto/tls/TlsServerProtocol.cs | 32 +- 8 files changed, 2258 insertions(+), 224 deletions(-) create mode 100644 crypto/TestResult.xml (limited to 'crypto') diff --git a/crypto/TestResult.xml b/crypto/TestResult.xml new file mode 100644 index 000000000..047a66cb3 --- /dev/null +++ b/crypto/TestResult.xml @@ -0,0 +1,1985 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/crypto/src/crypto/tls/AbstractTlsServer.cs b/crypto/src/crypto/tls/AbstractTlsServer.cs index 7fe3fcbe5..c3e250fd8 100644 --- a/crypto/src/crypto/tls/AbstractTlsServer.cs +++ b/crypto/src/crypto/tls/AbstractTlsServer.cs @@ -140,7 +140,11 @@ namespace Org.BouncyCastle.Crypto.Tls if (clientExtensions != null) { this.mEncryptThenMacOffered = TlsExtensionsUtilities.HasEncryptThenMacExtension(clientExtensions); + this.mMaxFragmentLengthOffered = TlsExtensionsUtilities.GetMaxFragmentLengthExtension(clientExtensions); + if (mMaxFragmentLengthOffered >= 0 && !MaxFragmentLength.IsValid((byte)mMaxFragmentLengthOffered)) + throw new TlsFatalAlert(AlertDescription.illegal_parameter); + this.mTruncatedHMacOffered = TlsExtensionsUtilities.HasTruncatedHMacExtension(clientExtensions); this.mSupportedSignatureAlgorithms = TlsUtilities.GetSignatureAlgorithmsExtension(clientExtensions); diff --git a/crypto/src/crypto/tls/DtlsClientProtocol.cs b/crypto/src/crypto/tls/DtlsClientProtocol.cs index 76635065c..411e7cca2 100644 --- a/crypto/src/crypto/tls/DtlsClientProtocol.cs +++ b/crypto/src/crypto/tls/DtlsClientProtocol.cs @@ -112,41 +112,12 @@ namespace Org.BouncyCastle.Crypto.Tls throw new TlsFatalAlert(AlertDescription.unexpected_message); } - if (state.maxFragmentLength >= 0) - { - int plainTextLimit = 1 << (8 + state.maxFragmentLength); - recordLayer.SetPlaintextLimit(plainTextLimit); - } - - securityParameters.cipherSuite = state.selectedCipherSuite; - securityParameters.compressionAlgorithm = (byte)state.selectedCompressionMethod; - securityParameters.prfAlgorithm = TlsProtocol.GetPrfAlgorithm(state.clientContext, state.selectedCipherSuite); - - /* - * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify verify_data_length has - * a verify_data_length equal to 12. This includes all existing cipher suites. - */ - securityParameters.verifyDataLength = 12; - handshake.NotifyHelloComplete(); - bool resumedSession = state.selectedSessionID.Length > 0 && state.tlsSession != null - && Arrays.AreEqual(state.selectedSessionID, state.tlsSession.SessionID); + ApplyMaxFragmentLengthExtension(recordLayer, securityParameters.maxFragmentLength); - if (resumedSession) + if (state.resumedSession) { - if (securityParameters.CipherSuite != state.sessionParameters.CipherSuite - || securityParameters.CompressionAlgorithm != state.sessionParameters.CompressionAlgorithm) - { - throw new TlsFatalAlert(AlertDescription.illegal_parameter); - } - - IDictionary sessionServerExtensions = state.sessionParameters.ReadServerExtensions(); - - // TODO Check encrypt-then-MAC extension and maybe others - - securityParameters.extendedMasterSecret = TlsExtensionsUtilities.HasExtendedMasterSecretExtension(sessionServerExtensions); - securityParameters.masterSecret = Arrays.Clone(state.sessionParameters.MasterSecret); recordLayer.InitPendingEpoch(state.client.GetCipher()); @@ -366,12 +337,14 @@ namespace Org.BouncyCastle.Crypto.Tls if (state.tlsSession != null) { state.sessionParameters = new SessionParameters.Builder() - .SetCipherSuite(securityParameters.cipherSuite) - .SetCompressionAlgorithm(securityParameters.compressionAlgorithm) - .SetMasterSecret(securityParameters.masterSecret) + .SetCipherSuite(securityParameters.CipherSuite) + .SetCompressionAlgorithm(securityParameters.CompressionAlgorithm) + .SetMasterSecret(securityParameters.MasterSecret) .SetPeerCertificate(serverCertificate) - .SetPskIdentity(securityParameters.pskIdentity) - .SetSrpIdentity(securityParameters.srpIdentity) + .SetPskIdentity(securityParameters.PskIdentity) + .SetSrpIdentity(securityParameters.SrpIdentity) + // TODO Consider filtering extensions that aren't relevant to resumed sessions + .SetServerExtensions(state.serverExtensions) .Build(); state.tlsSession = TlsUtilities.ImportSession(state.tlsSession.SessionID, state.sessionParameters); @@ -599,8 +572,10 @@ namespace Org.BouncyCastle.Crypto.Tls MemoryStream buf = new MemoryStream(body, false); - ProtocolVersion server_version = TlsUtilities.ReadVersion(buf); - ReportServerVersion(state, server_version); + { + ProtocolVersion server_version = TlsUtilities.ReadVersion(buf); + ReportServerVersion(state, server_version); + } securityParameters.serverRandom = TlsUtilities.ReadFully(32, buf); @@ -608,24 +583,24 @@ namespace Org.BouncyCastle.Crypto.Tls if (state.selectedSessionID.Length > 32) throw new TlsFatalAlert(AlertDescription.illegal_parameter); state.client.NotifySessionID(state.selectedSessionID); + state.resumedSession = state.selectedSessionID.Length > 0 && state.tlsSession != null + && Arrays.AreEqual(state.selectedSessionID, state.tlsSession.SessionID); - state.selectedCipherSuite = TlsUtilities.ReadUint16(buf); - if (!Arrays.Contains(state.offeredCipherSuites, state.selectedCipherSuite) - || state.selectedCipherSuite == CipherSuite.TLS_NULL_WITH_NULL_NULL - || CipherSuite.IsScsv(state.selectedCipherSuite) - || !TlsUtilities.IsValidCipherSuiteForVersion(state.selectedCipherSuite, server_version)) + int selectedCipherSuite = TlsUtilities.ReadUint16(buf); + if (!Arrays.Contains(state.offeredCipherSuites, selectedCipherSuite) + || selectedCipherSuite == CipherSuite.TLS_NULL_WITH_NULL_NULL + || CipherSuite.IsScsv(selectedCipherSuite) + || !TlsUtilities.IsValidCipherSuiteForVersion(selectedCipherSuite, state.clientContext.ServerVersion)) { throw new TlsFatalAlert(AlertDescription.illegal_parameter); } + ValidateSelectedCipherSuite(selectedCipherSuite, AlertDescription.illegal_parameter); + state.client.NotifySelectedCipherSuite(selectedCipherSuite); - ValidateSelectedCipherSuite(state.selectedCipherSuite, AlertDescription.illegal_parameter); - - state.client.NotifySelectedCipherSuite(state.selectedCipherSuite); - - state.selectedCompressionMethod = TlsUtilities.ReadUint8(buf); - if (!Arrays.Contains(state.offeredCompressionMethods, (byte)state.selectedCompressionMethod)) + byte selectedCompressionMethod = TlsUtilities.ReadUint8(buf); + if (!Arrays.Contains(state.offeredCompressionMethods, selectedCompressionMethod)) throw new TlsFatalAlert(AlertDescription.illegal_parameter); - state.client.NotifySelectedCompressionMethod((byte)state.selectedCompressionMethod); + state.client.NotifySelectedCompressionMethod(selectedCompressionMethod); /* * RFC3546 2.2 The extended server hello message format MAY be sent in place of the server @@ -643,16 +618,16 @@ namespace Org.BouncyCastle.Crypto.Tls */ // Integer -> byte[] - IDictionary serverExtensions = TlsProtocol.ReadExtensions(buf); + state.serverExtensions = TlsProtocol.ReadExtensions(buf); /* * RFC 3546 2.2 Note that the extended server hello message is only sent in response to an * extended client hello message. However, see RFC 5746 exception below. We always include * the SCSV, so an Extended Server Hello is always allowed. */ - if (serverExtensions != null) + if (state.serverExtensions != null) { - foreach (int extType in serverExtensions.Keys) + foreach (int extType in state.serverExtensions.Keys) { /* * RFC 5746 3.6. Note that sending a "renegotiation_info" extension in response to a @@ -679,64 +654,92 @@ namespace Org.BouncyCastle.Crypto.Tls * extensions appearing in the client hello, and send a server hello containing no * extensions[.] */ - // TODO[sessions] - // if (this.mResumedSession) - // { - // // TODO[compat-gnutls] GnuTLS test server sends server extensions e.g. ec_point_formats - // // TODO[compat-openssl] OpenSSL test server sends server extensions e.g. ec_point_formats - // // TODO[compat-polarssl] PolarSSL test server sends server extensions e.g. ec_point_formats - //// throw new TlsFatalAlert(AlertDescription.illegal_parameter); - // } + if (state.resumedSession) + { + // TODO[compat-gnutls] GnuTLS test server sends server extensions e.g. ec_point_formats + // TODO[compat-openssl] OpenSSL test server sends server extensions e.g. ec_point_formats + // TODO[compat-polarssl] PolarSSL test server sends server extensions e.g. ec_point_formats + //throw new TlsFatalAlert(AlertDescription.illegal_parameter); + } } + } + /* + * RFC 5746 3.4. Client Behavior: Initial Handshake + */ + { /* - * RFC 5746 3.4. Client Behavior: Initial Handshake + * When a ServerHello is received, the client MUST check if it includes the + * "renegotiation_info" extension: */ + byte[] renegExtData = TlsUtilities.GetExtensionData(state.serverExtensions, ExtensionType.renegotiation_info); + if (renegExtData != null) { /* - * When a ServerHello is received, the client MUST check if it includes the - * "renegotiation_info" extension: - */ - byte[] renegExtData = (byte[])serverExtensions[ExtensionType.renegotiation_info]; - if (renegExtData != null) - { - /* - * If the extension is present, set the secure_renegotiation flag to TRUE. The - * client MUST then verify that the length of the "renegotiated_connection" - * field is zero, and if it is not, MUST abort the handshake (by sending a fatal - * handshake_failure alert). - */ - state.secure_renegotiation = true; - - if (!Arrays.ConstantTimeAreEqual(renegExtData, TlsProtocol.CreateRenegotiationInfo(TlsUtilities.EmptyBytes))) - throw new TlsFatalAlert(AlertDescription.handshake_failure); - } + * If the extension is present, set the secure_renegotiation flag to TRUE. The + * client MUST then verify that the length of the "renegotiated_connection" + * field is zero, and if it is not, MUST abort the handshake (by sending a fatal + * handshake_failure alert). + */ + state.secure_renegotiation = true; + + if (!Arrays.ConstantTimeAreEqual(renegExtData, TlsProtocol.CreateRenegotiationInfo(TlsUtilities.EmptyBytes))) + throw new TlsFatalAlert(AlertDescription.handshake_failure); } + } - /* - * RFC 7366 3. If a server receives an encrypt-then-MAC request extension from a client - * and then selects a stream or Authenticated Encryption with Associated Data (AEAD) - * ciphersuite, it MUST NOT send an encrypt-then-MAC response extension back to the - * client. - */ - bool serverSentEncryptThenMAC = TlsExtensionsUtilities.HasEncryptThenMacExtension(serverExtensions); - if (serverSentEncryptThenMAC && !TlsUtilities.IsBlockCipherSuite(state.selectedCipherSuite)) + // TODO[compat-gnutls] GnuTLS test server fails to send renegotiation_info extension when resuming + state.client.NotifySecureRenegotiation(state.secure_renegotiation); + + IDictionary sessionClientExtensions = state.clientExtensions, sessionServerExtensions = state.serverExtensions; + if (state.resumedSession) + { + if (selectedCipherSuite != state.sessionParameters.CipherSuite + || selectedCompressionMethod != state.sessionParameters.CompressionAlgorithm) + { throw new TlsFatalAlert(AlertDescription.illegal_parameter); + } - securityParameters.encryptThenMac = serverSentEncryptThenMAC; + sessionClientExtensions = null; + sessionServerExtensions = state.sessionParameters.ReadServerExtensions(); + } - securityParameters.extendedMasterSecret = TlsExtensionsUtilities.HasExtendedMasterSecretExtension(serverExtensions); + securityParameters.cipherSuite = selectedCipherSuite; + securityParameters.compressionAlgorithm = selectedCompressionMethod; - state.maxFragmentLength = EvaluateMaxFragmentLengthExtension(state.clientExtensions, serverExtensions, - AlertDescription.illegal_parameter); + if (sessionServerExtensions != null) + { + { + /* + * RFC 7366 3. If a server receives an encrypt-then-MAC request extension from a client + * and then selects a stream or Authenticated Encryption with Associated Data (AEAD) + * ciphersuite, it MUST NOT send an encrypt-then-MAC response extension back to the + * client. + */ + bool serverSentEncryptThenMAC = TlsExtensionsUtilities.HasEncryptThenMacExtension(sessionServerExtensions); + if (serverSentEncryptThenMAC && !TlsUtilities.IsBlockCipherSuite(securityParameters.CipherSuite)) + throw new TlsFatalAlert(AlertDescription.illegal_parameter); + securityParameters.encryptThenMac = serverSentEncryptThenMAC; + } - securityParameters.truncatedHMac = TlsExtensionsUtilities.HasTruncatedHMacExtension(serverExtensions); + securityParameters.extendedMasterSecret = TlsExtensionsUtilities.HasExtendedMasterSecretExtension(sessionServerExtensions); + + securityParameters.maxFragmentLength = EvaluateMaxFragmentLengthExtension(state.resumedSession, + sessionClientExtensions, sessionServerExtensions, AlertDescription.illegal_parameter); - state.allowCertificateStatus = TlsUtilities.HasExpectedEmptyExtensionData(serverExtensions, - ExtensionType.status_request, AlertDescription.illegal_parameter); + securityParameters.truncatedHMac = TlsExtensionsUtilities.HasTruncatedHMacExtension(sessionServerExtensions); + + /* + * TODO It's surprising that there's no provision to allow a 'fresh' CertificateStatus to be + * sent in a session resumption handshake. + */ + state.allowCertificateStatus = !state.resumedSession + && TlsUtilities.HasExpectedEmptyExtensionData(sessionServerExtensions, ExtensionType.status_request, + AlertDescription.illegal_parameter); - state.expectSessionTicket = TlsUtilities.HasExpectedEmptyExtensionData(serverExtensions, - ExtensionType.session_ticket, AlertDescription.illegal_parameter); + state.expectSessionTicket = !state.resumedSession + && TlsUtilities.HasExpectedEmptyExtensionData(sessionServerExtensions, ExtensionType.session_ticket, + AlertDescription.illegal_parameter); } /* @@ -746,12 +749,19 @@ namespace Org.BouncyCastle.Crypto.Tls * that do not use the extended master secret [..]. (and see 5.2, 5.3) */ - state.client.NotifySecureRenegotiation(state.secure_renegotiation); - - if (state.clientExtensions != null) + if (sessionClientExtensions != null) { - state.client.ProcessServerExtensions(serverExtensions); + state.client.ProcessServerExtensions(sessionServerExtensions); } + + securityParameters.prfAlgorithm = TlsProtocol.GetPrfAlgorithm(state.clientContext, + securityParameters.CipherSuite); + + /* + * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify verify_data_length has + * a verify_data_length equal to 12. This includes all existing cipher suites. + */ + securityParameters.verifyDataLength = 12; } protected virtual void ProcessServerKeyExchange(ClientHandshakeState state, byte[] body) @@ -813,11 +823,10 @@ namespace Org.BouncyCastle.Crypto.Tls internal int[] offeredCipherSuites = null; internal byte[] offeredCompressionMethods = null; internal IDictionary clientExtensions = null; + internal IDictionary serverExtensions = null; internal byte[] selectedSessionID = null; - internal int selectedCipherSuite = -1; - internal short selectedCompressionMethod = -1; + internal bool resumedSession = false; internal bool secure_renegotiation = false; - internal short maxFragmentLength = -1; internal bool allowCertificateStatus = false; internal bool expectSessionTicket = false; internal TlsKeyExchange keyExchange = null; diff --git a/crypto/src/crypto/tls/DtlsProtocol.cs b/crypto/src/crypto/tls/DtlsProtocol.cs index 6d62c5a90..e4ebd436c 100644 --- a/crypto/src/crypto/tls/DtlsProtocol.cs +++ b/crypto/src/crypto/tls/DtlsProtocol.cs @@ -33,12 +33,32 @@ namespace Org.BouncyCastle.Crypto.Tls } /// - protected static short EvaluateMaxFragmentLengthExtension(IDictionary clientExtensions, IDictionary serverExtensions, - byte alertDescription) + internal static void ApplyMaxFragmentLengthExtension(DtlsRecordLayer recordLayer, short maxFragmentLength) + { + if (maxFragmentLength >= 0) + { + if (!MaxFragmentLength.IsValid((byte)maxFragmentLength)) + throw new TlsFatalAlert(AlertDescription.internal_error); + + int plainTextLimit = 1 << (8 + maxFragmentLength); + recordLayer.SetPlaintextLimit(plainTextLimit); + } + } + + /// + protected static short EvaluateMaxFragmentLengthExtension(bool resumedSession, IDictionary clientExtensions, + IDictionary serverExtensions, byte alertDescription) { short maxFragmentLength = TlsExtensionsUtilities.GetMaxFragmentLengthExtension(serverExtensions); - if (maxFragmentLength >= 0 && maxFragmentLength != TlsExtensionsUtilities.GetMaxFragmentLengthExtension(clientExtensions)) - throw new TlsFatalAlert(alertDescription); + if (maxFragmentLength >= 0) + { + if (!MaxFragmentLength.IsValid((byte)maxFragmentLength) + || (!resumedSession && maxFragmentLength != TlsExtensionsUtilities + .GetMaxFragmentLengthExtension(clientExtensions))) + { + throw new TlsFatalAlert(alertDescription); + } + } return maxFragmentLength; } diff --git a/crypto/src/crypto/tls/DtlsServerProtocol.cs b/crypto/src/crypto/tls/DtlsServerProtocol.cs index f148eb7d7..9c7caf290 100644 --- a/crypto/src/crypto/tls/DtlsServerProtocol.cs +++ b/crypto/src/crypto/tls/DtlsServerProtocol.cs @@ -94,24 +94,9 @@ namespace Org.BouncyCastle.Crypto.Tls { byte[] serverHelloBody = GenerateServerHello(state); - - if (state.maxFragmentLength >= 0) - { - int plainTextLimit = 1 << (8 + state.maxFragmentLength); - recordLayer.SetPlaintextLimit(plainTextLimit); - } - - securityParameters.cipherSuite = state.selectedCipherSuite; - securityParameters.compressionAlgorithm = (byte)state.selectedCompressionMethod; - securityParameters.prfAlgorithm = TlsProtocol.GetPrfAlgorithm(state.serverContext, - state.selectedCipherSuite); - /* - * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify verify_data_length - * has a verify_data_length equal to 12. This includes all existing cipher suites. - */ - securityParameters.verifyDataLength = 12; - + ApplyMaxFragmentLengthExtension(recordLayer, securityParameters.maxFragmentLength); + handshake.SendMessage(HandshakeType.server_hello, serverHelloBody); } @@ -302,17 +287,19 @@ namespace Org.BouncyCastle.Crypto.Tls MemoryStream buf = new MemoryStream(); - ProtocolVersion server_version = state.server.GetServerVersion(); - if (!server_version.IsEqualOrEarlierVersionOf(state.serverContext.ClientVersion)) - throw new TlsFatalAlert(AlertDescription.internal_error); + { + ProtocolVersion server_version = state.server.GetServerVersion(); + if (!server_version.IsEqualOrEarlierVersionOf(state.serverContext.ClientVersion)) + throw new TlsFatalAlert(AlertDescription.internal_error); - // TODO Read RFCs for guidance on the expected record layer version number - // recordStream.setReadVersion(server_version); - // recordStream.setWriteVersion(server_version); - // recordStream.setRestrictReadVersion(true); - state.serverContext.SetServerVersion(server_version); + // TODO Read RFCs for guidance on the expected record layer version number + // recordStream.setReadVersion(server_version); + // recordStream.setWriteVersion(server_version); + // recordStream.setRestrictReadVersion(true); + state.serverContext.SetServerVersion(server_version); - TlsUtilities.WriteVersion(state.serverContext.ServerVersion, buf); + TlsUtilities.WriteVersion(state.serverContext.ServerVersion, buf); + } buf.Write(securityParameters.ServerRandom, 0, securityParameters.ServerRandom.Length); @@ -322,23 +309,24 @@ namespace Org.BouncyCastle.Crypto.Tls */ TlsUtilities.WriteOpaque8(TlsUtilities.EmptyBytes, buf); - state.selectedCipherSuite = state.server.GetSelectedCipherSuite(); - if (!Arrays.Contains(state.offeredCipherSuites, state.selectedCipherSuite) - || state.selectedCipherSuite == CipherSuite.TLS_NULL_WITH_NULL_NULL - || CipherSuite.IsScsv(state.selectedCipherSuite) - || !TlsUtilities.IsValidCipherSuiteForVersion(state.selectedCipherSuite, server_version)) + int selectedCipherSuite = state.server.GetSelectedCipherSuite(); + if (!Arrays.Contains(state.offeredCipherSuites, selectedCipherSuite) + || selectedCipherSuite == CipherSuite.TLS_NULL_WITH_NULL_NULL + || CipherSuite.IsScsv(selectedCipherSuite) + || !TlsUtilities.IsValidCipherSuiteForVersion(selectedCipherSuite, state.serverContext.ServerVersion)) { throw new TlsFatalAlert(AlertDescription.internal_error); } + ValidateSelectedCipherSuite(selectedCipherSuite, AlertDescription.internal_error); + securityParameters.cipherSuite = selectedCipherSuite; - ValidateSelectedCipherSuite(state.selectedCipherSuite, AlertDescription.internal_error); - - state.selectedCompressionMethod = state.server.GetSelectedCompressionMethod(); - if (!Arrays.Contains(state.offeredCompressionMethods, (byte)state.selectedCompressionMethod)) + byte selectedCompressionMethod = state.server.GetSelectedCompressionMethod(); + if (!Arrays.Contains(state.offeredCompressionMethods, selectedCompressionMethod)) throw new TlsFatalAlert(AlertDescription.internal_error); + securityParameters.compressionAlgorithm = selectedCompressionMethod; - TlsUtilities.WriteUint16(state.selectedCipherSuite, buf); - TlsUtilities.WriteUint8((byte)state.selectedCompressionMethod, buf); + TlsUtilities.WriteUint16(selectedCipherSuite, buf); + TlsUtilities.WriteUint8(selectedCompressionMethod, buf); state.serverExtensions = state.server.GetServerExtensions(); @@ -375,24 +363,45 @@ namespace Org.BouncyCastle.Crypto.Tls TlsExtensionsUtilities.AddExtendedMasterSecretExtension(state.serverExtensions); } + /* + * TODO RFC 3546 2.3 If [...] the older session is resumed, then the server MUST ignore + * extensions appearing in the client hello, and send a server hello containing no + * extensions. + */ + if (state.serverExtensions != null) { securityParameters.encryptThenMac = TlsExtensionsUtilities.HasEncryptThenMacExtension(state.serverExtensions); - state.maxFragmentLength = EvaluateMaxFragmentLengthExtension(state.clientExtensions, state.serverExtensions, - AlertDescription.internal_error); + securityParameters.maxFragmentLength = EvaluateMaxFragmentLengthExtension(state.resumedSession, + state.clientExtensions, state.serverExtensions, AlertDescription.internal_error); securityParameters.truncatedHMac = TlsExtensionsUtilities.HasTruncatedHMacExtension(state.serverExtensions); - state.allowCertificateStatus = TlsUtilities.HasExpectedEmptyExtensionData(state.serverExtensions, - ExtensionType.status_request, AlertDescription.internal_error); + /* + * TODO It's surprising that there's no provision to allow a 'fresh' CertificateStatus to be sent in + * a session resumption handshake. + */ + state.allowCertificateStatus = !state.resumedSession + && TlsUtilities.HasExpectedEmptyExtensionData(state.serverExtensions, ExtensionType.status_request, + AlertDescription.internal_error); - state.expectSessionTicket = TlsUtilities.HasExpectedEmptyExtensionData(state.serverExtensions, - ExtensionType.session_ticket, AlertDescription.internal_error); + state.expectSessionTicket = !state.resumedSession + && TlsUtilities.HasExpectedEmptyExtensionData(state.serverExtensions, ExtensionType.session_ticket, + AlertDescription.internal_error); TlsProtocol.WriteExtensions(buf, state.serverExtensions); } + securityParameters.prfAlgorithm = TlsProtocol.GetPrfAlgorithm(state.serverContext, + securityParameters.CipherSuite); + + /* + * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify verify_data_length + * has a verify_data_length equal to 12. This includes all existing cipher suites. + */ + securityParameters.verifyDataLength = 12; + return buf.ToArray(); } @@ -628,16 +637,14 @@ namespace Org.BouncyCastle.Crypto.Tls { internal TlsServer server = null; internal TlsServerContextImpl serverContext = null; - internal int[] offeredCipherSuites; - internal byte[] offeredCompressionMethods; - internal IDictionary clientExtensions; - internal int selectedCipherSuite = -1; - internal short selectedCompressionMethod = -1; + internal int[] offeredCipherSuites = null; + internal byte[] offeredCompressionMethods = null; + internal IDictionary clientExtensions = null; + internal IDictionary serverExtensions = null; + internal bool resumedSession = false; internal bool secure_renegotiation = false; - internal short maxFragmentLength = -1; internal bool allowCertificateStatus = false; internal bool expectSessionTicket = false; - internal IDictionary serverExtensions = null; internal TlsKeyExchange keyExchange = null; internal TlsCredentials serverCredentials = null; internal CertificateRequest certificateRequest = null; diff --git a/crypto/src/crypto/tls/TlsClientProtocol.cs b/crypto/src/crypto/tls/TlsClientProtocol.cs index 5b9e81b3f..7b8439acc 100644 --- a/crypto/src/crypto/tls/TlsClientProtocol.cs +++ b/crypto/src/crypto/tls/TlsClientProtocol.cs @@ -224,24 +224,10 @@ namespace Org.BouncyCastle.Crypto.Tls ReceiveServerHelloMessage(buf); this.mConnectionState = CS_SERVER_HELLO; - if (this.mSecurityParameters.maxFragmentLength >= 0) - { - int plainTextLimit = 1 << (8 + this.mSecurityParameters.maxFragmentLength); - mRecordStream.SetPlaintextLimit(plainTextLimit); - } - - this.mSecurityParameters.prfAlgorithm = GetPrfAlgorithm(Context, - this.mSecurityParameters.CipherSuite); - - /* - * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify - * verify_data_length has a verify_data_length equal to 12. This includes all - * existing cipher suites. - */ - this.mSecurityParameters.verifyDataLength = 12; - this.mRecordStream.NotifyHelloComplete(); + ApplyMaxFragmentLengthExtension(); + if (this.mResumedSession) { this.mSecurityParameters.masterSecret = Arrays.Clone(this.mSessionParameters.MasterSecret); @@ -558,21 +544,23 @@ namespace Org.BouncyCastle.Crypto.Tls protected virtual void ReceiveServerHelloMessage(MemoryStream buf) { - ProtocolVersion server_version = TlsUtilities.ReadVersion(buf); - if (server_version.IsDtls) - throw new TlsFatalAlert(AlertDescription.illegal_parameter); + { + ProtocolVersion server_version = TlsUtilities.ReadVersion(buf); + if (server_version.IsDtls) + throw new TlsFatalAlert(AlertDescription.illegal_parameter); - // Check that this matches what the server is Sending in the record layer - if (!server_version.Equals(this.mRecordStream.ReadVersion)) - throw new TlsFatalAlert(AlertDescription.illegal_parameter); + // Check that this matches what the server is Sending in the record layer + if (!server_version.Equals(this.mRecordStream.ReadVersion)) + throw new TlsFatalAlert(AlertDescription.illegal_parameter); - ProtocolVersion client_version = Context.ClientVersion; - if (!server_version.IsEqualOrEarlierVersionOf(client_version)) - throw new TlsFatalAlert(AlertDescription.illegal_parameter); + ProtocolVersion client_version = Context.ClientVersion; + if (!server_version.IsEqualOrEarlierVersionOf(client_version)) + throw new TlsFatalAlert(AlertDescription.illegal_parameter); - this.mRecordStream.SetWriteVersion(server_version); - ContextAdmin.SetServerVersion(server_version); - this.mTlsClient.NotifyServerVersion(server_version); + this.mRecordStream.SetWriteVersion(server_version); + ContextAdmin.SetServerVersion(server_version); + this.mTlsClient.NotifyServerVersion(server_version); + } /* * Read the server random @@ -582,9 +570,7 @@ namespace Org.BouncyCastle.Crypto.Tls this.mSelectedSessionID = TlsUtilities.ReadOpaque8(buf); if (this.mSelectedSessionID.Length > 32) throw new TlsFatalAlert(AlertDescription.illegal_parameter); - this.mTlsClient.NotifySessionID(this.mSelectedSessionID); - this.mResumedSession = this.mSelectedSessionID.Length > 0 && this.mTlsSession != null && Arrays.AreEqual(this.mSelectedSessionID, this.mTlsSession.SessionID); @@ -596,11 +582,10 @@ namespace Org.BouncyCastle.Crypto.Tls if (!Arrays.Contains(this.mOfferedCipherSuites, selectedCipherSuite) || selectedCipherSuite == CipherSuite.TLS_NULL_WITH_NULL_NULL || CipherSuite.IsScsv(selectedCipherSuite) - || !TlsUtilities.IsValidCipherSuiteForVersion(selectedCipherSuite, server_version)) + || !TlsUtilities.IsValidCipherSuiteForVersion(selectedCipherSuite, Context.ServerVersion)) { throw new TlsFatalAlert(AlertDescription.illegal_parameter); } - this.mTlsClient.NotifySelectedCipherSuite(selectedCipherSuite); /* @@ -610,7 +595,6 @@ namespace Org.BouncyCastle.Crypto.Tls byte selectedCompressionMethod = TlsUtilities.ReadUint8(buf); if (!Arrays.Contains(this.mOfferedCompressionMethods, selectedCompressionMethod)) throw new TlsFatalAlert(AlertDescription.illegal_parameter); - this.mTlsClient.NotifySelectedCompressionMethod(selectedCompressionMethod); /* @@ -714,17 +698,19 @@ namespace Org.BouncyCastle.Crypto.Tls if (sessionServerExtensions != null) { - /* - * RFC 7366 3. If a server receives an encrypt-then-MAC request extension from a client - * and then selects a stream or Authenticated Encryption with Associated Data (AEAD) - * ciphersuite, it MUST NOT send an encrypt-then-MAC response extension back to the - * client. - */ - bool serverSentEncryptThenMAC = TlsExtensionsUtilities.HasEncryptThenMacExtension(sessionServerExtensions); - if (serverSentEncryptThenMAC && !TlsUtilities.IsBlockCipherSuite(selectedCipherSuite)) - throw new TlsFatalAlert(AlertDescription.illegal_parameter); + { + /* + * RFC 7366 3. If a server receives an encrypt-then-MAC request extension from a client + * and then selects a stream or Authenticated Encryption with Associated Data (AEAD) + * ciphersuite, it MUST NOT send an encrypt-then-MAC response extension back to the + * client. + */ + bool serverSentEncryptThenMAC = TlsExtensionsUtilities.HasEncryptThenMacExtension(sessionServerExtensions); + if (serverSentEncryptThenMAC && !TlsUtilities.IsBlockCipherSuite(selectedCipherSuite)) + throw new TlsFatalAlert(AlertDescription.illegal_parameter); - this.mSecurityParameters.encryptThenMac = serverSentEncryptThenMAC; + this.mSecurityParameters.encryptThenMac = serverSentEncryptThenMAC; + } this.mSecurityParameters.extendedMasterSecret = TlsExtensionsUtilities.HasExtendedMasterSecretExtension(sessionServerExtensions); @@ -757,6 +743,15 @@ namespace Org.BouncyCastle.Crypto.Tls { this.mTlsClient.ProcessServerExtensions(sessionServerExtensions); } + + this.mSecurityParameters.prfAlgorithm = GetPrfAlgorithm(Context, this.mSecurityParameters.CipherSuite); + + /* + * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify + * verify_data_length has a verify_data_length equal to 12. This includes all + * existing cipher suites. + */ + this.mSecurityParameters.verifyDataLength = 12; } protected virtual void SendCertificateVerifyMessage(DigitallySigned certificateVerify) diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index 4ea72cd57..8eb7beb3f 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -99,6 +99,18 @@ namespace Org.BouncyCastle.Crypto.Tls { } + protected virtual void ApplyMaxFragmentLengthExtension() + { + if (mSecurityParameters.maxFragmentLength >= 0) + { + if (!MaxFragmentLength.IsValid((byte)mSecurityParameters.maxFragmentLength)) + throw new TlsFatalAlert(AlertDescription.internal_error); + + int plainTextLimit = 1 << (8 + mSecurityParameters.maxFragmentLength); + mRecordStream.SetPlaintextLimit(plainTextLimit); + } + } + protected virtual void CheckReceivedChangeCipherSpec(bool expected) { if (expected != mReceivedChangeCipherSpec) @@ -164,12 +176,12 @@ namespace Org.BouncyCastle.Crypto.Tls if (this.mSessionParameters == null) { this.mSessionParameters = new SessionParameters.Builder() - .SetCipherSuite(this.mSecurityParameters.cipherSuite) - .SetCompressionAlgorithm(this.mSecurityParameters.compressionAlgorithm) - .SetMasterSecret(this.mSecurityParameters.masterSecret) + .SetCipherSuite(this.mSecurityParameters.CipherSuite) + .SetCompressionAlgorithm(this.mSecurityParameters.CompressionAlgorithm) + .SetMasterSecret(this.mSecurityParameters.MasterSecret) .SetPeerCertificate(this.mPeerCertificate) - .SetPskIdentity(this.mSecurityParameters.pskIdentity) - .SetSrpIdentity(this.mSecurityParameters.srpIdentity) + .SetPskIdentity(this.mSecurityParameters.PskIdentity) + .SetSrpIdentity(this.mSecurityParameters.SrpIdentity) // TODO Consider filtering extensions that aren't relevant to resumed sessions .SetServerExtensions(this.mServerExtensions) .Build(); @@ -761,10 +773,14 @@ namespace Org.BouncyCastle.Crypto.Tls byte alertDescription) { short maxFragmentLength = TlsExtensionsUtilities.GetMaxFragmentLengthExtension(serverExtensions); - if (maxFragmentLength >= 0 && !this.mResumedSession) + if (maxFragmentLength >= 0) { - if (maxFragmentLength != TlsExtensionsUtilities.GetMaxFragmentLengthExtension(clientExtensions)) + if (!MaxFragmentLength.IsValid((byte)maxFragmentLength) + || (!this.mResumedSession && maxFragmentLength != TlsExtensionsUtilities + .GetMaxFragmentLengthExtension(clientExtensions))) + { throw new TlsFatalAlert(alertDescription); + } } return maxFragmentLength; } diff --git a/crypto/src/crypto/tls/TlsServerProtocol.cs b/crypto/src/crypto/tls/TlsServerProtocol.cs index fd6808382..b73cb5a30 100644 --- a/crypto/src/crypto/tls/TlsServerProtocol.cs +++ b/crypto/src/crypto/tls/TlsServerProtocol.cs @@ -106,6 +106,8 @@ namespace Org.BouncyCastle.Crypto.Tls SendServerHelloMessage(); this.mConnectionState = CS_SERVER_HELLO; + mRecordStream.NotifyHelloComplete(); + IList serverSupplementalData = mTlsServer.GetServerSupplementalData(); if (serverSupplementalData != null) { @@ -618,16 +620,18 @@ namespace Org.BouncyCastle.Crypto.Tls { HandshakeMessage message = new HandshakeMessage(HandshakeType.server_hello); - ProtocolVersion server_version = mTlsServer.GetServerVersion(); - if (!server_version.IsEqualOrEarlierVersionOf(Context.ClientVersion)) - throw new TlsFatalAlert(AlertDescription.internal_error); + { + ProtocolVersion server_version = mTlsServer.GetServerVersion(); + if (!server_version.IsEqualOrEarlierVersionOf(Context.ClientVersion)) + throw new TlsFatalAlert(AlertDescription.internal_error); - mRecordStream.ReadVersion = server_version; - mRecordStream.SetWriteVersion(server_version); - mRecordStream.SetRestrictReadVersion(true); - ContextAdmin.SetServerVersion(server_version); + mRecordStream.ReadVersion = server_version; + mRecordStream.SetWriteVersion(server_version); + mRecordStream.SetRestrictReadVersion(true); + ContextAdmin.SetServerVersion(server_version); - TlsUtilities.WriteVersion(server_version, message); + TlsUtilities.WriteVersion(server_version, message); + } message.Write(this.mSecurityParameters.serverRandom); @@ -641,7 +645,7 @@ namespace Org.BouncyCastle.Crypto.Tls if (!Arrays.Contains(mOfferedCipherSuites, selectedCipherSuite) || selectedCipherSuite == CipherSuite.TLS_NULL_WITH_NULL_NULL || CipherSuite.IsScsv(selectedCipherSuite) - || !TlsUtilities.IsValidCipherSuiteForVersion(selectedCipherSuite, server_version)) + || !TlsUtilities.IsValidCipherSuiteForVersion(selectedCipherSuite, Context.ServerVersion)) { throw new TlsFatalAlert(AlertDescription.internal_error); } @@ -722,12 +726,6 @@ namespace Org.BouncyCastle.Crypto.Tls WriteExtensions(message, this.mServerExtensions); } - if (mSecurityParameters.maxFragmentLength >= 0) - { - int plainTextLimit = 1 << (8 + mSecurityParameters.maxFragmentLength); - mRecordStream.SetPlaintextLimit(plainTextLimit); - } - mSecurityParameters.prfAlgorithm = GetPrfAlgorithm(Context, mSecurityParameters.CipherSuite); /* @@ -736,9 +734,9 @@ namespace Org.BouncyCastle.Crypto.Tls */ mSecurityParameters.verifyDataLength = 12; - message.WriteToRecordStream(this); + ApplyMaxFragmentLengthExtension(); - this.mRecordStream.NotifyHelloComplete(); + message.WriteToRecordStream(this); } protected virtual void SendServerHelloDoneMessage() -- cgit 1.4.1