diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 992ee2a..04b2cdc 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -1,3 +1,4 @@ +using System.Security.Cryptography.X509Certificates; using Opc.Ua; using Opc.Ua.Client; using Opc.Ua.Configuration; @@ -248,17 +249,11 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d await config.ValidateAsync(ApplicationType.Client, ct).ConfigureAwait(false); - // Attach a cert-validator handler that honours the AutoAccept flag. Without this, - // AutoAcceptUntrustedCertificates on the config alone isn't always enough in newer - // SDK versions — the validator raises an event the app has to handle. - if (_options.AutoAcceptCertificates) - { - config.CertificateValidator.CertificateValidation += (s, e) => - { - if (e.Error.StatusCode == StatusCodes.BadCertificateUntrusted) - e.Accept = true; - }; - } + // Attach a cert-validator handler. The SDK's AutoAcceptUntrustedCertificates flag + // alone isn't always enough in newer SDK versions — the validator raises an event + // the app has to handle. We also use this hook to enforce the + // CertificateValidation policy (revoked, SHA-1, key size) regardless of AutoAccept. + config.CertificateValidator.CertificateValidation += OnCertificateValidation; // Ensure an application certificate exists. The SDK auto-generates one if missing. app.ApplicationConfiguration = config; @@ -268,6 +263,128 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d return config; } + /// + /// Cert-validator callback. Funnels into + /// for testability — the static helper takes the cert + status code + options and + /// returns the decision, which this method then applies to the SDK's event args. + /// + private void OnCertificateValidation(object sender, Opc.Ua.CertificateValidationEventArgs e) + { + var decision = EvaluateCertificateValidation( + e.Certificate, + e.Error.StatusCode, + _options.AutoAcceptCertificates, + _options.CertificateValidation); + + if (decision.LogMessage is { Length: > 0 }) + { + // Use the SDK's trace surface — no driver-side ILogger is plumbed today, and the + // SDK trace is already wired up by the host. Warning level for rejections so + // operators surface them without code changes. The non-telemetry overload is + // marked obsolete in the latest SDK; suppress locally to keep the gateway-driver + // surface free of an ITelemetryContext plumb-through (parity with the same + // pattern in BuildApplicationConfigurationAsync). +#pragma warning disable CS0618 + Opc.Ua.Utils.LogWarning( + "OpcUaClient[{0}] cert-validation: {1} (subject={2}, status=0x{3:X8})", + driverInstanceId, decision.LogMessage, + e.Certificate?.Subject ?? "", + (uint)e.Error.StatusCode.Code); +#pragma warning restore CS0618 + } + + e.Accept = decision.Accept; + } + + /// + /// Cert-validation decision pipeline. Pulled out as a static helper so unit tests can + /// drive each branch without standing up an OPC UA SDK CertificateValidator. + /// Order matters: revoked > SHA-1 > key-size > revocation-unknown > auto-accept-untrusted. + /// + /// Server certificate the SDK is asking us to validate. May be null in pathological cases. + /// The SDK's validation result. Good = no failure to inspect. + /// Mirror of . + /// The cert-validation knobs. + internal static CertificateValidationDecision EvaluateCertificateValidation( + System.Security.Cryptography.X509Certificates.X509Certificate2? cert, + Opc.Ua.StatusCode status, + bool autoAcceptUntrusted, + OpcUaCertificateValidationOptions opts) + { + // Revoked certs are always a hard fail — never auto-accept regardless of flags. + if (status.Code == Opc.Ua.StatusCodes.BadCertificateRevoked) + return new CertificateValidationDecision(false, "REVOKED server certificate — rejecting"); + if (status.Code == Opc.Ua.StatusCodes.BadCertificateIssuerRevoked) + return new CertificateValidationDecision(false, "REVOKED issuer certificate — rejecting"); + + // SHA-1 signature detection runs even when the SDK didn't surface a status — + // we want to reject SHA-1 certs on policy, not just when the SDK happens to flag them. + if (opts.RejectSHA1SignedCertificates && IsSha1Signed(cert)) + return new CertificateValidationDecision(false, "SHA-1 signed certificate rejected by policy"); + + // Key-size check: only meaningful for RSA keys; ECC bypasses. + if (cert is not null && TryGetRsaKeySize(cert, out var keyBits) && keyBits < opts.MinimumCertificateKeySize) + return new CertificateValidationDecision(false, + $"RSA key size {keyBits} bits below minimum {opts.MinimumCertificateKeySize}"); + + // Unknown revocation status — reject only if policy says so. + if (status.Code == Opc.Ua.StatusCodes.BadCertificateRevocationUnknown + || status.Code == Opc.Ua.StatusCodes.BadCertificateIssuerRevocationUnknown) + { + if (opts.RejectUnknownRevocationStatus) + return new CertificateValidationDecision(false, "revocation status unknown (no/stale CRL) — rejecting per policy"); + return new CertificateValidationDecision(true, "revocation status unknown (no/stale CRL) — accepting per policy"); + } + + // Untrusted: SDK couldn't chain the cert to a trusted issuer. Honour AutoAccept. + if (status.Code == Opc.Ua.StatusCodes.BadCertificateUntrusted) + { + if (autoAcceptUntrusted) return new CertificateValidationDecision(true, null); + return new CertificateValidationDecision(false, "untrusted certificate — rejecting (AutoAcceptCertificates=false)"); + } + + // Anything else is an SDK-level failure — let the SDK's default disposition stand + // (don't accept by default; surface the status code in the log). + if (status.Code != Opc.Ua.StatusCodes.Good) + return new CertificateValidationDecision(false, $"validation failed (status=0x{(uint)status.Code:X8})"); + + return new CertificateValidationDecision(true, null); + } + + /// + /// True when the cert's signature algorithm OID matches a SHA-1 RSA signature + /// (1.2.840.113549.1.1.5) or a SHA-1 ECDSA signature (1.2.840.10045.4.1). + /// Friendly-name prefix match is unreliable across .NET runtimes, so we use OIDs. + /// + internal static bool IsSha1Signed(System.Security.Cryptography.X509Certificates.X509Certificate2? cert) + { + if (cert is null) return false; + var oid = cert.SignatureAlgorithm?.Value; + return oid is "1.2.840.113549.1.1.5" // sha1RSA + or "1.2.840.10045.4.1"; // sha1ECDSA + } + + /// + /// Read the RSA public key size in bits if the cert has an RSA key. Returns false for + /// non-RSA (ECC, DSA) certs so the key-size check is skipped on them. + /// + internal static bool TryGetRsaKeySize( + System.Security.Cryptography.X509Certificates.X509Certificate2 cert, + out int keyBits) + { + using var rsa = cert.GetRSAPublicKey(); + if (rsa is null) { keyBits = 0; return false; } + keyBits = rsa.KeySize; + return true; + } + + /// + /// Outcome of . + /// is null when the decision is silently "accept (Good)" — no need to log healthy + /// validations. + /// + internal readonly record struct CertificateValidationDecision(bool Accept, string? LogMessage); + /// /// Resolve the ordered failover candidate list. EndpointUrls wins when /// non-empty; otherwise fall back to EndpointUrl as a single-URL shortcut so diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs index ca0d7b1..8563150 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs @@ -141,8 +141,56 @@ public sealed class OpcUaClientDriverOptions /// values so existing deployments see no behaviour change. /// public OpcUaSubscriptionDefaults Subscriptions { get; init; } = new(); + + /// + /// Server-certificate validation knobs applied during the + /// CertificateValidator.CertificateValidation callback. Surfaces explicit + /// handling for revoked certs (always rejected, never auto-accepted), unknown + /// revocation status (rejected only when + /// is set), SHA-1 signature rejection, and minimum RSA key size. Defaults preserve + /// existing behaviour wherever possible — the one tightening is + /// =true + /// since SHA-1 is spec-deprecated for OPC UA. + /// + public OpcUaCertificateValidationOptions CertificateValidation { get; init; } = new(); } +/// +/// Knobs governing the server-certificate validation callback. Plumbed onto +/// rather than the top-level +/// options to keep cert-related config grouped together. +/// +/// +/// +/// CRL discovery: the OPC UA SDK reads CRL files automatically from the +/// crl/ sub-directory of each cert store (own, trusted, issuers). Drop the +/// issuer's .crl in that folder and the SDK picks it up — no driver-side wiring +/// required. When the directory is absent or empty, the SDK reports +/// BadCertificateRevocationUnknown, which this driver gates with +/// . +/// +/// +/// +/// Reject server certificates whose signature uses SHA-1. Default true — SHA-1 was +/// deprecated by the OPC UA spec and is treated as a hard fail in production. Flip to +/// false only for short-term interop with legacy controllers. +/// +/// +/// When the SDK can't determine revocation status (no CRL present, or stale CRL), +/// reject the cert if true; allow if false. Default false — many +/// plant deployments don't run CRL infrastructure, and a hard-fail default would break +/// them on first connection. Set true in environments with a managed PKI. +/// +/// +/// Minimum RSA key size (bits) accepted. Certs with shorter keys are rejected. Default +/// 2048 matches the current OPC UA spec floor; raise to 3072 or 4096 for stricter +/// deployments. Non-RSA keys (ECC) bypass this check. +/// +public sealed record OpcUaCertificateValidationOptions( + bool RejectSHA1SignedCertificates = true, + bool RejectUnknownRevocationStatus = false, + int MinimumCertificateKeySize = 2048); + /// /// Tuning surface for OPC UA subscriptions created by . /// Lifted from the per-call hard-coded literals so operators can tune publish cadence, diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientCertValidationTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientCertValidationTests.cs new file mode 100644 index 0000000..46a3f58 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientCertValidationTests.cs @@ -0,0 +1,242 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Opc.Ua; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// +/// Unit coverage for the cert-validation knobs added in PR #277. Live revocation testing +/// requires standing up a CA + CRL; we cover the parts that are testable without one: +/// option defaults, the static decision pipeline, SHA-1 detection, and key-size checks. +/// +[Trait("Category", "Unit")] +public sealed class OpcUaClientCertValidationTests +{ + [Fact] + public void Defaults_match_documented_policy() + { + var opts = new OpcUaClientDriverOptions(); + opts.CertificateValidation.RejectSHA1SignedCertificates.ShouldBeTrue( + "SHA-1 is spec-deprecated for OPC UA — default must be hard-fail."); + opts.CertificateValidation.RejectUnknownRevocationStatus.ShouldBeFalse( + "Default must allow brownfield deployments without CRL infrastructure."); + opts.CertificateValidation.MinimumCertificateKeySize.ShouldBe(2048); + } + + [Fact] + public void Revoked_cert_is_rejected_even_when_AutoAccept_is_true() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.BadCertificateRevoked), + autoAcceptUntrusted: true, + new OpcUaCertificateValidationOptions()); + + decision.Accept.ShouldBeFalse(); + decision.LogMessage!.ShouldContain("REVOKED"); + } + + [Fact] + public void Issuer_revoked_is_rejected_even_when_AutoAccept_is_true() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.BadCertificateIssuerRevoked), + autoAcceptUntrusted: true, + new OpcUaCertificateValidationOptions()); + + decision.Accept.ShouldBeFalse(); + decision.LogMessage!.ShouldContain("REVOKED issuer"); + } + + [Fact] + public void RevocationUnknown_default_accepts_with_log_note() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.BadCertificateRevocationUnknown), + autoAcceptUntrusted: false, + new OpcUaCertificateValidationOptions { RejectUnknownRevocationStatus = false }); + + decision.Accept.ShouldBeTrue(); + decision.LogMessage!.ShouldContain("revocation status unknown"); + } + + [Fact] + public void RevocationUnknown_with_strict_flag_rejects() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.BadCertificateRevocationUnknown), + autoAcceptUntrusted: true, + new OpcUaCertificateValidationOptions { RejectUnknownRevocationStatus = true }); + + decision.Accept.ShouldBeFalse(); + decision.LogMessage!.ShouldContain("revocation status unknown"); + } + + [Fact] + public void Sha1_signed_cert_is_rejected_by_default() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA1); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.Good), + autoAcceptUntrusted: false, + new OpcUaCertificateValidationOptions()); + + decision.Accept.ShouldBeFalse(); + decision.LogMessage!.ShouldContain("SHA-1"); + } + + [Fact] + public void Sha1_acceptance_can_be_opted_back_into() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA1); + + // Untrusted + auto-accept = let it through; SHA-1 must NOT be the failing reason. + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.BadCertificateUntrusted), + autoAcceptUntrusted: true, + new OpcUaCertificateValidationOptions { RejectSHA1SignedCertificates = false }); + + decision.Accept.ShouldBeTrue(); + } + + [Fact] + public void Small_rsa_key_is_rejected_below_minimum() + { + using var cert = CreateRsaCert(1024, HashAlgorithmName.SHA256); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.Good), + autoAcceptUntrusted: false, + new OpcUaCertificateValidationOptions()); + + decision.Accept.ShouldBeFalse(); + decision.LogMessage!.ShouldContain("1024"); + } + + [Fact] + public void TryGetRsaKeySize_reports_correct_bit_count() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + OpcUaClientDriver.TryGetRsaKeySize(cert, out var bits).ShouldBeTrue(); + bits.ShouldBe(2048); + } + + [Fact] + public void IsSha1Signed_detects_sha1_signature() + { + using var sha1Cert = CreateRsaCert(2048, HashAlgorithmName.SHA1); + using var sha256Cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + OpcUaClientDriver.IsSha1Signed(sha1Cert).ShouldBeTrue(); + OpcUaClientDriver.IsSha1Signed(sha256Cert).ShouldBeFalse(); + OpcUaClientDriver.IsSha1Signed(null).ShouldBeFalse(); + } + + [Fact] + public void Untrusted_without_AutoAccept_is_rejected() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.BadCertificateUntrusted), + autoAcceptUntrusted: false, + new OpcUaCertificateValidationOptions()); + + decision.Accept.ShouldBeFalse(); + decision.LogMessage!.ShouldContain("untrusted"); + } + + [Fact] + public void Good_status_with_compliant_cert_accepts_silently() + { + using var cert = CreateRsaCert(2048, HashAlgorithmName.SHA256); + + var decision = OpcUaClientDriver.EvaluateCertificateValidation( + cert, + new StatusCode(StatusCodes.Good), + autoAcceptUntrusted: false, + new OpcUaCertificateValidationOptions()); + + decision.Accept.ShouldBeTrue(); + decision.LogMessage.ShouldBeNull("Good validations shouldn't emit log noise."); + } + + private static X509Certificate2 CreateRsaCert(int keySize, HashAlgorithmName hash) + { + // .NET 10's CertificateRequest.CreateSelfSigned rejects SHA-1 outright. For the + // SHA-256 path we use the supported API; for SHA-1 we route through a custom + // X509SignatureGenerator that signs with SHA-1 OID so we can synthesise a SHA-1 + // signed cert in-process without shipping a binary fixture. + var rsa = RSA.Create(keySize); + var req = new CertificateRequest( + new System.Security.Cryptography.X509Certificates.X500DistinguishedName( + "CN=OpcUaClientCertValidationTests"), + rsa, + hash == HashAlgorithmName.SHA1 ? HashAlgorithmName.SHA256 : hash, + RSASignaturePadding.Pkcs1); + + if (hash == HashAlgorithmName.SHA1) + { + var generator = new Sha1RsaSignatureGenerator(rsa); + var serial = new byte[8]; + System.Security.Cryptography.RandomNumberGenerator.Fill(serial); + var built = req.Create( + req.SubjectName, + generator, + DateTimeOffset.UtcNow.AddMinutes(-5), + DateTimeOffset.UtcNow.AddHours(1), + serial); + // Combine cert + key so GetRSAPublicKey works downstream. + return built.CopyWithPrivateKey(rsa); + } + + return req.CreateSelfSigned( + DateTimeOffset.UtcNow.AddMinutes(-5), + DateTimeOffset.UtcNow.AddHours(1)); + } + + /// + /// SHA-1 RSA signature generator. .NET 10's + /// refuses SHA-1; we subclass to emit the SHA-1 RSA algorithm identifier + /// (1.2.840.113549.1.1.5) and sign with SHA-1 explicitly. Test-only. + /// + private sealed class Sha1RsaSignatureGenerator : X509SignatureGenerator + { + private readonly RSA _rsa; + public Sha1RsaSignatureGenerator(RSA rsa) { _rsa = rsa; } + + public override byte[] GetSignatureAlgorithmIdentifier(HashAlgorithmName hashAlgorithm) + { + // DER: SEQUENCE { OID 1.2.840.113549.1.1.5, NULL } + return new byte[] + { + 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x05, 0x05, 0x00, + }; + } + + public override byte[] SignData(byte[] data, HashAlgorithmName hashAlgorithm) + => _rsa.SignData(data, HashAlgorithmName.SHA1, RSASignaturePadding.Pkcs1); + + protected override PublicKey BuildPublicKey() => PublicKey.CreateFromSubjectPublicKeyInfo( + _rsa.ExportSubjectPublicKeyInfo(), out _); + } +}