mcias/AUDIT.md

MCIAS Security Audit Report

Date: 2026-03-14 (updated — penetration test round 4) Original audit date: 2026-03-13 Auditor role: Penetration tester (code review + live instance probing) Scope: Full codebase and running instance at mcias.metacircular.net:8443 — authentication flows, token lifecycle, cryptography, database layer, REST/gRPC/UI servers, authorization, headers, and operational security. Methodology: Static code analysis, live HTTP probing, architectural review.

---

Executive Summary

MCIAS has a strong security posture. All findings from the first three audit rounds (CRIT-01/CRIT-02, DEF-01 through DEF-10, and SEC-01 through SEC-12) have been remediated. The cryptographic foundations are sound, JWT validation is correct, SQL injection is not possible, XSS is prevented by Go's html/template auto-escaping, and CSRF protection is well-implemented.

A fourth-round penetration test (PEN-01 through PEN-07) against the live instance at mcias.metacircular.net:8443 identified 7 new findings: 2 medium, 2 low, and 3 informational. Unauthorized access was not achieved — the system's defense-in-depth held. See the open findings table below for details.

---

Open Findings (PEN-01 through PEN-07)

Identified during the fourth-round penetration test on 2026-03-14 against the live instance at mcias.metacircular.net:8443 and the source code at the same commit.

ID	Severity	Finding	Status
PEN-01	Medium	extractBearerFromRequest does not validate "Bearer " prefix	Fixed — uses strings.SplitN + strings.EqualFold prefix validation, matching middleware implementation
PEN-02	Medium	Security headers missing from live instance responses	Fixed — redeployed; all headers confirmed present on live instance 2026-03-15
PEN-03	Low	CSP unsafe-inline on /docs Swagger UI endpoint	Accepted — self-hosting Swagger UI (1.7 MB) to enable nonces adds complexity disproportionate to the risk; inline script is static, no user-controlled input
PEN-04	Info	OpenAPI spec publicly accessible without authentication	Accepted — intentional; public access required for agents and external developers
PEN-05	Info	gRPC port 9443 publicly accessible	Accepted — intentional; required for server-to-server access by external systems
PEN-06	Low	REST login increments lockout counter for missing TOTP code	Fixed — RecordLoginFailure removed from TOTP-missing branch; TestTOTPMissingDoesNotIncrementLockout added
PEN-07	Info	Rate limiter is per-IP only, no per-account limiting	Accepted — per-account hard lockout (10 failures/15 min) already covers distributed brute-force; per-account rate limiting adds marginal benefit at this scale

Finding descriptions (click to expand)

PEN-01 — extractBearerFromRequest Does Not Validate "Bearer " Prefix (Medium)

File: internal/server/server.go (lines 1414–1425)

The server-level extractBearerFromRequest function extracts the token by slicing the Authorization header at offset 7 (len("Bearer ")) without first verifying that the header actually starts with "Bearer ". Any 8+ character Authorization value is accepted — e.g., Authorization: XXXXXXXX would extract X as the token string.

// Current (vulnerable):
if len(auth) <= len(prefix) {
    return "", fmt.Errorf("malformed Authorization header")
}
return auth[len(prefix):], nil  // no prefix check

The middleware-level extractBearerToken in internal/middleware/middleware.go (lines 303–316) correctly uses strings.SplitN and strings.EqualFold to validate the prefix. The server-level function should be replaced with a call to the middleware version, or the same validation logic should be applied.

Impact: Low in practice because the extracted garbage is then passed to JWT validation which will reject it. However, it violates defense-in-depth: a future change to token validation could widen the attack surface, and the inconsistency between the two extraction functions is a maintenance hazard.

Recommendation: Replace extractBearerFromRequest with a call to middleware.extractBearerToken (after exporting it or moving the function), or replicate the prefix validation.

Fix: extractBearerFromRequest now uses strings.SplitN and strings.EqualFold to validate the "Bearer" prefix before extracting the token, matching the middleware implementation. Test TestExtractBearerFromRequest covers valid tokens, missing headers, non-Bearer schemes (Token, Basic), empty tokens, case-insensitive matching, and the previously-accepted garbage input.

---

PEN-02 — Security Headers Missing from Live Instance Responses (Medium)

Live probe: https://mcias.metacircular.net:8443/login

The live instance's /login response did not include the security headers (X-Content-Type-Options, Strict-Transport-Security, Cache-Control, Permissions-Policy) that the source code's globalSecurityHeaders and UI securityHeaders middleware should be applying (SEC-04 and SEC-10 fixes).

This is likely a code/deployment discrepancy — the deployed binary may predate the SEC-04/SEC-10 fixes, or the middleware may not be wired into the route chain correctly for all paths.

Impact: Without HSTS, browsers will not enforce HTTPS-only access. Without X-Content-Type-Options: nosniff, MIME-type sniffing attacks are possible. Without Cache-Control: no-store, authenticated responses may be cached by proxies or browsers.

Recommendation: Redeploy the current source to the live instance and verify headers with curl -I.

Fix: Redeployed 2026-03-15. Live probe confirms all headers present on /login, /v1/health, and /: cache-control: no-store, content-security-policy, permissions-policy, referrer-policy, strict-transport-security: max-age=63072000; includeSubDomains, x-content-type-options: nosniff, x-frame-options: DENY.

---

PEN-03 — CSP unsafe-inline on /docs Swagger UI Endpoint (Low)

File: internal/server/server.go (lines 1450–1452)

The docsSecurityHeaders wrapper sets a Content-Security-Policy that includes script-src 'self' 'unsafe-inline' and style-src 'self' 'unsafe-inline'. This is required by Swagger UI's rendering approach, but it weakens CSP protection on the docs endpoint.

Impact: If an attacker can inject content into the Swagger UI page (e.g., via a reflected parameter in the OpenAPI spec URL), inline scripts would execute. The blast radius is limited to the /docs path, which requires no authentication (see PEN-04).

Recommendation: Consider serving Swagger UI from a separate subdomain or using CSP nonces instead of unsafe-inline. Alternatively, accept the risk given the limited scope.

---

PEN-04 — OpenAPI Spec Publicly Accessible Without Authentication (Informational)

Live probe: GET /openapi.yaml returns the full API specification without authentication.

The OpenAPI spec reveals all API endpoints, request/response schemas, authentication flows, and error codes. While security-through-obscurity is not a defense, exposing the full API surface to unauthenticated users provides a roadmap for attackers.

Recommendation: Consider requiring authentication for /openapi.yaml and /docs, or accept the risk if the API surface is intended to be public.

---

PEN-05 — gRPC Port 9443 Publicly Accessible (Informational)

Live probe: Port 9443 accepts TLS connections and serves gRPC.

The gRPC interface is accessible from the public internet. While it requires authentication for all RPCs, exposing it increases the attack surface (gRPC-specific vulnerabilities, protocol-level attacks).

Recommendation: If gRPC is only used for server-to-server communication, restrict access at the firewall/network level. If it must be public, ensure gRPC-specific rate limiting and monitoring are in place (SEC-06 fix applies here).

---

PEN-06 — REST Login Increments Lockout Counter for Missing TOTP Code (Low)

File: internal/server/server.go (lines 271–277)

When a TOTP-enrolled account submits a login request without a TOTP code, the REST handler calls s.db.RecordLoginFailure(acct.ID) before returning the "TOTP code required" error. This increments the lockout counter even though the user has not actually failed authentication — they simply omitted the TOTP field.

The gRPC handler was fixed for this exact issue in DEF-08, but the REST handler was not updated to match.

// Current (REST — increments lockout for missing TOTP):
if acct.TOTPRequired {
    if req.TOTPCode == "" {
        s.writeAudit(r, model.EventLoginFail, &acct.ID, nil, `{"reason":"totp_missing"}`)
        _ = s.db.RecordLoginFailure(acct.ID)  // should not increment
        middleware.WriteError(w, http.StatusUnauthorized, "TOTP code required", "totp_required")
        return
    }

Impact: An attacker who knows a username with TOTP enabled can lock the account by sending 10 login requests with a valid password but no TOTP code. The password must be correct (wrong passwords also increment the counter), but this lowers the bar from "must guess TOTP" to "must omit TOTP." More practically, legitimate users with buggy clients that forget the TOTP field could self-lock.

Recommendation: Remove the RecordLoginFailure call from the TOTP-missing branch, matching the gRPC handler's behavior after the DEF-08 fix.

Fix: RecordLoginFailure removed from the TOTP-missing branch in internal/server/server.go. The branch now matches the gRPC handler exactly, including the rationale comment. TestTOTPMissingDoesNotIncrementLockout verifies the fix: it fully enrolls TOTP via the HTTP API, sets LockoutThreshold=1, issues a TOTP-missing login, and asserts the account is not locked.

---

PEN-07 — Rate Limiter Is Per-IP Only, No Per-Account Limiting (Informational)

The rate limiter uses a per-IP token bucket. An attacker with access to multiple IP addresses (botnet, cloud instances, rotating proxies) can distribute brute-force attempts across IPs to bypass the per-IP limit.

The account lockout mechanism (10 failures in 15 minutes) provides a secondary defense, but it is a blunt instrument — it locks out the legitimate user as well.

Recommendation: Consider adding per-account rate limiting as a complement to per-IP limiting. This would cap login attempts per username regardless of source IP, without affecting other users. The account lockout already partially serves this role, but a softer rate limit (e.g., 1 req/s per username) would slow distributed attacks without locking out the user.

---

Remediated Findings (SEC-01 through SEC-12)

All findings from the SEC audit round have been remediated. The original descriptions are preserved below for reference.

ID	Severity	Finding	Status
SEC-01	Medium	TOTP enrollment did not require password re-authentication	Fixed — both REST and gRPC now require current password, with lockout counter on failure
SEC-02	Medium	Account lockout response leaked account existence	Fixed — locked accounts now return same 401 "invalid credentials" as wrong password, with dummy Argon2 for timing uniformity
SEC-03	Medium	Token renewal had no proximity or re-auth check	Fixed — renewal requires token to have consumed ≥50% of its lifetime
SEC-04	Low-Med	REST API responses lacked security headers	Fixed — globalSecurityHeaders middleware applies X-Content-Type-Options, HSTS, and Cache-Control: no-store to all routes
SEC-05	Low	No request body size limit on REST API	Fixed — decodeJSON wraps body with http.MaxBytesReader (1 MiB); max password length enforced
SEC-06	Low	gRPC rate limiter ignored TrustedProxy	Fixed — grpcClientIP extracts real client IP via metadata when peer matches trusted proxy
SEC-07	Low	Static file directory listing enabled	Fixed — noDirListing wrapper returns 404 for directory requests
SEC-08	Low	System token issuance was not atomic	Fixed — IssueSystemToken wraps revoke+track in a single SQLite transaction
SEC-09	Info	Navigation bar exposed admin UI structure to non-admin users	Fixed — nav links conditionally rendered with {{if .IsAdmin}}
SEC-10	Info	No Permissions-Policy header	Fixed — Permissions-Policy: camera=(), microphone=(), geolocation=(), payment=() added
SEC-11	Info	Audit log details used fmt.Sprintf instead of json.Marshal	Fixed — audit.JSON and audit.JSONWithRoles helpers use json.Marshal
SEC-12	Info	Default token expiry was 30 days	Fixed — default reduced to 7 days (168h); renewal proximity check (SEC-03) further limits exposure

Original finding descriptions (click to expand)

SEC-01 — TOTP Enrollment Does Not Require Password Re-authentication (Medium)

Files: internal/server/server.go, internal/grpcserver/auth.go

POST /v1/auth/totp/enroll and the gRPC EnrollTOTP RPC originally required only a valid JWT — no password confirmation. If an attacker stole a session token, they could enroll TOTP on the victim's account.

Fix: Both endpoints now require the current password, with lockout counter incremented on failure.

---

SEC-02 — Account Lockout Response Leaks Account Existence (Medium)

Locked accounts originally returned HTTP 429 / gRPC ResourceExhausted with "account temporarily locked", distinguishable from the HTTP 401 "invalid credentials" returned for wrong passwords.

Fix: All login paths now return the same "invalid credentials" response for locked accounts, with dummy Argon2 to maintain timing uniformity.

---

SEC-03 — Token Renewal Has No Proximity or Re-auth Check (Medium)

POST /v1/auth/renew originally accepted any valid token regardless of remaining lifetime.

Fix: Renewal now requires the token to have consumed ≥50% of its lifetime before it can be renewed.

---

SEC-04 — REST API Responses Lack Security Headers (Low-Medium)

API endpoints originally returned only Content-Type — no Cache-Control, X-Content-Type-Options, or HSTS.

Fix: globalSecurityHeaders middleware applies these headers to all routes (API and UI).

---

SEC-05 — No Request Body Size Limit on REST API Endpoints (Low)

decodeJSON originally read from r.Body without any size limit.

Fix: http.MaxBytesReader with 1 MiB limit added to decodeJSON. Maximum password length also enforced.

---

SEC-06 — gRPC Rate Limiter Ignores TrustedProxy (Low)

The gRPC rate limiter originally used peer.FromContext directly, always getting the proxy IP behind a reverse proxy.

Fix: grpcClientIP now reads from gRPC metadata headers when the peer matches the trusted proxy.

---

SEC-07 — Static File Directory Listing Enabled (Low)

http.FileServerFS served directory listings by default.

Fix: noDirListing wrapper returns 404 for directory requests.

---

SEC-08 — System Token Issuance Is Not Atomic (Low)

handleTokenIssue originally performed three sequential non-transactional operations.

Fix: IssueSystemToken wraps all operations in a single SQLite transaction.

---

SEC-09 — Navigation Bar Exposes Admin UI Structure to Non-Admin Users (Informational)

Nav links were rendered for all authenticated users.

Fix: Admin nav links wrapped in {{if .IsAdmin}} conditional.

---

SEC-10 — No Permissions-Policy Header (Informational)

The security headers middleware did not include Permissions-Policy.

Fix: Permissions-Policy: camera=(), microphone=(), geolocation=(), payment=() added.

---

SEC-11 — Audit Log Details Use fmt.Sprintf Instead of json.Marshal (Informational)

Audit details were constructed with fmt.Sprintf and %q, which is fragile for JSON.

Fix: audit.JSON and audit.JSONWithRoles helpers use json.Marshal.

---

SEC-12 — Default Token Expiry Is 30 Days (Informational / Configuration)

Default expiry was 720h (30 days).

Fix: Reduced to 168h (7 days). Combined with SEC-03's renewal proximity check, exposure window is significantly reduced.

---

Previously Remediated Findings (CRIT/DEF series)

The following findings from the initial audit (2026-03-12) were confirmed fixed in the 2026-03-13 audit:

ID	Finding	Status
CRIT-01	TOTP replay attack — no counter tracking	Fixed — CheckAndUpdateTOTPCounter with atomic SQL, migration 000007
CRIT-02	gRPC EnrollTOTP called SetTOTP instead of StorePendingTOTP	Fixed — now calls StorePendingTOTP
DEF-01	No rate limiting on UI login	Fixed — loginRateLimit applied to POST /login
DEF-02	pendingLogins map had no expiry cleanup	Fixed — cleanupPendingLogins goroutine runs every 5 minutes
DEF-03	Rate limiter ignored X-Forwarded-For	Fixed — ClientIP() respects TrustedProxy config
DEF-04	Missing nbf claim on tokens	Fixed — NotBefore: jwt.NewNumericDate(now) added
DEF-05	No max token expiry ceiling	Fixed — upper bounds enforced in config validation
DEF-06	Incorrect case-sensitivity comment	Fixed — comment corrected
DEF-07	SQLite synchronous=NORMAL	Fixed — changed to PRAGMA synchronous=FULL
DEF-08	gRPC counted TOTP-missing as failure	Fixed — no longer increments lockout counter
DEF-09	Security headers missing on docs endpoints	Fixed — docsSecurityHeaders wrapper added
DEF-10	Role strings not validated	Fixed — model.ValidateRole() with compile-time allowlist

---

Positive Findings (Preserved)

These implementation details are exemplary and should be maintained:

Area	Detail
JWT alg confusion	ValidateToken enforces alg=EdDSA in the key function before signature verification
Constant-time operations	crypto/subtle.ConstantTimeCompare for password hashes, CSRF tokens; all three TOTP windows evaluated without early exit
Timing uniformity	Dummy Argon2 via sync.Once for unknown/inactive users on all login paths
Token revocation	Fail-closed: untracked tokens are rejected, not silently accepted
Token renewal atomicity	RenewToken wraps revoke+track in a single SQLite transaction
TOTP replay prevention	Counter-based replay detection with atomic SQL UPDATE/WHERE
TOTP nonce design	128-bit single-use server-side nonce; password never retransmitted in step 2
CSRF protection	HMAC-SHA256 double-submit cookie, domain-separated key derivation, SameSite=Strict, constant-time validation
Credential exclusion	json:"-" on all credential fields; password hash never in API responses
Security headers (UI)	CSP (no unsafe-inline), X-Content-Type-Options, X-Frame-Options DENY, HSTS 2yr, Referrer-Policy no-referrer
Cookie hardening	HttpOnly + Secure + SameSite=Strict on session cookie
Account lockout	10-attempt rolling window, checked before Argon2, with timing-safe dummy hash
Argon2id parameters	Config validator enforces OWASP 2023 minimums; rejects weakening
SQL injection	Zero string concatenation — all queries parameterized
Input validation	Username regex + length, password min length, account type enum, role allowlist, JSON strict decoder
Audit logging	Append-only, no delete path, credentials never logged, actor/target/IP captured
Master key hygiene	Env var cleared after read, key zeroed on shutdown, AES-256-GCM at rest
TLS	MinVersion TLS 1.2, X25519 preferred, no plaintext listener, read/write/idle timeouts set

---

Penetration Test — Attacks That Failed (2026-03-14)

The following attacks were attempted against the live instance and failed, confirming the effectiveness of existing defenses:

Attack	Result
JWT alg:none bypass	Rejected — ValidateToken enforces alg=EdDSA
JWT alg:HS256 key-confusion	Rejected — only EdDSA accepted
Forged JWT with random Ed25519 key	Rejected — signature verification failed
Username enumeration via timing	Not possible — ~355ms for both existing and non-existing users (dummy Argon2 working)
Username enumeration via error messages	Not possible — identical "invalid credentials" for all failure modes
Account lockout enumeration	Not possible — locked accounts return same response as wrong password (SEC-02 fix confirmed)
SQL injection via login fields	Not possible — parameterized queries throughout
JSON body bomb (oversized payload)	Rejected — http.MaxBytesReader returns 413 (SEC-05 fix confirmed)
Unknown JSON fields	Rejected — DisallowUnknownFields active on decoder
Rate limit bypass	Working correctly — 429 after burst exhaustion, Retry-After header present
Admin endpoint access without auth	Properly returns 401
Directory traversal on static files	Not possible — noDirListing wrapper returns 404 (SEC-07 fix confirmed)
Public key endpoint	Returns Ed25519 PKIX key (expected; public by design)

---

Remediation Status

CRIT/DEF/SEC series: All 24 findings remediated. No open items.

PEN series (2026-03-14): All 7 findings resolved — 4 fixed, 3 accepted by design. Unauthorized access was not achieved. No open items remain.

Next audit should focus on:

• Any new features added since 2026-03-15
• Dependency updates and CVE review
• Re-evaluate PEN-03 if Swagger UI self-hosting becomes desirable