From e3d322cbf57844bcc55714d70e355a67ed3bac23 Mon Sep 17 00:00:00 2001 From: Marc Schier Date: Mon, 6 Jul 2026 08:48:07 +0200 Subject: [PATCH] Fix three high-severity audit findings in new 2.0 pub sub/GDS code (#3954) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Fixes three high-severity, remotely-relevant security weaknesses identified by a full-codebase security audit. Each fix is minimal and surgical, and ships with regression tests. ### 1. PubSub inbound message-security fail-open for non-UADP (JSON) frames `PubSubConnection` nested the inbound signature/encryption/replay enforcement **inside** the `UadpDecoder.TryReadOuterPrefix(...)` branch. Because that check returns `false` for a JSON frame, a reader configured for `Sign`/`SignAndEncrypt` over a JSON transport silently accepted attacker-forged, unauthenticated frames, delivering forged DataSet/MetaData messages to application sinks. The send path already fail-*closes* the identical case. **Fix:** drop any non-UADP frame fail-closed on a secured reader before decode (`else if (RequiresInboundSecurity)`), mirroring the send-side refusal. ### 2. Client schema-import SSRF / NetNTLM credential leak When loading complex types, the client falls back to the server's type dictionary and resolves each `Import`/`SchemaLocation` against the local file system with no sanitization. A malicious server could supply a UNC path (e.g. `\\attacker\share\x`), triggering an outbound SMB/NetNTLM handshake (hash theft / SSRF) or arbitrary local file read. This bypassed the existing XXE hardening because imports are followed by manual file-system code, not the XML resolver. **Fix:** added a deny-all `NullFileSystem` and wired it into the in-memory/import-table schema validator constructors, so network-supplied dictionaries never touch the local file system. The XML loader now prefers the in-memory import table before any (untrusted) location, so legitimate in-memory resolution is unchanged. ### 3. GDS AuthorizationService JWT impersonation / privilege escalation `FinishRequestToken` derived the JWT subject from the **unauthenticated** client-supplied `userIdentityToken` (never verifying `userTokenSignature`) and embedded the client's `requestedRoles` verbatim. Any caller able to open an encrypted channel — even anonymous — could obtain a GDS-signed token asserting `sub=admin`, `roles=[SecurityAdmin, ...]`, impersonating any principal to resource servers that trust GDS-issued JWTs. **Fix:** - The token subject is now bound to the identity that was **authenticated on the session** when the request was started; the client token is no longer trusted for the subject. - Requested roles are gated by a new **fail-closed** `AuthorizationServiceOptions.AuthorizeRoles` hook. With no callback configured, **no roles** are granted; the callback can only ever narrow the requested set. This removes privilege escalation via arbitrary role requests. - The obsolete single-call `RequestAccessToken` path no longer trusts the client-supplied token: it issues an unprivileged **anonymous**-subject token with no roles. Use `StartRequestToken`/`FinishRequestToken` for authenticated identity binding. ## Related Issues No public issue is linked — these findings were identified by an internal security audit and fixed directly. Maintainers may open tracking issues if desired. ## Verification - All affected projects build with **0 warnings / 0 errors** (net10.0). - Regression tests added and passing: - PubSub: `SecuredReaderRejectsNonUadpJsonFrameAsync` (+ existing receive-security suite, 6/6). - Schema/SSRF: `NullFileSystemTests` (6/6), `ImportTableValidatorDoesNotResolveServerSuppliedFilesystemLocation` (BSD, 3/3), source-gen schema validation (6 passed), full ComplexTypes client-load suite (2214/2214). - GDS: `FinishDoesNotGrantRequestedRolesWithoutAuthorization`, `FinishBindsSubjectToAuthenticatedIdentityNotRequestToken`, plus `LegacyRequestAccessTokenIssuesAnonymousUnprivilegedToken` and `AuthorizeRoles` intersection/empty edge cases (AuthorizationService unit 15/15, RefreshToken integration 3/3). _Review feedback from the automated reviewer has been addressed (NullFileSystem singleton constructor + header year, PubSub drop logged at Debug to avoid log amplification, and the obsolete `RequestAccessToken` path simplified to a uniform anonymous subject); all review threads resolved._ --- .../Hosting/AuthorizationServiceOptions.cs | 17 ++ .../Identity/InMemoryAccessTokenProvider.cs | 93 +++++-- .../Connections/PubSubConnection.cs | 27 ++ .../Schema/BinarySchemaValidator.cs | 2 +- Stack/Opc.Ua.Types/Schema/SchemaValidator.cs | 20 +- .../Opc.Ua.Types/Schema/XmlSchemaValidator.cs | 2 +- .../Utils/FileSystem/NullFileSystem.cs | 94 +++++++ .../BsdSchemaValidationTests.cs | 48 ++++ ...InMemoryAccessTokenProviderRefreshTests.cs | 7 +- .../StartRequestTokenTests.cs | 235 ++++++++++++++++++ .../PubSubConnectionSecurityReceiveTests.cs | 30 +++ .../Utils/FileSystem/NullFileSystemTests.cs | 92 +++++++ 12 files changed, 638 insertions(+), 29 deletions(-) create mode 100644 Stack/Opc.Ua.Types/Utils/FileSystem/NullFileSystem.cs create mode 100644 Tests/Opc.Ua.Types.Tests/Utils/FileSystem/NullFileSystemTests.cs diff --git a/Libraries/Opc.Ua.Gds.Server.Common/Hosting/AuthorizationServiceOptions.cs b/Libraries/Opc.Ua.Gds.Server.Common/Hosting/AuthorizationServiceOptions.cs index a35fcd699b..89be385877 100644 --- a/Libraries/Opc.Ua.Gds.Server.Common/Hosting/AuthorizationServiceOptions.cs +++ b/Libraries/Opc.Ua.Gds.Server.Common/Hosting/AuthorizationServiceOptions.cs @@ -89,5 +89,22 @@ namespace Opc.Ua.Gds.Server.Hosting /// audience and requested scopes. /// public Func, bool>? AccessControl { get; set; } + + /// + /// Gets or sets an optional authorization callback that decides which of + /// the roles requested during FinishRequestToken may be embedded into + /// the issued token. It receives the identity that was authenticated on + /// the session when the request was started, the audience and the + /// requested roles, and returns the subset of roles to grant. + /// + /// + /// When this callback is not set no roles are granted (fail-closed). + /// A caller can therefore never obtain a token asserting privileged + /// roles (such as SecurityAdmin) that the operator has not explicitly + /// authorized, which prevents privilege escalation through arbitrary + /// role requests. The returned roles are always intersected with the + /// requested roles, so the callback can only ever narrow the set. + /// + public Func, IReadOnlyList>? AuthorizeRoles { get; set; } } } diff --git a/Libraries/Opc.Ua.Gds.Server.Common/Identity/InMemoryAccessTokenProvider.cs b/Libraries/Opc.Ua.Gds.Server.Common/Identity/InMemoryAccessTokenProvider.cs index b7bce2f8c7..22498a7206 100644 --- a/Libraries/Opc.Ua.Gds.Server.Common/Identity/InMemoryAccessTokenProvider.cs +++ b/Libraries/Opc.Ua.Gds.Server.Common/Identity/InMemoryAccessTokenProvider.cs @@ -89,15 +89,33 @@ namespace Opc.Ua.Gds.Server.Identity /// [Obsolete("Use StartRequestTokenAsync + FinishRequestTokenAsync for Part 12 v1.05 compliance.")] - public async ValueTask RequestAccessTokenAsync( + public ValueTask RequestAccessTokenAsync( UserIdentityToken identityToken, string resourceId, CancellationToken ct = default) + { + return RequestAccessTokenAsync(identityToken, resourceId, null, ct); + } + + /// + /// Issues a token whose subject is bound to the identity the server + /// authenticated on the calling session rather than to the + /// (unauthenticated) request token. + /// + [Obsolete("Use StartRequestTokenAsync + FinishRequestTokenAsync for Part 12 v1.05 compliance.")] + public async ValueTask RequestAccessTokenAsync( + UserIdentityToken identityToken, + string resourceId, + IUserIdentity? callerIdentity, + CancellationToken ct = default) { ValidateAudience(resourceId); string[] scopes = NormalizeScopes(m_options.DefaultScopes); + // The subject is taken from the identity that the server + // authenticated on the session, never from the client-supplied + // request token, which carries no verified proof-of-possession. AccessToken token = await IssueAsync( - GetSubject(identityToken, null), + ResolveSubject(callerIdentity), resourceId, scopes, [], @@ -142,7 +160,8 @@ namespace Opc.Ua.Gds.Server.Identity resourceId, policyId ?? string.Empty, scopes, - callerIdentity?.DisplayName ?? string.Empty, + ResolveSubject(callerIdentity), + callerIdentity, DateTime.UtcNow.AddMinutes(5)); return new ValueTask<(ByteString serviceData, Guid requestId)>((ByteString.Empty, requestId)); @@ -171,10 +190,20 @@ namespace Opc.Ua.Gds.Server.Identity "AuthorizationService token request expired."); } - string[] roles = requestedRoles.IsEmpty - ? [] - : [.. requestedRoles.ToArray()!.Where(role => !string.IsNullOrWhiteSpace(role))]; - string subject = GetSubject(userIdentityToken, request.Subject); + // Bind the subject to the identity that was authenticated on the + // session when the request was started. The client-supplied + // userIdentityToken (and its userTokenSignature) are NOT trusted + // here: they carry no verified proof-of-possession, so honoring + // them would let any caller mint a token impersonating an + // arbitrary principal. + string subject = request.Subject; + + // Authorize the requested roles against the authenticated caller. + // Without an explicit authorization callback no roles are granted + // (fail-closed), which prevents a caller from escalating privileges + // by requesting arbitrary roles such as SecurityAdmin. + string[] roles = AuthorizeRoles(request, requestedRoles); + AccessToken token = await IssueAsync( subject, request.ResourceId, @@ -351,25 +380,50 @@ namespace Opc.Ua.Gds.Server.Identity } } - private static string GetSubject(UserIdentityToken? identityToken, string? fallback) + /// + /// Resolves the JWT subject from the identity the server authenticated + /// on the calling session. The client-supplied request token is never + /// used, so a caller cannot choose an arbitrary subject. + /// + private static string ResolveSubject(IUserIdentity? callerIdentity) { - if (identityToken is UserNameIdentityToken userName && !string.IsNullOrEmpty(userName.UserName)) + string? displayName = callerIdentity?.DisplayName; + return string.IsNullOrEmpty(displayName) ? "anonymous" : displayName!; + } + + /// + /// Determines which of the requested roles may be embedded into the + /// issued token for the authenticated caller. Fail-closed: no roles are + /// granted unless an authorization callback approves them, and the + /// approved set is always intersected with the requested roles so the + /// callback can never introduce roles the caller did not request. + /// + private string[] AuthorizeRoles(RequestRecord request, ArrayOf requestedRoles) + { + if (requestedRoles.IsEmpty || m_options.AuthorizeRoles == null) { - return userName.UserName; + return []; } - if (identityToken is X509IdentityToken x509 && x509.CertificateData.Length != 0) + + string[] requested = [.. requestedRoles.ToArray()! + .Where(role => !string.IsNullOrWhiteSpace(role)) + .Distinct(StringComparer.Ordinal)]; + if (requested.Length == 0) { - return Convert.ToBase64String(x509.CertificateData.ToArray()); + return []; } - if (!string.IsNullOrEmpty(fallback)) + + IReadOnlyList granted = m_options.AuthorizeRoles( + request.Caller, request.ResourceId, requested); + if (granted == null || granted.Count == 0) { - return fallback!; + return []; } - if (!string.IsNullOrEmpty(identityToken?.PolicyId)) - { - return identityToken!.PolicyId!; - } - return "anonymous"; + + var requestedSet = new HashSet(requested, StringComparer.Ordinal); + return [.. granted + .Where(role => !string.IsNullOrWhiteSpace(role) && requestedSet.Contains(role)) + .Distinct(StringComparer.Ordinal)]; } private static string[] DecodeScopes(ByteString requestorData) @@ -420,6 +474,7 @@ namespace Opc.Ua.Gds.Server.Identity string PolicyId, string[] Scopes, string Subject, + IUserIdentity? Caller, DateTime ExpiresAtUtc); private sealed record IssuedTokenRecord( diff --git a/Libraries/Opc.Ua.PubSub/Connections/PubSubConnection.cs b/Libraries/Opc.Ua.PubSub/Connections/PubSubConnection.cs index 79ef5ed671..ba80ed0b8e 100644 --- a/Libraries/Opc.Ua.PubSub/Connections/PubSubConnection.cs +++ b/Libraries/Opc.Ua.PubSub/Connections/PubSubConnection.cs @@ -894,6 +894,33 @@ namespace Opc.Ua.PubSub.Connections framePayload = unwrapped.Value; } } + else if (RequiresInboundSecurity) + { + // Fail-closed: the frame is not a UADP NetworkMessage + // (e.g. a JSON frame, for which no message-security + // wrapper exists), so it cannot be authenticated or + // decrypted to satisfy the reader's configured + // SecurityMode. Dropping it here prevents an attacker + // from delivering forged, unsecured non-UADP frames to + // application sinks on a secured reader. Mirrors the + // send-side refusal to publish unsecured non-UADP + // messages on a connection configured for security. + RecordSecurityFailure( + StatusCodes.BadSecurityModeRejected, + "Inbound non-UADP frame cannot satisfy the reader's " + + "configured SecurityMode."); + // Logged at Debug: this is a per-frame hot path and a + // flood of non-UADP frames (hostile or misconfigured) + // must not amplify into high-volume warning logs. The + // security-failure counter incremented above is the + // rate-safe operational signal. + m_logger.LogDebug( + "Dropping non-UADP inbound frame on connection " + + "'{Connection}' requiring {Mode}.", + Name, + m_requiredSecurityMode); + continue; + } PubSubNetworkMessage? message; try diff --git a/Stack/Opc.Ua.Types/Schema/BinarySchemaValidator.cs b/Stack/Opc.Ua.Types/Schema/BinarySchemaValidator.cs index cec1cc3674..01b5be7699 100644 --- a/Stack/Opc.Ua.Types/Schema/BinarySchemaValidator.cs +++ b/Stack/Opc.Ua.Types/Schema/BinarySchemaValidator.cs @@ -59,7 +59,7 @@ namespace Opc.Ua.Schema.Binary /// Intializes the object with a import table. /// public BinarySchemaValidator(IReadOnlyDictionary importTable) - : base(null, null, AndStandardTypeImports(importTable)) + : base(NullFileSystem.Instance, null, AndStandardTypeImports(importTable)) { } diff --git a/Stack/Opc.Ua.Types/Schema/SchemaValidator.cs b/Stack/Opc.Ua.Types/Schema/SchemaValidator.cs index 18aee5be9d..8ac58d7326 100644 --- a/Stack/Opc.Ua.Types/Schema/SchemaValidator.cs +++ b/Stack/Opc.Ua.Types/Schema/SchemaValidator.cs @@ -178,22 +178,28 @@ namespace Opc.Ua.Schema /// protected XmlSchema Load(string? path, string? namespaceUri, ValidationEventHandler handler) { - // check if path specified in the file table. if (namespaceUri != null) { - // check if path specified in the file table. + // An explicit namespace-to-location mapping (trusted + // configuration) takes precedence and overrides any + // supplied location. if (m_namespaceUriToLocationMapping.TryGetValue(namespaceUri, out string? location)) { path = location; } + // Otherwise prefer an in-memory import for the namespace over a + // (potentially untrusted) schema location before falling back + // to the file system. This prevents a peer-supplied import + // location from being resolved against the local file system + // when the namespace can be satisfied from memory. + else if (m_importFiles.TryGetValue(namespaceUri, out byte[]? schemaBuffer)) + { + using var istrm = new MemoryStream(schemaBuffer); + return Load(istrm, handler); + } if (path == null) { - if (m_importFiles.TryGetValue(namespaceUri, out byte[]? schemaBuffer)) - { - using var istrm = new MemoryStream(schemaBuffer); - return Load(istrm, handler); - } throw new FileNotFoundException(CoreUtils.Format( "Missing schema location for namespace {0}", namespaceUri)); } diff --git a/Stack/Opc.Ua.Types/Schema/XmlSchemaValidator.cs b/Stack/Opc.Ua.Types/Schema/XmlSchemaValidator.cs index 549b2bf722..2c736b1169 100644 --- a/Stack/Opc.Ua.Types/Schema/XmlSchemaValidator.cs +++ b/Stack/Opc.Ua.Types/Schema/XmlSchemaValidator.cs @@ -58,7 +58,7 @@ namespace Opc.Ua.Schema.Xml /// Intializes the object with a import table. /// public XmlSchemaValidator(IReadOnlyDictionary importTable) - : base(null, null, importTable) + : base(NullFileSystem.Instance, null, importTable) { } diff --git a/Stack/Opc.Ua.Types/Utils/FileSystem/NullFileSystem.cs b/Stack/Opc.Ua.Types/Utils/FileSystem/NullFileSystem.cs new file mode 100644 index 0000000000..db94f5ca76 --- /dev/null +++ b/Stack/Opc.Ua.Types/Utils/FileSystem/NullFileSystem.cs @@ -0,0 +1,94 @@ +/* ======================================================================== + * Copyright (c) 2005-2026 The OPC Foundation, Inc. All rights reserved. + * + * OPC Foundation MIT License 1.00 + * + * Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * The complete license agreement can be found here: + * http://opcfoundation.org/License/MIT/1.00/ + * ======================================================================*/ + +using System.IO; + +namespace Opc.Ua +{ + /// + /// A deny-all file system that never resolves any path. It is used when a + /// component must load resources exclusively from an in-memory table and + /// must never touch the local file system - for example when validating a + /// type dictionary received from an untrusted network peer, where an + /// attacker-supplied import location (such as a UNC path) would otherwise + /// trigger an outbound SMB connection or arbitrary local file access. + /// + public sealed class NullFileSystem : IFileSystem + { + /// + /// Get singleton instance + /// + public static readonly NullFileSystem Instance = new(); + + private NullFileSystem() + { + } + + /// + public bool Exists(string path, bool isDirectory) + { + return false; + } + + /// + public void Delete(string path, bool isDirectory) + { + throw new FileNotFoundException( + "The null file system does not provide access to any path.", path); + } + + /// + public Stream OpenRead(string path) + { + throw new FileNotFoundException( + "The null file system does not provide access to any path.", path); + } + + /// + public Stream OpenWrite(string path) + { + throw new FileNotFoundException( + "The null file system does not provide access to any path.", path); + } + + /// + public System.DateTime GetLastWriteTime(string path) + { + throw new FileNotFoundException( + "The null file system does not provide access to any path.", path); + } + + /// + public long GetLength(string path) + { + throw new FileNotFoundException( + "The null file system does not provide access to any path.", path); + } + } +} diff --git a/Tests/Opc.Ua.Core.Schema.Tests/BsdSchemaValidationTests.cs b/Tests/Opc.Ua.Core.Schema.Tests/BsdSchemaValidationTests.cs index cd6ad8bc89..b1e360ac5a 100644 --- a/Tests/Opc.Ua.Core.Schema.Tests/BsdSchemaValidationTests.cs +++ b/Tests/Opc.Ua.Core.Schema.Tests/BsdSchemaValidationTests.cs @@ -27,9 +27,13 @@ * http://opcfoundation.org/License/MIT/1.00/ * ======================================================================*/ +using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Text; using System.Xml.Linq; using NUnit.Framework; +using Opc.Ua.Schema.Binary; using Opc.Ua.Schema.Bsd; namespace Opc.Ua.Schema.Tests @@ -126,6 +130,50 @@ namespace Opc.Ua.Schema.Tests }); } + [Test] + public void ImportTableValidatorDoesNotResolveServerSuppliedFilesystemLocation() + { + // Security regression: a type dictionary received over the network is + // validated with the in-memory import-table constructor. A malicious + // server can place an arbitrary file-system path (for example a UNC + // path that triggers an SMB/NetNTLM handshake) into an + // Location. That location must never be resolved against the local + // file system, even when the referenced file actually exists. + string evilPath = Path.GetTempFileName(); + try + { + File.WriteAllText( + evilPath, + "" + + "" + + "" + + ""); + + string mainSchema = + "" + + "" + + "" + + "" + + ""; + + var validator = new BinarySchemaValidator(new Dictionary()); + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(mainSchema)); + + // The import cannot be resolved because the file system is never + // consulted; validation fails closed instead of reading the file. + Assert.That( + () => validator.Validate(stream), + Throws.InstanceOf()); + } + finally + { + File.Delete(evilPath); + } + } + private static DefaultSchemaProvider CreateProvider(params UaTypeDescription[] types) { var registry = new DataTypeDefinitionRegistry(); diff --git a/Tests/Opc.Ua.Gds.Tests/AuthorizationService/InMemoryAccessTokenProviderRefreshTests.cs b/Tests/Opc.Ua.Gds.Tests/AuthorizationService/InMemoryAccessTokenProviderRefreshTests.cs index 58b2ad5b83..35c1aab9ea 100644 --- a/Tests/Opc.Ua.Gds.Tests/AuthorizationService/InMemoryAccessTokenProviderRefreshTests.cs +++ b/Tests/Opc.Ua.Gds.Tests/AuthorizationService/InMemoryAccessTokenProviderRefreshTests.cs @@ -241,11 +241,16 @@ namespace Opc.Ua.Gds.Tests.AuthorizationService private static AuthorizationServiceOptions CreateOptions(Certificate certificate) { - return new AuthorizationServiceOptions + var options = new AuthorizationServiceOptions { IssuerUri = Issuer, SigningCertificate = new CertificateIdentifier { Thumbprint = certificate.Thumbprint } }; + // These tests exercise refresh mechanics, so the operator authorizes + // the requested roles; the fail-closed default (no roles) is covered + // by StartRequestTokenTests. + options.AuthorizeRoles = (identity, audience, requestedRoles) => requestedRoles; + return options; } private static InMemoryAccessTokenProvider CreateProvider( diff --git a/Tests/Opc.Ua.Gds.Tests/AuthorizationService/StartRequestTokenTests.cs b/Tests/Opc.Ua.Gds.Tests/AuthorizationService/StartRequestTokenTests.cs index e6404fd426..847bf346d9 100644 --- a/Tests/Opc.Ua.Gds.Tests/AuthorizationService/StartRequestTokenTests.cs +++ b/Tests/Opc.Ua.Gds.Tests/AuthorizationService/StartRequestTokenTests.cs @@ -53,6 +53,8 @@ namespace Opc.Ua.Gds.Tests.AuthorizationService private const string Issuer = "urn:opcua:test:gds"; private const string Audience = "urn:opcua:test:server"; private static readonly string[] s_requestedRoles = ["operator"]; + private static readonly string[] s_securityAdminRole = ["SecurityAdmin"]; + private static readonly string[] s_operatorAndAdminRoles = ["operator", "SecurityAdmin"]; [Test] public async Task StartThenFinishIssuesJwtAcceptedByJwtAuthenticator() @@ -69,6 +71,9 @@ namespace Opc.Ua.Gds.Tests.AuthorizationService }; options.AllowedAudiences.Add(Audience); options.DefaultScopes.Add("read"); + // The operator explicitly authorizes the requested roles for the + // authenticated caller; without this callback no roles are granted. + options.AuthorizeRoles = (identity, audience, requestedRoles) => requestedRoles; var issuer = new CertificateJwtIssuer(options, certificateProvider, NUnitTelemetryContext.Create()); var provider = new InMemoryAccessTokenProvider(issuer, options); @@ -113,6 +118,236 @@ namespace Opc.Ua.Gds.Tests.AuthorizationService Assert.That(claims.Roles, Does.Contain("operator")); } + [Test] + public async Task FinishDoesNotGrantRequestedRolesWithoutAuthorization() + { + using Certificate certificate = CertificateBuilder + .Create("CN=GDS JWT Signing, O=OPC Foundation") + .SetECCurve(ECCurve.NamedCurves.nistP256) + .CreateForECDsa(); + using var certificateProvider = new InProcessCertificateProvider(certificate); + var options = new AuthorizationServiceOptions + { + IssuerUri = Issuer, + SigningCertificate = new CertificateIdentifier { Thumbprint = certificate.Thumbprint } + }; + options.AllowedAudiences.Add(Audience); + options.DefaultScopes.Add("read"); + // No AuthorizeRoles callback is configured: the provider must be + // fail-closed and grant no roles at all. + + var issuer = new CertificateJwtIssuer(options, certificateProvider, NUnitTelemetryContext.Create()); + var provider = new InMemoryAccessTokenProvider(issuer, options); + var manager = new AuthorizationServiceManager(provider, issuer, options); + + (_, Guid requestId) = await manager + .StartRequestTokenAsync( + Audience, + "jwt", + ByteString.From(Encoding.UTF8.GetBytes("read")), + new UserIdentity("operator", [])) + .ConfigureAwait(false); + + AccessTokenResult tokenResult = await manager + .FinishRequestTokenAsync( + requestId, + s_securityAdminRole.ToArrayOf(), + new UserNameIdentityToken { UserName = "operator" }, + new SignatureData()) + .ConfigureAwait(false); + + IIdentityClaims claims = await AuthenticateAsync(certificate, tokenResult.AccessToken) + .ConfigureAwait(false); + + Assert.That(claims.Subject, Is.EqualTo("operator")); + Assert.That(claims.Roles, Is.Empty, + "A requested role must never be granted unless the operator explicitly authorizes it."); + } + + [Test] + public async Task FinishBindsSubjectToAuthenticatedIdentityNotRequestToken() + { + using Certificate certificate = CertificateBuilder + .Create("CN=GDS JWT Signing, O=OPC Foundation") + .SetECCurve(ECCurve.NamedCurves.nistP256) + .CreateForECDsa(); + using var certificateProvider = new InProcessCertificateProvider(certificate); + var options = new AuthorizationServiceOptions + { + IssuerUri = Issuer, + SigningCertificate = new CertificateIdentifier { Thumbprint = certificate.Thumbprint } + }; + options.AllowedAudiences.Add(Audience); + options.DefaultScopes.Add("read"); + + var issuer = new CertificateJwtIssuer(options, certificateProvider, NUnitTelemetryContext.Create()); + var provider = new InMemoryAccessTokenProvider(issuer, options); + var manager = new AuthorizationServiceManager(provider, issuer, options); + + (_, Guid requestId) = await manager + .StartRequestTokenAsync( + Audience, + "jwt", + ByteString.From(Encoding.UTF8.GetBytes("read")), + new UserIdentity("authenticated-user", [])) + .ConfigureAwait(false); + + // The client attempts to spoof a privileged subject through the + // request token; the issued token must ignore it. + AccessTokenResult tokenResult = await manager + .FinishRequestTokenAsync( + requestId, + Array.Empty().ToArrayOf(), + new UserNameIdentityToken { UserName = "admin" }, + new SignatureData()) + .ConfigureAwait(false); + + IIdentityClaims claims = await AuthenticateAsync(certificate, tokenResult.AccessToken) + .ConfigureAwait(false); + + Assert.That(claims.Subject, Is.EqualTo("authenticated-user"), + "The subject must be bound to the identity authenticated on the session, " + + "not to the client-supplied request token."); + } + + [Test] + public async Task LegacyRequestAccessTokenIssuesAnonymousUnprivilegedToken() + { + using Certificate certificate = CreateSigningCertificate(); + using var certificateProvider = new InProcessCertificateProvider(certificate); + var options = new AuthorizationServiceOptions + { + IssuerUri = Issuer, + SigningCertificate = new CertificateIdentifier { Thumbprint = certificate.Thumbprint } + }; + options.AllowedAudiences.Add(Audience); + options.DefaultScopes.Add("read"); + + var issuer = new CertificateJwtIssuer(options, certificateProvider, NUnitTelemetryContext.Create()); + var provider = new InMemoryAccessTokenProvider(issuer, options); + +#pragma warning disable CS0618 // exercising the obsolete single-call wire method on purpose + string jwt = await provider + .RequestAccessTokenAsync(new UserNameIdentityToken { UserName = "admin" }, Audience) + .ConfigureAwait(false); +#pragma warning restore CS0618 + + IIdentityClaims claims = await AuthenticateAsync(certificate, jwt).ConfigureAwait(false); + Assert.That(claims.Subject, Is.EqualTo("anonymous"), + "The obsolete single-call RequestAccessToken must not trust the client-supplied " + + "token for the subject."); + Assert.That(claims.Roles, Is.Empty, + "The obsolete single-call RequestAccessToken must not grant roles."); + } + + [Test] + public async Task FinishGrantsOnlyTheIntersectionOfAuthorizedAndRequestedRoles() + { + using Certificate certificate = CreateSigningCertificate(); + using var certificateProvider = new InProcessCertificateProvider(certificate); + var options = new AuthorizationServiceOptions + { + IssuerUri = Issuer, + SigningCertificate = new CertificateIdentifier { Thumbprint = certificate.Thumbprint } + }; + options.AllowedAudiences.Add(Audience); + options.DefaultScopes.Add("read"); + // The callback tries to grant an extra, unrequested role; it must be + // dropped because the granted set is intersected with the request. + options.AuthorizeRoles = (identity, audience, requestedRoles) => s_operatorAndAdminRoles; + + var issuer = new CertificateJwtIssuer(options, certificateProvider, NUnitTelemetryContext.Create()); + var provider = new InMemoryAccessTokenProvider(issuer, options); + var manager = new AuthorizationServiceManager(provider, issuer, options); + + (_, Guid requestId) = await manager + .StartRequestTokenAsync( + Audience, + "jwt", + ByteString.From(Encoding.UTF8.GetBytes("read")), + new UserIdentity("operator", [])) + .ConfigureAwait(false); + + AccessTokenResult tokenResult = await manager + .FinishRequestTokenAsync( + requestId, + s_requestedRoles.ToArrayOf(), + new UserNameIdentityToken { UserName = "operator" }, + new SignatureData()) + .ConfigureAwait(false); + + IIdentityClaims claims = await AuthenticateAsync(certificate, tokenResult.AccessToken) + .ConfigureAwait(false); + Assert.That(claims.Roles, Does.Contain("operator")); + Assert.That(claims.Roles, Does.Not.Contain("SecurityAdmin"), + "A role the caller did not request must never be granted, even if the " + + "authorization callback returns it."); + } + + [Test] + public async Task FinishGrantsNoRolesWhenAuthorizationReturnsEmpty() + { + using Certificate certificate = CreateSigningCertificate(); + using var certificateProvider = new InProcessCertificateProvider(certificate); + var options = new AuthorizationServiceOptions + { + IssuerUri = Issuer, + SigningCertificate = new CertificateIdentifier { Thumbprint = certificate.Thumbprint } + }; + options.AllowedAudiences.Add(Audience); + options.DefaultScopes.Add("read"); + options.AuthorizeRoles = (identity, audience, requestedRoles) => []; + + var issuer = new CertificateJwtIssuer(options, certificateProvider, NUnitTelemetryContext.Create()); + var provider = new InMemoryAccessTokenProvider(issuer, options); + var manager = new AuthorizationServiceManager(provider, issuer, options); + + (_, Guid requestId) = await manager + .StartRequestTokenAsync( + Audience, + "jwt", + ByteString.From(Encoding.UTF8.GetBytes("read")), + new UserIdentity("operator", [])) + .ConfigureAwait(false); + + AccessTokenResult tokenResult = await manager + .FinishRequestTokenAsync( + requestId, + s_requestedRoles.ToArrayOf(), + new UserNameIdentityToken { UserName = "operator" }, + new SignatureData()) + .ConfigureAwait(false); + + IIdentityClaims claims = await AuthenticateAsync(certificate, tokenResult.AccessToken) + .ConfigureAwait(false); + Assert.That(claims.Roles, Is.Empty, + "An authorization callback that returns no roles must yield a token with no roles."); + } + + private static Certificate CreateSigningCertificate() + { + return CertificateBuilder + .Create("CN=GDS JWT Signing, O=OPC Foundation") + .SetECCurve(ECCurve.NamedCurves.nistP256) + .CreateForECDsa(); + } + + private static async Task AuthenticateAsync(Certificate certificate, string jwt) + { + ECDsa verifier = certificate.GetECDsaPublicKey(); + using var resolver = new StaticIssuerKeyResolver( + Issuer, + [new IssuerVerificationKey(certificate.Thumbprint, verifier, "ES256")]); + AuthenticationResult result = await new JwtAuthenticator(resolver, Audience, TimeSpan.Zero) + .AuthenticateAsync(CreateContext(jwt)) + .ConfigureAwait(false); + + Assert.That(result.Outcome, Is.EqualTo(AuthenticationOutcome.Accepted)); + var claims = result.Identity as IIdentityClaims; + Assert.That(claims, Is.Not.Null); + return claims!; + } + private static AuthenticationContext CreateContext(string jwt) { return new AuthenticationContext( diff --git a/Tests/Opc.Ua.PubSub.Tests/Connections/PubSubConnectionSecurityReceiveTests.cs b/Tests/Opc.Ua.PubSub.Tests/Connections/PubSubConnectionSecurityReceiveTests.cs index 6a68ad886c..bc38722231 100644 --- a/Tests/Opc.Ua.PubSub.Tests/Connections/PubSubConnectionSecurityReceiveTests.cs +++ b/Tests/Opc.Ua.PubSub.Tests/Connections/PubSubConnectionSecurityReceiveTests.cs @@ -87,6 +87,36 @@ namespace Opc.Ua.PubSub.Tests.Connections "Forged plaintext frame must be dropped before decode."); } + [Test] + public async Task SecuredReaderRejectsNonUadpJsonFrameAsync() + { + // A reader configured for message security must drop a non-UADP + // (JSON) frame: there is no JSON message-security wrapper, so the + // frame cannot be authenticated and must never reach the decoder. + // Before the fix the inbound security gate lived inside the UADP + // branch, so a JSON frame bypassed it entirely and was delivered + // to application sinks unauthenticated. + (UadpSecurityWrapper _, UadpSecurityWrapper subscriber) = + CreateMatchingWrapperPair(tokenId: 1U); + + byte[] json = System.Text.Encoding.UTF8.GetBytes( + "{\"MessageId\":\"forged\",\"MessageType\":\"ua-data\"}"); + var transport = new ProgrammableTransport([json]); + var decoder = new RecordingDecoder(); + + await using PubSubConnection conn = NewConnection( + transport, decoder, subscriber, + MessageSecurityMode.SignAndEncrypt); + + await conn.EnableAsync().ConfigureAwait(false); + await transport.WaitUntilDrainedAsync().ConfigureAwait(false); + await conn.DisableAsync().ConfigureAwait(false); + + Assert.That(decoder.CallCount, Is.Zero, + "A non-UADP (JSON) frame must be dropped by the inbound security " + + "gate on a secured reader before decode."); + } + [Test] public async Task SecuredReaderAcceptsSecuredFrameAsync() { diff --git a/Tests/Opc.Ua.Types.Tests/Utils/FileSystem/NullFileSystemTests.cs b/Tests/Opc.Ua.Types.Tests/Utils/FileSystem/NullFileSystemTests.cs new file mode 100644 index 0000000000..3736029079 --- /dev/null +++ b/Tests/Opc.Ua.Types.Tests/Utils/FileSystem/NullFileSystemTests.cs @@ -0,0 +1,92 @@ +/* ======================================================================== + * Copyright (c) 2005-2026 The OPC Foundation, Inc. All rights reserved. + * + * OPC Foundation MIT License 1.00 + * + * Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * The complete license agreement can be found here: + * http://opcfoundation.org/License/MIT/1.00/ + * ======================================================================*/ + +using System.IO; +using NUnit.Framework; + +namespace Opc.Ua.Types.Tests.Utils.FileSystem +{ + /// + /// Tests for the deny-all . + /// + [TestFixture] + [Category("FileSystem")] + [SetCulture("en-us")] + [SetUICulture("en-us")] + [Parallelizable] + public class NullFileSystemTests + { + [Test] + public void ExistsAlwaysReturnsFalse() + { + Assert.That(NullFileSystem.Instance.Exists("anything", isDirectory: false), Is.False); + Assert.That(NullFileSystem.Instance.Exists("anything", isDirectory: true), Is.False); + } + + [Test] + public void OpenReadThrows() + { + Assert.That( + () => NullFileSystem.Instance.OpenRead(@"\\attacker\share\x"), + Throws.InstanceOf()); + } + + [Test] + public void OpenWriteThrows() + { + Assert.That( + () => NullFileSystem.Instance.OpenWrite("x"), + Throws.InstanceOf()); + } + + [Test] + public void DeleteThrows() + { + Assert.That( + () => NullFileSystem.Instance.Delete("x", isDirectory: false), + Throws.InstanceOf()); + } + + [Test] + public void GetLengthThrows() + { + Assert.That( + () => NullFileSystem.Instance.GetLength("x"), + Throws.InstanceOf()); + } + + [Test] + public void GetLastWriteTimeThrows() + { + Assert.That( + () => NullFileSystem.Instance.GetLastWriteTime("x"), + Throws.InstanceOf()); + } + } +}