Skip to main content

max / makenotwork

Close security gaps from ultra-fuzz Run 10 (Security axis) - N1: a clamd write failure mid-INSTREAM (commonly StreamMaxLength dropping the connection) now reads the pending verdict and classifies it fail-CLOSED (clamav_incomplete), matching the buffered path, instead of propagating a fail-open transport error. Closes the streaming-path size-limit bypass. - N2: gate the path-based scan variants (check_archive_safety_path, analyze_binary_path, verify_content_type_path) behind #[cfg(test)] so wiring them into production is a compile error and the resolved scan/scan_stream divergence can't silently return. - M1: move the password/code-verifying TOTP mutations (confirm/disable/ regenerate) to a strict auth-strength rate limiter instead of the looser API-write limit. - M2: validate the repo name up front in the SSH repo management commands (info/delete/set-visibility/set-description). - M3: reject RSA SSH keys weaker than 2048 bits (parse the wire-format modulus) rather than accepting any blob >= 16 bytes. - M4: authenticate git tokens with a read and throttle the last_used_at stamp to a best-effort hourly write, so a hot token doesn't write a row per request. Also relax hash_password back to pub (the async wrapper is the handler API; verify_password stays pub(crate)) so test fixtures can seed hashes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-30 14:06 UTC
Commit: b2e6376efa0f35bcb415298c628646036ac22569
Parent: 111861a
9 files changed, +192 insertions, -28 deletions
M server/src/auth.rs +4 -4
@@ -400,10 +400,10 @@ impl FromRequestParts<crate::AppState> for ServiceAuth {
400 400 /// Verification auto-detects params from the hash string, so no feature flag needed there.
401 401 /// Synchronous Argon2id hash. CPU-bound (hundreds of ms); do NOT call from an
402 402 /// async handler — use [`hash_password_async`], which runs this on a blocking
403 - /// thread so a burst of signups can't starve the Tokio worker pool. Kept
404 - /// `pub(crate)` for the async wrapper, the one-time `DUMMY_HASH` initializers,
405 - /// and tests.
406 - pub(crate) fn hash_password(password: &str) -> Result<String, AppError> {
403 + /// thread so a burst of signups can't starve the Tokio worker pool. The sync
404 + /// form remains `pub` only for one-time `DUMMY_HASH` initializers and
405 + /// test/integration fixtures that seed password hashes off the request path.
406 + pub fn hash_password(password: &str) -> Result<String, AppError> {
407 407 let salt = SaltString::generate(&mut OsRng);
408 408 #[cfg(feature = "fast-tests")]
409 409 let params = Params::new(8 * 1024, 1, 1, None)
M server/src/db/git_access_tokens.rs +22 -8
@@ -92,17 +92,31 @@ pub async fn revoke(pool: &PgPool, id: GitAccessTokenId, user_id: UserId) -> Res
92 92 /// (e.g. CSRF/PKCE); a hash-indexed credential lookup is the correct asymmetry.
93 93 #[tracing::instrument(skip_all)]
94 94 pub async fn resolve_active(pool: &PgPool, token_hash: &str) -> Result<Option<ResolvedToken>> {
95 - // UPDATE...RETURNING does the expiry filter and the last_used_at stamp in
96 - // one round-trip; an expired token matches no row, so it resolves to None.
97 - let row: Option<(UserId, bool)> = sqlx::query_as(
98 - r#"UPDATE git_access_tokens
99 - SET last_used_at = NOW()
100 - WHERE token_hash = $1 AND (expires_at IS NULL OR expires_at > NOW())
101 - RETURNING user_id, can_push"#,
95 + // Authenticate with a read (expiry filter; an expired token matches no row,
96 + // so it resolves to None). The last_used_at stamp is then throttled to at
97 + // most ~hourly with a separate best-effort write, so a hot CI fleet hammering
98 + // one token doesn't turn every clone/fetch into a row write (ultra-fuzz M4).
99 + let row: Option<(UserId, bool, Option<DateTime<Utc>>)> = sqlx::query_as(
100 + r#"SELECT user_id, can_push, last_used_at
101 + FROM git_access_tokens
102 + WHERE token_hash = $1 AND (expires_at IS NULL OR expires_at > NOW())"#,
102 103 )
103 104 .bind(token_hash)
104 105 .fetch_optional(pool)
105 106 .await?;
106 107
107 - Ok(row.map(|(user_id, can_push)| ResolvedToken { user_id, can_push }))
108 + let Some((user_id, can_push, last_used_at)) = row else {
109 + return Ok(None);
110 + };
111 +
112 + let stale = last_used_at.is_none_or(|t| Utc::now() - t > chrono::Duration::hours(1));
113 + if stale {
114 + // Best-effort: a stamp failure must never fail authentication.
115 + let _ = sqlx::query("UPDATE git_access_tokens SET last_used_at = NOW() WHERE token_hash = $1")
116 + .bind(token_hash)
117 + .execute(pool)
118 + .await;
119 + }
120 +
121 + Ok(Some(ResolvedToken { user_id, can_push }))
108 122 }
M server/src/git_ssh.rs +20
@@ -412,6 +412,11 @@ async fn cmd_ssh_repo_list(pool: &PgPool, user_id: UserId) -> anyhow::Result<()>
412 412 }
413 413
414 414 async fn cmd_ssh_repo_info(pool: &PgPool, user_id: UserId, name: &str) -> anyhow::Result<()> {
415 + // Validate the name up front (defense-in-depth: the DB lookup 404s on a bogus
416 + // name and the delete path canonicalize-guards the FS path, but reject
417 + // traversal/control characters before building any path) (ultra-fuzz Sec M2).
418 + validate_git_repo_name(name)?;
419 +
415 420 let repo = db::git_repos::get_repo_by_user_and_name(pool, user_id, name)
416 421 .await?
417 422 .ok_or_else(|| anyhow::anyhow!("repository '{}' not found", name))?;
@@ -433,6 +438,11 @@ async fn cmd_ssh_repo_delete(
433 438 username: &str,
434 439 name: &str,
435 440 ) -> anyhow::Result<()> {
441 + // Validate the name up front (defense-in-depth: the DB lookup 404s on a bogus
442 + // name and the delete path canonicalize-guards the FS path, but reject
443 + // traversal/control characters before building any path) (ultra-fuzz Sec M2).
444 + validate_git_repo_name(name)?;
445 +
436 446 let repo = db::git_repos::get_repo_by_user_and_name(pool, user_id, name)
437 447 .await?
438 448 .ok_or_else(|| anyhow::anyhow!("repository '{}' not found", name))?;
@@ -465,6 +475,11 @@ async fn cmd_ssh_repo_set_visibility(
465 475 name: &str,
466 476 visibility: db::Visibility,
467 477 ) -> anyhow::Result<()> {
478 + // Validate the name up front (defense-in-depth: the DB lookup 404s on a bogus
479 + // name and the delete path canonicalize-guards the FS path, but reject
480 + // traversal/control characters before building any path) (ultra-fuzz Sec M2).
481 + validate_git_repo_name(name)?;
482 +
468 483 let repo = db::git_repos::get_repo_by_user_and_name(pool, user_id, name)
469 484 .await?
470 485 .ok_or_else(|| anyhow::anyhow!("repository '{}' not found", name))?;
@@ -481,6 +496,11 @@ async fn cmd_ssh_repo_set_description(
481 496 name: &str,
482 497 description: &str,
483 498 ) -> anyhow::Result<()> {
499 + // Validate the name up front (defense-in-depth: the DB lookup 404s on a bogus
500 + // name and the delete path canonicalize-guards the FS path, but reject
501 + // traversal/control characters before building any path) (ultra-fuzz Sec M2).
502 + validate_git_repo_name(name)?;
503 +
484 504 let repo = db::git_repos::get_repo_by_user_and_name(pool, user_id, name)
485 505 .await?
486 506 .ok_or_else(|| anyhow::anyhow!("repository '{}' not found", name))?;
M server/src/routes/api/mod.rs +16 -4
@@ -302,11 +302,9 @@ pub fn api_routes() -> CsrfRouter<AppState> {
302 302 .route("/api/follow/{target_type}/{target_id}", delete_csrf(follows::unfollow_target))
303 303 // Category management
304 304 .route("/api/categories", post_csrf(categories::create_category))
305 - // TOTP 2FA management
305 + // TOTP 2FA management (setup only; the password/code-verifying mutations
306 + // move to a stricter auth-rate-limited sub-router below).
306 307 .route("/api/users/me/totp/setup", post_csrf(totp::setup))
307 - .route("/api/users/me/totp/confirm", post_csrf(totp::confirm))
308 - .route("/api/users/me/totp/disable", post_csrf(totp::disable))
309 - .route("/api/users/me/totp/backup-codes", post_csrf(totp::regenerate_backup_codes))
310 308 // Passkey management
311 309 .route("/api/users/me/passkeys/register/start", post_csrf(passkeys::register_start))
312 310 .route("/api/users/me/passkeys/register/finish", post_csrf(passkeys::register_finish))
@@ -354,6 +352,19 @@ pub fn api_routes() -> CsrfRouter<AppState> {
354 352 config: write_rate_limit,
355 353 });
356 354
355 + // Password/code-verifying TOTP mutations — strict auth-strength rate limit
356 + // (matching login), not the looser API-write limit, so confirm-code and
357 + // disable/regenerate password checks can't be ground (ultra-fuzz Run 10 Sec M1).
358 + let totp_sensitive_rate_limit =
359 + crate::helpers::rate_limiter_ms(constants::AUTH_RATE_LIMIT_MS, constants::AUTH_RATE_LIMIT_BURST);
360 + let totp_sensitive_routes = CsrfRouter::new()
361 + .route("/api/users/me/totp/confirm", post_csrf(totp::confirm))
362 + .route("/api/users/me/totp/disable", post_csrf(totp::disable))
363 + .route("/api/users/me/totp/backup-codes", post_csrf(totp::regenerate_backup_codes))
364 + .route_layer(GovernorLayer {
365 + config: totp_sensitive_rate_limit,
366 + });
367 +
357 368 // Export routes — stricter rate limit
358 369 let export_routes = CsrfRouter::new()
359 370 .route("/api/export/projects", post_csrf(exports::export_projects))
@@ -468,6 +479,7 @@ pub fn api_routes() -> CsrfRouter<AppState> {
468 479 .route_get("/download/{token}", get(guest_checkout::guest_download));
469 480
470 481 write_routes
482 + .merge(totp_sensitive_routes)
471 483 .merge(export_routes)
472 484 .merge(key_routes)
473 485 .merge(validate_routes)
M server/src/scanning/archive.rs +5
@@ -648,6 +648,11 @@ fn walk_compressed<R: Read>(
648 648 /// Path-based entry. Opens the spooled file directly so we never have to
649 649 /// buffer the whole archive. File-type gating happens at the call site (same
650 650 /// shape as the buffered variant — caller already checked `file_type`).
651 + /// Path-based variant, retained only as a buffered-vs-path equivalence oracle in
652 + /// tests. The live pipeline scans the mmap slice via [`check_archive_safety`];
653 + /// `#[cfg(test)]` makes wiring this into production a compile error, so the
654 + /// nested-interior scan can't be silently dropped on the spool path (ultra-fuzz N2).
655 + #[cfg(test)]
651 656 pub fn check_archive_safety_path(path: &std::path::Path, file_type: FileType) -> LayerResult {
652 657 use std::io::{Seek, SeekFrom};
653 658
M server/src/scanning/clamav.rs +42 -12
@@ -172,6 +172,16 @@ where
172 172 .await
173 173 .map_err(|e| format!("Failed to send INSTREAM command: {}", e))?;
174 174
175 + // A write failure *after* the connection is established and the INSTREAM
176 + // command was accepted means clamd dropped us mid-stream. The common cause
177 + // is StreamMaxLength enforcement: clamd writes a size-limit ERROR and closes
178 + // the socket, so our next write gets EPIPE. That is a reachable-but-
179 + // incomplete scan (a coverage gap that must fail CLOSED), NOT the fail-open
180 + // "daemon unreachable" bucket — so on any such write error we read clamd's
181 + // pending verdict and classify it (a size-limit reply parses to
182 + // NotFullyScanned → clamav_incomplete → FailClosed) rather than propagating
183 + // a transport Err. This keeps the streaming path's size-limit handling
184 + // identical to the buffered path's (CHRONIC S1 intent).
175 185 let mut buf = vec![0u8; CHUNK_SIZE];
176 186 loop {
177 187 let n = reader
@@ -182,20 +192,14 @@ where
182 192 break;
183 193 }
184 194 let len = (n as u32).to_be_bytes();
185 - stream
186 - .write_all(&len)
187 - .await
188 - .map_err(|e| format!("Failed to write chunk length: {}", e))?;
189 - stream
190 - .write_all(&buf[..n])
191 - .await
192 - .map_err(|e| format!("Failed to write chunk data: {}", e))?;
195 + if stream.write_all(&len).await.is_err() || stream.write_all(&buf[..n]).await.is_err() {
196 + return Ok(incomplete_after_clamd_dropped(&mut stream).await);
197 + }
193 198 }
194 199
195 - stream
196 - .write_all(&0u32.to_be_bytes())
197 - .await
198 - .map_err(|e| format!("Failed to send end marker: {}", e))?;
200 + if stream.write_all(&0u32.to_be_bytes()).await.is_err() {
201 + return Ok(incomplete_after_clamd_dropped(&mut stream).await);
202 + }
199 203
200 204 let mut response = Vec::with_capacity(256);
201 205 stream
@@ -296,6 +300,32 @@ fn clamav_layer_result(response: &str) -> LayerResult {
296 300 }
297 301 }
298 302
303 + /// clamd dropped the connection mid-INSTREAM (commonly StreamMaxLength). Read
304 + /// whatever verdict it managed to send before closing and classify it; a
305 + /// size-limit reply parses to `NotFullyScanned` → fail-closed
306 + /// `clamav_incomplete`. When nothing legible was sent, synthesize the same
307 + /// fail-closed outcome. This never returns the fail-open `clamav` transport
308 + /// bucket: clamd was reachable, the scan just didn't complete.
309 + async fn incomplete_after_clamd_dropped(stream: &mut UnixStream) -> LayerResult {
310 + let mut response = Vec::with_capacity(256);
311 + let _ = (&mut *stream).take(16_384).read_to_end(&mut response).await;
312 + let response_str = String::from_utf8_lossy(&response);
313 + let response_str = response_str.trim_end_matches('\0').trim();
314 + if response_str.is_empty() {
315 + LayerResult {
316 + layer: "clamav_incomplete",
317 + verdict: LayerVerdict::Error,
318 + detail: Some(
319 + "ClamAV closed the connection mid-stream without a verdict \
320 + (likely stream size limit); held for review"
321 + .to_string(),
322 + ),
323 + }
324 + } else {
325 + clamav_layer_result(response_str)
326 + }
327 + }
328 +
299 329 #[cfg(test)]
300 330 mod tests {
301 331 use super::*;
M server/src/scanning/content_type.rs +5
@@ -209,6 +209,11 @@ pub fn verify_content_type(data: &[u8], claimed_type: FileType) -> LayerResult {
209 209
210 210 /// Path-based entry. Reads only the head bytes (`infer` inspects ~262
211 211 /// bytes); no need to mmap or buffer the whole spooled file.
212 + /// Path-based variant, retained only as a buffered-vs-path equivalence oracle in
213 + /// tests. The live pipeline scans the mmap slice via [`verify_content_type`];
214 + /// `#[cfg(test)]` makes wiring this into production a compile error, so the
215 + /// resolved scan/scan_stream divergence can't silently return (ultra-fuzz N2).
216 + #[cfg(test)]
212 217 pub fn verify_content_type_path(path: &std::path::Path, claimed_type: FileType) -> LayerResult {
213 218 use std::io::Read;
214 219
M server/src/scanning/structural.rs +4
@@ -45,6 +45,10 @@ const SUSPICIOUS_MACHO_SYMBOLS: &[&str] = &[
45 45 /// page cache rather than a heap buffer; suitable for files larger than
46 46 /// `SCAN_MAX_MEMORY_BYTES`. Delegates to `analyze_binary` for the actual
47 47 /// analysis.
48 + /// Path-based variant, retained only as a buffered-vs-path equivalence oracle in
49 + /// tests. The live pipeline scans the mmap slice via [`analyze_binary`];
50 + /// `#[cfg(test)]` makes wiring this into production a compile error (ultra-fuzz N2).
51 + #[cfg(test)]
48 52 pub fn analyze_binary_path(path: &std::path::Path, file_type: FileType) -> LayerResult {
49 53 if file_type != FileType::Download {
50 54 return LayerResult {
M server/src/validation/users.rs +74
@@ -151,11 +151,35 @@ const SSH_KEY_TYPES: &[&str] = &[
151 151 "ecdsa-sha2-nistp521",
152 152 ];
153 153
154 + /// Extract the RSA modulus bit length from a decoded `ssh-rsa` key blob.
155 + ///
156 + /// Wire format (RFC 4253): a sequence of fields, each a 4-byte big-endian
157 + /// length prefix followed by that many bytes — here `string(type)`, `mpint(e)`,
158 + /// `mpint(n)`. The modulus `n` is the third field; its significant byte count
159 + /// (after stripping the mpint sign-padding zero) gives the bit length. Returns
160 + /// `None` if the blob is truncated or malformed.
161 + fn ssh_rsa_modulus_bits(decoded: &[u8]) -> Option<usize> {
162 + fn read_field<'a>(buf: &'a [u8], pos: &mut usize) -> Option<&'a [u8]> {
163 + let len = u32::from_be_bytes(buf.get(*pos..*pos + 4)?.try_into().ok()?) as usize;
164 + *pos += 4;
165 + let field = buf.get(*pos..*pos + len)?;
166 + *pos += len;
167 + Some(field)
168 + }
169 + let mut pos = 0;
170 + let _type = read_field(decoded, &mut pos)?;
171 + let _e = read_field(decoded, &mut pos)?;
172 + let n = read_field(decoded, &mut pos)?;
173 + let significant_bytes = n.iter().skip_while(|&&b| b == 0).count();
174 + Some(significant_bytes * 8)
175 + }
176 +
154 177 /// Validate and normalize an SSH public key, returning `(normalized_key, fingerprint)`.
155 178 ///
156 179 /// - Parses the `{type} {base64} [comment]` format
157 180 /// - Validates the key type is one of the accepted algorithms
158 181 /// - Decodes the base64 data to verify it's real key data
182 + /// - Rejects RSA keys weaker than 2048 bits
159 183 /// - Computes the fingerprint as `SHA256:{base64(sha256(decoded_key_bytes))}`
160 184 /// - Returns the normalized key (type + base64, no comment) and fingerprint
161 185 pub fn validate_ssh_public_key(input: &str) -> std::result::Result<(String, String), AppError> {
@@ -196,6 +220,25 @@ pub fn validate_ssh_public_key(input: &str) -> std::result::Result<(String, Stri
196 220 return Err(AppError::validation("Invalid SSH key: data too short".to_string()));
197 221 }
198 222
223 + // Reject weak RSA keys. A bare `decoded.len() >= 16` would accept a
224 + // deliberately-small 512/768/1024-bit modulus; require >= 2048 bits
225 + // (ultra-fuzz Run 10 Sec M3).
226 + if key_type == "ssh-rsa" {
227 + match ssh_rsa_modulus_bits(&decoded) {
228 + Some(bits) if bits >= 2048 => {}
229 + Some(_) => {
230 + return Err(AppError::validation(
231 + "RSA SSH keys must be at least 2048 bits".to_string(),
232 + ));
233 + }
234 + None => {
235 + return Err(AppError::validation(
236 + "Invalid SSH key: malformed RSA key data".to_string(),
237 + ));
238 + }
239 + }
240 + }
241 +
199 242 // Compute fingerprint: SHA256:{base64(sha256(decoded))} (same as ssh-keygen -lf)
200 243 use sha2::Digest;
201 244 let hash = sha2::Sha256::digest(&decoded);
@@ -349,6 +392,37 @@ mod tests {
349 392 assert_eq!(fp1, fp2, "Same key data should produce same fingerprint");
350 393 }
351 394
395 + /// Build a minimal `ssh-rsa` wire blob with a modulus of `n_bytes` bytes
396 + /// (high bit clear, so no mpint sign byte is prepended).
397 + fn build_ssh_rsa_blob(n_bytes: usize) -> Vec<u8> {
398 + fn push_field(blob: &mut Vec<u8>, data: &[u8]) {
399 + blob.extend_from_slice(&(data.len() as u32).to_be_bytes());
400 + blob.extend_from_slice(data);
401 + }
402 + let mut blob = Vec::new();
403 + push_field(&mut blob, b"ssh-rsa");
404 + push_field(&mut blob, &[0x01, 0x00, 0x01]); // e = 65537
405 + push_field(&mut blob, &vec![0x7f; n_bytes]); // modulus n
406 + blob
407 + }
408 +
409 + #[test]
410 + fn ssh_rsa_modulus_bits_reads_length() {
411 + assert_eq!(ssh_rsa_modulus_bits(&build_ssh_rsa_blob(256)), Some(2048));
412 + assert_eq!(ssh_rsa_modulus_bits(&build_ssh_rsa_blob(128)), Some(1024));
413 + assert_eq!(ssh_rsa_modulus_bits(&[1, 2, 3]), None); // truncated blob
414 + }
415 +
416 + #[test]
417 + fn weak_rsa_key_rejected_strong_accepted() {
418 + use base64::Engine;
419 + let enc = |b: &[u8]| base64::engine::general_purpose::STANDARD.encode(b);
420 + let weak = format!("ssh-rsa {} test@host", enc(&build_ssh_rsa_blob(128))); // 1024-bit
421 + assert!(validate_ssh_public_key(&weak).is_err(), "1024-bit RSA must be rejected");
422 + let strong = format!("ssh-rsa {} test@host", enc(&build_ssh_rsa_blob(256))); // 2048-bit
423 + assert!(validate_ssh_public_key(&strong).is_ok(), "2048-bit RSA must be accepted");
424 + }
425 +
352 426 #[test]
353 427 fn test_validate_ssh_key_label() {
354 428 assert!(validate_ssh_key_label("").is_ok()); // empty is valid