Validate issuer/CA certificate KeyUsage (keyCertSign, cRLSign) [master378 backport]#3948
Merged
Conversation
Backport to the 1.5.378 maintenance branch. OPC UA Part 6 6.2.4 Table 52 requires CA/issuer certificates to assert the keyCertSign and cRLSign KeyUsage bits. The validator only checked the leaf certificate's KeyUsage and delegated issuer usage to X509Chain, which per RFC 5280 does not reject a CA whose KeyUsage extension is absent, so non-compliant CAs were accepted (interop failures with strict third-party stacks). Add an explicit, suppressible BadCertificateIssuerUseNotAllowed check (Part 4 6.1.3 Table 100) over each issuer/CA certificate in the chain. Also fixes two coupled defects in CheckChainStatus: a FormatException from a named-placeholder format string, and an inverted issuer/leaf usage status-code mapping. Relates to #3944
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master378 #3948 +/- ##
=============================================
- Coverage 60.28% 60.13% -0.15%
=============================================
Files 378 378
Lines 79069 79082 +13
Branches 13838 13840 +2
=============================================
- Hits 47664 47555 -109
- Misses 26989 27105 +116
- Partials 4416 4422 +6
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Backports stricter OPC UA compliance checks by validating that issuer/CA certificates in a chain assert required KeyUsage bits, and fixes related chain-status handling issues.
Changes:
- Adds issuer/CA KeyUsage validation (
keyCertSign,cRLSign) and reportsBadCertificateIssuerUseNotAllowed(suppressible). - Fixes
CheckChainStatusformatting crash and corrects issuer vs leaf status code mapping forNotValidForUsage. - Adds tests and documentation describing CA (issuer) KeyUsage validation and suppression behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs | Adds unit + integration tests for issuer KeyUsage enforcement, suppression, and acceptance. |
| Stack/Opc.Ua.Core/Security/Certificates/X509Utils.cs | Introduces helper to verify issuer/CA KeyUsage requirements. |
| Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs | Validates issuer/CA KeyUsage during chain validation; fixes NotValidForUsage status handling. |
| Docs/Certificates.md | Documents new CA KeyUsage validation behavior and suppression guidance. |
mrsuciu
approved these changes
Jul 3, 2026
mrsuciu
left a comment
mrsuciu
left a comment
Contributor
There was a problem hiding this comment.
Code looks Ok, tests have copilot comments
cristipogacean
approved these changes
Jul 3, 2026
… event in finally Addresses PR review: the None-KeyUsage case exercised a present-but-empty extension, not an absent one. Add CreateCertificateWithoutKeyUsage (built via CertificateRequest so no KeyUsage extension is emitted) and assert the helper rejects it. Switch the test CA to a relative validity window (now minus one day) so it is always valid. Wrap the CertificateValidation event subscription in try/finally so the handler is always detached even if validation throws.
CI failed on Linux/macOS: the OpenSSL chain engine raises extra suppressible errors (BadCertificateUntrusted, BadCertificateChainIncomplete/PartialChain) for a self-signed CA with empty/absent KeyUsage that Windows CryptoAPI does not, so the fixed 4-code approver rejected the cert. Approve every error via the CertificateValidation event (e.Accept) and assert the issuer-use error was among them, making the test independent of the platform chain implementation. Verified on Linux via WSL.
marcschier
added a commit
that referenced
this pull request
Jul 4, 2026
## Summary Follow-up to #3948 (merged) that fixes a **macOS-only** CI failure introduced by the issuer/CA KeyUsage tests. On the macOS Core test job, three tests failed with: ``` Interop+AppleCrypto+AppleCommonCryptoCryptographicException : Unknown format in import. at System.Security.Cryptography.X509Certificates.X509Pal.AppleX509Pal.GetCertContentType(...) at System.Security.Cryptography.X509Certificates.X509CertificateLoader.LoadCertificate(...) ``` The affected tests built a test CA with an **empty** KeyUsage extension (`new X509KeyUsageExtension(X509KeyUsageFlags.None, true)` → a zero-bit `BIT STRING`). Windows (CryptoAPI) and Linux (OpenSSL) load such a certificate, but macOS (AppleCrypto) rejects it as an unknown format. The production code is unaffected — this is purely a test-certificate artifact. ## Changes - Removed the degenerate empty-KeyUsage test certificate. The **truly-absent** KeyUsage case (built via `CertificateRequest` so no KeyUsage extension is emitted) already covers the `GetKeyUsage() == None` code path. - The reject/suppress integration tests now use a present-but-insufficient `DigitalSignature` KeyUsage (a valid, macOS-loadable certificate that is still missing `keyCertSign`/`cRLSign`). ## Testing - `CertificateValidator` IssuerKeyUsage tests pass on **Windows** and **Linux** (net10.0). The change directly removes the macOS root cause (the empty-KeyUsage extension); all remaining test certificates use standard formats that macOS already loads (the compliant-CA test passed on macOS). Relates to #3944 · follow-up to #3948
This was referenced Jul 10, 2026
Open
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backport of the #3944 fix to the
master378(1.5.378) maintenance branch. See the 2.0 PR #3947 for the primary change; this PR applies the equivalent fix to the 1.5.378 code.The stack did not verify that CA/issuer certificates in a chain assert the KeyUsage required for a CA. Per OPC 10000-6 §6.2.4, Table 52, a CA certificate must carry a KeyUsage extension asserting
keyCertSignandcRLSign, and OPC 10000-4 §6.1.3, Table 100 ("Certificate Usage") requires validators to check issuer usage and reportBad_CertificateIssuerUseNotAllowed.The validator only checked the leaf certificate's KeyUsage and delegated issuer/CA usage to .NET
X509Chain, which (RFC 5280 §4.2.1.3) does not reject a CA whose KeyUsage extension is absent — so a CA with no KeyUsage was accepted while strict third-party stacks reject the chain withBadCertificateInvalid.Changes
X509Utils.HasRequiredIssuerKeyUsage(X509Certificate2)— returns whether bothkeyCertSignandcRLSignare asserted (absent KeyUsage →None→ fails).CertificateValidator.InternalValidateAsync— after the leaf KeyUsage check, verifies every issuer/CA certificate in the chain and appends a suppressibleBad_CertificateIssuerUseNotAllowedwhen the required bits are missing.CheckChainStatus:FormatExceptionfrom a named-placeholder format string ({Status}: {Information}→ positional{0}: {1}) that crashed validation on anyNotValidForUsagechain status.isIssuerstatus-code ternary (issuer↔leaf codes swapped, contrary to Part 4 Table 100).Docs/Certificates.md— new "CA (issuer) KeyUsage validation" subsection.Every new check/helper carries an in-code citation to the governing spec sections.
Testing
CertificateValidatorTest.cs(matching the branch's existing style:X509Certificate2, classic asserts, throwing contract, event-based approver): a deterministic helper unit test plus reject / suppress / accept integration tests.CertificateValidatorfixture green on net8.0 and net48, no regressions.Compatibility
masterandmaster378— not a 2.0 regression. No public API change; the new error is suppressible.Relates to #3944 · 2.0 PR: #3947