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

Support ECC profiles #1999

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -261,3 +261,4 @@ OPC\ Foundation/
/Microsoft.CodeDom.Providers.DotNetCompilerPlatform.xml
/SampleApplications/Samples/OPCOutput
!**/Assets/*.*
/Tests/Opc.Ua.Client.Tests/SubscriptionTest.xml
mregen marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions Applications/ClientControls.Net4/UA Client Controls.csproj
Expand Up @@ -1037,7 +1037,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions">
<Version>3.1.28</Version>
<Version>6.0.2</Version>
</PackageReference>
</ItemGroup>
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
Expand All @@ -1054,4 +1054,4 @@
<PreBuildEvent>
</PreBuildEvent>
</PropertyGroup>
</Project>
</Project>
Expand Up @@ -17,8 +17,10 @@
<StorePath>%LocalApplicationData%/OPC Foundation/pki/own</StorePath>
<SubjectName>CN=Console Reference Client, C=US, S=Arizona, O=OPC Foundation, DC=localhost</SubjectName>
</ApplicationCertificate>

<ApplicationCertificateTypes>Rsa,NistP256,NistP384,BrainpoolP256r1,BrainpoolP384r1</ApplicationCertificateTypes>

Copy link
Contributor

Choose a reason for hiding this comment

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

Discuss how the new config layout should be, @mregen provide sample from previous discussion in .NET user group.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feature request was to have a ApplicationCertificate per ECC security profile, to be able to set different subjects

<!-- Where the issuer certificate are stored (certificate authorities) -->
<!-- Where the issuer certificate are stored (certificate authorities) -->
<TrustedIssuerCertificates>
<StoreType>Directory</StoreType>
<StorePath>%LocalApplicationData%/OPC Foundation/pki/issuer</StorePath>
Expand Down
Expand Up @@ -18,6 +18,11 @@
<SubjectName>CN=Quickstart Reference Server, C=US, S=Arizona, O=OPC Foundation, DC=localhost</SubjectName>
</ApplicationCertificate>

<!-- Which certificate types are supported -->
<ApplicationCertificateTypes>Rsa,NistP256,NistP384,BrainpoolP256r1,BrainpoolP384r1</ApplicationCertificateTypes>

<!-- Where the other application certificates are stored -->

<!-- Where the issuer certificate are stored (certificate authorities) -->
<TrustedIssuerCertificates>
<StoreType>Directory</StoreType>
Expand Down Expand Up @@ -96,14 +101,11 @@
</AlternateBaseAddresses>
-->
<SecurityPolicies>
<!-- the first policy is used for the https endpoint -->
<ServerSecurityPolicy>
<SecurityMode>SignAndEncrypt_3</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#Basic256Sha256</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>None_1</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#None</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>Sign_2</SecurityMode>
<SecurityPolicyUri></SecurityPolicyUri>
Expand All @@ -112,7 +114,43 @@
<SecurityMode>SignAndEncrypt_3</SecurityMode>
<SecurityPolicyUri></SecurityPolicyUri>
Copy link
Contributor

Choose a reason for hiding this comment

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

expectation were that here are add the ECC profiles if the ECC cert is configured as app cert.

</ServerSecurityPolicy>
<!-- deprecated security policies for reference only
<ServerSecurityPolicy>
<SecurityMode>Sign_2</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_nistP256</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>Sign_2</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_nistP384</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>Sign_2</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_brainpoolP256r1</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>Sign_2</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_brainpoolP384r1</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>SignAndEncrypt_3</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_nistP256</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>SignAndEncrypt_3</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_nistP384</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>SignAndEncrypt_3</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_brainpoolP256r1</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>SignAndEncrypt_3</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#ECC_brainpoolP384r1</SecurityPolicyUri>
</ServerSecurityPolicy>
<ServerSecurityPolicy>
<SecurityMode>None_1</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#None</SecurityPolicyUri>
</ServerSecurityPolicy>
<!-- deprecated security policies for reference only -->
<ServerSecurityPolicy>
<SecurityMode>Sign_2</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#Basic256</SecurityPolicyUri>
Expand All @@ -129,7 +167,7 @@
<SecurityMode>SignAndEncrypt_3</SecurityMode>
<SecurityPolicyUri>http://opcfoundation.org/UA/SecurityPolicy#Basic128Rsa15</SecurityPolicyUri>
</ServerSecurityPolicy>
-->
<!-- -->
</SecurityPolicies>

<MinRequestThreadCount>5</MinRequestThreadCount>
Expand Down
4 changes: 2 additions & 2 deletions Applications/ServerControls.Net4/UA Server Controls.csproj
Expand Up @@ -153,7 +153,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions">
<Version>3.1.28</Version>
<Version>6.0.2</Version>
</PackageReference>
</ItemGroup>
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
Expand All @@ -170,4 +170,4 @@
<PostBuildEvent>
</PostBuildEvent>
</PropertyGroup>
</Project>
</Project>
42 changes: 37 additions & 5 deletions Libraries/Opc.Ua.Client/CoreClientUtils.cs
Expand Up @@ -113,6 +113,7 @@ int discoverTimeout
/// <param name="discoveryUrl">The discovery URL.</param>
/// <param name="useSecurity">if set to <c>true</c> select an endpoint that uses security.</param>
/// <returns>The best available endpoint.</returns>
[Obsolete("Use the SelectEndpoint with ApplicationConfiguration instead.")]
public static EndpointDescription SelectEndpoint(string discoveryUrl, bool useSecurity)
{
return SelectEndpoint(discoveryUrl, useSecurity, DefaultDiscoverTimeout);
Expand All @@ -125,6 +126,7 @@ public static EndpointDescription SelectEndpoint(string discoveryUrl, bool useSe
/// <param name="useSecurity">if set to <c>true</c> select an endpoint that uses security.</param>
/// <param name="discoverTimeout">Operation timeout in milliseconds.</param>
/// <returns>The best available endpoint.</returns>
[Obsolete("Use the SelectEndpoint with ApplicationConfiguration instead.")]
Copy link
Contributor

Choose a reason for hiding this comment

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

check why is it obsolete, was the function moved to ApplicationConfiguration?

public static EndpointDescription SelectEndpoint(
string discoveryUrl,
bool useSecurity,
Expand Down Expand Up @@ -172,7 +174,7 @@ int discoverTimeout
{
var url = new Uri(client.Endpoint.EndpointUrl);
var endpoints = client.GetEndpoints(null);
return SelectEndpoint(url, endpoints, useSecurity);
return SelectEndpoint(application, url, endpoints, useSecurity);
}
}

Expand Down Expand Up @@ -215,7 +217,7 @@ int discoverTimeout
// Connect to the server's discovery endpoint and find the available configuration.
Uri url = new Uri(client.Endpoint.EndpointUrl);
var endpoints = client.GetEndpoints(null);
var selectedEndpoint = SelectEndpoint(url, endpoints, useSecurity);
var selectedEndpoint = SelectEndpoint(application, url, endpoints, useSecurity);

Uri endpointUrl = Utils.ParseUri(selectedEndpoint.EndpointUrl);
if (endpointUrl != null && endpointUrl.Scheme == uri.Scheme)
Expand All @@ -234,10 +236,28 @@ int discoverTimeout
/// Select the best supported endpoint from an
/// EndpointDescriptionCollection, with or without security.
/// </summary>
/// <param name="url">The discovery Url of the server.</param>
/// <param name="endpoints"></param>
/// <param name="useSecurity"></param>
[Obsolete("Use the SelectEndpoint with ApplicationConfiguration instead.")]
public static EndpointDescription SelectEndpoint(
Uri url,
EndpointDescriptionCollection endpoints,
bool useSecurity)
{
return SelectEndpoint(null, url, endpoints, useSecurity);
}

/// <summary>
/// Select the best supported endpoint from an
/// EndpointDescriptionCollection, with or without security.
/// </summary>
/// <param name="configuration"></param>
/// <param name="url"></param>
/// <param name="endpoints"></param>
/// <param name="useSecurity"></param>
public static EndpointDescription SelectEndpoint(
ApplicationConfiguration configuration,
Uri url,
EndpointDescriptionCollection endpoints,
bool useSecurity)
Expand All @@ -260,10 +280,22 @@ int discoverTimeout
continue;
}

// skip unsupported security policies
if (SecurityPolicies.GetDisplayName(endpoint.SecurityPolicyUri) == null)
if (configuration != null)
{
continue;
// skip unsupported security policies
if (!configuration.SecurityConfiguration.SupportedSecurityPolicies.Contains(endpoint.SecurityPolicyUri))
Copy link
Contributor

Choose a reason for hiding this comment

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

check if this is a good idea

{
continue;
}
}
else
{
// skip unsupported security policies, for backward compatibility only
// may contain policies for which no certificate is available
if (SecurityPolicies.GetDisplayName(endpoint.SecurityPolicyUri) == null)
{
continue;
}
}
}
else
Expand Down
47 changes: 20 additions & 27 deletions Libraries/Opc.Ua.Client/Session.cs
Expand Up @@ -164,26 +164,28 @@ public Session(ITransportChannel channel, Session template, bool copyEventHandle

if (m_endpoint.Description.SecurityPolicyUri != SecurityPolicies.None)
{
// update client certificate.
m_instanceCertificate = clientCertificate;

if (clientCertificate == null)
{
// load the application instance certificate.
if (m_configuration.SecurityConfiguration.ApplicationCertificate == null)
m_instanceCertificate = LoadCertificate(configuration, m_endpoint.Description.SecurityPolicyUri).GetAwaiter().GetResult();
if (m_instanceCertificate == null)
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 if the policy none is used, but user token should be encrypted with ECC profile ..

{
throw new ServiceResultException(
StatusCodes.BadConfigurationError,
"The client configuration does not specify an application instance certificate.");
}

m_instanceCertificate = m_configuration.SecurityConfiguration.ApplicationCertificate.Find(true).Result;
}
else
{
// update client certificate.
m_instanceCertificate = clientCertificate;
}

// check for valid certificate.
if (m_instanceCertificate == null)
{
#pragma warning disable CS0618 // Type or member is obsolete
var cert = m_configuration.SecurityConfiguration.ApplicationCertificate;
#pragma warning restore CS0618 // Type or member is obsolete
Copy link
Contributor

Choose a reason for hiding this comment

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

marked as obsolete, to catch cases that need to be handled.

Copy link
Contributor

Choose a reason for hiding this comment

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

display config info in the exception.

throw ServiceResultException.Create(
StatusCodes.BadConfigurationError,
"Cannot find the application instance certificate. Store={0}, SubjectName={1}, Thumbprint={2}.",
Expand All @@ -196,19 +198,12 @@ public Session(ITransportChannel channel, Session template, bool copyEventHandle
throw ServiceResultException.Create(
StatusCodes.BadConfigurationError,
"No private key for the application instance certificate. Subject={0}, Thumbprint={1}.",
m_instanceCertificate.Subject,
m_instanceCertificate.Subject,
m_instanceCertificate.Thumbprint);
}

// load certificate chain.
m_instanceCertificateChain = new X509Certificate2Collection(m_instanceCertificate);
List<CertificateIdentifier> issuers = new List<CertificateIdentifier>();
configuration.CertificateValidator.GetIssuers(m_instanceCertificate, issuers).Wait();

for (int i = 0; i < issuers.Count; i++)
{
m_instanceCertificateChain.Add(issuers[i].Certificate);
}
m_instanceCertificateChain = LoadCertificateChain(configuration, m_instanceCertificate).GetAwaiter().GetResult();
}

// initialize the message context.
Expand Down Expand Up @@ -893,7 +888,7 @@ public int GoodPublishRequestCount
X509Certificate2Collection clientCertificateChain = null;
if (endpointDescription.SecurityPolicyUri != SecurityPolicies.None)
{
clientCertificate = await LoadCertificate(configuration).ConfigureAwait(false);
clientCertificate = await LoadCertificate(configuration, endpointDescription.SecurityPolicyUri).ConfigureAwait(false);
clientCertificateChain = await LoadCertificateChain(configuration, clientCertificate).ConfigureAwait(false);
}

Expand Down Expand Up @@ -5607,20 +5602,18 @@ private void DeleteSubscription(uint subscriptionId)
/// <summary>
/// Load certificate for connection.
/// </summary>
private static async Task<X509Certificate2> LoadCertificate(ApplicationConfiguration configuration)
private static async Task<X509Certificate2> LoadCertificate(ApplicationConfiguration configuration, string securityProfile)
{
X509Certificate2 clientCertificate;
if (configuration.SecurityConfiguration.ApplicationCertificate == null)
{
throw ServiceResultException.Create(StatusCodes.BadConfigurationError, "ApplicationCertificate must be specified.");
}

clientCertificate = await configuration.SecurityConfiguration.ApplicationCertificate.Find(true).ConfigureAwait(false);
X509Certificate2 clientCertificate =
await configuration.SecurityConfiguration.FindApplicationCertificateAsync(securityProfile, true).ConfigureAwait(false);

if (clientCertificate == null)
{
throw ServiceResultException.Create(StatusCodes.BadConfigurationError, "ApplicationCertificate cannot be found.");
throw ServiceResultException.Create(StatusCodes.BadConfigurationError,
"ApplicationCertificate for the security profile {0} cannot be found.",
securityProfile);
}

return clientCertificate;
}

Expand All @@ -5635,7 +5628,7 @@ private static async Task<X509Certificate2Collection> LoadCertificateChain(Appli
{
clientCertificateChain = new X509Certificate2Collection(clientCertificate);
List<CertificateIdentifier> issuers = new List<CertificateIdentifier>();
await configuration.CertificateValidator.GetIssuers(clientCertificate, issuers).ConfigureAwait(false);
await configuration.CertificateValidator.GetIssuers(clientCertificate, issuers, false).ConfigureAwait(false);

Copy link
Contributor

Choose a reason for hiding this comment

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

why false?

for (int i = 0; i < issuers.Count; i++)
{
Expand Down