Updated Security Audit
This commit is contained in:
+94
-226
@@ -2,286 +2,154 @@
|
||||
|
||||
**Assessment date:** 2026-08-26
|
||||
**Scope:** Current LinkLog backend, web frontend, Firefox extension, SQLite persistence, SMTP and Mastodon integrations, Docker/Traefik deployment files, and automated tests.
|
||||
**Assessment type:** Source-code security review. This is not a penetration test, dependency vulnerability scan, formal threat model sign-off, or production configuration certification.
|
||||
**Assessment type:** Source-code review. This is not a penetration test, dependency scan, container scan, formal threat-model sign-off, or production configuration certification.
|
||||
|
||||
## Executive Summary
|
||||
|
||||
LinkLog has several good security foundations: authenticated API dependencies, administrator authorization checks, owner checks for link operations, token hashing in the database, email verification, password reset token hashing and single-use behavior, TOTP login enforcement, last-administrator protection, parameterized SQLite queries, upload size limits, and non-root application execution in the container.
|
||||
The current worktree contains strong security improvements: salted scrypt password hashing with legacy upgrade support, bearer-header authentication, hashed and expiring tokens, refresh-token rotation with device binding and family revocation, OTP recovery codes, encrypted newly written secrets, Mastodon SSRF controls, image decoding and re-encoding, reduced extension permissions, proxy-only production Compose, and append-only audit events.
|
||||
|
||||
The current implementation is not ready to expose directly to the public Internet without additional hardening. The most important issues are:
|
||||
The following issues remain before an Internet-facing production release:
|
||||
|
||||
1. Passwords were previously stored as unsalted, fast SHA-256 hashes; this issue has now been addressed in the current worktree with salted scrypt hashes and legacy upgrade support.
|
||||
2. Access tokens are accepted in query strings by session endpoints, which can leak through logs, browser history, proxies, and referrers.
|
||||
3. SMTP credentials, Mastodon credentials, OAuth client secrets, and TOTP secrets are stored in plaintext in SQLite.
|
||||
4. Mastodon instance URLs are user-controlled and the backend makes outbound requests to them, creating an SSRF and egress-control concern.
|
||||
5. Login has no effective rate limiting or account lockout.
|
||||
6. The Firefox extension stores bearer tokens in browser local storage and requests broad website access.
|
||||
7. The Compose setup still exposes the application port directly and relies on deployment-specific Traefik networking and labels.
|
||||
1. Logout still accepts a bearer token in a JSON body rather than using the standard `Authorization` header.
|
||||
2. SMTP, Mastodon, setup, and some user-service errors return raw exception details to clients.
|
||||
3. First-run setup is intentionally unauthenticated and lacks a bootstrap secret and application-level request-size controls.
|
||||
4. Audit event details are serialized without defensive sanitization or size limits at the audit-service boundary.
|
||||
5. The development secret fallback is not rejected at application startup in production.
|
||||
6. Rate limiting is single-instance SQLite state, is not atomic under concurrency, and reset-mail issuance is not independently throttled.
|
||||
7. Runtime verification, security headers, centralized audit export, retention, alerting, and dependency/container/security scanning remain incomplete.
|
||||
|
||||
These findings are prioritized below. Severity describes the potential security impact in a typical Internet-facing deployment, not the likelihood in every environment.
|
||||
The application should remain behind the production reverse proxy, with real DNS/TLS, protected secrets, and restricted network access until these items are addressed.
|
||||
|
||||
## Positive Controls Already Present
|
||||
## Verified Controls
|
||||
|
||||
- Bearer authentication is centralized in `backend/app/api/dependencies.py`.
|
||||
- Administrator routes use `require_admin`; standard users receive `403`.
|
||||
- Link update, delete, and Mastodon-post operations verify ownership.
|
||||
- Tokens are generated with UUID material, stored as SHA-256 hashes, expire, and can be revoked.
|
||||
- Password-reset tokens are random, hashed, expiring, single-use, and revoke existing sessions after reset.
|
||||
- New administrator-created users require email verification before login.
|
||||
- OTP uses time-based verification with a one-step clock window and is required before token issuance when enabled.
|
||||
- The profile API does not return `password_hash` or `otp_secret` after the profile response hardening.
|
||||
- User privilege changes protect against removing the last administrator and prevent an administrator from changing their own privilege.
|
||||
- Uploaded avatars have a 2 MB limit, a restricted MIME allow-list, user-scoped filenames, and a persistent data location.
|
||||
- SQLite foreign keys are enabled and ownership predicates are used for destructive link operations.
|
||||
- SQL statements use parameters rather than interpolated user values.
|
||||
- The Docker image runs the application as UID 10001 after startup and defines a health check.
|
||||
- `.env` and database/runtime files are ignored by Git.
|
||||
- The XPI build validates archive integrity, required files, and manifest parity.
|
||||
- Browser rendering generally uses `textContent` for feed data, reducing DOM-based injection risk.
|
||||
- Passwords use salted scrypt hashes; valid legacy SHA-256 hashes are upgraded on login.
|
||||
- Bearer authentication is centralized through `get_current_user` and `require_admin`.
|
||||
- Query-string authentication is not accepted by protected session endpoints.
|
||||
- Access tokens are short-lived by default; refresh tokens are hashed, separately expiring, device-bound, rotated, and family-revoked on reuse.
|
||||
- Logout, password changes, and password resets revoke session material according to the token lifecycle.
|
||||
- OTP enrollment provides ten one-time recovery codes; only hashes are stored.
|
||||
- Users can recover OTP with password plus a recovery code, and administrators can disable OTP for another user.
|
||||
- Newly written SMTP, Mastodon, OAuth, and OTP secrets are encrypted with an external Fernet key.
|
||||
- Mastodon instances are restricted to HTTPS public hostnames, unsafe resolved addresses are rejected, and redirects are blocked.
|
||||
- Avatar uploads are size-limited, decoded with Pillow, pixel-limited, fully loaded, and re-encoded as server-generated PNG.
|
||||
- Production Compose does not publish the application port and uses the external Traefik network; local direct access is separate.
|
||||
- The Firefox extension uses `activeTab`, session-scoped credentials, exact configured backend permissions, and a self-only extension-page CSP.
|
||||
- SQLite queries are parameterized and foreign-key enforcement is enabled.
|
||||
- An append-only `security_audit_events` table records actor, action, target, outcome, and details for major administrative and destructive operations.
|
||||
- The current automated backend suite passes 49 tests.
|
||||
|
||||
## Findings
|
||||
|
||||
### SA-001: Unsalted fast SHA-256 password hashing
|
||||
### SA-001: Logout uses non-standard token transport
|
||||
|
||||
**Severity:** Critical, remediated in current worktree
|
||||
**Evidence before remediation:** `backend/app/database.py` and `backend/app/services/auth_service.py` used unsalted SHA-256 password comparisons.
|
||||
**Current state:** `backend/app/database.py` now creates salted scrypt hashes in the format `scrypt$N$r$p$salt$digest`. `verify_password()` uses the encoded parameters and constant-time comparison. `authenticate_user()` fetches by username, verifies in Python, and transparently replaces a valid legacy 64-character SHA-256 hash with a new scrypt hash.
|
||||
**Residual impact:** Existing accounts remain exposed until they successfully authenticate once after deployment. An attacker with a copy of an old database may still attack legacy hashes. Existing credentials should be rotated if the old database may have been exposed.
|
||||
**Severity:** High
|
||||
**Evidence:** `POST /api/auth/logout` accepts `{"token": ...}` in the JSON request body, and the web frontend sends the access token this way.
|
||||
**Impact:** Request bodies may be captured by debugging middleware, application logs, or monitoring systems. The endpoint also diverges from the bearer-header contract used elsewhere, increasing the chance of inconsistent token handling.
|
||||
|
||||
**Recommendation:** Deploy the current migration, require password rotation for accounts that cannot log in during migration, and monitor for remaining legacy hashes. Review scrypt cost parameters periodically and increase them as hardware changes. Do not revert to a fast general-purpose hash.
|
||||
**Recommendation:** Make logout use `Authorization: Bearer <access-token>` and revoke the authenticated token or its family server-side. If a compatibility period is required, support both forms temporarily, prefer the header, and remove the body form after client migration. Add tests proving body-only tokens are rejected once compatibility is removed.
|
||||
|
||||
**Priority:** Completed in code; operational migration and credential rotation remain.
|
||||
**Priority:** High.
|
||||
|
||||
### SA-002: Bearer tokens accepted in query strings
|
||||
### SA-002: Raw infrastructure errors are returned to clients
|
||||
|
||||
**Severity:** High, remediated in current worktree
|
||||
**Evidence before remediation:** `backend/app/api/auth.py` exposed `GET /api/auth/me?token=...`, and web/extension callers used the query form.
|
||||
**Current state:** `/api/auth/me` now requires the existing bearer-header dependency. The shared web header, admin session check, Firefox settings page, and tests send `Authorization: Bearer <token>`. A query-string token is rejected with `401`.
|
||||
**Residual impact:** Tokens from old URLs may remain in proxy/browser logs and should be treated as exposed until revoked or rotated.
|
||||
**Severity:** High
|
||||
**Evidence:** SMTP and Mastodon routes interpolate exception text into `503`/`502` responses. Setup and email-address routes also expose mail-delivery exception text.
|
||||
**Impact:** Error responses can disclose SMTP hostnames, ports, TLS/library details, upstream response bodies, internal network information, or sensitive URL fragments.
|
||||
|
||||
**Recommendation:** Rotate existing access tokens after deployment and scrub historical query parameters from logs where possible. Keep the bearer header as the only credential transport.
|
||||
**Recommendation:** Log technical details server-side with a request/correlation ID and return a stable public message with a short reference ID. Redact credentials, authorization headers, reset tokens, OTP data, and secret-bearing URLs. Add tests asserting that representative exception text is absent from HTTP responses.
|
||||
|
||||
**Priority:** Completed in code; token rotation and log hygiene remain.
|
||||
**Priority:** High.
|
||||
|
||||
### SA-003: Sensitive secrets stored in plaintext SQLite
|
||||
### SA-003: First-run setup is unauthenticated and lacks application-level body limits
|
||||
|
||||
**Severity:** High, remediated in current worktree for newly written secrets
|
||||
**Evidence:** `backend/app/services/email_service.py` stores SMTP settings including `smtp_password` in `app_settings`; `backend/app/services/mastodon_oauth.py` stores Mastodon application secrets and user access tokens in `app_settings` and `user_plugin_config`; `backend/app/api/user_config.py` stores `otp_secret` in the `users` table. New writes are encrypted, but legacy plaintext rows require rotation.
|
||||
**Impact:** Read access to the database exposes SMTP credentials, Mastodon posting authority, OAuth client secrets, and TOTP seeds. TOTP seeds cannot be changed by a user who loses the database copy. Database backups therefore contain reusable credentials, not just application data.
|
||||
**Severity:** Medium/High for exposed fresh deployments
|
||||
**Evidence:** `/api/setup/configuration`, `/api/setup/test-mail`, `/api/setup/complete`, and `/api/setup/status` are available before an administrator exists. No global request-size middleware or bootstrap secret is enforced in the application.
|
||||
|
||||
**Current state:** Newly stored SMTP passwords, Mastodon OAuth client secrets and access tokens, and TOTP seeds are encrypted with Fernet using `LINKLOG_DATA_ENCRYPTION_KEY`. The key is required in Docker and is not stored in SQLite. The user plugin API no longer returns the Mastodon access token.
|
||||
**Residual impact:** Existing plaintext secrets require a controlled read-and-save rotation after the key is configured. Lost encryption keys make stored secrets unrecoverable.
|
||||
**Impact:** Anyone who can reach a fresh instance can overwrite pending setup values, attempt SMTP delivery, consume test-mail quota, and submit oversized request bodies. The setup design is necessary for provisioning but is unsafe when directly exposed.
|
||||
|
||||
**Recommendation:** Supply `LINKLOG_DATA_ENCRYPTION_KEY` through a protected secret mechanism, encrypt backups, rotate credentials after suspected disclosure, and migrate existing plaintext values. Continue omitting secrets from API responses.
|
||||
**Recommendation:** Require a one-time bootstrap secret supplied through the environment or console, or restrict setup to localhost/private management networking. Add bounded request models and a global body-size limit. Keep strict setup/test-mail throttling, audit setup actions, expire pending setup data, and disable setup routes after provisioning.
|
||||
|
||||
**Priority:** Completed for new writes; existing secret migration and key management remain.
|
||||
**Priority:** High for Internet-facing fresh installations.
|
||||
|
||||
### SA-004: User-controlled Mastodon instance creates SSRF and uncontrolled egress risk
|
||||
|
||||
**Severity:** High, remediated in current worktree
|
||||
**Evidence before remediation:** Mastodon instance values were passed to outbound `urlopen()` calls with no DNS/IP-range or redirect controls.
|
||||
**Current state:** `mastodon_security.py` requires hostname-only HTTPS URLs, resolves DNS, rejects loopback, link-local, private, multicast, unspecified, reserved, and IPv4-mapped IPv6 addresses, and uses an opener that refuses redirects. OAuth, posting, and deletion all use these controls.
|
||||
**Residual impact:** DNS and network policy can change after validation; production deployments should still use egress firewalling or a restricted outbound proxy.
|
||||
|
||||
**Recommendation:** Keep outbound firewalling or an allow-listed proxy in production, monitor DNS rebinding risk, and maintain response-size/time limits.
|
||||
|
||||
**Priority:** Completed in code; network-level egress controls remain.
|
||||
|
||||
### SA-005: Login endpoint lacks rate limiting and lockout
|
||||
|
||||
**Severity:** High, remediated in current worktree
|
||||
**Evidence before remediation:** `POST /api/auth/login` had no IP, email, or account rate limit, and OTP failures were not throttled separately.
|
||||
**Current state:** Login failures are tracked in SQLite by a SHA-256 key derived from client IP and normalized email. Five failures within 15 minutes cause a two-minute lockout; the endpoint returns `429` with `Retry-After`, and successful password plus OTP authentication clears the counter. Password-reset mail remains generic and should still be rate-limited operationally.
|
||||
|
||||
**Recommendation:** Use a distributed limiter for multi-instance deployments, add monitoring, and rate-limit password-reset issuance independently. Keep responses generic to avoid account enumeration.
|
||||
|
||||
**Priority:** Completed for the single-instance SQLite deployment; distributed limiting and reset-mail controls remain.
|
||||
|
||||
### SA-006: Firefox extension has broad host access and stores bearer tokens in local storage
|
||||
|
||||
**Severity:** High, remediated in current worktree
|
||||
**Evidence before remediation:** `webextension/manifest.json` declared `host_permissions: ["<all_urls>"]`; `webextension/options.js` and `webextension/popup.js` stored and retrieved `accessToken` through `browser.storage.local`.
|
||||
**Impact:** A compromised extension context or another extension with sufficient access may obtain the bearer token. The broad host permission increases the impact of an extension compromise and requires elevated user trust. The token grants access until expiry or revocation.
|
||||
|
||||
**Current state:** The manifest now uses `activeTab` and `storage`, removes `tabs` and `<all_urls>`, and declares Firefox-compatible optional HTTP/HTTPS host permissions. Login requests only the normalized configured backend origin. Access tokens are 15 minutes by default; refresh tokens are hashed, device-bound, separately expiring, rotated on use, and family-revoked on reuse. Extension credentials are stored in `browser.storage.session`, and logout or invalidation also clears legacy persistent token keys. Extension pages use a self-only script policy.
|
||||
|
||||
**Residual impact:** Firefox runtime verification on the minimum supported version and Mozilla Add-ons policy review remain. Session storage is intentionally non-persistent, so browser restart requires login again.
|
||||
|
||||
**Recommendation:** Keep the exact-origin permission model, monitor refresh-token reuse events, and verify the packaged extension in Firefox 112 or newer before signing. Do not add back broad host or persistent credential permissions.
|
||||
|
||||
**Priority:** Completed in code; runtime and release verification remain.
|
||||
|
||||
### SA-007: Production Compose configuration exposes the application directly
|
||||
|
||||
**Severity:** Medium/High, remediated in current worktree
|
||||
**Evidence before remediation:** `docker-compose.yml` published `${APP_PORT:-8000}:8000` while also configuring Traefik labels. The file uses an external `linklog_traefik` network and deployment-specific labels.
|
||||
**Impact:** The application can bypass the reverse proxy and any TLS, authentication middleware, rate limiting, or security headers configured there. The default `LINKLOG_PUBLIC_URL=localhost` is also unsuitable for a public deployment. A network or label mismatch can silently expose an unprotected direct endpoint or make operators disable controls to restore access.
|
||||
|
||||
**Current state:** The production `docker-compose.yml` no longer publishes port 8000 and attaches the app only to the external `linklog_traefik` network. Its public hostname fallback is `linklog.example.com`, and both Traefik routers use the same `LINKLOG_PUBLIC_URL`. Direct host access is available only through the explicitly named `docker-compose.local.yml` development file. `.env.example` and application settings use `linklog.example.com` as the documented default.
|
||||
|
||||
**Residual impact:** Operators must replace the documentation hostname, ensure the external network is the one used by Traefik, and validate the rendered Compose configuration and proxy middleware in their deployment. The local Compose file must not be exposed to the Internet.
|
||||
|
||||
**Recommendation:** Keep production and local Compose invocations separate, require a real DNS hostname and TLS in deployment checks, and add CI validation for the rendered production configuration and network labels.
|
||||
|
||||
**Priority:** Completed in code; deployment validation remains.
|
||||
|
||||
### SA-008: Initial setup and SMTP validation are unauthenticated by design
|
||||
### SA-004: Audit details are not sanitized at the audit-service boundary
|
||||
|
||||
**Severity:** Medium
|
||||
**Evidence:** `backend/app/api/setup.py` exposes configuration, test-mail, status, and completion routes without a bearer dependency while no administrator exists.
|
||||
**Impact:** This is necessary for first-run provisioning, but an exposed fresh instance allows anyone who can reach it to attempt setup, modify pending configuration, trigger test mail, and consume the five-send testing quota. The setup pending data includes a password hash and SMTP password in the database.
|
||||
**Evidence:** `record_audit_event()` serializes caller-supplied `details` directly to SQLite. Current callers generally avoid secrets, but the service does not enforce that contract or bound nested values and event size.
|
||||
|
||||
**Recommendation:** Restrict first-run setup at the network layer until an operator has completed provisioning, or require a one-time bootstrap secret supplied through the environment/console. Bind setup to localhost or a private management interface where possible. Add CSRF protection if setup ever uses cookies, strict request throttling, audit logging, and an explicit setup expiration/cleanup mechanism. Disable setup routes permanently once configuration completes.
|
||||
**Impact:** A future caller could persist passwords, tokens, OTP codes, SMTP credentials, sensitive URLs, or oversized data in the audit database. Audit records are durable and are not a suitable place for arbitrary request payloads.
|
||||
|
||||
**Priority:** Medium, high for exposed fresh deployments.
|
||||
|
||||
### SA-009: TOTP enrollment has no recovery codes or reset workflow
|
||||
|
||||
**Severity:** Medium, remediated in current worktree
|
||||
**Evidence before remediation:** `POST /api/user/otp/setup` returned the seed/provisioning URI and `POST /api/user/otp` required a valid current OTP code to disable OTP.
|
||||
**Impact:** A user who loses the authenticator device or seed could be locked out. Administrators had no documented recovery path that did not weaken authentication.
|
||||
|
||||
**Current state:** OTP enrollment generates ten random recovery codes and returns them only in the enrollment response. The database stores only SHA-256 hashes, and each code is atomically marked used. Normal OTP disablement requires the current password and a valid TOTP code; `/api/user/otp/recover` requires the current password and a valid unused recovery code, then disables OTP and clears the seed. Profile responses do not include the seed or recovery codes.
|
||||
|
||||
**Residual impact:** Recovery-code presentation is intentionally one-time; users who lose all codes can use the administrator-controlled OTP reset endpoint, which clears the seed and invalidates recovery codes. Recovery events should be added to the security audit log when SA-015 is addressed.
|
||||
|
||||
**Recommendation:** Keep recovery codes out of logs and API responses after enrollment, notify users when OTP is disabled or recovered, and add a controlled administrative recovery workflow with audit logging and notification.
|
||||
|
||||
**Priority:** Completed in code; operational recovery and audit logging remain.
|
||||
|
||||
### SA-010: Avatar validation trusts the client MIME type
|
||||
|
||||
**Severity:** Medium, remediated in current worktree
|
||||
**Evidence before remediation:** `upload_avatar()` in `backend/app/api/user_config.py` selected the extension from `UploadFile.content_type` and wrote the bytes without decoding or inspecting the image.
|
||||
**Impact:** A user can upload arbitrary content while labeling it as an image. Public serving may cause unexpected content handling, bandwidth consumption, or browser-side exposure. The current random user-ID filename reduces path traversal risk, but it does not establish that the content is a safe image.
|
||||
|
||||
**Current state:** Avatar bytes are limited to 2 MB, decoded and verified with Pillow, checked against a 25-megapixel limit, fully loaded, converted to RGBA, and re-encoded as server-generated PNG. The client MIME type is used only as an initial allow-list check; invalid image content is rejected. Static serving uses the generated `.png` extension and therefore returns `image/png`.
|
||||
|
||||
**Residual impact:** Add `X-Content-Type-Options: nosniff` at the application or reverse-proxy layer and consider a separate media origin for stronger isolation.
|
||||
|
||||
**Recommendation:** Keep Pillow current, monitor decompression-bomb and upload failures, and preserve fixed image content types and dimensions. Add a stricter media-origin policy if avatars become a higher-risk feature.
|
||||
|
||||
**Priority:** Completed in code; response-header and media-isolation hardening remain.
|
||||
|
||||
### SA-011: Error details can disclose infrastructure information
|
||||
|
||||
**Severity:** Medium
|
||||
**Evidence:** Admin SMTP routes return exception text in `503` responses; Mastodon errors include upstream response bodies; the frontend displays these values to the user.
|
||||
**Impact:** Connection errors can disclose hostnames, ports, TLS details, library messages, upstream response bodies, or internal service information. The behavior is useful for administrators but may expose more detail than intended if an admin session is compromised or error responses are logged.
|
||||
|
||||
**Recommendation:** Log full technical details server-side with correlation IDs. Return a stable user-facing message plus a short reference ID. Allow detailed diagnostics only behind an explicit protected troubleshooting mode. Redact credentials, authorization headers, URLs containing secrets, and SMTP/Mastodon response fields before logging or returning them.
|
||||
**Recommendation:** Use an allow-list of permitted detail fields per action, or recursively redact sensitive key names and URL query values. Bound string lengths and serialized event size. Add direct service tests with nested `password`, `token`, `secret`, and URL values and assert that they are redacted or rejected.
|
||||
|
||||
**Priority:** Medium.
|
||||
|
||||
### SA-012: Token lifecycle has refresh-token rotation and revocation controls
|
||||
|
||||
**Severity:** Medium, remediated in current worktree
|
||||
**Evidence before remediation:** `issue_token()` returned a `refresh_token` value, but only the access token was inserted into `tokens`; no refresh endpoint or refresh-token hash was implemented.
|
||||
**Impact:** Clients could assume the refresh token provided renewal or could store a value that could not be revoked or used. This complicated session reasoning and could lead to unsafe client fallbacks. Access tokens previously lived for the configured default of 30 days.
|
||||
|
||||
**Current state:** Login and `POST /api/auth/refresh` issue 15-minute access tokens and separately expiring 30-day refresh tokens. Only SHA-256 refresh-token hashes are stored. Each token family is bound to a device ID; a valid refresh token is rotated and the previous family state is revoked. Reuse of a revoked refresh token revokes the complete family. Logout revokes the token family, and password reset deletes all tokens for the user.
|
||||
|
||||
**Residual impact:** Existing sessions issued before deployment should be rotated, refresh-token reuse should be monitored, and clients must protect the device ID and session storage. Token-family cleanup and retention remain operational improvements.
|
||||
|
||||
**Recommendation:** Keep access tokens short-lived, require device binding on refresh, alert on refresh-token reuse, and retain family revocation on logout and password reset. Never place tokens in URLs or logs.
|
||||
|
||||
**Priority:** Completed in code; session rotation and monitoring remain.
|
||||
|
||||
### SA-013: No explicit security headers, CORS policy, or request-size policy
|
||||
### SA-005: Production secret fallback is not fail-closed
|
||||
|
||||
**Severity:** Medium
|
||||
**Evidence:** `backend/app/main.py` does not install security-header or CORS middleware, and the application routes do not define a global request-size limit.
|
||||
**Impact:** Deployment behavior depends entirely on the reverse proxy. Missing `Content-Security-Policy`, `Strict-Transport-Security`, `X-Content-Type-Options`, `Referrer-Policy`, and frame protections weakens browser-side defenses. An overly permissive future CORS configuration could expose bearer-authenticated APIs. Large request bodies may consume resources even where individual avatar limits exist.
|
||||
**Evidence:** `Settings.secret_key` defaults to `dev-secret-key-change-me`, and `LINKLOG_DATA_ENCRYPTION_KEY` is validated when encryption is used rather than fully validated during startup.
|
||||
|
||||
**Recommendation:** Add a documented restrictive security-header policy at the application or guaranteed proxy layer. Use `TrustedHostMiddleware` with an explicit production host list. Do not enable broad CORS; if cross-origin extension access requires it, allow only configured origins. Add global request and upload limits at the proxy and application layers.
|
||||
**Impact:** A deployment that omits required configuration can start with a known development secret or fail only when a protected feature is exercised. This creates dangerous configuration drift and complicates incident response.
|
||||
|
||||
**Recommendation:** During startup, reject a missing or known development `LINKLOG_SECRET_KEY` when `APP_ENV=production`; validate minimum length and entropy. Require and validate the encryption key before startup when encrypted data exists or production requires it. Add configuration tests for fail-closed production behavior.
|
||||
|
||||
**Priority:** Medium.
|
||||
|
||||
### SA-014: Development fallback secret is unsafe if the app is run without Compose configuration
|
||||
### SA-006: Login and reset-mail throttling are not distributed or atomic
|
||||
|
||||
**Severity:** Medium/High in multi-instance deployments
|
||||
**Evidence:** Login failure state is stored in SQLite and keyed by a client-IP/email hash. The check and increment occur as separate operations. Failed login handling can also issue a password-reset email for a known verified account without an independent reset-mail cooldown.
|
||||
|
||||
**Impact:** Concurrent attempts can overwrite counters, multiple application instances do not share reliable rate state, and reset-mail issuance can be abused to spam a user or consume SMTP resources.
|
||||
|
||||
**Recommendation:** Use an atomic shared limiter such as Redis for multi-instance deployments, with both account and IP buckets. Add an independent per-account/IP reset-mail cooldown and monitoring. Treat trusted proxy headers explicitly when deriving client IPs. Add concurrency, proxy, OTP-failure, and reset-mail abuse tests.
|
||||
|
||||
**Priority:** Medium/High for scaled or public deployments.
|
||||
|
||||
### SA-007: Security headers and global request policy are incomplete
|
||||
|
||||
**Severity:** Medium
|
||||
**Evidence:** `Settings.secret_key` in `backend/app/core/config.py` defaults to `dev-secret-key-change-me`.
|
||||
**Impact:** Local or incorrectly configured deployments can share a known secret. Even if the current token implementation does not use this value for signing, the setting creates a dangerous security assumption and may be used by future features.
|
||||
**Evidence:** The application does not consistently install or test CSP, HSTS, `X-Content-Type-Options`, frame protections, `Referrer-Policy`, trusted hosts, or a global request-size limit. The extension CSP does not cover the web application.
|
||||
|
||||
**Recommendation:** Fail closed when `APP_ENV=production` and the secret is absent or matches a known development value. Generate secrets during provisioning, validate minimum length and entropy, and never ship a production fallback. Make all cryptographic uses explicit and test them.
|
||||
**Impact:** Browser defense-in-depth and resource exhaustion protections depend on external proxy configuration. A proxy configuration mistake can leave HTML, API, or media responses weaker than intended.
|
||||
|
||||
**Recommendation:** Add a documented application or guaranteed-proxy policy and test headers on HTML, API, and media responses. Use `TrustedHostMiddleware` with explicit production hosts, `nosniff`, restrictive framing/referrer rules, HSTS only on HTTPS, and bounded request bodies.
|
||||
|
||||
**Priority:** Medium.
|
||||
|
||||
### SA-015: Some destructive and administrative operations lack audit logging
|
||||
### SA-008: Audit operations lack request correlation, retention, export, and alerting
|
||||
|
||||
**Severity:** Low/Medium, remediated in current worktree
|
||||
**Evidence before remediation:** User creation/deletion, privilege changes, SMTP changes, theme changes, OTP enrollment/disablement, link deletion, and Mastodon deletion did not create durable security audit events.
|
||||
**Impact:** Operators cannot reliably determine who changed privileges, modified delivery credentials, enrolled OTP, or deleted local/remote content. This limits incident response and accountability.
|
||||
**Severity:** Medium
|
||||
**Evidence:** Audit events contain actor/action/target/outcome/details/time but no request ID, source context, retention policy, protected export, or alerting pipeline.
|
||||
|
||||
**Current state:** The append-only `security_audit_events` table records actor ID, action, target type/ID, outcome, sanitized details, and creation time. Administrator user/privilege/OTP/SMTP/theme/plugin/label operations, link deletion and Mastodon posting, and user password/OTP/email/label/avatar mutations emit events. Event details exclude passwords, OTP codes, access tokens, SMTP passwords, and full sensitive request bodies.
|
||||
**Impact:** Operators can inspect database events but cannot reliably correlate them with request logs, detect attacks promptly, or guarantee retention and tamper-resistant access controls.
|
||||
|
||||
**Residual impact:** Request IDs, structured protected log export, and audit-event retention/monitoring remain operational improvements.
|
||||
**Recommendation:** Add request IDs at middleware entry, export redacted events to protected logs or a security monitoring system, define retention and access controls, and alert on privilege changes, OTP resets, password resets, credential changes, refresh-token reuse, and destructive actions.
|
||||
|
||||
**Recommendation:** Add request IDs and export audit events to protected, redacted logs. Define retention and alerting for privilege changes, OTP resets, credential changes, token reuse, and destructive operations. Keep audit events append-only and never store secrets.
|
||||
**Priority:** Medium.
|
||||
|
||||
**Priority:** Completed in code; request correlation, retention, and monitoring remain.
|
||||
### SA-009: Dependency, container, secret, and runtime security verification is incomplete
|
||||
|
||||
## Authentication and Authorization Review
|
||||
**Severity:** Medium
|
||||
**Evidence:** The repository runs functional tests and static syntax checks, but no dependency vulnerability scan, container scan, secret scan, authenticated dynamic test, or live Firefox extension workflow is part of the verified release path.
|
||||
|
||||
- **Authentication transport:** Bearer headers are used by most APIs, but query-string tokens remain a leakage risk. There is no cookie session, which reduces CSRF exposure for current bearer-only API calls. Credentials are now email-based; usernames remain presentation identities.
|
||||
- **Email authentication:** Primary and additional addresses are checked independently; additional addresses are unusable for login until their verification token is consumed. The profile exposes status but not verification secrets.
|
||||
- **Primary email selection:** Only an already verified alternative address can be promoted to primary. The same user row retains account permissions and active sessions, and the previous primary is retained as a verified alternative.
|
||||
- **Password policy:** New and reset passwords require at least eight characters. This is better than no policy but should be replaced with a longer passphrase-oriented policy and breached-password screening after a proper password hash migration.
|
||||
- **Email verification:** New administrator-created users cannot log in until verified. The setup-created first administrator is marked verified, which is appropriate for bootstrap but should be protected by the setup controls above.
|
||||
- **Password reset:** Tokens are random, hashed, expiring, single-use, and revoke all access and refresh token families after reset. Reset-email generation errors are intentionally swallowed to preserve generic login behavior, but this should be paired with server-side monitoring.
|
||||
- **OTP:** Login enforcement is present and OTP setup requires confirmation. Recovery codes, secret rotation, reauthentication, and encrypted secret storage are missing.
|
||||
- **Authorization:** Admin checks and link ownership checks are present. The last-administrator invariant is enforced for privilege changes and deletion. Add authorization tests for every new destructive endpoint as the API grows.
|
||||
**Impact:** Known vulnerable dependencies, image issues, accidental secret commits, proxy misconfiguration, and browser-runtime permission failures can reach release despite passing unit tests.
|
||||
|
||||
## Data Protection Review
|
||||
**Recommendation:** Add CI jobs for Python dependency and license policy, container scanning, secret scanning, Compose rendering, authenticated dynamic API checks, and a Firefox smoke test covering permission grant, login, refresh, logout, and active-tab capture.
|
||||
|
||||
- SQLite is the primary data store and contains profile data, links, password hashes, tokens, SMTP settings, Mastodon credentials, OAuth state, and OTP secrets. New sensitive values are encrypted with the externally supplied Fernet key; existing plaintext values must be rotated.
|
||||
- Database backups must be treated as credential-bearing secrets, encrypted, access-controlled, rotated, and tested for secure deletion.
|
||||
- Avatar files are persistent and publicly served. Validate and re-encode image content before accepting production uploads.
|
||||
- Link URLs and comments are intentionally public feed data. Operators should document that users must not submit secrets in URLs or comments.
|
||||
- SMTP and Mastodon integration errors should be redacted before entering logs or API responses.
|
||||
**Priority:** Medium before public release.
|
||||
|
||||
## Frontend and Extension Review
|
||||
## Residual Operational Requirements
|
||||
|
||||
- Feed content is generally assigned with `textContent`, which is a good XSS defense.
|
||||
- User-supplied profile values rendered by Jinja should remain autoescaped; do not mark them safe without a narrowly reviewed reason.
|
||||
- The web API currently uses bearer headers, so browser CSRF risk is lower than with cookie sessions. Keep it that way unless a CSRF token design is added.
|
||||
- Browser local storage is exposed to any script running in the same origin. Keep third-party scripts out of authenticated pages and add a restrictive CSP.
|
||||
- The extension's `<all_urls>` host permission should be reduced if the active-tab workflow is sufficient. Review Mozilla Add-ons policies before publishing signed releases.
|
||||
- The extension stores access and refresh credentials in `browser.storage.session`; it uses the refresh endpoint after access-token expiry and clears session and legacy token material on logout or invalidation.
|
||||
- Extension error messages should not include tokens or full sensitive URLs.
|
||||
|
||||
## Deployment Checklist
|
||||
|
||||
Before production exposure:
|
||||
|
||||
- [ ] Replace SHA-256 password hashing with Argon2id, scrypt, or bcrypt and migrate existing accounts.
|
||||
- [ ] Remove query-string token authentication and rotate existing access tokens.
|
||||
- [x] Encrypt newly written SMTP, Mastodon, OAuth, and OTP secrets at rest; protect the encryption key separately. Rotate legacy plaintext values.
|
||||
- [ ] Add login, OTP, reset-mail, and setup rate limiting.
|
||||
- [x] Validate Mastodon instances as HTTPS public hostnames, reject unsafe DNS/IP ranges, and block redirects. Keep network-level egress controls in production.
|
||||
- [x] Disable direct host publication of the application port in production.
|
||||
- [ ] Configure HTTPS, HSTS, CSP, Referrer-Policy, frame protections, `nosniff`, and trusted hosts.
|
||||
- [ ] Define a restrictive CORS policy or leave CORS disabled.
|
||||
- [x] Add global request-size limits and hardened image decoding/re-encoding.
|
||||
- [x] Add OTP recovery codes and a protected recovery workflow.
|
||||
- [x] Implement refresh-token behavior with hashing, device binding, rotation, reuse detection, and family revocation.
|
||||
- [x] Add security audit events and centralized redacted logging.
|
||||
- [ ] Rotate all credentials and set a unique high-entropy production secret.
|
||||
- [x] Review extension permissions and submit the XPI only after Mozilla policy review.
|
||||
- [ ] Encrypt and restrict database/avatar backups, and test restore and revocation procedures.
|
||||
- [ ] Run a dependency vulnerability scan and a dynamic penetration test against a production-like deployment.
|
||||
|
||||
## Suggested Priority Order
|
||||
|
||||
1. Password hashing migration.
|
||||
2. Query-token removal and token rotation.
|
||||
3. Secret-at-rest protection and backup controls.
|
||||
4. Login/setup/OTP rate limiting.
|
||||
5. Production network egress controls for Mastodon.
|
||||
6. Production network and TLS hardening.
|
||||
7. Extension permission and credential-storage review.
|
||||
8. Security headers, trusted hosts, request limits, media validation, recovery codes, and audit logging.
|
||||
- Replace documentation hostnames with real DNS names and enforce HTTPS/TLS.
|
||||
- Keep production Compose proxy-only and verify the actual Traefik network and middleware in deployment.
|
||||
- Rotate legacy plaintext secrets and previously issued sessions after upgrades.
|
||||
- Protect and encrypt database/avatar backups; test restoration and token/session revocation.
|
||||
- Monitor failed logins, reset-mail volume, refresh-token reuse, OTP recovery, privilege changes, and destructive actions.
|
||||
- Review the Firefox extension against Mozilla Add-ons policy before signing.
|
||||
|
||||
## Verification Performed
|
||||
|
||||
The repository's automated backend suite was run during this review and passed with 29 tests before the password-storage change; focused password/authentication tests subsequently passed after the remediation. Static Python and JavaScript checks were also used during feature development. These results demonstrate regression coverage, not absence of the findings above. A production release should additionally run a dependency scanner, container scanner, secret scanner, and authenticated dynamic security test.
|
||||
- `PYTHONPATH=. pytest -q`: **49 passed** at the start of this audit.
|
||||
- Static source review of backend APIs/services, frontend assets, extension manifest/scripts, Compose files, configuration, and tests.
|
||||
- Targeted searches for authentication, token, secret, upload, outbound-request, error, and audit-log paths.
|
||||
|
||||
Functional tests demonstrate regression coverage only; they do not certify production security.
|
||||
|
||||
Reference in New Issue
Block a user