max / makenotwork
36 files changed,
+995 insertions,
-209 deletions
| @@ -18,6 +18,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" | |||
| 18 | 18 | checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" | |
| 19 | 19 | ||
| 20 | 20 | [[package]] | |
| 21 | + | name = "aead" | |
| 22 | + | version = "0.5.2" | |
| 23 | + | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 24 | + | checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" | |
| 25 | + | dependencies = [ | |
| 26 | + | "crypto-common 0.1.6", | |
| 27 | + | "generic-array", | |
| 28 | + | ] | |
| 29 | + | ||
| 30 | + | [[package]] | |
| 21 | 31 | name = "aes" | |
| 22 | 32 | version = "0.8.4" | |
| 23 | 33 | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| @@ -1560,6 +1570,30 @@ source = "registry+https://github.com/rust-lang/crates.io-index" | |||
| 1560 | 1570 | checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" | |
| 1561 | 1571 | ||
| 1562 | 1572 | [[package]] | |
| 1573 | + | name = "chacha20" | |
| 1574 | + | version = "0.9.1" | |
| 1575 | + | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 1576 | + | checksum = "c3613f74bd2eac03dad61bd53dbe620703d4371614fe0bc3b9f04dd36fe4e818" | |
| 1577 | + | dependencies = [ | |
| 1578 | + | "cfg-if", | |
| 1579 | + | "cipher", | |
| 1580 | + | "cpufeatures 0.2.17", | |
| 1581 | + | ] | |
| 1582 | + | ||
| 1583 | + | [[package]] | |
| 1584 | + | name = "chacha20poly1305" | |
| 1585 | + | version = "0.10.1" | |
| 1586 | + | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 1587 | + | checksum = "10cd79432192d1c0f4e1a0fef9527696cc039165d729fb41b3f4f4f354c2dc35" | |
| 1588 | + | dependencies = [ | |
| 1589 | + | "aead", | |
| 1590 | + | "chacha20", | |
| 1591 | + | "cipher", | |
| 1592 | + | "poly1305", | |
| 1593 | + | "zeroize", | |
| 1594 | + | ] | |
| 1595 | + | ||
| 1596 | + | [[package]] | |
| 1563 | 1597 | name = "chrono" | |
| 1564 | 1598 | version = "0.4.44" | |
| 1565 | 1599 | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| @@ -1581,6 +1615,7 @@ checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" | |||
| 1581 | 1615 | dependencies = [ | |
| 1582 | 1616 | "crypto-common 0.1.6", | |
| 1583 | 1617 | "inout", | |
| 1618 | + | "zeroize", | |
| 1584 | 1619 | ] | |
| 1585 | 1620 | ||
| 1586 | 1621 | [[package]] | |
| @@ -2069,6 +2104,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" | |||
| 2069 | 2104 | checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" | |
| 2070 | 2105 | dependencies = [ | |
| 2071 | 2106 | "generic-array", | |
| 2107 | + | "rand_core 0.6.4", | |
| 2072 | 2108 | "typenum", | |
| 2073 | 2109 | ] | |
| 2074 | 2110 | ||
| @@ -4161,6 +4197,7 @@ dependencies = [ | |||
| 4161 | 4197 | "axum-extra", | |
| 4162 | 4198 | "base64 0.22.1", | |
| 4163 | 4199 | "bzip2 0.4.4", | |
| 4200 | + | "chacha20poly1305", | |
| 4164 | 4201 | "chrono", | |
| 4165 | 4202 | "clap", | |
| 4166 | 4203 | "const-oid 0.9.6", | |
| @@ -4679,6 +4716,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" | |||
| 4679 | 4716 | checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" | |
| 4680 | 4717 | ||
| 4681 | 4718 | [[package]] | |
| 4719 | + | name = "opaque-debug" | |
| 4720 | + | version = "0.3.1" | |
| 4721 | + | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 4722 | + | checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" | |
| 4723 | + | ||
| 4724 | + | [[package]] | |
| 4682 | 4725 | name = "openssl" | |
| 4683 | 4726 | version = "0.10.76" | |
| 4684 | 4727 | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| @@ -5066,6 +5109,17 @@ dependencies = [ | |||
| 5066 | 5109 | ] | |
| 5067 | 5110 | ||
| 5068 | 5111 | [[package]] | |
| 5112 | + | name = "poly1305" | |
| 5113 | + | version = "0.8.0" | |
| 5114 | + | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 5115 | + | checksum = "8159bd90725d2df49889a078b54f4f79e87f1f8a8444194cdca81d38f5393abf" | |
| 5116 | + | dependencies = [ | |
| 5117 | + | "cpufeatures 0.2.17", | |
| 5118 | + | "opaque-debug", | |
| 5119 | + | "universal-hash", | |
| 5120 | + | ] | |
| 5121 | + | ||
| 5122 | + | [[package]] | |
| 5069 | 5123 | name = "pom-contract" | |
| 5070 | 5124 | version = "0.1.0" | |
| 5071 | 5125 | dependencies = [ | |
| @@ -7608,6 +7662,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" | |||
| 7608 | 7662 | checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" | |
| 7609 | 7663 | ||
| 7610 | 7664 | [[package]] | |
| 7665 | + | name = "universal-hash" | |
| 7666 | + | version = "0.5.1" | |
| 7667 | + | source = "registry+https://github.com/rust-lang/crates.io-index" | |
| 7668 | + | checksum = "fc1de2c688dc15305988b563c3854064043356019f97a4b46276fe734c4f07ea" | |
| 7669 | + | dependencies = [ | |
| 7670 | + | "crypto-common 0.1.6", | |
| 7671 | + | "subtle", | |
| 7672 | + | ] | |
| 7673 | + | ||
| 7674 | + | [[package]] | |
| 7611 | 7675 | name = "unsafe-libyaml" | |
| 7612 | 7676 | version = "0.2.11" | |
| 7613 | 7677 | source = "registry+https://github.com/rust-lang/crates.io-index" |
| @@ -69,6 +69,7 @@ hmac = "0.12.1" | |||
| 69 | 69 | sha1 = "0.10.6" | |
| 70 | 70 | sha2 = "0.10.9" | |
| 71 | 71 | subtle = "2.6" | |
| 72 | + | chacha20poly1305 = "0.10.1" | |
| 72 | 73 | hex = "0.4.3" | |
| 73 | 74 | base64 = "0.22.1" | |
| 74 | 75 |
| @@ -0,0 +1,11 @@ | |||
| 1 | + | -- Make pending_uploads uniqueness (and its ON CONFLICT target) bucket-aware. | |
| 2 | + | -- | |
| 3 | + | -- The deletion queue (pending_s3_deletions) and is_s3_key_live are already | |
| 4 | + | -- (s3_key, bucket)-scoped; pending_uploads lagged on s3_key alone. Today every | |
| 5 | + | -- pending row is bucket='main', so this is latent, but a key legitimately | |
| 6 | + | -- present in two buckets (main + a future synckit/OTA presign) would collide on | |
| 7 | + | -- INSERT and the ON CONFLICT (s3_key) upsert could refresh or suppress the | |
| 8 | + | -- wrong bucket's row. Align the unique key with the rest of the storage layer. | |
| 9 | + | DROP INDEX IF EXISTS idx_pending_uploads_s3_key; | |
| 10 | + | CREATE UNIQUE INDEX IF NOT EXISTS idx_pending_uploads_s3_key_bucket | |
| 11 | + | ON pending_uploads(s3_key, bucket); |
| @@ -217,8 +217,20 @@ impl Config { | |||
| 217 | 217 | .ok() | |
| 218 | 218 | .and_then(|s| s.parse::<UserId>().ok()); | |
| 219 | 219 | ||
| 220 | - | // SyncKit JWT secret - optional, sync endpoints return 503 if unset | |
| 221 | - | let synckit_jwt_secret = std::env::var("SYNCKIT_JWT_SECRET").ok(); | |
| 220 | + | // SyncKit JWT secret - optional, sync endpoints return 503 if unset. | |
| 221 | + | // When set it IS the HS256 symmetric signing key for SyncKit/OAuth | |
| 222 | + | // bearer tokens, so enforce the same >=32-char floor as SIGNING_SECRET: | |
| 223 | + | // a short value is offline-brute-forceable into token forgery. Fail | |
| 224 | + | // closed (refuse to start) rather than silently accepting a weak key. | |
| 225 | + | let synckit_jwt_secret = match std::env::var("SYNCKIT_JWT_SECRET") { | |
| 226 | + | Ok(secret) => { | |
| 227 | + | if secret.len() < 32 { | |
| 228 | + | return Err(ConfigError::WeakSynckitJwtSecret); | |
| 229 | + | } | |
| 230 | + | Some(secret) | |
| 231 | + | } | |
| 232 | + | Err(_) => None, | |
| 233 | + | }; | |
| 222 | 234 | ||
| 223 | 235 | // File scanning - enabled by default, set SCAN_ENABLED=false to disable | |
| 224 | 236 | let scan = ScanConfig::from_env(); | |
| @@ -592,6 +604,8 @@ pub enum ConfigError { | |||
| 592 | 604 | MissingSigningSecret, | |
| 593 | 605 | #[error("SIGNING_SECRET must be at least 32 characters long")] | |
| 594 | 606 | WeakSigningSecret, | |
| 607 | + | #[error("SYNCKIT_JWT_SECRET must be at least 32 characters long")] | |
| 608 | + | WeakSynckitJwtSecret, | |
| 595 | 609 | } | |
| 596 | 610 | ||
| 597 | 611 | #[cfg(test)] | |
| @@ -793,6 +807,44 @@ mod tests { | |||
| 793 | 807 | } | |
| 794 | 808 | ||
| 795 | 809 | #[test] | |
| 810 | + | fn from_env_fails_with_short_synckit_jwt_secret() { | |
| 811 | + | let guard = EnvGuard::new(); | |
| 812 | + | guard.clear_all(); | |
| 813 | + | ||
| 814 | + | // SAFETY: test-only, serialized by EnvGuard mutex | |
| 815 | + | unsafe { | |
| 816 | + | std::env::set_var("DATABASE_URL", "postgres://localhost/test_db"); | |
| 817 | + | std::env::set_var("SIGNING_SECRET", "x".repeat(32)); | |
| 818 | + | // 31 chars — one under the floor. | |
| 819 | + | std::env::set_var("SYNCKIT_JWT_SECRET", "x".repeat(31)); | |
| 820 | + | } | |
| 821 | + | ||
| 822 | + | let err = Config::from_env().unwrap_err(); | |
| 823 | + | assert!( | |
| 824 | + | matches!(err, ConfigError::WeakSynckitJwtSecret), | |
| 825 | + | "expected WeakSynckitJwtSecret, got: {err}" | |
| 826 | + | ); | |
| 827 | + | drop(guard); | |
| 828 | + | } | |
| 829 | + | ||
| 830 | + | #[test] | |
| 831 | + | fn from_env_accepts_strong_synckit_jwt_secret() { | |
| 832 | + | let guard = EnvGuard::new(); | |
| 833 | + | guard.clear_all(); | |
| 834 | + | ||
| 835 | + | // SAFETY: test-only, serialized by EnvGuard mutex | |
| 836 | + | unsafe { | |
| 837 | + | std::env::set_var("DATABASE_URL", "postgres://localhost/test_db"); | |
| 838 | + | std::env::set_var("SIGNING_SECRET", "x".repeat(32)); | |
| 839 | + | std::env::set_var("SYNCKIT_JWT_SECRET", "y".repeat(32)); | |
| 840 | + | } | |
| 841 | + | ||
| 842 | + | let config = Config::from_env().expect("32-char JWT secret should be accepted"); | |
| 843 | + | assert_eq!(config.synckit_jwt_secret.as_deref(), Some("y".repeat(32).as_str())); | |
| 844 | + | drop(guard); | |
| 845 | + | } | |
| 846 | + | ||
| 847 | + | #[test] | |
| 796 | 848 | fn from_env_uses_random_dev_secret_when_not_production() { | |
| 797 | 849 | let guard = EnvGuard::new(); | |
| 798 | 850 | guard.clear_all(); |
| @@ -171,7 +171,14 @@ pub const BUYER_DEPARTURE_MAX_NOTIFICATIONS: i64 = 50_000; | |||
| 171 | 171 | ||
| 172 | 172 | // -- File scanning -- | |
| 173 | 173 | pub const SCAN_MAX_MEMORY_BYTES: usize = 100 * 1024 * 1024; // 100 MB in-memory threshold | |
| 174 | - | pub const SCAN_MAX_CONCURRENT: usize = 4; // Max concurrent file scans (each can use up to 100 MB RAM) | |
| 174 | + | // Ceiling on in-flight scans, enforced by a semaphore around the CPU/clamd | |
| 175 | + | // phase. NOTE: with `SCAN_WORKER_COUNT` workers each scanning one file at a | |
| 176 | + | // time, the real concurrency is `min(SCAN_MAX_CONCURRENT, SCAN_WORKER_COUNT)` — | |
| 177 | + | // today that's 2 (~200 MB peak), so this semaphore only begins to bind if the | |
| 178 | + | // worker count is raised above it. Kept as an explicit ceiling so that raising | |
| 179 | + | // `SCAN_WORKER_COUNT` can't silently blow past the memory budget. The | |
| 180 | + | // assertion below documents that intent. | |
| 181 | + | pub const SCAN_MAX_CONCURRENT: usize = 4; // Memory-budget ceiling on concurrent scans | |
| 175 | 182 | pub const SCAN_WORKER_COUNT: usize = 2; // Background worker tasks draining scan_jobs queue | |
| 176 | 183 | /// Retention window for terminal-state (`done`, `failed`) `scan_jobs` rows. | |
| 177 | 184 | /// Queued/running rows are operational queue state and not affected. | |
| @@ -204,8 +211,18 @@ pub const CADDY_ASK_MAX_CONCURRENT: usize = 8; | |||
| 204 | 211 | pub const SCAN_ZIP_MAX_RATIO: f64 = 100.0; // Max compression ratio before ZIP bomb | |
| 205 | 212 | pub const SCAN_ZIP_MAX_DEPTH: u32 = 2; // Max nested archives (detection is 1 level deep; decompressed size limit is the primary defense) | |
| 206 | 213 | pub const SCAN_ZIP_MAX_UNCOMPRESSED: u64 = 2 * 1024 * 1024 * 1024; // 2 GB uncompressed limit | |
| 214 | + | // Cap the number of ZIP entries inspected. Depth/ratio/uncompressed-size are | |
| 215 | + | // already bounded, but a ZIP with millions of tiny entries forces a full | |
| 216 | + | // per-entry decompression pass bounded only by the 2 GB total. 100k entries is | |
| 217 | + | // far past any legitimate sample pack / content bundle; beyond it, fail closed. | |
| 218 | + | pub const SCAN_ZIP_MAX_ENTRIES: usize = 100_000; | |
| 207 | 219 | pub const SCAN_MALWAREBAZAAR_TIMEOUT_SECS: u64 = 5; | |
| 208 | 220 | pub const SCAN_CLAMAV_TIMEOUT_SECS: u64 = 30; | |
| 221 | + | // How often the background probe pings clamd to maintain the runtime liveness | |
| 222 | + | // flag. The boot gate proves clamd was up at startup; this catches a death | |
| 223 | + | // AFTER boot so a configured-but-erroring clamav scan holds otherwise-clean | |
| 224 | + | // uploads for review instead of passing them on zero AV coverage (FailOpen). | |
| 225 | + | pub const SCAN_CLAMAV_HEALTH_PROBE_SECS: u64 = 30; | |
| 209 | 226 | ||
| 210 | 227 | // -- Invite system -- | |
| 211 | 228 | pub const INVITES_ENABLED: bool = true; | |
| @@ -358,6 +375,10 @@ const _: () = assert!(SCAN_MAX_MEMORY_BYTES > 0); | |||
| 358 | 375 | const _: () = assert!(SCAN_SPOOL_FREE_RESERVE_BYTES < SCAN_SPOOL_MAX_BYTES); | |
| 359 | 376 | const _: () = assert!(SCAN_SPOOL_MAX_BYTES > SCAN_MAX_MEMORY_BYTES as u64); | |
| 360 | 377 | const _: () = assert!(SCAN_JOB_RETENTION_DAYS >= 7); // no same-day purge race | |
| 378 | + | // The concurrency ceiling must not sit below the worker count, or the memory | |
| 379 | + | // budget it's meant to enforce is unenforceable (workers would exceed it). | |
| 380 | + | const _: () = assert!(SCAN_MAX_CONCURRENT >= SCAN_WORKER_COUNT); | |
| 381 | + | const _: () = assert!(SCAN_ZIP_MAX_ENTRIES > 0); | |
| 361 | 382 | const _: () = assert!(BROADCAST_PARALLELISM > 0 && BROADCAST_PARALLELISM <= 64); | |
| 362 | 383 | const _: () = assert!(SCAN_ZIP_MAX_UNCOMPRESSED > SCAN_MAX_MEMORY_BYTES as u64); | |
| 363 | 384 | const _: () = assert!(GIT_RAW_MAX_BYTES > GIT_MAX_FILE_SIZE_BYTES); |
| @@ -1,4 +1,101 @@ | |||
| 1 | - | //! Cryptographic utilities: constant-time comparison, key generation, feed signing. | |
| 1 | + | //! Cryptographic utilities: constant-time comparison, key generation, feed | |
| 2 | + | //! signing, and secret encryption at rest. | |
| 3 | + | ||
| 4 | + | use crate::error::{AppError, Result}; | |
| 5 | + | ||
| 6 | + | /// Version-tagged prefix on an encrypted-at-rest TOTP secret. Its presence is | |
| 7 | + | /// how [`decrypt_totp_secret`] distinguishes a ciphertext from a legacy | |
| 8 | + | /// plaintext base32 seed during the dual-read migration window. | |
| 9 | + | const TOTP_ENC_PREFIX: &str = "enc:v1:"; | |
| 10 | + | ||
| 11 | + | /// Derive a domain-separated 32-byte key for TOTP-secret encryption from the | |
| 12 | + | /// global signing secret, using HMAC-SHA256 as a PRF. The label keeps this key | |
| 13 | + | /// independent of every other use of the signing secret (feed signing, | |
| 14 | + | /// backup-code HMAC, session tokens), so reuse in one context can't weaken | |
| 15 | + | /// another. | |
| 16 | + | fn totp_encryption_key(signing_secret: &str) -> [u8; 32] { | |
| 17 | + | use hmac::{Hmac, Mac}; | |
| 18 | + | use sha2::Sha256; | |
| 19 | + | ||
| 20 | + | let mut mac = Hmac::<Sha256>::new_from_slice(signing_secret.as_bytes()) | |
| 21 | + | .expect("HMAC-SHA256 accepts any key length"); | |
| 22 | + | mac.update(b"mnw-totp-secret-encryption-v1"); | |
| 23 | + | mac.finalize().into_bytes().into() | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | /// Encrypt a TOTP secret for storage at rest with ChaCha20-Poly1305 (AEAD). | |
| 27 | + | /// | |
| 28 | + | /// The on-disk form is `enc:v1:` + base64(`nonce(12) || ciphertext+tag`). A | |
| 29 | + | /// fresh random nonce is drawn per call, so encrypting the same seed twice | |
| 30 | + | /// yields distinct ciphertexts. A database read alone (snapshot, replica, SQL | |
| 31 | + | /// injection elsewhere) no longer yields a usable second factor — the attacker | |
| 32 | + | /// also needs `SIGNING_SECRET`. | |
| 33 | + | pub fn encrypt_totp_secret(plaintext: &str, signing_secret: &str) -> String { | |
| 34 | + | use base64::Engine; | |
| 35 | + | use chacha20poly1305::{aead::Aead, ChaCha20Poly1305, KeyInit, Nonce}; | |
| 36 | + | ||
| 37 | + | let key = totp_encryption_key(signing_secret); | |
| 38 | + | let cipher = ChaCha20Poly1305::new((&key).into()); | |
| 39 | + | ||
| 40 | + | let mut nonce_bytes = [0u8; 12]; | |
| 41 | + | rand::RngCore::fill_bytes(&mut rand::rng(), &mut nonce_bytes); | |
| 42 | + | let nonce = Nonce::from(nonce_bytes); | |
| 43 | + | ||
| 44 | + | let ciphertext = cipher | |
| 45 | + | .encrypt(&nonce, plaintext.as_bytes()) | |
| 46 | + | // Encryption of an in-memory plaintext with a valid key/nonce cannot | |
| 47 | + | // fail; the only error variant is for buffer-size issues we don't hit. | |
| 48 | + | .expect("ChaCha20-Poly1305 encryption is infallible here"); | |
| 49 | + | ||
| 50 | + | let mut payload = Vec::with_capacity(nonce_bytes.len() + ciphertext.len()); | |
| 51 | + | payload.extend_from_slice(&nonce_bytes); | |
| 52 | + | payload.extend_from_slice(&ciphertext); | |
| 53 | + | ||
| 54 | + | format!( | |
| 55 | + | "{TOTP_ENC_PREFIX}{}", | |
| 56 | + | base64::engine::general_purpose::STANDARD.encode(payload) | |
| 57 | + | ) | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | /// Decrypt a stored TOTP secret produced by [`encrypt_totp_secret`]. | |
| 61 | + | /// | |
| 62 | + | /// Every stored secret MUST carry the `enc:v1:` prefix. There is no legacy | |
| 63 | + | /// plaintext fallback — a value without the prefix is rejected as malformed | |
| 64 | + | /// (backwards compatibility with pre-encryption plaintext seeds was cut, so | |
| 65 | + | /// any such user re-enrolls their authenticator). Also errors when a tagged | |
| 66 | + | /// ciphertext fails to decode or authenticate (wrong key or tampering). | |
| 67 | + | pub fn decrypt_totp_secret(stored: &str, signing_secret: &str) -> Result<String> { | |
| 68 | + | use base64::Engine; | |
| 69 | + | use chacha20poly1305::{aead::Aead, ChaCha20Poly1305, KeyInit, Nonce}; | |
| 70 | + | ||
| 71 | + | let Some(b64) = stored.strip_prefix(TOTP_ENC_PREFIX) else { | |
| 72 | + | return Err(AppError::Internal(anyhow::anyhow!( | |
| 73 | + | "totp secret is not encrypted (missing {TOTP_ENC_PREFIX} prefix)" | |
| 74 | + | ))); | |
| 75 | + | }; | |
| 76 | + | ||
| 77 | + | let payload = base64::engine::general_purpose::STANDARD | |
| 78 | + | .decode(b64) | |
| 79 | + | .map_err(|e| AppError::Internal(anyhow::anyhow!("totp secret base64 decode: {e}")))?; | |
| 80 | + | if payload.len() < 12 { | |
| 81 | + | return Err(AppError::Internal(anyhow::anyhow!( | |
| 82 | + | "totp secret ciphertext too short" | |
| 83 | + | ))); | |
| 84 | + | } | |
| 85 | + | let (nonce_bytes, ciphertext) = payload.split_at(12); | |
| 86 | + | let nonce_arr: [u8; 12] = nonce_bytes | |
| 87 | + | .try_into() | |
| 88 | + | .expect("split_at(12) on a >=12-byte payload yields exactly 12 bytes"); | |
| 89 | + | ||
| 90 | + | let key = totp_encryption_key(signing_secret); | |
| 91 | + | let cipher = ChaCha20Poly1305::new((&key).into()); | |
| 92 | + | let plaintext = cipher | |
| 93 | + | .decrypt(&Nonce::from(nonce_arr), ciphertext) | |
| 94 | + | .map_err(|_| AppError::Internal(anyhow::anyhow!("totp secret decryption failed")))?; | |
| 95 | + | ||
| 96 | + | String::from_utf8(plaintext) | |
| 97 | + | .map_err(|e| AppError::Internal(anyhow::anyhow!("totp secret utf8: {e}"))) | |
| 98 | + | } | |
| 2 | 99 | ||
| 3 | 100 | /// Constant-time byte comparison for tokens, MACs, and other fixed-shape | |
| 4 | 101 | /// secrets. Backed by [`subtle::ConstantTimeEq`] (audited reference impl) | |
| @@ -86,6 +183,53 @@ pub fn verify_feed_signature( | |||
| 86 | 183 | mod tests { | |
| 87 | 184 | use super::*; | |
| 88 | 185 | ||
| 186 | + | // ── TOTP secret encryption at rest ── | |
| 187 | + | ||
| 188 | + | #[test] | |
| 189 | + | fn totp_secret_round_trips() { | |
| 190 | + | let secret = "JBSWY3DPEHPK3PXP"; | |
| 191 | + | let key = "a-stable-signing-secret-at-least-32c"; | |
| 192 | + | let enc = encrypt_totp_secret(secret, key); | |
| 193 | + | assert!(enc.starts_with("enc:v1:"), "ciphertext must be version-tagged"); | |
| 194 | + | assert_ne!(enc, secret, "ciphertext must not be the plaintext"); | |
| 195 | + | assert_eq!(decrypt_totp_secret(&enc, key).unwrap(), secret); | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | #[test] | |
| 199 | + | fn totp_secret_nonce_is_random() { | |
| 200 | + | let secret = "JBSWY3DPEHPK3PXP"; | |
| 201 | + | let key = "a-stable-signing-secret-at-least-32c"; | |
| 202 | + | // Same plaintext + key encrypted twice must differ (fresh nonce each time). | |
| 203 | + | assert_ne!(encrypt_totp_secret(secret, key), encrypt_totp_secret(secret, key)); | |
| 204 | + | } | |
| 205 | + | ||
| 206 | + | #[test] | |
| 207 | + | fn totp_secret_wrong_key_fails_to_decrypt() { | |
| 208 | + | let secret = "JBSWY3DPEHPK3PXP"; | |
| 209 | + | let enc = encrypt_totp_secret(secret, "a-stable-signing-secret-at-least-32c"); | |
| 210 | + | assert!(decrypt_totp_secret(&enc, "a-different-signing-secret-32-chars!").is_err()); | |
| 211 | + | } | |
| 212 | + | ||
| 213 | + | #[test] | |
| 214 | + | fn totp_secret_tampered_ciphertext_fails() { | |
| 215 | + | let key = "a-stable-signing-secret-at-least-32c"; | |
| 216 | + | let enc = encrypt_totp_secret("JBSWY3DPEHPK3PXP", key); | |
| 217 | + | // Flip a character in the base64 body — the AEAD tag must reject it. | |
| 218 | + | let mut bytes: Vec<char> = enc.chars().collect(); | |
| 219 | + | let last = bytes.len() - 1; | |
| 220 | + | bytes[last] = if bytes[last] == 'A' { 'B' } else { 'A' }; | |
| 221 | + | let tampered: String = bytes.into_iter().collect(); | |
| 222 | + | assert!(decrypt_totp_secret(&tampered, key).is_err()); | |
| 223 | + | } | |
| 224 | + | ||
| 225 | + | #[test] | |
| 226 | + | fn totp_secret_unprefixed_plaintext_is_rejected() { | |
| 227 | + | // Backwards compat was cut: a bare (pre-encryption) plaintext seed has | |
| 228 | + | // no `enc:v1:` prefix and must be rejected, not trusted. | |
| 229 | + | let key = "a-stable-signing-secret-at-least-32c"; | |
| 230 | + | assert!(decrypt_totp_secret("JBSWY3DPEHPK3PXP", key).is_err()); | |
| 231 | + | } | |
| 232 | + | ||
| 89 | 233 | // ── constant_time_compare ── | |
| 90 | 234 | ||
| 91 | 235 | #[test] |
| @@ -425,6 +425,11 @@ pub async fn get_storage_breakdown(pool: &PgPool, user_id: UserId) -> Result<Sto | |||
| 425 | 425 | /// whose items have not yet been hidden. | |
| 426 | 426 | #[tracing::instrument(skip_all)] | |
| 427 | 427 | pub async fn get_expired_grace_creators(pool: &PgPool) -> Result<Vec<UserId>> { | |
| 428 | + | // Bounded batch per call. The scheduler enforces these inline on the tick | |
| 429 | + | // (two DB round-trips per creator), so an unbounded result set would let a | |
| 430 | + | // backlog stall the tick. `grace_enforced_at` is set as each creator is | |
| 431 | + | // processed, so successive ticks drain the rest; ORDER BY oldest-first keeps | |
| 432 | + | // it deterministic and starvation-free. | |
| 428 | 433 | let ids: Vec<UserId> = sqlx::query_scalar( | |
| 429 | 434 | r#" | |
| 430 | 435 | SELECT user_id FROM creator_subscriptions | |
| @@ -432,6 +437,8 @@ pub async fn get_expired_grace_creators(pool: &PgPool) -> Result<Vec<UserId>> { | |||
| 432 | 437 | AND canceled_at IS NOT NULL | |
| 433 | 438 | AND canceled_at < NOW() - INTERVAL '30 days' | |
| 434 | 439 | AND grace_enforced_at IS NULL | |
| 440 | + | ORDER BY canceled_at ASC | |
| 441 | + | LIMIT 200 | |
| 435 | 442 | "#, | |
| 436 | 443 | ) | |
| 437 | 444 | .fetch_all(pool) |
| @@ -5,7 +5,81 @@ use sqlx::PgPool; | |||
| 5 | 5 | ||
| 6 | 6 | use crate::db::models::*; | |
| 7 | 7 | use crate::db::{ItemId, UserId}; | |
| 8 | - | use crate::error::Result; | |
| 8 | + | use crate::error::{AppError, Result}; | |
| 9 | + | ||
| 10 | + | /// Outcome of [`update_item_file_cas`]. | |
| 11 | + | #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| 12 | + | pub enum FileConfirmOutcome { | |
| 13 | + | /// The compare-and-swap matched: the new key/size were written. | |
| 14 | + | Committed, | |
| 15 | + | /// The target s3 column no longer held the expected key (a concurrent | |
| 16 | + | /// confirm won the race) or the item/owner no longer matched. The caller | |
| 17 | + | /// MUST NOT credit storage and should treat the staged object as an orphan. | |
| 18 | + | LostRace, | |
| 19 | + | } | |
| 20 | + | ||
| 21 | + | /// Confirm an uploaded item file onto its `items` row with a compare-and-swap. | |
| 22 | + | /// | |
| 23 | + | /// This is the ONLY way to write the generic item-file columns (audio/video | |
| 24 | + | /// key + size). The raw `UPDATE items SET <col> = ...` is sealed in this module | |
| 25 | + | /// so a handler can't hand-roll an unguarded write — the recurring CHRONIC | |
| 26 | + | /// where the item-confirm path lacked the `IS NOT DISTINCT FROM` guard its | |
| 27 | + | /// sibling version-confirm path had. | |
| 28 | + | /// | |
| 29 | + | /// The CAS predicate (`<s3_col> IS NOT DISTINCT FROM expected_old_key`) updates | |
| 30 | + | /// the row only if the column still holds the key the caller observed (`NULL` | |
| 31 | + | /// for a first upload). A concurrent confirm that already swapped the column in | |
| 32 | + | /// makes this match zero rows -> [`FileConfirmOutcome::LostRace`], so storage is | |
| 33 | + | /// never double-credited and a live object is never clobbered. | |
| 34 | + | /// | |
| 35 | + | /// Takes any `PgExecutor` so the caller can run it inside the same transaction | |
| 36 | + | /// as the storage credit (a rollback then undoes both atomically). | |
| 37 | + | pub async fn update_item_file_cas<'e>( | |
| 38 | + | executor: impl sqlx::PgExecutor<'e>, | |
| 39 | + | item_id: ItemId, | |
| 40 | + | owner: UserId, | |
| 41 | + | file_type: crate::storage::FileType, | |
| 42 | + | expected_old_key: Option<&str>, | |
| 43 | + | new_key: &str, | |
| 44 | + | size: i64, | |
| 45 | + | ) -> Result<FileConfirmOutcome> { | |
| 46 | + | use crate::storage::GenericItemConfirm; | |
| 47 | + | ||
| 48 | + | // Column names come from the exhaustive `generic_item_confirm` match — | |
| 49 | + | // `&'static str`, never user input — so the `format!` is injection-safe. | |
| 50 | + | let (s3_col, size_col) = match file_type.generic_item_confirm() { | |
| 51 | + | GenericItemConfirm::Columns { s3_key, size } => (s3_key, size), | |
| 52 | + | GenericItemConfirm::UseRoute(route) => { | |
| 53 | + | // The generic confirm handler rejects these before reaching here; | |
| 54 | + | // a call with such a type is a programming error, not user input. | |
| 55 | + | return Err(AppError::Internal(anyhow::anyhow!( | |
| 56 | + | "update_item_file_cas called for {} which must use {route}", | |
| 57 | + | file_type.as_str() | |
| 58 | + | ))); | |
| 59 | + | } | |
| 60 | + | }; | |
| 61 | + | ||
| 62 | + | let sql = format!( | |
| 63 | + | "UPDATE items SET {s3_col} = $2, {size_col} = $3, updated_at = NOW() \ | |
| 64 | + | WHERE id = $1 \ | |
| 65 | + | AND project_id IN (SELECT id FROM projects WHERE user_id = $4) \ | |
| 66 | + | AND {s3_col} IS NOT DISTINCT FROM $5" | |
| 67 | + | ); | |
| 68 | + | let res = sqlx::query(&sql) | |
| 69 | + | .bind(item_id) | |
| 70 | + | .bind(new_key) | |
| 71 | + | .bind(size) | |
| 72 | + | .bind(owner) | |
| 73 | + | .bind(expected_old_key) | |
| 74 | + | .execute(executor) | |
| 75 | + | .await?; | |
| 76 | + | ||
| 77 | + | Ok(if res.rows_affected() == 0 { | |
| 78 | + | FileConfirmOutcome::LostRace | |
| 79 | + | } else { | |
| 80 | + | FileConfirmOutcome::Committed | |
| 81 | + | }) | |
| 82 | + | } | |
| 9 | 83 | ||
| 10 | 84 | /// Get the audio, cover, and video file sizes for an item (for storage decrement on delete). | |
| 11 | 85 | #[tracing::instrument(skip_all)] |
| @@ -16,8 +16,8 @@ pub struct PendingS3Deletion { | |||
| 16 | 16 | ||
| 17 | 17 | /// Enqueue S3 keys for deletion. Each key is (s3_key, bucket). | |
| 18 | 18 | #[tracing::instrument(skip_all)] | |
| 19 | - | pub async fn enqueue_deletions( | |
| 20 | - | pool: &PgPool, | |
| 19 | + | pub async fn enqueue_deletions<'e>( | |
| 20 | + | executor: impl sqlx::PgExecutor<'e>, | |
| 21 | 21 | keys: &[(String, String)], | |
| 22 | 22 | source: &str, | |
| 23 | 23 | ) -> Result<()> { | |
| @@ -26,13 +26,16 @@ pub async fn enqueue_deletions( | |||
| 26 | 26 | } | |
| 27 | 27 | let s3_keys: Vec<&str> = keys.iter().map(|(k, _)| k.as_str()).collect(); | |
| 28 | 28 | let buckets: Vec<&str> = keys.iter().map(|(_, b)| b.as_str()).collect(); | |
| 29 | + | // Takes any `PgExecutor` so callers can enqueue inside a transaction — | |
| 30 | + | // letting a row delete, its storage refund, and the S3-delete enqueue commit | |
| 31 | + | // atomically (see `versions::delete_version`). | |
| 29 | 32 | sqlx::query( | |
| 30 | 33 | "INSERT INTO pending_s3_deletions (s3_key, bucket, source) SELECT * FROM unnest($1::text[], $2::text[], $3::text[]) ON CONFLICT DO NOTHING", | |
| 31 | 34 | ) | |
| 32 | 35 | .bind(&s3_keys) | |
| 33 | 36 | .bind(&buckets) | |
| 34 | 37 | .bind(vec![source; keys.len()]) | |
| 35 | - | .execute(pool) | |
| 38 | + | .execute(executor) | |
| 36 | 39 | .await?; | |
| 37 | 40 | Ok(()) | |
| 38 | 41 | } |
| @@ -22,7 +22,7 @@ pub async fn record_pending_upload( | |||
| 22 | 22 | // an orphan object alive indefinitely. | |
| 23 | 23 | sqlx::query( | |
| 24 | 24 | "INSERT INTO pending_uploads (user_id, s3_key, bucket) VALUES ($1, $2, $3) | |
| 25 | - | ON CONFLICT (s3_key) DO UPDATE SET created_at = NOW() | |
| 25 | + | ON CONFLICT (s3_key, bucket) DO UPDATE SET created_at = NOW() | |
| 26 | 26 | WHERE pending_uploads.user_id = EXCLUDED.user_id", | |
| 27 | 27 | ) | |
| 28 | 28 | .bind(user_id) |
| @@ -58,16 +58,15 @@ impl ScanTargetKind { | |||
| 58 | 58 | }) | |
| 59 | 59 | } | |
| 60 | 60 | ||
| 61 | - | /// Whether this kind is served directly from the CDN by `s3_key` with no | |
| 62 | - | /// app-proxied download route that can consult a `scan_status` column. | |
| 61 | + | /// Whether this kind carries no `scan_status` column and is rendered | |
| 62 | + | /// straight from `cdn.makenot.work/{key}`, so a `Quarantined` verdict can | |
| 63 | + | /// only be enforced by deleting the *DB row* (which stops the URL from | |
| 64 | + | /// rendering and makes the key non-live for the deletion queue). | |
| 63 | 65 | /// | |
| 64 | - | /// `Item`/`Version`/`Media` are gated at their download handlers (the route | |
| 65 | - | /// refuses to issue a non-`Clean` object). These three image kinds carry no | |
| 66 | - | /// `scan_status` column and are rendered straight from `cdn.makenot.work/{key}`, | |
| 67 | - | /// so the ONLY enforcement point for a `Quarantined` verdict is removing the | |
| 68 | - | /// object from storage — otherwise the malicious file keeps serving from the | |
| 69 | - | /// CDN until an admin manually acts on the WAM ticket. The worker purges the | |
| 70 | - | /// object for these kinds on quarantine. | |
| 66 | + | /// This governs the DB-row step only. The S3-object purge on quarantine is | |
| 67 | + | /// unconditional (see [`quarantine_purges_object`]); these three kinds | |
| 68 | + | /// additionally need their row removed because they have no status column | |
| 69 | + | /// to gate on. | |
| 71 | 70 | pub fn is_cdn_served_without_gate(&self) -> bool { | |
| 72 | 71 | matches!( | |
| 73 | 72 | self, | |
| @@ -76,6 +75,22 @@ impl ScanTargetKind { | |||
| 76 | 75 | | ScanTargetKind::ContentInsertion | |
| 77 | 76 | ) | |
| 78 | 77 | } | |
| 78 | + | ||
| 79 | + | /// Whether a `Quarantined` verdict purges the underlying S3 object. | |
| 80 | + | /// | |
| 81 | + | /// Always true: a confirmed-malicious object has no reason to remain in | |
| 82 | + | /// storage. Crucially, the per-request `scan_status` gate on | |
| 83 | + | /// `Item`/`Version`/`Media` is NOT sufficient on its own — free downloadable | |
| 84 | + | /// content is handed out as a *permanent* `cdn.makenot.work/{key}` URL with | |
| 85 | + | /// no per-request gate (see `routes/storage/downloads.rs::resolve_content_url`), | |
| 86 | + | /// so a leaked or edge-cached URL keeps serving the malware from origin until | |
| 87 | + | /// the object itself is gone. Removing the object closes that hole for | |
| 88 | + | /// downloadable content the same way it does for the gate-less image kinds. | |
| 89 | + | /// The decision lives here, beside `is_cdn_served_without_gate`, so the two | |
| 90 | + | /// halves of quarantine enforcement can't silently diverge per kind. | |
| 91 | + | pub fn quarantine_purges_object(&self) -> bool { | |
| 92 | + | true | |
| 93 | + | } | |
| 79 | 94 | } | |
| 80 | 95 | ||
| 81 | 96 | /// A queued or running scan job, as claimed by a worker. Most fields are | |
| @@ -367,4 +382,34 @@ mod tests { | |||
| 367 | 382 | } | |
| 368 | 383 | assert_eq!(ScanTargetKind::from_str("bogus"), None); | |
| 369 | 384 | } | |
| 385 | + | ||
| 386 | + | #[test] | |
| 387 | + | fn all_kinds_purge_object_on_quarantine() { | |
| 388 | + | // The S3-object purge is unconditional — a confirmed-malicious object | |
| 389 | + | // must never survive in storage, including downloadable Item/Version/ | |
| 390 | + | // Media content reachable via a permanent CDN URL. | |
| 391 | + | for kind in [ | |
| 392 | + | ScanTargetKind::Item, | |
| 393 | + | ScanTargetKind::Version, | |
| 394 | + | ScanTargetKind::Media, | |
| 395 | + | ScanTargetKind::ProjectImage, | |
| 396 | + | ScanTargetKind::ItemImage, | |
| 397 | + | ScanTargetKind::GalleryImage, | |
| 398 | + | ScanTargetKind::ContentInsertion, | |
| 399 | + | ] { | |
| 400 | + | assert!(kind.quarantine_purges_object(), "{} must purge object", kind.as_str()); | |
| 401 | + | } | |
| 402 | + | } | |
| 403 | + | ||
| 404 | + | #[test] | |
| 405 | + | fn only_gateless_image_kinds_delete_their_row() { | |
| 406 | + | // The DB-row deletion is the gate-less-image-only half of enforcement. | |
| 407 | + | use ScanTargetKind::*; | |
| 408 | + | for kind in [ProjectImage, GalleryImage, ContentInsertion] { | |
| 409 | + | assert!(kind.is_cdn_served_without_gate(), "{} is gate-less", kind.as_str()); | |
| 410 | + | } | |
| 411 | + | for kind in [Item, Version, Media, ItemImage] { | |
| 412 | + | assert!(!kind.is_cdn_served_without_gate(), "{} has a gate/status", kind.as_str()); | |
| 413 | + | } | |
| 414 | + | } | |
| 370 | 415 | } |
| @@ -5,28 +5,50 @@ use sqlx::PgPool; | |||
| 5 | 5 | use super::UserId; | |
| 6 | 6 | use crate::error::Result; | |
| 7 | 7 | ||
| 8 | - | /// Get the stored TOTP secret for a user (None if not set up). | |
| 8 | + | /// Get the stored TOTP secret for a user, decrypted (None if not set up). | |
| 9 | + | /// | |
| 10 | + | /// Secrets are encrypted at rest with the global signing secret; this returns | |
| 11 | + | /// the plaintext base32 seed. A stored value that isn't a valid `enc:v1:` | |
| 12 | + | /// ciphertext (e.g. a pre-encryption plaintext seed — backwards compat was | |
| 13 | + | /// cut) errors out, forcing the user to re-enroll their authenticator. | |
| 9 | 14 | #[tracing::instrument(skip_all)] | |
| 10 | - | pub async fn get_totp_secret(pool: &PgPool, user_id: UserId) -> Result<Option<String>> { | |
| 11 | - | let secret: Option<String> = | |
| 15 | + | pub async fn get_totp_secret( | |
| 16 | + | pool: &PgPool, | |
| 17 | + | user_id: UserId, | |
| 18 | + | signing_secret: &str, | |
| 19 | + | ) -> Result<Option<String>> { | |
| 20 | + | let stored: Option<String> = | |
| 12 | 21 | sqlx::query_scalar("SELECT totp_secret FROM users WHERE id = $1") | |
| 13 | 22 | .bind(user_id) | |
| 14 | 23 | .fetch_one(pool) | |
| 15 | 24 | .await?; | |
| 16 | 25 | ||
| 17 | - | Ok(secret) | |
| 26 | + | stored | |
| 27 | + | .map(|s| crate::crypto::decrypt_totp_secret(&s, signing_secret)) | |
| 28 | + | .transpose() | |
| 18 | 29 | } | |
| 19 | 30 | ||
| 20 | 31 | /// Store a TOTP secret for a user (does not enable 2FA yet). | |
| 32 | + | /// | |
| 33 | + | /// The plaintext base32 seed is encrypted at rest with the signing secret | |
| 34 | + | /// before it touches the database, so a DB read alone cannot recover a usable | |
| 35 | + | /// second factor. | |
| 21 | 36 | #[tracing::instrument(skip_all)] | |
| 22 | - | pub async fn set_totp_secret(pool: &PgPool, user_id: UserId, secret: &str) -> Result<()> { | |
| 37 | + | pub async fn set_totp_secret( | |
| 38 | + | pool: &PgPool, | |
| 39 | + | user_id: UserId, | |
| 40 | + | secret: &str, | |
| 41 | + | signing_secret: &str, | |
| 42 | + | ) -> Result<()> { | |
| 43 | + | let encrypted = crate::crypto::encrypt_totp_secret(secret, signing_secret); | |
| 44 | + | ||
| 23 | 45 | // Clear the replay step alongside the secret. Without this, a user who | |
| 24 | 46 | // disables and re-enables TOTP (potentially with a new secret) inherits a | |
| 25 | 47 | // stale `totp_last_used_step` and any first-attempt code in a lower step | |
| 26 | 48 | // window is false-rejected as a replay. | |
| 27 | 49 | sqlx::query("UPDATE users SET totp_secret = $2, totp_last_used_step = NULL WHERE id = $1") | |
| 28 | 50 | .bind(user_id) | |
| 29 | - | .bind(secret) | |
| 51 | + | .bind(encrypted) | |
| 30 | 52 | .execute(pool) | |
| 31 | 53 | .await?; | |
| 32 | 54 |
| @@ -330,12 +330,16 @@ impl sqlx::Encode<'_, sqlx::Postgres> for Cents { | |||
| 330 | 330 | ) -> Result<sqlx::encode::IsNull, Box<dyn std::error::Error + Send + Sync>> { | |
| 331 | 331 | // Encode as i32 for INT columns (most price fields). | |
| 332 | 332 | // All write paths originate from PriceCents (capped at $10k), so this | |
| 333 | - | // cast is safe. The assert catches misuse in all builds. | |
| 334 | - | assert!( | |
| 335 | - | self.0 >= i32::MIN as i64 && self.0 <= i32::MAX as i64, | |
| 336 | - | "Cents value {} overflows i32 on encode — use BIGINT column or check write path", | |
| 337 | - | self.0 | |
| 338 | - | ); | |
| 333 | + | // cast is safe. If a future write path ever binds an out-of-range value | |
| 334 | + | // to an INT4 column, return an encode error rather than panicking the | |
| 335 | + | // request task — it surfaces as a normal query failure. | |
| 336 | + | if self.0 < i32::MIN as i64 || self.0 > i32::MAX as i64 { | |
| 337 | + | return Err(format!( | |
| 338 | + | "Cents value {} overflows i32 on encode — use a BIGINT column or check the write path", | |
| 339 | + | self.0 | |
| 340 | + | ) | |
| 341 | + | .into()); | |
| 342 | + | } | |
| 339 | 343 | <i32 as sqlx::Encode<'_, sqlx::Postgres>>::encode(self.0 as i32, buf) | |
| 340 | 344 | } | |
| 341 | 345 | } |
| @@ -297,27 +297,31 @@ pub async fn delete_version(pool: &PgPool, version_id: VersionId) -> Result<()> | |||
| 297 | 297 | .execute(&mut *tx) | |
| 298 | 298 | .await?; | |
| 299 | 299 | ||
| 300 | - | if deleted.rows_affected() > 0 | |
| 301 | - | && let Some(user_id) = owner_id | |
| 302 | - | && let Some(size) = version.file_size_bytes | |
| 303 | - | && size > 0 | |
| 304 | - | { | |
| 305 | - | crate::db::creator_tiers::decrement_storage_used(&mut *tx, user_id, size).await?; | |
| 300 | + | if deleted.rows_affected() > 0 { | |
| 301 | + | if let Some(user_id) = owner_id | |
| 302 | + | && let Some(size) = version.file_size_bytes | |
| 303 | + | && size > 0 | |
| 304 | + | { | |
| 305 | + | crate::db::creator_tiers::decrement_storage_used(&mut *tx, user_id, size).await?; | |
| 306 | + | } | |
| 307 | + | ||
| 308 | + | // Enqueue the S3 delete inside the SAME tx as the row delete + refund. | |
| 309 | + | // After commit the row is gone (so the key is non-live and the deletion | |
| 310 | + | // worker can act), and a crash between commit and a post-commit enqueue | |
| 311 | + | // can no longer orphan the object — all three effects are atomic, as the | |
| 312 | + | // doc comment promises. | |
| 313 | + | if let Some(s3_key) = version.s3_key.as_deref() { | |
| 314 | + | crate::db::pending_s3_deletions::enqueue_deletions( | |
| 315 | + | &mut *tx, | |
| 316 | + | &[(s3_key.to_string(), "main".to_string())], | |
| 317 | + | "version_delete", | |
| 318 | + | ) | |
| 319 | + | .await?; | |
| 320 | + | } | |
| 306 | 321 | } | |
| 307 | 322 | ||
| 308 | 323 | tx.commit().await?; | |
| 309 | 324 | ||
| 310 | - | if let Some(s3_key) = version.s3_key.as_deref() | |
| 311 | - | && let Err(e) = crate::db::pending_s3_deletions::enqueue_deletions( | |
| 312 | - | pool, | |
| 313 | - | &[(s3_key.to_string(), "main".to_string())], | |
| 314 | - | "version_delete", | |
| 315 | - | ) | |
| 316 | - | .await | |
| 317 | - | { | |
| 318 | - | tracing::warn!(error = ?e, version_id = %version_id, "failed to enqueue S3 deletion for version"); | |
| 319 | - | } | |
| 320 | - | ||
| 321 | 325 | Ok(()) | |
| 322 | 326 | } | |
| 323 | 327 |
| @@ -363,18 +363,34 @@ async fn main() { | |||
| 363 | 363 | // Start scan worker pool. Only meaningful if a scanner is configured; | |
| 364 | 364 | // otherwise enqueue_scan_for never enqueues (trust-gate fast path). | |
| 365 | 365 | if let (Some(scanner), Some(s3_for_workers)) = (state.scanner.clone(), state.s3.clone()) { | |
| 366 | + | // Runtime ClamAV liveness: starts healthy (the boot gate already proved | |
| 367 | + | // clamd was up), maintained by the probe spawned below. | |
| 368 | + | let clamav_healthy = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)); | |
| 369 | + | let clamav_socket = scanner.clamav_socket().map(str::to_string); | |
| 366 | 370 | let scan_ctx = std::sync::Arc::new(makenotwork::scanning::worker::WorkerContext { | |
| 367 | 371 | db: state.db.clone(), | |
| 368 | 372 | s3: s3_for_workers, | |
| 369 | 373 | pipeline: scanner, | |
| 370 | 374 | scan_semaphore: state.scan_semaphore.clone(), | |
| 371 | 375 | wam: state.wam.clone(), | |
| 376 | + | clamav_healthy: clamav_healthy.clone(), | |
| 372 | 377 | }); | |
| 373 | 378 | let worker_count = makenotwork::constants::SCAN_WORKER_COUNT; | |
| 374 | 379 | let worker_shutdown_rx = shutdown_tx.subscribe(); | |
| 375 | 380 | makenotwork::scanning::worker::spawn_pool(worker_count, scan_ctx, worker_shutdown_rx); | |
| 376 | 381 | tracing::info!(worker_count, "scan worker pool started"); | |
| 377 | 382 | ||
| 383 | + | // Post-boot clamd liveness probe — flips clean verdicts to held-for-review | |
| 384 | + | // if clamd dies after startup (the boot gate can't catch that). | |
| 385 | + | if let Some(socket) = clamav_socket { | |
| 386 | + | makenotwork::scanning::worker::spawn_clamav_health_probe( | |
| 387 | + | socket, | |
| 388 | + | clamav_healthy, | |
| 389 | + | shutdown_tx.subscribe(), | |
| 390 | + | ); | |
| 391 | + | tracing::info!("clamav runtime health probe started"); | |
| 392 | + | } | |
| 393 | + | ||
| 378 | 394 | let report = makenotwork::scanning::spool::reap_all( | |
| 379 | 395 | std::path::Path::new(makenotwork::constants::SCAN_SPOOL_DIR), | |
| 380 | 396 | ); |
| @@ -208,7 +208,11 @@ pub fn spawn_monitor( | |||
| 208 | 208 | } | |
| 209 | 209 | ||
| 210 | 210 | // Notify opted-in users of status changes (fire-and-forget). | |
| 211 | - | // Paced at ~10/sec to stay under Postmark's default send rate. | |
| 211 | + | // Batched pacing: pause 1s every 50 sends (matching the | |
| 212 | + | // scheduler's announcement fan-out) instead of a hard 100ms | |
| 213 | + | // between every send, which turned a 1000-subscriber notify | |
| 214 | + | // into a ~100s single task holding the email-client clone. | |
| 215 | + | // The subscriber list is bounded by the query's LIMIT. | |
| 212 | 216 | { | |
| 213 | 217 | let pool = state.db.clone(); | |
| 214 | 218 | let email_client = state.email.clone(); | |
| @@ -220,7 +224,10 @@ pub fn spawn_monitor( | |||
| 220 | 224 | match db::users::get_status_alert_subscribers(&pool).await { | |
| 221 | 225 | Ok(subscribers) if !subscribers.is_empty() => { | |
| 222 | 226 | tracing::info!(count = subscribers.len(), "sending status notifications to opted-in users"); | |
| 223 | - | for sub in &subscribers { | |
| 227 | + | for (i, sub) in subscribers.iter().enumerate() { | |
| 228 | + | if i > 0 && i % 50 == 0 { | |
| 229 | + | tokio::time::sleep(std::time::Duration::from_secs(1)).await; | |
| 230 | + | } | |
| 224 | 231 | let unsub_url = crate::email::generate_unsubscribe_url( | |
| 225 | 232 | &host_url, sub.id, crate::email::UnsubscribeAction::Status, &sub.id.to_string(), &signing_secret, | |
| 226 | 233 | ); | |
| @@ -231,7 +238,6 @@ pub fn spawn_monitor( | |||
| 231 | 238 | &prev_status, | |
| 232 | 239 | &unsub_url, | |
| 233 | 240 | ).await; | |
| 234 | - | tokio::time::sleep(std::time::Duration::from_millis(100)).await; | |
| 235 | 241 | } | |
| 236 | 242 | } | |
| 237 | 243 | Err(e) => { |
| @@ -41,8 +41,9 @@ pub(super) async fn setup( | |||
| 41 | 41 | ||
| 42 | 42 | let secret_base32 = totp.get_secret_base32(); | |
| 43 | 43 | ||
| 44 | - | // Store the secret (not yet enabled) | |
| 45 | - | db::totp::set_totp_secret(&state.db, user.id, &secret_base32).await?; | |
| 44 | + | // Store the secret (not yet enabled), encrypted at rest. | |
| 45 | + | db::totp::set_totp_secret(&state.db, user.id, &secret_base32, &state.config.signing_secret) | |
| 46 | + | .await?; | |
| 46 | 47 | ||
| 47 | 48 | // Generate QR code as base64 PNG | |
| 48 | 49 | let qr_base64 = totp | |
| @@ -78,7 +79,7 @@ pub(super) async fn confirm( | |||
| 78 | 79 | AuthUser(user): AuthUser, | |
| 79 | 80 | Form(form): Form<ConfirmForm>, | |
| 80 | 81 | ) -> Result<Response> { | |
| 81 | - | let secret = db::totp::get_totp_secret(&state.db, user.id) | |
| 82 | + | let secret = db::totp::get_totp_secret(&state.db, user.id, &state.config.signing_secret) | |
| 82 | 83 | .await? | |
| 83 | 84 | .ok_or_else(|| AppError::BadRequest("2FA setup not started".to_string()))?; | |
| 84 | 85 |
| @@ -4,13 +4,13 @@ use axum::{ | |||
| 4 | 4 | extract::{Path, State}, | |
| 5 | 5 | response::IntoResponse, | |
| 6 | 6 | routing::get, | |
| 7 | - | Router, | |
| 8 | 7 | }; | |
| 9 | 8 | use tower_sessions::Session; | |
| 10 | 9 | ||
| 11 | 10 | use crate::{ | |
| 12 | 11 | auth::MaybeUserUnverified, | |
| 13 | 12 | constants, | |
| 13 | + | csrf::CsrfRouter, | |
| 14 | 14 | db::{self, Slug}, | |
| 15 | 15 | error::{AppError, Result}, | |
| 16 | 16 | helpers::{fetch_discussion_info, get_csrf_token, get_initials}, | |
| @@ -19,13 +19,14 @@ use crate::{ | |||
| 19 | 19 | AppState, | |
| 20 | 20 | }; | |
| 21 | 21 | ||
| 22 | - | /// Register blog page routes. | |
| 23 | - | pub fn blog_routes() -> Router<AppState> { | |
| 24 | - | Router::new() | |
| 25 | - | .route("/p/{slug}/blog", get(project_blog_page)) | |
| 26 | - | .route("/p/{slug}/blog/{post_slug}", get(blog_post_page)) | |
| 27 | - | .route("/changelog", get(changelog_index)) | |
| 28 | - | .route("/changelog/{post_slug}", get(changelog_post)) | |
| 22 | + | /// Register blog page routes. All GET-only; returned as a `CsrfRouter` so the | |
| 23 | + | /// page-tree aggregator only ever composes posture-declared routers. | |
| 24 | + | pub fn blog_routes() -> CsrfRouter<AppState> { | |
| 25 | + | CsrfRouter::new() | |
| 26 | + | .route_get("/p/{slug}/blog", get(project_blog_page)) | |
| 27 | + | .route_get("/p/{slug}/blog/{post_slug}", get(blog_post_page)) | |
| 28 | + | .route_get("/changelog", get(changelog_index)) | |
| 29 | + | .route_get("/changelog/{post_slug}", get(changelog_post)) | |
| 29 | 30 | } | |
| 30 | 31 | ||
| 31 | 32 | /// Public blog index page for a project. |
| @@ -8,6 +8,7 @@ use std::collections::HashMap; | |||
| 8 | 8 | ||
| 9 | 9 | use axum::{ | |
| 10 | 10 | extract::{Path, State}, | |
| 11 | + | http::{HeaderMap, HeaderValue, StatusCode}, | |
| 11 | 12 | response::{IntoResponse, Response}, | |
| 12 | 13 | Form, | |
| 13 | 14 | }; | |
| @@ -222,19 +223,40 @@ pub async fn step_save( | |||
| 222 | 223 | session: Session, | |
| 223 | 224 | AuthUser(user): AuthUser, | |
| 224 | 225 | Path((slug, id, step)): Path<(String, String, String)>, | |
| 226 | + | headers: HeaderMap, | |
| 225 | 227 | Form(form): Form<HashMap<String, String>>, | |
| 226 | 228 | ) -> Result<Response> { | |
| 227 | 229 | user.check_not_suspended()?; | |
| 228 | 230 | let (project, item) = verify_item_wizard_access(&state, &user, &slug, &id).await?; | |
| 229 | 231 | ||
| 230 | - | match step.as_str() { | |
| 231 | - | "type" => save::save_type(&state, &project, &item, &form, user.id).await?, | |
| 232 | - | "basics" => save::save_basics(&state, &item, &form, user.id).await?, | |
| 233 | - | "content" => save::save_content(&state, &item, &form, user.id).await?, | |
| 234 | - | "sections" => {} // Sections managed via HTMX API; pass-through | |
| 235 | - | "pricing" => save::save_pricing(&state, &item, &form, user.id).await?, | |
| 232 | + | let save_result = match step.as_str() { | |
| 233 | + | "type" => save::save_type(&state, &project, &item, &form, user.id).await, | |
| 234 | + | "basics" => save::save_basics(&state, &item, &form, user.id).await, | |
| 235 | + | "content" => save::save_content(&state, &item, &form, user.id).await, | |
| 236 | + | "sections" => Ok(()), // Sections managed via HTMX API; pass-through | |
| 237 | + | "pricing" => save::save_pricing(&state, &item, &form, user.id).await, | |
| 236 | 238 | "preview" => return save::save_preview(&state, &user, &project, &item, &form).await, | |
| 237 | 239 | _ => return Err(AppError::NotFound), | |
| 240 | + | }; | |
| 241 | + | ||
| 242 | + | if let Err(e) = save_result { | |
| 243 | + | // On an HTMX step submit, surface a validation error as an inline toast | |
| 244 | + | // and tell HTMX NOT to swap (`HX-Reswap: none`) — otherwise the full-page | |
| 245 | + | // 422 error template replaces `#wizard-step` and the user loses everything | |
| 246 | + | // they typed in this step. Mirrors the project wizard (`wizards/project.rs`). | |
| 247 | + | // Non-validation errors and non-HTMX requests fall through to the normal | |
| 248 | + | // error response. | |
| 249 | + | if crate::helpers::is_htmx_request(&headers) | |
| 250 | + | && let AppError::Validation(ref v) = e | |
| 251 | + | { | |
| 252 | + | let mut resp = StatusCode::OK.into_response(); | |
| 253 | + | resp.headers_mut() | |
| 254 | + | .insert("HX-Trigger", crate::helpers::hx_toast(&v.to_string(), "error")); | |
| 255 | + | resp.headers_mut() | |
| 256 | + | .insert("HX-Reswap", HeaderValue::from_static("none")); | |
| 257 | + | return Ok(resp); | |
| 258 | + | } | |
| 259 | + | return Err(e); | |
| 238 | 260 | } | |
| 239 | 261 | ||
| 240 | 262 | // Re-fetch item after update |
| @@ -4,44 +4,69 @@ mod account; | |||
| 4 | 4 | mod links; | |
| 5 | 5 | mod password; | |
| 6 | 6 | ||
| 7 | - | use axum::{routing::get, Router}; | |
| 7 | + | use axum::routing::get; | |
| 8 | 8 | use tower_governor::GovernorLayer; | |
| 9 | 9 | ||
| 10 | - | use crate::{constants, helpers::rate_limiter_ms, AppState}; | |
| 10 | + | use crate::{ | |
| 11 | + | constants, | |
| 12 | + | csrf::{with_csrf, with_csrf_skip, CsrfRouter}, | |
| 13 | + | helpers::rate_limiter_ms, | |
| 14 | + | AppState, | |
| 15 | + | }; | |
| 11 | 16 | ||
| 12 | 17 | /// Register email action routes. | |
| 13 | 18 | /// | |
| 14 | - | /// Every route here is unauthenticated (they're reached from email links or the | |
| 15 | - | /// pre-login forms that trigger them), so the whole router carries one per-IP | |
| 16 | - | /// auth rate limiter applied via `.layer`. Previously only `/forgot-password` | |
| 17 | - | /// was capped; `/reset-password`, `/login-link`, `/verify-email`, | |
| 18 | - | /// `/confirm-delete`, and `/unsubscribe` were uncapped, leaving a | |
| 19 | - | /// DoS/email-amplification surface (Run #11 Security MINOR). The tokens are | |
| 19 | + | /// Returns a `CsrfRouter` so each mutating route MUST declare a CSRF posture — | |
| 20 | + | /// the group can no longer be merged into the page tree as a bare `Router` that | |
| 21 | + | /// silently skips the envelope (the Run #16/#17 CHRONIC: `/forgot-password` | |
| 22 | + | /// rendered a CSRF token it never validated). Postures: | |
| 23 | + | /// - `/forgot-password` — auto-validated. It carries no signed-link token, so | |
| 24 | + | /// the session CSRF token IS its protection. | |
| 25 | + | /// - `/reset-password`, `/confirm-delete`, `/unsubscribe` — CSRF skip: each | |
| 26 | + | /// carries HMAC signed-link fields (user/expires/sig) re-validated in the | |
| 27 | + | /// handler, which an attacker cannot forge. That signature is the CSRF | |
| 28 | + | /// defense; a session token would add nothing for a logged-out email-link | |
| 29 | + | /// flow. | |
| 30 | + | /// - `/verify-email`, `/login-link` — GET-only (read-method, no posture). | |
| 31 | + | /// | |
| 32 | + | /// Every route is unauthenticated (reached from email links or pre-login forms) | |
| 33 | + | /// so the whole router carries one per-IP auth rate limiter. The tokens are | |
| 20 | 34 | /// 256-bit CSPRNG / HMAC so this was never a brute-force risk — the cap closes | |
| 21 | 35 | /// the abuse/amplification angle. Burst 5 + 500ms replenish comfortably covers | |
| 22 | 36 | /// the legitimate forgot -> reset -> login click sequence (~3 requests). | |
| 23 | - | pub fn email_action_routes() -> Router<AppState> { | |
| 37 | + | pub fn email_action_routes() -> CsrfRouter<AppState> { | |
| 24 | 38 | let auth_rate_limit = | |
| 25 | 39 | rate_limiter_ms(constants::AUTH_RATE_LIMIT_MS, constants::AUTH_RATE_LIMIT_BURST); | |
| 26 | 40 | ||
| 27 | - | Router::new() | |
| 41 | + | CsrfRouter::new() | |
| 28 | 42 | .route( | |
| 29 | 43 | "/forgot-password", | |
| 30 | - | get(password::forgot_password_page).post(password::forgot_password_handler), | |
| 44 | + | with_csrf( | |
| 45 | + | get(password::forgot_password_page).post(password::forgot_password_handler), | |
| 46 | + | ), | |
| 31 | 47 | ) | |
| 32 | 48 | .route( | |
| 33 | 49 | "/reset-password", | |
| 34 | - | get(password::reset_password_page).post(password::reset_password_handler), | |
| 50 | + | with_csrf_skip( | |
| 51 | + | "signed link: user/expires/sig HMAC re-validated in handler", | |
| 52 | + | get(password::reset_password_page).post(password::reset_password_handler), | |
| 53 | + | ), | |
| 35 | 54 | ) | |
| 36 | - | .route("/verify-email", get(links::verify_email_handler)) | |
| 37 | - | .route("/login-link", get(links::login_link_handler)) | |
| 55 | + | .route_get("/verify-email", get(links::verify_email_handler)) | |
| 56 | + | .route_get("/login-link", get(links::login_link_handler)) | |
| 38 | 57 | .route( | |
| 39 | 58 | "/confirm-delete", | |
| 40 | - | get(account::confirm_delete_page).post(account::confirm_delete_handler), | |
| 59 | + | with_csrf_skip( | |
| 60 | + | "signed link: user/expires/sig HMAC re-validated in handler", | |
| 61 | + | get(account::confirm_delete_page).post(account::confirm_delete_handler), | |
| 62 | + | ), | |
| 41 | 63 | ) | |
| 42 | 64 | .route( | |
| 43 | 65 | "/unsubscribe", | |
| 44 | - | get(account::unsubscribe_page).post(account::unsubscribe_handler), | |
| 66 | + | with_csrf_skip( | |
| 67 | + | "signed link: sig HMAC verified in handler", | |
| 68 | + | get(account::unsubscribe_page).post(account::unsubscribe_handler), | |
| 69 | + | ), | |
| 45 | 70 | ) | |
| 46 | 71 | .layer(GovernorLayer { | |
| 47 | 72 | config: auth_rate_limit, |