From e779238a8773133f88d6a4c3e554b39849e9f0c4 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 19:55:25 +1030 Subject: BJA-620 followup for Poly1305 - bug not present in C#, but include test case - conservatively added an extra step in carry propagation --- crypto/src/crypto/macs/Poly1305.cs | 32 ++++++++++++++--------------- crypto/test/src/crypto/test/Poly1305Test.cs | 11 +++++++++- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/crypto/src/crypto/macs/Poly1305.cs b/crypto/src/crypto/macs/Poly1305.cs index 0f66ccccc..c0a660fac 100644 --- a/crypto/src/crypto/macs/Poly1305.cs +++ b/crypto/src/crypto/macs/Poly1305.cs @@ -219,13 +219,13 @@ namespace Org.BouncyCastle.Crypto.Macs ulong tp3 = mul32x32_64(h0,r3) + mul32x32_64(h1,r2) + mul32x32_64(h2,r1) + mul32x32_64(h3,r0) + mul32x32_64(h4,s4); ulong tp4 = mul32x32_64(h0,r4) + mul32x32_64(h1,r3) + mul32x32_64(h2,r2) + mul32x32_64(h3,r1) + mul32x32_64(h4,r0); - ulong b; - h0 = (uint)tp0 & 0x3ffffff; b = (tp0 >> 26); - tp1 += b; h1 = (uint)tp1 & 0x3ffffff; b = (tp1 >> 26); - tp2 += b; h2 = (uint)tp2 & 0x3ffffff; b = (tp2 >> 26); - tp3 += b; h3 = (uint)tp3 & 0x3ffffff; b = (tp3 >> 26); - tp4 += b; h4 = (uint)tp4 & 0x3ffffff; b = (tp4 >> 26); - h0 += (uint)(b * 5); + h0 = (uint)tp0 & 0x3ffffff; tp1 += (tp0 >> 26); + h1 = (uint)tp1 & 0x3ffffff; tp2 += (tp1 >> 26); + h2 = (uint)tp2 & 0x3ffffff; tp3 += (tp2 >> 26); + h3 = (uint)tp3 & 0x3ffffff; tp4 += (tp3 >> 26); + h4 = (uint)tp4 & 0x3ffffff; + h0 += (uint)(tp4 >> 26) * 5; + h1 += (h0 >> 26); h0 &= 0x3ffffff; } public int DoFinal(byte[] output, int outOff) @@ -238,17 +238,14 @@ namespace Org.BouncyCastle.Crypto.Macs ProcessBlock(); } - ulong f0, f1, f2, f3; - - uint b = h0 >> 26; - h0 = h0 & 0x3ffffff; - h1 += b; b = h1 >> 26; h1 = h1 & 0x3ffffff; - h2 += b; b = h2 >> 26; h2 = h2 & 0x3ffffff; - h3 += b; b = h3 >> 26; h3 = h3 & 0x3ffffff; - h4 += b; b = h4 >> 26; h4 = h4 & 0x3ffffff; - h0 += b * 5; + h1 += (h0 >> 26); h0 &= 0x3ffffff; + h2 += (h1 >> 26); h1 &= 0x3ffffff; + h3 += (h2 >> 26); h2 &= 0x3ffffff; + h4 += (h3 >> 26); h3 &= 0x3ffffff; + h0 += (h4 >> 26) * 5; h4 &= 0x3ffffff; + h1 += (h0 >> 26); h0 &= 0x3ffffff; - uint g0, g1, g2, g3, g4; + uint g0, g1, g2, g3, g4, b; g0 = h0 + 5; b = g0 >> 26; g0 &= 0x3ffffff; g1 = h1 + b; b = g1 >> 26; g1 &= 0x3ffffff; g2 = h2 + b; b = g2 >> 26; g2 &= 0x3ffffff; @@ -263,6 +260,7 @@ namespace Org.BouncyCastle.Crypto.Macs h3 = (h3 & nb) | (g3 & b); h4 = (h4 & nb) | (g4 & b); + ulong f0, f1, f2, f3; f0 = ((h0 ) | (h1 << 26)) + (ulong)k0; f1 = ((h1 >> 6 ) | (h2 << 20)) + (ulong)k1; f2 = ((h2 >> 12) | (h3 << 14)) + (ulong)k2; diff --git a/crypto/test/src/crypto/test/Poly1305Test.cs b/crypto/test/src/crypto/test/Poly1305Test.cs index 17c4289aa..cfbff787e 100644 --- a/crypto/test/src/crypto/test/Poly1305Test.cs +++ b/crypto/test/src/crypto/test/Poly1305Test.cs @@ -86,7 +86,16 @@ namespace Org.BouncyCastle.Crypto.Tests null, "f05204a74f0f88a7fa1a95b84ec3d8ffb36fcdc7723ea65dfe7cd464e86e0abf6b9d51db3220cfd8496ad6e6d36ebee8d990f9ce0d3bb7f72b7ab5b3ab0a73240d11efe772c857021ae859db4933cdde4387b471d2ce700fef4b81087f8f47c307881fd83017afcd15b8d21edf9b704677f46df97b07e5b83f87c8abd90af9b1d0f9e2710e8ebd0d4d1c6a055abea861f42368bed94d9373e909c1d3715b221c16bc524c55c31ec3eab204850bb2474a84f9917038eff9d921130951391b5c54f09b5e1de833ea2cd7d3b306740abb7096d1e173da83427da2adddd3631eda30b54dbf487f2b082e8646f07d6e0a87e97522ca38d4ace4954bf3db6dd3a93b06fa18eb56856627ed6cffcd7ae26374554ca18ab8905f26331d323fe10e6e70624c7bc07a70f06ecd804b48f8f7e75e910165e1beb554f1f0ec7949c9c8d429a206b4d5c0653102249b6098e6b45fac2a07ff0220b0b8ae8f4c6bcc0c813a7cd141fa8b398b42575fc395747c5a0257ac41d6c1f434cfbf5dfe8349f5347ef6b60e611f5d6c3cbc20ca2555274d1934325824cef4809da293ea13f181929e2af025bbd1c9abdc3af93afd4c50a2854ade3887f4d2c8c225168052c16e74d76d2dd3e9467a2c5b8e15c06ffbffa42b8536384139f07e195a8c9f70f514f31dca4eb2cf262c0dcbde53654b6250a29efe21d54e83c80e005a1cad36d5934ff01c32e4bc5fe06d03064ff4a268517df4a94c759289f323734318cfa5d859d4ce9c16e63d02dff0896976f521607638535d2ee8dd3312e1ddc80a55d34fe829ab954c1ebd54d929954770f1be9d32b4c05003c5c9e97943b6431e2afe820b1e967b19843e5985a131b1100517cdc363799104af91e2cf3f53cb8fd003653a6dd8a31a3f9d566a7124b0ffe9695bcb87c482eb60106f88198f766a40bc0f4873c23653c5f9e7a8e446f770beb8034cf01d21028ba15ccee21a8db918c4829d61c88bfa927bc5def831501796c5b401a60a6b1b433c9fb905c8cd40412fffee81ab", "045be28cc52009f506bdbfabedacf0b4"), - }; + // Test case from JIRA issue BJA-620 + new TestCase( + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff", + null, + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffffff" + "ffffffffffffffffffffffffffffff", + "c80cb43844f387946e5aa6085bdf67da") + }; public override string Name { -- cgit 1.4.1 From 00153da4b9ca5da090a697b25901e319a1a98b32 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 20:12:54 +1030 Subject: Added support for empty sequences --- crypto/src/asn1/x509/UserNotice.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crypto/src/asn1/x509/UserNotice.cs b/crypto/src/asn1/x509/UserNotice.cs index 5938f7c49..f40916434 100644 --- a/crypto/src/asn1/x509/UserNotice.cs +++ b/crypto/src/asn1/x509/UserNotice.cs @@ -58,6 +58,7 @@ namespace Org.BouncyCastle.Asn1.X509 * calling @{link toASN1Object()} for a UserNotice * instance or from parsing it from a DER-encoded stream.

*/ + [Obsolete("Use GetInstance() instead")] public UserNotice( Asn1Sequence seq) { @@ -71,12 +72,19 @@ namespace Org.BouncyCastle.Asn1.X509 if (seq[0].ToAsn1Object() is Asn1Sequence) { noticeRef = NoticeReference.GetInstance(seq[0]); + explicitText = null; } else { + noticeRef = null; explicitText = DisplayText.GetInstance(seq[0]); } } + else if (seq.Count == 0) + { + noticeRef = null; // neither field set! + explicitText = null; + } else { throw new ArgumentException("Bad sequence size: " + seq.Count); -- cgit 1.4.1 From 7af3b108ac0e198acb9ce2df5670b8a7f7d1f652 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 22:13:26 +1030 Subject: Change TLS server default DH parameters to 2048-bit group from RFC 3526 --- crypto/src/crypto/tls/DefaultTlsServer.cs | 2 +- crypto/src/crypto/tls/PskTlsServer.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/src/crypto/tls/DefaultTlsServer.cs b/crypto/src/crypto/tls/DefaultTlsServer.cs index 44ceb30e3..87d0539b3 100644 --- a/crypto/src/crypto/tls/DefaultTlsServer.cs +++ b/crypto/src/crypto/tls/DefaultTlsServer.cs @@ -42,7 +42,7 @@ namespace Org.BouncyCastle.Crypto.Tls protected virtual DHParameters GetDHParameters() { - return DHStandardGroups.rfc5114_2048_256; + return DHStandardGroups.rfc3526_2048; } protected override int[] GetCipherSuites() diff --git a/crypto/src/crypto/tls/PskTlsServer.cs b/crypto/src/crypto/tls/PskTlsServer.cs index 85f3055fb..d6f54db1f 100644 --- a/crypto/src/crypto/tls/PskTlsServer.cs +++ b/crypto/src/crypto/tls/PskTlsServer.cs @@ -28,7 +28,7 @@ namespace Org.BouncyCastle.Crypto.Tls protected virtual DHParameters GetDHParameters() { - return DHStandardGroups.rfc5114_2048_256; + return DHStandardGroups.rfc3526_2048; } protected override int[] GetCipherSuites() -- cgit 1.4.1 From 511a6af913f64908ada0ad8aeb19fd7a8f7e9575 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 22:28:24 +1030 Subject: Simple refactoring to follow bc-java code --- crypto/src/crypto/tls/CertificateUrl.cs | 3 ++- crypto/src/crypto/tls/DigestInputBuffer.cs | 4 +--- crypto/src/crypto/tls/OcspStatusRequest.cs | 3 ++- crypto/src/crypto/tls/ServerNameList.cs | 3 ++- crypto/src/crypto/tls/SignerInputBuffer.cs | 4 +--- crypto/src/util/io/Streams.cs | 8 +++++++- 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/crypto/src/crypto/tls/CertificateUrl.cs b/crypto/src/crypto/tls/CertificateUrl.cs index d285fa0f6..aff999551 100644 --- a/crypto/src/crypto/tls/CertificateUrl.cs +++ b/crypto/src/crypto/tls/CertificateUrl.cs @@ -3,6 +3,7 @@ using System.Collections; using System.IO; using Org.BouncyCastle.Utilities; +using Org.BouncyCastle.Utilities.IO; namespace Org.BouncyCastle.Crypto.Tls { @@ -116,7 +117,7 @@ namespace Org.BouncyCastle.Crypto.Tls TlsUtilities.CheckUint16(length); this.Position = 0; TlsUtilities.WriteUint16((int)length, this); - this.WriteTo(output); + Streams.WriteBufTo(this, output); Platform.Dispose(this); } } diff --git a/crypto/src/crypto/tls/DigestInputBuffer.cs b/crypto/src/crypto/tls/DigestInputBuffer.cs index 547bcab54..4435b40a5 100644 --- a/crypto/src/crypto/tls/DigestInputBuffer.cs +++ b/crypto/src/crypto/tls/DigestInputBuffer.cs @@ -1,8 +1,6 @@ using System; using System.IO; -using Org.BouncyCastle.Crypto; -using Org.BouncyCastle.Crypto.IO; using Org.BouncyCastle.Utilities.IO; namespace Org.BouncyCastle.Crypto.Tls @@ -12,7 +10,7 @@ namespace Org.BouncyCastle.Crypto.Tls { internal void UpdateDigest(IDigest d) { - WriteTo(new DigStream(d)); + Streams.WriteBufTo(this, new DigStream(d)); } private class DigStream diff --git a/crypto/src/crypto/tls/OcspStatusRequest.cs b/crypto/src/crypto/tls/OcspStatusRequest.cs index 2dd8371e5..d9203a3c4 100644 --- a/crypto/src/crypto/tls/OcspStatusRequest.cs +++ b/crypto/src/crypto/tls/OcspStatusRequest.cs @@ -6,6 +6,7 @@ using Org.BouncyCastle.Asn1; using Org.BouncyCastle.Asn1.Ocsp; using Org.BouncyCastle.Asn1.X509; using Org.BouncyCastle.Utilities; +using Org.BouncyCastle.Utilities.IO; namespace Org.BouncyCastle.Crypto.Tls { @@ -71,7 +72,7 @@ namespace Org.BouncyCastle.Crypto.Tls } TlsUtilities.CheckUint16(buf.Length); TlsUtilities.WriteUint16((int)buf.Length, output); - buf.WriteTo(output); + Streams.WriteBufTo(buf, output); } if (mRequestExtensions == null) diff --git a/crypto/src/crypto/tls/ServerNameList.cs b/crypto/src/crypto/tls/ServerNameList.cs index 5b5b90e58..ed4e59359 100644 --- a/crypto/src/crypto/tls/ServerNameList.cs +++ b/crypto/src/crypto/tls/ServerNameList.cs @@ -3,6 +3,7 @@ using System.Collections; using System.IO; using Org.BouncyCastle.Utilities; +using Org.BouncyCastle.Utilities.IO; namespace Org.BouncyCastle.Crypto.Tls { @@ -52,7 +53,7 @@ namespace Org.BouncyCastle.Crypto.Tls TlsUtilities.CheckUint16(buf.Length); TlsUtilities.WriteUint16((int)buf.Length, output); - buf.WriteTo(output); + Streams.WriteBufTo(buf, output); } /** diff --git a/crypto/src/crypto/tls/SignerInputBuffer.cs b/crypto/src/crypto/tls/SignerInputBuffer.cs index ef2827c4d..7bc69624c 100644 --- a/crypto/src/crypto/tls/SignerInputBuffer.cs +++ b/crypto/src/crypto/tls/SignerInputBuffer.cs @@ -1,8 +1,6 @@ using System; using System.IO; -using Org.BouncyCastle.Crypto; -using Org.BouncyCastle.Crypto.IO; using Org.BouncyCastle.Utilities.IO; namespace Org.BouncyCastle.Crypto.Tls @@ -12,7 +10,7 @@ namespace Org.BouncyCastle.Crypto.Tls { internal void UpdateSigner(ISigner s) { - WriteTo(new SigStream(s)); + Streams.WriteBufTo(this, new SigStream(s)); } private class SigStream diff --git a/crypto/src/util/io/Streams.cs b/crypto/src/util/io/Streams.cs index 70957acc7..cc7fa924c 100644 --- a/crypto/src/util/io/Streams.cs +++ b/crypto/src/util/io/Streams.cs @@ -90,5 +90,11 @@ namespace Org.BouncyCastle.Utilities.IO } return total; } - } + + /// + public static void WriteBufTo(MemoryStream buf, Stream output) + { + buf.WriteTo(output); + } + } } -- cgit 1.4.1 From f3d758b0f79743e17123560508cd59a63b8607c1 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 22:47:04 +1030 Subject: Use new TlsNoCloseNotifyException instead of generic EndOfStreamException - New exception only used for this specific case, which should simplify the handling of possible truncations in application code. --- crypto/BouncyCastle.Android.csproj | 1 + crypto/BouncyCastle.csproj | 1 + crypto/BouncyCastle.iOS.csproj | 1 + crypto/crypto.csproj | 5 +++++ crypto/src/crypto/tls/TlsNoCloseNotifyException.cs | 19 +++++++++++++++++++ crypto/src/crypto/tls/TlsProtocol.cs | 4 +--- 6 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 crypto/src/crypto/tls/TlsNoCloseNotifyException.cs diff --git a/crypto/BouncyCastle.Android.csproj b/crypto/BouncyCastle.Android.csproj index 13ca4f05c..541b534cd 100644 --- a/crypto/BouncyCastle.Android.csproj +++ b/crypto/BouncyCastle.Android.csproj @@ -1067,6 +1067,7 @@ + diff --git a/crypto/BouncyCastle.csproj b/crypto/BouncyCastle.csproj index f72c9c527..31910f963 100644 --- a/crypto/BouncyCastle.csproj +++ b/crypto/BouncyCastle.csproj @@ -1061,6 +1061,7 @@ + diff --git a/crypto/BouncyCastle.iOS.csproj b/crypto/BouncyCastle.iOS.csproj index b3bf7b4fa..f9269337a 100644 --- a/crypto/BouncyCastle.iOS.csproj +++ b/crypto/BouncyCastle.iOS.csproj @@ -1062,6 +1062,7 @@ + diff --git a/crypto/crypto.csproj b/crypto/crypto.csproj index 6f1fdcf61..fe6b5fa1c 100644 --- a/crypto/crypto.csproj +++ b/crypto/crypto.csproj @@ -5193,6 +5193,11 @@ SubType = "Code" BuildAction = "Compile" /> + + /// This exception will be thrown(only) when the connection is closed by the peer without sending a + /// close_notify warning alert. + /// + /// + /// If this happens, the TLS protocol cannot rule out truncation of the connection data (potentially + /// malicious). It may be possible to check for truncation via some property of a higher level protocol + /// built upon TLS, e.g.the Content-Length header for HTTPS. + /// + public class TlsNoCloseNotifyException + : EndOfStreamException + { + } +} diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index 6d5c93f40..98c6399d3 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -488,9 +488,7 @@ namespace Org.BouncyCastle.Crypto.Tls { if (!mRecordStream.ReadRecord()) { - // TODO It would be nicer to allow graceful connection close if between records - // this.FailWithError(AlertLevel.warning, AlertDescription.close_notify); - throw new EndOfStreamException(); + throw new TlsNoCloseNotifyException(); } } catch (TlsFatalAlert e) -- cgit 1.4.1 From d53ac55ba52d6913e093377e64ec9cc7b0bdbe37 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 23:09:57 +1030 Subject: Non-blocking TLS validates header of partially-received records - https://github.com/bcgit/bc-java/issues/133 --- crypto/src/crypto/tls/RecordStream.cs | 39 +++++++++++++++++++++++++++++++++-- crypto/src/crypto/tls/TlsProtocol.cs | 25 +++++++++++++++++++--- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/crypto/src/crypto/tls/RecordStream.cs b/crypto/src/crypto/tls/RecordStream.cs index d510ed94e..b1060fd6d 100644 --- a/crypto/src/crypto/tls/RecordStream.cs +++ b/crypto/src/crypto/tls/RecordStream.cs @@ -121,6 +121,40 @@ namespace Org.BouncyCastle.Crypto.Tls this.mPendingCipher = null; } + internal virtual void CheckRecordHeader(byte[] recordHeader) + { + byte type = TlsUtilities.ReadUint8(recordHeader, TLS_HEADER_TYPE_OFFSET); + + /* + * RFC 5246 6. If a TLS implementation receives an unexpected record type, it MUST send an + * unexpected_message alert. + */ + CheckType(type, AlertDescription.unexpected_message); + + if (!mRestrictReadVersion) + { + int version = TlsUtilities.ReadVersionRaw(recordHeader, TLS_HEADER_VERSION_OFFSET); + if ((version & 0xffffff00) != 0x0300) + throw new TlsFatalAlert(AlertDescription.illegal_parameter); + } + else + { + ProtocolVersion version = TlsUtilities.ReadVersion(recordHeader, TLS_HEADER_VERSION_OFFSET); + if (mReadVersion == null) + { + // Will be set later in 'readRecord' + } + else if (!version.Equals(mReadVersion)) + { + throw new TlsFatalAlert(AlertDescription.illegal_parameter); + } + } + + int length = TlsUtilities.ReadUint16(recordHeader, TLS_HEADER_LENGTH_OFFSET); + + CheckLength(length, mCiphertextLimit, AlertDescription.record_overflow); + } + internal virtual bool ReadRecord() { byte[] recordHeader = TlsUtilities.ReadAllOrNothing(TLS_HEADER_SIZE, mInput); @@ -155,6 +189,9 @@ namespace Org.BouncyCastle.Crypto.Tls } int length = TlsUtilities.ReadUint16(recordHeader, TLS_HEADER_LENGTH_OFFSET); + + CheckLength(length, mCiphertextLimit, AlertDescription.record_overflow); + byte[] plaintext = DecodeAndVerify(type, mInput, length); mHandler.ProcessRecord(type, plaintext, 0, plaintext.Length); return true; @@ -162,8 +199,6 @@ namespace Org.BouncyCastle.Crypto.Tls internal virtual byte[] DecodeAndVerify(byte type, Stream input, int len) { - CheckLength(len, mCiphertextLimit, AlertDescription.record_overflow); - byte[] buf = TlsUtilities.ReadFully(len, input); byte[] decoded = mReadCipher.DecodeCiphertext(mReadSeqNo++, type, buf, 0, buf.Length); diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index 98c6399d3..afdaf0075 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -482,6 +482,24 @@ namespace Org.BouncyCastle.Crypto.Tls return len; } + protected virtual void SafeCheckRecordHeader(byte[] recordHeader) + { + try + { + mRecordStream.CheckRecordHeader(recordHeader); + } + catch (TlsFatalAlert e) + { + this.FailWithError(AlertLevel.fatal, e.AlertDescription, "Failed to read record", e); + throw e; + } + catch (Exception e) + { + this.FailWithError(AlertLevel.fatal, AlertDescription.internal_error, "Failed to read record", e); + throw e; + } + } + protected virtual void SafeReadRecord() { try @@ -660,13 +678,14 @@ namespace Org.BouncyCastle.Crypto.Tls // loop while there are enough bytes to read the length of the next record while (mInputBuffers.Available >= RecordStream.TLS_HEADER_SIZE) { - byte[] header = new byte[RecordStream.TLS_HEADER_SIZE]; - mInputBuffers.Peek(header); + byte[] recordHeader = new byte[RecordStream.TLS_HEADER_SIZE]; + mInputBuffers.Peek(recordHeader); - int totalLength = TlsUtilities.ReadUint16(header, RecordStream.TLS_HEADER_LENGTH_OFFSET) + RecordStream.TLS_HEADER_SIZE; + int totalLength = TlsUtilities.ReadUint16(recordHeader, RecordStream.TLS_HEADER_LENGTH_OFFSET) + RecordStream.TLS_HEADER_SIZE; if (mInputBuffers.Available < totalLength) { // not enough bytes to read a whole record + SafeCheckRecordHeader(recordHeader); break; } -- cgit 1.4.1 From f27949c208ef2fbfcb6484db97bffe260109fb3c Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 23:29:36 +1030 Subject: Add TlsProtocol.CloseInput for use in non-blocking mode - tighten up handling of closure during handshake --- crypto/src/crypto/tls/TlsProtocol.cs | 43 ++++++++++++++++++++-- .../crypto/tls/test/TlsProtocolNonBlockingTest.cs | 1 + 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index afdaf0075..f259812f8 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -169,7 +169,8 @@ namespace Org.BouncyCastle.Crypto.Tls { if (this.mClosed) { - // TODO What kind of exception/alert? + // NOTE: Any close during the handshake should have raised an exception. + throw new TlsFatalAlert(AlertDescription.internal_error); } SafeReadRecord(); @@ -384,14 +385,16 @@ namespace Org.BouncyCastle.Crypto.Tls } else { - /* * RFC 5246 7.2.1. The other party MUST respond with a close_notify alert of its own * and close down the connection immediately, discarding any pending writes. */ - // TODO Can close_notify be a fatal alert? if (description == AlertDescription.close_notify) { + if (!mAppDataReady) + { + throw new TlsFatalAlert(AlertDescription.handshake_failure); + } HandleClose(false); } @@ -506,6 +509,10 @@ namespace Org.BouncyCastle.Crypto.Tls { if (!mRecordStream.ReadRecord()) { + if (!mAppDataReady) + { + throw new TlsFatalAlert(AlertDescription.handshake_failure); + } throw new TlsNoCloseNotifyException(); } } @@ -648,6 +655,26 @@ namespace Org.BouncyCastle.Crypto.Tls } } + /** + * Should be called in non-blocking mode when the input data reaches EOF. + */ + public virtual void CloseInput() + { + if (mBlocking) + throw new InvalidOperationException("Cannot use CloseInput() in blocking mode!"); + + if (mClosed) + return; + + if (mInputBuffers.Available > 0) + throw new EndOfStreamException(); + + if (!mAppDataReady) + throw new TlsFatalAlert(AlertDescription.handshake_failure); + + throw new TlsNoCloseNotifyException(); + } + /** * Offer input from an arbitrary source. Only allowed in non-blocking mode.
*
@@ -690,6 +717,16 @@ namespace Org.BouncyCastle.Crypto.Tls } SafeReadRecord(); + + if (mClosed) + { + if (mConnectionState != CS_END) + { + // NOTE: Any close during the handshake should have raised an exception. + throw new TlsFatalAlert(AlertDescription.internal_error); + } + break; + } } } diff --git a/crypto/test/src/crypto/tls/test/TlsProtocolNonBlockingTest.cs b/crypto/test/src/crypto/tls/test/TlsProtocolNonBlockingTest.cs index 477e287f1..68f2341ee 100644 --- a/crypto/test/src/crypto/tls/test/TlsProtocolNonBlockingTest.cs +++ b/crypto/test/src/crypto/tls/test/TlsProtocolNonBlockingTest.cs @@ -52,6 +52,7 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests // close the connection clientProtocol.Close(); PumpData(clientProtocol, serverProtocol, fragment); + serverProtocol.CloseInput(); CheckClosed(serverProtocol); CheckClosed(clientProtocol); } -- cgit 1.4.1 From 40069ebd6c370109a29b7e2f8ff685d4bc4d038f Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 23:33:37 +1030 Subject: Records with ContentType.heartbeat should give 'unexpected_message' - Heartbeat support currently incomplete (never negotiated) --- crypto/src/crypto/tls/RecordStream.cs | 2 +- crypto/src/crypto/tls/TlsProtocol.cs | 28 ++++++++++++---------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/crypto/src/crypto/tls/RecordStream.cs b/crypto/src/crypto/tls/RecordStream.cs index b1060fd6d..dc66d0df7 100644 --- a/crypto/src/crypto/tls/RecordStream.cs +++ b/crypto/src/crypto/tls/RecordStream.cs @@ -359,7 +359,7 @@ namespace Org.BouncyCastle.Crypto.Tls case ContentType.alert: case ContentType.change_cipher_spec: case ContentType.handshake: - case ContentType.heartbeat: + //case ContentType.heartbeat: break; default: throw new TlsFatalAlert(alertDescription); diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index f259812f8..05f06f5ea 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -261,23 +261,19 @@ namespace Org.BouncyCastle.Crypto.Tls ProcessHandshake(); break; } - case ContentType.heartbeat: - { - if (!mAppDataReady) - throw new TlsFatalAlert(AlertDescription.unexpected_message); - - // TODO[RFC 6520] - // mHeartbeatQueue.AddData(buf, offset, len); - // ProcessHeartbeat(); - break; - } + //case ContentType.heartbeat: + //{ + // if (!mAppDataReady) + // throw new TlsFatalAlert(AlertDescription.unexpected_message); + + // // TODO[RFC 6520] + // //mHeartbeatQueue.AddData(buf, offset, len); + // //ProcessHeartbeat(); + // break; + //} default: - /* - * Uh, we don't know this protocol. - * - * RFC2246 defines on page 13, that we should ignore this. - */ - break; + // Record type should already have been checked + throw new TlsFatalAlert(AlertDescription.internal_error); } } -- cgit 1.4.1 From bb077227fa263bed86980f8cc6aa6d4631c5010e Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 22 Mar 2017 23:43:22 +1030 Subject: Stop TcpListener in TlsServerTest --- crypto/test/src/crypto/tls/test/TlsServerTest.cs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/crypto/test/src/crypto/tls/test/TlsServerTest.cs b/crypto/test/src/crypto/tls/test/TlsServerTest.cs index 77adf22f7..7920cb59a 100644 --- a/crypto/test/src/crypto/tls/test/TlsServerTest.cs +++ b/crypto/test/src/crypto/tls/test/TlsServerTest.cs @@ -26,14 +26,21 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests TcpListener ss = new TcpListener(IPAddress.Any, port); ss.Start(); Stream stdout = Console.OpenStandardOutput(); - while (true) + try { - TcpClient s = ss.AcceptTcpClient(); - Console.WriteLine("--------------------------------------------------------------------------------"); - Console.WriteLine("Accepted " + s); - ServerThread st = new ServerThread(s, stdout); - Thread t = new Thread(new ThreadStart(st.Run)); - t.Start(); + while (true) + { + TcpClient s = ss.AcceptTcpClient(); + Console.WriteLine("--------------------------------------------------------------------------------"); + Console.WriteLine("Accepted " + s); + ServerThread st = new ServerThread(s, stdout); + Thread t = new Thread(new ThreadStart(st.Run)); + t.Start(); + } + } + finally + { + ss.Stop(); } } -- cgit 1.4.1 From 700364c5056bb3f7c39096733d9dc0b7f200e5f8 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 23 Mar 2017 00:26:41 +1030 Subject: SSLv3 must calculate the master secret earlier (from bc-java) - include SSLv3 in test suite --- crypto/src/crypto/tls/TlsClientProtocol.cs | 11 ++++++++++- crypto/src/crypto/tls/TlsServerProtocol.cs | 11 ++++++++++- crypto/test/src/crypto/tls/test/TlsTestSuite.cs | 5 +++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/crypto/src/crypto/tls/TlsClientProtocol.cs b/crypto/src/crypto/tls/TlsClientProtocol.cs index 4c2a0a545..6352a893f 100644 --- a/crypto/src/crypto/tls/TlsClientProtocol.cs +++ b/crypto/src/crypto/tls/TlsClientProtocol.cs @@ -384,10 +384,19 @@ namespace Org.BouncyCastle.Crypto.Tls SendClientKeyExchangeMessage(); this.mConnectionState = CS_CLIENT_KEY_EXCHANGE; + if (TlsUtilities.IsSsl(Context)) + { + EstablishMasterSecret(Context, mKeyExchange); + } + TlsHandshakeHash prepareFinishHash = mRecordStream.PrepareToFinish(); this.mSecurityParameters.sessionHash = GetCurrentPrfHash(Context, prepareFinishHash, null); - EstablishMasterSecret(Context, mKeyExchange); + if (!TlsUtilities.IsSsl(Context)) + { + EstablishMasterSecret(Context, mKeyExchange); + } + mRecordStream.SetPendingConnectionState(Peer.GetCompression(), Peer.GetCipher()); if (clientCreds != null && clientCreds is TlsSignerCredentials) diff --git a/crypto/src/crypto/tls/TlsServerProtocol.cs b/crypto/src/crypto/tls/TlsServerProtocol.cs index 38f2befea..fca685295 100644 --- a/crypto/src/crypto/tls/TlsServerProtocol.cs +++ b/crypto/src/crypto/tls/TlsServerProtocol.cs @@ -625,10 +625,19 @@ namespace Org.BouncyCastle.Crypto.Tls AssertEmpty(buf); + if (TlsUtilities.IsSsl(Context)) + { + EstablishMasterSecret(Context, mKeyExchange); + } + this.mPrepareFinishHash = mRecordStream.PrepareToFinish(); this.mSecurityParameters.sessionHash = GetCurrentPrfHash(Context, mPrepareFinishHash, null); - EstablishMasterSecret(Context, mKeyExchange); + if (!TlsUtilities.IsSsl(Context)) + { + EstablishMasterSecret(Context, mKeyExchange); + } + mRecordStream.SetPendingConnectionState(Peer.GetCompression(), Peer.GetCipher()); if (!mExpectSessionTicket) diff --git a/crypto/test/src/crypto/tls/test/TlsTestSuite.cs b/crypto/test/src/crypto/tls/test/TlsTestSuite.cs index 77cebe0a6..5dd9cf0f5 100644 --- a/crypto/test/src/crypto/tls/test/TlsTestSuite.cs +++ b/crypto/test/src/crypto/tls/test/TlsTestSuite.cs @@ -19,6 +19,7 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests IList testSuite = new ArrayList(); AddFallbackTests(testSuite); + AddVersionTests(testSuite, ProtocolVersion.SSLv3); AddVersionTests(testSuite, ProtocolVersion.TLSv10); AddVersionTests(testSuite, ProtocolVersion.TLSv11); AddVersionTests(testSuite, ProtocolVersion.TLSv12); @@ -194,10 +195,10 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests private static TlsTestConfig CreateTlsTestConfig(ProtocolVersion version) { TlsTestConfig c = new TlsTestConfig(); - c.clientMinimumVersion = ProtocolVersion.TLSv10; + c.clientMinimumVersion = ProtocolVersion.SSLv3; c.clientOfferVersion = ProtocolVersion.TLSv12; c.serverMaximumVersion = version; - c.serverMinimumVersion = ProtocolVersion.TLSv10; + c.serverMinimumVersion = ProtocolVersion.SSLv3; return c; } } -- cgit 1.4.1 From 916d94d230a54e03ceb6210daf6aa7207c5821f1 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 23 Mar 2017 03:55:36 +1030 Subject: Support DH_anon cipher suites in TLS API --- crypto/src/crypto/tls/DefaultTlsClient.cs | 1 + crypto/src/crypto/tls/DefaultTlsServer.cs | 2 + crypto/src/crypto/tls/TlsDHKeyExchange.cs | 46 +++++++++++++++--- crypto/src/crypto/tls/TlsProtocol.cs | 4 ++ crypto/src/crypto/tls/TlsUtilities.cs | 78 ++++++++++++++++++++++++------- 5 files changed, 107 insertions(+), 24 deletions(-) diff --git a/crypto/src/crypto/tls/DefaultTlsClient.cs b/crypto/src/crypto/tls/DefaultTlsClient.cs index 7dadf8a1a..32a86e503 100644 --- a/crypto/src/crypto/tls/DefaultTlsClient.cs +++ b/crypto/src/crypto/tls/DefaultTlsClient.cs @@ -52,6 +52,7 @@ namespace Org.BouncyCastle.Crypto.Tls switch (keyExchangeAlgorithm) { + case KeyExchangeAlgorithm.DH_anon: case KeyExchangeAlgorithm.DH_DSS: case KeyExchangeAlgorithm.DH_RSA: return CreateDHKeyExchange(keyExchangeAlgorithm); diff --git a/crypto/src/crypto/tls/DefaultTlsServer.cs b/crypto/src/crypto/tls/DefaultTlsServer.cs index 87d0539b3..8b9a7c9a0 100644 --- a/crypto/src/crypto/tls/DefaultTlsServer.cs +++ b/crypto/src/crypto/tls/DefaultTlsServer.cs @@ -79,6 +79,7 @@ namespace Org.BouncyCastle.Crypto.Tls case KeyExchangeAlgorithm.DHE_DSS: return GetDsaSignerCredentials(); + case KeyExchangeAlgorithm.DH_anon: case KeyExchangeAlgorithm.ECDH_anon: return null; @@ -104,6 +105,7 @@ namespace Org.BouncyCastle.Crypto.Tls switch (keyExchangeAlgorithm) { + case KeyExchangeAlgorithm.DH_anon: case KeyExchangeAlgorithm.DH_DSS: case KeyExchangeAlgorithm.DH_RSA: return CreateDHKeyExchange(keyExchangeAlgorithm); diff --git a/crypto/src/crypto/tls/TlsDHKeyExchange.cs b/crypto/src/crypto/tls/TlsDHKeyExchange.cs index 93ef1fa4a..eec9daaca 100644 --- a/crypto/src/crypto/tls/TlsDHKeyExchange.cs +++ b/crypto/src/crypto/tls/TlsDHKeyExchange.cs @@ -27,6 +27,7 @@ namespace Org.BouncyCastle.Crypto.Tls { switch (keyExchange) { + case KeyExchangeAlgorithm.DH_anon: case KeyExchangeAlgorithm.DH_RSA: case KeyExchangeAlgorithm.DH_DSS: this.mTlsSigner = null; @@ -56,11 +57,14 @@ namespace Org.BouncyCastle.Crypto.Tls public override void SkipServerCredentials() { - throw new TlsFatalAlert(AlertDescription.unexpected_message); + if (mKeyExchange != KeyExchangeAlgorithm.DH_anon) + throw new TlsFatalAlert(AlertDescription.unexpected_message); } public override void ProcessServerCertificate(Certificate serverCertificate) { + if (mKeyExchange == KeyExchangeAlgorithm.DH_anon) + throw new TlsFatalAlert(AlertDescription.unexpected_message); if (serverCertificate.IsEmpty) throw new TlsFatalAlert(AlertDescription.bad_certificate); @@ -109,9 +113,9 @@ namespace Org.BouncyCastle.Crypto.Tls { switch (mKeyExchange) { + case KeyExchangeAlgorithm.DH_anon: case KeyExchangeAlgorithm.DHE_DSS: case KeyExchangeAlgorithm.DHE_RSA: - case KeyExchangeAlgorithm.DH_anon: return true; default: return false; @@ -119,6 +123,32 @@ namespace Org.BouncyCastle.Crypto.Tls } } + public override byte[] GenerateServerKeyExchange() + { + if (!RequiresServerKeyExchange) + return null; + + // DH_anon is handled here, DHE_* in a subclass + + MemoryStream buf = new MemoryStream(); + this.mDHAgreePrivateKey = TlsDHUtilities.GenerateEphemeralServerKeyExchange(mContext.SecureRandom, + this.mDHParameters, buf); + return buf.ToArray(); + } + + public override void ProcessServerKeyExchange(Stream input) + { + if (!RequiresServerKeyExchange) + throw new TlsFatalAlert(AlertDescription.unexpected_message); + + // DH_anon is handled here, DHE_* in a subclass + + ServerDHParams dhParams = ServerDHParams.Parse(input); + + this.mDHAgreePublicKey = TlsDHUtilities.ValidateDHPublicKey(dhParams.PublicKey); + this.mDHParameters = ValidateDHParameters(mDHAgreePublicKey.Parameters); + } + public override void ValidateCertificateRequest(CertificateRequest certificateRequest) { byte[] types = certificateRequest.CertificateTypes; @@ -140,6 +170,9 @@ namespace Org.BouncyCastle.Crypto.Tls public override void ProcessClientCredentials(TlsCredentials clientCredentials) { + if (mKeyExchange == KeyExchangeAlgorithm.DH_anon) + throw new TlsFatalAlert(AlertDescription.internal_error); + if (clientCredentials is TlsAgreementCredentials) { // TODO Validate client cert has matching parameters (see 'areCompatibleParameters')? @@ -172,12 +205,11 @@ namespace Org.BouncyCastle.Crypto.Tls public override void ProcessClientCertificate(Certificate clientCertificate) { - // TODO Extract the public key and validate + if (mKeyExchange == KeyExchangeAlgorithm.DH_anon) + throw new TlsFatalAlert(AlertDescription.unexpected_message); - /* - * TODO If the certificate is 'fixed', take the public key as dhAgreePublicKey and check - * that the parameters match the server's (see 'areCompatibleParameters'). - */ + // TODO Extract the public key + // TODO If the certificate is 'fixed', take the public key as dhAgreePublicKey } public override void ProcessClientKeyExchange(Stream input) diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index 05f06f5ea..c6c3375d9 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -1214,6 +1214,9 @@ namespace Org.BouncyCastle.Crypto.Tls switch (ciphersuite) { + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_128_GCM_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA256: case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_GCM_SHA256: case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256: @@ -1317,6 +1320,7 @@ namespace Org.BouncyCastle.Crypto.Tls throw new TlsFatalAlert(AlertDescription.illegal_parameter); } + case CipherSuite.TLS_DH_anon_WITH_AES_256_GCM_SHA384: case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_GCM_SHA384: case CipherSuite.TLS_DH_DSS_WITH_AES_256_GCM_SHA384: case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_256_GCM_SHA384: diff --git a/crypto/src/crypto/tls/TlsUtilities.cs b/crypto/src/crypto/tls/TlsUtilities.cs index d51a8ff48..4f709a645 100644 --- a/crypto/src/crypto/tls/TlsUtilities.cs +++ b/crypto/src/crypto/tls/TlsUtilities.cs @@ -1260,6 +1260,7 @@ namespace Org.BouncyCastle.Crypto.Tls { switch (ciphersuite) { + case CipherSuite.TLS_DH_anon_WITH_3DES_EDE_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA: case CipherSuite.TLS_DH_RSA_WITH_3DES_EDE_CBC_SHA: case CipherSuite.TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA: @@ -1279,6 +1280,8 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_SRP_SHA_WITH_3DES_EDE_CBC_SHA: return EncryptionAlgorithm.cls_3DES_EDE_CBC; + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA256: case CipherSuite.TLS_DH_DSS_WITH_AES_128_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_AES_128_CBC_SHA256: case CipherSuite.TLS_DH_RSA_WITH_AES_128_CBC_SHA: @@ -1325,6 +1328,7 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_RSA_WITH_AES_128_CCM_8: return EncryptionAlgorithm.AES_128_CCM_8; + case CipherSuite.TLS_DH_anon_WITH_AES_128_GCM_SHA256: case CipherSuite.TLS_DH_DSS_WITH_AES_128_GCM_SHA256: case CipherSuite.TLS_DH_RSA_WITH_AES_128_GCM_SHA256: case CipherSuite.TLS_DHE_DSS_WITH_AES_128_GCM_SHA256: @@ -1347,6 +1351,8 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.DRAFT_TLS_PSK_WITH_AES_128_OCB: return EncryptionAlgorithm.AES_128_OCB_TAGLEN96; + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA256: case CipherSuite.TLS_DH_DSS_WITH_AES_256_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_AES_256_CBC_SHA256: case CipherSuite.TLS_DH_RSA_WITH_AES_256_CBC_SHA: @@ -1393,6 +1399,7 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_RSA_WITH_AES_256_CCM_8: return EncryptionAlgorithm.AES_256_CCM_8; + case CipherSuite.TLS_DH_anon_WITH_AES_256_GCM_SHA384: case CipherSuite.TLS_DH_DSS_WITH_AES_256_GCM_SHA384: case CipherSuite.TLS_DH_RSA_WITH_AES_256_GCM_SHA384: case CipherSuite.TLS_DHE_DSS_WITH_AES_256_GCM_SHA384: @@ -1415,17 +1422,16 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.DRAFT_TLS_PSK_WITH_AES_256_OCB: return EncryptionAlgorithm.AES_256_OCB_TAGLEN96; + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_128_CBC_SHA: - case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_128_CBC_SHA: - case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA: - case CipherSuite.TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA: - case CipherSuite.TLS_RSA_WITH_CAMELLIA_128_CBC_SHA: - return EncryptionAlgorithm.CAMELLIA_128_CBC; - case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_128_CBC_SHA256: + case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_128_CBC_SHA: case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_128_CBC_SHA256: + case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA: case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_DHE_PSK_WITH_CAMELLIA_128_CBC_SHA256: + case CipherSuite.TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA: case CipherSuite.TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_ECDH_ECDSA_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_ECDH_RSA_WITH_CAMELLIA_128_CBC_SHA256: @@ -1433,10 +1439,12 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_ECDHE_PSK_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_ECDHE_RSA_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_PSK_WITH_CAMELLIA_128_CBC_SHA256: - case CipherSuite.TLS_RSA_PSK_WITH_CAMELLIA_128_CBC_SHA256: + case CipherSuite.TLS_RSA_WITH_CAMELLIA_128_CBC_SHA: case CipherSuite.TLS_RSA_WITH_CAMELLIA_128_CBC_SHA256: + case CipherSuite.TLS_RSA_PSK_WITH_CAMELLIA_128_CBC_SHA256: return EncryptionAlgorithm.CAMELLIA_128_CBC; + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_GCM_SHA256: case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_128_GCM_SHA256: case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_128_GCM_SHA256: case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_128_GCM_SHA256: @@ -1451,30 +1459,29 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_RSA_WITH_CAMELLIA_128_GCM_SHA256: return EncryptionAlgorithm.CAMELLIA_128_GCM; + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256: case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_256_CBC_SHA: - case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_256_CBC_SHA: - case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA: - case CipherSuite.TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA: - case CipherSuite.TLS_RSA_WITH_CAMELLIA_256_CBC_SHA: - return EncryptionAlgorithm.CAMELLIA_256_CBC; - case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_256_CBC_SHA256: + case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_256_CBC_SHA: case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_256_CBC_SHA256: + case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA: case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA256: - case CipherSuite.TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA256: - case CipherSuite.TLS_RSA_WITH_CAMELLIA_256_CBC_SHA256: - return EncryptionAlgorithm.CAMELLIA_256_CBC; - case CipherSuite.TLS_DHE_PSK_WITH_CAMELLIA_256_CBC_SHA384: + case CipherSuite.TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA: + case CipherSuite.TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA256: case CipherSuite.TLS_ECDH_ECDSA_WITH_CAMELLIA_256_CBC_SHA384: case CipherSuite.TLS_ECDH_RSA_WITH_CAMELLIA_256_CBC_SHA384: case CipherSuite.TLS_ECDHE_ECDSA_WITH_CAMELLIA_256_CBC_SHA384: case CipherSuite.TLS_ECDHE_PSK_WITH_CAMELLIA_256_CBC_SHA384: case CipherSuite.TLS_ECDHE_RSA_WITH_CAMELLIA_256_CBC_SHA384: case CipherSuite.TLS_PSK_WITH_CAMELLIA_256_CBC_SHA384: + case CipherSuite.TLS_RSA_WITH_CAMELLIA_256_CBC_SHA: + case CipherSuite.TLS_RSA_WITH_CAMELLIA_256_CBC_SHA256: case CipherSuite.TLS_RSA_PSK_WITH_CAMELLIA_256_CBC_SHA384: return EncryptionAlgorithm.CAMELLIA_256_CBC; + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_GCM_SHA384: case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_256_GCM_SHA384: case CipherSuite.TLS_DH_RSA_WITH_CAMELLIA_256_GCM_SHA384: case CipherSuite.TLS_DHE_DSS_WITH_CAMELLIA_256_GCM_SHA384: @@ -1542,6 +1549,7 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_RSA_PSK_WITH_RC4_128_SHA: return EncryptionAlgorithm.RC4_128; + case CipherSuite.TLS_DH_anon_WITH_SEED_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_SEED_CBC_SHA: case CipherSuite.TLS_DH_RSA_WITH_SEED_CBC_SHA: case CipherSuite.TLS_DHE_DSS_WITH_SEED_CBC_SHA: @@ -1558,6 +1566,23 @@ namespace Org.BouncyCastle.Crypto.Tls { switch (ciphersuite) { + case CipherSuite.TLS_DH_anon_WITH_3DES_EDE_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_128_GCM_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_256_GCM_SHA384: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_GCM_SHA256: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_GCM_SHA384: + case CipherSuite.TLS_DH_anon_WITH_RC4_128_MD5: + case CipherSuite.TLS_DH_anon_WITH_SEED_CBC_SHA: + return KeyExchangeAlgorithm.DH_anon; + case CipherSuite.TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_AES_128_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_AES_128_CBC_SHA256: @@ -1837,6 +1862,10 @@ namespace Org.BouncyCastle.Crypto.Tls { switch (ciphersuite) { + case CipherSuite.TLS_DH_anon_WITH_AES_128_GCM_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_256_GCM_SHA384: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_GCM_SHA256: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_GCM_SHA384: case CipherSuite.TLS_DH_DSS_WITH_AES_128_GCM_SHA256: case CipherSuite.TLS_DH_DSS_WITH_AES_256_GCM_SHA384: case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_128_GCM_SHA256: @@ -1926,10 +1955,17 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_RSA_WITH_CAMELLIA_256_GCM_SHA384: return MacAlgorithm.cls_null; + case CipherSuite.TLS_DH_anon_WITH_RC4_128_MD5: case CipherSuite.TLS_RSA_WITH_NULL_MD5: case CipherSuite.TLS_RSA_WITH_RC4_128_MD5: return MacAlgorithm.hmac_md5; + case CipherSuite.TLS_DH_anon_WITH_3DES_EDE_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA: + case CipherSuite.TLS_DH_anon_WITH_SEED_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_AES_128_CBC_SHA: case CipherSuite.TLS_DH_DSS_WITH_AES_256_CBC_SHA: @@ -2018,6 +2054,10 @@ namespace Org.BouncyCastle.Crypto.Tls case CipherSuite.TLS_SRP_SHA_WITH_AES_256_CBC_SHA: return MacAlgorithm.hmac_sha1; + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256: case CipherSuite.TLS_DH_DSS_WITH_AES_128_CBC_SHA256: case CipherSuite.TLS_DH_DSS_WITH_AES_256_CBC_SHA256: case CipherSuite.TLS_DH_DSS_WITH_CAMELLIA_128_CBC_SHA256: @@ -2092,6 +2132,10 @@ namespace Org.BouncyCastle.Crypto.Tls { switch (ciphersuite) { + case CipherSuite.TLS_DH_anon_WITH_AES_128_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_128_GCM_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_256_CBC_SHA256: + case CipherSuite.TLS_DH_anon_WITH_AES_256_GCM_SHA384: case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_CBC_SHA256: case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_128_GCM_SHA256: case CipherSuite.TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256: -- cgit 1.4.1 From 52a6a57be973336a28d3608856f80f425527f9c4 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 23 Mar 2017 20:02:14 +1030 Subject: Avoid extra copy when processing TLS handshake messages - move handshake hash updates (outbound) to a more sensible place --- crypto/src/crypto/tls/ByteQueue.cs | 93 ++++++++++++++++++++---------- crypto/src/crypto/tls/RecordStream.cs | 33 +++++++---- crypto/src/crypto/tls/TlsClientProtocol.cs | 4 +- crypto/src/crypto/tls/TlsProtocol.cs | 21 +++---- crypto/src/crypto/tls/TlsServerProtocol.cs | 4 +- crypto/src/util/io/BaseOutputStream.cs | 7 ++- 6 files changed, 104 insertions(+), 58 deletions(-) diff --git a/crypto/src/crypto/tls/ByteQueue.cs b/crypto/src/crypto/tls/ByteQueue.cs index f9398bbaf..c23ec2fbf 100644 --- a/crypto/src/crypto/tls/ByteQueue.cs +++ b/crypto/src/crypto/tls/ByteQueue.cs @@ -1,4 +1,5 @@ using System; +using System.IO; namespace Org.BouncyCastle.Crypto.Tls { @@ -57,36 +58,14 @@ namespace Org.BouncyCastle.Crypto.Tls this.databuf = new byte[capacity]; } - /// Read data from the buffer. - /// The buffer where the read data will be copied to. - /// How many bytes to skip at the beginning of buf. - /// How many bytes to read at all. - /// How many bytes from our data to skip. - public void Read( - byte[] buf, - int offset, - int len, - int skip) - { - if ((buf.Length - offset) < len) - { - throw new ArgumentException("Buffer size of " + buf.Length + " is too small for a read of " + len + " bytes"); - } - if ((available - skip) < len) - { - throw new InvalidOperationException("Not enough data to read"); - } - Array.Copy(databuf, skipped + skip, buf, offset, len); - } - /// Add some data to our buffer. /// A byte-array to read data from. /// How many bytes to skip at the beginning of the array. /// How many bytes to read from the array. public void AddData( - byte[] data, - int offset, - int len) + byte[] data, + int offset, + int len) { if ((skipped + available + len) > databuf.Length) { @@ -108,6 +87,64 @@ namespace Org.BouncyCastle.Crypto.Tls available += len; } + /// The number of bytes which are available in this buffer. + public int Available + { + get { return available; } + } + + /// Copy some bytes from the beginning of the data to the provided Stream. + /// The Stream to copy the bytes to. + /// How many bytes to copy. + /// If insufficient data is available. + /// If there is a problem copying the data. + public void CopyTo(Stream output, int length) + { + if (length > available) + throw new InvalidOperationException("Cannot copy " + length + " bytes, only got " + available); + + output.Write(databuf, skipped, length); + } + + /// Read data from the buffer. + /// The buffer where the read data will be copied to. + /// How many bytes to skip at the beginning of buf. + /// How many bytes to read at all. + /// How many bytes from our data to skip. + public void Read( + byte[] buf, + int offset, + int len, + int skip) + { + if ((buf.Length - offset) < len) + { + throw new ArgumentException("Buffer size of " + buf.Length + " is too small for a read of " + len + " bytes"); + } + if ((available - skip) < len) + { + throw new InvalidOperationException("Not enough data to read"); + } + Array.Copy(databuf, skipped + skip, buf, offset, len); + } + + /// Return a MemoryStream over some bytes at the beginning of the data. + /// How many bytes will be readable. + /// A MemoryStream over the data. + /// If insufficient data is available. + public MemoryStream ReadFrom(int length) + { + if (length > available) + throw new InvalidOperationException("Cannot read " + length + " bytes, only got " + available); + + int position = skipped; + + available -= length; + skipped += length; + + return new MemoryStream(databuf, position, length, false); + } + /// Remove some bytes from our data from the beginning. /// How many bytes to remove. public void RemoveData( @@ -137,11 +174,5 @@ namespace Org.BouncyCastle.Crypto.Tls RemoveData(buf, 0, len, skip); return buf; } - - /// The number of bytes which are available in this buffer. - public int Available - { - get { return available; } - } } } diff --git a/crypto/src/crypto/tls/RecordStream.cs b/crypto/src/crypto/tls/RecordStream.cs index dc66d0df7..cddd5e6d7 100644 --- a/crypto/src/crypto/tls/RecordStream.cs +++ b/crypto/src/crypto/tls/RecordStream.cs @@ -2,6 +2,7 @@ using System; using System.IO; using Org.BouncyCastle.Utilities; +using Org.BouncyCastle.Utilities.IO; namespace Org.BouncyCastle.Crypto.Tls { @@ -24,6 +25,7 @@ namespace Org.BouncyCastle.Crypto.Tls private MemoryStream mBuffer = new MemoryStream(); private TlsHandshakeHash mHandshakeHash = null; + private readonly BaseOutputStream mHandshakeHashUpdater; private ProtocolVersion mReadVersion = null, mWriteVersion = null; private bool mRestrictReadVersion = true; @@ -37,6 +39,7 @@ namespace Org.BouncyCastle.Crypto.Tls this.mOutput = output; this.mReadCompression = new TlsNullCompression(); this.mWriteCompression = this.mReadCompression; + this.mHandshakeHashUpdater = new HandshakeHashUpdateStream(this); } internal virtual void Init(TlsContext context) @@ -257,11 +260,6 @@ namespace Org.BouncyCastle.Crypto.Tls if (plaintextLength < 1 && type != ContentType.application_data) throw new TlsFatalAlert(AlertDescription.internal_error); - if (type == ContentType.handshake) - { - UpdateHandshakeData(plaintext, plaintextOffset, plaintextLength); - } - Stream cOut = mWriteCompression.Compress(mBuffer); byte[] ciphertext; @@ -308,6 +306,11 @@ namespace Org.BouncyCastle.Crypto.Tls get { return mHandshakeHash; } } + internal virtual Stream HandshakeHashUpdater + { + get { return mHandshakeHashUpdater; } + } + internal virtual TlsHandshakeHash PrepareToFinish() { TlsHandshakeHash result = mHandshakeHash; @@ -315,11 +318,6 @@ namespace Org.BouncyCastle.Crypto.Tls return result; } - internal virtual void UpdateHandshakeData(byte[] message, int offset, int len) - { - mHandshakeHash.BlockUpdate(message, offset, len); - } - internal virtual void SafeClose() { try @@ -371,5 +369,20 @@ namespace Org.BouncyCastle.Crypto.Tls if (length > limit) throw new TlsFatalAlert(alertDescription); } + + private class HandshakeHashUpdateStream + : BaseOutputStream + { + private readonly RecordStream mOuter; + public HandshakeHashUpdateStream(RecordStream mOuter) + { + this.mOuter = mOuter; + } + + public override void Write(byte[] buf, int off, int len) + { + mOuter.mHandshakeHash.BlockUpdate(buf, off, len); + } + } } } diff --git a/crypto/src/crypto/tls/TlsClientProtocol.cs b/crypto/src/crypto/tls/TlsClientProtocol.cs index 6352a893f..d24d13bb5 100644 --- a/crypto/src/crypto/tls/TlsClientProtocol.cs +++ b/crypto/src/crypto/tls/TlsClientProtocol.cs @@ -135,10 +135,8 @@ namespace Org.BouncyCastle.Crypto.Tls get { return mTlsClient; } } - protected override void HandleHandshakeMessage(byte type, byte[] data) + protected override void HandleHandshakeMessage(byte type, MemoryStream buf) { - MemoryStream buf = new MemoryStream(data, false); - if (this.mResumedSession) { if (type != HandshakeType.finished || this.mConnectionState != CS_SERVER_HELLO) diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index c6c3375d9..eb0ad1101 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -114,7 +114,7 @@ namespace Org.BouncyCastle.Crypto.Tls { } - protected abstract void HandleHandshakeMessage(byte type, byte[] buf); + protected abstract void HandleHandshakeMessage(byte type, MemoryStream buf); protected virtual void HandleWarningMessage(byte description) { @@ -291,18 +291,14 @@ namespace Org.BouncyCastle.Crypto.Tls byte[] beginning = new byte[4]; mHandshakeQueue.Read(beginning, 0, 4, 0); byte type = TlsUtilities.ReadUint8(beginning, 0); - int len = TlsUtilities.ReadUint24(beginning, 1); + int length = TlsUtilities.ReadUint24(beginning, 1); + int totalLength = 4 + length; /* * Check if we have enough bytes in the buffer to read the full message. */ - if (mHandshakeQueue.Available >= (len + 4)) + if (mHandshakeQueue.Available >= totalLength) { - /* - * Read the message. - */ - byte[] buf = mHandshakeQueue.RemoveData(len, 4); - CheckReceivedChangeCipherSpec(mConnectionState == CS_END || type == HandshakeType.finished); /* @@ -325,12 +321,15 @@ namespace Org.BouncyCastle.Crypto.Tls this.mExpectedVerifyData = CreateVerifyData(!ctx.IsServer); } - mRecordStream.UpdateHandshakeData(beginning, 0, 4); - mRecordStream.UpdateHandshakeData(buf, 0, len); + mHandshakeQueue.CopyTo(mRecordStream.HandshakeHashUpdater, totalLength); break; } } + mHandshakeQueue.RemoveData(4); + + MemoryStream buf = mHandshakeQueue.ReadFrom(length); + /* * Now, parse the message. */ @@ -629,6 +628,8 @@ namespace Org.BouncyCastle.Crypto.Tls protected virtual void WriteHandshakeMessage(byte[] buf, int off, int len) { + mRecordStream.HandshakeHashUpdater.Write(buf, off, len); + while (len > 0) { // Fragment data according to the current fragment limit. diff --git a/crypto/src/crypto/tls/TlsServerProtocol.cs b/crypto/src/crypto/tls/TlsServerProtocol.cs index fca685295..6642f43f4 100644 --- a/crypto/src/crypto/tls/TlsServerProtocol.cs +++ b/crypto/src/crypto/tls/TlsServerProtocol.cs @@ -124,10 +124,8 @@ namespace Org.BouncyCastle.Crypto.Tls get { return mTlsServer; } } - protected override void HandleHandshakeMessage(byte type, byte[] data) + protected override void HandleHandshakeMessage(byte type, MemoryStream buf) { - MemoryStream buf = new MemoryStream(data); - switch (type) { case HandshakeType.client_hello: diff --git a/crypto/src/util/io/BaseOutputStream.cs b/crypto/src/util/io/BaseOutputStream.cs index a0608d111..0dbe821de 100644 --- a/crypto/src/util/io/BaseOutputStream.cs +++ b/crypto/src/util/io/BaseOutputStream.cs @@ -60,5 +60,10 @@ namespace Org.BouncyCastle.Utilities.IO { Write(buffer, 0, buffer.Length); } - } + + public override void WriteByte(byte b) + { + Write(new byte[]{ b }, 0, 1); + } + } } -- cgit 1.4.1 From 549033c625177468f1fe876a7d1deed27df3d27b Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 23 Mar 2017 22:44:22 +1030 Subject: Improvements to WriteHandshakeMessage method --- crypto/src/crypto/tls/TlsProtocol.cs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index eb0ad1101..667b3eb33 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -628,16 +628,24 @@ namespace Org.BouncyCastle.Crypto.Tls protected virtual void WriteHandshakeMessage(byte[] buf, int off, int len) { - mRecordStream.HandshakeHashUpdater.Write(buf, off, len); + if (len < 4) + throw new TlsFatalAlert(AlertDescription.internal_error); - while (len > 0) + byte type = TlsUtilities.ReadUint8(buf, off); + if (type != HandshakeType.hello_request) + { + mRecordStream.HandshakeHashUpdater.Write(buf, off, len); + } + + int total = 0; + do { // Fragment data according to the current fragment limit. - int toWrite = System.Math.Min(len, mRecordStream.GetPlaintextLimit()); - SafeWriteRecord(ContentType.handshake, buf, off, toWrite); - off += toWrite; - len -= toWrite; + int toWrite = System.Math.Min(len - total, mRecordStream.GetPlaintextLimit()); + SafeWriteRecord(ContentType.handshake, buf, off + total, toWrite); + total += toWrite; } + while (total < len); } /// The secure bidirectional stream for this connection -- cgit 1.4.1 From 9ab556d76be2afa047b14e3c42359f9d2fc15a83 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 23 Mar 2017 23:35:29 +1030 Subject: Reduce TLS memory usage --- crypto/src/crypto/tls/ByteQueue.cs | 20 ++++++++++++++++++++ crypto/src/crypto/tls/TlsClientProtocol.cs | 2 -- crypto/src/crypto/tls/TlsProtocol.cs | 7 ++++++- crypto/src/crypto/tls/TlsServerProtocol.cs | 1 - 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/crypto/src/crypto/tls/ByteQueue.cs b/crypto/src/crypto/tls/ByteQueue.cs index c23ec2fbf..f9d4ee606 100644 --- a/crypto/src/crypto/tls/ByteQueue.cs +++ b/crypto/src/crypto/tls/ByteQueue.cs @@ -174,5 +174,25 @@ namespace Org.BouncyCastle.Crypto.Tls RemoveData(buf, 0, len, skip); return buf; } + + public void Shrink() + { + if (available == 0) + { + databuf = TlsUtilities.EmptyBytes; + skipped = 0; + } + else + { + int desiredSize = ByteQueue.NextTwoPow(available); + if (desiredSize < databuf.Length) + { + byte[] tmp = new byte[desiredSize]; + Array.Copy(databuf, skipped, tmp, 0, available); + databuf = tmp; + skipped = 0; + } + } + } } } diff --git a/crypto/src/crypto/tls/TlsClientProtocol.cs b/crypto/src/crypto/tls/TlsClientProtocol.cs index d24d13bb5..24fb3f63d 100644 --- a/crypto/src/crypto/tls/TlsClientProtocol.cs +++ b/crypto/src/crypto/tls/TlsClientProtocol.cs @@ -147,7 +147,6 @@ namespace Org.BouncyCastle.Crypto.Tls SendFinishedMessage(); this.mConnectionState = CS_CLIENT_FINISHED; - this.mConnectionState = CS_END; CompleteHandshake(); return; @@ -241,7 +240,6 @@ namespace Org.BouncyCastle.Crypto.Tls ProcessFinishedMessage(buf); this.mConnectionState = CS_SERVER_FINISHED; - this.mConnectionState = CS_END; CompleteHandshake(); break; diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index 667b3eb33..69361bf6f 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -43,7 +43,7 @@ namespace Org.BouncyCastle.Crypto.Tls /* * Queues for data from some protocols. */ - private ByteQueue mApplicationDataQueue = new ByteQueue(); + private ByteQueue mApplicationDataQueue = new ByteQueue(0); private ByteQueue mAlertQueue = new ByteQueue(2); private ByteQueue mHandshakeQueue = new ByteQueue(); // private ByteQueue mHeartbeatQueue = new ByteQueue(); @@ -182,6 +182,11 @@ namespace Org.BouncyCastle.Crypto.Tls { try { + this.mConnectionState = CS_END; + + this.mAlertQueue.Shrink(); + this.mHandshakeQueue.Shrink(); + this.mRecordStream.FinaliseHandshake(); this.mAppDataSplitEnabled = !TlsUtilities.IsTlsV11(Context); diff --git a/crypto/src/crypto/tls/TlsServerProtocol.cs b/crypto/src/crypto/tls/TlsServerProtocol.cs index 6642f43f4..db823539a 100644 --- a/crypto/src/crypto/tls/TlsServerProtocol.cs +++ b/crypto/src/crypto/tls/TlsServerProtocol.cs @@ -365,7 +365,6 @@ namespace Org.BouncyCastle.Crypto.Tls SendFinishedMessage(); this.mConnectionState = CS_SERVER_FINISHED; - this.mConnectionState = CS_END; CompleteHandshake(); break; -- cgit 1.4.1 From 29d257cfe975d62169584ad1b8896157d415c5d7 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Fri, 24 Mar 2017 17:58:58 +1030 Subject: Avoid copies when handshake records not fragmented --- crypto/src/crypto/tls/ByteQueue.cs | 15 +++- crypto/src/crypto/tls/TlsProtocol.cs | 128 ++++++++++++++++++----------------- 2 files changed, 81 insertions(+), 62 deletions(-) diff --git a/crypto/src/crypto/tls/ByteQueue.cs b/crypto/src/crypto/tls/ByteQueue.cs index f9d4ee606..b4df6850e 100644 --- a/crypto/src/crypto/tls/ByteQueue.cs +++ b/crypto/src/crypto/tls/ByteQueue.cs @@ -48,6 +48,8 @@ namespace Org.BouncyCastle.Crypto.Tls */ private int available = 0; + private bool readOnlyBuf = false; + public ByteQueue() : this(DefaultCapacity) { @@ -55,7 +57,15 @@ namespace Org.BouncyCastle.Crypto.Tls public ByteQueue(int capacity) { - this.databuf = new byte[capacity]; + this.databuf = capacity == 0 ? TlsUtilities.EmptyBytes : new byte[capacity]; + } + + public ByteQueue(byte[] buf, int off, int len) + { + this.databuf = buf; + this.skipped = off; + this.available = len; + this.readOnlyBuf = true; } /// Add some data to our buffer. @@ -67,6 +77,9 @@ namespace Org.BouncyCastle.Crypto.Tls int offset, int len) { + if (readOnlyBuf) + throw new InvalidOperationException("Cannot add data to read-only buffer"); + if ((skipped + available + len) > databuf.Length) { int desiredSize = ByteQueue.NextTwoPow(available + len); diff --git a/crypto/src/crypto/tls/TlsProtocol.cs b/crypto/src/crypto/tls/TlsProtocol.cs index 69361bf6f..490580fad 100644 --- a/crypto/src/crypto/tls/TlsProtocol.cs +++ b/crypto/src/crypto/tls/TlsProtocol.cs @@ -45,7 +45,7 @@ namespace Org.BouncyCastle.Crypto.Tls */ private ByteQueue mApplicationDataQueue = new ByteQueue(0); private ByteQueue mAlertQueue = new ByteQueue(2); - private ByteQueue mHandshakeQueue = new ByteQueue(); + private ByteQueue mHandshakeQueue = new ByteQueue(0); // private ByteQueue mHeartbeatQueue = new ByteQueue(); /* @@ -233,7 +233,7 @@ namespace Org.BouncyCastle.Crypto.Tls } } - protected internal void ProcessRecord(byte protocol, byte[] buf, int offset, int len) + protected internal void ProcessRecord(byte protocol, byte[] buf, int off, int len) { /* * Have a look at the protocol type, and add it to the correct queue. @@ -242,8 +242,8 @@ namespace Org.BouncyCastle.Crypto.Tls { case ContentType.alert: { - mAlertQueue.AddData(buf, offset, len); - ProcessAlert(); + mAlertQueue.AddData(buf, off, len); + ProcessAlertQueue(); break; } case ContentType.application_data: @@ -251,19 +251,32 @@ namespace Org.BouncyCastle.Crypto.Tls if (!mAppDataReady) throw new TlsFatalAlert(AlertDescription.unexpected_message); - mApplicationDataQueue.AddData(buf, offset, len); - ProcessApplicationData(); + mApplicationDataQueue.AddData(buf, off, len); + ProcessApplicationDataQueue(); break; } case ContentType.change_cipher_spec: { - ProcessChangeCipherSpec(buf, offset, len); + ProcessChangeCipherSpec(buf, off, len); break; } case ContentType.handshake: { - mHandshakeQueue.AddData(buf, offset, len); - ProcessHandshake(); + if (mHandshakeQueue.Available > 0) + { + mHandshakeQueue.AddData(buf, off, len); + ProcessHandshakeQueue(mHandshakeQueue); + } + else + { + ByteQueue tmpQueue = new ByteQueue(buf, off, len); + ProcessHandshakeQueue(tmpQueue); + int remaining = tmpQueue.Available; + if (remaining > 0) + { + mHandshakeQueue.AddData(buf, off + len - remaining, remaining); + } + } break; } //case ContentType.heartbeat: @@ -282,71 +295,64 @@ namespace Org.BouncyCastle.Crypto.Tls } } - private void ProcessHandshake() + private void ProcessHandshakeQueue(ByteQueue queue) { - bool read; - do + while (queue.Available >= 4) { - read = false; /* * We need the first 4 bytes, they contain type and length of the message. */ - if (mHandshakeQueue.Available >= 4) - { - byte[] beginning = new byte[4]; - mHandshakeQueue.Read(beginning, 0, 4, 0); - byte type = TlsUtilities.ReadUint8(beginning, 0); - int length = TlsUtilities.ReadUint24(beginning, 1); - int totalLength = 4 + length; + byte[] beginning = new byte[4]; + queue.Read(beginning, 0, 4, 0); + byte type = TlsUtilities.ReadUint8(beginning, 0); + int length = TlsUtilities.ReadUint24(beginning, 1); + int totalLength = 4 + length; - /* - * Check if we have enough bytes in the buffer to read the full message. - */ - if (mHandshakeQueue.Available >= totalLength) + /* + * Check if we have enough bytes in the buffer to read the full message. + */ + if (queue.Available < totalLength) + break; + + CheckReceivedChangeCipherSpec(mConnectionState == CS_END || type == HandshakeType.finished); + + /* + * RFC 2246 7.4.9. The value handshake_messages includes all handshake messages + * starting at client hello up to, but not including, this finished message. + * [..] Note: [Also,] Hello Request messages are omitted from handshake hashes. + */ + switch (type) + { + case HandshakeType.hello_request: + break; + case HandshakeType.finished: + default: + { + TlsContext ctx = Context; + if (type == HandshakeType.finished + && this.mExpectedVerifyData == null + && ctx.SecurityParameters.MasterSecret != null) { - CheckReceivedChangeCipherSpec(mConnectionState == CS_END || type == HandshakeType.finished); + this.mExpectedVerifyData = CreateVerifyData(!ctx.IsServer); + } - /* - * RFC 2246 7.4.9. The value handshake_messages includes all handshake messages - * starting at client hello up to, but not including, this finished message. - * [..] Note: [Also,] Hello Request messages are omitted from handshake hashes. - */ - switch (type) - { - case HandshakeType.hello_request: - break; - case HandshakeType.finished: - default: - { - TlsContext ctx = Context; - if (type == HandshakeType.finished - && this.mExpectedVerifyData == null - && ctx.SecurityParameters.MasterSecret != null) - { - this.mExpectedVerifyData = CreateVerifyData(!ctx.IsServer); - } - - mHandshakeQueue.CopyTo(mRecordStream.HandshakeHashUpdater, totalLength); - break; - } - } + queue.CopyTo(mRecordStream.HandshakeHashUpdater, totalLength); + break; + } + } - mHandshakeQueue.RemoveData(4); + queue.RemoveData(4); - MemoryStream buf = mHandshakeQueue.ReadFrom(length); + MemoryStream buf = queue.ReadFrom(length); - /* - * Now, parse the message. - */ - HandleHandshakeMessage(type, buf); - read = true; - } - } + /* + * Now, parse the message. + */ + HandleHandshakeMessage(type, buf); } - while (read); } - private void ProcessApplicationData() + private void ProcessApplicationDataQueue() { /* * There is nothing we need to do here. @@ -355,7 +361,7 @@ namespace Org.BouncyCastle.Crypto.Tls */ } - private void ProcessAlert() + private void ProcessAlertQueue() { while (mAlertQueue.Available >= 2) { -- cgit 1.4.1 From dffe6e019ea03887b0f1dd9f2ded8a92ff6efcbc Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Wed, 29 Mar 2017 20:19:05 +1030 Subject: Fix fatal alert type for unexpected server_hello_done --- crypto/src/crypto/tls/TlsClientProtocol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/src/crypto/tls/TlsClientProtocol.cs b/crypto/src/crypto/tls/TlsClientProtocol.cs index 24fb3f63d..4e4a7a492 100644 --- a/crypto/src/crypto/tls/TlsClientProtocol.cs +++ b/crypto/src/crypto/tls/TlsClientProtocol.cs @@ -427,7 +427,7 @@ namespace Org.BouncyCastle.Crypto.Tls break; } default: - throw new TlsFatalAlert(AlertDescription.handshake_failure); + throw new TlsFatalAlert(AlertDescription.unexpected_message); } this.mConnectionState = CS_CLIENT_FINISHED; -- cgit 1.4.1 From 4f46fe2c101b74cc594bae681c1591ee8c968bf0 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 30 Mar 2017 01:54:32 +1030 Subject: Check candidate cipher suites against signature_algorithms --- crypto/src/crypto/tls/AbstractTlsServer.cs | 6 ++- crypto/src/crypto/tls/TlsUtilities.cs | 69 ++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/crypto/src/crypto/tls/AbstractTlsServer.cs b/crypto/src/crypto/tls/AbstractTlsServer.cs index d87a294f0..52a79c9d8 100644 --- a/crypto/src/crypto/tls/AbstractTlsServer.cs +++ b/crypto/src/crypto/tls/AbstractTlsServer.cs @@ -194,11 +194,12 @@ namespace Org.BouncyCastle.Crypto.Tls public virtual int GetSelectedCipherSuite() { /* - * TODO RFC 5246 7.4.3. In order to negotiate correctly, the server MUST check any candidate + * RFC 5246 7.4.3. In order to negotiate correctly, the server MUST check any candidate * cipher suites against the "signature_algorithms" extension before selecting them. This is * somewhat inelegant but is a compromise designed to minimize changes to the original * cipher suite design. */ + IList sigAlgs = TlsUtilities.GetUsableSignatureAlgorithms(this.mSupportedSignatureAlgorithms); /* * RFC 4429 5.1. A server that receives a ClientHello containing one or both of these @@ -216,7 +217,8 @@ namespace Org.BouncyCastle.Crypto.Tls if (Arrays.Contains(this.mOfferedCipherSuites, cipherSuite) && (eccCipherSuitesEnabled || !TlsEccUtilities.IsEccCipherSuite(cipherSuite)) - && TlsUtilities.IsValidCipherSuiteForVersion(cipherSuite, mServerVersion)) + && TlsUtilities.IsValidCipherSuiteForVersion(cipherSuite, mServerVersion) + && TlsUtilities.IsValidCipherSuiteForSignatureAlgorithms(cipherSuite, sigAlgs)) { return this.mSelectedCipherSuite = cipherSuite; } diff --git a/crypto/src/crypto/tls/TlsUtilities.cs b/crypto/src/crypto/tls/TlsUtilities.cs index 4f709a645..48e51a7b6 100644 --- a/crypto/src/crypto/tls/TlsUtilities.cs +++ b/crypto/src/crypto/tls/TlsUtilities.cs @@ -571,6 +571,16 @@ namespace Org.BouncyCastle.Crypto.Tls buf[offset + 1] = (byte)version.MinorVersion; } + public static IList GetAllSignatureAlgorithms() + { + IList v = Platform.CreateArrayList(4); + v.Add(SignatureAlgorithm.anonymous); + v.Add(SignatureAlgorithm.rsa); + v.Add(SignatureAlgorithm.dsa); + v.Add(SignatureAlgorithm.ecdsa); + return v; + } + public static IList GetDefaultDssSignatureAlgorithms() { return VectorOfOne(new SignatureAndHashAlgorithm(HashAlgorithm.sha1, SignatureAlgorithm.dsa)); @@ -2286,9 +2296,68 @@ namespace Org.BouncyCastle.Crypto.Tls return CipherType.stream == GetCipherType(ciphersuite); } + public static bool IsValidCipherSuiteForSignatureAlgorithms(int cipherSuite, IList sigAlgs) + { + int keyExchangeAlgorithm; + try + { + keyExchangeAlgorithm = GetKeyExchangeAlgorithm(cipherSuite); + } + catch (IOException e) + { + return true; + } + + switch (keyExchangeAlgorithm) + { + case KeyExchangeAlgorithm.DH_anon: + case KeyExchangeAlgorithm.DH_anon_EXPORT: + case KeyExchangeAlgorithm.ECDH_anon: + return sigAlgs.Contains(SignatureAlgorithm.anonymous); + + case KeyExchangeAlgorithm.DHE_RSA: + case KeyExchangeAlgorithm.DHE_RSA_EXPORT: + case KeyExchangeAlgorithm.ECDHE_RSA: + case KeyExchangeAlgorithm.SRP_RSA: + return sigAlgs.Contains(SignatureAlgorithm.rsa); + + case KeyExchangeAlgorithm.DHE_DSS: + case KeyExchangeAlgorithm.DHE_DSS_EXPORT: + case KeyExchangeAlgorithm.SRP_DSS: + return sigAlgs.Contains(SignatureAlgorithm.dsa); + + case KeyExchangeAlgorithm.ECDHE_ECDSA: + return sigAlgs.Contains(SignatureAlgorithm.ecdsa); + + default: + return true; + } + } + public static bool IsValidCipherSuiteForVersion(int cipherSuite, ProtocolVersion serverVersion) { return GetMinimumVersion(cipherSuite).IsEqualOrEarlierVersionOf(serverVersion.GetEquivalentTLSVersion()); } + + public static IList GetUsableSignatureAlgorithms(IList sigHashAlgs) + { + if (sigHashAlgs == null) + return GetAllSignatureAlgorithms(); + + IList v = Platform.CreateArrayList(4); + v.Add(SignatureAlgorithm.anonymous); + foreach (SignatureAndHashAlgorithm sigHashAlg in sigHashAlgs) + { + //if (sigHashAlg.Hash >= MINIMUM_HASH_STRICT) + { + byte sigAlg = sigHashAlg.Signature; + if (!v.Contains(sigAlg)) + { + v.Add(sigAlg); + } + } + } + return v; + } } } -- cgit 1.4.1 From 44c1a7c05913560362d3d241dbd0bc58a5b6dbac Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Sat, 8 Apr 2017 23:34:26 +0700 Subject: Fix a bunch of RFC references in comments --- crypto/src/crypto/tls/AbstractTlsKeyExchange.cs | 6 +++--- crypto/src/crypto/tls/DtlsClientProtocol.cs | 2 +- crypto/src/crypto/tls/DtlsRecordLayer.cs | 2 +- crypto/src/crypto/tls/DtlsServerProtocol.cs | 2 +- crypto/src/crypto/tls/RecordStream.cs | 16 ++++++++-------- crypto/src/crypto/tls/TlsClientProtocol.cs | 2 +- crypto/src/crypto/tls/TlsRsaSigner.cs | 2 +- crypto/src/crypto/tls/TlsServerProtocol.cs | 2 +- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs b/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs index 09fb8782d..294b24929 100644 --- a/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs +++ b/crypto/src/crypto/tls/AbstractTlsKeyExchange.cs @@ -38,7 +38,7 @@ namespace Org.BouncyCastle.Crypto.Tls if (TlsUtilities.IsSignatureAlgorithmsExtensionAllowed(clientVersion)) { /* - * RFC 5264 7.4.1.4.1. If the client does not send the signature_algorithms extension, + * RFC 5246 7.4.1.4.1. If the client does not send the signature_algorithms extension, * the server MUST do the following: * * - If the negotiated key exchange algorithm is one of (RSA, DHE_RSA, DH_RSA, RSA_PSK, @@ -106,14 +106,14 @@ namespace Org.BouncyCastle.Crypto.Tls if (mSupportedSignatureAlgorithms == null) { /* - * TODO RFC 2264 7.4.2. Unless otherwise specified, the signing algorithm for the + * TODO RFC 2246 7.4.2. Unless otherwise specified, the signing algorithm for the * certificate must be the same as the algorithm for the certificate key. */ } else { /* - * TODO RFC 5264 7.4.2. If the client provided a "signature_algorithms" extension, then + * TODO RFC 5246 7.4.2. If the client provided a "signature_algorithms" extension, then * all certificates provided by the server MUST be signed by a hash/signature algorithm * pair that appears in that extension. */ diff --git a/crypto/src/crypto/tls/DtlsClientProtocol.cs b/crypto/src/crypto/tls/DtlsClientProtocol.cs index 90430d772..ae6e6a573 100644 --- a/crypto/src/crypto/tls/DtlsClientProtocol.cs +++ b/crypto/src/crypto/tls/DtlsClientProtocol.cs @@ -776,7 +776,7 @@ namespace Org.BouncyCastle.Crypto.Tls securityParameters.CipherSuite); /* - * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify verify_data_length has + * RFC 5246 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; diff --git a/crypto/src/crypto/tls/DtlsRecordLayer.cs b/crypto/src/crypto/tls/DtlsRecordLayer.cs index 4a781b5b5..3c3e1821f 100644 --- a/crypto/src/crypto/tls/DtlsRecordLayer.cs +++ b/crypto/src/crypto/tls/DtlsRecordLayer.cs @@ -491,7 +491,7 @@ namespace Org.BouncyCastle.Crypto.Tls throw new TlsFatalAlert(AlertDescription.internal_error); /* - * RFC 5264 6.2.1 Implementations MUST NOT send zero-length fragments of Handshake, Alert, + * RFC 5246 6.2.1 Implementations MUST NOT send zero-length fragments of Handshake, Alert, * or ChangeCipherSpec content types. */ if (len < 1 && contentType != ContentType.application_data) diff --git a/crypto/src/crypto/tls/DtlsServerProtocol.cs b/crypto/src/crypto/tls/DtlsServerProtocol.cs index fbf33045b..3032269d1 100644 --- a/crypto/src/crypto/tls/DtlsServerProtocol.cs +++ b/crypto/src/crypto/tls/DtlsServerProtocol.cs @@ -425,7 +425,7 @@ namespace Org.BouncyCastle.Crypto.Tls securityParameters.CipherSuite); /* - * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify verify_data_length + * RFC 5246 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; diff --git a/crypto/src/crypto/tls/RecordStream.cs b/crypto/src/crypto/tls/RecordStream.cs index cddd5e6d7..46673cf7e 100644 --- a/crypto/src/crypto/tls/RecordStream.cs +++ b/crypto/src/crypto/tls/RecordStream.cs @@ -208,7 +208,7 @@ namespace Org.BouncyCastle.Crypto.Tls CheckLength(decoded.Length, mCompressedLimit, AlertDescription.record_overflow); /* - * TODO RFC5264 6.2.2. Implementation note: Decompression functions are responsible for + * TODO 5246 6.2.2. Implementation note: Decompression functions are responsible for * ensuring that messages cannot cause internal buffer overflows. */ Stream cOut = mReadCompression.Decompress(mBuffer); @@ -220,14 +220,14 @@ namespace Org.BouncyCastle.Crypto.Tls } /* - * RFC 5264 6.2.2. If the decompression function encounters a TLSCompressed.fragment that + * RFC 5246 6.2.2. If the decompression function encounters a TLSCompressed.fragment that * would decompress to a length in excess of 2^14 bytes, it should report a fatal * decompression failure error. */ CheckLength(decoded.Length, mPlaintextLimit, AlertDescription.decompression_failure); /* - * RFC 5264 6.2.1 Implementations MUST NOT send zero-length fragments of Handshake, Alert, + * RFC 5246 6.2.1 Implementations MUST NOT send zero-length fragments of Handshake, Alert, * or ChangeCipherSpec content types. */ if (decoded.Length < 1 && type != ContentType.application_data) @@ -243,18 +243,18 @@ namespace Org.BouncyCastle.Crypto.Tls return; /* - * RFC 5264 6. Implementations MUST NOT send record types not defined in this document + * RFC 5246 6. Implementations MUST NOT send record types not defined in this document * unless negotiated by some extension. */ CheckType(type, AlertDescription.internal_error); /* - * RFC 5264 6.2.1 The length should not exceed 2^14. + * RFC 5246 6.2.1 The length should not exceed 2^14. */ CheckLength(plaintextLength, mPlaintextLimit, AlertDescription.internal_error); /* - * RFC 5264 6.2.1 Implementations MUST NOT send zero-length fragments of Handshake, Alert, + * RFC 5246 6.2.1 Implementations MUST NOT send zero-length fragments of Handshake, Alert, * or ChangeCipherSpec content types. */ if (plaintextLength < 1 && type != ContentType.application_data) @@ -274,7 +274,7 @@ namespace Org.BouncyCastle.Crypto.Tls byte[] compressed = GetBufferContents(); /* - * RFC5264 6.2.2. Compression must be lossless and may not increase the content length + * RFC 5246 6.2.2. Compression must be lossless and may not increase the content length * by more than 1024 bytes. */ CheckLength(compressed.Length, plaintextLength + 1024, AlertDescription.internal_error); @@ -283,7 +283,7 @@ namespace Org.BouncyCastle.Crypto.Tls } /* - * RFC 5264 6.2.3. The length may not exceed 2^14 + 2048. + * RFC 5246 6.2.3. The length may not exceed 2^14 + 2048. */ CheckLength(ciphertext.Length, mCiphertextLimit, AlertDescription.internal_error); diff --git a/crypto/src/crypto/tls/TlsClientProtocol.cs b/crypto/src/crypto/tls/TlsClientProtocol.cs index 4e4a7a492..0ea84c05c 100644 --- a/crypto/src/crypto/tls/TlsClientProtocol.cs +++ b/crypto/src/crypto/tls/TlsClientProtocol.cs @@ -790,7 +790,7 @@ namespace Org.BouncyCastle.Crypto.Tls this.mSecurityParameters.prfAlgorithm = GetPrfAlgorithm(Context, this.mSecurityParameters.CipherSuite); /* - * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify + * RFC 5246 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. */ diff --git a/crypto/src/crypto/tls/TlsRsaSigner.cs b/crypto/src/crypto/tls/TlsRsaSigner.cs index 6da1c5e9b..1614f503b 100644 --- a/crypto/src/crypto/tls/TlsRsaSigner.cs +++ b/crypto/src/crypto/tls/TlsRsaSigner.cs @@ -91,7 +91,7 @@ namespace Org.BouncyCastle.Crypto.Tls protected virtual IAsymmetricBlockCipher CreateRsaImpl() { /* - * RFC 5264 7.4.7.1. Implementation note: It is now known that remote timing-based attacks + * RFC 5246 7.4.7.1. Implementation note: It is now known that remote timing-based attacks * on TLS are possible, at least when the client and server are on the same LAN. * Accordingly, implementations that use static RSA keys MUST use RSA blinding or some other * anti-timing technique, as described in [TIMING]. diff --git a/crypto/src/crypto/tls/TlsServerProtocol.cs b/crypto/src/crypto/tls/TlsServerProtocol.cs index db823539a..5f3ce18e2 100644 --- a/crypto/src/crypto/tls/TlsServerProtocol.cs +++ b/crypto/src/crypto/tls/TlsServerProtocol.cs @@ -786,7 +786,7 @@ namespace Org.BouncyCastle.Crypto.Tls mSecurityParameters.prfAlgorithm = GetPrfAlgorithm(Context, mSecurityParameters.CipherSuite); /* - * RFC 5264 7.4.9. Any cipher suite which does not explicitly specify verify_data_length has + * RFC 5246 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. */ mSecurityParameters.verifyDataLength = 12; -- cgit 1.4.1 From fefed20bec7e07456cd50a710cc11fff18cf46c1 Mon Sep 17 00:00:00 2001 From: David Hook Date: Tue, 16 May 2017 00:36:34 +1000 Subject: Added validation of integer/enumerated encoding. --- crypto/src/asn1/DerEnumerated.cs | 13 ++++++++++++- crypto/src/asn1/DerInteger.cs | 13 ++++++++++++- crypto/test/src/tsp/test/ParseTest.cs | 3 ++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/crypto/src/asn1/DerEnumerated.cs b/crypto/src/asn1/DerEnumerated.cs index 476b7fa9a..db27065bb 100644 --- a/crypto/src/asn1/DerEnumerated.cs +++ b/crypto/src/asn1/DerEnumerated.cs @@ -64,7 +64,18 @@ namespace Org.BouncyCastle.Asn1 public DerEnumerated( byte[] bytes) { - this.bytes = bytes; + if (bytes.Length > 1) + { + if (bytes[0] == 0 && (bytes[1] & 0x80) == 0) + { + throw new ArgumentException("malformed enumerated"); + } + if (bytes[0] == (byte)0xff && (bytes[1] & 0x80) != 0) + { + throw new ArgumentException("malformed enumerated"); + } + } + this.bytes = Arrays.Clone(bytes); } public BigInteger Value diff --git a/crypto/src/asn1/DerInteger.cs b/crypto/src/asn1/DerInteger.cs index 3610de588..5b240d281 100644 --- a/crypto/src/asn1/DerInteger.cs +++ b/crypto/src/asn1/DerInteger.cs @@ -70,7 +70,18 @@ namespace Org.BouncyCastle.Asn1 public DerInteger( byte[] bytes) { - this.bytes = bytes; + if (bytes.Length > 1) + { + if (bytes[0] == 0 && (bytes[1] & 0x80) == 0) + { + throw new ArgumentException("malformed integer"); + } + if (bytes[0] == (byte)0xff && (bytes[1] & 0x80) != 0) + { + throw new ArgumentException("malformed integer"); + } + } + this.bytes = Arrays.Clone(bytes); } public BigInteger Value diff --git a/crypto/test/src/tsp/test/ParseTest.cs b/crypto/test/src/tsp/test/ParseTest.cs index ec9ba72f6..e9489a278 100644 --- a/crypto/test/src/tsp/test/ParseTest.cs +++ b/crypto/test/src/tsp/test/ParseTest.cs @@ -360,7 +360,8 @@ namespace Org.BouncyCastle.Tsp.Tests [Test] public void TestGeneralizedTime() { - generalizedTimeParse(generalizedTime); + // TODO: response is invalid - malformed integer + // generalizedTimeParse(generalizedTime); } [Test] -- cgit 1.4.1