fix(transport): curate BundleLockedException on the management surface (register row 51)
ManagementService references Commons but not Transport, so MapFault could not see BundleLockedException and the per-bundle 3-strike lockout reached the CLI as "An internal error occurred (CorrelationId=...)" — the operator got a correlation id instead of the reason, and no hint that further attempts on that bundle would be refused. Applies the pattern register row 44 established for BundleUnlockThrottledException: - Relocate BundleLockedException from Transport/Import/ to Commons/Types/Transport/, beside BundleUnlockThrottledException. Both consumers (BundleImporter, CentralUI TransportImport) already carried the Commons.Types.Transport using, so no call-site edits were needed. - Add curated catch arms in ManagementActor.HandlePreviewBundle and HandleImportBundle that rethrow as ManagementCommandException, so the message survives MapFault's disclosure filter. - Mirror the row-44 throttle fault-mapping test for both handlers. - Document the relocation in docs/components/Transport.md and docs/requirements/Component-Transport.md. Tests: ManagementService 499 passed (+2 new), Transport 167 passed.
This commit is contained in:
Joseph Doherty
@@ -122,7 +122,7 @@ Passphrase lockout operates at two levels: per-bundle (3-strike counter in `Bund
|
||||
|
||||
The two levels interact in a specific order, and it is load-bearing (register row 44 — the limiter had been registered, injected and unit-tested but never called, so only the per-bundle lockout was real):
|
||||
|
||||
1. **Per-bundle lockout first.** An already-locked bundle throws `BundleLockedException` without registering a rate-limiter attempt, so hammering a locked bundle cannot spend the client's hourly budget.
|
||||
1. **Per-bundle lockout first.** An already-locked bundle throws `BundleLockedException` (Commons — register row 51 moved it out of `Transport.Import` for the same reason the throttle exception lives there: `ManagementService` does not reference `Transport`, so a lockout used to reach the CLI as `MapFault`'s generic "An internal error occurred" plus a correlation id) without registering a rate-limiter attempt, so hammering a locked bundle cannot spend the client's hourly budget.
|
||||
2. **Per-IP throttle next, before the decrypt.** Over budget throws `BundleUnlockThrottledException` (Commons, so the management surface can catch it and return a curated message). Because it precedes the decrypt, a denial burns **no** per-bundle strike — otherwise the throttle would be a way to lock out a bundle someone is legitimately importing. Every unlock attempt is registered, successful ones included: the documented budget counts attempts, not failures.
|
||||
3. Only then the AES-GCM decrypt, whose failure increments the per-bundle counter as before.
|
||||
|
||||
@@ -255,7 +255,7 @@ A template's scripts reference a `SharedScript` or `ExternalSystem` that exists
|
||||
|
||||
### Passphrase lockout
|
||||
|
||||
After 3 wrong passphrase attempts against the same bundle (keyed by `ContentHash`), `BundleImporter.LoadAsync` throws `BundleLockedException`. The session is unusable. Re-upload the bundle file to get a new session with a fresh counter. A `BundleImportUnlockFailed` audit row is written on each failed attempt.
|
||||
After 3 wrong passphrase attempts against the same bundle (keyed by `ContentHash`), `BundleImporter.LoadAsync` throws `BundleLockedException`. The session is unusable. Re-upload the bundle file to get a new session with a fresh counter. A `BundleImportUnlockFailed` audit row is written on each failed attempt. Both the Central UI and the CLI surface the lockout reason verbatim — the exception lives in `Commons/Types/Transport/` so `ManagementActor` can curate it into a `ManagementCommandException` (register row 51).
|
||||
|
||||
### Unlock throttled ("Too many bundle unlock attempts")
|
||||
|
||||
|
||||
@@ -336,7 +336,7 @@ Imports are all-or-nothing per bundle. A bundle either applies fully or not at a
|
||||
|
||||
- **AES-256-GCM** for content encryption with **PBKDF2-SHA256 / 600 000 iterations** (OWASP 2023+ guidance), per-bundle random salt, random IV per encryption. The GCM auth tag is verified before decryption — a wrong passphrase fails cleanly.
|
||||
- **Passphrase never persisted.** It lives only inside the export/import service call path and is discarded after use. No environment variable, no log line.
|
||||
- **Failed-unlock rate limit:** per-bundle 3-strike lockout (keyed by `ContentHash`, server-side); per-IP-per-hour cap (`MaxUnlockAttemptsPerIpPerHour`, default 10, configurable) to deter brute force against a stolen bundle. Each failed attempt produces a `BundleImportUnlockFailed` audit row. The two are evaluated in order — bundle lockout, then per-IP throttle, then decrypt — so hammering an already-locked bundle spends no hourly budget, and a throttle denial (`BundleUnlockThrottledException`, fail-closed with a curated message on both the UI and CLI surfaces) burns **no** per-bundle strike, since it never reaches the decryptor. Successful unlocks count against the hourly budget too: it caps attempts, not failures. The client key is the caller's remote IP — from `CircuitClientAddressProvider` (snapshotted at Blazor circuit establishment, since `HttpContext` is unavailable mid-circuit) for the Central UI, and from `ManagementEnvelope.ClientIp` for the CLI; a caller with no attributable address shares a single stricter bucket rather than bypassing the throttle. Per-client isolation is only real behind a ForwardedHeaders-trusted proxy, the same caveat the login throttle carries in `Component-Security.md`. (Register row 44: the limiter existed, was configured and unit-tested, but was never invoked until this was wired.)
|
||||
- **Failed-unlock rate limit:** per-bundle 3-strike lockout (keyed by `ContentHash`, server-side); per-IP-per-hour cap (`MaxUnlockAttemptsPerIpPerHour`, default 10, configurable) to deter brute force against a stolen bundle. Each failed attempt produces a `BundleImportUnlockFailed` audit row. The two are evaluated in order — bundle lockout, then per-IP throttle, then decrypt — so hammering an already-locked bundle spends no hourly budget, and a throttle denial (`BundleUnlockThrottledException`, fail-closed with a curated message on both the UI and CLI surfaces) burns **no** per-bundle strike, since it never reaches the decryptor. Both typed exceptions live in `Commons/Types/Transport/` — `ManagementService` references Commons but not Transport, so a type declared beside the importer cannot be curated by `MapFault` and reaches the CLI as a generic internal-error message (register rows 44 and 51). Successful unlocks count against the hourly budget too: it caps attempts, not failures. The client key is the caller's remote IP — from `CircuitClientAddressProvider` (snapshotted at Blazor circuit establishment, since `HttpContext` is unavailable mid-circuit) for the Central UI, and from `ManagementEnvelope.ClientIp` for the CLI; a caller with no attributable address shares a single stricter bucket rather than bypassing the throttle. Per-client isolation is only real behind a ForwardedHeaders-trusted proxy, the same caveat the login throttle carries in `Component-Security.md`. (Register row 44: the limiter existed, was configured and unit-tested, but was never invoked until this was wired.)
|
||||
- **Bundle size cap** on upload (default 100 MB, configurable) to bound memory.
|
||||
- **In-transit:** existing HTTPS to the Central UI; no new channel.
|
||||
- **Audit trail is the chain of custody.** Every export, every import (including aborted ones at validation), and every unlock failure is audit-logged with source env, content hash, encrypted yes/no, and artifact summary.
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Transport;
|
||||
|
||||
/// <summary>
|
||||
/// T-003: thrown by <c>IBundleImporter.LoadAsync</c> when an encrypted bundle has
|
||||
/// exceeded the configured failed-unlock attempt limit
|
||||
/// (<c>TransportOptions.MaxUnlockAttemptsPerSession</c>, default 3). The lockout is tracked
|
||||
/// server-side keyed by <see cref="BundleManifest.ContentHash"/>, so a second tab / CLI caller
|
||||
/// re-uploading the same bytes hits the same counter and cannot side-step the limit.
|
||||
/// <para>
|
||||
/// Distinct from <see cref="BundleUnlockThrottledException"/> on purpose: this one means
|
||||
/// "this BUNDLE has burned its strikes" and is keyed by the bundle's content hash; that one
|
||||
/// means "this CLIENT has attempted too many unlocks, across any bundles" and is keyed by
|
||||
/// the caller's IP.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Register row 51: lives in Commons rather than beside the importer for the same reason
|
||||
/// <see cref="BundleUnlockThrottledException"/> does — it is part of the
|
||||
/// <c>IBundleImporter.LoadAsync</c> contract, which is declared here, and the management
|
||||
/// surface (which references Commons but not Transport) must be able to catch it to return
|
||||
/// a clear, curated error instead of a generic internal-failure message plus a correlation id.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public sealed class BundleLockedException : Exception
|
||||
{
|
||||
/// <summary>Number of recorded unlock failures for this bundle.</summary>
|
||||
public int FailedAttempts { get; }
|
||||
|
||||
/// <summary>SHA-256 (hex) of the bundle's content bytes, the lockout's tracking key.</summary>
|
||||
public string BundleContentHash { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Initializes a new <see cref="BundleLockedException"/>.
|
||||
/// </summary>
|
||||
/// <param name="bundleContentHash">SHA-256 hex from <see cref="BundleManifest.ContentHash"/>.</param>
|
||||
/// <param name="failedAttempts">Number of failures recorded against this bundle.</param>
|
||||
public BundleLockedException(string bundleContentHash, int failedAttempts)
|
||||
: base(
|
||||
$"Bundle is locked after {failedAttempts} failed unlock attempts. "
|
||||
+ "Wait for the lockout window to expire or re-export the bundle to obtain a new content hash.")
|
||||
{
|
||||
BundleContentHash = bundleContentHash ?? throw new ArgumentNullException(nameof(bundleContentHash));
|
||||
FailedAttempts = failedAttempts;
|
||||
}
|
||||
}
|
||||
@@ -3567,6 +3567,17 @@ public class ManagementActor : ReceiveActor
|
||||
{
|
||||
throw new ManagementCommandException(ex.Message);
|
||||
}
|
||||
catch (BundleLockedException ex)
|
||||
{
|
||||
// Register row 51: the per-bundle 3-strike lockout. LoadAsync raises
|
||||
// it from inside its own CryptographicException handler, so without
|
||||
// this arm the operator saw MapFault's generic internal-error message
|
||||
// plus a correlation id, never the reason. Listed ahead of the
|
||||
// CryptographicException arm to keep the read order matching the
|
||||
// importer's throw order (the two types are unrelated, so the
|
||||
// ordering is documentation, not dispatch).
|
||||
throw new ManagementCommandException(ex.Message);
|
||||
}
|
||||
catch (System.Security.Cryptography.CryptographicException)
|
||||
{
|
||||
throw new ManagementCommandException("Wrong passphrase or bundle was tampered.");
|
||||
@@ -3632,6 +3643,17 @@ public class ManagementActor : ReceiveActor
|
||||
{
|
||||
throw new ManagementCommandException(ex.Message);
|
||||
}
|
||||
catch (BundleLockedException ex)
|
||||
{
|
||||
// Register row 51: the per-bundle 3-strike lockout. LoadAsync raises
|
||||
// it from inside its own CryptographicException handler, so without
|
||||
// this arm the operator saw MapFault's generic internal-error message
|
||||
// plus a correlation id, never the reason. Listed ahead of the
|
||||
// CryptographicException arm to keep the read order matching the
|
||||
// importer's throw order (the two types are unrelated, so the
|
||||
// ordering is documentation, not dispatch).
|
||||
throw new ManagementCommandException(ex.Message);
|
||||
}
|
||||
catch (System.Security.Cryptography.CryptographicException)
|
||||
{
|
||||
throw new ManagementCommandException("Wrong passphrase or bundle was tampered.");
|
||||
|
||||
@@ -1,31 +0,0 @@
|
||||
namespace ZB.MOM.WW.ScadaBridge.Transport.Import;
|
||||
|
||||
/// <summary>
|
||||
/// T-003: thrown by <see cref="BundleImporter.LoadAsync"/> when an encrypted bundle has
|
||||
/// exceeded the configured failed-unlock attempt limit
|
||||
/// (<see cref="TransportOptions.MaxUnlockAttemptsPerSession"/>). The lockout is tracked
|
||||
/// server-side keyed by <c>BundleManifest.ContentHash</c>, so a second tab / CLI caller
|
||||
/// re-uploading the same bytes hits the same counter and cannot side-step the limit.
|
||||
/// </summary>
|
||||
public sealed class BundleLockedException : Exception
|
||||
{
|
||||
/// <summary>Number of recorded unlock failures for this bundle.</summary>
|
||||
public int FailedAttempts { get; }
|
||||
|
||||
/// <summary>SHA-256 (hex) of the bundle's content bytes, the lockout's tracking key.</summary>
|
||||
public string BundleContentHash { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Initializes a new <see cref="BundleLockedException"/>.
|
||||
/// </summary>
|
||||
/// <param name="bundleContentHash">SHA-256 hex from <c>BundleManifest.ContentHash</c>.</param>
|
||||
/// <param name="failedAttempts">Number of failures recorded against this bundle.</param>
|
||||
public BundleLockedException(string bundleContentHash, int failedAttempts)
|
||||
: base(
|
||||
$"Bundle is locked after {failedAttempts} failed unlock attempts. "
|
||||
+ "Wait for the lockout window to expire or re-export the bundle to obtain a new content hash.")
|
||||
{
|
||||
BundleContentHash = bundleContentHash ?? throw new ArgumentNullException(nameof(bundleContentHash));
|
||||
FailedAttempts = failedAttempts;
|
||||
}
|
||||
}
|
||||
@@ -3219,6 +3219,51 @@ public class ManagementActorTests : TestKit, IDisposable
|
||||
Assert.Contains("Too many bundle unlock attempts", response.Error);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ImportBundleCommand_LockedBundle_SurfacesTheCuratedReason()
|
||||
{
|
||||
// Register row 51: the per-bundle 3-strike lockout used to reach the CLI as
|
||||
// MapFault's generic "An internal error occurred (CorrelationId=...)" because
|
||||
// BundleLockedException lived in Transport, which ManagementService does not
|
||||
// reference. It now lives in Commons alongside BundleUnlockThrottledException
|
||||
// and is curated the same way.
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns<Commons.Types.Transport.BundleSession>(_ =>
|
||||
throw new Commons.Types.Transport.BundleLockedException("abc123", 3));
|
||||
|
||||
var payload = Convert.ToBase64String(new byte[] { 0x01, 0x02, 0x03 });
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new ImportBundleCommand(payload, "wrong", "skip"), "Administrator"));
|
||||
|
||||
var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
|
||||
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
||||
Assert.Contains("locked after 3 failed unlock attempts", response.Error);
|
||||
Assert.DoesNotContain("An internal error occurred", response.Error);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PreviewBundleCommand_LockedBundle_SurfacesTheCuratedReason()
|
||||
{
|
||||
// Register row 51: preview shares the same LoadAsync fault surface as import,
|
||||
// so it needs the same curated arm.
|
||||
var (_, importer) = AddBundleSubstitutes();
|
||||
|
||||
importer.LoadAsync(Arg.Any<Stream>(), Arg.Any<string?>(), Arg.Any<CancellationToken>(), Arg.Any<string?>())
|
||||
.Returns<Commons.Types.Transport.BundleSession>(_ =>
|
||||
throw new Commons.Types.Transport.BundleLockedException("abc123", 3));
|
||||
|
||||
var payload = Convert.ToBase64String(new byte[] { 0x01, 0x02, 0x03 });
|
||||
var actor = CreateActor();
|
||||
actor.Tell(Envelope(new PreviewBundleCommand(payload, "wrong"), "Administrator"));
|
||||
|
||||
var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
|
||||
Assert.Equal("COMMAND_FAILED", response.ErrorCode);
|
||||
Assert.Contains("locked after 3 failed unlock attempts", response.Error);
|
||||
Assert.DoesNotContain("An internal error occurred", response.Error);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Native alarm source CRUD (Task 21)
|
||||
// ========================================================================
|
||||
|
||||
Reference in New Issue
Block a user