admin/_docs/05_security/static_analysis_cycle2.md

Static Analysis — Cycle 2 (Auth Modernization, AZ-531..AZ-538)

Date: 2026-05-14 Scope: cycle-2 source files + cross-cutting changes; the cycle-1 surface was already audited in static_analysis.md.

Files reviewed

• New: Azaion.Services/{Security,RefreshTokenService,SessionService,MfaService,MissionTokenService,JwtSigningKeyProvider,AuditLog}.cs
• Modified: Azaion.Services/{AuthService,UserService}.cs, Azaion.AdminApi/{Program,BusinessExceptionHandler}.cs
• Common: Azaion.Common/{BusinessException,Configs/AuthConfig,Configs/JwtConfig,Configs/SessionConfig,Entities/Session,Entities/AuditEvent,Entities/User,Database/AzaionDb,Database/AzaionDbShemaHolder,Requests/*}.cs
• DB: env/db/{07_auth_lockout_and_audit,08_sessions,09_sessions_logout_and_mission,10_users_mfa}.sql

Findings — cycle 2 (severity-ranked)

F-2026Q2-AUTH-1: User enumeration via login error codes (HIGH)

• Location: Azaion.Services/UserService.cs:120-148, Azaion.Common/BusinessException.cs:33-52.
• Description: ValidateUser returns BusinessException(NoEmailFound) (code 10, "No such email found.") when the email doesn't exist, and BusinessException(WrongPassword) (code 30, "Passwords do not match.") when the email exists but the password is wrong. The two codes (and human-readable messages) are distinguishable to any client. This is a textbook user-enumeration leak.
• Status: pre-existing in cycle 1 (not introduced by cycle 2) but the threat model is materially worse now — cycle 2 added per-account lockout that an attacker can selectively trigger only on real accounts.
• Impact: aids credential stuffing (attacker pre-filters their email list), aids targeted phishing, allows opportunistic DoS-via-lockout against known-real accounts.
• Remediation: collapse both branches to a single generic error — either WrongPassword for both, or a new InvalidCredentials code. Cycle-2 audit recommends: introduce InvalidCredentials (code 70 or similar) and migrate over a deprecation window.
• Cross-ref: _docs/05_security/static_analysis.md already captured this for cycle 1 but as Medium; upgraded to High for cycle 2.

F-2026Q2-AUTH-2: MFA brute force not rate-limited per account (HIGH)

• Location: Azaion.Services/MfaService.cs:247-278.
• Description: VerifyForLogin records audit_events.mfa_login_failed on a wrong code but does NOT increment users.failed_login_count or users.lockout_until. The per-account sliding-window check in UserService.ValidateUser only counts event_type='login_failed' rows, not mfa_login_failed. The per-IP rate limiter on /login/mfa is the only throttle.
• Impact: an attacker with a leaked password can brute-force the 6-digit TOTP from rotating IPs. ~1M tries per minute saturate the 30s TOTP step before lockout; with rotation across IPs the per-IP limit is bypassed. The account never locks.
• Remediation: either (a) make CountRecentFailedLogins also count mfa_login_failed events, or (b) call RegisterFailedLogin-equivalent inside VerifyForLogin so failed_login_count increments. Recommend (b) because it surfaces the MFA failures into the existing lockout mechanism.

F-2026Q2-AUTH-3: Disabled-account leak via auth ordering (MEDIUM)

• Location: Azaion.Services/UserService.cs:144-152.

• Description: ValidateUser checks IsEnabled AFTER password verification:

  var verify = Security.VerifyPassword(...);
  if (!verify.Valid) throw WrongPassword;
  if (!user.IsEnabled) throw UserDisabled;
  

  An attacker who knows the password of a disabled account learns the password is correct (sees UserDisabled instead of WrongPassword).

• Impact: confirms a credential pair is valid even if the account is disabled — useful for credential-replay against re-enabled accounts and for tracking which disabled accounts to keep targeting.

• Remediation: check IsEnabled BEFORE the password verify, return the same generic error as F-AUTH-1.

F-2026Q2-AUTH-4: MFA management endpoints not rate-limited (MEDIUM)

• Location: Azaion.AdminApi/Program.cs:452-481.
• Description: /users/me/mfa/{enroll,confirm,disable} are mounted with RequireAuthorization() but NOT RequireRateLimiting(LoginPerIpPolicy). Only /login and /login/mfa have the per-IP limit attached. An attacker with a stolen access token can brute-force the disable-confirm code unbounded.
• Impact: stolen-access-token escalation path that ends in MFA being silently turned off; account is then a single-factor target.
• Remediation: add a dedicated mfa-per-account policy (or attach LoginPerIpPolicy) to all three /users/me/mfa/* endpoints.

F-2026Q2-AUTH-5: Mission tokens missing amr claim (MEDIUM)

• Location: Azaion.Services/MissionTokenService.cs:103-111.
• Description: The mission token claim list omits amr. The cycle-2 design intent (per _docs/02_document/system-flows.md F13 and the // TODO (AZ-534) comment in Program.cs:489) is amr=["pwd","mfa"]. Verifiers downstream cannot enforce "missions require MFA-authenticated pilots" because the claim is absent.
• Impact: a pilot logged in with pwd only can issue mission tokens that look identical to MFA-pilot ones to verifiers. The intended MFA-step-up gating is unenforceable.
• Remediation: read the pilot's current access-token AMR (or look it up via sid → Session.MfaAuthenticated) and stamp amr=["pwd"] or amr=["pwd","mfa"] accordingly. Reject if policy requires MFA and the pilot has none.

F-2026Q2-AUTH-6: Mission token issuance does not auto-revoke prior aircraft missions (MEDIUM)

• Location: Azaion.Services/MissionTokenService.cs:46-87.
• Description: Issue inserts a new mission session row but does NOT call SessionService.RevokeMissionsForAircraft(aircraftId) first. Auto-revoke happens only when the aircraft itself logs in (Program.cs:347-349). Consequence: an admin can issue two concurrent mission tokens for the same aircraft and both remain valid until the aircraft re-connects.
• Impact: violates AZ-533 AC-4. Two pilots can hold parallel valid mission tokens for the same aircraft, breaking the "one mission per aircraft at a time" invariant. Audit / forensic confusion.
• Remediation: call SessionService.RevokeMissionsForAircraft(request.AircraftId, ct) immediately before the db.InsertAsync(new Session ...) for the mission row.
• Doc impact: also fix the cycle-2 docs (system-flows.md F13, components/03_auth_and_security/description.md) which currently claim this auto-revoke happens at issuance.

F-2026Q2-AUTH-7: Silent fallback to first PEM if ActiveKid is unset (MEDIUM)

• Location: Azaion.Services/JwtSigningKeyProvider.cs:73-86.
• Description: When JwtConfig.ActiveKid is not configured, the provider falls back to the alphabetically-first PEM and only logs a LogInformation. Adding a new PEM whose filename sorts earlier than the current active kid silently changes the signing key.
• Impact: rotation accidents — operator drops kid-2026-04-aaa.pem thinking it's a side-by-side key, but it becomes the new signer because of name ordering. Tokens minted under the wrong kid; verifiers may reject (depending on JWKS cache state).
• Remediation: fail-fast when ActiveKid is unset and more than one PEM exists. If exactly one PEM exists, accept it but log at Warning (not Information).

F-2026Q2-AUTH-8: Health-readiness leaks DB exception type to anonymous callers (LOW)

• Location: Azaion.AdminApi/Program.cs:261-280.
• Description: /health/ready returns { status: "not-ready", reason: ex.GetType().Name } to any anonymous caller on DB failure. This leaks the underlying exception type (e.g., NpgsqlException, TimeoutException).
• Impact: minor info leak; no direct exploit but useful for fingerprinting in a compromise chain.
• Remediation: log the exception type internally; return only { status: "not-ready" } externally.
• Mitigating control: per Program.cs line 252 comment, /health/* is "expected to be exposed only on the management interface (not via the public Nginx vhost)". If that nginx config is verified, drop this finding.

F-2026Q2-AUTH-9: User cache TTL holds lockout / MFA-state for 4h across instances (MEDIUM, deployment-coupled)

• Location: Azaion.Services/UserService.GetByEmail (cycle 1) + cycle-2 surface that consumes User.LockoutUntil / MfaEnabled.
• Description: GetByEmail returns a 4-hour-cached User that includes LockoutUntil, MfaEnabled, MfaSecret. cache.Invalidate is local to the process. In a multi-instance deploy, instance A's lockout / MFA-disable / password-rehash is invisible to instance B for up to 4 h.
• Impact: a locked account can keep authenticating against a cold instance; an MFA-disabled user can keep being challenged for MFA against a stale instance; lazy-rehash race window grows.
• Mitigating control: per architecture.md §3, deployment is "self-hosted Linux server (single container deployment via deploy.cmd)". Single-instance = no cross-instance staleness today. Risk surfaces if/when the system scales horizontally.
• Remediation: bypass the cache for the security-critical fields (LockoutUntil, FailedLoginCount, MfaEnabled, MfaSecret, IsEnabled) by reading them directly in ValidateUser and MfaService, OR move to a shared-cache (Redis) before horizontal scaling.

F-2026Q2-DOCS-1: Documentation drift surfaced during audit (LOW)

• Location: _docs/02_document/components/03_auth_and_security/description.md, _docs/02_document/data_model.md §Configuration POCOs, _docs/02_document/modules/services_mfa_service.md.
• Description: cycle-2 documentation drifted from the actual code on:
  • AuthConfig.PasswordHashing { TimeCost, MemoryCostKiB, Parallelism } does NOT exist — Argon2id parameters are hardcoded constants in Security.cs:16-23.
  • LockoutOptions has fields MaxAttempts (not ConsecutiveFailureThreshold) and DurationSeconds (not LockoutSeconds).
  • RateLimitOptions has PerAccountPermitLimit (not PerAccountFailedThreshold).
  • Recovery codes are SHA-256-hashed (high-entropy server-issued secrets), not Argon2id as docs claimed.
• Impact: misleads operators when tuning Argon2id; misleads ops when wiring appsettings.json.
• Remediation: docs-only fix in Step 13 follow-up — to be addressed in the next docs sync. Tracked here so it's not forgotten.

Vulnerability patterns scan (clean)

Pattern	Result
SQL injection (string-interpolated SQL)	Clean — only db.Execute("UPDATE ... WHERE id=@id", DataParameter) and SELECT 1. All cycle-2 queries are LinqToDB expression-based.
Command injection / Process.Start / subprocess	Clean — no shell-out from C# in the cycle-2 surface.
XSS	N/A — JSON-only API; responses serialized via Newtonsoft.Json / Results.Ok.
Hardcoded secrets / credentials	Clean — no plaintext credentials in Azaion.{AdminApi,Services,Common}/. The seed admin@azaion.com/uploader@azaion.com rows in env/db/02_structure.sql use SHA-384 hashes; cycle 2 lazy-migrates these on first login.
Missing auth on endpoints	Clean — verified each MapGet/MapPost/MapPut/MapDelete carries either RequireAuthorization, RequireAuthorization(<policy>), or explicit AllowAnyonymous with documented justification.
Insecure deserialization	Clean — Newtonsoft.Json only deserializes typed DTOs; System.Text.Json deserializes the recovery-code array; jsonb mapping for MfaRecoveryCodes uses DataType.BinaryJson.
Sensitive data in logs	Clean — passwords, hashes, secrets, refresh tokens, recovery codes are not logged. BusinessExceptionHandler logs the exception type + message but no payload.
Cryptographic regressions	Clean — Argon2id (RFC 9106) for passwords; SHA-256 for high-entropy refresh tokens + recovery codes; ES256 for JWTs; SHA-256 of UTF-8 bytes for refresh-token hashing. No MD5 / SHA-1 / DES anywhere.

Self-verification

• All cycle-2 source directories scanned
• Each finding has file path and line numbers
• No false positives from test files (test files were excluded except where they confirm a finding)
• Vulnerability patterns swept (SQLi, cmd injection, secrets, missing auth, insecure deserialization, sensitive logs, weak crypto)