From 806f7224c9c1f6708358115f0bb8a3fcfd0a0663 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 6 Jul 2023 17:15:29 +0700 Subject: (D)TLS: Refactoring around the MFL extension --- crypto/src/tls/DtlsClientProtocol.cs | 5 ++--- crypto/src/tls/DtlsProtocol.cs | 18 ------------------ crypto/src/tls/DtlsServerProtocol.cs | 8 +++++--- crypto/src/tls/TlsClientProtocol.cs | 8 ++++---- crypto/src/tls/TlsProtocol.cs | 13 ++----------- crypto/src/tls/TlsServerProtocol.cs | 4 ++-- crypto/src/tls/TlsUtilities.cs | 17 +++++++++++++++++ 7 files changed, 32 insertions(+), 41 deletions(-) diff --git a/crypto/src/tls/DtlsClientProtocol.cs b/crypto/src/tls/DtlsClientProtocol.cs index b6876bdd1..e96e161d4 100644 --- a/crypto/src/tls/DtlsClientProtocol.cs +++ b/crypto/src/tls/DtlsClientProtocol.cs @@ -893,9 +893,8 @@ namespace Org.BouncyCastle.Tls securityParameters.m_encryptThenMac = serverSentEncryptThenMac; } - securityParameters.m_maxFragmentLength = EvaluateMaxFragmentLengthExtension( - securityParameters.IsResumedSession, sessionClientExtensions, sessionServerExtensions, - AlertDescription.illegal_parameter); + securityParameters.m_maxFragmentLength = TlsUtilities.ProcessMaxFragmentLengthExtension( + sessionClientExtensions, sessionServerExtensions, AlertDescription.illegal_parameter); securityParameters.m_truncatedHmac = TlsExtensionsUtilities.HasTruncatedHmacExtension( sessionServerExtensions); diff --git a/crypto/src/tls/DtlsProtocol.cs b/crypto/src/tls/DtlsProtocol.cs index 745535148..566d07cb4 100644 --- a/crypto/src/tls/DtlsProtocol.cs +++ b/crypto/src/tls/DtlsProtocol.cs @@ -38,24 +38,6 @@ namespace Org.BouncyCastle.Tls } } - /// - internal static short EvaluateMaxFragmentLengthExtension(bool resumedSession, - IDictionary clientExtensions, IDictionary serverExtensions, - short alertDescription) - { - short maxFragmentLength = TlsExtensionsUtilities.GetMaxFragmentLengthExtension(serverExtensions); - if (maxFragmentLength >= 0) - { - if (!MaxFragmentLength.IsValid(maxFragmentLength) - || (!resumedSession && maxFragmentLength != TlsExtensionsUtilities - .GetMaxFragmentLengthExtension(clientExtensions))) - { - throw new TlsFatalAlert(alertDescription); - } - } - return maxFragmentLength; - } - /// internal static byte[] GenerateCertificate(TlsContext context, Certificate certificate, Stream endPointHash) { diff --git a/crypto/src/tls/DtlsServerProtocol.cs b/crypto/src/tls/DtlsServerProtocol.cs index 82c6ff290..66ab6d294 100644 --- a/crypto/src/tls/DtlsServerProtocol.cs +++ b/crypto/src/tls/DtlsServerProtocol.cs @@ -581,10 +581,12 @@ namespace Org.BouncyCastle.Tls securityParameters.m_encryptThenMac = TlsExtensionsUtilities.HasEncryptThenMacExtension( state.serverExtensions); - securityParameters.m_maxFragmentLength = EvaluateMaxFragmentLengthExtension(resumedSession, - state.clientExtensions, state.serverExtensions, AlertDescription.internal_error); + securityParameters.m_maxFragmentLength = TlsUtilities.ProcessMaxFragmentLengthExtension( + resumedSession ? null : state.clientExtensions, state.serverExtensions, + AlertDescription.internal_error); - securityParameters.m_truncatedHmac = TlsExtensionsUtilities.HasTruncatedHmacExtension(state.serverExtensions); + securityParameters.m_truncatedHmac = TlsExtensionsUtilities.HasTruncatedHmacExtension( + state.serverExtensions); /* * TODO It's surprising that there's no provision to allow a 'fresh' CertificateStatus to be sent in diff --git a/crypto/src/tls/TlsClientProtocol.cs b/crypto/src/tls/TlsClientProtocol.cs index 7b2f148d3..731638467 100644 --- a/crypto/src/tls/TlsClientProtocol.cs +++ b/crypto/src/tls/TlsClientProtocol.cs @@ -1306,8 +1306,8 @@ namespace Org.BouncyCastle.Tls securityParameters.m_encryptThenMac = serverSentEncryptThenMAC; } - securityParameters.m_maxFragmentLength = ProcessMaxFragmentLengthExtension(sessionClientExtensions, - sessionServerExtensions, AlertDescription.illegal_parameter); + securityParameters.m_maxFragmentLength = TlsUtilities.ProcessMaxFragmentLengthExtension( + sessionClientExtensions, sessionServerExtensions, AlertDescription.illegal_parameter); securityParameters.m_truncatedHmac = TlsExtensionsUtilities.HasTruncatedHmacExtension( sessionServerExtensions); @@ -1431,8 +1431,8 @@ namespace Org.BouncyCastle.Tls sessionServerExtensions = m_sessionParameters.ReadServerExtensions(); } - securityParameters.m_maxFragmentLength = ProcessMaxFragmentLengthExtension(sessionClientExtensions, - sessionServerExtensions, AlertDescription.illegal_parameter); + securityParameters.m_maxFragmentLength = TlsUtilities.ProcessMaxFragmentLengthExtension( + sessionClientExtensions, sessionServerExtensions, AlertDescription.illegal_parameter); securityParameters.m_encryptThenMac = false; securityParameters.m_truncatedHmac = false; diff --git a/crypto/src/tls/TlsProtocol.cs b/crypto/src/tls/TlsProtocol.cs index 3689f3a76..9b8039a5e 100644 --- a/crypto/src/tls/TlsProtocol.cs +++ b/crypto/src/tls/TlsProtocol.cs @@ -1814,20 +1814,11 @@ namespace Org.BouncyCastle.Tls } /// + [Obsolete("Will be removed")] protected virtual short ProcessMaxFragmentLengthExtension(IDictionary clientExtensions, IDictionary serverExtensions, short alertDescription) { - short maxFragmentLength = TlsExtensionsUtilities.GetMaxFragmentLengthExtension(serverExtensions); - if (maxFragmentLength >= 0) - { - if (!MaxFragmentLength.IsValid(maxFragmentLength) || - (clientExtensions != null && - maxFragmentLength != TlsExtensionsUtilities.GetMaxFragmentLengthExtension(clientExtensions))) - { - throw new TlsFatalAlert(alertDescription); - } - } - return maxFragmentLength; + return TlsUtilities.ProcessMaxFragmentLengthExtension(clientExtensions, serverExtensions, alertDescription); } /// diff --git a/crypto/src/tls/TlsServerProtocol.cs b/crypto/src/tls/TlsServerProtocol.cs index 6d66b065d..def8ae49b 100644 --- a/crypto/src/tls/TlsServerProtocol.cs +++ b/crypto/src/tls/TlsServerProtocol.cs @@ -315,7 +315,7 @@ namespace Org.BouncyCastle.Tls if (serverEncryptedExtensions.Count > 0) { - securityParameters.m_maxFragmentLength = ProcessMaxFragmentLengthExtension( + securityParameters.m_maxFragmentLength = TlsUtilities.ProcessMaxFragmentLengthExtension( securityParameters.IsResumedSession ? null : clientHelloExtensions, serverEncryptedExtensions, AlertDescription.internal_error); } @@ -692,7 +692,7 @@ namespace Org.BouncyCastle.Tls securityParameters.m_encryptThenMac = TlsExtensionsUtilities.HasEncryptThenMacExtension( m_serverExtensions); - securityParameters.m_maxFragmentLength = ProcessMaxFragmentLengthExtension( + securityParameters.m_maxFragmentLength = TlsUtilities.ProcessMaxFragmentLengthExtension( resumedSession ? null : m_clientExtensions, m_serverExtensions, AlertDescription.internal_error); securityParameters.m_truncatedHmac = TlsExtensionsUtilities.HasTruncatedHmacExtension( diff --git a/crypto/src/tls/TlsUtilities.cs b/crypto/src/tls/TlsUtilities.cs index 2f95e71ab..a9c7629d6 100644 --- a/crypto/src/tls/TlsUtilities.cs +++ b/crypto/src/tls/TlsUtilities.cs @@ -5731,6 +5731,23 @@ namespace Org.BouncyCastle.Tls return v; } + /// + internal static short ProcessMaxFragmentLengthExtension(IDictionary clientExtensions, + IDictionary serverExtensions, short alertDescription) + { + short maxFragmentLength = TlsExtensionsUtilities.GetMaxFragmentLengthExtension(serverExtensions); + if (maxFragmentLength >= 0) + { + if (!MaxFragmentLength.IsValid(maxFragmentLength) || + (clientExtensions != null && + maxFragmentLength != TlsExtensionsUtilities.GetMaxFragmentLengthExtension(clientExtensions))) + { + throw new TlsFatalAlert(alertDescription); + } + } + return maxFragmentLength; + } + // TODO[api] Not needed once GetHandshakeResendTimeMillis() has been added to TlsPeer internal static int GetHandshakeResendTimeMillis(TlsPeer tlsPeer) { -- cgit 1.4.1