From 85298946c6c34ebbff296c9bfb4637d309c644e2 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Thu, 31 Oct 2024 13:56:06 +0200 Subject: [PATCH 01/11] Enforce TLS certificate validation for all services including GetEndpoitns --- Libraries/Opc.Ua.Client/Session/Session.cs | 11 +++- .../Stack/Https/HttpsTransportListener.cs | 64 ++++++++++++++++++- .../Stack/Client/DiscoveryClient.cs | 12 +++- .../Configuration/ConfiguredEndpoints.cs | 15 +++-- .../Stack/Https/HttpsTransportChannel.cs | 19 +++++- 5 files changed, 110 insertions(+), 11 deletions(-) diff --git a/Libraries/Opc.Ua.Client/Session/Session.cs b/Libraries/Opc.Ua.Client/Session/Session.cs index 3de3e6573..f5c2638d2 100644 --- a/Libraries/Opc.Ua.Client/Session/Session.cs +++ b/Libraries/Opc.Ua.Client/Session/Session.cs @@ -1064,7 +1064,16 @@ public static async Task CreateChannelAsync( // update endpoint description using the discovery endpoint. if (endpoint.UpdateBeforeConnect && connection == null) { - await endpoint.UpdateFromServerAsync(ct).ConfigureAwait(false); + if (Utils.IsUriHttpsScheme(endpoint.EndpointUrl.AbsoluteUri)) + { + // https channels need a TLS certificate for the TLS authentication + await endpoint.UpdateFromServerAsync(ct, configuration).ConfigureAwait(false); + } + else + { + await endpoint.UpdateFromServerAsync(ct).ConfigureAwait(false); + } + endpointDescription = endpoint.Description; endpointConfiguration = endpoint.Configuration; } diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index 49a8bbc32..705c9aa2a 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -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; @@ -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 @@ -283,9 +285,10 @@ public void Start() var httpsOptions = new HttpsConnectionAdapterOptions() { CheckCertificateRevocation = false, - ClientCertificateMode = ClientCertificateMode.NoCertificate, + ClientCertificateMode = ClientCertificateMode.RequireCertificate, // note: this is the TLS certificate! ServerCertificate = serverCertificate, + ClientCertificateValidation = ValidateClientCertificate, }; #if NET462 @@ -333,7 +336,7 @@ public void Stop() { Dispose(); } - #endregion +#endregion #region Private Methods /// @@ -368,6 +371,33 @@ public async Task SendAsync(HttpContext context) return; } + // Access client certificate + var clientCertificate = context.Connection.ClientCertificate; + + if (clientCertificate == null) + { + // No client certificate is provided + message = "Client certificate is required"; + await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false); + return; + } + + // Validate the client certificate + if (clientCertificate != null && m_quotas.CertificateValidator != null) + { + try + { + await m_quotas.CertificateValidator.ValidateAsync(clientCertificate, ct).ConfigureAwait(false); + } + catch (Exception ex) + { + message = "Client certificate validation failed."; + Utils.LogError(ex, message); + await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait (false); + return; + } + } + IServiceRequest input = (IServiceRequest)BinaryDecoder.DecodeMessage(buffer, null, m_quotas.MessageContext); // extract the JWT token from the HTTP headers. @@ -524,6 +554,36 @@ private static async Task ReadBodyAsync(HttpRequest req) return memory.ToArray(); } } + + /// + /// Validate TLS Client certificate + /// + /// + /// + /// + /// + private bool ValidateClientCertificate( + 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; + } #endregion #region Private Fields diff --git a/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs b/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs index 1cc658329..e5003c3b7 100644 --- a/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs +++ b/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs @@ -48,7 +48,17 @@ public static DiscoveryClient Create( configuration = EndpointConfiguration.Create(); } - ITransportChannel channel = DiscoveryChannel.Create(application, discoveryUrl, configuration, application.CreateMessageContext()); + ITransportChannel channel; + if (Utils.IsUriHttpsScheme(discoveryUrl.AbsoluteUri)) + { + // https channels need a TLS certificate for the TLS authentication + channel = DiscoveryChannel.Create(application, discoveryUrl, configuration, application.CreateMessageContext(), + application.SecurityConfiguration.ApplicationCertificate.Certificate); + } + else + { + channel = DiscoveryChannel.Create(application, discoveryUrl, configuration, application.CreateMessageContext()); + } return new DiscoveryClient(channel); } diff --git a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs index 29cf6b273..f24595c4f 100644 --- a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs +++ b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs @@ -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; @@ -1115,9 +1116,9 @@ public void UpdateFromServer( /// /// Updates an endpoint with information from the server's discovery endpoint. /// - public Task UpdateFromServerAsync(CancellationToken ct = default) + public Task UpdateFromServerAsync(CancellationToken ct = default, ApplicationConfiguration applicationConfiguration = null) { - return UpdateFromServerAsync(EndpointUrl, m_description.SecurityMode, m_description.SecurityPolicyUri, ct); + return UpdateFromServerAsync(EndpointUrl, m_description.SecurityMode, m_description.SecurityPolicyUri, ct, applicationConfiguration); } /// @@ -1127,9 +1128,10 @@ public Task UpdateFromServerAsync( Uri endpointUrl, MessageSecurityMode securityMode, string securityPolicyUri, - CancellationToken ct = default) + CancellationToken ct = default, + ApplicationConfiguration applicationConfiguration = null) { - return UpdateFromServerAsync(endpointUrl, null, securityMode, securityPolicyUri, ct); + return UpdateFromServerAsync(endpointUrl, null, securityMode, securityPolicyUri, ct, applicationConfiguration); } /// @@ -1140,7 +1142,8 @@ public async Task UpdateFromServerAsync( ITransportWaitingConnection connection, MessageSecurityMode securityMode, string securityPolicyUri, - CancellationToken ct = default) + CancellationToken ct = default, + ApplicationConfiguration applicationConfiguration = null) { // get the a discovery url. Uri discoveryUrl = GetDiscoveryUrl(endpointUrl); @@ -1153,7 +1156,7 @@ public async Task UpdateFromServerAsync( } else { - client = DiscoveryClient.Create(discoveryUrl, m_configuration); + client = DiscoveryClient.Create(discoveryUrl, m_configuration, applicationConfiguration); } try diff --git a/Stack/Opc.Ua.Core/Stack/Https/HttpsTransportChannel.cs b/Stack/Opc.Ua.Core/Stack/Https/HttpsTransportChannel.cs index 387f22453..b56b62f2a 100644 --- a/Stack/Opc.Ua.Core/Stack/Https/HttpsTransportChannel.cs +++ b/Stack/Opc.Ua.Core/Stack/Https/HttpsTransportChannel.cs @@ -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; @@ -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 + 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); } } From 8521c5598adba30d226bac1a9c46b6daf4b01aa0 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Mon, 4 Nov 2024 17:21:22 +0200 Subject: [PATCH 02/11] Add HttpsMutualTls ServerConfiguration configuration option. --- .../ApplicationConfigurationBuilder.cs | 7 +++++++ .../IApplicationConfigurationBuilder.cs | 3 +++ .../Stack/Https/HttpsTransportListener.cs | 5 ++++- Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs | 12 ++++++++++++ .../Opc.Ua.Core/Schema/ApplicationConfiguration.xsd | 1 + Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs | 4 ++++ .../Stack/Transport/TransportListenerSettings.cs | 12 ++++++++++++ 7 files changed, 43 insertions(+), 1 deletion(-) diff --git a/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs b/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs index a5240c95b..9e7844497 100644 --- a/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs +++ b/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs @@ -779,6 +779,13 @@ public IApplicationConfigurationBuilderServerOptions SetAuditingEnabled(bool aud return this; } + /// + public IApplicationConfigurationBuilderServerOptions SetHttpsMutualTls(bool mTlsEnabled) + { + ApplicationConfiguration.ServerConfiguration.HttpsMutualTls = mTlsEnabled; + return this; + } + /// public IApplicationConfigurationBuilderClientOptions SetDefaultSessionTimeout(int defaultSessionTimeout) { diff --git a/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs b/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs index 753c2666a..4d89e47a6 100644 --- a/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs +++ b/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs @@ -265,6 +265,9 @@ public interface IApplicationConfigurationBuilderServerOptions : /// IApplicationConfigurationBuilderServerOptions SetAuditingEnabled(bool auditingEnabled); + + /// + IApplicationConfigurationBuilderServerOptions SetHttpsMutualTls(bool mTlsEnabled); } /// diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index 705c9aa2a..0ddd99b82 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -207,6 +207,8 @@ public void Open( m_serverCertificate = settings.ServerCertificate; m_serverCertificateChain = settings.ServerCertificateChain; + m_ClientCertificateMode = settings.HttpMutualTls ? ClientCertificateMode.RequireCertificate : ClientCertificateMode.NoCertificate; + // start the listener Start(); } @@ -285,7 +287,7 @@ public void Start() var httpsOptions = new HttpsConnectionAdapterOptions() { CheckCertificateRevocation = false, - ClientCertificateMode = ClientCertificateMode.RequireCertificate, + ClientCertificateMode = m_ClientCertificateMode, // note: this is the TLS certificate! ServerCertificate = serverCertificate, ClientCertificateValidation = ValidateClientCertificate, @@ -597,6 +599,7 @@ private bool ValidateClientCertificate( private IWebHost m_host; private X509Certificate2 m_serverCertificate; private X509Certificate2Collection m_serverCertificateChain; + private ClientCertificateMode m_ClientCertificateMode; #endregion } } diff --git a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs index 43ac29014..f2f0036f8 100644 --- a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs +++ b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs @@ -1942,6 +1942,17 @@ public bool AuditingEnabled get { return m_auditingEnabled; } set { m_auditingEnabled = value; } } + + /// + /// Whether mTLS is required/enforced by the HttpsTransportListener + /// + /// true if mutual TLS is enabled; otherwise, false. + [DataMember(IsRequired = false, Order = 38)] + public bool HttpsMutualTls + { + get { return m_httpsMTls; } + set { m_httpsMTls = value; } + } #endregion #region Private Members @@ -1980,6 +1991,7 @@ public bool AuditingEnabled private ReverseConnectServerConfiguration m_reverseConnect; private OperationLimits m_operationLimits; private bool m_auditingEnabled; + private bool m_httpsMTls; #endregion } #endregion diff --git a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd index d3d201f67..f6e4aec92 100644 --- a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd +++ b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd @@ -168,6 +168,7 @@ + diff --git a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs index c29d980f3..fcbc23d1a 100644 --- a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs +++ b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs @@ -884,6 +884,10 @@ ICertificateValidator certificateValidator if (m_configuration is ApplicationConfiguration applicationConfiguration) { settings.MaxChannelCount = applicationConfiguration.ServerConfiguration.MaxChannelCount; + if (Utils.IsUriHttpsScheme(endpointUri.AbsoluteUri)) + { + settings.HttpMutualTls = applicationConfiguration.ServerConfiguration.HttpsMutualTls; + } } listener.Open( diff --git a/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs b/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs index a67ce0f4d..9c15c1fa5 100644 --- a/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs +++ b/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs @@ -127,6 +127,17 @@ public int MaxChannelCount get { return m_maxChannelCount; } set { m_maxChannelCount = value; } } + + /// + /// 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. + /// + public bool HttpMutualTls + { + get { return m_httpMutualTls; } + set { m_httpMutualTls = value; } + } #endregion #region Private Fields @@ -139,6 +150,7 @@ public int MaxChannelCount private IEncodeableFactory m_channelFactory; private bool m_reverseConnectListener; private int m_maxChannelCount; + private bool m_httpMutualTls; #endregion } } From 665d405a7832297a988cd207f8698aebfc2aef0e Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Mon, 4 Nov 2024 18:35:03 +0200 Subject: [PATCH 03/11] Validate client certificate only on mutual TLS --- .../Stack/Https/HttpsTransportListener.cs | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index 0ddd99b82..edcb3a6b4 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -374,29 +374,32 @@ public async Task SendAsync(HttpContext context) } // Access client certificate - var clientCertificate = context.Connection.ClientCertificate; - - if (clientCertificate == null) + if (m_ClientCertificateMode == ClientCertificateMode.RequireCertificate) { - // No client certificate is provided - message = "Client certificate is required"; - await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false); - return; - } + var clientCertificate = context.Connection.ClientCertificate; - // Validate the client certificate - if (clientCertificate != null && m_quotas.CertificateValidator != null) - { - try + if (clientCertificate == null) { - await m_quotas.CertificateValidator.ValidateAsync(clientCertificate, ct).ConfigureAwait(false); + // No client certificate is provided + message = "Client certificate is required"; + await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false); + return; } - catch (Exception ex) + + // Validate the client certificate + if (clientCertificate != null && m_quotas.CertificateValidator != null) { - message = "Client certificate validation failed."; - Utils.LogError(ex, message); - await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait (false); - return; + try + { + await m_quotas.CertificateValidator.ValidateAsync(clientCertificate, ct).ConfigureAwait(false); + } + catch (Exception ex) + { + message = "Client certificate validation failed."; + Utils.LogError(ex, message); + await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false); + return; + } } } From 982c8f441a149bd8ebab4f3a2b34bac21b9d5c3c Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Thu, 7 Nov 2024 17:14:01 +0200 Subject: [PATCH 04/11] Discovery url route allows non mutual tls access while endpoint enforces it --- Libraries/Opc.Ua.Client/CoreClientUtils.cs | 4 +++- Libraries/Opc.Ua.Client/Session/Session.cs | 11 +---------- .../Stack/Https/HttpsTransportListener.cs | 8 ++++++-- Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs | 12 +----------- .../Stack/Configuration/ConfiguredEndpoints.cs | 14 ++++++-------- 5 files changed, 17 insertions(+), 32 deletions(-) diff --git a/Libraries/Opc.Ua.Client/CoreClientUtils.cs b/Libraries/Opc.Ua.Client/CoreClientUtils.cs index b73f01aba..d0b021148 100644 --- a/Libraries/Opc.Ua.Client/CoreClientUtils.cs +++ b/Libraries/Opc.Ua.Client/CoreClientUtils.cs @@ -307,7 +307,9 @@ 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 (discoveryUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + discoveryUrl.StartsWith(Utils.UriSchemeHttps, StringComparison.Ordinal) || + discoveryUrl.StartsWith(Utils.UriSchemeOpcHttps, StringComparison.Ordinal)) { if (!discoveryUrl.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)) { diff --git a/Libraries/Opc.Ua.Client/Session/Session.cs b/Libraries/Opc.Ua.Client/Session/Session.cs index f5c2638d2..3de3e6573 100644 --- a/Libraries/Opc.Ua.Client/Session/Session.cs +++ b/Libraries/Opc.Ua.Client/Session/Session.cs @@ -1064,16 +1064,7 @@ public static async Task CreateChannelAsync( // update endpoint description using the discovery endpoint. if (endpoint.UpdateBeforeConnect && connection == null) { - if (Utils.IsUriHttpsScheme(endpoint.EndpointUrl.AbsoluteUri)) - { - // https channels need a TLS certificate for the TLS authentication - await endpoint.UpdateFromServerAsync(ct, configuration).ConfigureAwait(false); - } - else - { - await endpoint.UpdateFromServerAsync(ct).ConfigureAwait(false); - } - + await endpoint.UpdateFromServerAsync(ct).ConfigureAwait(false); endpointDescription = endpoint.Description; endpointConfiguration = endpoint.Configuration; } diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index edcb3a6b4..50040ae3e 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -287,7 +287,7 @@ public void Start() var httpsOptions = new HttpsConnectionAdapterOptions() { CheckCertificateRevocation = false, - ClientCertificateMode = m_ClientCertificateMode, + ClientCertificateMode = ClientCertificateMode.AllowCertificate, // note: this is the TLS certificate! ServerCertificate = serverCertificate, ClientCertificateValidation = ValidateClientCertificate, @@ -373,8 +373,11 @@ public async Task SendAsync(HttpContext context) return; } + string path = context.Request.Path.Value?.TrimEnd('/') ?? string.Empty; + string currentPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix; + bool isDiscoveryPath = path.Equals(currentPath, StringComparison.OrdinalIgnoreCase); // Access client certificate - if (m_ClientCertificateMode == ClientCertificateMode.RequireCertificate) + if (!isDiscoveryPath && m_ClientCertificateMode == ClientCertificateMode.RequireCertificate) { var clientCertificate = context.Connection.ClientCertificate; @@ -572,6 +575,7 @@ private bool ValidateClientCertificate( X509Chain chain, SslPolicyErrors sslPolicyErrors) { + if (sslPolicyErrors == SslPolicyErrors.None) { // certificate is valid diff --git a/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs b/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs index e5003c3b7..1cc658329 100644 --- a/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs +++ b/Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs @@ -48,17 +48,7 @@ public static DiscoveryClient Create( configuration = EndpointConfiguration.Create(); } - ITransportChannel channel; - if (Utils.IsUriHttpsScheme(discoveryUrl.AbsoluteUri)) - { - // https channels need a TLS certificate for the TLS authentication - channel = DiscoveryChannel.Create(application, discoveryUrl, configuration, application.CreateMessageContext(), - application.SecurityConfiguration.ApplicationCertificate.Certificate); - } - else - { - channel = DiscoveryChannel.Create(application, discoveryUrl, configuration, application.CreateMessageContext()); - } + ITransportChannel channel = DiscoveryChannel.Create(application, discoveryUrl, configuration, application.CreateMessageContext()); return new DiscoveryClient(channel); } diff --git a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs index f24595c4f..74b663e6c 100644 --- a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs +++ b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs @@ -1116,9 +1116,9 @@ public void UpdateFromServer( /// /// Updates an endpoint with information from the server's discovery endpoint. /// - public Task UpdateFromServerAsync(CancellationToken ct = default, ApplicationConfiguration applicationConfiguration = null) + public Task UpdateFromServerAsync(CancellationToken ct = default) { - return UpdateFromServerAsync(EndpointUrl, m_description.SecurityMode, m_description.SecurityPolicyUri, ct, applicationConfiguration); + return UpdateFromServerAsync(EndpointUrl, m_description.SecurityMode, m_description.SecurityPolicyUri, ct); } /// @@ -1128,10 +1128,9 @@ public Task UpdateFromServerAsync( Uri endpointUrl, MessageSecurityMode securityMode, string securityPolicyUri, - CancellationToken ct = default, - ApplicationConfiguration applicationConfiguration = null) + CancellationToken ct = default) { - return UpdateFromServerAsync(endpointUrl, null, securityMode, securityPolicyUri, ct, applicationConfiguration); + return UpdateFromServerAsync(endpointUrl, null, securityMode, securityPolicyUri, ct); } /// @@ -1142,8 +1141,7 @@ public async Task UpdateFromServerAsync( ITransportWaitingConnection connection, MessageSecurityMode securityMode, string securityPolicyUri, - CancellationToken ct = default, - ApplicationConfiguration applicationConfiguration = null) + CancellationToken ct = default) { // get the a discovery url. Uri discoveryUrl = GetDiscoveryUrl(endpointUrl); @@ -1156,7 +1154,7 @@ public async Task UpdateFromServerAsync( } else { - client = DiscoveryClient.Create(discoveryUrl, m_configuration, applicationConfiguration); + client = DiscoveryClient.Create(discoveryUrl, m_configuration); } try From beb0dfa0594a32907fa328bf0b053a3f692bd2ec Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Fri, 8 Nov 2024 16:49:55 +0200 Subject: [PATCH 05/11] Match tls client certificate against client application certificate --- .../Stack/Https/HttpsTransportListener.cs | 79 +++++++++++++------ 1 file changed, 57 insertions(+), 22 deletions(-) diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index 50040ae3e..deb6f4605 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -376,38 +376,33 @@ public async Task SendAsync(HttpContext context) string path = context.Request.Path.Value?.TrimEnd('/') ?? string.Empty; string currentPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix; bool isDiscoveryPath = path.Equals(currentPath, StringComparison.OrdinalIgnoreCase); - // Access client certificate + // Access and validate tls client certificate if (!isDiscoveryPath && m_ClientCertificateMode == ClientCertificateMode.RequireCertificate) { - var clientCertificate = context.Connection.ClientCertificate; - - if (clientCertificate == null) + if (!await ValidateClientTlsCertificateAsync(context, ct).ConfigureAwait(false)) { - // No client certificate is provided - message = "Client certificate is required"; - await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false); return; } + } + + IServiceRequest input = (IServiceRequest)BinaryDecoder.DecodeMessage(buffer, null, m_quotas.MessageContext); - // Validate the client certificate - if (clientCertificate != null && m_quotas.CertificateValidator != null) + // Match tls client certificate against client application certificate provided in CreateSessionRequest + if (!isDiscoveryPath && + m_ClientCertificateMode == ClientCertificateMode.RequireCertificate && + input.TypeId == DataTypeIds.CreateSessionRequest) + { + byte[] tlsClientCertificate = context.Connection.ClientCertificate.RawData; + byte[] opcUaClientCertificate = ((CreateSessionRequest)input).ClientCertificate; + if (!Utils.IsEqual(tlsClientCertificate, opcUaClientCertificate)) { - try - { - await m_quotas.CertificateValidator.ValidateAsync(clientCertificate, ct).ConfigureAwait(false); - } - catch (Exception ex) - { - message = "Client certificate validation failed."; - Utils.LogError(ex, message); - await WriteResponseAsync(context.Response, message, HttpStatusCode.Unauthorized).ConfigureAwait(false); - return; - } + 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; } } - IServiceRequest input = (IServiceRequest)BinaryDecoder.DecodeMessage(buffer, null, m_quotas.MessageContext); - // extract the JWT token from the HTTP headers. if (input.RequestHeader == null) { @@ -501,6 +496,8 @@ public async Task SendAsync(HttpContext context) await WriteResponseAsync(context.Response, message, HttpStatusCode.InternalServerError).ConfigureAwait(false); } + + /// /// Called when a UpdateCertificate event occured. /// @@ -593,6 +590,44 @@ private bool ValidateClientCertificate( return true; } + + /// + /// Validates the client TLS certificate + /// + /// Context of the validation + /// Continuation token + /// true if validation passes, elst false + private async Task 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 From c326d429434c9e43194e3d14150375e35df5249e Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Mon, 11 Nov 2024 17:23:22 +0200 Subject: [PATCH 06/11] A few renamings --- .../Stack/Https/HttpsTransportListener.cs | 9 +++++---- Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs | 2 +- .../Stack/Transport/TransportListenerSettings.cs | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index deb6f4605..558af8061 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -207,7 +207,7 @@ public void Open( m_serverCertificate = settings.ServerCertificate; m_serverCertificateChain = settings.ServerCertificateChain; - m_ClientCertificateMode = settings.HttpMutualTls ? ClientCertificateMode.RequireCertificate : ClientCertificateMode.NoCertificate; + m_ClientCertificateMode = settings.HttpsMutualTls ? ClientCertificateMode.RequireCertificate : ClientCertificateMode.NoCertificate; // start the listener Start(); @@ -376,8 +376,10 @@ public async Task SendAsync(HttpContext context) string path = context.Request.Path.Value?.TrimEnd('/') ?? string.Empty; string currentPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix; bool isDiscoveryPath = path.Equals(currentPath, StringComparison.OrdinalIgnoreCase); + bool validateClientTlsCertificate = !isDiscoveryPath && m_ClientCertificateMode == ClientCertificateMode.RequireCertificate; + // Access and validate tls client certificate - if (!isDiscoveryPath && m_ClientCertificateMode == ClientCertificateMode.RequireCertificate) + if (validateClientTlsCertificate) { if (!await ValidateClientTlsCertificateAsync(context, ct).ConfigureAwait(false)) { @@ -388,8 +390,7 @@ public async Task SendAsync(HttpContext context) IServiceRequest input = (IServiceRequest)BinaryDecoder.DecodeMessage(buffer, null, m_quotas.MessageContext); // Match tls client certificate against client application certificate provided in CreateSessionRequest - if (!isDiscoveryPath && - m_ClientCertificateMode == ClientCertificateMode.RequireCertificate && + if (validateClientTlsCertificate && input.TypeId == DataTypeIds.CreateSessionRequest) { byte[] tlsClientCertificate = context.Connection.ClientCertificate.RawData; diff --git a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs index fcbc23d1a..dd1725641 100644 --- a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs +++ b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs @@ -886,7 +886,7 @@ ICertificateValidator certificateValidator settings.MaxChannelCount = applicationConfiguration.ServerConfiguration.MaxChannelCount; if (Utils.IsUriHttpsScheme(endpointUri.AbsoluteUri)) { - settings.HttpMutualTls = applicationConfiguration.ServerConfiguration.HttpsMutualTls; + settings.HttpsMutualTls = applicationConfiguration.ServerConfiguration.HttpsMutualTls; } } diff --git a/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs b/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs index 9c15c1fa5..c4b79666e 100644 --- a/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs +++ b/Stack/Opc.Ua.Core/Stack/Transport/TransportListenerSettings.cs @@ -133,7 +133,7 @@ public int MaxChannelCount /// 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. /// - public bool HttpMutualTls + public bool HttpsMutualTls { get { return m_httpMutualTls; } set { m_httpMutualTls = value; } From 994ee729ea8e54077ce316be2bf6deb200ef13a2 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Fri, 15 Nov 2024 14:51:42 +0200 Subject: [PATCH 07/11] Mutual TLS validation is on by default --- Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs index f2f0036f8..11c81733f 100644 --- a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs +++ b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs @@ -1509,6 +1509,7 @@ private void Initialize() m_maxTrustListSize = 0; m_multicastDnsEnabled = false; m_auditingEnabled = false; + m_httpsMTls = true; } /// From c19537081202ea2a4f12651820be6860b558552e Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Mon, 18 Nov 2024 10:30:31 +0200 Subject: [PATCH 08/11] Minor fixes/comments --- .../Stack/Https/HttpsTransportListener.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index 558af8061..d16753087 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -207,8 +207,7 @@ public void Open( m_serverCertificate = settings.ServerCertificate; m_serverCertificateChain = settings.ServerCertificateChain; - m_ClientCertificateMode = settings.HttpsMutualTls ? ClientCertificateMode.RequireCertificate : ClientCertificateMode.NoCertificate; - + m_ClientCertificateMode = settings.HttpsMutualTls; // start the listener Start(); } @@ -287,7 +286,7 @@ public void Start() var httpsOptions = new HttpsConnectionAdapterOptions() { CheckCertificateRevocation = false, - ClientCertificateMode = ClientCertificateMode.AllowCertificate, + ClientCertificateMode = m_ClientCertificateMode == true ? ClientCertificateMode.AllowCertificate : ClientCertificateMode.NoCertificate, // note: this is the TLS certificate! ServerCertificate = serverCertificate, ClientCertificateValidation = ValidateClientCertificate, @@ -374,9 +373,9 @@ public async Task SendAsync(HttpContext context) } string path = context.Request.Path.Value?.TrimEnd('/') ?? string.Empty; - string currentPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix; - bool isDiscoveryPath = path.Equals(currentPath, StringComparison.OrdinalIgnoreCase); - bool validateClientTlsCertificate = !isDiscoveryPath && m_ClientCertificateMode == ClientCertificateMode.RequireCertificate; + string discoveryPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix; + bool isDiscoveryPath = path.Equals(discoveryPath, StringComparison.OrdinalIgnoreCase); + bool validateClientTlsCertificate = !isDiscoveryPath && m_ClientCertificateMode == true; // Access and validate tls client certificate if (validateClientTlsCertificate) @@ -562,7 +561,7 @@ private static async Task ReadBodyAsync(HttpRequest req) } /// - /// Validate TLS Client certificate + /// Validate TLS client certificate at TLS handshake /// /// /// @@ -593,7 +592,7 @@ private bool ValidateClientCertificate( } /// - /// Validates the client TLS certificate + /// Validates the client TLS certificate per request /// /// Context of the validation /// Continuation token @@ -642,7 +641,7 @@ private async Task ValidateClientTlsCertificateAsync(HttpContext context, private IWebHost m_host; private X509Certificate2 m_serverCertificate; private X509Certificate2Collection m_serverCertificateChain; - private ClientCertificateMode m_ClientCertificateMode; + private bool m_ClientCertificateMode; #endregion } } From 3615b1b7e41920573fe489ec1ef55a529b44f4f3 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Mon, 18 Nov 2024 13:51:16 +0200 Subject: [PATCH 09/11] Fix unit tests --- Libraries/Opc.Ua.Client/CoreClientUtils.cs | 3 +-- .../Stack/Https/HttpsTransportListener.cs | 3 ++- Tests/Opc.Ua.Client.Tests/ClientFixture.cs | 11 ++++++++++- Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs | 11 ++++++++++- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Libraries/Opc.Ua.Client/CoreClientUtils.cs b/Libraries/Opc.Ua.Client/CoreClientUtils.cs index d0b021148..bfce8efd1 100644 --- a/Libraries/Opc.Ua.Client/CoreClientUtils.cs +++ b/Libraries/Opc.Ua.Client/CoreClientUtils.cs @@ -308,8 +308,7 @@ public static Uri GetDiscoveryUrl(string discoveryUrl) { // needs to add the '/discovery' back onto non-UA TCP URLs. if (discoveryUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || - discoveryUrl.StartsWith(Utils.UriSchemeHttps, StringComparison.Ordinal) || - discoveryUrl.StartsWith(Utils.UriSchemeOpcHttps, StringComparison.Ordinal)) + Utils.IsUriHttpsScheme(discoveryUrl)) { if (!discoveryUrl.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)) { diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index d16753087..b0a282d4f 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -374,7 +374,8 @@ public async Task SendAsync(HttpContext context) string path = context.Request.Path.Value?.TrimEnd('/') ?? string.Empty; string discoveryPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix; - bool isDiscoveryPath = path.Equals(discoveryPath, StringComparison.OrdinalIgnoreCase); + + bool isDiscoveryPath = discoveryPath.EndsWith(path, StringComparison.OrdinalIgnoreCase); bool validateClientTlsCertificate = !isDiscoveryPath && m_ClientCertificateMode == true; // Access and validate tls client certificate diff --git a/Tests/Opc.Ua.Client.Tests/ClientFixture.cs b/Tests/Opc.Ua.Client.Tests/ClientFixture.cs index f0e6337b8..0d7b963f0 100644 --- a/Tests/Opc.Ua.Client.Tests/ClientFixture.cs +++ b/Tests/Opc.Ua.Client.Tests/ClientFixture.cs @@ -30,6 +30,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Security.Policy; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using NUnit.Framework; @@ -208,7 +209,15 @@ public async Task Connect(string endpointUrl) /// public async Task ConnectAsync(Uri url, string securityProfile, EndpointDescriptionCollection endpoints = null, IUserIdentity userIdentity = null) { - return await ConnectAsync(await GetEndpointAsync(url, securityProfile, endpoints).ConfigureAwait(false), userIdentity).ConfigureAwait(false); + string uri = url.AbsoluteUri; + Uri getEndpointsUrl = url; + if (uri.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + Utils.IsUriHttpsScheme(uri)) + { + getEndpointsUrl = CoreClientUtils.GetDiscoveryUrl(uri); + } + + return await ConnectAsync(await GetEndpointAsync(getEndpointsUrl, securityProfile, endpoints).ConfigureAwait(false), userIdentity).ConfigureAwait(false); } /// diff --git a/Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs b/Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs index a93c6e512..207f739b4 100644 --- a/Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs +++ b/Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs @@ -158,7 +158,16 @@ public async Task OneTimeSetUpAsync(TextWriter writer = null, } else { - ServerUrl = new Uri(UriScheme + "://localhost:" + ServerFixturePort.ToString(CultureInfo.InvariantCulture)); + string url = UriScheme + "://localhost:" + ServerFixturePort.ToString(CultureInfo.InvariantCulture); + + if (UriScheme.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + Utils.IsUriHttpsScheme(UriScheme)) + { + url = url + ConfiguredEndpoint.DiscoverySuffix; + } + + ServerUrl = new Uri(url); + } if (SingleSession) From 4582a0e293991e21bddf56a8af77ba85b5760916 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Mon, 18 Nov 2024 13:52:52 +0200 Subject: [PATCH 10/11] Addeed missing UriSchemeHttps and UriSchemeOpcHttps hadnling --- .../Stack/Configuration/ConfiguredEndpoints.cs | 12 ++++++++---- .../Stack/Configuration/EndpointDescription.cs | 3 ++- Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs index 74b663e6c..b7c161bdc 100644 --- a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs +++ b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs @@ -131,7 +131,8 @@ public static ConfiguredEndpointCollection Load(string filePath) { string discoveryUrl = endpoint.Description.EndpointUrl; - if (discoveryUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal)) + if (discoveryUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + Utils.IsUriHttpsScheme(discoveryUrl)) { discoveryUrl += ConfiguredEndpoint.DiscoverySuffix; } @@ -531,7 +532,8 @@ public void SetApplicationDescription(string serverUri, ApplicationDescription s } if (endpointUrl != null && - endpointUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) && + (endpointUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + Utils.IsUriHttpsScheme(endpointUrl)) && endpointUrl.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)) { endpointUrl = endpointUrl.Substring(0, endpointUrl.Length - ConfiguredEndpoint.DiscoverySuffix.Length); @@ -816,7 +818,8 @@ public ConfiguredEndpoint( if (baseUrl != null) { - if (baseUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) && + if ((baseUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + Utils.IsUriHttpsScheme(baseUrl)) && baseUrl.EndsWith(DiscoverySuffix, StringComparison.Ordinal)) { baseUrl = baseUrl.Substring(0, baseUrl.Length - DiscoverySuffix.Length); @@ -1209,7 +1212,8 @@ 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 (endpointUrl.Scheme.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal)|| + Utils.IsUriHttpsScheme(endpointUrl.Scheme)) { return new Uri(Utils.Format("{0}{1}", endpointUrl, DiscoverySuffix)); } diff --git a/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs b/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs index 8de37f269..570ad4d3d 100644 --- a/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs +++ b/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs @@ -32,7 +32,8 @@ public EndpointDescription(string url) UriBuilder parsedUrl = new UriBuilder(url); - if (parsedUrl.Scheme.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal)) + if (parsedUrl.Scheme.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + Utils.IsUriHttpsScheme(parsedUrl.Scheme)) { if (!parsedUrl.Path.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)) { diff --git a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs index dd1725641..09972c487 100644 --- a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs +++ b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs @@ -1144,7 +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.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || + Utils.IsUriHttpsScheme(url)) && (!(url.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)))) { url += ConfiguredEndpoint.DiscoverySuffix; From 5fdbe71a74ed09384c62b499dd65475506d44190 Mon Sep 17 00:00:00 2001 From: mrsuciu Date: Tue, 19 Nov 2024 16:39:51 +0200 Subject: [PATCH 11/11] Fixed nits --- Libraries/Opc.Ua.Client/CoreClientUtils.cs | 3 +-- .../ApplicationConfigurationBuilder.cs | 4 ++-- .../Stack/Https/HttpsTransportListener.cs | 22 ++++++++++--------- .../Schema/ApplicationConfiguration.cs | 8 +++---- .../Configuration/ConfiguredEndpoints.cs | 12 ++++------ .../Configuration/EndpointDescription.cs | 3 +-- Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs | 5 ++--- Stack/Opc.Ua.Core/Types/Utils/Utils.cs | 10 +++++++++ 8 files changed, 36 insertions(+), 31 deletions(-) diff --git a/Libraries/Opc.Ua.Client/CoreClientUtils.cs b/Libraries/Opc.Ua.Client/CoreClientUtils.cs index bfce8efd1..958caa85c 100644 --- a/Libraries/Opc.Ua.Client/CoreClientUtils.cs +++ b/Libraries/Opc.Ua.Client/CoreClientUtils.cs @@ -307,8 +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) || - Utils.IsUriHttpsScheme(discoveryUrl)) + if (Utils.IsUriHttpRelatedScheme(discoveryUrl)) { if (!discoveryUrl.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)) { diff --git a/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs b/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs index 9e7844497..015d886d3 100644 --- a/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs +++ b/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs @@ -780,9 +780,9 @@ public IApplicationConfigurationBuilderServerOptions SetAuditingEnabled(bool aud } /// - public IApplicationConfigurationBuilderServerOptions SetHttpsMutualTls(bool mTlsEnabled) + public IApplicationConfigurationBuilderServerOptions SetHttpsMutualTls(bool mutualTlsEnabeld) { - ApplicationConfiguration.ServerConfiguration.HttpsMutualTls = mTlsEnabled; + ApplicationConfiguration.ServerConfiguration.HttpsMutualTls = mutualTlsEnabeld; return this; } diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index b0a282d4f..1d53d196a 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -207,7 +207,7 @@ public void Open( m_serverCertificate = settings.ServerCertificate; m_serverCertificateChain = settings.ServerCertificateChain; - m_ClientCertificateMode = settings.HttpsMutualTls; + m_mutualTlsEnabeld = settings.HttpsMutualTls; // start the listener Start(); } @@ -286,7 +286,7 @@ public void Start() var httpsOptions = new HttpsConnectionAdapterOptions() { CheckCertificateRevocation = false, - ClientCertificateMode = m_ClientCertificateMode == true ? ClientCertificateMode.AllowCertificate : ClientCertificateMode.NoCertificate, + ClientCertificateMode = m_mutualTlsEnabeld == true ? ClientCertificateMode.AllowCertificate : ClientCertificateMode.NoCertificate, // note: this is the TLS certificate! ServerCertificate = serverCertificate, ClientCertificateValidation = ValidateClientCertificate, @@ -374,9 +374,11 @@ public async Task SendAsync(HttpContext context) string path = context.Request.Path.Value?.TrimEnd('/') ?? string.Empty; string discoveryPath = m_uri.AbsolutePath?.TrimEnd('/') + ConfiguredEndpoint.DiscoverySuffix; + ReadOnlyMemory pathMemory = path.AsMemory(); + ReadOnlyMemory discoveryPathMemory = discoveryPath.AsMemory(); - bool isDiscoveryPath = discoveryPath.EndsWith(path, StringComparison.OrdinalIgnoreCase); - bool validateClientTlsCertificate = !isDiscoveryPath && m_ClientCertificateMode == true; + bool isDiscoveryPath = discoveryPathMemory.Span.EndsWith(pathMemory.Span, StringComparison.OrdinalIgnoreCase); + bool validateClientTlsCertificate = !isDiscoveryPath && m_mutualTlsEnabeld == true; // Access and validate tls client certificate if (validateClientTlsCertificate) @@ -564,9 +566,9 @@ private static async Task ReadBodyAsync(HttpRequest req) /// /// Validate TLS client certificate at TLS handshake /// - /// - /// - /// + /// Client certificate + /// Certificate chain + /// SSl policy errors /// private bool ValidateClientCertificate( X509Certificate2 clientCertificate, @@ -596,8 +598,8 @@ private bool ValidateClientCertificate( /// Validates the client TLS certificate per request /// /// Context of the validation - /// Continuation token - /// true if validation passes, elst false + /// Cancellation token + /// true if validation passes, else false private async Task ValidateClientTlsCertificateAsync(HttpContext context, CancellationToken ct) { var clientCertificate = context.Connection.ClientCertificate; @@ -642,7 +644,7 @@ private async Task ValidateClientTlsCertificateAsync(HttpContext context, private IWebHost m_host; private X509Certificate2 m_serverCertificate; private X509Certificate2Collection m_serverCertificateChain; - private bool m_ClientCertificateMode; + private bool m_mutualTlsEnabeld; #endregion } } diff --git a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs index 11c81733f..bbf1a6af8 100644 --- a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs +++ b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs @@ -1509,7 +1509,7 @@ private void Initialize() m_maxTrustListSize = 0; m_multicastDnsEnabled = false; m_auditingEnabled = false; - m_httpsMTls = true; + m_httpsMutualTls = true; } /// @@ -1951,8 +1951,8 @@ public bool AuditingEnabled [DataMember(IsRequired = false, Order = 38)] public bool HttpsMutualTls { - get { return m_httpsMTls; } - set { m_httpsMTls = value; } + get { return m_httpsMutualTls; } + set { m_httpsMutualTls = value; } } #endregion @@ -1992,7 +1992,7 @@ public bool HttpsMutualTls private ReverseConnectServerConfiguration m_reverseConnect; private OperationLimits m_operationLimits; private bool m_auditingEnabled; - private bool m_httpsMTls; + private bool m_httpsMutualTls; #endregion } #endregion diff --git a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs index b7c161bdc..941028966 100644 --- a/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs +++ b/Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs @@ -131,8 +131,7 @@ public static ConfiguredEndpointCollection Load(string filePath) { string discoveryUrl = endpoint.Description.EndpointUrl; - if (discoveryUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || - Utils.IsUriHttpsScheme(discoveryUrl)) + if (Utils.IsUriHttpRelatedScheme(discoveryUrl)) { discoveryUrl += ConfiguredEndpoint.DiscoverySuffix; } @@ -532,8 +531,7 @@ public void SetApplicationDescription(string serverUri, ApplicationDescription s } if (endpointUrl != null && - (endpointUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || - Utils.IsUriHttpsScheme(endpointUrl)) && + Utils.IsUriHttpRelatedScheme(endpointUrl) && endpointUrl.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)) { endpointUrl = endpointUrl.Substring(0, endpointUrl.Length - ConfiguredEndpoint.DiscoverySuffix.Length); @@ -818,8 +816,7 @@ public ConfiguredEndpoint( if (baseUrl != null) { - if ((baseUrl.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || - Utils.IsUriHttpsScheme(baseUrl)) && + if (Utils.IsUriHttpRelatedScheme(baseUrl) && baseUrl.EndsWith(DiscoverySuffix, StringComparison.Ordinal)) { baseUrl = baseUrl.Substring(0, baseUrl.Length - DiscoverySuffix.Length); @@ -1212,8 +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)|| - Utils.IsUriHttpsScheme(endpointUrl.Scheme)) + if (Utils.IsUriHttpRelatedScheme(endpointUrl.Scheme)) { return new Uri(Utils.Format("{0}{1}", endpointUrl, DiscoverySuffix)); } diff --git a/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs b/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs index 570ad4d3d..20c2d319d 100644 --- a/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs +++ b/Stack/Opc.Ua.Core/Stack/Configuration/EndpointDescription.cs @@ -32,8 +32,7 @@ public EndpointDescription(string url) UriBuilder parsedUrl = new UriBuilder(url); - if (parsedUrl.Scheme.StartsWith(Utils.UriSchemeHttp, StringComparison.Ordinal) || - Utils.IsUriHttpsScheme(parsedUrl.Scheme)) + if (Utils.IsUriHttpRelatedScheme(parsedUrl.Scheme)) { if (!parsedUrl.Path.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)) { diff --git a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs index 09972c487..3b9b0f8e7 100644 --- a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs +++ b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs @@ -1144,9 +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) || - Utils.IsUriHttpsScheme(url)) && - (!(url.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase)))) + Utils.IsUriHttpRelatedScheme(url) && + (!url.EndsWith(ConfiguredEndpoint.DiscoverySuffix, StringComparison.OrdinalIgnoreCase))) { url += ConfiguredEndpoint.DiscoverySuffix; } diff --git a/Stack/Opc.Ua.Core/Types/Utils/Utils.cs b/Stack/Opc.Ua.Core/Types/Utils/Utils.cs index e6236c51e..75c1b2b05 100644 --- a/Stack/Opc.Ua.Core/Types/Utils/Utils.cs +++ b/Stack/Opc.Ua.Core/Types/Utils/Utils.cs @@ -162,6 +162,16 @@ public static bool IsUriHttpsScheme(string url) return url.StartsWith(Utils.UriSchemeHttps, StringComparison.Ordinal) || url.StartsWith(Utils.UriSchemeOpcHttps, StringComparison.Ordinal); } + + /// + /// Returns true if the url starts with http, opc.https or https. + /// + /// The url + public static bool IsUriHttpRelatedScheme(string url) + { + return url.StartsWith(Utils.UriSchemeHttps, StringComparison.Ordinal) || + IsUriHttpsScheme(url); + } #endregion #region Trace Support