From 363988d581a457b31ada173ce9e5002373cfe225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Sun, 22 Sep 2024 12:21:19 +0200 Subject: [PATCH] Expand ID range to harden brute forcing Expand the domain for IDs from 2^32 to 2^64, resulting in keys of length 11 instead of 6. In particular important for one-time-pastes. i64 is used as inner type since sqlite does not support storing u64. --- CHANGELOG.md | 5 ++ src/cache.rs | 22 +++++++- src/db.rs | 22 ++++---- src/id.rs | 129 +++++++++++++++++++++++++++++++++------------ src/routes/form.rs | 4 +- src/routes/json.rs | 4 +- src/routes/mod.rs | 2 +- 7 files changed, 136 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53708ce..da21b00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- **Breaking**: Expand the domain for IDs from 2^32 to 2^64, + resulting in keys of length 11 instead of 6. + Old 6 character keys are still accepted for existing links. + Existing Database entries are not affected. + ## 2.7.0 diff --git a/src/cache.rs b/src/cache.rs index a69871c..47dacf8 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -60,16 +60,34 @@ mod tests { #[test] fn cache_key() { + /* + * Support ID generated in the old 32-bit format + */ + let key = Key::from_str("bJZCna").unwrap(); - assert_eq!(key.id(), "bJZCna"); + assert_eq!(key.id(), "aaaaaay83de"); assert_eq!(key.id, 104651828.into()); assert_eq!(key.ext, "txt"); let key = Key::from_str("sIiFec.rs").unwrap(); - assert_eq!(key.id(), "sIiFec"); + assert_eq!(key.id(), "aaaaaeOIhXc"); assert_eq!(key.id, 1243750162.into()); assert_eq!(key.ext, "rs"); + /* + * Support new 64-bit format + */ + + let key = Key::from_str("bJZCna1237p").unwrap(); + assert_eq!(key.id(), "bJZCna1237p"); + assert_eq!(key.id, 449476178952511423.into()); + assert_eq!(key.ext, "txt"); + + let key = Key::from_str("-IiFec1237p.rs").unwrap(); + assert_eq!(key.id(), "-IiFec1237p"); + assert_eq!(key.id, (-422741260676702273).into()); + assert_eq!(key.ext, "rs"); + assert!(Key::from_str("foo").is_err()); assert!(Key::from_str("bar.rs").is_err()); } diff --git a/src/db.rs b/src/db.rs index 044a4be..5ff460f 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1,6 +1,6 @@ use crate::crypto::Password; use crate::errors::Error; -use crate::id::Id; +use crate::id::{Id, Inner}; use parking_lot::Mutex; use rusqlite::{params, Connection, Transaction}; use rusqlite_migration::{HookError, Migrations, M}; @@ -23,7 +23,7 @@ static MIGRATIONS: LazyLock = LazyLock::new(|| { let rows = stmt .query_map([], |row| Ok((row.get(0)?, row.get(1)?)))? - .collect::, _>>()?; + .collect::, _>>()?; tracing::debug!("compressing {} rows", rows.len()); @@ -262,18 +262,18 @@ impl Database { /// Insert `entry` under `id` into the database and optionally set owner to `uid`. pub async fn insert(&self, id: Id, entry: write::Entry) -> Result<(), Error> { let conn = self.conn.clone(); - let id = id.as_u32(); + let id_inner = id.as_inner(); let write::DatabaseEntry { entry, data, nonce } = entry.compress().await?.encrypt().await?; spawn_blocking(move || match entry.expires { None => conn.lock().execute( "INSERT INTO entries (id, uid, data, burn_after_reading, nonce, title) VALUES (?1, ?2, ?3, ?4, ?5, ?6)", - params![id, entry.uid, data, entry.burn_after_reading, nonce, entry.title], + params![id_inner, entry.uid, data, entry.burn_after_reading, nonce, entry.title], ), Some(expires) => conn.lock().execute( "INSERT INTO entries (id, uid, data, burn_after_reading, nonce, expires, title) VALUES (?1, ?2, ?3, ?4, ?5, datetime('now', ?6), ?7)", params![ - id, + id_inner, entry.uid, data, entry.burn_after_reading, @@ -291,12 +291,12 @@ impl Database { /// Get entire entry for `id`. pub async fn get(&self, id: Id, password: Option) -> Result { let conn = self.conn.clone(); - let id_as_u32 = id.as_u32(); + let id_inner = id.as_inner(); let entry = spawn_blocking(move || { conn.lock().query_row( "SELECT data, burn_after_reading, uid, nonce, expires < datetime('now'), title FROM entries WHERE id=?1", - params![id_as_u32], + params![id_inner], |row| { Ok(read::DatabaseEntry { data: row.get(0)?, @@ -323,12 +323,12 @@ impl Database { /// expired or does not exist. pub async fn get_uid(&self, id: Id) -> Result, Error> { let conn = self.conn.clone(); - let id_as_u32 = id.as_u32(); + let id_inner = id.as_inner(); let (uid, expired) = spawn_blocking(move || { conn.lock().query_row( "SELECT uid, expires < datetime('now') FROM entries WHERE id=?1", - params![id_as_u32], + params![id_inner], |row| { let uid: Option = row.get(0)?; let expired: Option = row.get(1)?; @@ -349,11 +349,11 @@ impl Database { /// Delete `id`. pub async fn delete(&self, id: Id) -> Result<(), Error> { let conn = self.conn.clone(); - let id = id.as_u32(); + let id_inner = id.as_inner(); spawn_blocking(move || { conn.lock() - .execute("DELETE FROM entries WHERE id=?1", params![id]) + .execute("DELETE FROM entries WHERE id=?1", params![id_inner]) }) .await??; diff --git a/src/id.rs b/src/id.rs index 9df9a97..5d6ce0e 100644 --- a/src/id.rs +++ b/src/id.rs @@ -1,24 +1,23 @@ use crate::db::write::Entry; use crate::errors::Error; +use core::str; use std::fmt; use std::str::FromStr; -const CHAR_TABLE: &[char; 64] = &[ - 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', - 't', 'u', 'v', 'w', 'x', 'y', 'z', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', - 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '0', '1', '2', '3', '4', - '5', '6', '7', '8', '9', '-', '+', -]; +const CHAR_TABLE: &[u8; 64] = b"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-+"; -/// Represents a 32-bit integer either numerically or mapped to a 6 character string. +pub type Inner = i64; +const ID_LENGTH: usize = 11; + +/// Represents a 64-bit integer either numerically or mapped to a 11 character string. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct Id { - n: u32, + n: Inner, } impl Id { /// Return the value itself. - pub fn as_u32(self) -> u32 { + pub fn as_inner(self) -> Inner { self.n } @@ -33,16 +32,25 @@ impl Id { impl fmt::Display for Id { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut s = String::with_capacity(6); - - s.push(CHAR_TABLE[((self.n >> 26) & 0x3f) as usize]); - s.push(CHAR_TABLE[((self.n >> 20) & 0x3f) as usize]); - s.push(CHAR_TABLE[((self.n >> 14) & 0x3f) as usize]); - s.push(CHAR_TABLE[((self.n >> 8) & 0x3f) as usize]); - s.push(CHAR_TABLE[((self.n >> 2) & 0x3f) as usize]); - s.push(CHAR_TABLE[(self.n & 0x3) as usize]); - - write!(f, "{s}") + #[allow(clippy::cast_sign_loss)] + let buf: [u8; ID_LENGTH] = [ + CHAR_TABLE[((self.n >> 58) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 52) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 46) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 40) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 34) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 28) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 22) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 16) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 10) & 0x3f) as usize], + CHAR_TABLE[((self.n >> 4) & 0x3f) as usize], + CHAR_TABLE[(self.n & 0xf) as usize], + ]; + + let str = str::from_utf8(&buf).expect("characters are valid UTF-8"); + debug_assert!(str.len() == ID_LENGTH); + + write!(f, "{str}") } } @@ -50,24 +58,48 @@ impl FromStr for Id { type Err = Error; fn from_str(value: &str) -> Result { - if value.len() != 6 { + /* Support ID generated in the old 32-bit format */ + if value.len() == 6 { + let mut n: Inner = 0; + + for (pos, char) in value.as_bytes().iter().enumerate() { + let bits: Option = CHAR_TABLE.iter().enumerate().find_map(|(bits, c)| { + (char == c).then(|| bits.try_into().expect("bits not 64 bits")) + }); + + match bits { + None => return Err(Error::IllegalCharacters), + Some(bits) => { + if pos < 5 { + n = (n << 6) | bits; + } else { + n = (n << 2) | bits; + } + } + } + } + + return Ok(Self { n }); + } + + if value.len() != ID_LENGTH { return Err(Error::WrongSize); } - let mut n: u32 = 0; + let mut n: Inner = 0; - for (pos, char) in value.chars().enumerate() { - let bits: Option = CHAR_TABLE.iter().enumerate().find_map(|(bits, c)| { - (char == *c).then(|| bits.try_into().expect("bits not 32 bits")) + for (pos, char) in value.as_bytes().iter().enumerate() { + let bits: Option = CHAR_TABLE.iter().enumerate().find_map(|(bits, c)| { + (char == c).then(|| bits.try_into().expect("bits not 64 bits")) }); match bits { None => return Err(Error::IllegalCharacters), Some(bits) => { - if pos < 5 { + if pos < ID_LENGTH - 1 { n = (n << 6) | bits; } else { - n = (n << 2) | bits; + n = (n << 4) | bits; } } } @@ -77,32 +109,61 @@ impl FromStr for Id { } } -impl From for Id { - fn from(n: u32) -> Self { +impl From for Id { + fn from(n: Inner) -> Self { Self { n } } } #[cfg(test)] mod tests { + use std::i64; + use super::*; #[test] - fn convert_u32_to_id_and_back() { + fn convert_inner_to_id_and_back() { let id = Id::from(0); - assert_eq!(id.to_string(), "aaaaaa"); - assert_eq!(id.as_u32(), 0); + assert_eq!(id.to_string(), "aaaaaaaaaaa"); + assert_eq!(id.as_inner(), 0); + assert_eq!(Id::from_str(&id.to_string()).unwrap(), id); + + let id = Id::from(-1); + assert_eq!(id.to_string(), "++++++++++p"); + assert_eq!(id.as_inner(), -1); + assert_eq!(Id::from_str(&id.to_string()).unwrap(), id); let id = Id::from(0xffffffff); - assert_eq!(id.to_string(), "+++++d"); - assert_eq!(id.as_u32(), 0xffffffff); + assert_eq!(id.to_string(), "aaaaap++++p"); + assert_eq!(id.as_inner(), 0xffffffff); + assert_eq!(Id::from_str(&id.to_string()).unwrap(), id); + + let id = Id::from(i64::MAX); + assert_eq!(id.to_string(), "F+++++++++p"); + assert_eq!(id.as_inner(), i64::MAX); + assert_eq!(Id::from_str(&id.to_string()).unwrap(), id); + + let id = Id::from(i64::MIN); + assert_eq!(id.to_string(), "Gaaaaaaaaaa"); + assert_eq!(id.as_inner(), i64::MIN); + assert_eq!(Id::from_str(&id.to_string()).unwrap(), id); } #[test] fn convert_id_from_string() { - assert!(Id::from_str("abDE+-").is_ok()); + /* Support ID generated in the old 32-bit format */ + //assert!(Id::from_str("abDE+-").is_ok()); assert!(Id::from_str("#bDE+-").is_err()); assert!(Id::from_str("abDE+-1").is_err()); assert!(Id::from_str("abDE+").is_err()); + + /* New 64-bit format */ + assert_eq!( + Id::from_str("abDE+-12345").unwrap(), + Id::from(6578377758007225) + ); + assert!(Id::from_str("#bDE+-12345").is_err()); + assert!(Id::from_str("abDE+-123456").is_err()); + assert!(Id::from_str("abDE+12345").is_err()); } } diff --git a/src/routes/form.rs b/src/routes/form.rs index 846759e..0abad88 100644 --- a/src/routes/form.rs +++ b/src/routes/form.rs @@ -2,7 +2,7 @@ use std::num::NonZeroU32; use crate::db::write; use crate::env::BASE_PATH; -use crate::id::Id; +use crate::id::{Id, Inner}; use crate::{pages, AppState, Error}; use axum::extract::{Form, State}; use axum::response::Redirect; @@ -50,7 +50,7 @@ pub async fn insert( ) -> Result<(SignedCookieJar, Redirect), pages::ErrorResponse<'static>> { let id: Id = tokio::task::spawn_blocking(|| { let mut rng = rand::thread_rng(); - rng.gen::() + rng.gen::() }) .await .map_err(Error::from)? diff --git a/src/routes/json.rs b/src/routes/json.rs index 698e02a..68d1fe4 100644 --- a/src/routes/json.rs +++ b/src/routes/json.rs @@ -3,7 +3,7 @@ use std::num::NonZeroU32; use crate::db::write; use crate::env::BASE_PATH; use crate::errors::{Error, JsonErrorResponse}; -use crate::id::Id; +use crate::id::{Id, Inner}; use crate::AppState; use axum::extract::State; use axum::Json; @@ -45,7 +45,7 @@ pub async fn insert( ) -> Result, JsonErrorResponse> { let id: Id = tokio::task::spawn_blocking(|| { let mut rng = rand::thread_rng(); - rng.gen::() + rng.gen::() }) .await .map_err(Error::from)? diff --git a/src/routes/mod.rs b/src/routes/mod.rs index 387d05e..f07aeff 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -39,7 +39,7 @@ mod tests { async fn unknown_paste() -> Result<(), Box> { let client = Client::new(make_app()?).await; - let res = client.get(&BASE_PATH.join("000000")).send().await?; + let res = client.get(&BASE_PATH.join("00000000000")).send().await?; assert_eq!(res.status(), StatusCode::NOT_FOUND); Ok(())