mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-29 15:11:53 +08:00
Server: accept acknowledgements of messages retired by a user change (#4911)
# Description Since #4834, an `ActivateSession` that changes the user retires the subscription's retransmission queue, so the new user can never republish data authorized for the previous one. A side effect: when the client then acknowledges a message it already received under the previous user, the Publish result is `Bad_SequenceNumberUnknown`. The CTT nightly on master `f3980f9e7` reported this as a regression in **Session Change User / 001.js**. A rerun of p15/p16 on the same commit reproduced it. The fix: `SentMessageQueue.InvalidateAuthorization` now remembers the sequence numbers of the retired messages that were already sent, and `TryAcknowledge` accepts them. Republish of those messages is still refused with `Bad_MessageNotAvailable`, so #4834's protection is unchanged. Part 4 5.7.3.1 allows a discontinuity on a user change, and the client's acknowledgement of a message it received stays valid. A later user change replaces the set, so its size stays bounded by the message queue. A new regression test, `SecurityR37ChangedUserAcknowledgesDeliveredMessageAndKeepsPublishingAsync`, checks that the acknowledgement succeeds after a user change. It also checks that values queued after the change are still delivered. The second regression in that nightly, Subscription Transfer / Err-011.js (`Bad_IdentityTokenRejected`), was a CTT setup issue and is fixed in the private CTT repository. #4834 stopped the reference server's `-a` from auto-accepting X509 *user* certificates, and the CTT's main server PKI had no user trust list. Verification: CTT run 40347120243 (p15-session and p16-subscription, this commit plus the CTT script fix) shows no regressions against the baseline. ## Related Issues - Follow-up to #4834 ## 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. - [ ] 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>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
9e48f3d04d
commit
0c727b30ed
@@ -430,7 +430,7 @@ namespace Opc.Ua.Server
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
return m_retiredSequenceNumbers?.Remove(sequenceNumber) == true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -572,9 +572,18 @@ namespace Opc.Ua.Server
|
||||
public void InvalidateAuthorization()
|
||||
{
|
||||
uint[] removedSequenceNumbers = new uint[m_sent.Count];
|
||||
int sentCount = Math.Min(m_lastSentMessage, m_sent.Count);
|
||||
// a later identity change replaces the set, which keeps it bounded by the queue size.
|
||||
m_retiredSequenceNumbers = sentCount > 0 ? [] : null;
|
||||
for (int ii = 0; ii < m_sent.Count; ii++)
|
||||
{
|
||||
removedSequenceNumbers[ii] = m_sent[ii].Message.SequenceNumber;
|
||||
if (ii < sentCount)
|
||||
{
|
||||
// Part 4 5.7.3.1 allows the discontinuity, but the client already received
|
||||
// these, so its acknowledgement of them stays valid.
|
||||
m_retiredSequenceNumbers!.Add(removedSequenceNumbers[ii]);
|
||||
}
|
||||
}
|
||||
if (m_retransmissionStore is ISubscriptionRetransmissionDeltaStore deltaStore)
|
||||
{
|
||||
@@ -659,6 +668,12 @@ namespace Opc.Ua.Server
|
||||
private uint m_sequenceNumber;
|
||||
private int m_lastSentMessage;
|
||||
private uint m_mirroredFirstUnsentSequenceNumber;
|
||||
|
||||
/// <summary>
|
||||
/// Sequence numbers of messages the client received before the last authorization epoch
|
||||
/// retired them; they can still be acknowledged but never republished.
|
||||
/// </summary>
|
||||
private HashSet<uint>? m_retiredSequenceNumbers;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -405,6 +405,39 @@ namespace Opc.Ua.Server.Tests
|
||||
.EqualTo(StatusCodes.BadMessageNotAvailable));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A changed user keeps an active subscription: acknowledging a message the client already
|
||||
/// received succeeds and later values are delivered (CTT Session Change User 001).
|
||||
/// </summary>
|
||||
[Test]
|
||||
[Category("SecurityRegression")]
|
||||
public async Task SecurityR37ChangedUserAcknowledgesDeliveredMessageAndKeepsPublishingAsync()
|
||||
{
|
||||
await using var harness = new Harness();
|
||||
await harness.InitializeAsync().ConfigureAwait(false);
|
||||
Subscription subscription = await harness.CreateValueSubscriptionAsync().ConfigureAwait(false);
|
||||
harness.Clock.Advance(TimeSpan.FromMilliseconds(101));
|
||||
Assert.That(subscription.PublishTimerExpired(), Is.EqualTo(PublishingState.NotificationsAvailable));
|
||||
uint sequence;
|
||||
using (OperationContext context = await harness.AdmitAsync(RequestType.Publish).ConfigureAwait(false))
|
||||
{
|
||||
NotificationMessage original = subscription.Publish(context, out _, out _)!;
|
||||
sequence = original.SequenceNumber;
|
||||
AssertValue(original, 42);
|
||||
}
|
||||
harness.SetObserver(true, "mallory");
|
||||
await harness.ActivateAsync("mallory").ConfigureAwait(false);
|
||||
await harness.QueueValueAsync(43).ConfigureAwait(false);
|
||||
harness.Clock.Advance(TimeSpan.FromMilliseconds(101));
|
||||
Assert.That(subscription.PublishTimerExpired(), Is.EqualTo(PublishingState.NotificationsAvailable));
|
||||
using OperationContext next = await harness.AdmitAsync(RequestType.Publish).ConfigureAwait(false);
|
||||
ServiceResult? acknowledged = subscription.Acknowledge(next, sequence);
|
||||
NotificationMessage? message = subscription.Publish(next, out _, out _);
|
||||
Assert.That(message, Is.Not.Null);
|
||||
AssertValue(message!, 43);
|
||||
Assert.That(acknowledged, Is.Null);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reactivation of the same principal retains sequence, payload and acknowledgement semantics.
|
||||
/// </summary>
|
||||
@@ -630,11 +663,11 @@ namespace Opc.Ua.Server.Tests
|
||||
/// <summary>
|
||||
/// Changes the sole observer mapping using the real role manager.
|
||||
/// </summary>
|
||||
public void SetObserver(bool granted)
|
||||
public void SetObserver(bool granted, string userName = "alice")
|
||||
{
|
||||
var rule = new IdentityMappingRuleType
|
||||
{
|
||||
CriteriaType = IdentityCriteriaType.UserName, Criteria = "alice"
|
||||
CriteriaType = IdentityCriteriaType.UserName, Criteria = userName
|
||||
};
|
||||
ServiceResult result = granted
|
||||
? m_roles.AddIdentity(ObjectIds.WellKnownRole_Observer, rule)
|
||||
|
||||
Reference in New Issue
Block a user