## Proposed changes
Fixes#4729 on master378 (1.5.378.x), the branch the issue was reported
against. The master counterpart is #4736.
If a server is stopped while requests are queued, a request could be
completed twice. The second completion landed on a
`ServiceResponsePooledValueTaskSource` that had already gone back to the
process-wide pool and been rented by another request. That request,
possibly on another server in the same process, then received a foreign
response, which showed up as `InvalidCastException` or
`BadSecurityChecksFailed`.
### Changes
- **Leased completion (root defense, same as #4736):** each rental of
the pooled source gets a lease number. `EndpointIncomingRequest`
captures it at construction and completes through `TrySetResult(lease,
response)`, a single CAS on the pending lease. Only the first completion
of the current rental is accepted, and a late completion can never reach
the next owner.
- **`ServerBase.RequestQueue`:**
- `Dispose` drains the queue with `TryDequeue` instead of completing a
`ToList()` snapshot. Each request is now removed by exactly one party,
either a worker or `Dispose`. This replaces the `m_queue.Clear()` that
only compiled under `NETSTANDARD2_1_OR_GREATER`.
- The stop flag is now an `int`. `Dispose` sets it with
`Interlocked.Exchange`, which is a full fence, and is now idempotent.
- `ScheduleIncomingRequest` re-checks the stop flag after enqueuing and
drains the queue instead of letting `ObjectDisposedException` from
`Release()` escape. Previously that exception made `ProcessAsync`
complete the already-queued request a second time.
- A worker that dequeues a request after stop completes it with
`BadServerHalted` instead of running it. Workers also treat a disposed
semaphore as shutdown.
- Completions from the queue go through a helper that catches and logs
exceptions.
## Tests
- `RequestQueueShutdownTests.DisposeCompletesQueuedRequestsOnceAsync`:
fails on the current master378 (a request is completed twice) and passes
with the fix.
- `RequestQueueShutdownTests.ScheduleAfterDisposeCompletesOnce`.
-
`RequestQueueShutdownTests.ConcurrentScheduleAndDisposeCompletesEachRequestOnceAsync`:
a stress guard. It did not reproduce the old bug locally.
- `ServiceResponsePooledValueTaskSourceTests`: a duplicate completion is
ignored, and a stale completion after the source is recycled does not
complete the next rental.
- Local net10.0 runs: `Opc.Ua.Core.Tests` Stack.* 1496 passed, 0 failed;
`Opc.Ua.Server.Tests` 513 passed, 0 failed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Backport of the secure channel fix from #4698 to `master378`.
## Problem (OPC 10000-4 §5.6.2.1)
1. **Server:** after a token renew, the server keeps securing messages
with the old token until the client sends something with the new one.
That activation only happened on *receive*. A client that sends nothing,
for example one whose Publish requests are all outstanding, kept getting
responses secured with the old token after it expired.
2. **Client:** the client rejected a response secured with a
just-expired token right away (`BadTcpSecureChannelUnknown`) and shut
the channel down. The spec says to accept such messages for up to 25 %
of the token lifetime.
Together these can drop the subscription channel, time out the session
and abandon the subscription. Whether it happens depends on timing.
## Fix
- `UaSCBinaryChannel.ActivateRenewedTokenIfDue()` activates a pending
renewed token once the current one passes `TokenActivationPeriod` (95
%). The receive path now uses it, and `TcpServerChannel.SendResponse`
calls it before securing a response.
- `ReadSymmetricMessage` accepts responses (not requests) secured with
an expired token for `TcpMessageLimits.TokenExpiryGracePeriod` (25 %) of
its lifetime.
### Differences from #4698
- Ported to this branch's `HiResClock`-based `ChannelToken` (`Expired` /
`ActivationRequired`). There is no `TimeProvider` here, so the new
overload is `IsExpired(double gracePeriod)`.
- The `FakeTimeProvider`-based `TcpReconnectKeyAgreementTests` fixture
does not exist on this branch. It is replaced by
`ChannelTokenExpiryTests`, which checks the grace-period and activation
thresholds directly.
- The CI changes (soak/stress labels, `assert-tests-ran.ps1`) are not
backported. `stability-test.yml` on this branch still targets
`Opc.Ua.Client.Tests`, which is correct here.
## Test plan
- [x] `Opc.Ua.Core.Tests` `Stack.Transport` (net10.0): 58/58 passed,
including the 4 new tests
- [x] `Opc.Ua.Core.Tests` builds for net48 with 0 warnings
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## Proposed changes
Three spec alignment fixes for the 1.5.378 line.
- **Session AuthenticationToken** (`SessionManager`): CreateSession now
always assigns a random 32 byte opaque AuthenticationToken. Previously a
numeric, incrementing token was used for sessions created on a secured
endpoint. OPC 10000-6 7.4.1 requires a random token of at least 32 bytes
for HTTPS, and a single format keeps all transports consistent.
- **OpenSecureChannel response policy** (`UaSCBinaryClientChannel`,
`UaSCBinaryChannel.Asymmetric`): a client channel opened with a
SecurityPolicy other than None now requires the OpenSecureChannel
response to use the same SecurityPolicy and SecurityMode and to carry
the server certificate. Previously the response could switch the channel
to SecurityPolicy None (OPC 10000-4 5.6.2.1, OPC 10000-6 6.7.2.3).
Channels configured for None are unchanged.
- **Identity token decryption errors** (`Session`, `RsaUtils`): every
failure while decrypting a user identity token is now reported as
`Bad_IdentityTokenInvalid` without an inner result, as OPC 10000-4
7.40.2.1 specifies; the cause is logged on the server instead. An
invalid length prefix in RSA-decrypted data now fails the same way as
other decryption errors instead of `Bad_EndOfStream`.
## Behaviour changes
- AuthenticationTokens on secured endpoints are ByteString (opaque)
NodeIds instead of numeric NodeIds.
- ActivateSession with an undecryptable identity token returns
`Bad_IdentityTokenInvalid` in all cases, also when the client requests
inner diagnostics.
## Tests
- `ClientOpenSecureChannelDowngradeTests` (Core): secured client rejects
a None OpenSecureChannel response; None client still accepts it.
- `RsaUtilsDecryptTests` (Core): round trip; invalid length prefixes
fail with the same exception type as invalid padding.
- `SessionSecurityRegressionTests` (Server): opaque 32 byte tokens on a
shared secured channel id; uniform ActivateSession status for identity
token decryption failures.
Local results (net10.0): Core 7938 passed / 0 failed, Server 511 passed
/ 0 failed, Client 1063 passed / 3 failed. The three Client failures
(`TransferSubscriptionNativeAsync` x2, `ShortHaulStabilityTestAsync`)
pass when run in isolation on both this branch and unmodified master378.
The new tests also pass on net48 (Release).
## Types of changes
- [x] Bugfix (non-breaking change which fixes an issue)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Minimal backports to the 1.5.378 line of three findings from the codec
audit behind #4599 / #4602 (B1-1, X1-1, P1-1). All three can be reached
from untrusted input. The changes are kept small, and the performance of
legitimate messages is unchanged.
## Changes
**B1-1: nested array preallocation (`BinaryDecoder`)**
Variant, DataValue, ExtensionObject and encodeable arrays were allocated
at the full length prefix before any element was read. Arrays nested in
the first element of their parent all see the same remaining message, so
a chain of them multiplied the allocation: about 1 KB of input in
`RequestHeader.AdditionalHeader` preallocated about 200 MB before
authentication.
- These arrays now use the #4602 budget. An array is allocated at full
length only while all such arrays still being read take at most 8 bytes
per remaining message byte. Otherwise it starts at 16 KB and grows as
elements are read.
- The reservation is released in a `finally`.
- Leaf arrays (numbers, strings, NodeIds, …) are unchanged and stay
bounded by MaxArrayLength.
**X1-1: XML element depth (`XmlDecoder`, `BinaryDecoder`,
`JsonDecoder`)**
The depth of XmlElement values and XML ExtensionObject bodies was
unbounded, and the `InnerXml` setter and `CloneNode(true)` recurse once
per level. A known-type XML body of about 200 KB inside a binary message
terminated the process with a stack overflow.
- Before any network XML is loaded into a DOM, it is now checked with a
streaming reader. XML nested deeper than `MaxEncodingNestingLevels`
(default 200, the same limit master uses) is rejected with
`BadEncodingLimitsExceeded`.
- Master's node-by-node copier is not ported.
**P1-1: inbound DataSetMetaData (`UaPubSubConnection`)**
Received metadata is not authenticated, yet it replaced the metadata of
every reader with a matching DataSetWriterId, whichever publisher sent
it.
- It now only updates readers whose PublisherId filter accepts the
sender, using the same rule as data messages. A reader without a
PublisherId still accepts any publisher.
- Master's registry cap is not needed, because 378 has no registry.
## Verification (net10.0 unless noted)
| Check | Result |
|---|---|
| Nested Variant-array chain, 995 bytes | 199.2 MB → 3.3 MB allocated |
| Known-type XML body 30 000 levels deep inside binary | process crash →
`BadEncodingLimitsExceeded` |
| `Opc.Ua.Types.Tests` (incl. new `DecoderNestingLimitTests`) | 327 /
327; new tests also 9 / 9 on net48 |
| `Opc.Ua.Encoders.Fuzz.Tests` (crash/timeout/slow corpus) | 150 / 150 |
| `Opc.Ua.Core.Tests` (Encoders + Types) | 6 327 passed, 0 failed |
| `Opc.Ua.PubSub.Tests` (without broker-dependent MQTT/UDP) + new
`MetaDataPublisherFilterTests` | 617 + 4 passed, 13 skipped |
| Types / Core / PubSub build, all TFMs | 0 warnings |
**Performance vs. db39629d2:**
- Test cases: decoding `Variant[100000]`, `Variant(Variant[])`,
`DataValue[10000]`, `ReadValueId[20000]` and `ExtensionObject[20000]`.
- Allocations are byte-identical to the baseline.
- Median times are within run-to-run noise over two interleaved rounds.
- The only new cost is one extra streaming parse for XmlElement values.
## Notes for review
- If a deployment raises `MaxEncodingNestingLevels` well above the
default, the allowed XML depth rises with it. Master behaves the same
way.
- No public API changes. The XML depth helper is internal to
`Opc.Ua.Types`, and `JsonDecoder` in `Opc.Ua.Core` has a private copy.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
# Description
Backport the essential incomplete-message memory protection from
57c0a32ed without changing public/protected APIs or configuration
schemas.
- Introduce a server-wide budget for retained intermediate chunks, with
reliable buffer cleanup on completion, rejection, and channel shutdown.
- Restrict channels without activated sessions to a 50% total-usage
threshold. Track live session membership so full-budget eligibility
follows activation, transfer, closure, and expiration cleanup.
_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._
- [x] 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.
- [ ] 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.
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Backport of
https://github.com/OPCFoundation/UA-.NETStandard/commit/9883c41e48992a7cbc7347f9b2f1e50c8a3c1df6
(#4055) from `master` to `master378`.
## Description
`ValidateAsync(ApplicationType.Server)` accepted a null
`TransportQuotas` without error, but the transport/secure-channel layers
assume it is non-null, so `ApplicationInstance.StartAsync` later threw a
`NullReferenceException`. The fluent builder path
(`ApplicationInstance.Build`) already always initializes
`TransportQuotas = new TransportQuotas()`; manually-assembled
configurations could leave it null.
Changes:
- **`ApplicationConfiguration.ValidateAsync`**: default
`TransportQuotas` when null, aligning the manual-construction path with
the builder path.
```csharp
// Transport/secure-channel layers rely on this being non-null during
bring-up.
TransportQuotas ??= new TransportQuotas();
```
`TransportQuotas`'s default constructor yields complete, valid limits
(`DefaultEncodingLimits` / `TcpMessageLimits`), so minimal server
configs now start with sensible defaults instead of crashing.
- **Tests** (`ApplicationConfigurationTests`, new file on this branch):
`ValidateAsyncDefaultsTransportQuotasWhenNull`,
`ValidateAsyncKeepsExplicitTransportQuotas`, and
`ValidateAsyncDefaultsTransportQuotasWhenNullForClient`.
## Notes
- Ported to `master378`. The test fixture did not exist on this branch,
so it was added.
- Built `Opc.Ua.Core.Tests` (net8.0) with 0 warnings; the 3 new tests
pass.
## Checklist
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fixed timer leaks in ChannelAsyncOperation.EndAsync
## Proposed changes
- Added linked cancellation token source for delay task (runtimes <=
.NET 6.0)
- Added delay task cancellation (runtimes <= .NET 6.0)
## Related Issues
- Fixes#3668
## Types of changes
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] Enhancement (non-breaking change which adds functionality)
- [ ] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)
## Checklist
- [x] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [ ] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines.
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.
## Further comments
Fix already accepted in master branch, please add it to latest 1.5.378
version
---------
Co-authored-by: Suciu Mircea Adrian <Mircea-Adrian.Suciu@softing.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
## Proposed changes
`ApplicationConfiguration.TraceConfiguration` still existed as a live
configuration surface after `ITelemetryContext` was introduced, which
made the migration path unclear. This change makes that legacy surface
explicitly discoverable as transitional API and points consumers to the
telemetry-based replacement.
- **Public API guidance**
- Marks `ApplicationConfiguration.TraceConfiguration` as obsolete with a
message directing consumers to `ITelemetryContext` and `ILogger`-based
diagnostics.
- Keeps the property functional so existing configuration loading and
legacy trace application continue to work during migration.
- **Internal compatibility**
- Adds targeted `CS0618` suppressions at the remaining internal legacy
call sites that intentionally preserve `TraceConfiguration` behavior.
- Avoids broad warning suppression; only the compatibility path is
annotated.
- **Coverage**
- Adds a focused reflection-based test that verifies the property is
marked obsolete and that the migration message stays stable.
```csharp
[Obsolete("Use ITelemetryContext and ILogger-based diagnostics instead.")]
public TraceConfiguration TraceConfiguration { get; set; }
```
## Related Issues
## Types of changes
What types of changes does your code introduce?
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR._
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] Enhancement (non-breaking change which adds functionality)
- [x] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)
## Checklist
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR. If you're unsure about any of them, don't hesitate to
ask. We're here to help! This is simply a reminder of what we are going
to look for before merging your code._
- [ ] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines.
- [x] I fixed all introduced issues with CodeQL and LGTM.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.
## Further comments
This is intentionally narrow in scope: it clarifies the migration path
without removing the legacy trace pipeline yet. The change is limited to
surfacing the deprecation at the public configuration boundary and
isolating the remaining intentional legacy usages.
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes#4056
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This reverts commit 00519fa6dd.
# 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.
## 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
# Description
`ConditionState.m_branches` (a plain `Dictionary`) was read and written
from multiple threads without synchronization. A state change on a
background thread could mutate the dictionary while readers
(`GetRetainState`/`UpdateRetainState`, `ConditionRefresh`, `GetBranch`)
enumerated it, yielding torn entries and native access violations
(observed as `IsBranch` on a null `this`). This ports the master378 fix
to `master`, where the type lives at
`Stack/Opc.Ua.Core.Types/State/ConditionState.cs` and uses
`ByteString`/nullable refs.
- **Lock all branch access**: added a private `m_branchesLock` guarding
every read/write of `m_branches` (`CreateBranch`, `GetBranch`,
`ReplaceBranchEvent`, `RemoveBranchEvent`, `ClearBranches`,
`GetBranchCount`).
- **Snapshot enumeration**: `GetBranches()` now returns a copy taken
under the lock, so internal readers and
`AlarmHolder.GetBranchesForConditionRefresh` iterate a stable collection
while other threads mutate the live one. Public signature is unchanged.
- **Test**: added `ConcurrentBranchAccessIsThreadSafe`, stressing
concurrent create/clear writers against enumerate/count readers.
`lock` is used rather than `SemaphoreSlim` because all affected methods
are synchronous. Each `ConditionState` holds its own lock, so the
recursive `UpdateRetainState` traversal across branches neither contends
nor deadlocks. `ConcurrentDictionary` was rejected: it would change the
public return type and still leave compound read-modify-write sequences
(e.g. `ReplaceBranchEvent`) unprotected.
## Related Issues
## 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.
- [x] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Suciu Mircea Adrian <Mircea-Adrian.Suciu@softing.com>
# Description
Fixes#3896 on the `1.5.x` maintenance line. A server configured with
`SecurityConfiguration.SendCertificateChain = true`, whose application
instance certificate is issued by a CA placed in the **issuer store**
(`TrustedIssuerCertificates`), sent only the **leaf** certificate
instead of the full chain. Clients that rely on receiving the chain then
failed to connect.
## Root cause
Introduced in `1.5.375` by commit `f001eabcc` ("Add support for ECC
profiles", #2398), which moved chain loading into
`CertificateTypesProvider.LoadCertificateChainAsync`:
```csharp
if (await m_certificateValidator.GetIssuersAsync(certificate, issuers).ConfigureAwait(false))
{
for (int i = 0; i < issuers.Count; i++)
{
certificateChain.Add(issuers[i].Certificate);
}
}
```
`GetIssuersAsync` returns `isTrusted` — `true` only when the issuer is
found in the **trusted** store — but populates the `issuers` out-list
regardless (trusted → issuer → untrusted). When the CA lives in the
**issuer store** (the spec-recommended setup), the call returns `false`,
so the already-resolved issuers were discarded and the chain stayed
leaf-only.
This is a regression from `1.5.374`, where the issuers were appended
**unconditionally**.
## Fix
Append the resolved issuers unconditionally; the boolean trust result is
intentionally ignored — a server must send its chain even when the
issuing CA is in the issuer store rather than the trusted store.
```csharp
_ = await m_certificateValidator.GetIssuersAsync(certificate, issuers).ConfigureAwait(false);
for (int i = 0; i < issuers.Count; i++)
{
certificateChain.Add(issuers[i].Certificate);
}
```
The companion fix for the 2.0 line (`master`) is in a separate PR.
## Tests
New `CertificateTypesProviderTests`:
- `LoadCertificateChainResolvesIssuerFromIssuerStoreAsync`: the CA is
placed in the issuer store only — **not** the trusted store — and the
resolved chain is asserted to be `leaf + CA` (and the raw blob `leaf ||
CA` byte-for-byte). Confirmed this test **fails** with the original gate
and **passes** with the fix.
- `LoadCertificateChainFallsBackToLeafWhenIssuerMissingAsync`: when the
CA is in neither store, the chain is leaf-only without throwing.
Verified locally on **net48** and **net10.0** (new tests + existing
`VerifyIssuerChain*` validator tests pass). Build is clean.
## Related Issues
Fixes#3896.
## 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 (no public API change;
behavioural fix only).
- [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. (Ran the
certificate suites on net48 + net10.0; full-solution run pending CI.)
- [ ] 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: agent <agent@local>
Keep the generated children that belong on instances, and prune the
extra decoded children that should stay on the type definition only,
such as Optional/Mandatory placeholders and ones that do not have
Mandatory/Optional ModellingRules.
# Description
Currently child nodes that are type definition detailes such as ones
that do not have Mandatory/Optional ModellingRules are instantiated in
the runtime instance of such type definitions. This PR proposes to
correct this behaviour during NodeState creation as an alternative from
it being fixed by the ModelCompiler.
## Related Issues
- Fixes#3770
## 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.
* Test that OrderedListState does not instantiate OrderedObjectPlaceholder
* Add global NodeState placeholder scan with assembly context
* Prune optional placeholder children during instance creation while leaving type-definition trees untouched.
With the following call stack the EndAsync method is called with timeout=Int.MaxValue and a vaild cancellation token.
In this case after a successfull operation the Delay will not finish and stay forever.
So with each KeepAliveIntervall an task leak is left over after the operation has finished.
Opc.Ua.Core.dll!Opc.Ua.Bindings.ChannelAsyncOperation<int>.EndAsync(int timeout, bool throwOnError, System.Threading.CancellationToken ct) Line 263
Opc.Ua.Core.dll!Opc.Ua.Bindings.UaSCUaBinaryClientChannel.SendRequestAsync(Opc.Ua.IServiceRequest request, int timeout, System.Threading.CancellationToken ct) Line 338
Opc.Ua.Core.dll!Opc.Ua.Bindings.UaSCUaBinaryTransportChannel.SendRequestAsync(Opc.Ua.IServiceRequest request, System.Threading.CancellationToken ct) Line 259
Opc.Ua.Core.dll!Opc.Ua.SessionClient.ReadAsync(Opc.Ua.RequestHeader requestHeader, double maxAge, Opc.Ua.TimestampsToReturn timestampsToReturn, Opc.Ua.ReadValueIdCollection nodesToRead, System.Threading.CancellationToken ct) Line 6590
Opc.Ua.Client.dll!Opc.Ua.SessionClientBatched.ReadAsync(Opc.Ua.RequestHeader requestHeader, double maxAge, Opc.Ua.TimestampsToReturn timestampsToReturn, Opc.Ua.ReadValueIdCollection nodesToRead, System.Threading.CancellationToken ct) Line 376
Opc.Ua.Client.dll!Opc.Ua.Client.Session.OnSendKeepAliveAsync(Opc.Ua.ReadValueIdCollection nodesToRead, System.Threading.CancellationToken ct) Line 3076
* Fix memory leak from undisposed sockets and retained event handlers during server restart
When an OPC UA server is stopped and restarted in-process, TcpServerChannel
instances were never garbage collected because UaSCBinaryChannel.Dispose() did
not close the underlying socket. Pending ReceiveAsync operations kept the
channels pinned via SocketAsyncEngine, causing a steady heap growth per cycle.
Changes:
- Close socket in UaSCBinaryChannel.Dispose() to cancel pending async I/O
- Unsubscribe CertificateValidator.CertificateUpdate in StandardServer.Dispose()
- Unsubscribe ConfigurationWatcher.Changed in StandardServer.Dispose()
- Unsubscribe ConnectionStatusChanged from TransportListeners in ServerBase.Dispose()
Verified over 786 restart cycles with stable heap.
* Remove verbose comment
* Remove redundant event unsubscribe before dispose
* Remove duplicate CertificateUpdate unsubscribe (already existed in master)
(cherry picked from commit eae41a55ac)
* test(numeric-range): add failing test
* fix(numeric-range): include sub ranges in equality check
* test(numeric-range): add tests for coverage
(cherry picked from commit 56486efcdb)
* test(type-info): add failing test
* fix(type-info): require IEnumerable implementation before treating generic type as array
* test(type-info): add test for coverage
(cherry picked from commit 25a4882145)
ServerBase.StopAsync() calls Close() on each transport listener then
Clear() on the list. ServerBase.Dispose() later iterates
TransportListeners to call Dispose() on each — but the list is already
empty, so TcpTransportListener.Dispose() never runs.
Add Utils.SilentDispose() after Close() in StopAsync to ensure
TcpTransportListener.Dispose() executes before the list is cleared.
This pull request modernizes the OPC UA .NET codebase and tests, removes .NET Standard 2.0 support from certificate/security code, and updates test code to use C# 12 and .NET 8+ features for clarity and maintainability.
- Certificate and BouncyCastle interop files (e.g., CertificateBuilder.cs, X509Extensions.cs, X509SignatureFactory.cs) are refactored to drop NETSTANDARD2_0 support and use modern .NET APIs and conditional compilation.
- Test code across the codebase is updated to use default, strongly-typed Parse methods, collection expressions, and improved async/await patterns.
- Exception handling and assertions in tests are modernized, and test method naming is made consistent with Async suffixes.
- The project and build files are updated to clarify Roslynator analyzer usage and remove obsolete netstandard tags.
- General code and test cleanup includes improved null safety, type safety, and removal of deprecated or unused code.
* Improve logging and fix UserNameIdentityToken to return BadIdentityTokenInvalid for failed verification
Refactor DataChangeQueueHandler to return overflow status from QueueValue and Enqueue, updating MonitoredItem to log overflows accurately. Update IDataChangeQueueHandler interface accordingly. Enhance ConsoleTelemetry logger configuration management and ensure proper disposal of old logger factories.
Change UserNameIdentityToken password/nonce failure to return BadIdentityTokenInvalid for more precise error reporting. To prevent the leakage of information useful to attackers, Servers shall ensure that the process of validating UserIdentityTokens completes in a fixed interval independent of whether an error occurs or not. The process of validation includes decrypting, check for padding and checking for a valid nonce. If any errors occur the return code is Bad_IdentityTokenInvalid.
* fix GDS ApplicationNames to include the Locale
* fix ctt testcase by allowing faster sampling for XmlNodeNode
* remove all usages of SoftwareCertificates
- Client
- Server
As per spec they are not relevant for security.
If an SDK User wants to use the SoftwareCertificates ActivateSession can be overriden
* remove from SessionClientBatchTests
* Refactor ApplicationConfiguration serialization for deprecated settings and add tests for configuration behavior
* Update ApplicationConfiguration to prefer modern elements in "hybrid" configurations and add corresponding tests
* Fix GDS Tests on netstandard2.1 custom test target
* Add Test reading file containing ApplicationCertificate and ApplicationCertificates ensuring it is not marked legacy.
* Add legacy config test with only one ApplicationCertificate
---------
Co-authored-by: Roman Ettlinger <romanett98@gmail.com>
* Initial plan
* Fix BadUserAccessDenied when SecurityPolicy/SecurityMode not matching
Modified MatchEndpoints method to throw appropriate exceptions:
- BadSecurityPolicyRejected when requested policy is not supported
- BadSecurityModeRejected when requested mode is not supported
- Maintains backward compatibility when no specific security settings requested
Added comprehensive tests to verify the fix.
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Improve code readability with extracted boolean variables
Applied code review feedback to extract parameter validation checks
into separate boolean variables for better readability.
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Add integration test for unsupported security policy
Added ConnectWithUnsupportedSecurityPolicyAsync test that:
- Uses ClientFixture to connect to test server
- Requests a security policy not supported by server
- Verifies BadSecurityPolicyRejected is thrown instead of BadUserAccessDenied
- Tests run successfully on net8.0 and net9.0 targets
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Add policy that will never work
* ignore faulty test
* update
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: Roman Ettlinger <romanett98@gmail.com>
* Initial plan
* Fix parent-child references when importing NodeSet2.xml
- Added LinkParentChildRelationships method to establish parent-child links after import
- Sets Parent property and adds child to parent's children collection
- Added test to verify parent-child references work correctly after import
- All existing tests continue to pass
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Make parent-child linking optional with default false
- Added linkParentChild parameter to Import method (defaults to false)
- Maintains backward compatibility by not establishing parent-child relationships by default
- Added test to verify backward compatibility (default behavior)
- Updated ParentChildReferencesTest to explicitly enable the feature
- All 7 tests pass
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
* Initial plan
* Add support for private encodeable factory in server initialization
- Add new ServiceMessageContext constructor that accepts a custom IEncodeableFactory
- Add CreateMessageContext(IEncodeableFactory) overload to ApplicationConfiguration
- Mark CreateMessageContext(bool) as obsolete since the clonedFactory parameter was never used
- Add PrivateEncodeableFactory property to ServerBase for specifying a private factory
- Update all internal calls to use the new parameterless CreateMessageContext()
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Add unit tests for ServiceMessageContext private factory support
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Improve documentation for ServiceMessageContext constructors
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Initial plan
* Fix ByteString vs Byte array distinction in WrappedValue
Modified BaseVariableState.WrappedValue to use DataType and ValueRank
when creating Variant to correctly distinguish between byte[] (Byte array)
and ByteString. Added tests to verify the fix.
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
* Address code review feedback
- Remove redundant null check for NodeId (NodeId.IsNull handles null)
- Update comment in test to be more specific about ByteString testing
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
romanett