diff --git a/docs/ldap-windows-sso.md b/docs/ldap-windows-sso.md index aee03df..168542c 100644 --- a/docs/ldap-windows-sso.md +++ b/docs/ldap-windows-sso.md @@ -28,6 +28,7 @@ All login paths issue the same server-side, revocable NodePilot session. Directo - Windows SSO is Kerberos-only. `AllowNtlmFallback` must remain `false`, and startup requires `NtlmDisabledByPolicy=true` as an operator attestation that host/domain policy rejects incoming NTLM. - Memberships are authority-scoped server-side snapshots. AD sync runs every one to five minutes, and external authorization is rejected once the last authoritative snapshot is more than 15 minutes old. - Deactivation, tombstoning or access-group removal revokes sessions and stops pending, running or paused executions belonging to the effective user, including schedules, webhooks and external triggers. +- Password length is bounded per authentication path, not globally. Local accounts end at 72 UTF-8 bytes — BCrypt truncates past that, so a longer secret is silently weaker than it looks and is refused at both the password-setting and the login end. Directory passwords are governed by the directory: NodePilot never truncates them and rejects a login payload only past 256 bytes, Active Directory's own maximum. A long AD passphrase therefore always reaches the bind. - Local login defaults to `BreakGlassOnly`; only bootstrap and explicitly marked break-glass local accounts can use a password in that mode. Every successful emergency login emits the dedicated `BREAK_GLASS_LOGIN_SUCCESS` audit action for SIEM alerting. - Authentication scheme changes are process-start decisions. Restart the NodePilot service after saving this section. diff --git a/src/NodePilot.Api/Controllers/AuthController.cs b/src/NodePilot.Api/Controllers/AuthController.cs index bce748b..7a97717 100644 --- a/src/NodePilot.Api/Controllers/AuthController.cs +++ b/src/NodePilot.Api/Controllers/AuthController.cs @@ -43,6 +43,12 @@ public class AuthController : ControllerBase // they have a stronger secret than they actually do (L1). internal const int MaxPasswordBytes = 72; + // The same bound does NOT apply to a directory account: an LDAP/AD password lives in the + // directory, never reaches BCrypt, and Active Directory permits up to 256 characters. The + // login entrance therefore bounds the payload at the directory maximum and leaves the + // BCrypt-specific 72-byte reject to the local branch, where it is actually true. + internal const int MaxDirectoryPasswordBytes = 256; + // M-32: /api/auth/login is AllowAnonymous, so its body is bound in full before any // credential is checked. Without an endpoint limit it inherits Kestrel's 30 MiB default — // the rate limiter caps requests per minute, never bytes per request. A login payload is a @@ -361,13 +367,13 @@ public async Task> Login(LoginRequest request, Cance // using a multi-megabyte JSON string as a CPU/allocation amplifier in the throttle. var invalidUsername = string.IsNullOrWhiteSpace(request.Username) || request.Username.Length > ExternalLoginThrottle.MaximumUsernameLength; - // M-32: every password-setting path already runs ValidatePasswordPolicy, which caps at - // MaxPasswordBytes, so no stored hash can correspond to a longer secret — a longer login - // password is unauthenticatable by construction and only exists to make the server work. - // Checking it here keeps the bound explicit at the entry point instead of relying on - // BCrypt's internal 72-byte truncation to absorb it. + // M-32: bound the password payload before any credential work, but at the DIRECTORY + // maximum — this gate sits ahead of the LDAP bind, and BCrypt's 72-byte truncation is a + // property of local accounts only. Capping at 72 here would 401 every AD user whose + // passphrase is longer than ~72 ASCII characters without ever attempting the bind. The + // BCrypt bound is enforced on the local branch instead, where it is actually true. var invalidPassword = - System.Text.Encoding.UTF8.GetByteCount(request.Password ?? string.Empty) > MaxPasswordBytes; + System.Text.Encoding.UTF8.GetByteCount(request.Password ?? string.Empty) > MaxDirectoryPasswordBytes; if (invalidUsername || invalidPassword) { _ = BCrypt.Net.BCrypt.Verify(request.Password, DummyHash); @@ -534,6 +540,21 @@ await _audit.LogAsync(AuditActions.LoginFailed, "User", user.Id, // Atomically reserve the attempt before BCrypt. The conditional UPDATE takes the // database row lock and closes the old check-then-save race where parallel requests // all observed the same failure count and overwrote each other's increments. + // M-32 (local half): every local password-setting path runs ValidatePasswordPolicy, which + // caps at MaxPasswordBytes, so no stored hash can correspond to a longer secret — a longer + // login password is unauthenticatable by construction. Rejecting it here, ahead of the + // throttle reservation, keeps that bound explicit instead of relying on BCrypt's silent + // truncation to absorb it, and costs the caller no lockout budget. LDAP users never reach + // this line: TryLdapLoginAsync short-circuits above. + if (System.Text.Encoding.UTF8.GetByteCount(request.Password ?? string.Empty) > MaxPasswordBytes) + { + _ = BCrypt.Net.BCrypt.Verify(request.Password, DummyHash); + await _audit.LogAsync(AuditActions.LoginFailed, "User", user.Id, + AuditDetails.Json(("username", user.Username), ("reason", "invalid_password_length")), ct); + RecordLoginAttempt("failure", "invalid_password_length"); + return Unauthorized(new { message = "Invalid credentials" }); + } + var localAttempt = await TryReserveUserAttemptAsync(user, DateTime.UtcNow, ct); if (!localAttempt.IsAllowed) { diff --git a/src/nodepilot-docs-ui/content/security/overview.md b/src/nodepilot-docs-ui/content/security/overview.md index 6763bac..1772c50 100644 --- a/src/nodepilot-docs-ui/content/security/overview.md +++ b/src/nodepilot-docs-ui/content/security/overview.md @@ -6,6 +6,7 @@ NodePilot aktiviert Sicherheitsprüfungen standardmäßig. `appsettings.Developm - **Serverseitige Sessions:** standardmäßig acht Stunden absolute Lebensdauer, einzeln widerrufbar. JWTs tragen Session-ID, Security-Stamp und `jti`; Gruppen bleiben aus JWT und Cookie heraus. - **Lokale BCrypt-Passwörter:** Produktionsdefault `BreakGlassOnly`; nur explizit markierte Notfallkonten dürfen sich lokal anmelden. +- **Passwortlänge pro Pfad:** Lokale Konten enden bei 72 UTF-8 Byte (BCrypt schneidet darüber hinaus stillschweigend ab). Directory-Passwörter regelt das Directory: NodePilot kürzt sie nie und weist einen Login-Payload erst jenseits von 256 Byte ab — dem AD-Maximum. Eine lange AD-Passphrase erreicht damit immer den Bind. - **Externe Pfade:** LDAP ausschließlich über validiertes LDAPS, Windows Negotiate ausschließlich Kerberos sowie release-gated OIDC Authorization Code + PKCE. Windows lädt bei jedem Login einen autoritativen LDAPS-Snapshot und vertraut keinen PAC-Gruppen. - **Kanonische Identität:** `(Authority, Subject)`; LDAP und Windows teilen sich den AD-`objectSid`, OIDC verwendet `(iss, sub)`. Gleichnamige bestehende Benutzer werden nicht automatisch zusammengeführt. - **Serverseitige Autorisierung:** Gruppen-Memberships kommen aus Directory-Snapshots. AD-Sync läuft standardmäßig alle fünf Minuten mit 16 parallelen LDAPS-Lookups (konfigurierbar 1–32); Snapshots über 15 Minuten werden für Sessions, Jobs und Trigger abgewiesen. diff --git a/tests/NodePilot.Api.Tests/Controllers/AuthControllerLdapTests.cs b/tests/NodePilot.Api.Tests/Controllers/AuthControllerLdapTests.cs index 7fd055f..86d55c3 100644 --- a/tests/NodePilot.Api.Tests/Controllers/AuthControllerLdapTests.cs +++ b/tests/NodePilot.Api.Tests/Controllers/AuthControllerLdapTests.cs @@ -330,14 +330,13 @@ public async Task OverlongUsername_IsRejectedBeforeLdapOrThrottleWork() [Fact] public async Task OverlongPassword_IsRejectedBeforeLdapOrThrottleWork() { - // M-32: every password-setting path runs ValidatePasswordPolicy, which caps at - // MaxPasswordBytes, so no stored hash can correspond to a longer secret — an over-long - // login password is unauthenticatable by construction. Reject it at the entry point - // instead of spending a directory round-trip and a throttle slot on it. + // M-32: the entry point still bounds the payload, but at the DIRECTORY maximum — no AD + // password can be longer, so anything past it is unauthenticatable by construction and + // must not cost a directory round-trip or a throttle slot. var (controller, adapter) = NewController(); var result = await controller.Login( - new LoginRequest("alice", new string('a', AuthController.MaxPasswordBytes + 1)), + new LoginRequest("alice", new string('a', AuthController.MaxDirectoryPasswordBytes + 1)), CancellationToken.None); result.Result.Should().BeOfType(); @@ -345,6 +344,50 @@ public async Task OverlongPassword_IsRejectedBeforeLdapOrThrottleWork() (await _db.IdempotencyKeys.CountAsync()).Should().Be(0); } + [Fact] + public async Task LongPassphrase_PastBcryptsLimit_StillReachesTheLdapBind() + { + // BCrypt's 72-byte truncation is a property of LOCAL accounts. An AD passphrase lives in + // the directory and never touches BCrypt, so a ~12-word passphrase — exactly what the + // security-conscious orgs that deploy LDAP SSO hand out — must reach the bind intact. + var passphrase = new string('a', AuthController.MaxPasswordBytes + 28); + var (controller, adapter) = NewController(); + controller.Request.Headers[AuthController.TokenResponseHeader] = "true"; + adapter.Result = new LdapAuthResult( + ExternalId: "guid-aaa", + Upn: "alice@firma.de", + DisplayName: "Alice Example", + GroupSids: new[] { "S-1-5-21-1-1-1-512" }); + + var result = await controller.Login(new LoginRequest("alice", passphrase), CancellationToken.None); + + result.Result.Should().BeOfType(); + adapter.Calls.Should().Be(1); + adapter.LastPassword.Should().Be(passphrase, "the directory verifies the full secret, unshortened"); + } + + [Fact] + public async Task OverlongPassword_AgainstLocalAccount_IsStillRejectedWithoutSpendingLockoutBudget() + { + // The local half of the same bound: no local hash can correspond to more than 72 bytes, + // so the reject stays — but on this branch only, and still ahead of the throttle. + var audit = new CapturingAuditWriter(); + var (controller, adapter) = NewController( + options: new LdapOptions { Enabled = false }, audit: audit); + + var result = await controller.Login( + new LoginRequest("preexisting-admin", new string('a', AuthController.MaxPasswordBytes + 1)), + CancellationToken.None); + + result.Result.Should().BeOfType(); + adapter.Calls.Should().Be(0); + audit.Calls.Should().ContainSingle(c => + c.Action == AuditActions.LoginFailed && c.Details!.Contains("invalid_password_length")); + + var admin = await _db.Users.SingleAsync(u => u.Username == "preexisting-admin"); + admin.FailedLoginCount.Should().Be(0, "an unauthenticatable length costs no lockout budget"); + } + [Fact] public async Task ParallelPreJitLogins_AcrossDbContexts_OnlyFiveReachDirectory() { diff --git a/tests/NodePilot.Api.Tests/TestSupport/FakeLdapConnectionAdapter.cs b/tests/NodePilot.Api.Tests/TestSupport/FakeLdapConnectionAdapter.cs index 1edf989..d5bd28e 100644 --- a/tests/NodePilot.Api.Tests/TestSupport/FakeLdapConnectionAdapter.cs +++ b/tests/NodePilot.Api.Tests/TestSupport/FakeLdapConnectionAdapter.cs @@ -34,6 +34,12 @@ public sealed class FakeLdapConnectionAdapter : ILdapConnectionAdapter /// The UPN of the most recent authenticate attempt. public string? LastUpn { get; private set; } + /// + /// The password of the most recent authenticate attempt — the bind sees whatever the caller + /// typed, so this is what proves a long AD passphrase arrives at the directory unshortened. + /// + public string? LastPassword { get; private set; } + // --- LookupBySubjectAsync (background directory synchronization) ----------------- /// Snapshot served for any subject without a entry. @@ -52,6 +58,7 @@ public sealed class FakeLdapConnectionAdapter : ILdapConnectionAdapter { Calls++; LastUpn = upn; + LastPassword = password; if (ExceptionToThrow is not null) throw ExceptionToThrow; if (ThrowCancellation) throw new OperationCanceledException(ct); if (ThrowUserObjectMissing)