-
Notifications
You must be signed in to change notification settings - Fork 950
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
Check server application Uri with the create session response #2731
base: master
Are you sure you want to change the base?
Changes from all commits
a3386f1
0a65ea4
c6e35ae
d4cb133
b7c787c
7d3ec37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2338,7 +2338,6 @@ | |
|
||
if (requireEncryption) | ||
{ | ||
ValidateServerCertificateApplicationUri(serverCertificate); | ||
if (checkDomain) | ||
{ | ||
m_configuration.CertificateValidator.Validate(serverCertificateChain, m_endpoint); | ||
|
@@ -2417,24 +2416,24 @@ | |
if (!successCreateSession) | ||
{ | ||
base.CreateSession( | ||
null, | ||
clientDescription, | ||
m_endpoint.Description.Server.ApplicationUri, | ||
m_endpoint.EndpointUrl.ToString(), | ||
sessionName, | ||
clientNonce, | ||
clientCertificateChainData != null ? clientCertificateChainData : clientCertificateData, | ||
sessionTimeout, | ||
(uint)MessageContext.MaxMessageSize, | ||
out sessionId, | ||
out sessionCookie, | ||
out m_sessionTimeout, | ||
out serverNonce, | ||
out serverCertificateData, | ||
out serverEndpoints, | ||
out serverSoftwareCertificates, | ||
out serverSignature, | ||
out m_maxRequestMessageSize); | ||
null, | ||
clientDescription, | ||
m_endpoint.Description.Server.ApplicationUri, | ||
m_endpoint.EndpointUrl.ToString(), | ||
sessionName, | ||
clientNonce, | ||
clientCertificateChainData != null ? clientCertificateChainData : clientCertificateData, | ||
sessionTimeout, | ||
(uint)MessageContext.MaxMessageSize, | ||
out sessionId, | ||
out sessionCookie, | ||
out m_sessionTimeout, | ||
out serverNonce, | ||
out serverCertificateData, | ||
out serverEndpoints, | ||
out serverSoftwareCertificates, | ||
out serverSignature, | ||
out m_maxRequestMessageSize); | ||
} | ||
|
||
// save session id. | ||
|
@@ -2456,6 +2455,8 @@ | |
|
||
ValidateServerEndpoints(serverEndpoints); | ||
|
||
ValidateServerCertificateApplicationUri(m_endpoint, serverCertificate); | ||
|
||
ValidateServerSignature(serverCertificate, serverSignature, clientCertificateData, clientCertificateChainData, clientNonce); | ||
|
||
HandleSignedSoftwareCertificates(serverSoftwareCertificates); | ||
|
@@ -5352,26 +5353,53 @@ | |
!String.IsNullOrEmpty(identityPolicy.SecurityPolicyUri); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Validates the ServerCertificate ApplicationUri to match the ApplicationUri of the Endpoint for an open call (Spec Part 4 5.4.1) | ||
/// Validates the ServerCertificate ApplicationUri to match the ApplicationUri | ||
/// of the Endpoint (Spec Part 4 5.4.1) returned by the CreateSessionResponse. | ||
/// Ensure the endpoint was matched in <see cref="ValidateServerEndpoints"/> | ||
/// with the applicationUri of the server description before the validation. | ||
/// </summary> | ||
private void ValidateServerCertificateApplicationUri( | ||
X509Certificate2 serverCertificate) | ||
private static void ValidateServerCertificateApplicationUri(ConfiguredEndpoint endpoint, X509Certificate2 serverCertificate) | ||
{ | ||
var applicationUri = m_endpoint?.Description?.Server?.ApplicationUri; | ||
//check is only neccessary if the ApplicatioUri is specified for the Endpoint | ||
if (string.IsNullOrEmpty(applicationUri)) | ||
if (serverCertificate != null) | ||
{ | ||
throw ServiceResultException.Create( | ||
StatusCodes.BadSecurityChecksFailed, | ||
"No ApplicationUri is specified for the server in the EndpointDescription."); | ||
} | ||
string certificateApplicationUri = X509Utils.GetApplicationUriFromCertificate(serverCertificate); | ||
if (!string.Equals(certificateApplicationUri, applicationUri, StringComparison.Ordinal)) | ||
{ | ||
throw ServiceResultException.Create( | ||
StatusCodes.BadSecurityChecksFailed, | ||
"Server did not return a Certificate matching the ApplicationUri specified in the EndpointDescription."); | ||
var applicationUri = endpoint?.Description?.Server?.ApplicationUri; | ||
|
||
// check that an ApplicatioUri is specified for the Endpoint | ||
if (string.IsNullOrEmpty(applicationUri)) | ||
{ | ||
throw ServiceResultException.Create( | ||
StatusCodes.BadCertificateUriInvalid, | ||
"Server did not return an ApplicationUri in the EndpointDescription."); | ||
} | ||
|
||
var certificateApplicationUris = X509Utils.GetApplicationUrisFromCertificate(serverCertificate); | ||
if (certificateApplicationUris.Count == 0) | ||
{ | ||
throw ServiceResultException.Create( | ||
StatusCodes.BadCertificateUriInvalid, | ||
"The Server Certificate ({1}) does not contain an applicationUri.", | ||
serverCertificate.Subject); | ||
} | ||
|
||
bool noMatch = true; | ||
foreach (var certificateApplicationUri in certificateApplicationUris) | ||
{ | ||
if (string.Equals(certificateApplicationUri, applicationUri, StringComparison.Ordinal)) | ||
{ | ||
noMatch = false; | ||
break; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not noMatch = certificateApplicationUris .Where(x => String.Equals(certificateApplicationUri, applicationUri, StringComparison.Ordinal).Any() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you never know what Linq is up to... |
||
|
||
if (noMatch) | ||
{ | ||
throw ServiceResultException.Create( | ||
StatusCodes.BadCertificateUriInvalid, | ||
"The Application in the EndpointDescription ({0}) is not in the Server Certificate ({1}).", | ||
applicationUri, serverCertificate.Subject); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -5538,7 +5566,7 @@ | |
} | ||
} | ||
|
||
// find the matching description (TBD - check domains against certificate). | ||
// find the matching description (TBD - check domains and application URI against certificate). | ||
bool found = false; | ||
|
||
var foundDescription = FindMatchingDescription(serverEndpoints, m_endpoint.Description, true); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -671,23 +671,38 @@ | |
} | ||
} | ||
|
||
// check uri. | ||
string applicationUri = X509Utils.GetApplicationUriFromCertificate(certificate); | ||
// default application uri | ||
string applicationUri = configuration.ApplicationUri; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need a better way to pick the right one. |
||
if (String.IsNullOrEmpty(applicationUri)) | ||
// check certificate Uri and find a match | ||
var applicationUris = X509Utils.GetApplicationUrisFromCertificate(certificate); | ||
bool foundMatch = false; | ||
foreach (var appUri in applicationUris) | ||
{ | ||
string message = "The Application URI could not be read from the certificate. Use certificate anyway?"; | ||
if (!await ApplicationInstance.ApproveMessageAsync(message, silent).ConfigureAwait(false)) | ||
if (configuration.ApplicationUri.Equals(appUri, StringComparison.Ordinal)) | ||
{ | ||
return false; | ||
foundMatch = true; | ||
break; | ||
} | ||
} | ||
else if (!configuration.ApplicationUri.Equals(applicationUri, StringComparison.Ordinal)) | ||
|
||
if (!foundMatch && applicationUris.Count > 0) | ||
{ | ||
Utils.LogInfo("Updated the ApplicationUri: {0} --> {1}", configuration.ApplicationUri, applicationUri); | ||
foundMatch = true; | ||
applicationUri = applicationUris[0]; | ||
Utils.LogInfo("Updating the ApplicationUri: {0} --> {1}", configuration.ApplicationUri, applicationUris[0]); | ||
configuration.ApplicationUri = applicationUri; | ||
} | ||
|
||
if (!foundMatch) | ||
{ | ||
string message = "The Application URI could not be found in the certificate. Use certificate anyway?"; | ||
if (!await ApplicationInstance.ApproveMessageAsync(message, silent).ConfigureAwait(false)) | ||
{ | ||
return false; | ||
} | ||
} | ||
|
||
Utils.LogInfo("Using the ApplicationUri: {0}", applicationUri); | ||
|
||
// update configuration. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Runtime.Serialization; | ||
using System.Security.Cryptography.X509Certificates; | ||
using Opc.Ua.Security.Certificates; | ||
|
@@ -213,7 +214,7 @@ | |
{ | ||
try | ||
{ | ||
return X509Utils.GetApplicationUriFromCertificate(Certificate); | ||
return X509Utils.GetApplicationUrisFromCertificate(Certificate).FirstOrDefault(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need a better way to pick the right one. |
||
} | ||
catch (Exception e) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need logic to pick the right one.