fix(logging): fail-closed bearer redaction; hoist per-request logger creation
This commit is contained in:
+27
-37
@@ -84,42 +84,32 @@ The names match the MXAccess command list in `gateway.md` exactly. `Write` and `
|
|||||||
|
|
||||||
### API key redaction
|
### API key redaction
|
||||||
|
|
||||||
`RedactApiKey` is built around the `mxgw_` API key format issued by the gateway. It preserves the bearer scheme and the key id segment so that operators can correlate a log entry to a specific principal, but always strips the secret tail:
|
`RedactClientIdentity` is the single redaction path for identity-bearing values; `RedactApiKey` is a
|
||||||
|
name-preserving alias for it. Redaction **fails closed**: the only value that survives with any of its
|
||||||
|
content is a gateway-issued `mxgw_<key-id>_<secret>` key, whose key id is kept so operators can
|
||||||
|
correlate a log entry to a specific principal.
|
||||||
|
|
||||||
```csharp
|
| Input | Output | Why |
|
||||||
public static string? RedactApiKey(string? authorizationHeader)
|
|-------|--------|-----|
|
||||||
{
|
| `Bearer mxgw_operator01_super-secret` | `Bearer mxgw_operator01_[redacted]` | Recognized gateway key; key id identifies the principal |
|
||||||
if (string.IsNullOrWhiteSpace(authorizationHeader))
|
| `Bearer eyJhbGciOi…` (any foreign token) | `Bearer [redacted]` | Structure is unknown, so the whole credential goes |
|
||||||
{
|
| `Basic dXNlcjpwYXNz` | `Basic [redacted]` | Same, for any recognized scheme |
|
||||||
return authorizationHeader;
|
| `Bearer mxgw_operator01` (no secret separator) | `Bearer mxgw_[redacted]` | No trustworthy key-id boundary |
|
||||||
}
|
| `Bearer` (scheme only), `anonymous`, `some junk` | `[redacted]` | No scheme/credential split that can be trusted |
|
||||||
|
| `null`, `""`, whitespace | unchanged | Nothing to redact |
|
||||||
|
|
||||||
const string bearerPrefix = "Bearer ";
|
A scheme word survives only when it is one of the recognized authorization schemes (`Bearer`,
|
||||||
if (!authorizationHeader.StartsWith(bearerPrefix, StringComparison.OrdinalIgnoreCase))
|
`Basic`, `Digest`, `Negotiate`, `NTLM`, `ApiKey`, `Token`). An unrecognized leading word is as likely
|
||||||
{
|
to be credential material as it is to be a scheme, so it is dropped along with the rest. The key id is
|
||||||
return RedactedValue;
|
also dropped when it runs longer than 64 characters, which no issued key id does — a long run before
|
||||||
}
|
the first `_` is secret material, not an identifier.
|
||||||
|
|
||||||
string token = authorizationHeader[bearerPrefix.Length..].Trim();
|
The parse is span-based (no regex, no `Split` allocation): the value is split once at the first space,
|
||||||
|
and the key id is read up to the first `_` of the remainder.
|
||||||
|
|
||||||
if (!token.StartsWith("mxgw_", StringComparison.OrdinalIgnoreCase))
|
The consequence for callers is that a non-key identity (for example a Windows account name) reaching
|
||||||
{
|
`RedactClientIdentity` is now replaced rather than passed through. `DashboardRedactor` routes only
|
||||||
return $"{bearerPrefix}{RedactedValue}";
|
values containing the `mxgw_` marker here, so dashboard display names are unaffected.
|
||||||
}
|
|
||||||
|
|
||||||
string[] tokenParts = token.Split('_', 3, StringSplitOptions.RemoveEmptyEntries);
|
|
||||||
if (tokenParts.Length < 2)
|
|
||||||
{
|
|
||||||
return $"{bearerPrefix}mxgw_{RedactedValue}";
|
|
||||||
}
|
|
||||||
|
|
||||||
return $"{bearerPrefix}mxgw_{tokenParts[1]}_{RedactedValue}";
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
The split uses `count: 3` because the secret portion may itself contain underscores; only the first two segments (`mxgw` and the key id) are kept verbatim. Authorization headers that are not bearer tokens are reduced to `[redacted]` rather than passed through, since the gateway cannot reason about their structure.
|
|
||||||
|
|
||||||
`RedactClientIdentity` is the entry point used by `GatewayLogScope` and `DashboardRedactor`. It only invokes `RedactApiKey` when the input contains the `mxgw_` marker, leaving non-key identities (for example, Windows account names) untouched.
|
|
||||||
|
|
||||||
### Command value redaction
|
### Command value redaction
|
||||||
|
|
||||||
@@ -160,12 +150,12 @@ public static IApplicationBuilder UseGatewayRequestLoggingScope(this IApplicatio
|
|||||||
{
|
{
|
||||||
ArgumentNullException.ThrowIfNull(app);
|
ArgumentNullException.ThrowIfNull(app);
|
||||||
|
|
||||||
|
ILogger logger = app.ApplicationServices
|
||||||
|
.GetRequiredService<ILoggerFactory>()
|
||||||
|
.CreateLogger("MxGateway.Request");
|
||||||
|
|
||||||
return app.Use(async (context, next) =>
|
return app.Use(async (context, next) =>
|
||||||
{
|
{
|
||||||
ILogger logger = context.RequestServices
|
|
||||||
.GetRequiredService<ILoggerFactory>()
|
|
||||||
.CreateLogger("ZB.MOM.WW.MxGateway.Request");
|
|
||||||
|
|
||||||
using IDisposable? scope = logger.BeginGatewayScope(new GatewayLogScope(
|
using IDisposable? scope = logger.BeginGatewayScope(new GatewayLogScope(
|
||||||
SessionId: ReadHeader(context, SessionIdHeaderName),
|
SessionId: ReadHeader(context, SessionIdHeaderName),
|
||||||
WorkerProcessId: ReadInt32Header(context, WorkerProcessIdHeaderName),
|
WorkerProcessId: ReadInt32Header(context, WorkerProcessIdHeaderName),
|
||||||
@@ -190,7 +180,7 @@ The scope is keyed off four custom headers and the standard `authorization` head
|
|||||||
|
|
||||||
The numeric headers use `int.TryParse` and `ulong.TryParse`; missing or unparseable values become `null` and are dropped by `GatewayLogScope.ToDictionary`. This keeps the middleware tolerant of clients that do not yet emit every header, which matters because the earliest call in a session (`OpenSession`) has no `SessionId` to send.
|
The numeric headers use `int.TryParse` and `ulong.TryParse`; missing or unparseable values become `null` and are dropped by `GatewayLogScope.ToDictionary`. This keeps the middleware tolerant of clients that do not yet emit every header, which matters because the earliest call in a session (`OpenSession`) has no `SessionId` to send.
|
||||||
|
|
||||||
The logger category is `ZB.MOM.WW.MxGateway.Request`, which lets operators filter the request scope events independently from per-component categories.
|
The logger category is `MxGateway.Request`, which lets operators filter the request scope events independently from per-component categories. The logger is resolved once at registration rather than per request: the category is fixed, so a per-request `IServiceProvider` resolve and `ILoggerFactory.CreateLogger` (which takes the factory lock) bought nothing. Scope construction itself stays unconditional — gating it on `ILogger.IsEnabled` would drop scope state for providers and scope consumers registered after startup.
|
||||||
|
|
||||||
### Pipeline ordering
|
### Pipeline ordering
|
||||||
|
|
||||||
|
|||||||
@@ -15,6 +15,28 @@ public static class GatewayLogRedactor
|
|||||||
"WriteSecured2"
|
"WriteSecured2"
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Authorization schemes whose name may survive redaction. Anything outside this list is
|
||||||
|
/// dropped whole: an unrecognized leading word is as likely to be credential material as it
|
||||||
|
/// is to be a scheme, so it is not worth the leak.
|
||||||
|
/// </summary>
|
||||||
|
private static readonly string[] KnownAuthorizationSchemes =
|
||||||
|
[
|
||||||
|
"Bearer",
|
||||||
|
"Basic",
|
||||||
|
"Digest",
|
||||||
|
"Negotiate",
|
||||||
|
"NTLM",
|
||||||
|
"ApiKey",
|
||||||
|
"Token",
|
||||||
|
];
|
||||||
|
|
||||||
|
/// <summary>Prefix identifying a gateway-issued API key.</summary>
|
||||||
|
private const string GatewayKeyPrefix = "mxgw_";
|
||||||
|
|
||||||
|
/// <summary>Upper bound on a key id kept in the clear; a longer run is treated as secret material.</summary>
|
||||||
|
private const int MaxKeyIdLength = 64;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Determines whether a command method bears credentials.
|
/// Determines whether a command method bears credentials.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
@@ -27,44 +49,24 @@ public static class GatewayLogRedactor
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Redacts the API key secret portion of a Bearer authorization header.
|
/// Redacts the credential portion of an authorization header value.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <param name="authorizationHeader">The authorization header value to redact.</param>
|
/// <param name="authorizationHeader">The authorization header value to redact.</param>
|
||||||
/// <returns>The header with the secret portion redacted, or the original value when it is null, blank, or not a Bearer header.</returns>
|
/// <returns>The header with the credential redacted, or the original value when it is null or blank.</returns>
|
||||||
public static string? RedactApiKey(string? authorizationHeader)
|
public static string? RedactApiKey(string? authorizationHeader)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(authorizationHeader))
|
return RedactClientIdentity(authorizationHeader);
|
||||||
{
|
|
||||||
return authorizationHeader;
|
|
||||||
}
|
|
||||||
|
|
||||||
const string bearerPrefix = "Bearer ";
|
|
||||||
if (!authorizationHeader.StartsWith(bearerPrefix, StringComparison.OrdinalIgnoreCase))
|
|
||||||
{
|
|
||||||
return RedactedValue;
|
|
||||||
}
|
|
||||||
|
|
||||||
string token = authorizationHeader[bearerPrefix.Length..].Trim();
|
|
||||||
|
|
||||||
if (!token.StartsWith("mxgw_", StringComparison.OrdinalIgnoreCase))
|
|
||||||
{
|
|
||||||
return $"{bearerPrefix}{RedactedValue}";
|
|
||||||
}
|
|
||||||
|
|
||||||
string[] tokenParts = token.Split('_', 3, StringSplitOptions.RemoveEmptyEntries);
|
|
||||||
if (tokenParts.Length < 2)
|
|
||||||
{
|
|
||||||
return $"{bearerPrefix}mxgw_{RedactedValue}";
|
|
||||||
}
|
|
||||||
|
|
||||||
return $"{bearerPrefix}mxgw_{tokenParts[1]}_{RedactedValue}";
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Redacts the client identity if it contains an API key.
|
/// Redacts the credential carried by a client identity. Redaction fails closed: only a
|
||||||
|
/// gateway-issued API key keeps its <c>mxgw_<key-id>_</c> shape (so operators can tell keys
|
||||||
|
/// apart in logs), and only a recognized scheme keeps its name. Every other value — a foreign
|
||||||
|
/// bearer token, a scheme-less string, junk — is replaced whole, because nothing that reaches
|
||||||
|
/// this method is known to be safe to log.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <param name="clientIdentity">The client identity string to redact.</param>
|
/// <param name="clientIdentity">The client identity string to redact.</param>
|
||||||
/// <returns>The redacted client identity, or the original value when it contains no API key.</returns>
|
/// <returns>The redacted client identity, or the original value when it is null or blank.</returns>
|
||||||
public static string? RedactClientIdentity(string? clientIdentity)
|
public static string? RedactClientIdentity(string? clientIdentity)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(clientIdentity))
|
if (string.IsNullOrWhiteSpace(clientIdentity))
|
||||||
@@ -72,9 +74,61 @@ public static class GatewayLogRedactor
|
|||||||
return clientIdentity;
|
return clientIdentity;
|
||||||
}
|
}
|
||||||
|
|
||||||
return clientIdentity.Contains("mxgw_", StringComparison.OrdinalIgnoreCase)
|
ReadOnlySpan<char> value = clientIdentity.AsSpan().Trim();
|
||||||
? RedactApiKey(clientIdentity)
|
int separatorIndex = value.IndexOf(' ');
|
||||||
: clientIdentity;
|
|
||||||
|
if (separatorIndex < 0)
|
||||||
|
{
|
||||||
|
// A single token carries no scheme, so the token itself is the credential.
|
||||||
|
return RedactedValue;
|
||||||
|
}
|
||||||
|
|
||||||
|
ReadOnlySpan<char> scheme = value[..separatorIndex];
|
||||||
|
ReadOnlySpan<char> credential = value[(separatorIndex + 1)..].Trim();
|
||||||
|
|
||||||
|
if (credential.IsEmpty || !IsKnownAuthorizationScheme(scheme))
|
||||||
|
{
|
||||||
|
return RedactedValue;
|
||||||
|
}
|
||||||
|
|
||||||
|
return credential.StartsWith(GatewayKeyPrefix, StringComparison.OrdinalIgnoreCase)
|
||||||
|
? $"{scheme} {GatewayKeyPrefix}{RedactKeyId(credential)}"
|
||||||
|
: $"{scheme} {RedactedValue}";
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Renders the trailing portion of a gateway API key: the key id when the key is well formed,
|
||||||
|
/// otherwise nothing but the placeholder.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="credential">The credential, known to start with the gateway key prefix.</param>
|
||||||
|
/// <returns>The <c><key-id>_[redacted]</c> tail, or just the placeholder.</returns>
|
||||||
|
private static string RedactKeyId(ReadOnlySpan<char> credential)
|
||||||
|
{
|
||||||
|
ReadOnlySpan<char> remainder = credential[GatewayKeyPrefix.Length..];
|
||||||
|
int secretIndex = remainder.IndexOf('_');
|
||||||
|
|
||||||
|
// No separator means no secret boundary to trust, so the whole remainder is treated as secret.
|
||||||
|
return secretIndex is <= 0 or > MaxKeyIdLength
|
||||||
|
? RedactedValue
|
||||||
|
: $"{remainder[..secretIndex]}_{RedactedValue}";
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Determines whether a leading word is a recognized authorization scheme.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="scheme">The candidate scheme word.</param>
|
||||||
|
/// <returns><see langword="true"/> when the word may survive redaction; otherwise <see langword="false"/>.</returns>
|
||||||
|
private static bool IsKnownAuthorizationScheme(ReadOnlySpan<char> scheme)
|
||||||
|
{
|
||||||
|
foreach (string knownScheme in KnownAuthorizationSchemes)
|
||||||
|
{
|
||||||
|
if (scheme.Equals(knownScheme, StringComparison.OrdinalIgnoreCase))
|
||||||
|
{
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
+8
-4
@@ -24,12 +24,16 @@ public static class GatewayRequestLoggingMiddlewareExtensions
|
|||||||
{
|
{
|
||||||
ArgumentNullException.ThrowIfNull(app);
|
ArgumentNullException.ThrowIfNull(app);
|
||||||
|
|
||||||
|
// Resolved once at registration: the logger is keyed by category, not by request, so the
|
||||||
|
// per-request DI resolve and logger-factory lock bought nothing.
|
||||||
|
ILogger logger = app.ApplicationServices
|
||||||
|
.GetRequiredService<ILoggerFactory>()
|
||||||
|
.CreateLogger("MxGateway.Request");
|
||||||
|
|
||||||
return app.Use(async (context, next) =>
|
return app.Use(async (context, next) =>
|
||||||
{
|
{
|
||||||
ILogger logger = context.RequestServices
|
// Scope construction is deliberately unconditional: gating it on IsEnabled would drop
|
||||||
.GetRequiredService<ILoggerFactory>()
|
// scope state for providers (and scope consumers) registered after startup.
|
||||||
.CreateLogger("MxGateway.Request");
|
|
||||||
|
|
||||||
using IDisposable? scope = logger.BeginGatewayScope(new GatewayLogScope(
|
using IDisposable? scope = logger.BeginGatewayScope(new GatewayLogScope(
|
||||||
SessionId: ReadHeader(context, SessionIdHeaderName),
|
SessionId: ReadHeader(context, SessionIdHeaderName),
|
||||||
WorkerProcessId: ReadInt32Header(context, WorkerProcessIdHeaderName),
|
WorkerProcessId: ReadInt32Header(context, WorkerProcessIdHeaderName),
|
||||||
|
|||||||
@@ -24,6 +24,67 @@ public sealed class GatewayLogRedactorTests
|
|||||||
Assert.DoesNotContain("super_secret_value", redacted);
|
Assert.DoesNotContain("super_secret_value", redacted);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that a bearer credential the gateway does not issue is redacted too. A client that
|
||||||
|
/// pastes a JWT (or any other token) into the authorization header must not have it logged.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void RedactClientIdentity_RedactsNonGatewayBearerCredential()
|
||||||
|
{
|
||||||
|
const string token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIn0.c2lnbmF0dXJl";
|
||||||
|
|
||||||
|
string? redacted = GatewayLogRedactor.RedactClientIdentity($"Bearer {token}");
|
||||||
|
|
||||||
|
Assert.Equal("Bearer [redacted]", redacted);
|
||||||
|
Assert.DoesNotContain(token, redacted, StringComparison.Ordinal);
|
||||||
|
Assert.DoesNotContain("eyJ", redacted, StringComparison.Ordinal);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Verifies that a gateway API key keeps its key-id shape so an operator can still tell keys apart.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void RedactClientIdentity_PreservesGatewayKeyIdShape()
|
||||||
|
{
|
||||||
|
string? redacted = GatewayLogRedactor.RedactClientIdentity("Bearer mxgw_operator01_super-secret");
|
||||||
|
|
||||||
|
Assert.Equal("Bearer mxgw_operator01_[redacted]", redacted);
|
||||||
|
Assert.DoesNotContain("super-secret", redacted, StringComparison.Ordinal);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that anything not recognized as a gateway API key fails closed: the scheme word
|
||||||
|
/// survives only when it looks like an auth scheme, and the credential never does.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="clientIdentity">The raw client identity value.</param>
|
||||||
|
/// <param name="expected">The expected redacted value.</param>
|
||||||
|
[Theory]
|
||||||
|
[InlineData("Bearer", "[redacted]")]
|
||||||
|
[InlineData("Bearer ", "[redacted]")]
|
||||||
|
[InlineData("Basic dXNlcjpwYXNzd29yZA==", "Basic [redacted]")]
|
||||||
|
[InlineData("Negotiate YIIJvwYGKwYBBQUCoIIJ", "Negotiate [redacted]")]
|
||||||
|
[InlineData("mxgw_operator01_super-secret", "[redacted]")]
|
||||||
|
[InlineData("mxgw_operator01_super-secret trailing", "[redacted]")]
|
||||||
|
[InlineData("Bearer mxgw_operator01", "Bearer mxgw_[redacted]")]
|
||||||
|
[InlineData("Bearer mxgw_", "Bearer mxgw_[redacted]")]
|
||||||
|
[InlineData("Bearer mxgw__super-secret", "Bearer mxgw_[redacted]")]
|
||||||
|
[InlineData("bearer mxgw_operator01_super-secret", "bearer mxgw_operator01_[redacted]")]
|
||||||
|
[InlineData("anonymous", "[redacted]")]
|
||||||
|
[InlineData("some random junk", "[redacted]")]
|
||||||
|
public void RedactClientIdentity_FailsClosedForUnrecognizedCredentials(string clientIdentity, string expected)
|
||||||
|
{
|
||||||
|
Assert.Equal(expected, GatewayLogRedactor.RedactClientIdentity(clientIdentity));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Verifies that a blank client identity is passed through — there is nothing to redact.</summary>
|
||||||
|
/// <param name="clientIdentity">The raw client identity value.</param>
|
||||||
|
[Theory]
|
||||||
|
[InlineData(null)]
|
||||||
|
[InlineData("")]
|
||||||
|
[InlineData(" ")]
|
||||||
|
public void RedactClientIdentity_PassesThroughBlankValues(string? clientIdentity)
|
||||||
|
{
|
||||||
|
Assert.Equal(clientIdentity, GatewayLogRedactor.RedactClientIdentity(clientIdentity));
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that IsCredentialBearingCommand identifies credential-bearing MXAccess commands.</summary>
|
/// <summary>Verifies that IsCredentialBearingCommand identifies credential-bearing MXAccess commands.</summary>
|
||||||
/// <param name="commandMethod">Name of the MXAccess command method.</param>
|
/// <param name="commandMethod">Name of the MXAccess command method.</param>
|
||||||
[Theory]
|
[Theory]
|
||||||
|
|||||||
Reference in New Issue
Block a user