Harden security and reliability across the OPC UA stack - #4021
Closed
marcschier wants to merge 27 commits into
Closed
marcschier wants to merge 27 commits into
marcschier wants to merge 27 commits into
Conversation
Verify default registration keeps classic and async factory paths separate and the Reference Server helper flags remain reusable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the current ISA95 JobControl NodeSet2 input as a focused source-generator regression without changing generator or codec behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the incomplete resource with the unmodified official ISA95 JobControl 2.0.0 NodeSet and assert its model, node coverage, and local references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verify cancellation preserves the caller token and stops the connect attempt started by ManagedSession.CreateAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the caller-visible cancellation and primary disposal path covered while excluding only the defensive secondary disposal-failure classifier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…upstream-parity-session-cleanup # Conflicts: # Libraries/Opc.Ua.Client/Session/ManagedSession.cs # Tests/Opc.Ua.Server.Tests/QuickstartsServerUtilitiesTests.cs
…upstream-parity-session-cleanup # Conflicts: # tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/Isa95JobControl.NodeSet2.xml
## Summary - Adds the official ISA95 JobControl 2.0.0 NodeSet2 as a complete source-generator regression fixture. - Extends the lightweight generated-stack test stub with `NamespaceMetadataState`, required by the complete model. - Adds a deterministic ManagedSession cancellation regression proving that a started connect attempt is stopped. The ManagedSession cleanup implementation and Quickstarts utility coverage now come from `master`; this PR retains the additional deterministic cleanup regression. ## ISA95 fixture provenance and integrity The fixture is byte-for-byte the official file: - Source: `OPCFoundation/UA-Nodeset@e0b5d80cfff698f0276393e28df138a7fbc4b541` - Path: `ISA95-JOBCONTROL/opc.ua.isa95-jobcontrol.nodeset2.xml` - Git blob: `82f008cd126ade54e5f2c737bcc116621a8c3087` - SHA-256: `52327e736beb604c7253a5a5393d1ad525b9ecf35b71fbacb4f429cacb334064` The fixture test retains the OPC Foundation MIT header and verifies the 2.0.0 model/date, 258 nodes (including 134 variables), representative late variables, and that every `ns=1` reference target resolves locally. ## Validation - ManagedSession tests: 17 passing on net10.0; 15 passing and 2 framework-gated skips on net48. - Quickstarts server utility tests: 3 passing on net10.0 and net48. - ISA95 generator and fixture integrity regression: 5 passing on net10.0.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
romanett
reviewed
Jul 18, 2026
| return subscription; | ||
| } | ||
|
|
||
| internal static UserIdentityToken? SanitizeUserIdentityToken( |
Contributor
There was a problem hiding this comment.
wont this break all restore scenarios?
batter to enrcrypt + sign the entire subscription payload before writing to disk?
romanett
reviewed
Jul 18, 2026
| { | ||
| // the server nonce should be validated if the token includes a secret. | ||
| if (!Nonce.ValidateNonce( | ||
| if (serverNonce.IsNull || |
Contributor
There was a problem hiding this comment.
isnt the valid nonce length not dependent on the SecurityPolicy?
Dont we allow to configure Nonce Lenght any more? if no we should remove the option from SecurityConfiguraiton
romanett
reviewed
Jul 18, 2026
| using var cert = Certificate.FromRawData(certBytes); | ||
| IReadOnlyList<string> applicationUris = X509Utils.GetApplicationUrisFromCertificate(cert); | ||
| if (applicationUris.Count != 1 || | ||
| !string.Equals(applicationUris[0], server.ServerUri, StringComparison.Ordinal)) |
Contributor
There was a problem hiding this comment.
why only check the first applicationUri, this is not spec conformant, ApplicationUri can be at any position in the cert.
This was referenced Jul 19, 2026
Merged
Collaborator
Author
|
PR #4021 has been split into independent, reviewable draft PRs. Each replacement targets
The umbrella PR remains closed; review and merge the replacement drafts independently. |
marcschier
added a commit
that referenced
this pull request
Jul 21, 2026
# Description Stream socket semantics permit a successful vectored send to complete after fewer bytes than requested. The previous one-shot send treated that legal partial completion as a closed connection and left the remaining bytes unsent, risking truncation of an OPC 10000-6 UA-TCP message chunk. The transport now continues sending while advancing across copied buffer segments, without mutating the caller's `BufferCollection`, and rejects zero-byte or impossible over-reported sends. ## Validation - `TcpByteTransportTests`: net10.0 (10 passed), net48 (10 passed). - `git diff --check` passed. ## Related Issues - Split from #4021. ## Checklist _Only verified claims are checked._ - [ ] 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. - [x] 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 21, 2026
…OPC 10000-4 security) (#4043) ## Failure MCP auto-accept mutated the shared application certificate manager and accepted every certificate error, so one permissive connection could affect concurrent or later strict sessions. The durable-subscription sample also serialized complete user identity tokens, including user-name passwords and issued bearer tokens. ## Fix - Isolate auto-accept certificate managers/configurations per connection and keep their lifetime with the managed session. - Auto-accept only `BadCertificateUntrusted`; retain strict validation for all other certificate errors and shared sessions. - Version the durable store and reject the previous unsafe format. - Remove user-name passwords before persistence and reject issued-token subscriptions rather than storing bearer credentials. - Add focused tools tests plus solution, project, and durable-subscription docs. ## Reference - OPC 10000-4 Session identity-token and application-certificate security. - Split from #4021. ## Tests - `Opc.Ua.Tools.Tests` Release: 7 passed on net10.0; 4 passed on net48. - `Quickstarts.Servers` Release builds passed on net10.0 and net48 with zero warnings/errors. - `Opc.Ua.Mcp` Release net10.0 build passed with zero warnings/errors. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 21, 2026
## Failure Lease takeover trusted another replica's wall-clock `RenewTime`, so clock skew could steal a live Lease. A hung renewal could also leave local leadership asserted, and a stale completion or notification could resurrect leadership after fencing or disposal. ## Fix - Measure unchanged foreign `resourceVersion` age with local monotonic time. - Serialize and bound API attempts, and self-fence with a lease-duration watchdog. - Reject stale completions and suppress out-of-order leadership notifications. ## Standard This is a Kubernetes coordination extension to OPC 10000-4 §6.6 redundancy. Kubernetes Lease ownership is now evaluated from stable resource observations rather than untrusted peer wall clocks. ## Tests - `dotnet test tests\Opc.Ua.Redundancy.Kubernetes.Tests\Opc.Ua.Redundancy.Kubernetes.Tests.csproj -c Release --nologo -m:1` — 97 passed on net8.0 and 97 passed on net10.0 - Review follow-up: `KubernetesLeaseLeaderElectionTests` — 22 passed on net8.0. ## Split Split from #4021. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 21, 2026
…147) (#4044) # Description This draft is the focused PubSub/UDP/DTLS security split from #4021. The changes: - enforce replay commit ordering: reject UADP messages that fail signature or required-security-mode checks before mutating replay state, and advance the DTLS replay window only after record authentication succeeds; - scope UADP sequence windows by `(PublisherId, WriterGroupId, SecurityTokenId)` while retaining global per-key nonce uniqueness across all publisher and writer-group scopes; - serialize outbound DTLS record sequence allocation and cryptographic use, reconstruct truncated DTLS sequence numbers, and reject epoch sequence exhaustion; - keep unsecured Action opt-in on each registered responder instead of promoting it to connection-wide policy; - pin connection-ID-less DTLS associations to the endpoint authenticated by the handshake and drop records from other peers before replay processing. These changes implement the replay and nonce requirements in [OPC 10000-14 §7.2.4.4.2](https://reference.opcfoundation.org/Core/Part14/v105/docs/7.2.4.4.2) and the DTLS security considerations in [RFC 9147 §11](https://www.rfc-editor.org/rfc/rfc9147.html#section-11). ## Validation - `git diff --check origin/master...HEAD` — passed. - `dotnet test tests\Opc.Ua.PubSub.Tests\Opc.Ua.PubSub.Tests.csproj --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0` — 1,324 passed, 1 skipped. - `dotnet test tests\Opc.Ua.PubSub.Udp.Tests\Opc.Ua.PubSub.Udp.Tests.csproj --configuration Release --framework net10.0 -p:CustomTestTarget=net10.0` — 227 passed. - `dotnet test tests\Opc.Ua.PubSub.Tests\Opc.Ua.PubSub.Tests.csproj --configuration Release --framework net48 -p:CustomTestTarget=net48` — 1,324 passed, 1 skipped. - `dotnet test tests\Opc.Ua.PubSub.Udp.Tests\Opc.Ua.PubSub.Udp.Tests.csproj --configuration Release --framework net48 -p:CustomTestTarget=net48` — 175 passed, 4 skipped. Direct regression coverage includes forged-record replay poisoning, scoped sequence windows and nonce reuse, concurrent sequence serialization, sequence exhaustion, per-responder unsecured Action isolation, and authenticated-peer pinning. ## Related Issues - Split from #4021. ## Checklist - [ ] 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. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 21, 2026
## Failure Durable Subscription restore depended on a process-local cache, while full snapshots were written record-by-record under shared mutable keys. A restart or concurrent writer could therefore expose stale, mixed, or partially committed Subscription state. ## Fix - Write each snapshot to an immutable generation and atomically publish a protected manifest. - Restore only the committed generation, validate record identities and completeness, and retain legacy-key fallback. - Replace the cache only after the full snapshot is authenticated and decoded, including explicit empty snapshots. ## Standard OPC 10000-4 §6.6.2.4.4 and §6.6.2.4.5.5 require durable Subscription and retransmission state to remain coherent across redundant Server failover and restart. ## Tests - `dotnet test tests\Opc.Ua.Redundancy.Server.Tests\Opc.Ua.Redundancy.Server.Tests.csproj -f net10.0 --no-restore --nologo --verbosity minimal` — 542 passed - `CustomTestTarget=net48 dotnet test tests\Opc.Ua.Redundancy.Server.Tests\Opc.Ua.Redundancy.Server.Tests.csproj --no-restore --nologo --verbosity minimal -m:1` — 542 passed - Review follow-up: `SharedKeyValueSubscriptionStoreTests` — 39 passed on net10.0 and 39 passed on net48. ## Split Split from #4021. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 21, 2026
…80) (#4027) # Description This draft splits the certificate-related changes from #4021, sourced only from commits `31350bf3b` and `1aed3b7ed`. - Enforces OPC UA HTTPS trust-list semantics for every presented client certificate, even when platform TLS/OS trust reports no policy errors. This prevents OS trust from bypassing the configured UA HTTPS trust list. - Passes the full TLS-supplied certificate chain to the UA validator and selects `TrustListIdentifier.Https`, preserving intermediates for application-level chain validation. - Uses the signature generator's exact `AlgorithmIdentifier` in both the CRL `TBSCertList` and outer `CertificateList`. RFC 5280 §5.1.1 requires these inner and outer identifiers to match; independently encoding the inner value could disagree with the actual signature algorithm. ## Validation - HTTPS certificate-validation tests: net10.0 (6 passed), net48 (6 passed). - CRL tests: net10.0 (95 passed), net48 (95 passed). - `Opc.Ua.Bindings.Https` and `Opc.Ua.Security.Certificates`: netstandard2.1 builds succeeded with 0 warnings and 0 errors. - `git diff --check` passed. ## Related Issues - Split from #4021. ## Checklist _Only verified claims are checked._ - [ ] 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. - [x] 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 22, 2026
…4) (#4052) # Description This PR splits the LDS registration-validation changes from #4021. ## Specification [OPC 10000-4 v1.05.07 §5.5.5.1](https://reference.opcfoundation.org/specs/OPC-10000-4/v1.05.07/5.5.5.1) requires `RegisterServer` to use a SecureChannel that supports Client authentication and requires a Discovery Server to reject a registration when `serverUri` does not match the `applicationUri` in the Server Certificate used for that SecureChannel. ## Failure LDS registration validation previously skipped the ApplicationUri check when the SecureChannel had no Client certificate, accepted a certificate with no ApplicationUri, and logged malformed-certificate parsing errors before continuing with a successful registration. ## Fix - Require a SecureChannel Client certificate for registration. - Parse the certificate fail closed and return `BadCertificateInvalid` when it is malformed. - Require at least one certificate ApplicationUri and compare every URI entry to `ServerUri` with ordinal equality. - Return `BadServerUriInvalid` when no certificate ApplicationUri exactly matches `ServerUri`. ## Validation - Full `Opc.Ua.Lds.Tests`: net10.0 (171 passed, 3 skipped), net48 (171 passed, 3 skipped). - `git diff --check` passed. ## Related Issues - Split from #4021. ## Checklist _Only verified claims are checked._ - [ ] 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. - [x] 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 22, 2026
## Failure The encoder fuzz host relied on NUnit or corpus-tool setup to assign `FuzzableCode.MessageContext`. Direct afl-fuzz/libFuzzer startup could therefore reach encoder targets without an initialized OPC UA message context. ## Fix - Initialize one process-lifetime `ServiceMessageContext` with null logging. - Make the context immutable and remove test/tool reassignment. - Exercise the same context from regression tests and the standalone fuzz host. ## Reference - OPC 10000-6 encoding validation. - Split from #4021. ## Tests - `dotnet test fuzzing\Opc.Ua.Encoders.Fuzz.Tests\Opc.Ua.Encoders.Fuzz.Tests.csproj -c Release -f net10.0 --nologo` (4,569 passed) - `dotnet run --project fuzzing\Opc.Ua.Encoders.Fuzz\Opc.Ua.Encoders.Fuzz.csproj -c Release --` (standalone smoke passed) --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 22, 2026
## Failure UA0003 unwrapped `Nullable<T>` before classifying OPC UA built-in structs. That reported valid nullable `== null` and `!= null` checks and offered fixes whose `.IsNull` semantics differ from nullable-value absence. ## Fix - Leave `System.Nullable<T>` comparisons unchanged. - Continue reporting direct null comparisons on OPC UA structs. - Add coverage for nullable `NodeId` and `LocalizedText` operand orderings. ## Reference - OPC UA .NET 2.0 migration semantics. - Split from #4021. ## Tests - `dotnet test tests\Opc.Ua.MigrationAnalyzer.Tests\Opc.Ua.MigrationAnalyzer.Tests.csproj -c Release -f net10.0 --nologo` (121 passed) - `dotnet test tests\Opc.Ua.MigrationAnalyzer.Tests\Opc.Ua.MigrationAnalyzer.Tests.csproj -c Release -f net48 --nologo` (121 passed) --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 22, 2026
…kaging) (#4041) ## Failure The source-generator projects disabled normal build-output packing without adding their analyzer assemblies and private runtime closure back to the NuGet payload. Debug and Release packages also shared IDs, and both preview workflows passed the unsupported `--configuration` option to `dotnet restore`. ## Fix - Package each generator and its private runtime closure under `analyzers/dotnet/cs`, excluding Roslyn host assemblies and package dependencies. - Preserve Roslyn compile references for project-reference test consumers. - Give Debug generator packages `.Debug` IDs. - Validate package contents and clean net10 consumers in both preview pipelines. - Remove invalid restore configuration arguments from both preview workflows. ## Reference - Roslyn analyzer packaging and MSBuild restore semantics; no normative OPC UA change. - Split from #4021. ## Tests - `Opc.Ua.SourceGeneration.Tests`: 68 passed on net10.0 and 68 on net48. - `Opc.Ua.SourceGeneration.Stack.Tests`: 90 passed on net10.0 and 87 on net48. - Generated `preview-pack.slnx` and restored it with `--disable-parallel`. - Packed both Release generators with `--no-build`; package-content checks and two clean net10 consumer builds passed with zero warnings/errors. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 22, 2026
…-100) (#4053) # Description This PR splits the DI software-package containment changes from #4021. ## Specification - [OPC 10000-100 v1.05.0 §3.1.14](https://reference.opcfoundation.org/specs/OPC-10000-100/v1.05.0/3.1.14) defines a Software Package as a single file containing the software-update data. - [OPC 10000-100 v1.05.0 §8.7.1](https://reference.opcfoundation.org/specs/OPC-10000-100/v1.05.0/8.7.1) defines the standard Software Package as one ZIP file. - [OPC 10000-100 v1.05.0 §8.7.3](https://reference.opcfoundation.org/specs/OPC-10000-100/v1.05.0/8.7.3) defines package metadata as the identity of that Software Package. The provider-backed package cache must therefore keep each package payload and its metadata together beneath the configured store root. ## Failure `FileSystemPackageStore` rejected path separators in package identifiers but accepted `.` and `..`. It also accepted configured roots containing dot segments. Provider path normalization could therefore resolve a package operation outside the configured package-store root. ## Fix - Canonicalize configured provider-relative roots to forward-slash absolute provider paths. - Reject dot segments in both package identifiers and configured roots. - Re-canonicalize every constructed package path and require it to be a strict descendant of the configured root before calling the file-system provider. - Preserve valid mixed-separator roots by normalizing them deterministically. ## Validation - Full `Opc.Ua.Di.Tests`: net10.0 (288 passed), net48 (288 passed). - Review follow-up `PackageStoreTests`: net10.0 (15 passed), net48 (15 passed). - `git diff --check` passed. ## Related Issues - Split from #4021. ## Checklist _Only verified claims are checked._ - [ ] 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. - [x] 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. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 22, 2026
# Description This focused draft extracts the FileType handle ownership fix from #4021. It contains only the server FileSystem handle implementation, focused tests, and FileSystem client documentation. ## Failure `FileHandle` issued predictable per-file identifiers without recording the owning Session. A different Session that obtained a handle could read, write, seek, or close the stream, and open streams survived Session closure. ## Fix - Generate non-zero opaque handles and bind every open stream to the Session that called `Open`. - Require a valid Session for `FileType.Open`, `CreateFile` when it requests an open handle, and all subsequent handle operations. - Reject a handle presented by another Session. - Close only the departing Session's handles from `FileSystemNodeManager.SessionClosingAsync`. - Document the Session ownership and lifetime. OPC 10000-5 Section 6.2.5.2, Table 27 states that a `fileHandle` is valid only for the Session used to call `Open` and that the Server shall automatically close the file when that Session closes. ## Tests - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net10.0 --filter "FullyQualifiedName~FileHandleTests|FullyQualifiedName~FileObjectStateTests|FullyQualifiedName~DirectoryObjectStateTests" --no-restore`: 71 passed. - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net48 --filter "FullyQualifiedName~FileHandleTests|FullyQualifiedName~FileObjectStateTests|FullyQualifiedName~DirectoryObjectStateTests" --no-restore`: 71 passed. Both builds emitted the same existing CA1873 warnings in unchanged `Opc.Ua.Client` files; the changed files produced no warnings. ## Related Issues - Focused split from #4021 (closed umbrella draft); no separate issue. ## Checklist - [ ] 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. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 23, 2026
…4046) ## Summary - Fixes missing `AddNode`, `DeleteNode`, `AddReference`, and `RemoveReference` authorization before NodeManagement mutations. - Enforces applicable namespace and Node `AccessRestrictions`. - Distinguishes local targets from remote `targetServerUri`, server-index, and namespace-URI targets. - Validates both local reference endpoints and uses the actual reciprocal `NodeClass` for cross-NodeManager mutations. - Adds bounded, cancellation-safe compensation when reciprocal add/delete mutations fail. ## Specification - OPC 10000-4 §§5.8.2-5.8.3 - OPC 10000-18 role-based access control This is split from #4021. ## Tests Focused Release validation passed on both `net10.0` and `net48`: - `dotnet build tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f <TFM> -p:CustomTestTarget=<TFM>` - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f <TFM> -p:CustomTestTarget=<TFM> --no-build --no-restore --filter FullyQualifiedName~Opc.Ua.Server.Tests.MasterNodeManagerNodeManagementTests` - 55 tests passed per TFM, including `AddNodesDeniedAddNodePermissionDoesNotMutateAsync`, `AddNodesDeniedParentAddReferencePermissionDoesNotMutateAsync`, `DeleteNodesDeniedDeleteNodePermissionDoesNotMutateAsync`, `AddReferencesGrantedPermissionsMutateBothSidesAsync`, `AddReferencesTargetServerUriSkipsLocalTargetProcessingAsync`, `AddReferencesInverseFailureRollsBackSourceAsync`, `AddReferencesCancellationUsesIndependentRollbackToken`, `DeleteReferencesRemoteTargetSkipsReciprocalProcessingAsync`, `DeleteReferencesInverseFailureRestoresSourceAsync`, and `DeleteReferencesUnexpectedFailureRestoresSourceWithIndependentToken`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 23, 2026
## Failure Concurrent active-Session refreshes observed the coordinator outside the facade lock and then applied results in completion order. An older refresh could overwrite a newer Session, while a non-null to non-null swap retained the already-completed waiter and routed later async calls to the stale Session. ## Fix - Assign every refresh invocation a generation before accessing the Session and reject earlier-started refreshes after a later refresh has applied. - Reset the active-Session completion source for live Session replacements. - Cover concurrent stale refreshes and async calls immediately after a live swap. ## Standard OPC 10000-4 §6.6 redundant-client continuity requires the stable facade to route service calls through the current active Session during replica and Session transitions. ## Tests - `dotnet test tests\Opc.Ua.Redundancy.Client.Tests\Opc.Ua.Redundancy.Client.Tests.csproj -f net10.0 --no-restore --nologo --verbosity minimal` — 122 passed - `CustomTestTarget=net48 dotnet test tests\Opc.Ua.Redundancy.Client.Tests\Opc.Ua.Redundancy.Client.Tests.csproj --no-restore --nologo --verbosity minimal -m:1` — 122 passed - Review follow-up: `RedundantClientSessionTests` — 7 passed on net10.0 and 7 passed on net48. ## Split Split from #4021. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 23, 2026
# Description This focused change extracts the `MaxNotificationsPerPublish` conformance fix from #4021. It contains only `Subscription.cs`, `SubscriptionManager.cs`, and focused `SubscriptionTests`. ## Failure OPC UA defines zero as no notification limit. The message builder compared the queued count directly with zero, so an effective zero limit emitted no event or data-change notifications and left the queues pending. Separately, the revision logic treated a configured Server maximum of zero as a numeric cap, revising a non-zero Client request to zero and silently removing the Client's requested limit. ## Fix - Treat an effective zero limit as unlimited while draining event and data-change queues. - When the Client requests zero, use the Server's configured maximum, which may itself remain zero/unlimited. - When the Server maximum is zero, preserve a non-zero Client request, including `uint.MaxValue`. - When the Server maximum is finite, cap zero, `uint.MaxValue`, and larger finite Client requests to that maximum. - Cover the same revision matrix through both CreateSubscription and ModifySubscription. This implements the zero-means-unlimited semantics in OPC 10000-4 v1.05.07 Sections 5.14.2.2 and 5.14.3.2. Those Service responses do not contain a `revisedMaxNotificationsPerPublish` field; the effective value is retained by the Server-side Subscription and its diagnostics. ## Tests - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net10.0 --filter "FullyQualifiedName~Opc.Ua.Server.Tests.SubscriptionTests" --no-restore`: 25 passed. - `dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net48 --filter "FullyQualifiedName~Opc.Ua.Server.Tests.SubscriptionTests" --no-restore`: 25 passed. The regression tests cover zero, finite, and `uint.MaxValue` Client requests against unlimited and finite Server limits for both CreateSubscription and ModifySubscription. They also verify that zero drains both event and data-change queues. ## Related Issues - Focused split from #4021 (closed umbrella draft); no separate issue. ## Checklist - [ ] 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. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] 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. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 24, 2026
….5-8.5.7) (#4054) # Description This PR splits the GDS KeyCredential changes from #4021. ## Specification - [OPC 10000-12 v1.05.07 §8.5.5](https://reference.opcfoundation.org/specs/OPC-10000-12/v1.05.07/8.5.5) requires `StartRequest` to use an encrypted SecureChannel and a Client with the `KeyCredentialAdmin` Role, `ApplicationAdmin` Privilege, or `ApplicationSelfAdmin` Privilege. The `ApplicationUri` must uniquely identify an application known to the GDS. - [OPC 10000-12 v1.05.07 §8.5.6](https://reference.opcfoundation.org/specs/OPC-10000-12/v1.05.07/8.5.6) applies the same authorization requirements, requires `FinishRequest` to use the same SecureChannel Certificate as `StartRequest`, and defines `Bad_RequestNotComplete` for a pending request. - [OPC 10000-12 v1.05.07 §8.5.7](https://reference.opcfoundation.org/specs/OPC-10000-12/v1.05.07/8.5.7) applies the same authorization requirements to `Revoke` and defines `Bad_InvalidArgument` for an unknown credential. ## Failure The KeyCredential methods previously verified only an authenticated encrypted SecureChannel. They did not enforce the specification's administrative Role or application-scoped Privileges, persist immutable application ownership, or bind `FinishRequest` to the initiating Client certificate. A pending request returned `BadNothingToDo`, invalid ownership lookups used non-conformant status codes, and failure audit text could expose exception details. ## Fix - Publish and enforce `KeyCredentialAdmin`, `ApplicationAdmin`, and `ApplicationSelfAdmin` permissions, including application scoping. - Resolve `ApplicationUri` to exactly one registered application and persist its immutable `ApplicationId`. - Bind requests to the initiating SecureChannel certificate with a SHA-256 fingerprint and compare it in fixed time when finishing. - Atomically re-check request and credential ownership, and fail closed for stores that cannot enforce the new ownership contract. - Return `BadRequestNotComplete` for pending requests and conformant invalid-argument results for unknown request or credential identifiers. - Emit success and failure audit events while redacting KeyCredential exception details. ## Validation - Full `Opc.Ua.Gds.Tests`: net10.0 (1053 passed, 42 skipped), net48 (858 passed, 237 skipped). - Review follow-up `KeyCredentialRequestStoreTests`: net10.0 (16 passed), net48 (16 passed). - `git diff --check` passed. ## Related Issues - Split from #4021. ## Checklist _Only verified claims are checked._ - [ ] 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. - [x] 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. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
added a commit
that referenced
this pull request
Jul 28, 2026
…§5.7.3.1) (#4050) # Description This draft isolates the Session/ActivateSession, reconnect, and distributed Session security work split from #4021. ### Failure - `ActivateSession` could transfer a Session to a new SecureChannel without consistently enforcing the original client certificate, MessageSecurityMode, SecurityPolicy, and exact OPC `ClientUserId` semantics. - Missing or legacy distributed Session security state did not provide enough information to validate transfer safely. - Malformed or reused nonces, concurrent activation, and cancellation could leave activation state inconsistent. - Client reconnect could treat recoverable Session-loss/security responses as fatal, reuse a Session after client-certificate rotation, or report a managed channel ready before Session recreation completed. ### Fix - Serialize activation, consume server nonces once, validate application signatures and user tokens asynchronously, and enforce transfer continuity against the original channel and authenticated identity. - Persist versioned transfer-security state for distributed Sessions and fail closed when certificate, identity, activation, or nonce state is absent or invalid. - Validate server signatures/nonces on the client, recreate Sessions for certificate rotation and recoverable activation failures, and await participant recreation before transitioning the managed channel to `Ready`. - Make the restart recovery integration test use bounded reconnect cycles instead of a fixed startup delay. ### Specification - OPC 10000-4 §5.7.2.2 — CreateSession signatures and nonces. - OPC 10000-4 §5.7.3.1 — ActivateSession identity continuity and SecureChannel transfer. - OPC 10000-4 §6.1.8 — application nonce generation, validation, and reuse requirements. ### Validation - `git diff --check` - Release `net10.0`: `Opc.Ua.Redundancy.Server.Tests` 567/567 plus the targeted server Session, Subscription transfer, ClientUserId, client reconnect and certificate rotation suites pass. - Release `net48`: the same redundancy suite (567/567) and the targeted Session/Subscription suites pass. - Full `Opc.Ua.Server.Tests` run on `net10.0`: 3633/3634. The single failure, `ServerFluentApiHostingTests.ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, reproduces identically on the unmodified commit and is a local certificate-store path issue rather than a regression. These were targeted validation runs; no long-soak result is claimed.
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
2 participants
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
This draft addresses the high-confidence security, protocol, reliability, and tooling findings from a repository-wide review.
The changes:
ActivateSessionchannel transfer conform to OPC 10000-4 §5.7.3.1, including certificate, ClientUserId, SecurityPolicy, SecurityMode, anonymous Sign-only, nonce, and signature requirements;The implementation is split into focused commits with direct positive, negative, concurrency, restart, and cross-TFM regression tests.
The final transfer-subscription and certificate-rotation/reconnect follow-up is integrated. Its targeted net10.0/net48 suites pass, including transfer, managed reconnect, channel, client Session, and server Session coverage; the net10 stress certificate-rotation and shorter chaos/outage tests also pass.
This remains a draft while the complete final
UA.slnxmatrix is rerun. Package and clean-consumer validation pass, all phase-targeted suites pass on their supported TFMs, and full builds complete. The remaining validation work includes the net48 issued-token failures discovered by the pre-follow-up full run and the full 60-minute chaos soak.Related Issues
Checklist