From 3ad16da3f107f766a8ba50eb85906040f25bde95 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Tue, 10 May 2022 18:19:08 +0700 Subject: Improve TLS handshake hash tracking --- crypto/src/tls/DtlsClientProtocol.cs | 9 +-------- crypto/src/tls/DtlsServerProtocol.cs | 18 +++++++++--------- crypto/src/tls/TlsClientProtocol.cs | 7 ++----- crypto/src/tls/TlsServerProtocol.cs | 21 ++++++++++++--------- crypto/src/tls/crypto/TlsCrypto.cs | 22 +++++++++++++++++----- crypto/src/tls/crypto/impl/AbstractTlsCrypto.cs | 6 ++++-- crypto/src/tls/crypto/impl/bc/BcTlsCrypto.cs | 18 ++++++++++++++++-- 7 files changed, 61 insertions(+), 40 deletions(-) diff --git a/crypto/src/tls/DtlsClientProtocol.cs b/crypto/src/tls/DtlsClientProtocol.cs index dd273f3e7..fd9985ab5 100644 --- a/crypto/src/tls/DtlsClientProtocol.cs +++ b/crypto/src/tls/DtlsClientProtocol.cs @@ -137,10 +137,6 @@ namespace Org.BouncyCastle.Tls } handshake.HandshakeHash.NotifyPrfDetermined(); - if (!ProtocolVersion.DTLSv12.Equals(securityParameters.NegotiatedVersion)) - { - handshake.HandshakeHash.SealHashAlgorithms(); - } ApplyMaxFragmentLengthExtension(recordLayer, securityParameters.MaxFragmentLength); @@ -300,10 +296,7 @@ namespace Org.BouncyCastle.Tls } } - if (ProtocolVersion.DTLSv12.Equals(securityParameters.NegotiatedVersion)) - { - handshake.HandshakeHash.SealHashAlgorithms(); - } + handshake.HandshakeHash.SealHashAlgorithms(); if (clientAuthCredentials == null) { diff --git a/crypto/src/tls/DtlsServerProtocol.cs b/crypto/src/tls/DtlsServerProtocol.cs index 2c6ddf31f..c019eb9fb 100644 --- a/crypto/src/tls/DtlsServerProtocol.cs +++ b/crypto/src/tls/DtlsServerProtocol.cs @@ -146,10 +146,6 @@ namespace Org.BouncyCastle.Tls } handshake.HandshakeHash.NotifyPrfDetermined(); - if (!ProtocolVersion.DTLSv12.Equals(securityParameters.NegotiatedVersion)) - { - handshake.HandshakeHash.SealHashAlgorithms(); - } IList serverSupplementalData = state.server.GetServerSupplementalData(); if (serverSupplementalData != null) @@ -233,7 +229,14 @@ namespace Org.BouncyCastle.Tls { TlsUtilities.TrackHashAlgorithms(handshake.HandshakeHash, securityParameters.ServerSigAlgs); - if (!state.serverContext.Crypto.HasAllRawSignatureAlgorithms()) + if (state.serverContext.Crypto.HasAnyStreamVerifiers(securityParameters.ServerSigAlgs)) + { + handshake.HandshakeHash.ForceBuffering(); + } + } + else + { + if (state.serverContext.Crypto.HasAnyStreamVerifiersLegacy(state.certificateRequest.CertificateTypes)) { handshake.HandshakeHash.ForceBuffering(); } @@ -241,10 +244,7 @@ namespace Org.BouncyCastle.Tls } } - if (ProtocolVersion.DTLSv12.Equals(securityParameters.NegotiatedVersion)) - { - handshake.HandshakeHash.SealHashAlgorithms(); - } + handshake.HandshakeHash.SealHashAlgorithms(); if (null != state.certificateRequest) { diff --git a/crypto/src/tls/TlsClientProtocol.cs b/crypto/src/tls/TlsClientProtocol.cs index 930294eae..ba2b565ca 100644 --- a/crypto/src/tls/TlsClientProtocol.cs +++ b/crypto/src/tls/TlsClientProtocol.cs @@ -462,7 +462,7 @@ namespace Org.BouncyCastle.Tls { ProcessServerHello(serverHello); m_handshakeHash.NotifyPrfDetermined(); - if (!ProtocolVersion.TLSv12.Equals(securityParameters.NegotiatedVersion)) + if (TlsUtilities.IsTlsV13(securityParameters.NegotiatedVersion)) { m_handshakeHash.SealHashAlgorithms(); } @@ -569,10 +569,7 @@ namespace Org.BouncyCastle.Tls } } - if (ProtocolVersion.TLSv12.Equals(securityParameters.NegotiatedVersion)) - { - m_handshakeHash.SealHashAlgorithms(); - } + m_handshakeHash.SealHashAlgorithms(); if (clientAuthCredentials == null) { diff --git a/crypto/src/tls/TlsServerProtocol.cs b/crypto/src/tls/TlsServerProtocol.cs index a21ae69cd..f32ecc2da 100644 --- a/crypto/src/tls/TlsServerProtocol.cs +++ b/crypto/src/tls/TlsServerProtocol.cs @@ -899,16 +899,15 @@ namespace Org.BouncyCastle.Tls ServerHello serverHello = GenerateServerHello(clientHello, buf); m_handshakeHash.NotifyPrfDetermined(); - if (!ProtocolVersion.TLSv12.Equals(securityParameters.NegotiatedVersion)) - { - m_handshakeHash.SealHashAlgorithms(); - } if (TlsUtilities.IsTlsV13(securityParameters.NegotiatedVersion)) { + m_handshakeHash.SealHashAlgorithms(); + if (serverHello.IsHelloRetryRequest()) { TlsUtilities.AdjustTranscriptForRetry(m_handshakeHash); + SendServerHelloMessage(serverHello); this.m_connectionState = CS_SERVER_HELLO_RETRY_REQUEST; @@ -1032,7 +1031,14 @@ namespace Org.BouncyCastle.Tls { TlsUtilities.TrackHashAlgorithms(m_handshakeHash, securityParameters.ServerSigAlgs); - if (!m_tlsServerContext.Crypto.HasAllRawSignatureAlgorithms()) + if (m_tlsServerContext.Crypto.HasAnyStreamVerifiers(securityParameters.ServerSigAlgs)) + { + m_handshakeHash.ForceBuffering(); + } + } + else + { + if (m_tlsServerContext.Crypto.HasAnyStreamVerifiersLegacy(m_certificateRequest.CertificateTypes)) { m_handshakeHash.ForceBuffering(); } @@ -1040,10 +1046,7 @@ namespace Org.BouncyCastle.Tls } } - if (ProtocolVersion.TLSv12.Equals(securityParameters.NegotiatedVersion)) - { - m_handshakeHash.SealHashAlgorithms(); - } + m_handshakeHash.SealHashAlgorithms(); if (null != m_certificateRequest) { diff --git a/crypto/src/tls/crypto/TlsCrypto.cs b/crypto/src/tls/crypto/TlsCrypto.cs index 4dab6bc57..27c5fb9e1 100644 --- a/crypto/src/tls/crypto/TlsCrypto.cs +++ b/crypto/src/tls/crypto/TlsCrypto.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.IO; using Org.BouncyCastle.Math; @@ -10,11 +11,22 @@ namespace Org.BouncyCastle.Tls.Crypto /// cryptography in the API. public interface TlsCrypto { - /// Return true if this TlsCrypto can perform raw signatures and verifications for all supported - /// algorithms. - /// true if this instance can perform raw signatures and verifications for all supported algorithms, - /// false otherwise. - bool HasAllRawSignatureAlgorithms(); + /// Return true if this TlsCrypto would use a stream verifier for any of the passed in algorithms. + /// + /// This method is only relevant to handshakes negotiating (D)TLS 1.2. + /// A list of + /// values. + /// true if this instance would use a stream verifier for any of the passed in algorithms, otherwise + /// false. + bool HasAnyStreamVerifiers(IList signatureAndHashAlgorithms); + + /// Return true if this TlsCrypto would use a stream verifier for any of the passed in algorithms. + /// + /// This method is only relevant to handshakes negotiating (D)TLS versions older than 1.2. + /// An array of values. + /// true if this instance would use a stream verifier for any of the passed in algorithms, otherwise + /// false. + bool HasAnyStreamVerifiersLegacy(short[] clientCertificateTypes); /// Return true if this TlsCrypto can support the passed in hash algorithm. /// the algorithm of interest. diff --git a/crypto/src/tls/crypto/impl/AbstractTlsCrypto.cs b/crypto/src/tls/crypto/impl/AbstractTlsCrypto.cs index 39d86c241..87fe66dff 100644 --- a/crypto/src/tls/crypto/impl/AbstractTlsCrypto.cs +++ b/crypto/src/tls/crypto/impl/AbstractTlsCrypto.cs @@ -1,5 +1,5 @@ using System; -using System.IO; +using System.Collections; using Org.BouncyCastle.Math; using Org.BouncyCastle.Security; @@ -12,7 +12,9 @@ namespace Org.BouncyCastle.Tls.Crypto.Impl public abstract class AbstractTlsCrypto : TlsCrypto { - public abstract bool HasAllRawSignatureAlgorithms(); + public abstract bool HasAnyStreamVerifiers(IList signatureAndHashAlgorithms); + + public abstract bool HasAnyStreamVerifiersLegacy(short[] clientCertificateTypes); public abstract bool HasCryptoHashAlgorithm(int cryptoHashAlgorithm); diff --git a/crypto/src/tls/crypto/impl/bc/BcTlsCrypto.cs b/crypto/src/tls/crypto/impl/bc/BcTlsCrypto.cs index a56835105..d8a46ad41 100644 --- a/crypto/src/tls/crypto/impl/bc/BcTlsCrypto.cs +++ b/crypto/src/tls/crypto/impl/bc/BcTlsCrypto.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using Org.BouncyCastle.Crypto; using Org.BouncyCastle.Crypto.Agreement.Srp; @@ -152,9 +153,22 @@ namespace Org.BouncyCastle.Tls.Crypto.Impl.BC return new BcTlsNonceGenerator(randomGenerator); } - public override bool HasAllRawSignatureAlgorithms() + public override bool HasAnyStreamVerifiers(IList signatureAndHashAlgorithms) + { + foreach (SignatureAndHashAlgorithm algorithm in signatureAndHashAlgorithms) + { + switch (SignatureScheme.From(algorithm)) + { + case SignatureScheme.ed25519: + case SignatureScheme.ed448: + return true; + } + } + return false; + } + + public override bool HasAnyStreamVerifiersLegacy(short[] clientCertificateTypes) { - // TODO[RFC 8422] Revisit the need to buffer the handshake for "Intrinsic" hash signatures return false; } -- cgit 1.4.1