From 3ddb6876dbe65660c7b2f5e7a5fb3eba4a870151 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 13:35:54 -0400 Subject: [PATCH] fix(security): first-party JwtBearer with HS256 pinned; /auth/token mints for a cookie session only The hand-rolled bearer handler is replaced by AddJwtBearer (Microsoft.AspNetCore.Authentication.JwtBearer 10.0.7, pinned on the same servicing line as the other AspNetCore packages). Validation parameters come from JwtTokenService.BuildValidationParameters, which now pins ValidAlgorithms = HS256 for both the bearer scheme and TryValidate; MapInboundClaims is off so the minted Role/zb:username keys resolve. /auth/token requires the cookie scheme, so a bearer can no longer mint a fresh token for itself. Refs #733 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- Directory.Packages.props | 4 + docs/security.md | 13 ++- .../Auth/JwtBearerAuthenticationHandler.cs | 99 ------------------- .../Auth/OtOpcUaAuthSchemes.cs | 39 ++++++++ .../Endpoints/AuthEndpoints.cs | 16 ++- .../Jwt/JwtTokenService.cs | 28 ++---- .../ServiceCollectionExtensions.cs | 18 +++- .../ZB.MOM.WW.OtOpcUa.Security.csproj | 1 + .../AuthEndpointsIntegrationTests.cs | 93 +++++++++++++++++ .../JwtConfigurationGateTests.cs | 2 +- 10 files changed, 187 insertions(+), 126 deletions(-) delete mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/JwtBearerAuthenticationHandler.cs create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/OtOpcUaAuthSchemes.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 5b95a7e1..ed2dca67 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -35,6 +35,10 @@ + + diff --git a/docs/security.md b/docs/security.md index 6e44569a..fa8269a2 100644 --- a/docs/security.md +++ b/docs/security.md @@ -6,11 +6,14 @@ > auth + role-grant pages live in `OtOpcUa.AdminUI`), and Admin auth policies > register from `OtOpcUa.Host/Program.cs` via `AddOtOpcUaAuth` > (`src/Server/ZB.MOM.WW.OtOpcUa.Security/ServiceCollectionExtensions.cs`) rather -> than in a separate Admin process. The Admin UI uses a **single Cookie -> authentication scheme** — there is no `AddJwtBearer` pipeline. The -> `Security:Jwt` section configures `JwtTokenService`, which mints a JWT at the -> `/auth/token` endpoint for **external** consumers (OPC UA clients / automation -> scripts); the cookie itself stores the `ClaimsPrincipal` directly. DataProtection +> than in a separate Admin process. The Admin UI session is a **Cookie** +> scheme; the cookie stores the `ClaimsPrincipal` directly. JWT is **opt-in by +> `Security:Jwt:SigningKey`** (validated at host start, ≥32 bytes): with a key, +> `/auth/token` mints an HS256 token for a **cookie session only** (a bearer +> cannot renew itself), and a first-party `AddJwtBearer` scheme (`Bearer`, +> `ValidAlgorithms = HS256`, parameters from `JwtTokenService.BuildValidationParameters`) +> accepts it on `/api/*` and every `RequireAuthorization` endpoint through the +> `OtOpcUa.CookieOrBearer` default policy scheme. Without a key neither exists. DataProtection > keys persist to the shared Config DB (`PersistKeysToDbContext`) > so cookies survive failover between admin-role nodes. > diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/JwtBearerAuthenticationHandler.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/JwtBearerAuthenticationHandler.cs deleted file mode 100644 index cc996758..00000000 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/JwtBearerAuthenticationHandler.cs +++ /dev/null @@ -1,99 +0,0 @@ -using System.Text.Encodings.Web; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; -using Microsoft.Extensions.Primitives; -using ZB.MOM.WW.OtOpcUa.Security.Jwt; - -namespace ZB.MOM.WW.OtOpcUa.Security.Auth; - -/// The authentication scheme names OtOpcUa registers. -public static class OtOpcUaAuthSchemes -{ - /// - /// The bearer scheme that validates the HS256 tokens /auth/token mints, through - /// . Registered only when - /// . - /// - public const string Bearer = "Bearer"; - - /// - /// The default scheme: a policy scheme that forwards to when the request - /// carries an Authorization: Bearer header and to the cookie scheme otherwise, so one - /// RequireAuthorization() / policy accepts a browser session AND a machine token. - /// - public const string CookieOrBearer = "OtOpcUa.CookieOrBearer"; - - /// Whether the request presents a bearer token (the selector's whole rule). - /// The request. - /// when an Authorization: Bearer … header is present. - public static bool HasBearerToken(HttpContext context) => - TryGetBearerToken(context.Request.Headers.Authorization, out _); - - internal static bool TryGetBearerToken(StringValues authorization, out string token) - { - token = string.Empty; - var header = authorization.ToString(); - const string prefix = "Bearer "; - if (!header.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) return false; - token = header[prefix.Length..].Trim(); - return token.Length > 0; - } -} - -/// -/// Validates an Authorization: Bearer token minted by /auth/token (R03:2.4). The JWT -/// feature used to be a dead end: tokens were issued and nothing in the product accepted them. This -/// handler is the consuming side, built on so the validation -/// parameters can never drift from the minting side ( -/// sets RoleClaimType/NameClaimType to the minted keys, so [Authorize(Roles)] and -/// IsInRole resolve). -/// -/// -/// Fail closed: no header ⇒ NoResult (the cookie scheme is not consulted by this handler — the -/// policy scheme selects one or the other); a malformed, expired, tampered or wrong-issuer token ⇒ -/// Fail, never a fall-through to the cookie. A direct AddJwtBearer needs the -/// Microsoft.AspNetCore.Authentication.JwtBearer package, which the central package list does -/// not carry; this handler is ~40 lines over the already-referenced System.IdentityModel.Tokens.Jwt -/// and is a drop-in to replace once that package is pinned. -/// -public sealed class JwtBearerAuthenticationHandler : AuthenticationHandler -{ - private readonly JwtTokenService _jwt; - - /// Initializes a new instance of the class. - /// The scheme options monitor. - /// The logger factory. - /// The URL encoder. - /// The token service whose validation parameters mirror its minting side. - public JwtBearerAuthenticationHandler( - IOptionsMonitor options, - ILoggerFactory logger, - UrlEncoder encoder, - JwtTokenService jwt) - : base(options, logger, encoder) - { - _jwt = jwt; - } - - /// - protected override Task HandleAuthenticateAsync() - { - if (!OtOpcUaAuthSchemes.TryGetBearerToken(Request.Headers.Authorization, out var token)) - return Task.FromResult(AuthenticateResult.NoResult()); - - if (!_jwt.TryValidate(token, out var principal) || principal is null) - return Task.FromResult(AuthenticateResult.Fail("Invalid bearer token.")); - - return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(principal, Scheme.Name))); - } - - /// - protected override Task HandleChallengeAsync(AuthenticationProperties properties) - { - Response.StatusCode = StatusCodes.Status401Unauthorized; - Response.Headers.WWWAuthenticate = OtOpcUaAuthSchemes.Bearer; - return Task.CompletedTask; - } -} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/OtOpcUaAuthSchemes.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/OtOpcUaAuthSchemes.cs new file mode 100644 index 00000000..7c30915e --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Auth/OtOpcUaAuthSchemes.cs @@ -0,0 +1,39 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; +using ZB.MOM.WW.OtOpcUa.Security.Jwt; + +namespace ZB.MOM.WW.OtOpcUa.Security.Auth; + +/// The authentication scheme names OtOpcUa registers. +public static class OtOpcUaAuthSchemes +{ + /// + /// The bearer scheme that validates the HS256 tokens /auth/token mints: the first-party + /// AddJwtBearer handler over . + /// Registered only when . + /// + public const string Bearer = "Bearer"; + + /// + /// The default scheme: a policy scheme that forwards to when the request + /// carries an Authorization: Bearer header and to the cookie scheme otherwise, so one + /// RequireAuthorization() / policy accepts a browser session AND a machine token. + /// + public const string CookieOrBearer = "OtOpcUa.CookieOrBearer"; + + /// Whether the request presents a bearer token (the selector's whole rule). + /// The request. + /// when an Authorization: Bearer … header is present. + public static bool HasBearerToken(HttpContext context) => + TryGetBearerToken(context.Request.Headers.Authorization, out _); + + internal static bool TryGetBearerToken(StringValues authorization, out string token) + { + token = string.Empty; + var header = authorization.ToString(); + const string prefix = "Bearer "; + if (!header.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) return false; + token = header[prefix.Length..].Trim(); + return token.Length > 0; + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs index 6b0876d7..a3f661b6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Endpoints/AuthEndpoints.cs @@ -24,6 +24,17 @@ public static class AuthEndpoints /// Response for a token issue request. public sealed record TokenResponse(string Token); + /// + /// The /auth/token policy: an authenticated principal from the COOKIE scheme. Naming the scheme + /// makes the authorization middleware authenticate that scheme alone, so an Authorization: Bearer + /// header is never consulted here (with login disabled the auto-login handler sits under the same + /// scheme name, so the dev rig is unchanged). + /// + internal static readonly AuthorizationPolicy CookieSessionOnly = + new AuthorizationPolicyBuilder(CookieAuthenticationDefaults.AuthenticationScheme) + .RequireAuthenticatedUser() + .Build(); + /// Maps OtOpcUa authentication endpoints to the application route builder. /// The endpoint route builder. /// The endpoint route builder for chaining. @@ -40,7 +51,10 @@ public static class AuthEndpoints // Mapped only when a signing key is configured (JwtOptions.IsConfigured): an unconfigured // endpoint used to answer HTTP 500 from the JwtTokenService constructor on every call. if (app.ServiceProvider.GetRequiredService>().Value.IsConfigured) - app.MapPost("/auth/token", (Delegate)IssueToken).RequireAuthorization(); + // COOKIE scheme only (fix round 1 / I2): the default scheme also accepts a bearer, and a bearer + // that could mint a fresh token would renew itself forever — outliving its own expiry and the + // user's LDAP membership. A token is minted only for an interactive (cookie) session. + app.MapPost("/auth/token", (Delegate)IssueToken).RequireAuthorization(CookieSessionOnly); else app.ServiceProvider.GetService()? .CreateLogger("ZB.MOM.WW.OtOpcUa.Security.AuthEndpoints") diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Jwt/JwtTokenService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Jwt/JwtTokenService.cs index 4dd38a2e..9dae8c14 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Jwt/JwtTokenService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Jwt/JwtTokenService.cs @@ -25,24 +25,12 @@ public sealed class JwtTokenService /// /// Role claim type used in the JWT payload. /// - /// Issued-only / no internal JwtBearer scheme: OtOpcUa uses a single Cookie - /// authentication scheme; the JWT is minted by the /auth/token endpoint and - /// consumed externally (e.g. by OPC-UA clients or automation scripts). There is no - /// AddJwtBearer pipeline in OtOpcUa — the cookie stores the - /// directly. Because no internal - /// bearer validation path exists, the short "Role" key is intentionally used here rather - /// than the long URI; external consumers receive exactly the - /// key they expect. - /// - /// - /// If a JwtBearer scheme is ever added: the - /// passed to - /// AddJwtBearer MUST set RoleClaimType = JwtTokenService.RoleClaimType (and - /// NameClaimType = JwtTokenService.UsernameClaimType) so that - /// [Authorize(Roles=...)] and ClaimsPrincipal.IsInRole resolve correctly. - /// is already wired to do this and MUST be used - /// rather than constructing - /// ad hoc. + /// The short "Role" key is what mints and what external consumers of the token + /// expect. The internal bearer scheme (AddJwtBearer in AddOtOpcUaAuth) validates with + /// , which sets RoleClaimType = RoleClaimType and + /// NameClaimType = UsernameClaimType, and turns MapInboundClaims off, so + /// [Authorize(Roles=...)] and ClaimsPrincipal.IsInRole resolve against this key. Never + /// construct ad hoc. /// /// public const string RoleClaimType = "Role"; @@ -150,5 +138,9 @@ public sealed class JwtTokenService // if this is ever passed to AddJwtBearer. See RoleClaimType doc comment for rationale. RoleClaimType = RoleClaimType, NameClaimType = UsernameClaimType, + // Pinned to the one algorithm Issue signs with: an unsigned (alg:none) token or one signed with a + // different HMAC variant over the same key is refused on both the bearer and TryValidate paths. + ValidAlgorithms = [SecurityAlgorithms.HmacSha256], + RequireSignedTokens = true, }; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/ServiceCollectionExtensions.cs index d58fbcf3..494145be 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/ServiceCollectionExtensions.cs @@ -1,5 +1,6 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; @@ -108,8 +109,21 @@ public static class ServiceCollectionExtensions }); if (jwtConfigured) { - authBuilder.AddScheme( - OtOpcUaAuthSchemes.Bearer, "OtOpcUa bearer token", _ => { }); + // First-party JwtBearer (R03:2.4, fix round 1 / I1) — the earlier hand-rolled handler is gone. + // Validation parameters come from the SAME JwtTokenService that mints, so issuer/audience/key/ + // claim types can never drift; BuildValidationParameters pins ValidAlgorithms = HS256 (an + // alg:none or a different-HMAC token is refused). MapInboundClaims=false keeps the minted + // "Role"/"zb:username" claim keys verbatim, which RoleClaimType/NameClaimType name. A bad token + // is AuthenticateResult.Fail ⇒ 401 + WWW-Authenticate: Bearer, never a 500. + authBuilder.AddJwtBearer(OtOpcUaAuthSchemes.Bearer, "OtOpcUa bearer token", _ => { }); + services.AddOptions(OtOpcUaAuthSchemes.Bearer) + .Configure((o, jwt) => + { + o.TokenValidationParameters = jwt.BuildValidationParameters(); + o.MapInboundClaims = false; + o.SaveToken = false; + o.IncludeErrorDetails = false; + }); } if (disableLogin) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/ZB.MOM.WW.OtOpcUa.Security.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.Security/ZB.MOM.WW.OtOpcUa.Security.csproj index d81e4eb6..507aca6c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/ZB.MOM.WW.OtOpcUa.Security.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/ZB.MOM.WW.OtOpcUa.Security.csproj @@ -10,6 +10,7 @@ + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs index e5e3b620..599d0cf2 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/AuthEndpointsIntegrationTests.cs @@ -12,6 +12,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; +using Microsoft.IdentityModel.Tokens; using Shouldly; using Xunit; using ZB.MOM.WW.Auth.AspNetCore; @@ -208,6 +209,98 @@ public sealed class AuthEndpointsIntegrationTests : IAsyncLifetime response.Headers.WwwAuthenticate.ToString().ShouldContain("Bearer"); } + /// + /// Fix round 1 / I1 — the first-party JwtBearer handler pins HS256: an unsigned (alg: none) + /// token carrying the right issuer/audience/roles, and a token signed with a different HMAC + /// variant, both fail CLOSED with a 401 — never a 500, never a pass. + /// + [Theory] + [InlineData("none")] + [InlineData("HS384")] + public async Task Bearer_with_a_non_HS256_algorithm_is_rejected_with_401(string alg) + { + var token = alg == "none" ? UnsignedToken() : SignedToken(SecurityAlgorithms.HmacSha384); + var client = NewClient(); + var req = new HttpRequestMessage(HttpMethod.Get, "/api/viewer-only"); + req.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token); + + var response = await client.SendAsync(req, Ct); + + response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + response.Headers.WwwAuthenticate.ToString().ShouldContain("Bearer"); + + // Positive control: the same claims signed HS256 with the configured key ARE accepted, so the + // 401 above measured the algorithm and not a broken token shape. + var good = new HttpRequestMessage(HttpMethod.Get, "/api/viewer-only"); + good.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue( + "Bearer", SignedToken(SecurityAlgorithms.HmacSha256)); + (await client.SendAsync(good, Ct)).StatusCode.ShouldBe(HttpStatusCode.OK); + } + + /// + /// Fix round 1 / I2 — /auth/token mints only for a COOKIE session. A bearer token used to + /// be able to mint a fresh token for itself, so a stolen token could renew itself forever and + /// outlive both its own expiry and the user's LDAP membership. Positive control: the cookie + /// session that minted the bearer still gets a token. + /// + [Fact] + public async Task Auth_token_refuses_a_bearer_only_caller_and_serves_a_cookie_session() + { + var client = NewClient(); + var loginResponse = await client.PostAsJsonAsync("/auth/login", + new AuthEndpoints.LoginRequest("alice", "valid-password"), Ct); + loginResponse.EnsureSuccessStatusCode(); + + var cookieReq = new HttpRequestMessage(HttpMethod.Post, "/auth/token"); + AttachCookies(cookieReq, loginResponse); + var cookieResp = await client.SendAsync(cookieReq, Ct); + cookieResp.StatusCode.ShouldBe(HttpStatusCode.OK); + var bearer = (await cookieResp.Content.ReadFromJsonAsync(Ct)).GetProperty("token").GetString()!; + + // The bearer authenticates elsewhere (positive control on the token itself) … + var root = new HttpRequestMessage(HttpMethod.Get, "/"); + root.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", bearer); + (await client.SendAsync(root, Ct)).StatusCode.ShouldBe(HttpStatusCode.OK); + + // … but cannot renew itself. + var renew = new HttpRequestMessage(HttpMethod.Post, "/auth/token"); + renew.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", bearer); + renew.Headers.Add("X-Requested-With", "XMLHttpRequest"); + var renewResp = await client.SendAsync(renew, Ct); + renewResp.StatusCode.ShouldBeOneOf(HttpStatusCode.Unauthorized, HttpStatusCode.Forbidden); + (await renewResp.Content.ReadAsStringAsync(Ct)).ShouldNotContain("token"); + } + + private const string TestSigningKey = "test-signing-key-with-at-least-32-bytes-of-utf8-content"; + + private static IEnumerable ViewerClaims() => + [ + new(JwtTokenService.UsernameClaimType, "mallory"), + new(JwtTokenService.RoleClaimType, "Viewer"), + ]; + + /// A token signed with using the configured key bytes. + private static string SignedToken(string algorithm) + { + var keyBytes = System.Text.Encoding.UTF8.GetBytes(TestSigningKey); + // The SAME key bytes for every algorithm, so a rejection measures the algorithm pin alone. + var token = new System.IdentityModel.Tokens.Jwt.JwtSecurityToken( + issuer: "otopcua-test", audience: "otopcua-test", claims: ViewerClaims(), + expires: DateTime.UtcNow.AddMinutes(5), + signingCredentials: new SigningCredentials(new SymmetricSecurityKey(keyBytes), algorithm)); + return new System.IdentityModel.Tokens.Jwt.JwtSecurityTokenHandler().WriteToken(token); + } + + /// An alg: none token with otherwise valid issuer/audience/lifetime/roles. + private static string UnsignedToken() + { + static string B64(string json) => Microsoft.IdentityModel.Tokens.Base64UrlEncoder.Encode(json); + var exp = DateTimeOffset.UtcNow.AddMinutes(5).ToUnixTimeSeconds(); + return B64("{\"alg\":\"none\",\"typ\":\"JWT\"}") + "." + + B64($"{{\"zb:username\":\"mallory\",\"Role\":\"Viewer\",\"iss\":\"otopcua-test\",\"aud\":\"otopcua-test\",\"exp\":{exp}}}") + + "."; + } + private async Task LoginAndIssueTokenAsync() { var client = NewClient(); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/JwtConfigurationGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/JwtConfigurationGateTests.cs index 96fc88ca..9d2cc7ef 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/JwtConfigurationGateTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/JwtConfigurationGateTests.cs @@ -65,7 +65,7 @@ public sealed class JwtConfigurationGateTests var schemes = host.Services.GetRequiredService(); var bearer = await schemes.GetSchemeAsync(OtOpcUaAuthSchemes.Bearer); bearer.ShouldNotBeNull(); - bearer.HandlerType.ShouldBe(typeof(JwtBearerAuthenticationHandler)); + bearer.HandlerType.ShouldBe(typeof(Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerHandler)); await host.StopAsync(Ct); }