From 141bdeb89c161957c3cef10eaab941cb4fae0541 Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Sat, 24 Oct 2026 11:28:17 -0700 Subject: [PATCH] case 4710 - 2FA step requires the temp token from the password step The 2FA step matched the user by temp token alone, and the token was left empty after a successful 2FA login (null right after turning 2FA on), so an empty or missing token skipped the password step (security review F5). Now: an empty or missing temp token is refused before the lookup; the token is cleared to null after success; after 5 wrong codes the token is cleared, so the password must be entered again (counted in memory per user). Changelog: general security notice for self-hosted upgrades, and the one behaviour change users can see. SECURITY-REVIEW.md: F5 fixed. Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/NET10-PORT.md | 2 +- devdocs/modernization/SECURITY-REVIEW.md | 2 +- docs/8.0/ayanova/docs/changelog.md | 3 +++ server/AyaNova/Controllers/AuthController.cs | 27 +++++++++++++++++--- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index 00eb1f85..7269ae6e 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -177,7 +177,7 @@ John's "next release" cases for AyaNova (exported 2026-10-24). Each is its own s | Case | Name | Findings | |---|---|---| -| 4710 | 2FA step requires the temp token (raven and sockeye) | F5, F41 | +| 4710 | 2FA step requires the temp token (raven and sockeye) | F5, F41. **raven done** (proof script 401 on both bypasses; new integration test `TwoFactor`); sockeye next | | 4711 | Placeholder/short JWT secret treated as unset (4698 folded in: random secret generated at install, server ignores a leftover placeholder) | F1 | | 4712 | Customer contact route allowlist + ownership checks | F13, F14, F15, F16, F17 | | 4713 | Contact users limited to customer roles | F28 | diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index f91a2a9a..deb6df26 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -191,7 +191,7 @@ take the next free number; a fixed or accepted finding keeps its ID and changes |---|---|---|---| | F41 | **Critical** | confirmed (code) | F5 also in Sockeye (internet-facing admin login) and in the 8.2.x business AyaNova server | | F42 | High | confirmed (code) | The licence-signing RSA private keys are hardcoded in Sockeye's source and binaries | -| F5 | **Critical** | **confirmed** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on | +| F5 | **Critical** | **fixed in case 4710** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on | | F1 | Critical (installs that kept it) | **confirmed** | Placeholder JWT secret in the LAN/IIS and Linux server config templates | | F19 | Critical | **confirmed** (import) | Any logged-in user, customer contacts included, can import report templates, whose JavaScript runs in the server's browser | | F21 | Critical | candidate | Download file names from the URL are not checked: `report/download` and `export/download` may serve files outside the temp folder on Windows | diff --git a/docs/8.0/ayanova/docs/changelog.md b/docs/8.0/ayanova/docs/changelog.md index 50439047..d1bbf424 100644 --- a/docs/8.0/ayanova/docs/changelog.md +++ b/docs/8.0/ayanova/docs/changelog.md @@ -14,8 +14,11 @@ IN PROGRESS **WARNING:** this release contains changes that will require a **pre upgrade step** on all Windows and Linux network platforms from the normal upgrade process you may be accustomed to. Be sure to follow our [upgrade instructions](ops-upgrade.md) and the **pre upgrade step** for your platform for this particular upgrade from version 8.2.4 (or older) to version 8.3.0 (or newer). +**IMPORTANT:** this release includes important security improvements along with the move to .NET 10. We recommend upgrading self-hosted installations to 8.3.0 promptly. + **Changed** +- Server: after several incorrect two-factor authentication codes, the password must be entered again before another code can be tried. - Server: updated to use .NET 10.x LTS from previously used .NET 8.x LTS. .NET 8.x reaches end of support in November 2026. With this update to .NET 10.x come performance and security gains. - Server: all 3rd party components used by the AyaNova server have been upgraded to their most recent release versions to use with .NET 10.x. This improves performance, stability and security for each component used. - Server: the report rendering system now uses the smaller and faster "Chrome headless shell" browser instead of the full Chrome browser. It is downloaded into a `report-browser` folder under the AyaNova data folder instead of the program folder, so it is no longer removed by an upgrade. There will be a longer than normal delay the _first time_ a report is rendered after upgrading while AyaNova automatically downloads the new report rendering browser. Any old `Chrome` and `ChromeHeadlessShell` folders in the AyaNova program folder are no longer used and can be removed. diff --git a/server/AyaNova/Controllers/AuthController.cs b/server/AyaNova/Controllers/AuthController.cs index 77262b6e..3bb70d44 100644 --- a/server/AyaNova/Controllers/AuthController.cs +++ b/server/AyaNova/Controllers/AuthController.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Authorization; @@ -34,6 +35,10 @@ namespace AyaNova.Api.Controllers private readonly ApiServerState serverState; private const int JWT_LIFETIME_DAYS = 5; + //case 4710: wrong 2fa codes allowed per password entry, counted per user (in memory, a restart only resets the count) + private const int MAX_TFA_FAILURES = 5; + private static readonly ConcurrentDictionary TfaFailures = new ConcurrentDictionary(); + /// /// ctor /// @@ -103,6 +108,7 @@ namespace AyaNova.Api.Controllers //Generate a temporary token to identify and verify this is the same user u.TempToken = Hasher.GenerateSalt().Replace("=", "").Replace("+", ""); await ct.SaveChangesAsync(); + TfaFailures.TryRemove(u.Id, out _); var UOpt = await ct.UserOptions.AsNoTracking().FirstAsync(z => z.UserId == u.Id); List TranslationKeysToFetch = new List { "AuthTwoFactor", "AuthEnterPin", "AuthVerifyCode", "Cancel", "AuthPinInvalid" }; @@ -167,7 +173,14 @@ namespace AyaNova.Api.Controllers return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED)); } - //Match to temp token that would have been set by initial credentialed login for 2fa User + //case 4710: the temp token is the only proof the password step was passed, so it must be present + if (string.IsNullOrWhiteSpace(pin.TempToken)) + { + await Task.Delay(AyaNova.Util.ServerBootConfig.FAILED_AUTH_DELAY); + return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED)); + } + + //Match to temp token that would have been set by initial credentialed login for 2fa User var user = await ct.User.Where(z => z.TempToken == pin.TempToken && z.Active == true && z.AllowLogin==true && z.TwoFactorEnabled == true).FirstOrDefaultAsync(); @@ -184,14 +197,22 @@ namespace AyaNova.Api.Controllers var tfa = new TwoFactorAuth("AyaNova"); if (!tfa.VerifyCode(user.TotpSecret, pin.Pin.Replace(" ", "").Trim())) { + //case 4710: after too many wrong codes the password must be entered again + if (TfaFailures.AddOrUpdate(user.Id, 1, (k, n) => n + 1) >= MAX_TFA_FAILURES) + { + TfaFailures.TryRemove(user.Id, out _); + user.TempToken = null; + await ct.SaveChangesAsync(); + } await Task.Delay(AyaNova.Util.ServerBootConfig.FAILED_AUTH_DELAY); return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED)); } //User is valid and authenticated - //clear temp token - user.TempToken = string.Empty; + //clear temp token (case 4710: null, not empty, so no value is left to match) + user.TempToken = null; await ct.SaveChangesAsync(); + TfaFailures.TryRemove(user.Id, out _); return await ReturnUserCredsOnSuccessfulAuthentication(user); }