TLS: refactoring around extended_master_secret

- especially the interaction with session resumption and the methods relating to use of EMS.

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();
         }
 
         /// <exception cref="IOException"/>
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)