Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable mutual tls on server https endpoints #2849

Open
wants to merge 11 commits into
base: develop/main374
Choose a base branch
from
2 changes: 1 addition & 1 deletion Libraries/Opc.Ua.Client/CoreClientUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public static EndpointDescription SelectEndpoint(
public static Uri GetDiscoveryUrl(string discoveryUrl)
{
// needs to add the '/discovery' back onto non-UA TCP URLs.
if (discoveryUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal))
if (Utils.IsUriHttpRelatedScheme(discoveryUrl))
{
if (!discoveryUrl.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,13 @@ public IApplicationConfigurationBuilderServerOptions SetAuditingEnabled(bool aud
return this;
}

/// <inheritdoc/>
public IApplicationConfigurationBuilderServerOptions SetHttpsMutualTls(bool mutualTlsEnabeld)
{
ApplicationConfiguration.ServerConfiguration.HttpsMutualTls = mutualTlsEnabeld;
return this;
}

/// <inheritdoc/>
public IApplicationConfigurationBuilderClientOptions SetDefaultSessionTimeout(int defaultSessionTimeout)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ public interface IApplicationConfigurationBuilderServerOptions :

/// <inheritdoc cref="ServerConfiguration.AuditingEnabled"/>
IApplicationConfigurationBuilderServerOptions SetAuditingEnabled(bool auditingEnabled);

/// <inheritdoc cref="ServerConfiguration.HttpsMutualTls"/>
IApplicationConfigurationBuilderServerOptions SetHttpsMutualTls(bool mTlsEnabled);
}

/// <summary>
Expand Down
112 changes: 110 additions & 2 deletions Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
using System.Collections.Generic;
using System.IO;
using System.Net;
using System.Net.Security;
using System.Security.Authentication;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
Expand All @@ -24,6 +25,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Server.Kestrel.Https;
using Microsoft.Extensions.Hosting;
using Opc.Ua.Security.Certificates;


namespace Opc.Ua.Bindings
Expand Down Expand Up @@ -205,6 +207,7 @@ public void Open(
m_serverCertificate = settings.ServerCertificate;
m_serverCertificateChain = settings.ServerCertificateChain;

m_mutualTlsEnabeld = settings.HttpsMutualTls;
// start the listener
Start();
}
Expand Down Expand Up @@ -283,9 +286,10 @@ public void Start()

var httpsOptions = new HttpsConnectionAdapterOptions() {
CheckCertificateRevocation = false,
ClientCertificateMode = ClientCertificateMode.NoCertificate,
ClientCertificateMode = m_mutualTlsEnabeld == true ? ClientCertificateMode.AllowCertificate : ClientCertificateMode.NoCertificate,
// note: this is the TLS certificate!
ServerCertificate = serverCertificate,
ClientCertificateValidation = ValidateClientCertificate,
};

#if NET462
Expand Down Expand Up @@ -333,7 +337,7 @@ public void Stop()
{
Dispose();
}
#endregion
#endregion

#region Private Methods
/// <summary>
Expand Down Expand Up @@ -368,8 +372,40 @@ public async Task SendAsync(HttpContext context)
return;
}

string path = context.Request.Path.Value?.TrimEnd('/') ?? string.Empty;
string discoveryPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix;
ReadOnlyMemory<char> pathMemory = path.AsMemory();
ReadOnlyMemory<char> discoveryPathMemory = discoveryPath.AsMemory();

bool isDiscoveryPath = discoveryPathMemory.Span.EndsWith(pathMemory.Span, StringComparison.OrdinalIgnoreCase);
bool validateClientTlsCertificate = !isDiscoveryPath && m_mutualTlsEnabeld == true;

// Access and validate tls client certificate
if (validateClientTlsCertificate)
{
if (!await ValidateClientTlsCertificateAsync(context, ct).ConfigureAwait(false))
{
return;
}
}

IServiceRequest input = (IServiceRequest)BinaryDecoder.DecodeMessage(buffer, null, m_quotas.MessageContext);

// Match tls client certificate against client application certificate provided in CreateSessionRequest
if (validateClientTlsCertificate &&
input.TypeId == DataTypeIds.CreateSessionRequest)
{
byte[] tlsClientCertificate = context.Connection.ClientCertificate.RawData;
byte[] opcUaClientCertificate = ((CreateSessionRequest)input).ClientCertificate;
if (!Utils.IsEqual(tlsClientCertificate, opcUaClientCertificate))
{
message = "Client TLS certificate does not match with ClientCertificate provided in CreateSessionRequest";
Utils.LogError(message);
await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false);
return;
}
}

// extract the JWT token from the HTTP headers.
if (input.RequestHeader == null)
{
Expand Down Expand Up @@ -463,6 +499,8 @@ public async Task SendAsync(HttpContext context)
await WriteResponseAsync(context.Response, message, HttpStatusCode.InternalServerError).ConfigureAwait(false);
}



/// <summary>
/// Called when a UpdateCertificate event occured.
/// </summary>
Expand Down Expand Up @@ -524,6 +562,75 @@ private static async Task<byte[]> ReadBodyAsync(HttpRequest req)
return memory.ToArray();
}
}

/// <summary>
/// Validate TLS client certificate at TLS handshake
/// </summary>
/// <param name="clientCertificate">Client certificate</param>
/// <param name="chain">Certificate chain</param>
/// <param name="sslPolicyErrors">SSl policy errors</param>
/// <returns></returns>
private bool ValidateClientCertificate(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove empty params

X509Certificate2 clientCertificate,
X509Chain chain,
SslPolicyErrors sslPolicyErrors)
{

if (sslPolicyErrors == SslPolicyErrors.None)
{
// certificate is valid
return true;
}

try
{
m_quotas.CertificateValidator.Validate(clientCertificate);
}
catch (Exception)
{
return false;
}

return true;
}

/// <summary>
/// Validates the client TLS certificate per request
/// </summary>
/// <param name="context">Context of the validation</param>
/// <param name="ct">Cancellation token</param>
/// <returns>true if validation passes, else false</returns>
private async Task<bool> ValidateClientTlsCertificateAsync(HttpContext context, CancellationToken ct)
{
var clientCertificate = context.Connection.ClientCertificate;

if (clientCertificate == null)
{
string message = "Client TLS certificate is required";
Utils.LogError(message);
// No tls client certificate is provided
await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false);
return false;
}

// Validate the client certificate
if (clientCertificate != null && m_quotas.CertificateValidator != null)
{
try
{
await m_quotas.CertificateValidator.ValidateAsync(clientCertificate, ct).ConfigureAwait(false);
}
catch (Exception ex)
{
string message = "Client TLS certificate validation failed.";
Utils.LogError(ex, message);
await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false);
return false;
}
}

return true;
}
#endregion

#region Private Fields
Expand All @@ -537,6 +644,7 @@ private static async Task<byte[]> ReadBodyAsync(HttpRequest req)
private IWebHost m_host;
private X509Certificate2 m_serverCertificate;
private X509Certificate2Collection m_serverCertificateChain;
private bool m_mutualTlsEnabeld;
#endregion
}
}
13 changes: 13 additions & 0 deletions Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,7 @@ private void Initialize()
m_maxTrustListSize = 0;
m_multicastDnsEnabled = false;
m_auditingEnabled = false;
m_httpsMutualTls = true;
}

/// <summary>
Expand Down Expand Up @@ -1942,6 +1943,17 @@ public bool AuditingEnabled
get { return m_auditingEnabled; }
set { m_auditingEnabled = value; }
}

/// <summary>
/// Whether mTLS is required/enforced by the HttpsTransportListener
/// </summary>
/// <value><c>true</c> if mutual TLS is enabled; otherwise, <c>false</c>.</value>
[DataMember(IsRequired = false, Order = 38)]
public bool HttpsMutualTls
{
get { return m_httpsMutualTls; }
set { m_httpsMutualTls = value; }
}
#endregion

#region Private Members
Expand Down Expand Up @@ -1980,6 +1992,7 @@ public bool AuditingEnabled
private ReverseConnectServerConfiguration m_reverseConnect;
private OperationLimits m_operationLimits;
private bool m_auditingEnabled;
private bool m_httpsMutualTls;
#endregion
}
#endregion
Expand Down
1 change: 1 addition & 0 deletions Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@
<xs:element name="ReverseConnect" type="ReverseConnectServerConfiguration" minOccurs="0" />
<xs:element name="OperationLimits" type="OperationLimits" minOccurs="0" />
<xs:element name="AuditingEnabled" type="xs:boolean" minOccurs="0" />
<xs:element name="HttpsMutualTls" type="xs:boolean" minOccurs="0" />
</xs:sequence>
</xs:extension>
</xs:complexContent>
Expand Down
9 changes: 5 additions & 4 deletions Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
using System.Globalization;
using System.IO;
using System.Runtime.Serialization;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
Expand Down Expand Up @@ -130,7 +131,7 @@ public static ConfiguredEndpointCollection Load(string filePath)
{
string discoveryUrl = endpoint.Description.EndpointUrl;

if (discoveryUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal))
if (Utils.IsUriHttpRelatedScheme(discoveryUrl))
{
discoveryUrl += ConfiguredEndpoint.DiscoverySuffix;
}
Expand Down Expand Up @@ -530,7 +531,7 @@ public void SetApplicationDescription(string serverUri, ApplicationDescription s
}

if (endpointUrl != null &&
endpointUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) &&
Utils.IsUriHttpRelatedScheme(endpointUrl) &&
endpointUrl.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase))
{
endpointUrl = endpointUrl.Substring(0, endpointUrl.Length - ConfiguredEndpoint.DiscoverySuffix.Length);
Expand Down Expand Up @@ -815,7 +816,7 @@ public ConfiguredEndpoint(

if (baseUrl != null)
{
if (baseUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) &&
if (Utils.IsUriHttpRelatedScheme(baseUrl) &&
baseUrl.EndsWith(DiscoverySuffix, StringComparison.Ordinal))
{
baseUrl = baseUrl.Substring(0, baseUrl.Length - DiscoverySuffix.Length);
Expand Down Expand Up @@ -1208,7 +1209,7 @@ public Uri GetDiscoveryUrl(Uri endpointUrl)
// attempt to construct a discovery url by appending 'discovery' to the endpoint.
if (discoveryUrls == null || discoveryUrls.Count == 0)
{
if (endpointUrl.Scheme.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal))
if (Utils.IsUriHttpRelatedScheme(endpointUrl.Scheme))
{
return new Uri(Utils.Format("{0}{1}", endpointUrl, DiscoverySuffix));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public EndpointDescription(string url)

UriBuilder parsedUrl = new UriBuilder(url);

if (parsedUrl.Scheme.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal))
if (Utils.IsUriHttpRelatedScheme(parsedUrl.Scheme))
{
if (!parsedUrl.Path.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase))
{
Expand Down
19 changes: 18 additions & 1 deletion Stack/Opc.Ua.Core/Stack/Https/HttpsTransportChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
using System.Net.Http.Headers;
using System.Net.Security;
using System.Reflection;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -158,11 +159,27 @@ public void Open()
// send client certificate for servers that require TLS client authentication
if (m_settings.ClientCertificate != null)
{
// prepare the server TLS certificate
var clientCertificate = m_settings.ClientCertificate;
#if NETCOREAPP3_1_OR_GREATER || NETSTANDARD2_1 || NET472_OR_GREATER || NET5_0_OR_GREATER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this guard clause?

try
{
// Create a copy of the certificate with the private key on platforms
// which default to the ephemeral KeySet. Also a new certificate must be reloaded.
// If the key fails to copy, its probably a non exportable key from the X509Store.
// Then we can use the original certificate, the private key is already in the key store.
clientCertificate = X509Utils.CreateCopyWithPrivateKey(m_settings.ClientCertificate, false);
}
catch (CryptographicException ce)
{
Utils.LogTrace("Copy of the private key for https was denied: {0}", ce.Message);
}
#endif
PropertyInfo certProperty = handler.GetType().GetProperty("ClientCertificates");
if (certProperty != null)
{
X509CertificateCollection clientCertificates = (X509CertificateCollection)certProperty.GetValue(handler);
_ = clientCertificates?.Add(m_settings.ClientCertificate);
_ = clientCertificates?.Add(clientCertificate);
}
}

Expand Down
8 changes: 6 additions & 2 deletions Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,10 @@ ICertificateValidator certificateValidator
if (m_configuration is ApplicationConfiguration applicationConfiguration)
{
settings.MaxChannelCount = applicationConfiguration.ServerConfiguration.MaxChannelCount;
if (Utils.IsUriHttpsScheme(endpointUri.AbsoluteUri))
{
settings.HttpsMutualTls = applicationConfiguration.ServerConfiguration.HttpsMutualTls;
}
}

listener.Open(
Expand Down Expand Up @@ -1140,8 +1144,8 @@ private static string GetBestDiscoveryUrl(Uri clientUrl, BaseAddress baseAddress
string url = baseAddress.Url.ToString();

if ((baseAddress.ProfileUri == Profiles.HttpsBinaryTransport) &&
url.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) &&
(!(url.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase))))
Utils.IsUriHttpRelatedScheme(url) &&
(!url.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)))
{
url += ConfiguredEndpoint.DiscoverySuffix;
}
Expand Down
12 changes: 12 additions & 0 deletions Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ public int MaxChannelCount
get { return m_maxChannelCount; }
set { m_maxChannelCount = value; }
}

/// <summary>
/// Indicates if Http listener requires mutual TLS
/// Handled only by HttpsTransportListner
/// In case true, the client should provide it's own valid TLS certificate to the TLS layer for the connection to succeed.
/// </summary>
public bool HttpsMutualTls
{
get { return m_httpMutualTls; }
set { m_httpMutualTls = value; }
}
#endregion

#region Private Fields
Expand All @@ -139,6 +150,7 @@ public int MaxChannelCount
private IEncodeableFactory m_channelFactory;
private bool m_reverseConnectListener;
private int m_maxChannelCount;
private bool m_httpMutualTls;
#endregion
}
}
Loading