Skip to content

Commit

Permalink
Check for bad chunk headers and provide remote endpoint in `IMessageS…
Browse files Browse the repository at this point in the history 
…ocket` (#2510)

- Reduce socket log output and demote to Trace
- Check for invalid chunk header and reduce log output
- Close socket immediately if a bad chunk header is detected (e.g. if socket is opened by another protocol)
- Expose Remote endpoint for application
  • Loading branch information
@mregen
mregen authored Feb 14, 2024
1 parent e9da6dc commit f8f1571
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 41 deletions.
22 changes: 11 additions & 11 deletions Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1301,11 +1301,12 @@ protected virtual async Task InternalValidateAsync(X509Certificate2Collection ce
}
}

if (endpoint != null && !FindDomain(certificate, endpoint))
Uri endpointUrl = endpoint?.EndpointUrl;
if (endpointUrl != null && !FindDomain(certificate, endpointUrl))
{
string message = Utils.Format(
"The domain '{0}' is not listed in the server certificate.",
endpoint.EndpointUrl.DnsSafeHost);
endpointUrl.DnsSafeHost);
sresult = new ServiceResult(StatusCodes.BadCertificateHostNameInvalid,
null, null, message, null, sresult
);
Expand Down Expand Up @@ -1462,13 +1463,12 @@ public void ValidateDomains(X509Certificate2 serverCertificate, ConfiguredEndpoi
}
}

bool domainFound = FindDomain(serverCertificate, endpoint);

if (!domainFound)
Uri endpointUrl = endpoint?.EndpointUrl;
if (endpointUrl != null && !FindDomain(serverCertificate, endpointUrl))
{
bool accept = false;
const string message = "The domain '{0}' is not listed in the server certificate.";
var serviceResult = ServiceResultException.Create(StatusCodes.BadCertificateHostNameInvalid, message, endpoint.EndpointUrl.DnsSafeHost);
var serviceResult = ServiceResultException.Create(StatusCodes.BadCertificateHostNameInvalid, message, endpointUrl.DnsSafeHost);
if (m_CertificateValidation != null)
{
var args = new CertificateValidationEventArgs(new ServiceResult(serviceResult), serverCertificate);
Expand All @@ -1480,7 +1480,7 @@ public void ValidateDomains(X509Certificate2 serverCertificate, ConfiguredEndpoi
{
if (serverValidation)
{
Utils.LogError(message, endpoint.EndpointUrl.DnsSafeHost);
Utils.LogError(message, endpointUrl.DnsSafeHost);
}
else
{
Expand Down Expand Up @@ -1689,9 +1689,9 @@ private static bool IsSignatureValid(X509Certificate2 cert)
/// endpoint that was used to connect a session.
/// </summary>
/// <param name="serverCertificate">The server certificate which is tested for domain names.</param>
/// <param name="endpoint">The endpoint which was used to connect.</param>
/// <param name="endpointUrl">The endpoint Url which was used to connect.</param>
/// <returns>True if domain was found.</returns>
private bool FindDomain(X509Certificate2 serverCertificate, ConfiguredEndpoint endpoint)
private bool FindDomain(X509Certificate2 serverCertificate, Uri endpointUrl)
{
bool domainFound = false;

Expand All @@ -1701,9 +1701,9 @@ private bool FindDomain(X509Certificate2 serverCertificate, ConfiguredEndpoint e
if (domains != null && domains.Count > 0)
{
string hostname;
string dnsHostName = hostname = endpoint.EndpointUrl.DnsSafeHost;
string dnsHostName = hostname = endpointUrl.DnsSafeHost;
bool isLocalHost = false;
if (endpoint.EndpointUrl.HostNameType == UriHostNameType.Dns)
if (endpointUrl.HostNameType == UriHostNameType.Dns)
{
if (String.Equals(dnsHostName, "localhost", StringComparison.OrdinalIgnoreCase))
{
Expand Down
52 changes: 34 additions & 18 deletions Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public void Attach(uint channelId, Socket socket)

Socket = new TcpMessageSocket(this, socket, BufferManager, Quotas.MaxBufferSize);

Utils.LogInfo("{0} SOCKET ATTACHED: {1:X8}, ChannelId={2}", ChannelName, Socket.Handle, ChannelId);
Utils.LogTrace("{0} SOCKET ATTACHED: {1:X8}, ChannelId={2}", ChannelName, Socket.Handle, ChannelId);

Socket.ReadNextMessage();

Expand Down Expand Up @@ -206,13 +206,6 @@ protected void ForceChannelFault(ServiceResult reason)
{
lock (DataLock)
{
Utils.LogError(
"{0} ForceChannelFault Socket={1:X8}, ChannelId={2}, TokenId={3}, Reason={4}",
ChannelName,
(Socket != null) ? Socket.Handle : 0,
(CurrentToken != null) ? CurrentToken.ChannelId : 0,
(CurrentToken != null) ? CurrentToken.TokenId : 0,
reason);

CompleteReverseHello(new ServiceResultException(reason));

Expand All @@ -222,6 +215,23 @@ protected void ForceChannelFault(ServiceResult reason)
return;
}

bool close = false;
if (State != TcpChannelState.Connecting)
{
Utils.LogError(
"{0} ForceChannelFault Socket={1:X8}, ChannelId={2}, TokenId={3}, Reason={4}",
ChannelName,
(Socket != null) ? Socket.Handle : 0,
(CurrentToken != null) ? CurrentToken.ChannelId : 0,
(CurrentToken != null) ? CurrentToken.TokenId : 0,
reason);
}
else
{
// Close immediately if the client never got out of connecting state
close = true;
}

// send error and close response.
if (Socket != null)
{
Expand All @@ -234,11 +244,19 @@ protected void ForceChannelFault(ServiceResult reason)
State = TcpChannelState.Faulted;
m_responseRequired = false;

// notify any monitors.
NotifyMonitors(reason, false);
if (close)
{
// close channel immediately.
ChannelClosed();
}
else
{
// notify any monitors.
NotifyMonitors(reason, false);

// ensure the channel will be cleaned up if the client does not reconnect.
StartCleanupTimer(reason);
// ensure the channel will be cleaned up if the client does not reconnect.
StartCleanupTimer(reason);
}
}
}

Expand All @@ -256,11 +274,8 @@ protected void StartCleanupTimer(ServiceResult reason)
/// </summary>
protected void CleanupTimer()
{
if (m_cleanupTimer != null)
{
m_cleanupTimer.Dispose();
m_cleanupTimer = null;
}
Utils.SilentDispose(m_cleanupTimer);
m_cleanupTimer = null;
}

/// <summary>
Expand All @@ -271,6 +286,7 @@ private void OnCleanup(object state)
lock (DataLock)
{
CleanupTimer();

// nothing to do if the channel is now open or closed.
if (State == TcpChannelState.Closed || State == TcpChannelState.Open)
{
Expand All @@ -284,7 +300,7 @@ private void OnCleanup(object state)
reason = new ServiceResult(StatusCodes.BadTimeout);
}

Utils.LogInfo(
Utils.LogTrace(
"{0} Cleanup Socket={1:X8}, ChannelId={2}, TokenId={3}, Reason={4}",
ChannelName,
(Socket != null) ? Socket.Handle : 0,
Expand Down
32 changes: 23 additions & 9 deletions Stack/Opc.Ua.Core/Stack/Tcp/TcpMessageSocket.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
m_socket.Dispose();
m_socket?.Dispose();
}
}
#endregion
Expand All @@ -309,7 +309,7 @@ protected virtual void Dispose(bool disposing)
/// Gets the socket handle.
/// </summary>
/// <value>The socket handle.</value>
public int Handle => m_socket != null ? m_socket.GetHashCode() : -1;
public int Handle => m_socket?.GetHashCode() ?? -1;

/// <summary>
/// Gets the local endpoint.
Expand All @@ -318,7 +318,16 @@ protected virtual void Dispose(bool disposing)
/// See the Remarks section for more information.</exception>
/// <exception cref="System.ObjectDisposedException">The System.Net.Sockets.Socket has been closed.</exception>
/// <returns>The System.Net.EndPoint that the System.Net.Sockets.Socket is using for communications.</returns>
public EndPoint LocalEndpoint => m_socket.LocalEndPoint;
public EndPoint LocalEndpoint => m_socket?.LocalEndPoint;

/// <summary>
/// Gets the local endpoint.
/// </summary>
/// <exception cref="System.Net.Sockets.SocketException">An error occurred when attempting to access the socket.
/// See the Remarks section for more information.</exception>
/// <exception cref="System.ObjectDisposedException">The System.Net.Sockets.Socket has been closed.</exception>
/// <returns>The System.Net.EndPoint that the System.Net.Sockets.Socket is using for communications.</returns>
public EndPoint RemoteEndpoint => m_socket?.RemoteEndPoint;

/// <summary>
/// Gets the transport channel features implemented by this message socket.
Expand Down Expand Up @@ -521,15 +530,20 @@ private ServiceResult DoReadComplete(SocketAsyncEventArgs e)
// start reading the message body.
if (m_incomingMessageSize < 0)
{
m_incomingMessageSize = BitConverter.ToInt32(m_receiveBuffer, 4);
UInt32 messageType = BitConverter.ToUInt32(m_receiveBuffer, 0);
if (!TcpMessageType.IsValid(messageType))
{
m_readState = ReadState.Error;

return ServiceResult.Create(
StatusCodes.BadTcpMessageTypeInvalid,
"Message type {0:X8} is invalid.",
messageType);
}

m_incomingMessageSize = BitConverter.ToInt32(m_receiveBuffer, 4);
if (m_incomingMessageSize <= 0 || m_incomingMessageSize > m_receiveBufferSize)
{
Utils.LogError(
"BadTcpMessageTooLarge: BufferSize={0}; MessageSize={1}",
m_receiveBufferSize,
m_incomingMessageSize);

m_readState = ReadState.Error;

return ServiceResult.Create(
Expand Down
3 changes: 2 additions & 1 deletion Stack/Opc.Ua.Core/Stack/Tcp/TcpMessageType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ public static bool IsValid(uint messageType)
}
}

if (((messageType & ChunkTypeMask) != Final) && ((messageType & ChunkTypeMask) != Intermediate))
uint chunkTypeMask = messageType & ChunkTypeMask;
if ((chunkTypeMask != Final) && (chunkTypeMask != Intermediate))
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion Stack/Opc.Ua.Core/Stack/Tcp/UaSCBinaryChannel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ protected TcpChannelState State
{
if (m_state != value)
{
Utils.LogInfo("ChannelId {0}: in {1} state.", ChannelId, value);
Utils.LogTrace("ChannelId {0}: in {1} state.", ChannelId, value);
}

m_state = value;
Expand Down
12 changes: 11 additions & 1 deletion Stack/Opc.Ua.Core/Stack/Transport/IMessageSocket.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.


using System;
using System.Net;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -148,7 +149,16 @@ public interface IMessageSocket : IDisposable
/// See the Remarks section for more information.</exception>
/// <exception cref="System.ObjectDisposedException">The Socket has been closed.</exception>
/// <returns>The System.Net.EndPoint that the Socket is using for communications.</returns>
System.Net.EndPoint LocalEndpoint { get; }
EndPoint LocalEndpoint { get; }

/// <summary>
/// Gets the remote endpoint.
/// </summary>
/// <exception cref="System.Net.Sockets.SocketException">An error occurred when attempting to access the socket.
/// See the Remarks section for more information.</exception>
/// <exception cref="System.ObjectDisposedException">The Socket has been closed.</exception>
/// <returns>The System.Net.EndPoint that the Socket is using for communications.</returns>
EndPoint RemoteEndpoint { get; }

/// <summary>
/// Returns the features implemented by the message socket.
Expand Down

0 comments on commit f8f1571

Please sign in to comment.