diff --git a/Hazel.UnitTests/Dtls/DtlsConnectionTests.cs b/Hazel.UnitTests/Dtls/DtlsConnectionTests.cs
index 55e0c91..6cb2335 100644
--- a/Hazel.UnitTests/Dtls/DtlsConnectionTests.cs
+++ b/Hazel.UnitTests/Dtls/DtlsConnectionTests.cs
@@ -5,6 +5,7 @@
using System;
using System.Net;
using System.Net.Sockets;
+using System.Reflection;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
@@ -85,6 +86,15 @@ private static X509Certificate2Collection GetCertificateForClient()
return clientCertificates;
}
+ private static int GetAvailableUdpPort()
+ {
+ using (Socket socket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp))
+ {
+ socket.Bind(new IPEndPoint(IPAddress.Loopback, 0));
+ return ((IPEndPoint)socket.LocalEndPoint).Port;
+ }
+ }
+
protected DtlsConnectionListener CreateListener(int numWorkers, IPEndPoint endPoint, ILogger logger, IPMode ipMode = IPMode.IPv4)
{
DtlsConnectionListener listener = new DtlsConnectionListener(2, endPoint, logger, ipMode);
@@ -838,6 +848,191 @@ protected override void SendClientHello(bool isRetransmit)
}
}
+ private class ControlledClientHelloDtlsConnection : DtlsUnityConnection
+ {
+ public ControlledClientHelloDtlsConnection(ILogger logger, IPEndPoint remoteEndPoint)
+ : base(logger, remoteEndPoint)
+ {
+ }
+
+ public void ResendClientHelloForTest()
+ {
+ base.SendClientHello(true);
+ }
+ }
+
+ private sealed class CountingRsa : RSA
+ {
+ private RSA inner;
+
+ public int SignCount;
+
+ public CountingRsa(RSA inner)
+ {
+ this.inner = inner;
+ }
+
+ public override int KeySize
+ {
+ get => this.inner.KeySize;
+ set => this.inner.KeySize = value;
+ }
+
+ public override KeySizes[] LegalKeySizes => this.inner.LegalKeySizes;
+
+ public override byte[] Decrypt(byte[] data, RSAEncryptionPadding padding)
+ {
+ return this.inner.Decrypt(data, padding);
+ }
+
+ public override byte[] Encrypt(byte[] data, RSAEncryptionPadding padding)
+ {
+ return this.inner.Encrypt(data, padding);
+ }
+
+ public override RSAParameters ExportParameters(bool includePrivateParameters)
+ {
+ return this.inner.ExportParameters(includePrivateParameters);
+ }
+
+ public override void ImportParameters(RSAParameters parameters)
+ {
+ this.inner.ImportParameters(parameters);
+ }
+
+ public override byte[] SignHash(byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
+ {
+ Interlocked.Increment(ref this.SignCount);
+ return this.inner.SignHash(hash, hashAlgorithm, padding);
+ }
+
+ public override bool VerifyHash(byte[] hash, byte[] signature, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
+ {
+ return this.inner.VerifyHash(hash, signature, hashAlgorithm, padding);
+ }
+
+ protected override void Dispose(bool disposing)
+ {
+ if (disposing)
+ {
+ this.inner?.Dispose();
+ this.inner = null;
+ }
+
+ base.Dispose(disposing);
+ }
+ }
+
+ private class HalfOpenDtlsConnection : DtlsUnityConnection
+ {
+ public HalfOpenDtlsConnection(ILogger logger, IPEndPoint remoteEndPoint)
+ : base(logger, remoteEndPoint)
+ {
+ }
+
+ protected override void SendClientKeyExchangeFlight(bool isRetransmit)
+ {
+ // Stop after the cookie-verified ClientHello so the listener
+ // has a peer record but no application-level connection.
+ }
+ }
+
+ [TestMethod]
+ public void StaleHalfOpenPeerIsRemovedTest()
+ {
+ IPEndPoint listenerEndPoint = new IPEndPoint(IPAddress.Loopback, GetAvailableUdpPort());
+
+ using (DtlsConnectionListener listener = this.CreateListener(2, listenerEndPoint, new TestLogger("Server")))
+ using (HalfOpenDtlsConnection client = new HalfOpenDtlsConnection(new TestLogger("Client "), listenerEndPoint))
+ {
+ client.SetValidServerCertificates(GetCertificateForClient());
+
+ int connections = 0;
+ listener.NewConnection += (_) => Interlocked.Increment(ref connections);
+
+ listener.Start();
+ client.ConnectAsync();
+
+ Assert.IsTrue(
+ SpinWait.SpinUntil(() => listener.PeerCount == 1, 5000),
+ "The listener did not create a peer for the cookie-verified ClientHello.");
+ Assert.IsTrue(
+ SpinWait.SpinUntil(() => listener.PeerCount == 0, 7000),
+ "The listener did not evict the stale half-open peer.");
+ Assert.AreEqual(0, connections, "A half-open DTLS handshake must not create an application connection.");
+ }
+ }
+
+ [TestMethod]
+ public void StalePeerWithoutFinishedIsRemovedTest()
+ {
+ IPEndPoint listenerEndPoint = new IPEndPoint(IPAddress.Loopback, GetAvailableUdpPort());
+ IPEndPoint captureEndPoint = new IPEndPoint(IPAddress.Loopback, GetAvailableUdpPort());
+ int truncatedFlights = 0;
+
+ using (SocketCapture capture = new SocketCapture(captureEndPoint, listenerEndPoint, new TestLogger("Capture")))
+ using (DtlsConnectionListener listener = this.CreateListener(2, listenerEndPoint, new TestLogger("Server")))
+ using (DtlsUnityConnection client = this.CreateConnection(captureEndPoint, new TestLogger("Client ")))
+ using (Semaphore serverToClient = new Semaphore(0, int.MaxValue))
+ {
+ capture.SendToLocalSemaphore = serverToClient;
+ capture.PacketForRemoteTransform = packet =>
+ {
+ if (!Record.Parse(out Record firstRecord, expectedProtocolVersion: null, packet))
+ {
+ return packet;
+ }
+
+ int secondRecordOffset = Record.Size + firstRecord.Length;
+ if (firstRecord.ContentType != ContentType.Handshake || packet.Length <= secondRecordOffset
+ || !Record.Parse(out Record secondRecord, expectedProtocolVersion: null, packet.Slice(secondRecordOffset)))
+ {
+ return packet;
+ }
+
+ int thirdRecordOffset = secondRecordOffset + Record.Size + secondRecord.Length;
+ if (secondRecord.ContentType != ContentType.ChangeCipherSpec || packet.Length <= thirdRecordOffset
+ || !Record.Parse(out Record thirdRecord, expectedProtocolVersion: null, packet.Slice(thirdRecordOffset))
+ || thirdRecord.ContentType != ContentType.Handshake
+ || thirdRecord.Epoch == 0)
+ {
+ return packet;
+ }
+
+ Interlocked.Increment(ref truncatedFlights);
+ return packet.Slice(0, thirdRecordOffset);
+ };
+
+ int connections = 0;
+ listener.NewConnection += (_) => Interlocked.Increment(ref connections);
+
+ listener.Start();
+ client.ConnectAsync();
+
+ // Deliver the server flight in order so this test isolates the
+ // missing Finished state instead of exercising packet reordering.
+ capture.AssertPacketsToLocalCountEquals(1);
+ capture.ReleasePacketsToLocal(1);
+ capture.AssertPacketsToLocalCountEquals(3);
+ for (int i = 0; i < 3; ++i)
+ {
+ capture.ReleasePacketsToLocal(1);
+ Thread.Sleep(25);
+ }
+
+ Assert.IsTrue(
+ SpinWait.SpinUntil(() => Volatile.Read(ref truncatedFlights) > 0, 5000),
+ "The client did not send a ClientKeyExchange and ChangeCipherSpec flight.");
+ Assert.IsTrue(
+ SpinWait.SpinUntil(() => listener.PeerCount == 1, 5000),
+ "The listener did not retain the peer while waiting for Finished.");
+ Assert.IsTrue(
+ SpinWait.SpinUntil(() => listener.PeerCount == 0, 7000),
+ "The listener did not evict the peer after Finished was omitted.");
+ Assert.AreEqual(0, connections, "A DTLS handshake without Finished must not create an application connection.");
+ }
+ }
+
private class MultipleClientKeyExchangeFlightDtlsConnection : DtlsUnityConnection
{
@@ -882,6 +1077,40 @@ public void ConnectLikeAJerkTest()
}
}
+ [TestMethod]
+ public void ResentClientHelloReusesServerKeyExchangeSignatureTest()
+ {
+ IPEndPoint listenerEndPoint = new IPEndPoint(IPAddress.Loopback, GetAvailableUdpPort());
+ IPEndPoint captureEndPoint = new IPEndPoint(IPAddress.Loopback, GetAvailableUdpPort());
+
+ using (SocketCapture capture = new SocketCapture(captureEndPoint, listenerEndPoint, new TestLogger("Capture")))
+ using (DtlsConnectionListener listener = this.CreateListener(2, listenerEndPoint, new TestLogger("Server")))
+ using (ControlledClientHelloDtlsConnection client = new ControlledClientHelloDtlsConnection(new TestLogger("Client "), captureEndPoint))
+ using (Semaphore serverToClient = new Semaphore(0, int.MaxValue))
+ {
+ capture.SendToLocalSemaphore = serverToClient;
+ FieldInfo privateKeyField = typeof(DtlsConnectionListener).GetField("certificatePrivateKey", BindingFlags.Instance | BindingFlags.NonPublic);
+ CountingRsa countingRsa = new CountingRsa((RSA)privateKeyField.GetValue(listener));
+ privateKeyField.SetValue(listener, countingRsa);
+
+ client.SetValidServerCertificates(GetCertificateForClient());
+ listener.Start();
+ client.SetHandshakeResendTimeout(TimeSpan.FromSeconds(10));
+ client.ConnectAsync();
+
+ capture.AssertPacketsToLocalCountEquals(1);
+ capture.ReleasePacketsToLocal(1);
+
+ capture.AssertPacketsToLocalCountEquals(3);
+ Assert.AreEqual(1, countingRsa.SignCount);
+
+ client.ResendClientHelloForTest();
+ capture.AssertPacketsToLocalCountEquals(6);
+
+ Assert.AreEqual(1, countingRsa.SignCount, "ClientHello retransmission must reuse the signed ServerKeyExchange payload.");
+ }
+ }
+
///
/// Tests IPv4 resilience to multiple ClientKeyExchange packets.
///
diff --git a/Hazel.UnitTests/SocketCapture.cs b/Hazel.UnitTests/SocketCapture.cs
index c160069..5731df5 100644
--- a/Hazel.UnitTests/SocketCapture.cs
+++ b/Hazel.UnitTests/SocketCapture.cs
@@ -45,6 +45,7 @@ public class SocketCapture : IDisposable
public Semaphore SendToLocalSemaphore = null;
public Semaphore SendToRemoteSemaphore = null;
+ public Func PacketForRemoteTransform = null;
private CancellationTokenSource cancellationSource = new CancellationTokenSource();
private readonly CancellationToken cancellationToken;
@@ -158,6 +159,7 @@ private void SendToRemoteLoop()
if (this.forRemote.TryTake(out var packet))
{
+ packet = this.PacketForRemoteTransform?.Invoke(packet) ?? packet;
this.logger.WriteInfo($"Passed 1 packet of {packet.Length} bytes to remote");
this.captureSocket.SendTo(packet.GetUnderlyingArray(), packet.Offset, packet.Length, SocketFlags.None, this.remoteEndPoint);
}
@@ -303,4 +305,4 @@ internal string PacketsForLocalToString()
return sb.ToString();
}
}
-}
\ No newline at end of file
+}
diff --git a/Hazel/Dtls/DtlsConnectionListener.cs b/Hazel/Dtls/DtlsConnectionListener.cs
index 5db32cd..e4f0ea4 100644
--- a/Hazel/Dtls/DtlsConnectionListener.cs
+++ b/Hazel/Dtls/DtlsConnectionListener.cs
@@ -76,6 +76,8 @@ struct NextEpoch
public ByteSpan ClientVerification;
public ByteSpan ServerVerification;
+ public ByteSpan ServerCertificate;
+ public ByteSpan ServerKeyExchange;
}
@@ -99,6 +101,7 @@ sealed class PeerData : IDisposable
public readonly ProtocolVersion ProtocolVersion;
public DateTime StartOfNegotiation;
+ public int ApplicationDataProcessingCount;
public PeerData(ConnectionId connectionId, ulong nextExpectedSequenceNumber, ProtocolVersion protocolVersion)
{
@@ -136,6 +139,8 @@ public void ResetPeer(ConnectionId connectionId, ulong nextExpectedSequenceNumbe
this.NextEpoch.VerificationStream = new Sha256Stream();
this.NextEpoch.ClientVerification = block.Slice(Random.Size * 2, Finished.Size);
this.NextEpoch.ServerVerification = block.Slice(Random.Size * 2 + Finished.Size, Finished.Size);
+ this.NextEpoch.ServerCertificate = ByteSpan.Empty;
+ this.NextEpoch.ServerKeyExchange = ByteSpan.Empty;
this.ConnectionId = connectionId;
@@ -166,6 +171,7 @@ public void Dispose()
// Private key component of certificate's public key
private ByteSpan encodedCertificate;
private RSA certificatePrivateKey;
+ private readonly ReaderWriterLockSlim certificateLock = new ReaderWriterLockSlim();
// HMAC key to validate ClientHello cookie
private ThreadedHmacHelper hmacHelper;
@@ -250,10 +256,21 @@ public void SetCertificate(X509Certificate2 certificate)
throw new ArgumentException("Certificate must be signed by an RSA key", nameof(certificate));
}
- this.certificatePrivateKey?.Dispose();
- this.certificatePrivateKey = privateKey;
+ ByteSpan encodedCertificate = Certificate.Encode(certificate);
+ RSA previousPrivateKey;
+ this.certificateLock.EnterWriteLock();
+ try
+ {
+ previousPrivateKey = this.certificatePrivateKey;
+ this.certificatePrivateKey = privateKey;
+ this.encodedCertificate = encodedCertificate;
+ }
+ finally
+ {
+ this.certificateLock.ExitWriteLock();
+ }
- this.encodedCertificate = Certificate.Encode(certificate);
+ previousPrivateKey?.Dispose();
}
///
@@ -294,9 +311,19 @@ private void ProcessIncomingMessage(ByteSpan message, IPEndPoint peerAddress)
}
ConnectionId peerConnectionId;
+ bool processApplicationData = false;
lock (peer)
{
+ // The peer may have been removed while this packet was waiting
+ // for its lock. Do not process state that has already been
+ // disposed or replaced by a new peer at the same endpoint.
+ if (!this.existingPeers.TryGetValue(peerAddress, out PeerData currentPeer)
+ || !ReferenceEquals(peer, currentPeer))
+ {
+ return;
+ }
+
peerConnectionId = peer.ConnectionId;
// Each incoming packet may contain multiple DTLS
@@ -461,6 +488,8 @@ private void ProcessIncomingMessage(ByteSpan message, IPEndPoint peerAddress)
peer.NextEpoch.State = HandshakeState.ExpectingHello;
peer.NextEpoch.Handshake?.Dispose();
peer.NextEpoch.Handshake = null;
+ peer.NextEpoch.ServerCertificate = ByteSpan.Empty;
+ peer.NextEpoch.ServerKeyExchange = ByteSpan.Empty;
peer.NextEpoch.NextOutgoingSequence = 1;
peer.NextEpoch.RecordProtection = null;
peer.NextEpoch.VerificationStream.Reset();
@@ -485,17 +514,36 @@ private void ProcessIncomingMessage(ByteSpan message, IPEndPoint peerAddress)
reader.Length = recordPayload.Length;
recordPayload.CopyTo(reader.Buffer);
- peer.ApplicationData.Add(reader);
- break;
+ peer.ApplicationData.Add(reader);
+ break;
}
}
+
+ if (!peer.ApplicationData.IsEmpty)
+ {
+ ++peer.ApplicationDataProcessingCount;
+ processApplicationData = true;
+ }
}
// The peer lock must be exited before leaving the DtlsConnectionListener context to prevent deadlocks
// because ApplicationData processing may reenter this context
- while (peer.ApplicationData.TryTake(out var appMsg))
+ if (processApplicationData)
{
- base.ReadCallback(appMsg, peerAddress, peerConnectionId);
+ try
+ {
+ while (peer.ApplicationData.TryTake(out var appMsg))
+ {
+ base.ReadCallback(appMsg, peerAddress, peerConnectionId);
+ }
+ }
+ finally
+ {
+ lock (peer)
+ {
+ --peer.ApplicationDataProcessingCount;
+ }
+ }
}
}
@@ -708,7 +756,6 @@ private bool ProcessHandshake(PeerData peer, IPEndPoint peerAddress, ref Record
// Either way, there is not a feasible
// way to progress the connection.
MarkConnectionAsStale(peer.ConnectionId);
- this.existingPeers.TryRemove(peerAddress, out _);
return false;
}
@@ -918,6 +965,8 @@ private bool HandleClientHello(PeerData peer, IPEndPoint peerAddress, ref Record
peer.NextEpoch.State = HandshakeState.ExpectingClientKeyExchange;
peer.NextEpoch.SelectedCipherSuite = selectedCipherSuite;
peer.NextEpoch.Handshake = handshakeCipherSuite;
+ peer.NextEpoch.ServerCertificate = ByteSpan.Empty;
+ peer.NextEpoch.ServerKeyExchange = ByteSpan.Empty;
clientHello.Random.CopyTo(peer.NextEpoch.ClientRandom);
peer.NextEpoch.ServerRandom.FillWithRandom(this.random);
recordMessagesForVerifyData = true;
@@ -962,11 +1011,21 @@ private bool HandleClientHello(PeerData peer, IPEndPoint peerAddress, ref Record
// ServerKeyExchange and the ServerHelloDone
// messages.
- // Describe first record of the flight
- ServerHello serverHello = new ServerHello();
- serverHello.ServerProtocolVersion = protocolVersion;
- serverHello.Random = peer.NextEpoch.ServerRandom;
- serverHello.CipherSuite = selectedCipherSuite;
+ bool certificateLockTaken = false;
+ if (peer.NextEpoch.ServerKeyExchange.Length == 0)
+ {
+ this.certificateLock.EnterReadLock();
+ certificateLockTaken = true;
+ peer.NextEpoch.ServerCertificate = this.encodedCertificate;
+ }
+
+ try
+ {
+ // Describe first record of the flight
+ ServerHello serverHello = new ServerHello();
+ serverHello.ServerProtocolVersion = protocolVersion;
+ serverHello.Random = peer.NextEpoch.ServerRandom;
+ serverHello.CipherSuite = selectedCipherSuite;
Handshake serverHelloHandshake = new Handshake();
serverHelloHandshake.MessageType = HandshakeType.ServerHello;
@@ -983,7 +1042,7 @@ private bool HandleClientHello(PeerData peer, IPEndPoint peerAddress, ref Record
// * ServerHello payload
// * Certificate header
- var certificateData = this.encodedCertificate;
+ var certificateData = peer.NextEpoch.ServerCertificate;
int initialCertPadding = Record.Size + Handshake.Size + serverHello.Size + Handshake.Size;
int certInitialFragmentSize = Math.Min(certificateData.Length, maxCertFragmentSize - initialCertPadding);
@@ -1052,7 +1111,7 @@ ref initialRecord
certWriter = certWriter.Slice(Handshake.Size);
peer.NextEpoch.VerificationStream.AddData(certPacket);
- peer.NextEpoch.VerificationStream.AddData(this.encodedCertificate);
+ peer.NextEpoch.VerificationStream.AddData(peer.NextEpoch.ServerCertificate);
}
// Process additional certificate records
@@ -1099,10 +1158,16 @@ ref additionalRecord
base.QueueRawData(certBuffer, peerAddress);
}
- // Describe final record of the flight
- Handshake serverKeyExchangeHandshake = new Handshake();
+ // Describe final record of the flight
+ if (peer.NextEpoch.ServerKeyExchange.Length == 0)
+ {
+ peer.NextEpoch.ServerKeyExchange = new byte[peer.NextEpoch.Handshake.CalculateServerMessageSize(this.certificatePrivateKey)];
+ peer.NextEpoch.Handshake.EncodeServerKeyExchangeMessage(peer.NextEpoch.ServerKeyExchange, this.certificatePrivateKey);
+ }
+
+ Handshake serverKeyExchangeHandshake = new Handshake();
serverKeyExchangeHandshake.MessageType = HandshakeType.ServerKeyExchange;
- serverKeyExchangeHandshake.Length = (uint)peer.NextEpoch.Handshake.CalculateServerMessageSize(this.certificatePrivateKey);
+ serverKeyExchangeHandshake.Length = (uint)peer.NextEpoch.ServerKeyExchange.Length;
serverKeyExchangeHandshake.MessageSequence = 3;
serverKeyExchangeHandshake.FragmentOffset = 0;
serverKeyExchangeHandshake.FragmentLength = serverKeyExchangeHandshake.Length;
@@ -1136,7 +1201,7 @@ ref additionalRecord
writer = writer.Slice(Record.Size);
serverKeyExchangeHandshake.Encode(writer);
writer = writer.Slice(Handshake.Size);
- peer.NextEpoch.Handshake.EncodeServerKeyExchangeMessage(writer, this.certificatePrivateKey);
+ peer.NextEpoch.ServerKeyExchange.CopyTo(writer);
writer = writer.Slice((int)serverKeyExchangeHandshake.Length);
serverHelloDoneHandshake.Encode(writer);
@@ -1158,9 +1223,17 @@ ref additionalRecord
ref finalRecord
);
- base.QueueRawData(finalBuffer, peerAddress);
+ base.QueueRawData(finalBuffer, peerAddress);
- return true;
+ return true;
+ }
+ finally
+ {
+ if (certificateLockTaken)
+ {
+ this.certificateLock.ExitReadLock();
+ }
+ }
}
///
@@ -1395,7 +1468,11 @@ private void HandleStaleConnections(object _)
PeerData peer = kvp.Value;
lock (peer)
{
- if (peer.Epoch == 0 || peer.NextEpoch.State != HandshakeState.ExpectingHello)
+ bool waitingForApplicationConnection = !this.allConnections.ContainsKey(peer.ConnectionId)
+ && peer.ApplicationDataProcessingCount == 0;
+ if (waitingForApplicationConnection
+ || peer.Epoch == 0
+ || peer.NextEpoch.State != HandshakeState.ExpectingHello)
{
TimeSpan negotiationAge = now - peer.StartOfNegotiation;
if (negotiationAge > maxAge)
@@ -1422,16 +1499,61 @@ protected void MarkConnectionAsStale(ConnectionId connectionId)
if (this.allConnections.ContainsKey(connectionId))
{
this.staleConnections.Push(connectionId);
+ return;
+ }
+
+ // A DTLS peer is created before its application-level UDP
+ // connection. If negotiation stalls before that connection is
+ // created, there is nothing to disconnect, so remove and dispose
+ // the half-open peer directly.
+ if (this.existingPeers.TryGetValue(connectionId.EndPoint, out PeerData peer))
+ {
+ lock (peer)
+ {
+ if (peer.ConnectionId.Equals(connectionId)
+ && peer.ApplicationDataProcessingCount == 0
+ && !this.allConnections.ContainsKey(connectionId))
+ {
+ this.RemovePeerRecord(connectionId.EndPoint, peer);
+ }
+ }
}
}
///
internal override void RemovePeerRecord(ConnectionId connectionId)
{
- if (this.existingPeers.TryRemove(connectionId.EndPoint, out var peer))
+ if (this.existingPeers.TryGetValue(connectionId.EndPoint, out PeerData peer))
+ {
+ lock (peer)
+ {
+ if (peer.ConnectionId.Equals(connectionId))
+ {
+ this.RemovePeerRecord(connectionId.EndPoint, peer);
+ }
+ }
+ }
+ }
+
+ private bool RemovePeerRecord(IPEndPoint peerAddress, PeerData expectedPeer)
+ {
+ PeerData removedPeer = null;
+ lock (this.existingPeers)
+ {
+ if (this.existingPeers.TryGetValue(peerAddress, out PeerData currentPeer)
+ && ReferenceEquals(expectedPeer, currentPeer))
+ {
+ this.existingPeers.TryRemove(peerAddress, out removedPeer);
+ }
+ }
+
+ if (removedPeer != null)
{
- peer.Dispose();
+ removedPeer.Dispose();
+ return true;
}
+
+ return false;
}
///