mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 22:55:08 +08:00
a0d569d756
## Summary Fixes the findings of a bug audit of `Opc.Ua.Server`, `Opc.Ua.Gds.Server` and `Opc.Ua.Gds.Client`. Rebased on master after #4516. What went in: - **Audit:** 93 findings, each checked against the OPC UA specification. - **Code review of those fixes:** 35 further findings. - **Spec grounding of every fix commit:** 18 conformance gaps, now closed. Commits are one per finding, named `Fix <ID>`, `Fix review <ID>` or `Fix spec gap <name>`. The IDs refer to the audit report (S = Server areas, G1 = GDS server, G2 = GDS client). Validation: - Each finding was traced in code and checked against OPC 10000 (parts 3, 4, 5, 9, 11, 12, 13, 18, 20). - Every fix commit was checked against the spec text again afterwards: 0 non-conformant, remaining gaps fixed. - Regression tests were added for most fixes. - Suites run on net10.0 before the rebase: Server, GDS, History, Configuration, Features, Core.Security, DI, Redundancy, LDS, InformationModel, PubSub.Server, Client, Sessions and the three Subscriptions suites, all green. The same suites on the rebased branch are still running; results will be posted as a comment. ## Behaviour and user-facing changes ### Secure defaults - **`Historize()` / `HistorianBuilder`** grant HistoryRead only. History writes need an explicit `historyAccessLevel: HistoryRead | HistoryWrite`. - **File-system mount:** - Anonymous sessions can no longer create, delete, move or open files for writing (`Bad_UserAccessDenied`). - UserWritable and UserExecutable now reflect the user. - `AllowAnonymousWrite` restores the old behaviour; the ReferenceServer sample turns it on. - **Default PKI root** for the hosted server, hosted GDS, hosted LDS and `OpcUaApplicationConfigurationProvider`: - Moved from the shared temp directory to `{LocalApplicationData}/OPC Foundation/{ApplicationName}/pki`, and startup fails if no per-user directory exists. - When certificate stores of an earlier version still exist under temp, a warning names both paths. Existing deployments that relied on the default get a new application certificate (and the hosted GDS a new CA) unless `PkiRoot` is set to the old path. - **GDS:** - `GdsServerOptions.AutoApprove` defaults to `false`. - The default KeyCredential store grants no roles unless `AuthorizeRoles` allows them. - Certificate requests (StartSigningRequest, StartNewKeyPairRequest, FinishRequest) need an encrypted channel. - **Hosted server anonymous logins:** - Anonymous is rejected (`Bad_IdentityTokenRejected`) when the default authenticator options disable it. - A custom `AddIdentityAuthenticator<T>()` on its own no longer disables anonymous. - With anonymous disabled and no configured token policies, the endpoint advertises the authenticators' token types instead of Anonymous. - **Runtime-added roles:** their properties and methods need SecurityAdmin over an encrypted channel, like the well-known roles. - **Disabled users:** users disabled through Part 18 UserManagement can no longer log in through `RegisterDefaultAuthenticators(IUserDatabase)`. ### GDS server (Part 12) - **Certificate requests:** - FinishRequest / ReadRequest with another application's RequestId returns `Bad_InvalidArgument`. - Every Start*Request returns a new RequestId. - A CSR without the ApplicationUri is rejected (`Bad_CertificateUriInvalid`). - An abstract or unsupported certificate type is rejected at Start (`Bad_InvalidArgument`). - **Private-key password:** no longer persisted in the JSON applications database. A password saved by an earlier version is read into memory once and dropped on the next save. - **CheckRevocationStatus:** validates offline against the GDS's own trust lists and CRLs. It returns Good for good GDS-issued certificates and `Bad_CertificateInvalid` for input that is not a certificate. - **Certificate-group trust lists:** - Writes need CertificateAuthorityAdmin; SecurityAdmin and application admins are read-only. - Writes are validated with the Part 4 certificate validation. - **AuthorizationService:** - The legacy RequestAccessToken applies `AccessControl` and issues the token for the session identity. - StartRequestToken passes the session identity to every identity-aware provider. - Unknown ResourceIds return `Bad_NotFound`. - Expired token requests and refresh tokens are pruned, with at most 10 000 pending requests (`Bad_TooManyOperations`). - **KeyCredential:** requests with PublicKey + SecurityPolicyUri are rejected with `Bad_SecurityPolicyRejected` (secret encryption is not implemented); a PublicKey without SecurityPolicyUri is rejected with `Bad_InvalidArgument`. - **Audit events:** access-token audit events no longer contain the password or token data. - **RegisterTickets / UnregisterTickets:** need the RegistrarAdmin role when called from a session. - **Managed application nodes:** moved from namespace 0 to their own namespace (`ManagedApplicationsNamespaceUri`); the NodeId is the ApplicationUri, and the mandatory child properties exist. - **Hosted GDS:** - New `GdsServerOptions.CertificateGroups` (`GdsCertificateGroupOptions`), bindable from configuration. - The mandatory DefaultApplicationGroup is always present. - The default CA subject uses the resolved application name. - **JSON applications database:** - A corrupt file throws instead of being silently replaced by an empty database. - Saves are atomic and write in place on bind mounts; on net8+ builds they also follow symlinks and keep the Unix file mode. - Issued certificates and trust-list links survive a reload. ### GDS client - **Trust-list download:** - Reads until an empty chunk (Part 20 short reads). - A size-limit rejection of the final end-of-file read, as sent by released servers, counts as end of file. - **Default certificate group:** a null group means DefaultApplicationGroup, and vendor groups in other namespaces are no longer mistaken for well-known groups. - **Status codes and errors:** - Missing directory proxies report `Bad_ServerNotConnected`. - Short or wrongly typed method outputs throw `BadUnexpectedError` / `BadTypeMismatch`. - **GdsAccessTokenProvider:** no longer caches a failed client task, and one caller's cancellation no longer fails other callers. ### Trust lists, push and ConfigurationFile (Part 12 / Part 20) - **AddCertificate:** - `IsTrustedCertificate=false` returns `Bad_CertificateInvalid`. - Added certificates are validated (Part 4), and a certificate revoked by a current CRL returns `Bad_CertificateRevoked`. - **CloseAndUpdate:** validates the uploaded list against its own certificates and CRLs, not the stores it replaces. Validation failures are no longer recorded in the server's Rejected store. - **RemoveCertificate:** removing a trusted or issuer CA that other certificates still need returns `Bad_CertificateChainIncomplete`. - **Handles:** - Several read handles may be open at once. - Any open while a write handle exists returns `Bad_NotReadable` / `Bad_NotWritable`. - A write open while any handle is open returns `Bad_NotWritable`. - Handles of closed sessions are released. - Idle handles close after ActivityTimeout (default 60 s). - **Status codes:** - Read clamps to the remaining bytes and no longer enforces the size limit. - Oversized Write / AddCertificate returns `Bad_RequestTooLarge`. - Unsupported open modes return `Bad_NotSupported`. - **ConfigurationFile.CloseAndUpdate:** - Empty Targets return `Bad_InvalidArgument`. - The ConfigurationUpdatedAuditEvent is raised with the configuration owner as source. - With the new `IApplicationConfigurationFileTargetProvider`, providers receive the Targets, a bad target returns Uncertain with per-target results, and session-interrupting updates are applied after RestartDelayTime. - **Default namespace permissions:** namespaces registered after the first permission lookup now get their NamespaceMetadata default RolePermissions and AccessRestrictions (no stale negative cache). ### Sessions, identity and roles (Part 18) - **Session timeouts:** a failure closing one timed-out session no longer stops the session-timeout monitor. - **ChangePassword:** an empty old password returns `Bad_IdentityTokenInvalid`. - **Removing or disabling a user:** closes only that user's UserName-token sessions. - **Role configuration changes:** roles granted by an `ImpersonateUser` mapping are kept. - **X509Subject rules:** a certificate subject value containing a quote never matches an X509Subject rule. - **`ImpersonateUser` callback:** no longer runs under the global session lock. - **Session diagnostics:** LocaleIds are kept as the client sent them, while null LocaleIds are ignored for translation. ### Subscriptions and monitored items (Part 4) - **Acknowledgements:** a failing acknowledgement returns a per-entry Bad status instead of failing the whole Publish. - **`DiscardedMessageCount`:** counts messages dropped from the retransmission queue and unsent overflow messages. - **Durable subscriptions:** transferring a restored anonymous subscription works (no NullReferenceException). - **Transferred subscriptions:** sampling and permission checks use the item's current owner session. - **Access denied:** a monitored item whose user may no longer read the attribute gets a `Bad_UserAccessDenied` notification instead of going silent. Non-Value attributes need Browse permission (RolePermissions needs ReadRolePermissions), and User* attributes are evaluated per subscriber. - **ModifyMonitoredItems:** a SamplingInterval of -1 uses the subscription's publishing interval. - **AggregateFilter priming:** needs HistoryRead / ReadHistory permission and is bounded (100 000 values, bounded page size). - **Resource leak:** event monitored items no longer leak a consumer task per create/delete cycle. ### Node management services (Part 3 / Part 4) - **Reference types:** AddNodes, AddReferences, DeleteReferences and Browse reject ids that are not ReferenceTypes with `Bad_ReferenceTypeIdInvalid`. AddNodes, AddReferences and DeleteReferences also reject abstract ReferenceTypes; Browse still accepts them (e.g. `HierarchicalReferences` with includeSubtypes). - **Concurrent AddNodes:** duplicate BrowseNames under one parent get `Bad_BrowseNameDuplicated`. - **TranslateBrowsePathsToNodeIds:** - Requires Browse permission on intermediate nodes and final targets (RolePermissions and AccessRestrictions). - A long RelativePath no longer rejects the whole request. - **SemanticChangeEvent:** - Raised once per actual change of a semantic property, whether or not it is monitored. - Also raised for properties with the SemanticChange access-level bit. - Reports the owning node's TypeDefinition as AffectedType. - **Localized diagnostics:** operation-level text is translated when OperationLocalizedText is requested. ### History and aggregates (Part 11 / Part 13) - **DeleteRawModified:** an unspecified or reversed range returns `Bad_InvalidTimestampArgument`, and StartTime == EndTime deletes the value at StartTime for every provider. - **ReadAtTime fallback:** reads only the values around each requested time. A time whose bound is not found within the search limit returns `Bad_BoundNotSupported` instead of failing the request. - **ReadProcessed:** a ProcessingInterval shorter than one tick returns `Bad_AggregateInvalidInputs`. - **Delta / DeltaBounds:** widen to the next larger signed type when the result does not fit the source type (e.g. a decreasing unsigned counter, or an Int16 overflow). - **Range / Range2:** return the source data type. ### Alarms and fluent API (Part 9) - **Suppression:** an alarm stays suppressed while any suppression group is active, and re-evaluation without a state change does not move TransitionTime. - **`MonitorVariable(variable)`:** sets SourceNode, SourceName and InputNode to the variable and makes it a ConditionSource (HasCondition to the alarm, HasEventSource from the object). - **Fluent input/output arguments:** InputArguments / OutputArguments added through the fluent API to a registered method are readable. - **Root notifiers:** a failed root-notifier registration no longer leaves an orphaned event worker. - **State-machine handlers:** exceptions are logged at Error. ### File system (Part 20) - **Delete / MoveOrCopy:** - Only act on direct children of the directory. - A NodeId whose kind does not match the entry returns `Bad_NotFound` (a file NodeId can no longer delete a directory). - Copying a directory into its own subtree returns `Bad_InvalidArgument`. - **Locking:** - An open file, or a directory containing one, cannot be deleted or moved (`Bad_InvalidState`). - The check is atomic with opens. - While a Delete or Move runs, read opens get `Bad_NotReadable` and write opens `Bad_InvalidState`. - **Open:** - Reserved mode bits or EraseExisting without Write return `Bad_InvalidArgument`. - Read+Write returns `Bad_NotSupported`. - Read while a writer is open returns `Bad_NotReadable`. - **Client `FileSystemErrors`:** maps `Bad_NotReadable` to `IOException`. - **RuntimeNodeSet:** well-formed NodeSets with a stray `Model` element, or two models in one file where one requires the other, now load. ### Server core, hosting and lifecycle - **`SessionEstablishmentQueueLimit` > 0:** CreateSession/ActivateSession now queue for a permit. The permit is acquired before the request is registered, and a waiting handshake releases its worker through the request-parking policy. - **Node manager lifecycle:** - Add, reload and remove no longer wait for parked Publish requests. - A Migrate reload racing with CreateMonitoredItems no longer blocks later lifecycle operations. - Late items on retired or removed managers get `Bad_NodeIdUnknown`. - **HistoryUpdate:** a batch mixing data and event details is bounded by both limits. - **Server stop/start:** stopping the server disposes the configuration watcher and certificate subscription; starting recreates them. - **Monitored-item queue factory:** a factory supplied by the caller or the DI container is no longer disposed by the server. - **Audit events:** each certificate validation error raises its own AuditCertificate event. A failed OpenSecureChannel without a service result is audited as failed. ## New public API (additive) - `IQueuedSessionEstablishmentLimiter` - `StandardServer.OwnsMonitoredItemQueueFactory` (protected virtual) - `DefaultPkiRoot.Get` (Opc.Ua.Configuration) - `TrustList.SetCertificateValidation` - `NamespaceMetadataRegistry.Invalidate` - `IApplicationConfigurationFileTargetProvider` and `ApplicationConfigurationUpdatePlan` - `GdsServerOptions.CertificateGroups` / `GdsCertificateGroupOptions` - `InMemoryKeyCredentialRequestStore.AuthorizeRoles` - `DefaultManagedApplicationsNodeManager.ManagedApplicationsNamespaceUri` - `AuthorizationServiceManager.RequestAccessTokenAsync(..., IUserIdentity?, ...)` - `AllowAnonymousWrite` on the file-system node manager and its factory - `IFileSystemHost.TryBeginMutation` / `EndMutation`, replacing `HasOpenHandles` ## Not fixed / known limitations - **KeyCredential:** secret encryption with the client's PublicKey (requests needing it are rejected). - **CSR SAN restriction:** restricting subject / SAN values copied from a CSR (CA policy). - **Ticket signatures:** ticket signature verification (needs a trusted-signer configuration). - **FileType Read+Write open:** needs a read/write stream contract in `IFileSystemProvider`. - **Custom `IUserDatabase` implementations:** the disabled-user check still scans all users per login. The built-in stores do a single lookup. - **64-bit Delta precision:** limited by the calculator's double arithmetic. ## Tests with changed expectations Existing tests were updated where they asserted the old behaviour: - File-system status codes - Trust-list handle eviction - `DiscardedMessageCount` - Localized diagnostics bits - Delta on unsigned sources - SourceNode of fluent alarms - ReferenceServer transfer permission test (now expects `Bad_UserAccessDenied` notifications) - `UserDeactivationShutdownReviewTests` (the revoked session is a UserName login) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
170 lines
7.3 KiB
C#
170 lines
7.3 KiB
C#
/* ========================================================================
|
|
* Copyright (c) 2005-2025 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;
|
|
using System.IO;
|
|
using NUnit.Framework;
|
|
using Opc.Ua.Client.FileSystem;
|
|
|
|
namespace Opc.Ua.Client.Tests.FileSystem
|
|
{
|
|
/// <summary>
|
|
/// Unit tests for the internal <c>FileSystemErrors</c> mapper.
|
|
/// </summary>
|
|
[TestFixture]
|
|
[Category("FileSystem")]
|
|
[Parallelizable]
|
|
public class FileSystemErrorsTests
|
|
{
|
|
[Test]
|
|
public void BadNoMatchOnDirectoryReturnsDirectoryNotFoundException()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadNoMatch);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo/bar", targetIsDirectory: true);
|
|
Assert.That(mapped, Is.InstanceOf<DirectoryNotFoundException>());
|
|
Assert.That(mapped.Message, Does.Contain("/foo/bar"));
|
|
Assert.That(mapped.InnerException, Is.SameAs(ex));
|
|
}
|
|
|
|
[Test]
|
|
public void BadNoMatchOnFileReturnsFileNotFoundException()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadNoMatch);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo/bar.txt", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<FileNotFoundException>());
|
|
Assert.That(((FileNotFoundException)mapped).FileName, Is.EqualTo("/foo/bar.txt"));
|
|
}
|
|
|
|
[Test]
|
|
public void BadNotFoundMapsToNotFound()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadNotFound);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/x", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<FileNotFoundException>());
|
|
}
|
|
|
|
[Test]
|
|
public void BadNodeIdUnknownMapsToNotFound()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadNodeIdUnknown);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/x", targetIsDirectory: true);
|
|
Assert.That(mapped, Is.InstanceOf<DirectoryNotFoundException>());
|
|
}
|
|
|
|
[Test]
|
|
public void BadBrowseNameDuplicatedMapsToIOException()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadBrowseNameDuplicated);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<IOException>());
|
|
Assert.That(mapped.Message, Does.Contain("already exists"));
|
|
}
|
|
|
|
[Test]
|
|
public void BadUserAccessDeniedMapsToUnauthorizedAccessException()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadUserAccessDenied);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<UnauthorizedAccessException>());
|
|
Assert.That(mapped.Message, Does.Contain("access denied"));
|
|
}
|
|
|
|
[Test]
|
|
public void BadNotWritableMapsToUnauthorizedAccessException()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadNotWritable);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<UnauthorizedAccessException>());
|
|
}
|
|
|
|
[Test]
|
|
public void BadResourceUnavailableMapsToIOExceptionNotUnauthorized()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadResourceUnavailable);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<IOException>());
|
|
Assert.That(mapped, Is.Not.InstanceOf<UnauthorizedAccessException>());
|
|
}
|
|
|
|
[Test]
|
|
public void BadInvalidStateMapsToIOException()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadInvalidState);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<IOException>());
|
|
}
|
|
|
|
[Test]
|
|
public void BadNotReadableMapsToIOExceptionNotUnauthorized()
|
|
{
|
|
// The server's FileType Open reports a file locked for writing
|
|
// as Bad_NotReadable, which is a sharing violation, not an
|
|
// access denial.
|
|
var ex = new ServiceResultException(StatusCodes.BadNotReadable);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.InstanceOf<IOException>());
|
|
Assert.That(mapped, Is.Not.InstanceOf<UnauthorizedAccessException>());
|
|
}
|
|
|
|
[Test]
|
|
public void UnmappedStatusCodePassesThroughOriginalException()
|
|
{
|
|
var ex = new ServiceResultException(StatusCodes.BadInternalError);
|
|
Exception mapped = FileSystemErrors.Translate(ex, "/foo", targetIsDirectory: false);
|
|
Assert.That(mapped, Is.SameAs(ex));
|
|
}
|
|
|
|
[Test]
|
|
public void NotFoundFactoryReturnsCorrectExceptionType()
|
|
{
|
|
Exception fileException = FileSystemErrors.NotFound("/x.txt", targetIsDirectory: false);
|
|
Exception dirException = FileSystemErrors.NotFound("/x", targetIsDirectory: true);
|
|
Assert.That(fileException, Is.InstanceOf<FileNotFoundException>());
|
|
Assert.That(((FileNotFoundException)fileException).FileName, Is.EqualTo("/x.txt"));
|
|
Assert.That(dirException, Is.InstanceOf<DirectoryNotFoundException>());
|
|
}
|
|
|
|
[Test]
|
|
public void AmbiguousFactoryReturnsIOExceptionWithCount()
|
|
{
|
|
IOException ex = FileSystemErrors.Ambiguous("/foo", 3);
|
|
Assert.That(ex.Message, Does.Contain("ambiguous"));
|
|
Assert.That(ex.Message, Does.Contain("3"));
|
|
Assert.That(ex.Message, Does.Contain("/foo"));
|
|
}
|
|
|
|
[Test]
|
|
public void TranslateNullExceptionThrowsArgumentNullException()
|
|
{
|
|
Assert.Throws<ArgumentNullException>(
|
|
() => FileSystemErrors.Translate(null!, "/x", false));
|
|
}
|
|
}
|
|
}
|