From e604b3112a7c44a9e7b1fdc90d066f037763a404 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 5 Jul 2023 11:14:17 +0700 Subject: TLS: refactoring around extended_master_secret - especially the interaction with session resumption and the methods relating to use of EMS. --- crypto/src/tls/DtlsClientProtocol.cs | 2 +- crypto/src/tls/TlsClientProtocol.cs | 115 ++++++++++++++++++++++++----------- crypto/src/tls/TlsProtocol.cs | 30 ++++++--- crypto/src/tls/TlsServerProtocol.cs | 94 +++++++++++++++++----------- crypto/src/tls/TlsUtilities.cs | 23 ++++--- 5 files changed, 172 insertions(+), 92 deletions(-) diff --git a/crypto/src/tls/DtlsClientProtocol.cs b/crypto/src/tls/DtlsClientProtocol.cs index c1bad2e6f..aab3853fb 100644 --- a/crypto/src/tls/DtlsClientProtocol.cs +++ b/crypto/src/tls/DtlsClientProtocol.cs @@ -468,7 +468,7 @@ namespace Org.BouncyCastle.Tls state.clientAgreements = TlsUtilities.AddKeyShareToClientHello(state.clientContext, state.client, state.clientExtensions); - if (TlsUtilities.IsExtendedMasterSecretOptionalDtls(context.ClientSupportedVersions) + if (TlsUtilities.IsExtendedMasterSecretOptional(context.ClientSupportedVersions) && state.client.ShouldUseExtendedMasterSecret()) { TlsExtensionsUtilities.AddExtendedMasterSecretExtension(state.clientExtensions); diff --git a/crypto/src/tls/TlsClientProtocol.cs b/crypto/src/tls/TlsClientProtocol.cs index cff541db3..7b2f148d3 100644 --- a/crypto/src/tls/TlsClientProtocol.cs +++ b/crypto/src/tls/TlsClientProtocol.cs @@ -1229,37 +1229,41 @@ namespace Org.BouncyCastle.Tls // TODO[compat-gnutls] GnuTLS test server fails to send renegotiation_info extension when resuming m_tlsClient.NotifySecureRenegotiation(securityParameters.IsSecureRenegotiation); - /* - * RFC 7627 4. Clients and servers SHOULD NOT accept handshakes that do not use the extended - * master secret [..]. (and see 5.2, 5.3) - * - * RFC 8446 Appendix D. Because TLS 1.3 always hashes in the transcript up to the server - * Finished, implementations which support both TLS 1.3 and earlier versions SHOULD indicate - * the use of the Extended Master Secret extension in their APIs whenever TLS 1.3 is used. - */ + // extended_master_secret { - bool acceptedExtendedMasterSecret = TlsExtensionsUtilities.HasExtendedMasterSecretExtension( - m_serverExtensions); - bool resumedSession = securityParameters.IsResumedSession; + bool negotiatedEms = false; - if (acceptedExtendedMasterSecret) + if (TlsExtensionsUtilities.HasExtendedMasterSecretExtension(m_clientExtensions)) { - if (server_version.IsSsl - || (!resumedSession && !m_tlsClient.ShouldUseExtendedMasterSecret())) + negotiatedEms = TlsExtensionsUtilities.HasExtendedMasterSecretExtension(m_serverExtensions); + + if (TlsUtilities.IsExtendedMasterSecretOptional(server_version)) { - throw new TlsFatalAlert(AlertDescription.handshake_failure); + if (!negotiatedEms && + m_tlsClient.RequiresExtendedMasterSecret()) + { + throw new TlsFatalAlert(AlertDescription.handshake_failure, + "Extended Master Secret extension is required"); + } } - } - else - { - if (m_tlsClient.RequiresExtendedMasterSecret() - || (resumedSession && !m_tlsClient.AllowLegacyResumption())) + else { - throw new TlsFatalAlert(AlertDescription.handshake_failure); + if (negotiatedEms) + { + throw new TlsFatalAlert(AlertDescription.illegal_parameter, + "Server sent an unexpected extended_master_secret extension negotiating " + server_version); + } } } - securityParameters.m_extendedMasterSecret = acceptedExtendedMasterSecret; + securityParameters.m_extendedMasterSecret = negotiatedEms; + } + + if (securityParameters.IsResumedSession && + securityParameters.IsExtendedMasterSecret != m_sessionParameters.IsExtendedMasterSecret) + { + throw new TlsFatalAlert(AlertDescription.handshake_failure, + "Server resumed session with mismatched extended_master_secret negotiation"); } /* @@ -1670,8 +1674,17 @@ namespace Org.BouncyCastle.Tls securityParameters.m_clientRandom = CreateRandomBlock(useGmtUnixTime, m_tlsClientContext); } - EstablishSession(offeringTlsV12Minus ? m_tlsClient.GetSessionToResume() : null); - m_tlsClient.NotifySessionToResume(m_tlsSession); + TlsSession sessionToResume = offeringTlsV12Minus ? m_tlsClient.GetSessionToResume() : null; + + bool fallback = m_tlsClient.IsFallback(); + + int[] offeredCipherSuites = m_tlsClient.GetCipherSuites(); + + this.m_clientExtensions = TlsExtensionsUtilities.EnsureExtensionsInitialised(m_tlsClient.GetClientExtensions()); + + bool shouldUseEms = m_tlsClient.ShouldUseExtendedMasterSecret(); + + EstablishSession(sessionToResume); /* * TODO RFC 5077 3.4. When presenting a ticket, the client MAY generate and include a @@ -1679,11 +1692,7 @@ namespace Org.BouncyCastle.Tls */ byte[] legacy_session_id = TlsUtilities.GetSessionID(m_tlsSession); - bool fallback = m_tlsClient.IsFallback(); - - int[] offeredCipherSuites = m_tlsClient.GetCipherSuites(); - - if (legacy_session_id.Length > 0 && m_sessionParameters != null) + if (legacy_session_id.Length > 0) { if (!Arrays.Contains(offeredCipherSuites, m_sessionParameters.CipherSuite)) { @@ -1691,8 +1700,42 @@ namespace Org.BouncyCastle.Tls } } - this.m_clientExtensions = TlsExtensionsUtilities.EnsureExtensionsInitialised( - m_tlsClient.GetClientExtensions()); + ProtocolVersion sessionVersion = null; + if (legacy_session_id.Length > 0) + { + sessionVersion = m_sessionParameters.NegotiatedVersion; + + if (!ProtocolVersion.Contains(supportedVersions, sessionVersion)) + { + legacy_session_id = TlsUtilities.EmptyBytes; + } + } + + if (legacy_session_id.Length > 0 && TlsUtilities.IsExtendedMasterSecretOptional(sessionVersion)) + { + if (shouldUseEms) + { + if (!m_sessionParameters.IsExtendedMasterSecret && + !m_tlsClient.AllowLegacyResumption()) + { + legacy_session_id = TlsUtilities.EmptyBytes; + } + } + else + { + if (m_sessionParameters.IsExtendedMasterSecret) + { + legacy_session_id = TlsUtilities.EmptyBytes; + } + } + } + + if (legacy_session_id.Length < 1) + { + CancelSession(); + } + + m_tlsClient.NotifySessionToResume(m_tlsSession); ProtocolVersion legacy_version = latestVersion; if (offeringTlsV13Plus) @@ -1731,15 +1774,13 @@ namespace Org.BouncyCastle.Tls this.m_clientAgreements = TlsUtilities.AddKeyShareToClientHello(m_tlsClientContext, m_tlsClient, m_clientExtensions); - if (TlsUtilities.IsExtendedMasterSecretOptionalTls(supportedVersions) - && (m_tlsClient.ShouldUseExtendedMasterSecret() || - (null != m_sessionParameters && m_sessionParameters.IsExtendedMasterSecret))) + if (shouldUseEms && TlsUtilities.IsExtendedMasterSecretOptional(supportedVersions)) { - TlsExtensionsUtilities.AddExtendedMasterSecretExtension(m_clientExtensions); + TlsExtensionsUtilities.AddExtendedMasterSecretExtension(this.m_clientExtensions); } - else if (!offeringTlsV13Plus && m_tlsClient.RequiresExtendedMasterSecret()) + else { - throw new TlsFatalAlert(AlertDescription.internal_error); + this.m_clientExtensions.Remove(ExtensionType.extended_master_secret); } // NOT renegotiating diff --git a/crypto/src/tls/TlsProtocol.cs b/crypto/src/tls/TlsProtocol.cs index 0ee702a96..3689f3a76 100644 --- a/crypto/src/tls/TlsProtocol.cs +++ b/crypto/src/tls/TlsProtocol.cs @@ -1454,16 +1454,20 @@ namespace Org.BouncyCastle.Tls if (null == sessionParameters) return false; - if (!sessionParameters.IsExtendedMasterSecret) + ProtocolVersion sessionVersion = sessionParameters.NegotiatedVersion; + if (null == sessionVersion || !sessionVersion.IsTls) + return false; + + bool isEms = sessionParameters.IsExtendedMasterSecret; + if (sessionVersion.IsSsl) { - TlsPeer peer = Peer; - if (!peer.AllowLegacyResumption() || peer.RequiresExtendedMasterSecret()) + if (isEms) + return false; + } + else if (!TlsUtilities.IsExtendedMasterSecretOptional(sessionVersion)) + { + if (!isEms) return false; - - /* - * NOTE: For session resumption without extended_master_secret, renegotiation MUST be disabled - * (see RFC 7627 5.4). - */ } TlsSecret sessionMasterSecret = TlsUtilities.GetSessionMasterSecret(Context.Crypto, @@ -1478,7 +1482,7 @@ namespace Org.BouncyCastle.Tls return true; } - protected virtual void InvalidateSession() + protected virtual void CancelSession() { if (m_sessionMasterSecret != null) { @@ -1492,11 +1496,17 @@ namespace Org.BouncyCastle.Tls this.m_sessionParameters = null; } + this.m_tlsSession = null; + } + + protected virtual void InvalidateSession() + { if (m_tlsSession != null) { m_tlsSession.Invalidate(); - this.m_tlsSession = null; } + + CancelSession(); } /// diff --git a/crypto/src/tls/TlsServerProtocol.cs b/crypto/src/tls/TlsServerProtocol.cs index c7a250042..6d66b065d 100644 --- a/crypto/src/tls/TlsServerProtocol.cs +++ b/crypto/src/tls/TlsServerProtocol.cs @@ -522,9 +522,6 @@ namespace Org.BouncyCastle.Tls m_tlsServer.NotifySecureRenegotiation(securityParameters.IsSecureRenegotiation); - bool offeredExtendedMasterSecret = TlsExtensionsUtilities.HasExtendedMasterSecretExtension( - m_clientExtensions); - if (m_clientExtensions != null) { // NOTE: Validates the padding extension data, if present @@ -548,11 +545,63 @@ namespace Org.BouncyCastle.Tls m_tlsServer.ProcessClientExtensions(m_clientExtensions); } - bool resumedSession = EstablishSession(m_tlsServer.GetSessionToResume(clientHello.SessionID)); - securityParameters.m_resumedSession = resumedSession; + TlsSession sessionToResume = m_tlsServer.GetSessionToResume(clientHello.SessionID); + + bool resumedSession = EstablishSession(sessionToResume); + + if (resumedSession && !serverVersion.Equals(m_sessionParameters.NegotiatedVersion)) + { + resumedSession = false; + } + + // TODO Check the session cipher suite is selectable by the same rules that getSelectedCipherSuite uses + + // TODO Check the resumed session has a peer certificate if we NEED client-auth + + // extended_master_secret + { + bool negotiateEms = false; + + if (TlsUtilities.IsExtendedMasterSecretOptional(serverVersion) && + m_tlsServer.ShouldUseExtendedMasterSecret()) + { + if (TlsExtensionsUtilities.HasExtendedMasterSecretExtension(m_clientExtensions)) + { + negotiateEms = true; + } + else if (m_tlsServer.RequiresExtendedMasterSecret()) + { + throw new TlsFatalAlert(AlertDescription.handshake_failure, + "Extended Master Secret extension is required"); + } + else if (resumedSession) + { + if (m_sessionParameters.IsExtendedMasterSecret) + { + throw new TlsFatalAlert(AlertDescription.handshake_failure, + "Extended Master Secret extension is required for EMS session resumption"); + } + + if (!m_tlsServer.AllowLegacyResumption()) + { + throw new TlsFatalAlert(AlertDescription.handshake_failure, + "Extended Master Secret extension is required for legacy session resumption"); + } + } + } + + if (resumedSession && negotiateEms != m_sessionParameters.IsExtendedMasterSecret) + { + resumedSession = false; + } + + securityParameters.m_extendedMasterSecret = negotiateEms; + } if (!resumedSession) { + CancelSession(); + byte[] newSessionID = m_tlsServer.GetNewSessionID(); if (null == newSessionID) { @@ -560,10 +609,9 @@ namespace Org.BouncyCastle.Tls } this.m_tlsSession = TlsUtilities.ImportSession(newSessionID, null); - this.m_sessionParameters = null; - this.m_sessionMasterSecret = null; } + securityParameters.m_resumedSession = resumedSession; securityParameters.m_sessionID = m_tlsSession.SessionID; m_tlsServer.NotifySession(m_tlsSession); @@ -627,41 +675,13 @@ namespace Org.BouncyCastle.Tls } } - /* - * RFC 7627 4. Clients and servers SHOULD NOT accept handshakes that do not use the extended - * master secret [..]. (and see 5.2, 5.3) - */ - if (resumedSession) + if (securityParameters.IsExtendedMasterSecret) { - if (!m_sessionParameters.IsExtendedMasterSecret) - { - /* - * TODO[resumption] ProvTlsServer currently only resumes EMS sessions. Revisit this - * in relation to 'tlsServer.allowLegacyResumption()'. - */ - throw new TlsFatalAlert(AlertDescription.internal_error); - } - - if (!offeredExtendedMasterSecret) - throw new TlsFatalAlert(AlertDescription.handshake_failure); - - securityParameters.m_extendedMasterSecret = true; - TlsExtensionsUtilities.AddExtendedMasterSecretExtension(m_serverExtensions); } else { - securityParameters.m_extendedMasterSecret = offeredExtendedMasterSecret && !serverVersion.IsSsl - && m_tlsServer.ShouldUseExtendedMasterSecret(); - - if (securityParameters.IsExtendedMasterSecret) - { - TlsExtensionsUtilities.AddExtendedMasterSecretExtension(m_serverExtensions); - } - else if (m_tlsServer.RequiresExtendedMasterSecret()) - { - throw new TlsFatalAlert(AlertDescription.handshake_failure); - } + m_serverExtensions.Remove(ExtensionType.extended_master_secret); } securityParameters.m_applicationProtocol = TlsExtensionsUtilities.GetAlpnExtensionServer(m_serverExtensions); diff --git a/crypto/src/tls/TlsUtilities.cs b/crypto/src/tls/TlsUtilities.cs index 7337e9f52..2f95e71ab 100644 --- a/crypto/src/tls/TlsUtilities.cs +++ b/crypto/src/tls/TlsUtilities.cs @@ -1156,17 +1156,26 @@ namespace Org.BouncyCastle.Tls return new TlsSessionImpl(sessionID, sessionParameters); } - internal static bool IsExtendedMasterSecretOptionalDtls(ProtocolVersion[] activeProtocolVersions) + internal static bool IsExtendedMasterSecretOptional(ProtocolVersion protocolVersion) { - return ProtocolVersion.Contains(activeProtocolVersions, ProtocolVersion.DTLSv12) - || ProtocolVersion.Contains(activeProtocolVersions, ProtocolVersion.DTLSv10); + ProtocolVersion tlsVersion = protocolVersion.GetEquivalentTlsVersion(); + + return ProtocolVersion.TLSv12.Equals(tlsVersion) + || ProtocolVersion.TLSv11.Equals(tlsVersion) + || ProtocolVersion.TLSv10.Equals(tlsVersion); } - internal static bool IsExtendedMasterSecretOptionalTls(ProtocolVersion[] activeProtocolVersions) + internal static bool IsExtendedMasterSecretOptional(ProtocolVersion[] protocolVersions) { - return ProtocolVersion.Contains(activeProtocolVersions, ProtocolVersion.TLSv12) - || ProtocolVersion.Contains(activeProtocolVersions, ProtocolVersion.TLSv11) - || ProtocolVersion.Contains(activeProtocolVersions, ProtocolVersion.TLSv10); + if (protocolVersions != null) + { + for (int i = 0; i < protocolVersions.Length; ++i) + { + if (IsExtendedMasterSecretOptional(protocolVersions[i])) + return true; + } + } + return false; } public static bool IsNullOrContainsNull(object[] array) -- cgit 1.4.1