fix(mgmt): re-assert MxGateway protocol at secured-write execute (D2 TOCTOU guard, T14b)
This commit is contained in:
@@ -1045,6 +1045,35 @@ public class ManagementActor : ReceiveActor
|
||||
await EmitSecuredWriteAuditAsync(
|
||||
sp, AuditKind.SecuredWriteApprove, AuditStatus.Submitted, row, actor: user.Username);
|
||||
|
||||
// D2 / T14 TOCTOU guard: re-assert the MxGateway protocol AT EXECUTE, not only at
|
||||
// submit. The connection named on the row may have been reconfigured/recreated as a
|
||||
// non-MxGateway (e.g. OPC UA) connection between submit and approval; relaying then
|
||||
// would execute the secured write against a non-MxGateway adapter, violating the
|
||||
// feature's core safety invariant. Re-load the connection with the same lookup
|
||||
// submit uses (resolve site -> its data connections -> match by name) and fail the
|
||||
// row deterministically (mirrors the decode-error containment below) if it is
|
||||
// missing or no longer MxGateway — WITHOUT relaying.
|
||||
var siteRepo = sp.GetRequiredService<ISiteRepository>();
|
||||
var execSite = await siteRepo.GetSiteByIdentifierAsync(row.SiteId);
|
||||
var execConn = execSite is null
|
||||
? null
|
||||
: (await siteRepo.GetDataConnectionsBySiteIdAsync(execSite.Id))
|
||||
.FirstOrDefault(c => string.Equals(c.Name, row.ConnectionName, StringComparison.Ordinal));
|
||||
if (execConn is null ||
|
||||
!string.Equals(execConn.Protocol, "MxGateway", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
row.Status = "Failed";
|
||||
row.ExecutedAtUtc = DateTime.UtcNow;
|
||||
row.ExecutionError = execConn is null
|
||||
? $"connection '{row.ConnectionName}' not found"
|
||||
: $"connection '{row.ConnectionName}' is no longer an MxGateway connection";
|
||||
await repo.UpdateAsync(row);
|
||||
await EmitSecuredWriteAuditAsync(
|
||||
sp, AuditKind.SecuredWriteExecute, AuditStatus.Failed, row,
|
||||
actor: user.Username, errorMessage: row.ExecutionError);
|
||||
return ToSecuredWriteDto(row);
|
||||
}
|
||||
|
||||
// Validate the value type BEFORE attempting the relay. An unknown type can
|
||||
// never be decoded/written, so fail the row deterministically rather than
|
||||
// leaving it stuck Approved. (Addresses the C2 reviewer's deferred
|
||||
|
||||
@@ -225,6 +225,12 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
|
||||
SubmittedAtUtc = DateTime.UtcNow
|
||||
};
|
||||
_securedWriteRepo.GetAsync(id, Arg.Any<CancellationToken>()).Returns(row);
|
||||
// The approve handler re-asserts the MxGateway protocol AT EXECUTE (D2 / T14
|
||||
// TOCTOU guard), so the connection named on the row must still resolve to an
|
||||
// MxGateway connection at approve-time for the relay to proceed. Seed it here
|
||||
// so every approve test that should relay sees a valid MxGateway target; tests
|
||||
// exercising the reconfigured/missing-connection case re-stub this lookup.
|
||||
SeedSiteWithConnection(1, "SITE1", "Mx1", "MxGateway");
|
||||
return row;
|
||||
}
|
||||
|
||||
@@ -501,6 +507,75 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
|
||||
Assert.Equal("unknown value type", updated.ExecutionError);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Approve_WhenConnectionNoLongerMxGateway_FlipsStatusToFailed_NoRelay()
|
||||
{
|
||||
// D2 / T14 TOCTOU guard: the MxGateway protocol is re-asserted AT EXECUTE, not
|
||||
// only at submit. The named connection was reconfigured/recreated as a
|
||||
// non-MxGateway (OPC UA) connection between submit and approval; relaying then
|
||||
// would execute the secured write against a non-MxGateway adapter, violating the
|
||||
// feature's core safety invariant. The row must fail deterministically with the
|
||||
// protocol error and NO relay — even though the CAS already won.
|
||||
SeedPendingWrite(7, operatorUser: "alice");
|
||||
ArmCasSuccess(7);
|
||||
// Re-stub the approve-time connection lookup to a non-MxGateway protocol,
|
||||
// overriding the MxGateway seed inside SeedPendingWrite.
|
||||
SeedSiteWithConnection(1, "SITE1", "Mx1", "OpcUa");
|
||||
var captured = CaptureAuditEvents();
|
||||
|
||||
PendingSecuredWrite? updated = null;
|
||||
_securedWriteRepo
|
||||
.When(r => r.UpdateAsync(Arg.Any<PendingSecuredWrite>(), Arg.Any<CancellationToken>()))
|
||||
.Do(ci => updated = ci.Arg<PendingSecuredWrite>());
|
||||
|
||||
var actor = CreateActor();
|
||||
var envelope = Envelope(new ApproveSecuredWriteCommand(7, "approved"), "bob", "Verifier");
|
||||
|
||||
actor.Tell(envelope);
|
||||
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
// No relay — the connection is no longer an MxGateway target.
|
||||
Assert.Equal(0, _comms.CallCount);
|
||||
Assert.NotNull(updated);
|
||||
Assert.Equal("Failed", updated!.Status);
|
||||
Assert.NotNull(updated.ExecutedAtUtc);
|
||||
Assert.Contains("no longer an MxGateway connection", updated.ExecutionError);
|
||||
|
||||
// The Execute audit row records the failure (canonical Failure outcome).
|
||||
WaitForAuditRows(captured, 2);
|
||||
var execute = SingleOfKind(captured, AuditKind.SecuredWriteExecute);
|
||||
Assert.Equal(ZB.MOM.WW.Audit.AuditOutcome.Failure, execute.Outcome);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Approve_WhenConnectionDeleted_FlipsStatusToFailed_NotFound_NoRelay()
|
||||
{
|
||||
// TOCTOU companion: the connection named on the row was deleted between submit
|
||||
// and approval. The re-load at execute returns no match → fail with "not found"
|
||||
// and no relay.
|
||||
SeedPendingWrite(7, operatorUser: "alice");
|
||||
ArmCasSuccess(7);
|
||||
// Re-stub the approve-time lookup so the site resolves but exposes no connections.
|
||||
_siteRepo.GetDataConnectionsBySiteIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<DataConnection>());
|
||||
|
||||
PendingSecuredWrite? updated = null;
|
||||
_securedWriteRepo
|
||||
.When(r => r.UpdateAsync(Arg.Any<PendingSecuredWrite>(), Arg.Any<CancellationToken>()))
|
||||
.Do(ci => updated = ci.Arg<PendingSecuredWrite>());
|
||||
|
||||
var actor = CreateActor();
|
||||
var envelope = Envelope(new ApproveSecuredWriteCommand(7, "approved"), "bob", "Verifier");
|
||||
|
||||
actor.Tell(envelope);
|
||||
|
||||
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
|
||||
Assert.Equal(0, _comms.CallCount);
|
||||
Assert.NotNull(updated);
|
||||
Assert.Equal("Failed", updated!.Status);
|
||||
Assert.Contains("not found", updated.ExecutionError);
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------------
|
||||
// Audit emission (T14b — SecuredWrite AuditLog rows)
|
||||
// ------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user