Skip to content

Commit 7f037ef

Browse files
romanettromanett
authored
Enforce Valid Certificate Store configuration (#3023)
* fix Null Reference Exceptions on nonconfigured certificate stores * try to fix gds tests * Use explicit null check * Validate Certificate Store configuration on startup in SecurityConfiguration.Validate. Remove Obsolete CertificateStore Properties. * remove unecessary fields * fix tests * fix client config * Validate Stores by opening them * fix typo * check stores instead of storepath * fix ClientTest * Add Tests for Validate Mehtod of SecurityConfiguration * Update ApplicationConfiguration xsd to respect stricter validation
1 parent 55ded8a commit 7f037ef

File tree

12 files changed

+744
-513
lines changed

12 files changed

+744
-513
lines changed

‎Applications/ConsoleReferenceClient/Quickstarts.ReferenceClient.Config.xml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,17 @@
8080
<AddAppCertToTrustedStore>false</AddAppCertToTrustedStore>
8181
<SendCertificateChain>true</SendCertificateChain>
8282

83+
<!-- Where the User issers list is stored-->
84+
<UserIssuerCertificates>
85+
<StoreType>Directory</StoreType>
86+
<StorePath>%LocalApplicationData%/OPC Foundation/pki/userIssuer</StorePath>
87+
</UserIssuerCertificates>
88+
8389
<!-- Where the User trust list is stored-->
8490
<TrustedUserCertificates>
8591
<StoreType>Directory</StoreType>
8692
<StorePath>%LocalApplicationData%/OPC Foundation/pki/trustedUser</StorePath>
8793
</TrustedUserCertificates>
88-
8994
</SecurityConfiguration>
9095

9196
<TransportConfigurations></TransportConfigurations>

‎Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs‎

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -88,24 +88,35 @@ ApplicationConfiguration configuration
8888
IssuerStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedIssuerCertificates.StorePath),
8989
TrustedStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedPeerCertificates.StorePath)
9090
};
91+
m_certificateGroups.Add(defaultApplicationGroup);
9192

92-
ServerCertificateGroup defaultUserGroup = new ServerCertificateGroup {
93-
NodeId = Opc.Ua.ObjectIds.ServerConfiguration_CertificateGroups_DefaultUserTokenGroup,
94-
BrowseName = Opc.Ua.BrowseNames.DefaultUserTokenGroup,
95-
CertificateTypes = new NodeId[] { },
96-
ApplicationCertificates = new CertificateIdentifierCollection(),
97-
IssuerStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.UserIssuerCertificates?.StorePath),
98-
TrustedStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedUserCertificates?.StorePath)
99-
};
100-
101-
ServerCertificateGroup defaultHttpsGroup = new ServerCertificateGroup {
102-
NodeId = Opc.Ua.ObjectIds.ServerConfiguration_CertificateGroups_DefaultHttpsGroup,
103-
BrowseName = Opc.Ua.BrowseNames.DefaultHttpsGroup,
104-
CertificateTypes = new NodeId[] { },
105-
ApplicationCertificates = new CertificateIdentifierCollection(),
106-
IssuerStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.HttpsIssuerCertificates?.StorePath),
107-
TrustedStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedHttpsCertificates?.StorePath)
108-
};
93+
if (configuration.SecurityConfiguration.UserIssuerCertificates != null && configuration.SecurityConfiguration.TrustedUserCertificates != null)
94+
{
95+
ServerCertificateGroup defaultUserGroup = new ServerCertificateGroup {
96+
NodeId = Opc.Ua.ObjectIds.ServerConfiguration_CertificateGroups_DefaultUserTokenGroup,
97+
BrowseName = Opc.Ua.BrowseNames.DefaultUserTokenGroup,
98+
CertificateTypes = new NodeId[] { },
99+
ApplicationCertificates = new CertificateIdentifierCollection(),
100+
IssuerStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.UserIssuerCertificates.StorePath),
101+
TrustedStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedUserCertificates.StorePath)
102+
};
103+
104+
m_certificateGroups.Add(defaultUserGroup);
105+
}
106+
ServerCertificateGroup defaultHttpsGroup = null;
107+
if (configuration.SecurityConfiguration.HttpsIssuerCertificates != null && configuration.SecurityConfiguration.TrustedHttpsCertificates != null)
108+
{
109+
defaultHttpsGroup = new ServerCertificateGroup {
110+
NodeId = Opc.Ua.ObjectIds.ServerConfiguration_CertificateGroups_DefaultHttpsGroup,
111+
BrowseName = Opc.Ua.BrowseNames.DefaultHttpsGroup,
112+
CertificateTypes = new NodeId[] { },
113+
ApplicationCertificates = new CertificateIdentifierCollection(),
114+
IssuerStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.HttpsIssuerCertificates.StorePath),
115+
TrustedStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedHttpsCertificates.StorePath)
116+
};
117+
118+
m_certificateGroups.Add(defaultHttpsGroup);
119+
}
109120

110121
// For each certificate in ApplicationCertificates, add the certificate type to ServerConfiguration_CertificateGroups_DefaultApplicationGroup
111122
// under the CertificateTypes field.
@@ -114,16 +125,13 @@ ApplicationConfiguration configuration
114125
defaultApplicationGroup.CertificateTypes = defaultApplicationGroup.CertificateTypes.Concat(new NodeId[] { cert.CertificateType }).ToArray();
115126
defaultApplicationGroup.ApplicationCertificates.Add(cert);
116127

117-
if (cert.CertificateType == ObjectTypeIds.HttpsCertificateType)
128+
if (cert.CertificateType == ObjectTypeIds.HttpsCertificateType && defaultHttpsGroup != null)
118129
{
119130
defaultHttpsGroup.CertificateTypes = defaultHttpsGroup.CertificateTypes.Concat(new NodeId[] { cert.CertificateType }).ToArray();
120131
defaultHttpsGroup.ApplicationCertificates.Add(cert);
121132
}
122133
}
123134

124-
m_certificateGroups.Add(defaultApplicationGroup);
125-
m_certificateGroups.Add(defaultHttpsGroup);
126-
m_certificateGroups.Add(defaultUserGroup);
127135
}
128136
#endregion
129137

@@ -557,6 +565,11 @@ private ServiceResult UpdateCertificate(
557565
{
558566
using (ICertificateStore appStore = existingCertIdentifier.OpenStore())
559567
{
568+
if (appStore == null)
569+
{
570+
throw new ServiceResultException(StatusCodes.BadConfigurationError, "Failed to open application certificate store.");
571+
}
572+
560573
Utils.LogCertificate(Utils.TraceMasks.Security, "Delete application certificate: ", existingCertIdentifier.Certificate);
561574
appStore.Delete(existingCertIdentifier.Thumbprint).Wait();
562575
Utils.LogCertificate(Utils.TraceMasks.Security, "Add new application certificate: ", updateCertificate.CertificateWithPrivateKey);
@@ -573,6 +586,11 @@ private ServiceResult UpdateCertificate(
573586
ICertificateStore issuerStore = certificateGroup.IssuerStore.OpenStore();
574587
try
575588
{
589+
if (issuerStore == null)
590+
{
591+
throw new ServiceResultException(StatusCodes.BadConfigurationError, "Failed to open issuer certificate store.");
592+
}
593+
576594
foreach (var issuer in updateCertificate.IssuerCollection)
577595
{
578596
try
@@ -769,13 +787,16 @@ private ServiceResult GetRejectedList(
769787
ICertificateStore store = m_rejectedStore.OpenStore();
770788
try
771789
{
772-
X509Certificate2Collection collection = store.Enumerate().Result;
773-
List<byte[]> rawList = new List<byte[]>();
774-
foreach (var cert in collection)
790+
if (store != null)
775791
{
776-
rawList.Add(cert.RawData);
792+
X509Certificate2Collection collection = store.Enumerate().Result;
793+
List<byte[]> rawList = new List<byte[]>();
794+
foreach (var cert in collection)
795+
{
796+
rawList.Add(cert.RawData);
797+
}
798+
certificates = rawList.ToArray();
777799
}
778-
certificates = rawList.ToArray();
779800
}
780801
finally
781802
{

‎Libraries/Opc.Ua.Server/Configuration/TrustList.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,11 @@ private ServiceResult Open(
144144
ICertificateStore store = m_trustedStore.OpenStore();
145145
try
146146
{
147+
if (store == null)
148+
{
149+
throw new ServiceResultException(StatusCodes.BadConfigurationError, "Failed to open trusted certificate store.");
150+
}
151+
147152
if ((masks & TrustListMasks.TrustedCertificates) != 0)
148153
{
149154
X509Certificate2Collection certificates = store.Enumerate().GetAwaiter().GetResult();
@@ -160,6 +165,7 @@ private ServiceResult Open(
160165
trustList.TrustedCrls.Add(crl.RawData);
161166
}
162167
}
168+
163169
}
164170
finally
165171
{
@@ -169,6 +175,12 @@ private ServiceResult Open(
169175
store = m_issuerStore.OpenStore();
170176
try
171177
{
178+
179+
if (store == null)
180+
{
181+
throw new ServiceResultException(StatusCodes.BadConfigurationError, "Failed to open issuer certificate store.");
182+
}
183+
172184
if ((masks & TrustListMasks.IssuerCertificates) != 0)
173185
{
174186
X509Certificate2Collection certificates = store.Enumerate().GetAwaiter().GetResult();
@@ -185,6 +197,7 @@ private ServiceResult Open(
185197
trustList.IssuerCrls.Add(crl.RawData);
186198
}
187199
}
200+
188201
}
189202
finally
190203
{
@@ -524,6 +537,11 @@ private ServiceResult RemoveCertificate(
524537
var storeIdentifier = isTrustedCertificate ? m_trustedStore : m_issuerStore;
525538
using (ICertificateStore store = storeIdentifier.OpenStore())
526539
{
540+
if (store == null)
541+
{
542+
throw new ServiceResultException(StatusCodes.BadConfigurationError, "Failed to open certificate store.");
543+
}
544+
527545
var certCollection = store.FindByThumbprint(thumbprint).GetAwaiter().GetResult();
528546

529547
if (certCollection.Count == 0)
@@ -563,6 +581,7 @@ private ServiceResult RemoveCertificate(
563581
}
564582
}
565583
}
584+
566585
}
567586

568587
m_node.LastUpdateTime.Value = DateTime.UtcNow;
@@ -622,6 +641,11 @@ private async Task<bool> UpdateStoreCrls(
622641
ICertificateStore store = storeIdentifier.OpenStore();
623642
try
624643
{
644+
if (store == null)
645+
{
646+
throw new ServiceResultException(StatusCodes.BadConfigurationError, "Failed to open certificate store.");
647+
}
648+
625649
var storeCrls = await store.EnumerateCRLs().ConfigureAwait(false);
626650
foreach (var crl in storeCrls)
627651
{
@@ -664,6 +688,11 @@ private async Task<bool> UpdateStoreCertificates(
664688
ICertificateStore store = storeIdentifier.OpenStore();
665689
try
666690
{
691+
if (store == null)
692+
{
693+
throw new ServiceResultException(StatusCodes.BadConfigurationError, "Failed to open certificate store.");
694+
}
695+
667696
var storeCerts = await store.Enumerate().ConfigureAwait(false);
668697
foreach (var cert in storeCerts)
669698
{

‎Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs‎

Lines changed: 6 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,7 @@ public CertificateIdentifierCollection ApplicationCertificates
884884
/// <summary>
885885
/// The store containing any additional issuer certificates.
886886
/// </summary>
887-
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 2)]
887+
[DataMember(IsRequired = true, EmitDefaultValue = false, Order = 2)]
888888
public CertificateTrustList TrustedIssuerCertificates
889889
{
890890
get
@@ -901,7 +901,7 @@ public CertificateTrustList TrustedIssuerCertificates
901901
/// <summary>
902902
/// The trusted certificate store.
903903
/// </summary>
904-
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 4)]
904+
[DataMember(IsRequired = true, EmitDefaultValue = false, Order = 4)]
905905
public CertificateTrustList TrustedPeerCertificates
906906
{
907907
get
@@ -2052,7 +2052,7 @@ public bool HttpsMutualTls
20522052
/// Enable / disable support for durable subscriptions
20532053
/// </summary>
20542054
/// <value><c>true</c> if durable subscriptions are enabled; otherwise, <c>false</c>.</value>
2055-
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 38)]
2055+
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 39)]
20562056
public bool DurableSubscriptionsEnabled
20572057
{
20582058
get { return m_DurableSubscriptionsEnabled; }
@@ -2063,7 +2063,7 @@ public bool DurableSubscriptionsEnabled
20632063
/// The maximum number of notifications saved in the durable queue for each monitored item.
20642064
/// </summary>
20652065
/// <value>The maximum size of the durable notification queue.</value>
2066-
[DataMember(IsRequired = false, Order = 39)]
2066+
[DataMember(IsRequired = false, Order = 40)]
20672067
public int MaxDurableNotificationQueueSize
20682068
{
20692069
get { return m_maxDurableNotificationQueueSize; }
@@ -2074,7 +2074,7 @@ public int MaxDurableNotificationQueueSize
20742074
/// The max size of the durable event queue.
20752075
/// </summary>
20762076
/// <value>The max size of the durable event queue.</value>
2077-
[DataMember(IsRequired = false, Order = 40)]
2077+
[DataMember(IsRequired = false, Order = 41)]
20782078
public int MaxDurableEventQueueSize
20792079
{
20802080
get { return m_maxDurableEventQueueSize; }
@@ -2085,7 +2085,7 @@ public int MaxDurableEventQueueSize
20852085
/// How long the durable subscriptions will remain open without a publish from the client.
20862086
/// </summary>
20872087
/// <value>The maximum durable subscription lifetime.</value>
2088-
[DataMember(IsRequired = false, Order = 41)]
2088+
[DataMember(IsRequired = false, Order = 42)]
20892089
public int MaxDurableSubscriptionLifetimeInHours
20902090
{
20912091
get { return m_maxDurableSubscriptionLifetimeInHours; }
@@ -2920,11 +2920,6 @@ public string StoreType
29202920
{
29212921
get
29222922
{
2923-
if (!String.IsNullOrEmpty(m_storeName))
2924-
{
2925-
return CertificateStoreType.X509Store;
2926-
}
2927-
29282923
return m_storeType;
29292924
}
29302925

@@ -2947,16 +2942,6 @@ public string StorePath
29472942
{
29482943
get
29492944
{
2950-
if (!String.IsNullOrEmpty(m_storeName))
2951-
{
2952-
if (String.IsNullOrEmpty(m_storeLocation))
2953-
{
2954-
return CurrentUser + m_storeName;
2955-
}
2956-
2957-
return Utils.Format("{0}\\{1}", m_storeLocation, m_storeName);
2958-
}
2959-
29602945
return m_storePath;
29612946
}
29622947

@@ -2974,28 +2959,6 @@ public string StorePath
29742959
}
29752960
}
29762961

2977-
/// <summary>
2978-
/// The name of the certificate store that contains the trusted certificates.
2979-
/// </summary>
2980-
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 2)]
2981-
[Obsolete("Use StoreType/StorePath instead")]
2982-
public string StoreName
2983-
{
2984-
get { return m_storeName; }
2985-
set { m_storeName = value; }
2986-
}
2987-
2988-
/// <summary>
2989-
/// The location of the certificate store that contains the trusted certificates.
2990-
/// </summary>
2991-
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 3)]
2992-
[Obsolete("Use StoreType/StorePath instead")]
2993-
public string StoreLocation
2994-
{
2995-
get { return m_storeLocation; }
2996-
set { m_storeLocation = value; }
2997-
}
2998-
29992962
/// <summary>
30002963
/// Options that can be used to suppress certificate validation errors.
30012964
/// </summary>
@@ -3010,8 +2973,6 @@ private int XmlEncodedValidationOptions
30102973
#region Private Fields
30112974
private string m_storeType;
30122975
private string m_storePath;
3013-
private string m_storeLocation;
3014-
private string m_storeName;
30152976
private CertificateValidationOptions m_validationOptions;
30162977
#endregion
30172978
}

0 commit comments

Comments
 (0)