Infrastructure for Tls #52

Drawaes · 2020-01-17T00:57:53Z

Includes

Helper for reversing all structs and enums such that you don't have to have ugly casts everywhere and can reverse for network order
Basic enums for TLS (cipher suites etc) all coded to the required values based on RFCs and values that will be sent across the wire
A set of infrastructure for all the basic building blocks of Tls (Hashing, Certs, KeyExchange, BulkCiphers)

I have not included any implementations yet and made all internal for now, this can be reviewed at a later date but none should be used externally for now (except TlsProtocolVersion).

Next there are a few steps

Provide some implementations (or at least one for OpenSsl) that shows how the infrastructure is provided
Provide the upper parts of the protocol (state machines, parsers/writers)

I will probably work on 1 first so that there is something to actually use in 2 when we come to it (and my Tls 1.3 is a little rusty so I need to review so I will work on 1.2 first for part 2).

Cheers
Tim

davidfowl · 2020-01-17T06:27:01Z

src/Bedrock.Framework.Experimental/Infrastructure/Endianness.cs

+
+namespace Bedrock.Framework.Experimental.Infrastructure
+{
+    public static class Endianness


Internal. Do you need this though?

If I don't have this I end up with a lot of (SomeEnum) BinaryPrimatives.Reverse((ushort) someEnum);

Etc it's really ugly and this using generics goes down all the way to basically the bswap in all cases. Also in later code for Tls there are other structs (including int24) that are just unsupported otherwise

src/Bedrock.Framework.Experimental/Protocols/Tls/ApplicationLayerProtocolType.cs

davidfowl · 2020-01-17T06:27:39Z

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/AdditionalInfo.cs

+namespace Bedrock.Framework.Experimental.Protocols.Tls.BulkCiphers
+{
+    [StructLayout(LayoutKind.Sequential, Pack = 1)]
+    internal struct AdditionalInfo


Mutable struct? Why?

There is a sequence number on this we increased for each message sent. We then feed this into BulkCipher as additional information used during the encryption for AES etc. Also by making the methods convert to the correct bitness in the set it means we can directly blit the struct (by using a pointer)

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/BulkCipher.cs

davidfowl · 2020-01-17T06:28:39Z

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/BulkCipher.cs

+
+namespace Bedrock.Framework.Experimental.Protocols.Tls.BulkCiphers
+{
+    internal abstract class BulkCipher : IDisposable


What is this type for?

There is a difference in how TLS 1.2 and TLS 1.3 for instance encrypt each record (different additional info, different IV calculations for each, different headers and footers for the encryption frame) so this way the up level protocol handlers can abstract that detail and not care. They just have a Decrypt/Encrypt. I will upload the TLS 1.2 and TLS 1.3 implementation of each of these next.

davidfowl · 2020-01-17T06:29:55Z

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/BulkCipherKey.cs

+{
+    internal abstract class BulkCipherKey : IDisposable
+    {
+        public abstract Memory<byte> IV { get; }


Why does this need to exist and why isn't it read only

The IV is the initialisation vector that is used by the various symmetrical encryption algos. The spec defines how these are set so the "BulkCipher" will set them before every record that is encrypted (They contain a mixture of random/info from the handshake and counters). They are calculated differently for TLS 1.2 /1.3 etc and therefore cannot be controlled by the key implementation. They are mutated every record so they need to be editable.

davidfowl · 2020-01-17T06:30:36Z

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/BulkCipherProvider.cs

+{
+    internal abstract class BulkCipherProvider
+    {
+        public abstract BulkCipher GetCipher<T>(BulkCipherType cipherType, Memory<byte> keyStorage) where T : BulkCipher, new();


Why is this memory writable?

Originally this was "OwnedMemory" because it came from a pool, however the BulkCipher needed to hold it for it's lifetime and then say when it was done. Is there still a concept of this now?

src/Bedrock.Framework.Experimental/Protocols/Tls/Certificates/Certificate.cs

davidfowl · 2020-01-17T06:31:27Z

src/Bedrock.Framework.Experimental/Protocols/Tls/Hashs/Hash.cs

+
+namespace Bedrock.Framework.Experimental.Protocols.Tls.Hashs
+{
+    internal abstract class Hash


What's this?

That is the same as below the provider below will provide an instance of this (from say OpenSsl or BCrypt)

davidfowl · 2020-01-17T06:31:50Z

src/Bedrock.Framework.Experimental/Protocols/Tls/Hashs/HashProvider.cs

+
+namespace Bedrock.Framework.Experimental.Protocols.Tls.Hashs
+{
+    internal abstract class HashProvider


Can we use any of the BCL types?

So Tls during the handshake requires you to keep a running hash of the messages sent/received during the exchange. There is a BCL incremental hash function which is great but....

TLS requires you to grab the hash up until now at various points. This poses a problem as there is only a "gethashandreset" method where as I want to get the hash up to now and continue. Both BCrypt and OpenSsl support that scenario but the BCL does not.

Without this you would have to buffer the messages during the handshake which is significantly worse and the interop for hash isn't a big deal.

src/Bedrock.Framework.Experimental/Protocols/Tls/TlsFrameType.cs

Drawaes · 2020-01-17T20:45:26Z

All changed or answered, read for comment

src/Bedrock.Framework.Experimental/Protocols/Tls/Certificates/CertificateType.cs

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/BulkCipher.cs

davidfowl · 2020-01-21T03:11:39Z

src/Bedrock.Framework.Experimental/Protocols/Tls/Hashs/Hash.cs

+        public abstract int Finish(Span<byte> output);
+        public abstract HashType HashType { get; }
+
+        public void HashData(SequenceReader<byte> reader)


Should this be ref?

jkotalik

I think there are a few outstanding comments, but I don't see any reason to block this.

davidfowl · 2020-01-31T11:45:18Z

@Drawaes Can you rebase?

Drawaes added 6 commits January 16, 2020 23:37

Started adding Tls Primatives

7f6c31d

clean up reverse

5943234

Update to reduce unsafe surface area

cbc3d81

Fixed to using sizeof to reduce Jitter Size

5d1520c

Update to add the extra infra for the bulk ciphers

8040a57

Better layout

05aa957

davidfowl reviewed Jan 17, 2020

View reviewed changes

src/Bedrock.Framework.Experimental/Protocols/Tls/ApplicationLayerProtocolType.cs Show resolved Hide resolved

davidfowl reviewed Jan 17, 2020

View reviewed changes

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/BulkCipher.cs Outdated Show resolved Hide resolved

davidfowl reviewed Jan 17, 2020

View reviewed changes

src/Bedrock.Framework.Experimental/Protocols/Tls/Certificates/Certificate.cs Outdated Show resolved Hide resolved

davidfowl reviewed Jan 17, 2020

View reviewed changes

src/Bedrock.Framework.Experimental/Protocols/Tls/TlsFrameType.cs Outdated Show resolved Hide resolved

Drawaes added 4 commits January 17, 2020 18:29

Move to use IBufferWriter<byte> in BulkCipher

7a81a17

Add IANA ref for ALPN

187b867

Updated to change frametype to record type and add ref to RFC

fa2a437

Fix for rename

557164e

Removed custom certificate

cbfa9ab

davidfowl reviewed Jan 18, 2020

View reviewed changes

src/Bedrock.Framework.Experimental/Protocols/Tls/Certificates/CertificateType.cs Outdated Show resolved Hide resolved

Casing

53caa4f

davidfowl reviewed Jan 18, 2020

View reviewed changes

src/Bedrock.Framework.Experimental/Protocols/Tls/BulkCiphers/BulkCipher.cs Outdated Show resolved Hide resolved

Removed usings

7534763

davidfowl reviewed Jan 21, 2020

View reviewed changes

jkotalik approved these changes Jan 21, 2020

View reviewed changes

JanEggers mentioned this pull request May 5, 2020

TLS with ASP.Net Core 2.1 dotnet/MQTTnet#464

Closed

davidfowl closed this Apr 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Infrastructure for Tls #52

Infrastructure for Tls #52

Drawaes commented Jan 17, 2020

davidfowl Jan 17, 2020

Drawaes Jan 17, 2020

davidfowl Jan 17, 2020

Drawaes Jan 17, 2020

davidfowl Jan 17, 2020

Drawaes Jan 17, 2020

davidfowl Jan 17, 2020

Drawaes Jan 17, 2020

davidfowl Jan 17, 2020

Drawaes Jan 17, 2020

davidfowl Jan 17, 2020

Drawaes Jan 17, 2020

davidfowl Jan 17, 2020

Drawaes Jan 17, 2020

Drawaes commented Jan 17, 2020

davidfowl Jan 21, 2020

jkotalik left a comment

davidfowl commented Jan 31, 2020

Infrastructure for Tls #52

Infrastructure for Tls #52

Conversation

Drawaes commented Jan 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Drawaes commented Jan 17, 2020

Choose a reason for hiding this comment

jkotalik left a comment

Choose a reason for hiding this comment

davidfowl commented Jan 31, 2020