Validate issuer/CA certificate KeyUsage (keyCertSign, cRLSign)#3947
Merged
Conversation
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 @@
## master #3947 +/- ##
=======================================
Coverage 73.51% 73.52%
=======================================
Files 1170 1170
Lines 170085 170175 +90
Branches 29338 29363 +25
=======================================
+ Hits 125041 125120 +79
- Misses 34050 34052 +2
- Partials 10994 11003 +9
🚀 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.
Closes a certificate-validation compliance gap by explicitly validating CA/issuer KeyUsage in the certificate chain (keyCertSign + cRLSign) and mapping violations to BadCertificateIssuerUseNotAllowed, aligning behavior with OPC UA specs and strict peer stacks.
Changes:
- Added
HasRequiredIssuerKeyUsagehelper and enforced it for every issuer/CA cert during validation. - Fixed
CheckChainStatushandling forNotValidForUsage(format string crash + swapped issuer/leaf status codes). - Added targeted unit/integration tests and documented the new issuer KeyUsage validation behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs | Adds helper tests and end-to-end validation tests for issuer/CA KeyUsage reject/suppress/accept flows. |
| Stack/Opc.Ua.Core/Security/Certificates/CertificateManager/CertificateValidationHelpers.cs | Introduces a dedicated helper to verify issuer/CA KeyUsage bits required by OPC UA. |
| Stack/Opc.Ua.Core/Security/Certificates/CertificateManager/CertificateValidationCore.cs | Enforces issuer/CA KeyUsage validation across chain issuers; fixes NotValidForUsage formatting and status mapping. |
| Docs/Certificates.md | Documents the new CA (issuer) KeyUsage validation rule and suppression guidance. |
mrsuciu
approved these changes
Jul 3, 2026
cristipogacean
approved these changes
Jul 3, 2026
…dity 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. Also switch the test CA to a relative validity window (now minus one day) so it is always valid regardless of run date.
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 callback 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
…r378 backport] (#3948) ## 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 `keyCertSign` and `cRLSign`, and **OPC 10000-4 §6.1.3, Table 100 ("Certificate Usage")** requires validators to check issuer usage and report `Bad_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 with `BadCertificateInvalid`. ## Changes - `X509Utils.HasRequiredIssuerKeyUsage(X509Certificate2)` — returns whether both `keyCertSign` and `cRLSign` are asserted (absent KeyUsage → `None` → fails). - `CertificateValidator.InternalValidateAsync` — after the leaf KeyUsage check, verifies every issuer/CA certificate in the chain and appends a **suppressible** `Bad_CertificateIssuerUseNotAllowed` when the required bits are missing. - **Two coupled defects fixed in `CheckChainStatus`**: 1. A `FormatException` from a named-placeholder format string (`{Status}: {Information}` → positional `{0}: {1}`) that crashed validation on any `NotValidForUsage` chain status. 2. An inverted `isIssuer` status-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 - New tests in `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. - Full `CertificateValidator` fixture green on **net8.0** and **net48**, no regressions. ## Compatibility - This is long-standing behaviour on both `master` and `master378` — not a 2.0 regression. No public API change; the new error is suppressible. Relates to #3944 · 2.0 PR: #3947
The macOS Core test job failed with AppleCommonCryptoCryptographicException 'Unknown format in import' when loading a certificate built with an empty X509KeyUsageExtension (X509KeyUsageFlags.None -> zero-bit BIT STRING). Drop the degenerate empty-KeyUsage test cert: the truly-absent-KeyUsage case (built via CertificateRequest) already covers the GetKeyUsage()==None path, and the reject/suppress integration tests now use a present-but-insufficient DigitalSignature KeyUsage. Verified on Windows and Linux.
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
Fixes a certificate-validation compliance gap: 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 (Issuer Certificate), 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 all issuer/CA usage to .NET
X509Chain. Per RFC 5280 §4.2.1.3,X509Chaindoes not reject a CA whose KeyUsage extension is absent (absence implies no usage restriction), so a root/intermediate CA with no KeyUsage — exactly the scenario reported in #3944 — was accepted by two .NET peers while strict third-party OPC UA stacks reject the same chain withBadCertificateInvalid.Changes
CertificateValidationHelpers.HasRequiredIssuerKeyUsage(Certificate)— returns whether bothkeyCertSignandcRLSignare asserted (absent KeyUsage →None→ fails).CertificateValidationCore.InternalValidateAsync— after the leaf KeyUsage check, verifies every resolved issuer/CA certificate in the chain and appends a suppressibleBad_CertificateIssuerUseNotAllowedwhen the required bits are missing. Existing deployments that must interoperate with a non-compliant CA can suppress it via the validation callback.CheckChainStatus(surfaced while testing theNotValidForUsagepath):FormatException—ServiceResult.Createwas given a named-placeholder format string ({Status}: {Information}) where positional{0}: {1}is required; this crashed validation wheneverX509ChainreportedNotValidForUsage.isIssuerstatus-code ternary (issuer↔leaf codes were 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: a deterministic helper unit test (absent / partial / compliant KeyUsage) plus integration tests for reject / suppress / accept.CertificateValidatorfixture green on net10.0 and net48, no regressions.Compatibility
master(2.0) andmaster378(1.5.378) — not a 2.0 regression. The 1.5.378 backport is submitted separately.Relates to #3944