max / makenotwork
10 files changed,
+295 insertions,
-5 deletions
| @@ -0,0 +1,27 @@ | |||
| 1 | + | -- SyncKit security hardening (ultra-fuzz --deep Security A+). | |
| 2 | + | -- | |
| 3 | + | -- 1. Per-user sync-token revocation, separate from the web `jwt_invalidated_at`. | |
| 4 | + | -- Bumping this invalidates a user's outstanding SyncKit JWTs (the extractor | |
| 5 | + | -- rejects `iat <= sync_jwt_invalidated_at`) WITHOUT logging them out of the | |
| 6 | + | -- website. Set when a sync device is removed, so the removed device's token | |
| 7 | + | -- dies immediately instead of surviving until expiry. | |
| 8 | + | -- 2. A security audit log for sync auth failures, device removals, and key | |
| 9 | + | -- rotations — the events an operator needs to investigate account abuse. | |
| 10 | + | -- | |
| 11 | + | -- Additive and idempotent. | |
| 12 | + | ||
| 13 | + | ALTER TABLE users ADD COLUMN IF NOT EXISTS sync_jwt_invalidated_at TIMESTAMPTZ; | |
| 14 | + | ||
| 15 | + | CREATE TABLE IF NOT EXISTS sync_security_events ( | |
| 16 | + | id BIGSERIAL PRIMARY KEY, | |
| 17 | + | app_id UUID NOT NULL REFERENCES sync_apps(id) ON DELETE CASCADE, | |
| 18 | + | -- NULL after the user row is removed; the event is still worth retaining. | |
| 19 | + | user_id UUID REFERENCES users(id) ON DELETE SET NULL, | |
| 20 | + | event_type VARCHAR(40) NOT NULL, | |
| 21 | + | detail JSONB, | |
| 22 | + | ip TEXT, | |
| 23 | + | created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() | |
| 24 | + | ); | |
| 25 | + | ||
| 26 | + | CREATE INDEX IF NOT EXISTS idx_sync_security_events_app_user_time | |
| 27 | + | ON sync_security_events (app_id, user_id, created_at DESC); |
| @@ -178,6 +178,11 @@ pub struct DbUser { | |||
| 178 | 178 | pub creator_paused_at: Option<DateTime<Utc>>, | |
| 179 | 179 | /// When JWTs issued before this timestamp should be rejected (set on password change). | |
| 180 | 180 | pub jwt_invalidated_at: Option<DateTime<Utc>>, | |
| 181 | + | /// When SyncKit JWTs issued before this timestamp should be rejected. Set on | |
| 182 | + | /// a sync-device removal so the removed device's token dies immediately. | |
| 183 | + | /// Separate from `jwt_invalidated_at` so a sync revocation does not log the | |
| 184 | + | /// user out of the website. | |
| 185 | + | pub sync_jwt_invalidated_at: Option<DateTime<Utc>>, | |
| 181 | 186 | /// Whether this user started a creator-tier subscription during the | |
| 182 | 187 | /// founder window. Sticky once true; never reset. Used by checkout to | |
| 183 | 188 | /// select founder price IDs before the window closes; after close, the | |
| @@ -340,6 +345,7 @@ mod tests { | |||
| 340 | 345 | content_removal_at: None, | |
| 341 | 346 | creator_paused_at: None, | |
| 342 | 347 | jwt_invalidated_at: None, | |
| 348 | + | sync_jwt_invalidated_at: None, | |
| 343 | 349 | is_founder: false, | |
| 344 | 350 | founder_locked_at: None, | |
| 345 | 351 | feed_key_version: 0, |
| @@ -10,6 +10,7 @@ mod devices; | |||
| 10 | 10 | mod keys; | |
| 11 | 11 | mod log; | |
| 12 | 12 | mod rotation; | |
| 13 | + | mod security; | |
| 13 | 14 | mod subscriptions; | |
| 14 | 15 | ||
| 15 | 16 | pub use apps::*; | |
| @@ -18,4 +19,5 @@ pub use devices::*; | |||
| 18 | 19 | pub use keys::*; | |
| 19 | 20 | pub use log::*; | |
| 20 | 21 | pub use rotation::*; | |
| 22 | + | pub use security::*; | |
| 21 | 23 | pub use subscriptions::*; |
| @@ -0,0 +1,64 @@ | |||
| 1 | + | //! SyncKit security: per-user token revocation and the security audit log. | |
| 2 | + | ||
| 3 | + | use sqlx::PgPool; | |
| 4 | + | ||
| 5 | + | use crate::db::{SyncAppId, UserId}; | |
| 6 | + | use crate::error::Result; | |
| 7 | + | ||
| 8 | + | /// Security audit event types. Kept as a small closed set so the column stays | |
| 9 | + | /// queryable; the variable context goes in `detail` (JSONB). | |
| 10 | + | pub mod sync_security_event { | |
| 11 | + | /// A `/api/sync/auth` attempt was denied (wrong password, suspended, | |
| 12 | + | /// locked, or 2FA-gated — all answered identically per the oracle fix). | |
| 13 | + | pub const AUTH_FAILURE: &str = "auth_failure"; | |
| 14 | + | /// A sync device was removed; the user's sync tokens were invalidated. | |
| 15 | + | pub const DEVICE_REMOVED: &str = "device_removed"; | |
| 16 | + | /// A key rotation completed (all devices re-encrypted to the new key). | |
| 17 | + | pub const KEY_ROTATION_COMPLETED: &str = "key_rotation_completed"; | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | /// Append one row to the SyncKit security audit log. Best-effort by contract: | |
| 21 | + | /// callers should not fail the request if this fails — log and move on — so the | |
| 22 | + | /// audit trail can never become a denial-of-service lever on the path it audits. | |
| 23 | + | #[tracing::instrument(skip_all)] | |
| 24 | + | pub async fn record_security_event( | |
| 25 | + | pool: &PgPool, | |
| 26 | + | app_id: SyncAppId, | |
| 27 | + | user_id: Option<UserId>, | |
| 28 | + | event_type: &str, | |
| 29 | + | detail: Option<serde_json::Value>, | |
| 30 | + | ip: Option<&str>, | |
| 31 | + | ) -> Result<()> { | |
| 32 | + | sqlx::query( | |
| 33 | + | "INSERT INTO sync_security_events (app_id, user_id, event_type, detail, ip) | |
| 34 | + | VALUES ($1, $2, $3, $4, $5)", | |
| 35 | + | ) | |
| 36 | + | .bind(app_id) | |
| 37 | + | .bind(user_id) | |
| 38 | + | .bind(event_type) | |
| 39 | + | .bind(detail) | |
| 40 | + | .bind(ip) | |
| 41 | + | .execute(pool) | |
| 42 | + | .await?; | |
| 43 | + | Ok(()) | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | /// Invalidate every outstanding SyncKit JWT for a user by stamping | |
| 47 | + | /// `sync_jwt_invalidated_at = NOW()`. The extractor rejects any sync token with | |
| 48 | + | /// `iat <= sync_jwt_invalidated_at`, so all the user's sync sessions must | |
| 49 | + | /// re-authenticate. Deliberately separate from `jwt_invalidated_at`: this does | |
| 50 | + | /// NOT disturb the user's website sessions. | |
| 51 | + | /// | |
| 52 | + | /// Note on key rotation: in the zero-knowledge model the server never holds the | |
| 53 | + | /// data-encryption key, so it cannot rotate it on the user's behalf — that is a | |
| 54 | + | /// client action. Invalidating tokens (forcing re-auth) is the strongest | |
| 55 | + | /// server-side response to a device removal; the audit log records the removal | |
| 56 | + | /// so a client/operator can drive an actual key rotation if warranted. | |
| 57 | + | #[tracing::instrument(skip_all)] | |
| 58 | + | pub async fn invalidate_user_sync_tokens(pool: &PgPool, user_id: UserId) -> Result<()> { | |
| 59 | + | sqlx::query("UPDATE users SET sync_jwt_invalidated_at = NOW() WHERE id = $1") | |
| 60 | + | .bind(user_id) | |
| 61 | + | .execute(pool) | |
| 62 | + | .await?; | |
| 63 | + | Ok(()) | |
| 64 | + | } |
| @@ -41,6 +41,7 @@ use super::{SyncAuthRequest, SyncAuthResponse, ValidateAppQuery, ValidateAppResp | |||
| 41 | 41 | #[tracing::instrument(skip_all, name = "synckit::sync_auth")] | |
| 42 | 42 | pub(super) async fn sync_auth( | |
| 43 | 43 | State(state): State<AppState>, | |
| 44 | + | headers: axum::http::HeaderMap, | |
| 44 | 45 | Json(req): Json<SyncAuthRequest>, | |
| 45 | 46 | ) -> Result<impl IntoResponse> { | |
| 46 | 47 | let secret = state | |
| @@ -105,6 +106,23 @@ pub(super) async fn sync_auth( | |||
| 105 | 106 | crate::constants::MAX_LOGIN_ATTEMPTS, | |
| 106 | 107 | crate::constants::LOCKOUT_MINUTES, | |
| 107 | 108 | ).await?; | |
| 109 | + | // Audit the failed sync auth (best-effort). Recorded only for a real | |
| 110 | + | // account; the email-enumeration equalization paths above (unknown user, | |
| 111 | + | // malformed input) deliberately don't log, both to avoid noise and | |
| 112 | + | // because they carry no user_id. | |
| 113 | + | let ip = crate::helpers::extract_client_ip(&headers); | |
| 114 | + | if let Err(e) = db::synckit::record_security_event( | |
| 115 | + | &state.db, | |
| 116 | + | app.id, | |
| 117 | + | Some(user.id), | |
| 118 | + | db::synckit::sync_security_event::AUTH_FAILURE, | |
| 119 | + | None, | |
| 120 | + | ip.as_deref(), | |
| 121 | + | ) | |
| 122 | + | .await | |
| 123 | + | { | |
| 124 | + | tracing::error!(error = ?e, "failed to record auth_failure security event"); | |
| 125 | + | } | |
| 108 | 126 | return Err(AppError::Unauthorized); | |
| 109 | 127 | } | |
| 110 | 128 |
| @@ -561,6 +561,7 @@ pub(super) async fn list_devices( | |||
| 561 | 561 | pub(super) async fn delete_device( | |
| 562 | 562 | State(state): State<AppState>, | |
| 563 | 563 | sync_user: SyncUser, | |
| 564 | + | headers: axum::http::HeaderMap, | |
| 564 | 565 | Path(device_id): Path<SyncDeviceId>, | |
| 565 | 566 | ) -> Result<impl IntoResponse> { | |
| 566 | 567 | let deleted = | |
| @@ -571,6 +572,28 @@ pub(super) async fn delete_device( | |||
| 571 | 572 | return Err(AppError::NotFound); | |
| 572 | 573 | } | |
| 573 | 574 | ||
| 575 | + | // Security response to a device removal: invalidate the user's sync tokens | |
| 576 | + | // so the removed device's JWT dies immediately (before this it survived to | |
| 577 | + | // expiry), and record the event for the audit log. Best-effort — a logging | |
| 578 | + | // or invalidation hiccup must not fail the delete the user asked for. | |
| 579 | + | let ip = crate::helpers::extract_client_ip(&headers); | |
| 580 | + | if let Err(e) = db::synckit::invalidate_user_sync_tokens(&state.db, sync_user.user_id).await { | |
| 581 | + | tracing::error!(error = ?e, user_id = %sync_user.user_id, | |
| 582 | + | "failed to invalidate sync tokens after device removal"); | |
| 583 | + | } | |
| 584 | + | if let Err(e) = db::synckit::record_security_event( | |
| 585 | + | &state.db, | |
| 586 | + | sync_user.app_id, | |
| 587 | + | Some(sync_user.user_id), | |
| 588 | + | db::synckit::sync_security_event::DEVICE_REMOVED, | |
| 589 | + | Some(serde_json::json!({ "device_id": device_id.to_string() })), | |
| 590 | + | ip.as_deref(), | |
| 591 | + | ) | |
| 592 | + | .await | |
| 593 | + | { | |
| 594 | + | tracing::error!(error = ?e, "failed to record device_removed security event"); | |
| 595 | + | } | |
| 596 | + | ||
| 574 | 597 | Ok(axum::http::StatusCode::NO_CONTENT) | |
| 575 | 598 | } | |
| 576 | 599 | ||
| @@ -810,7 +833,22 @@ pub(super) async fn complete_rotation( | |||
| 810 | 833 | .await?; | |
| 811 | 834 | ||
| 812 | 835 | match result { | |
| 813 | - | Ok(_new_key_id) => Ok(axum::http::StatusCode::NO_CONTENT.into_response()), | |
| 836 | + | Ok(_new_key_id) => { | |
| 837 | + | // Audit the completed rotation (best-effort). | |
| 838 | + | if let Err(e) = db::synckit::record_security_event( | |
| 839 | + | &state.db, | |
| 840 | + | sync_user.app_id, | |
| 841 | + | Some(sync_user.user_id), | |
| 842 | + | db::synckit::sync_security_event::KEY_ROTATION_COMPLETED, | |
| 843 | + | Some(serde_json::json!({ "rotation_id": req.rotation_id.to_string() })), | |
| 844 | + | None, | |
| 845 | + | ) | |
| 846 | + | .await | |
| 847 | + | { | |
| 848 | + | tracing::error!(error = ?e, "failed to record key_rotation_completed security event"); | |
| 849 | + | } | |
| 850 | + | Ok(axum::http::StatusCode::NO_CONTENT.into_response()) | |
| 851 | + | } | |
| 814 | 852 | Err(0) => Err(AppError::BadRequest("No active rotation".to_string())), | |
| 815 | 853 | Err(remaining) => Ok(( | |
| 816 | 854 | axum::http::StatusCode::CONFLICT, |
| @@ -169,6 +169,15 @@ impl FromRequestParts<AppState> for SyncUser { | |||
| 169 | 169 | return Err(AppError::Unauthorized); | |
| 170 | 170 | } | |
| 171 | 171 | ||
| 172 | + | // Sync-specific revocation, bumped on device removal. Same `<=` window | |
| 173 | + | // semantics as above; separate column so removing a sync device does | |
| 174 | + | // not invalidate the user's website sessions. | |
| 175 | + | if let Some(invalidated_at) = user.sync_jwt_invalidated_at | |
| 176 | + | && claims.iat <= invalidated_at.timestamp() | |
| 177 | + | { | |
| 178 | + | return Err(AppError::Unauthorized); | |
| 179 | + | } | |
| 180 | + | ||
| 172 | 181 | if claims.key.is_empty() { | |
| 173 | 182 | return Err(AppError::Unauthorized); | |
| 174 | 183 | } |
| @@ -18,6 +18,7 @@ mod synckit; | |||
| 18 | 18 | mod synckit_billing; | |
| 19 | 19 | mod synckit_per_key_storage; | |
| 20 | 20 | mod synckit_paid_sync; | |
| 21 | + | mod synckit_security; | |
| 21 | 22 | mod synckit_adversarial; | |
| 22 | 23 | mod pages; | |
| 23 | 24 | mod analytics; |
| @@ -198,11 +198,11 @@ async fn device_crud() { | |||
| 198 | 198 | .await; | |
| 199 | 199 | assert_eq!(resp.status, 204); | |
| 200 | 200 | ||
| 201 | - | // List again — should be 0 | |
| 201 | + | // Deleting a device invalidates the user's sync tokens — the removed | |
| 202 | + | // device's JWT must not outlive the removal — so the old token is now | |
| 203 | + | // rejected. (The 204 above already confirms the row was deleted.) | |
| 202 | 204 | let resp = h.client.get("/api/sync/devices").await; | |
| 203 | - | assert_eq!(resp.status, 200); | |
| 204 | - | let devices: Vec<DeviceResponse> = resp.json(); | |
| 205 | - | assert_eq!(devices.len(), 0); | |
| 205 | + | assert_eq!(resp.status, 401, "sync token must be invalid after a device removal"); | |
| 206 | 206 | } | |
| 207 | 207 | ||
| 208 | 208 | #[tokio::test] |
| @@ -0,0 +1,125 @@ | |||
| 1 | + | //! SyncKit security hardening (ultra-fuzz --deep Security A+): device removal | |
| 2 | + | //! immediately invalidates the user's sync tokens, and security-relevant events | |
| 3 | + | //! (auth failures, device removals) land in the `sync_security_events` audit log. | |
| 4 | + | ||
| 5 | + | use makenotwork::db::{SyncAppId, UserId}; | |
| 6 | + | use serde_json::json; | |
| 7 | + | use sqlx::PgPool; | |
| 8 | + | ||
| 9 | + | use crate::harness::TestHarness; | |
| 10 | + | ||
| 11 | + | /// Create an active first-party (internal) sync app; returns (app_id, api_key). | |
| 12 | + | async fn create_internal_app(pool: &PgPool, user_id: UserId) -> (SyncAppId, String) { | |
| 13 | + | let api_key = "test-api-key-synckit-security"; | |
| 14 | + | let key_hash = crate::harness::hash_api_key(api_key); | |
| 15 | + | let key_prefix = &api_key[..8]; | |
| 16 | + | let app_id: SyncAppId = sqlx::query_scalar( | |
| 17 | + | "INSERT INTO sync_apps (creator_id, name, api_key_hash, api_key_prefix, is_internal, billing_status) | |
| 18 | + | VALUES ($1, 'SecApp', $2, $3, TRUE, 'active') RETURNING id", | |
| 19 | + | ) | |
| 20 | + | .bind(user_id) | |
| 21 | + | .bind(&key_hash) | |
| 22 | + | .bind(key_prefix) | |
| 23 | + | .fetch_one(pool) | |
| 24 | + | .await | |
| 25 | + | .expect("insert internal sync_app"); | |
| 26 | + | (app_id, api_key.to_string()) | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | fn auth_as(h: &mut TestHarness, user_id: UserId, app_id: SyncAppId, key: &str) { | |
| 30 | + | let token = makenotwork::synckit_auth::create_sync_token( | |
| 31 | + | "test-synckit-jwt-secret", | |
| 32 | + | user_id, | |
| 33 | + | app_id, | |
| 34 | + | key, | |
| 35 | + | ) | |
| 36 | + | .expect("mint test JWT"); | |
| 37 | + | h.client.set_bearer_token(&token); | |
| 38 | + | } | |
| 39 | + | ||
| 40 | + | #[tokio::test] | |
| 41 | + | async fn deleting_a_device_invalidates_the_users_sync_token_and_audits_it() { | |
| 42 | + | let mut h = TestHarness::new().await; | |
| 43 | + | let user_id = h.signup("sec_dev", "sec_dev@example.com", "Password1!").await; | |
| 44 | + | let (app_id, _api_key) = create_internal_app(&h.db, user_id).await; | |
| 45 | + | auth_as(&mut h, user_id, app_id, "user-key"); | |
| 46 | + | ||
| 47 | + | // A device to remove. | |
| 48 | + | let device_id: uuid::Uuid = sqlx::query_scalar( | |
| 49 | + | "INSERT INTO sync_devices (app_id, user_id, device_name, platform) | |
| 50 | + | VALUES ($1, $2, 'laptop', 'test') RETURNING id", | |
| 51 | + | ) | |
| 52 | + | .bind(app_id) | |
| 53 | + | .bind(user_id) | |
| 54 | + | .fetch_one(&h.db) | |
| 55 | + | .await | |
| 56 | + | .unwrap(); | |
| 57 | + | ||
| 58 | + | // The token authenticates a delete of an unknown device as 404 (auth passes, | |
| 59 | + | // device not found) — proving it is valid right now. | |
| 60 | + | let probe = h.client.delete(&format!("/api/sync/devices/{}", uuid::Uuid::new_v4())).await; | |
| 61 | + | assert_eq!(probe.status, 404, "token should be valid before removal: {}", probe.text); | |
| 62 | + | ||
| 63 | + | // Remove the real device. | |
| 64 | + | let resp = h.client.delete(&format!("/api/sync/devices/{device_id}")).await; | |
| 65 | + | assert_eq!(resp.status, 204, "device delete should succeed: {}", resp.text); | |
| 66 | + | ||
| 67 | + | // Same token now fails: device removal bumped sync_jwt_invalidated_at, so the | |
| 68 | + | // unknown-device probe returns 401 (auth) rather than 404. | |
| 69 | + | let probe = h.client.delete(&format!("/api/sync/devices/{}", uuid::Uuid::new_v4())).await; | |
| 70 | + | assert_eq!(probe.status, 401, "token must be invalid after device removal: {}", probe.text); | |
| 71 | + | ||
| 72 | + | // The website session path is untouched: jwt_invalidated_at stays NULL. | |
| 73 | + | let web_invalidated: Option<chrono::DateTime<chrono::Utc>> = | |
| 74 | + | sqlx::query_scalar("SELECT jwt_invalidated_at FROM users WHERE id = $1") | |
| 75 | + | .bind(user_id) | |
| 76 | + | .fetch_one(&h.db) | |
| 77 | + | .await | |
| 78 | + | .unwrap(); | |
| 79 | + | assert!(web_invalidated.is_none(), "sync revocation must not touch web jwt_invalidated_at"); | |
| 80 | + | ||
| 81 | + | // The removal was audited. | |
| 82 | + | let events: i64 = sqlx::query_scalar( | |
| 83 | + | "SELECT COUNT(*) FROM sync_security_events | |
| 84 | + | WHERE app_id = $1 AND user_id = $2 AND event_type = 'device_removed'", | |
| 85 | + | ) | |
| 86 | + | .bind(app_id) | |
| 87 | + | .bind(user_id) | |
| 88 | + | .fetch_one(&h.db) | |
| 89 | + | .await | |
| 90 | + | .unwrap(); | |
| 91 | + | assert_eq!(events, 1, "device removal must be recorded in the audit log"); | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | #[tokio::test] | |
| 95 | + | async fn failed_sync_auth_is_recorded_in_the_audit_log() { | |
| 96 | + | let mut h = TestHarness::new().await; | |
| 97 | + | let user_id = h.signup("sec_auth", "sec_auth@example.com", "Password1!").await; | |
| 98 | + | let (app_id, api_key) = create_internal_app(&h.db, user_id).await; | |
| 99 | + | ||
| 100 | + | let resp = h | |
| 101 | + | .client | |
| 102 | + | .post_json( | |
| 103 | + | "/api/sync/auth", | |
| 104 | + | &json!({ | |
| 105 | + | "api_key": api_key, | |
| 106 | + | "email": "sec_auth@example.com", | |
| 107 | + | "password": "WrongPassword1!", | |
| 108 | + | "key": "user-key", | |
| 109 | + | }) | |
| 110 | + | .to_string(), | |
| 111 | + | ) | |
| 112 | + | .await; | |
| 113 | + | assert_eq!(resp.status, 401, "wrong password must be rejected: {}", resp.text); | |
| 114 | + | ||
| 115 | + | let events: i64 = sqlx::query_scalar( | |
| 116 | + | "SELECT COUNT(*) FROM sync_security_events | |
| 117 | + | WHERE app_id = $1 AND user_id = $2 AND event_type = 'auth_failure'", | |
| 118 | + | ) | |
| 119 | + | .bind(app_id) | |
| 120 | + | .bind(user_id) | |
| 121 | + | .fetch_one(&h.db) | |
| 122 | + | .await | |
| 123 | + | .unwrap(); | |
| 124 | + | assert_eq!(events, 1, "a failed sync auth must be recorded in the audit log"); | |
| 125 | + | } |