Repository navigation
[release/2.0] Server: hold anonymous session-less requests to the channel security of CreateSession - #4695
Closed
biancode wants to merge 1 commit into
Conversation
…of CreateSession (OPCFoundation#4689) # Description `AddSessionlessInvocation(o => o.AllowAnonymous = true)` (OPCFoundation#4659) lets a request without an Access Token run as an anonymous user. OPC 10000-4 §6.3.1 allows that only "if application authentication through the SecureChannel is sufficient". On HTTPS the channel gave no such authentication: on an endpoint with mutual TLS and a SignAndEncrypt policy, a session-less request without a TLS client certificate and without an Access Token was answered `Good`, over the REST binding and over the binary POST with `OPCUA-SecurityPolicy: Basic256Sha256`. `CreateSession` on the same endpoint answers `Bad_SecurityChecksFailed` without the certificate, and ActivateSession on a new channel does too (OPC 10000-6 §7.4.1: applications that use HTTPS and require application authentication check the application certificates during the CreateSession/ActivateSession handshake). This PR makes a session-less request without an Access Token meet what the endpoint demands of a Session. OPCFoundation#4659 is merged, in no release tag and opt-in, so this is a change to a new feature; the default `AllowAnonymous = false` is not affected. ## Changes ### `SessionManager.ValidateSessionlessRequestAsync` (`src/Opc.Ua.Server/Session/SessionManager.cs`) With `AllowAnonymous`, a request without an Access Token now needs: | Check | Where it applies | Status otherwise | Same as | | --- | --- | --- | --- | | The client certificate of the channel (`SecureChannelContext.ClientChannelCertificate`) | endpoints with a security mode or policy other than None, and HTTPS endpoints when `HttpsMutualTls` is set | `Bad_SecurityChecksFailed` | CreateSession on the HTTPS listener; ActivateSession on a new channel | | An Anonymous user token policy on the endpoint (an endpoint without policies does not restrict) | every endpoint | `Bad_IdentityTokenRejected` | ActivateSession with an anonymous identity token | | An endpoint on the channel | every request | `Bad_SecurityModeInsufficient` | the same request with an Access Token | - The certificate is the one the transport has validated: the SecureChannel certificate of opc.tcp and opc.wss, the TLS client certificate of the HTTPS listener (REST, binary, JSON). So opc.tcp channels with security keep working as they did, and an HTTPS client that wants an anonymous session-less call presents its application certificate in the TLS handshake. - An HTTPS listener without `HttpsMutualTls` advertises SecurityMode None without an Anonymous user token policy (`HttpsServiceHost`); an anonymous session-less request there is now rejected as an anonymous Session is. - A request **with an Access Token** is unchanged: the token authenticates the caller and HTTPS keeps it confidential (HTTPS clients are not required to present a certificate, OPC 10000-6 §7.4.1). A `ValidateSessionLessRequest` handler still takes over completely. - Not changed: the transport credential authentication of the REST routes (OPCFoundation#4674). `SessionlessInvocationOptions.AllowAnonymous` documents the rule. `docs/SessionlessInvocation.md` has a new section "Anonymous requests and the channel" with the table. ### Tests - `tests/Opc.Ua.Sessions.Tests/SessionlessChannelSecurityTests.cs` (new, net8.0+): a real reference server with an HTTPS listener, twice (mutual TLS on with a SignAndEncrypt-only endpoint; mutual TLS off), over REST and the binary POST, with and without a TLS client certificate: anonymous without a certificate, with a certificate, `AllowAnonymous = false`, a valid and a bogus Access Token, and CreateSession on the binary POST without the certificate for comparison. 44 cases. - `tests/Opc.Ua.Server.Tests/SessionlessInvocationTests.cs`: opc.tcp with and without a channel certificate (Sign, SignAndEncrypt, None), HTTPS with and without mutual TLS and with a secured endpoint, user token policies with and without Anonymous, no endpoint, an Access Token and a handler without a certificate. The existing tests that use `AllowAnonymous` set `HttpsMutualTls = false` or give the channel an endpoint. - Fail without the change: 7 of the 9 new `SessionlessInvocationTests` cases fail with the `SessionManager.cs` change removed (the opc.tcp with security, HTTPS with and without mutual TLS, user token policy and unknown endpoint tests); the 6 failing `SessionlessChannelSecurityTests` cases are below. ## Testing - **Reproduction on master (`c03c12838`)**: `SessionlessChannelSecurityTests` without the change: 6 of 44 cases fail, 38 pass. With mutual TLS on and no TLS client certificate the anonymous request is answered `Good` instead of `Bad_SecurityChecksFailed` (REST and binary). With mutual TLS off it is answered `Good` instead of `Bad_IdentityTokenRejected` (REST and binary, with and without a certificate). All 44 pass with the change; the Access Token cases pass both ways. - Results on macOS: `Opc.Ua.Server.Tests` full run on net10.0 8197 passed / 9 skipped / 0 failed; the session-less and SessionManager filters 139 passed / 0 failed on net10.0 and on net8.0. `Opc.Ua.Sessions.Tests` with the HTTPS/WebApi/Sessionless filters 125 passed / 0 failed on net10.0 and on net8.0, and with the Wss/OpenApi/ClientTest filters 320 passed / 241 skipped / 0 failed on net10.0. - Builds: `Opc.Ua.Server`, `Opc.Ua.Server.Tests` and `Opc.Ua.Sessions.Tests` for net48, net8.0, net9.0 and net10.0: 0 warnings, 0 errors. - `dotnet format` (whitespace, style, analyzers `--severity info`, one `--include` per changed file): no finding on a changed line (CA1849 at `SessionManager.cs` 235 is on master). - Coverage (`.azurepipelines/check-coverage.ps1` against master, session-less and SessionManager tests of `Opc.Ua.Server.Tests` on net10.0): patch 36/36 changed lines (100 %). The project floor is not meaningful for this subset. - CI and CodeQL on the PR branch: [CI run 37529750287](https://github.com/biancode/UA-.NETStandard/actions/runs/37529750287) green (158/158 jobs, incl. macOS and Windows net48), [CodeQL run 37529754883](https://github.com/biancode/UA-.NETStandard/actions/runs/37529754883) green, on `03a8e3d91`. ## Related Issues - Fixes OPCFoundation#4688 - Relates to OPCFoundation#4659 (session-less invocation), OPCFoundation#4657 (the REST binding's session-less service set) and OPCFoundation#4674 (REST authentication) ## 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. - [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. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the branch CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> (cherry picked from commit fa55157)
4 of 7 tasks
Contributor
|
@biancode 2.0 shipped 3 hours ago... |
Contributor
|
We will include this in a later merge that includes multiple commits |
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
Backport of #4689 (fixes #4688) to
release/2.0, so that 2.0.0 ships with it.release/2.0contains #4659 (session-less invocation withAllowAnonymous) but not #4689. Without this change a session-less request without an Access Token is answeredGoodon an HTTPS endpoint with mutual TLS and a SignAndEncrypt policy, althoughCreateSessionon the same endpoint answersBad_SecurityChecksFailedwithout the client certificate. #4689 makes such a request meet what the endpoint demands of a Session. The defaultAllowAnonymous = falseis not affected.Changes
Cherry-pick of master commit
fa55157ce(git cherry-pick -x), applied without conflicts onrelease/2.0(bc5da1006).release/2.0already containsc03c12838. All six changed files are identical to the master versions:src/Opc.Ua.Server/Session/SessionManager.cssrc/Opc.Ua.Server/Session/SessionlessInvocationOptions.csdocs/SessionlessInvocation.mdtests/Opc.Ua.Server.Tests/SessionlessInvocationTests.cstests/Opc.Ua.Server.Tests/SessionlessRequestBudgetTests.cstests/Opc.Ua.Sessions.Tests/SessionlessChannelSecurityTests.cs(new)Testing
Local, macOS, SDK 10.0.401 from
release/2.0global.json:Opc.Ua.Server.TestsandOpc.Ua.Sessions.Tests(withOpc.Ua.Server) build for all target frameworks including net48: 0 warnings, 0 errors.Opc.Ua.Server.Testson net10.0, filterSessionless: 90 passed, 0 failed.Opc.Ua.Sessions.Testson net10.0, filterSessionlessChannelSecurityTests|SessionlessServiceInvocation: 56 passed, 0 failed.dotnet formatnot rerun: the changed files are identical to master.The PR checks of the repository run on this PR.
Related Issues
Checklist