Prepare 1.5378 maintenance release #3982
Merged
Merged
Conversation
…ion policy (master378) (#3936) ## Proposed changes Backport of the #3931 fix to master378. On this branch, server-side GDS Push `UpdateCertificate` validated the new application certificate with a throwaway `CertificateValidator` that was never seeded with the server's `SecurityConfiguration`. It therefore enforced hard-coded defaults (`MinimumCertificateKeySize = 2048`, `RejectSHA1SignedCertificates = true`) regardless of operator configuration — e.g. a server configured for a 1024-bit minimum still rejected a re-issued 1024-bit certificate with `BadCertificatePolicyCheckFailed`. Regression originated in #3795 (integrity check switched from a lenient `X509Chain` check to the full validator). The master branch reworked this area entirely, so the fix is re-implemented from scratch here, adapted to master378's `X509Certificate2` types. - **`ConfigurationNodeManager.UpdateCertificateAsync`**: replaced the throwaway `CertificateValidator` with a lenient `X509Chain` integrity check that verifies only chain integrity — valid signatures and a complete issuer chain — using the caller-supplied issuers as the trust anchor (`AllowUnknownCertificateAuthority`, revocation `NoCheck`, downloads disabled). Policy (min key size, SHA-1, revocation) is intentionally **not** applied here; it remains governed by the server's `SecurityConfiguration` on its normal communication channels, so configured values are honored in both stricter and more-lenient directions. Chain status handling: `NoError`/`UntrustedRoot` pass; `NotSignatureValid`/`PartialChain`/`NotValidForUsage`/`InvalidBasicConstraints` fail with `BadSecurityChecksFailed`; and the chain element count must match the supplied issuer count to reject incomplete chains.
…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
## 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
…nition only. (#3843)" (#3976) This reverts commit 00519fa. # Description Due to longer investigation time needed on ModelCompiler the pruning of the extra decoded children that should stay on the type definition only is reverted ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received.
|
|
romanett
approved these changes
Jul 10, 2026
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.
Description
Prepare 1.5378 maintenance release
Checklist
Put an
xin the boxes that apply. You can complete these step by step after opening the PR.