Skip to content

Commit

Permalink
Fix Authority Key Identifier in SubCA and cert validator return codes. (
Browse files Browse the repository at this point in the history
#1665)

- AuthorityKeyIdentifier in the SubCA contains the SubjectName of the Issuer instead of the IssuerName. Also an application certificate that is signed by a SubCA would contain the false information.
- The false information has no effect on Windows and macOS, however on linux OpenSSL tests all fields and a chain cannot be fully validated.
- For CTT tests a few error codes were return false codes because ServiceResult replaces the error code with the InnerException error.
- Do not allow to ignore PartialChain validation error, it may have different root cause then the error returned by GetIssuer.
  • Loading branch information
mregen committed Jan 14, 2022
1 parent 15f1fcb commit 6747b47
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public static X509Extension BuildAuthorityKeyIdentifier(X509Certificate2 issuerC
var ski = issuerCaCertificate.Extensions.OfType<X509SubjectKeyIdentifierExtension>().Single();
return new X509AuthorityKeyIdentifierExtension(
ski.SubjectKeyIdentifier.FromHexString(),
issuerCaCertificate.SubjectName,
issuerCaCertificate.IssuerName,
issuerCaCertificate.GetSerialNumber());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,8 @@ private void CreateX509Extensions(CertificateRequest request, bool forECDsa)
? X509Extensions.BuildAuthorityKeyIdentifier(IssuerCAKeyCert)
: new X509AuthorityKeyIdentifierExtension(
ski.SubjectKeyIdentifier.FromHexString(),
SubjectName,
m_serialNumber
);
IssuerName,
m_serialNumber);
request.CertificateExtensions.Add(authorityKeyIdentifier);

X509KeyUsageFlags keyUsageFlags;
Expand Down
20 changes: 10 additions & 10 deletions Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ public virtual void Validate(X509Certificate2Collection chain, ConfiguredEndpoin
/// must not be supressed according to (e.g.) version 1.04 of the spec)
/// </summary>
/// <param name="sr"></param>
static private bool ContainsUnsuppressibleSC(ServiceResult sr)
private static bool ContainsUnsuppressibleSC(ServiceResult sr)
{
while (sr != null)
{
Expand Down Expand Up @@ -1064,21 +1064,23 @@ private static ServiceResult CheckChainStatus(X509ChainStatus status, Certificat
case X509ChainStatusFlags.UntrustedRoot:
{
// self signed cert signature validation
// .Net Core ChainStatus returns NotSignatureValid only on Windows,
// .NET Core ChainStatus returns NotSignatureValid only on Windows,
// so we have to do the extra cert signature check on all platforms
if (issuer == null && !isIssuer &&
id.Certificate != null && X509Utils.CompareDistinguishedName(id.Certificate.Subject, id.Certificate.Issuer))
if (issuer == null && id.Certificate != null &&
X509Utils.CompareDistinguishedName(id.Certificate.Subject, id.Certificate.Issuer))
{
if (!IsSignatureValid(id.Certificate))
{
goto case X509ChainStatusFlags.NotSignatureValid;
}
break;
}

// ignore this error because the root check is done
// by looking the certificate up in the trusted issuer stores passed to the validator.
// the ChainStatus uses the trusted issuer stores.
break;
return ServiceResult.Create(
StatusCodes.BadCertificateChainIncomplete,
"Certificate chain validation failed. {0}: {1}",
status.Status,
status.StatusInformation);
}

case X509ChainStatusFlags.RevocationStatusUnknown:
Expand Down Expand Up @@ -1117,7 +1119,6 @@ private static ServiceResult CheckChainStatus(X509ChainStatus status, Certificat
{
if (id != null && ((id.ValidationOptions & CertificateValidationOptions.SuppressCertificateExpired) != 0))
{
// TODO: add logging
break;
}

Expand Down Expand Up @@ -1386,7 +1387,6 @@ public class CertificateUpdateEventArgs : EventArgs
#endregion
}


/// <summary>
/// Used to handle certificate update events.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion Stack/Opc.Ua.Core/Stack/Tcp/TcpServerChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ private bool ProcessOpenSecureChannelRequest(uint messageType, ArraySegment<byte
innerException.StatusCode == StatusCodes.BadCertificateChainIncomplete ||
innerException.StatusCode == StatusCodes.BadCertificateRevoked ||
innerException.StatusCode == StatusCodes.BadCertificateInvalid ||
innerException.StatusCode == StatusCodes.BadCertificatePolicyCheckFailed ||
(innerException.InnerResult != null && innerException.InnerResult.StatusCode == StatusCodes.BadCertificateUntrusted))
{
ForceChannelFault(StatusCodes.BadSecurityChecksFailed, e.Message);
Expand All @@ -528,7 +529,7 @@ private bool ProcessOpenSecureChannelRequest(uint messageType, ArraySegment<byte
}
}

ForceChannelFault(e, StatusCodes.BadSecurityChecksFailed, "Could not verify security on OpenSecureChannel request.");
ForceChannelFault(StatusCodes.BadSecurityChecksFailed, "Could not verify security on OpenSecureChannel request.");
return false;
}

Expand Down

0 comments on commit 6747b47

Please sign in to comment.