Commit Graph
5 Commits
Author SHA1 Message Date
romanettandClaude Opus 5.5 1da9fc0543 Redundancy.Server session mirroring: fix fast reconnect lifecycle (#4627) (#4704)
Fixes #4627 — the eight Redundancy.Server session mirroring defects
(HotAndMirrored fast reconnect).

## Changes

| ID | Fix |
|----|-----|
| RS-1 | The owning replica mirrors a liveness heartbeat
(`LastContactAt`), written at most once per quarter of the session
timeout while the client uses the session
(`DistributedSessionOptions.LivenessInterval`, default 1 s). A restore
expires the timeout after the later of the last activation and the last
mirrored contact, plus the heartbeat's maximum lag. |
| RS-2 | Entries record their owning replica (`OwnerId`). Only the owner
mirrors and deletes the entry. A restore takes ownership with a
conditional write. A stale copy on the previous replica is dropped
without deleting the entry, both when its heartbeat sees the change and
when the client fails back to it. `ISharedSessionStore` gains
`TryReplaceAsync`, `TryRemoveAsync` and `EnumerateAsync`. Over a store
without CAS (the CRDT gossip store) they fall back to
compare-then-write. |
| RS-3 | A restored session keeps the SessionId from `CreateSession`:
`Session.RequestedId`, and `DiagnosticsNodeManager` honours a requested
id when it is free. `DistributedSessionManager` mints GUID SessionIds
that are unique across the replica set. Continuation points therefore
stay under one owner id across repeated failovers. |
| RS-4 | Sessions whose user token is encrypted with an ECC/RSA-DH
EphemeralKey are flagged. Their restore is rejected up front
(`EphemeralKeyUnavailable`), before anything is consumed or counted as a
failed authentication. The documented option from the issue was chosen
over mirroring ephemeral private keys. |
| RS-5 | New `SessionManager.AdmitRestoredSessionAsync` hook: the
mirrored nonce is consumed only after the restored activation has passed
the signature, identity and continuity checks. If a restored session's
first activation fails, the session is discarded without the virtual
close, so the shared entry is untouched. Restored copies that are not
admitted now also release their diagnostics node, and
`CurrentSessionCount` is balanced for restored sessions. |
| RS-6 | Expired entries are deleted on restore and by a periodic sweep
(`SweepInterval`, default 5 min). The sweep also purges consumed-nonce
markers that no live entry references and that are older than
`NonceMarkerRetention` (default 1 h), via
`ISingleUseNonceRegistry.PurgeAsync`. |
| RS-7 | `SharedKeyValueSessionStore` and
`DistributedSessionManagerFactory` now throw for an external store
without an `IRecordProtector`, as the DI registration already does. Pass
`NullRecordProtector.Instance` to opt out explicitly. |
| RS-8 | Entries carry the original server certificate. A restored
session accepts a client signature over either that certificate or its
own one (`Session.OriginalServerCertificate`). |

Entries are now written at `SecurityStateVersion` 5. Entries at versions
3 and 4 can still be restored, without an owner.

### API changes (Redundancy.Server)
- New members on `ISharedSessionStore` and `ISingleUseNonceRegistry`.
- An optional `TimeProvider` parameter on
`SharedSingleUseNonceRegistry`.
- New `DistributedSessionOptions` properties.
- `SharedSessionEntry` v5 fields.
- `SharedKeyValueSessionStore` and `DistributedSessionManagerFactory`
throw for external stores without a protector.

## Tests
- New unit tests for every finding: the
restore/heartbeat/ownership/sweep/SessionId paths of
`DistributedSessionManager`; store conditional writes, the CAS fallback
and v5 round-trips; the registry purge; the factory/store guard; base
`SessionRestoreTests` (admitted once, discarded on validation failure or
rejected admission); and `SessionOriginalServerCertificateTests`.
- New end-to-end test
`SessionUsedPastItsTimeoutFailsOverWithItsSessionIdAsync`. The client
uses the session for 2× its timeout, then fails over. The test checks
that the SessionId is preserved and that the standby serves that
SessionId and owns the entry. It also closes the stale copy and checks
the entry survives. With the heartbeat disabled, the test fails as
expected.
- Local net10.0 runs: Redundancy.Server.Tests 883/883, Server.Tests 8347
passed / 5 skipped, Sessions.Tests 849 passed / 379 skipped (the usual
baseline). The net48 build is clean. The Redundancy session tests pass
on net48; the ECC-only cases are skipped there.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 21:54:14 +02:00
romanettandClaude Opus 5.5 9b034f8291 Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700)
# Description

Nullable reference types are now enabled once, in `common.props`, and
every project in the repository builds warning-clean with them.

**Configuration**
- `<Nullable>enable</Nullable>` lives in `common.props`. All per-project
`<Nullable>` settings are removed: 178 project files (enable,
annotations, and the `disable` in the three Aot test projects).
- 419 per-file `#nullable enable`/`restore` directives and the 4
`#nullable disable` directives are removed.
- Kept on purpose:
- The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type
Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`,
`PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the
project setting in generated code, so the directive is required there.
- `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against
the 1.5.378 packages and does not import the repository props.
- `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now
describe the central setting.

**Migration** (the 43 unset and 13 annotations-only projects, roughly
10k diagnostics)
- **Production code: annotated at the declarations.** Covers
`tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the
LDS sample.
- Members, parameters and returns are nullable where null actually
flows.
- Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`,
`IsMethodTypeDesign`).
- `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take
`params object?[]`. `Template.AddReplacement` takes `string?` (rendering
already skips null replacements).
- `IRegisteredServerStore.RegisterAsync` returns
`Task<RegistrationEntry?>`: it returns null when a registration is
removed.
- Dereferences backed by an invariant (for example, the validated model)
use `!`, so runtime behaviour is unchanged.
- **Behavioural changes in production code** (small, deliberate):
- `NodeSetToModelDesign` throws `InvalidOperationException` when the
file is not a NodeSet. Before, it threw a NullReferenceException on the
next line.
- `ModelDesignValidator` starts its lookup tables empty instead of null.
A redundant `if (m_nodes == null) return;` guard is gone.
- **Test code: minimal annotations.**
  - `?` on values that can be null, `!` where the test asserts non-null.
  - `= null!` for fixture members assigned in SetUp.
- `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the
`IEncodeable` constraint.
- A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`)
folded into the adjacent `IsNull`/`Count` checks.

**Notes for reviewers**
- The diff is large (~1,500 files) but almost entirely mechanical: `?`,
`!` and directive removal. The non-annotation changes are listed above.
- Known follow-up, not done here:
`IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay
non-nullable even though the validator returns null before it loads a
dictionary. Making them nullable would ripple through every generator
call site.

## Related Issues

- Fixes #4439

## 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.
- [ ] 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.

The "no new warnings" box covers net10.0 and net48: full non-incremental
builds of `UA.slnx`, `tools/SourceGeneration.slnx`,
`tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0
warnings and 0 errors under the repository's warnings-as-errors
settings. net9.0 and net8.0 also had no nullable diagnostics, but they
were not rebuilt after the final fixes. Tests have not been run locally
yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 08:48:13 +02:00
romanettandClaude Opus 5.5 d98eb6b28c Fix server session defects found by a spec-checked review of CreateSession, ActivateSession, session lifecycle and continuation points (#4632)
# Description

Fixes defects found by a spec-checked review of the server session
implementation: CreateSession, ActivateSession, the session lifecycle
(per-request validation, CloseSession, Cancel, timeout, shutdown) and
session continuation points. Each finding was checked against OPC
10000-4 (§5.7, §7.9, §7.32), OPC 10000-5 (session diagnostics and audit
events) and OPC 10000-6 §6.8.2.

There is one commit per finding, and each commit message cites the spec
section. A max-effort code review of the fixes then found 15 more
issues, mostly regressions the first round introduced; they are fixed in
the `revfix` merges.

## What changed

**CreateSession**
- At MaxSessionCount the oldest session that was never activated is
closed to make room (§5.7.2.1). Sessions that are activated, already
closing, or have an activation in flight are never evicted. The victim
is claimed atomically, so a late activation commit on it is rejected.
- `requestedSessionTimeout` NaN is revised. A malformed `endpointUrl` no
longer fails the call with Bad_UnexpectedError. `serverUri` is ignored,
as §5.7.2.2 requires.
- Unnamed sessions get distinct names.
- Session diagnostics now report `MaxResponseMessageSize` and
`ServerUri`, and `SessionSecurityDiagnostics.Encoding` uses the
specified names ("UA Binary", "XML", "JSON").
- AuditCreateSessionEvent carries SecureChannelId and the request
certificate, also on failure.
- Every rejected CreateSession is counted, and only once. A rejection
during server startup no longer turns into Bad_UnexpectedError.
- If an `OnApplicationCertificateError` override accepts an error, the
certificate is now kept and the remaining checks still run. A
certificate accepted this way does not grant TrustedApplication or
application-based roles.

**ActivateSession**
- EphemeralKey handling follows OPC 10000-6 §6.8.2:
- A key used to decrypt a UserName or IssuedToken secret is retired and
replaced in the same commit.
  - A key that was not used is kept, and nothing is returned.
- ECDHPolicyUri is honoured; an unsupported policy gets
Bad_SecurityPolicyRejected.
  - Request parameters are no longer echoed back.
- After the activation commits, a failure or cancellation no longer
faults the response, which would hide the new server nonce. Post-commit
callbacks run uncancellably.
- An activation that overlaps a session close or timeout fails with
Bad_SessionClosed instead of returning Good.
- The PolicyId of an AnonymousIdentityToken is validated.
- Status codes now follow §5.7.3.3:
- Bad_ApplicationSignatureInvalid for a mismatched client-signature
algorithm
- Bad_UserSignatureInvalid for a mismatched X509 user-token signature
algorithm
- Bad_IdentityTokenInvalid for an undecodable token or an unknown policy
- `ClientUserIdOfSession` keeps the user of the first activation.

**Lifecycle and requests**
- Requests on an expired or closing session are rejected with
Bad_SessionClosed (§5.7.2.1). A CloseSession on an expired session that
is not yet closing is still processed as the client's close and honours
`deleteSubscriptions`.
- Closing a session aborts its outstanding requests with
Bad_SessionClosed. A request that registers after that abort pass is
rejected too.
- Cancel completes the matching requests with
Bad_RequestCancelledByClient (§5.7.5.1). It also reaches requests still
waiting in the request queue, but only when both timestamps prove the
request was sent earlier.
- There is one AuditCancelEvent per Cancel call, carrying the client's
audit entry id and user.
- Every close path goes through a single close claim, so
SessionAbortCount, SessionTimeoutCount and the "Session/Terminated" and
"Session/Timeout" audits are each counted once.
- Teardown that started cannot be cancelled. Requests rejected by
session validation are counted.

**Continuation points**
- HistoryRead returns Bad_NoContinuationPoints instead of evicting
points of the same request (§7.9). Only the request that saved a point
pins it, so points from prior requests can still be freed.
- Points saved by a HistoryRead that faults are released.
- Session-less Browse and BrowseNext no longer dereference a null
session.

## Behaviour changes reviewers should know about
- At the session cap, CreateSession evicts a non-activated session
instead of returning Bad_TooManySessions. The eviction is counted in
SessionAbortCount.
- Clients see Bad_SessionClosed for requests on expired or closing
sessions, and Bad_RequestCancelledByClient for cancelled requests.
- `ActivateSessionProcessAdditionalParameters` is now called on every
successful activation, with or without an AdditionalHeader.
- If an `OnApplicationCertificateError` override returns instead of
throwing, CreateSession now accepts the certificate.
- Timeout and termination audits are now raised after the session
teardown.
- The only public API changes are additive overloads in `AuditEvents`
and `RequestManager`.

**Known partial fixes:**
- The session's `maxResponseMessageSize` is reported but not enforced;
only the channel limit applies.
- With overlapping ActivateSession calls on one session, an earlier
response can still carry a key that a later activation has already
replaced. Closing that fully needs a new `ISession` member.

## Related Issues

- The Redundancy.Server session-mirroring findings from the same review
are tracked separately in #4627.

## Tests
- Regression tests cover each finding. The new files are:
  - `CreateSessionSpecRegressionTests`
  - `ActivateSessionValidationRegressionTests`
  - `StandardServerActivateSessionRegressionTests`
  - `SessionLifecycleRegressionTests`
  - `SessionRequestAdmissionRegressionTests`
  - `HistoryReadContinuationDispatchTests`
  
  Existing test classes were extended too.
- Local results on net10.0:
  - `Opc.Ua.Server.Tests`: 7883 passed / 0 failed / 5 skipped
- `Opc.Ua.Sessions.Tests`: 790 passed / 0 failed / 379 skipped (same as
master)
- net48 Release build of `Opc.Ua.Server.Tests`: 0 errors / 0 warnings.

## Checklist

- [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.
- [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.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 05:42:01 +02:00
Marc Schier fde50c5700 Enforce ActivateSession transfer and reconnect security (OPC 10000-4 §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.
2026-07-28 12:07:02 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00