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

[Test] Add ECC Certificates to various tests #2961

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+227 −88
Open

[Test] Add ECC Certificates to various tests #2961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
76ee2b7
Generate ECC Certificates in Client Tests
romanett Jan 20, 2025
f11d752
Add more tests
romanett Jan 23, 2025
6f3102a
reenable check of certificate
romanett Jan 23, 2025
cc5b744
Merge branch 'OPCFoundation:master' into EccGdsTests
romanett Jan 27, 2025
0b78d33
fix tests
romanett Jan 28, 2025
fd7f9c3
Dont use random in test
romanett Jan 28, 2025
bd7b13f
fix unsupported certificate types
romanett Jan 30, 2025
de6ef14
fix Hash algorithm for nistP384 curve on linux as the friendly name i…
romanett Feb 1, 2025
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
8 changes: 6 additions & 2 deletions Libraries/Opc.Ua.Gds.Server.Common/CertificateGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@
{
if (!Utils.IsSupportedCertificateType(certificateType))
{
throw new NotImplementedException($"Unsupported certificate type {certificateType}");
Utils.LogError("Certificate type {0} specified for Certificate Group is not supported on this platform", certificateType);
continue;

Check warning on line 86 in Libraries/Opc.Ua.Gds.Server.Common/CertificateGroup.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Gds.Server.Common/CertificateGroup.cs#L85-L86 

Added lines #L85 - L86 were not covered by tests
}

CertificateTypes.Add(certificateType);
Expand Down Expand Up @@ -356,7 +357,10 @@

#if ECC_SUPPORT
certificate = TryGetECCCurve(certificateType, out ECCurve curve) ?
builder.SetIssuer(signingKey).SetECDsaPublicKey(info.SubjectPublicKeyInfo.GetEncoded()).CreateForECDsa() :
builder
.SetIssuer(signingKey)
.SetECDsaPublicKey(info.SubjectPublicKeyInfo.GetEncoded())
.CreateForECDsa() :
builder.SetHashAlgorithm(X509Utils.GetRSAHashAlgorithmName(Configuration.DefaultCertificateHashSize))
.SetIssuer(signingKey)
.SetRSAPublicKey(info.SubjectPublicKeyInfo.GetEncoded())
Expand Down
8 changes: 4 additions & 4 deletions Libraries/Opc.Ua.Security.Certificates/X509Certificate/CertificateBuilderBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,13 @@ public virtual ICertificateBuilderIssuer SetIssuer(X509Certificate2 issuerCertif
/// <param name="curve"></param>
private void SetHashAlgorithmSize(ECCurve curve)
{
if (curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.nistP384.Oid.FriendlyName) == 0 ||
(curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.brainpoolP384r1.Oid.FriendlyName) == 0))
if (curve.Oid.Value.CompareTo(ECCurve.NamedCurves.nistP384.Oid.Value) == 0 ||
(curve.Oid.Value.CompareTo(ECCurve.NamedCurves.brainpoolP384r1.Oid.Value) == 0))
{
SetHashAlgorithm(HashAlgorithmName.SHA384);
}
if (curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.nistP521.Oid.FriendlyName) == 0 ||
(curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.brainpoolP512r1.Oid.FriendlyName) == 0))
if (curve.Oid.Value.CompareTo(ECCurve.NamedCurves.nistP521.Oid.Value) == 0 ||
(curve.Oid.Value.CompareTo(ECCurve.NamedCurves.brainpoolP512r1.Oid.Value) == 0))
{
SetHashAlgorithm(HashAlgorithmName.SHA512);
}
Expand Down
6 changes: 4 additions & 2 deletions Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ private ServiceResult UpdateCertificate(
// identify the existing certificate to be updated
// it should be of the same type and same subject name as the new certificate
CertificateIdentifier existingCertIdentifier = certificateGroup.ApplicationCertificates.FirstOrDefault(cert =>
X509Utils.CompareDistinguishedName(cert.Certificate.Subject, newCert.Subject) &&
X509Utils.CompareDistinguishedName(cert.SubjectName, newCert.Subject) &&
cert.CertificateType == certificateTypeId);

// if no cert was found search by ApplicationUri
Expand Down Expand Up @@ -566,6 +566,8 @@ private ServiceResult UpdateCertificate(
var certOnly = X509CertificateLoader.LoadCertificate(updateCertificate.CertificateWithPrivateKey.RawData);
updateCertificate.CertificateWithPrivateKey.Dispose();
updateCertificate.CertificateWithPrivateKey = certOnly;
//update certificate identifier with new certificate
existingCertIdentifier.Find(m_configuration.ApplicationUri).GetAwaiter().GetResult();
}

ICertificateStore issuerStore = certificateGroup.IssuerStore.OpenStore();
Expand Down Expand Up @@ -800,7 +802,7 @@ private ServiceResult GetCertificates(
}

certificateTypeIds = certificateGroup.CertificateTypes;
certificates = certificateGroup.ApplicationCertificates.Select(s => s.Certificate.RawData).ToArray();
certificates = certificateGroup.ApplicationCertificates.Select(s => s.Certificate?.RawData).ToArray();

return ServiceResult.Good;
}
Expand Down
3 changes: 1 addition & 2 deletions Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,9 @@ public virtual async Task UpdateCertificateAsync(SecurityConfiguration securityC

try
{

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 rational for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The certificate identifier is already updated before calling the Update Method of the certificate validator. So the Check is not deleting the old cert from m_applicationCertificates this can lead to memory leaks or exceptions If find is not called on the certificate identifier before trying to Access RawData

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The m_appliction certificates list is rebuilt in the UpdateAsync method

m_applicationCertificates.Clear();
foreach (var applicationCertificate in securityConfiguration.ApplicationCertificates)
{
m_applicationCertificates.RemoveAll(cert => Utils.IsEqual(cert.RawData, applicationCertificate.RawData));
applicationCertificate.DisposeCertificate();
}

Expand Down
16 changes: 15 additions & 1 deletion Tests/Opc.Ua.Gds.Tests/ClientTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
using System.Threading;
using System.Threading.Tasks;
using NUnit.Framework;
using Opc.Ua.Gds.Server;
using Assert = NUnit.Framework.Legacy.ClassicAssert;


Expand Down Expand Up @@ -108,6 +109,14 @@ protected async Task OneTimeSetUp()
m_invalidRegistrationOk = false;
m_goodNewKeyPairRequestOk = false;
m_gdsRegisteredTestClient = false;

//get supported CertificateTypes from GDS
m_supportedCertificateTypes = m_server.Config.ParseExtension<GlobalDiscoveryServerConfiguration>().CertificateGroups
.Where(cg => cg.Id == "Default")
.SelectMany(cg => cg.CertificateTypes)
.Select(s => typeof(Ua.ObjectTypeIds).GetField(s).GetValue(null) as NodeId)
.Where(n => n != null && Utils.IsSupportedCertificateType(n))
.ToList();
}

/// <summary>
Expand Down Expand Up @@ -652,8 +661,12 @@ public void StartGoodNewKeyPairRequests()
{
AssertIgnoreTestWithoutGoodRegistration();
ConnectGDS(true);
int certificateTypeIndex = 0;
foreach (var application in m_goodApplicationTestSet)
{
application.CertificateTypeId = m_supportedCertificateTypes[certificateTypeIndex];
certificateTypeIndex = (certificateTypeIndex + 1) % m_supportedCertificateTypes.Count;

Assert.Null(application.CertificateRequestId);
NodeId requestId = m_gdsClient.GDSClient.StartNewKeyPairRequest(
application.ApplicationRecord.ApplicationId,
Expand Down Expand Up @@ -875,7 +888,7 @@ out byte[][] issuerCertificates

}


[Test, Order(540)]
public void GetGoodCertificates()
{
Expand Down Expand Up @@ -1531,6 +1544,7 @@ private int GoodServersOnNetworkCount()
private bool m_goodRegistrationOk;
private bool m_invalidRegistrationOk;
private bool m_gdsRegisteredTestClient;
private List<NodeId> m_supportedCertificateTypes;
private bool m_goodNewKeyPairRequestOk;
private string m_storeType;
#endregion
Expand Down
8 changes: 7 additions & 1 deletion Tests/Opc.Ua.Gds.Tests/GlobalDiscoveryTestServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,13 @@ private static async Task<ApplicationConfiguration> Load(ApplicationInstance app
{
new CertificateGroupConfiguration() {
Id = "Default",
CertificateType = "RsaSha256ApplicationCertificateType",
CertificateTypes = new StringCollection() {
"RsaSha256ApplicationCertificateType",
"EccNistP256ApplicationCertificateType",
"EccNistP384ApplicationCertificateType",
"EccBrainpoolP256r1ApplicationCertificateType",
"EccBrainpoolP384r1ApplicationCertificateType"
},
SubjectName = "CN=GDS Test CA, O=OPC Foundation",
BaseStorePath = Path.Combine(gdsRoot, "CA", "default"),
DefaultCertificateHashSize = 256,
Expand Down
26 changes: 26 additions & 0 deletions Tests/Opc.Ua.Gds.Tests/Opc.Ua.GlobalDiscoveryTestServer.Config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>RsaSha256</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>Directory</StoreType>
<StorePath>%LocalApplicationData%/OPC/GDS/own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>NistP256</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>Directory</StoreType>
<StorePath>%LocalApplicationData%/OPC/GDS/own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>NistP384</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>Directory</StoreType>
<StorePath>%LocalApplicationData%/OPC/GDS/own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>BrainpoolP256r1</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>Directory</StoreType>
<StorePath>%LocalApplicationData%/OPC/GDS/own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>BrainpoolP384r1</CertificateTypeString>
</CertificateIdentifier>
</ApplicationCertificates>

<TrustedIssuerCertificates>
Expand Down Expand Up @@ -143,6 +167,8 @@
<ua:String>RsaSha256ApplicationCertificateType</ua:String>
<ua:String>EccNistP256ApplicationCertificateType</ua:String>
<ua:String>EccNistP384ApplicationCertificateType</ua:String>
<ua:String>EccBrainpoolP256r1ApplicationCertificateType</ua:String>
<ua:String>EccBrainpoolP384r1ApplicationCertificateType</ua:String>
</CertificateTypes>
<SubjectName>CN=GDS Test CA, O=OPC Foundation</SubjectName>
<BaseStorePath>%LocalApplicationData%/OPC/GDS/CA/default</BaseStorePath>
Expand Down
26 changes: 26 additions & 0 deletions Tests/Opc.Ua.Gds.Tests/Opc.Ua.GlobalDiscoveryTestServerX509Stores.Config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>RsaSha256</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>X509Store</StoreType>
<StorePath>CurrentUser\UA_Test_GDS_Server_own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>NistP256</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>X509Store</StoreType>
<StorePath>CurrentUser\UA_Test_GDS_Server_own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>NistP384</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>X509Store</StoreType>
<StorePath>CurrentUser\UA_Test_GDS_Server_own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>BrainpoolP256r1</CertificateTypeString>
</CertificateIdentifier>
<CertificateIdentifier>
<StoreType>X509Store</StoreType>
<StorePath>CurrentUser\UA_Test_GDS_Server_own</StorePath>
<SubjectName>CN=Global Discovery Test Server, O=OPC Foundation, DC=localhost</SubjectName>
<CertificateTypeString>BrainpoolP384r1</CertificateTypeString>
</CertificateIdentifier>
</ApplicationCertificates>

<TrustedIssuerCertificates>
Expand Down Expand Up @@ -143,6 +167,8 @@
<ua:String>RsaSha256ApplicationCertificateType</ua:String>
<ua:String>EccNistP256ApplicationCertificateType</ua:String>
<ua:String>EccNistP384ApplicationCertificateType</ua:String>
<ua:String>EccBrainpoolP256r1ApplicationCertificateType</ua:String>
<ua:String>EccBrainpoolP384r1ApplicationCertificateType</ua:String>
</CertificateTypes>
<SubjectName>CN=GDS Test CA, O=OPC Foundation</SubjectName>
<BaseStorePath>%LocalApplicationData%/OPC/GDS/CA/default</BaseStorePath>
Expand Down
Loading
Loading