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.

Sign up for GitHub

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

Jump to bottom

Support ECC profiles #1999

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Support ECC profiles #1999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f407a5a
Squash proto ecc (#1998)
mregen Nov 18, 2022
5c96b3e
clean a few warning
mregen Nov 18, 2022
62ee797
merge master
mregen Jan 8, 2023
983cfce
Instantiate HMAC instance depending on provided algorithm.
mrsuciu Feb 13, 2023
198a494
Merge branch 'work_ecc' of https://github.com/OPCFoundation/UA-.NETSt…
mrsuciu Mar 13, 2023
bd50cf8
merge master
mregen Mar 13, 2023
86e08fa
fix build
mregen Mar 13, 2023
9f39161
merge main
mregen May 11, 2023
50774b6
print endpoints
mregen May 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Applications/ClientControls.Net4/UA Client Controls.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions">
<Version>3.1.32</Version>
<Version>6.0.2</Version>
</PackageReference>
</ItemGroup>
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
Expand Down
4 changes: 3 additions & 1 deletion Applications/ConsoleReferenceClient/Quickstarts.ReferenceClient.Config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
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
50 changes: 44 additions & 6 deletions Applications/ConsoleReferenceServer/Quickstarts.ReferenceServer.Config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
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
9 changes: 9 additions & 0 deletions Applications/ConsoleReferenceServer/UAServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,21 @@ public async Task StartAsync()
ExitCode = ExitCode.ErrorRunning;

// print endpoint info
m_output.WriteLine("Endpoints:");
var endpoints = m_application.Server.GetEndpoints().Select(e => e.EndpointUrl).Distinct();
foreach (var endpoint in endpoints)
{
m_output.WriteLine(endpoint);
}

// print security profiles
m_output.WriteLine("Security profiles:");
endpoints = m_application.Server.GetEndpoints().Select(e => e.SecurityPolicyUri).Distinct();
foreach (var endpoint in endpoints)
{
m_output.WriteLine(endpoint);
}

// start the status thread
m_status = Task.Run(StatusThreadAsync);

Expand Down
2 changes: 1 addition & 1 deletion Applications/ServerControls.Net4/UA Server Controls.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions">
<Version>3.1.32</Version>
<Version>6.0.2</Version>
</PackageReference>
</ItemGroup>
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
Expand Down
42 changes: 37 additions & 5 deletions Libraries/Opc.Ua.Client/CoreClientUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@
/// <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 @@
/// <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 @@
{
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 @@
// 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 @@
/// 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 @@
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;

Check warning on line 288 in Libraries/Opc.Ua.Client/CoreClientUtils.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Client/CoreClientUtils.cs#L288 

Added line #L288 was not covered by tests
}
}
else
{
// skip unsupported security policies, for backward compatibility only
// may contain policies for which no certificate is available
if (SecurityPolicies.GetDisplayName(endpoint.SecurityPolicyUri) == null)

Check warning on line 295 in Libraries/Opc.Ua.Client/CoreClientUtils.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Client/CoreClientUtils.cs#L295 

Added line #L295 was not covered by tests
{
continue;

Check warning on line 297 in Libraries/Opc.Ua.Client/CoreClientUtils.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Client/CoreClientUtils.cs#L297 

Added line #L297 was not covered by tests
}
}
}
else
Expand Down
47 changes: 20 additions & 27 deletions Libraries/Opc.Ua.Client/Session.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,28 @@

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 warning on line 180 in Libraries/Opc.Ua.Client/Session.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Client/Session.cs#L180 

Added line #L180 was not covered by tests
}

// 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 @@
throw ServiceResultException.Create(
StatusCodes.BadConfigurationError,
"No private key for the application instance certificate. Subject={0}, Thumbprint={1}.",
m_instanceCertificate.Subject,
m_instanceCertificate.Subject,

Check warning on line 201 in Libraries/Opc.Ua.Client/Session.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Client/Session.cs#L201 

Added line #L201 was not covered by tests
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 @@
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 @@ -5618,20 +5613,18 @@
/// <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);

Check warning on line 5625 in Libraries/Opc.Ua.Client/Session.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Client/Session.cs#L5623-L5625 

Added lines #L5623 - L5625 were not covered by tests
}

return clientCertificate;
}

Expand All @@ -5646,7 +5639,7 @@
{
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
Loading
Loading