Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand ID range to harden brute forcing #71

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 20 additions & 2 deletions src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
22 changes: 11 additions & 11 deletions src/db.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -23,7 +23,7 @@ static MIGRATIONS: LazyLock<Migrations> = LazyLock::new(|| {

let rows = stmt
.query_map([], |row| Ok((row.get(0)?, row.get(1)?)))?
.collect::<Result<Vec<(u32, String)>, _>>()?;
.collect::<Result<Vec<(Inner, String)>, _>>()?;

tracing::debug!("compressing {} rows", rows.len());

Expand Down Expand Up @@ -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,
Expand All @@ -291,12 +291,12 @@ impl Database {
/// Get entire entry for `id`.
pub async fn get(&self, id: Id, password: Option<Password>) -> Result<read::Entry, Error> {
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)?,
Expand All @@ -323,12 +323,12 @@ impl Database {
/// expired or does not exist.
pub async fn get_uid(&self, id: Id) -> Result<Option<i64>, 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<i64> = row.get(0)?;
let expired: Option<bool> = row.get(1)?;
Expand All @@ -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??;

Expand Down
129 changes: 95 additions & 34 deletions src/id.rs
Original file line number Diff line number Diff line change
@@ -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;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stupid question: what's the point of this type alias?

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
}

Expand All @@ -33,41 +32,74 @@ 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)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

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}")
}
}

impl FromStr for Id {
type Err = Error;

fn from_str(value: &str) -> Result<Self, Self::Err> {
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<Inner> = 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<u32> = 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<Inner> = 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;
}
}
}
Expand All @@ -77,32 +109,61 @@ impl FromStr for Id {
}
}

impl From<u32> for Id {
fn from(n: u32) -> Self {
impl From<Inner> 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());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this outcommented? Also, please use proper Rust comments, i.e. // instead of /* */.

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());
}
}
4 changes: 2 additions & 2 deletions src/routes/form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<u32>()
rng.gen::<Inner>()
})
.await
.map_err(Error::from)?
Expand Down
4 changes: 2 additions & 2 deletions src/routes/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -45,7 +45,7 @@ pub async fn insert(
) -> Result<Json<RedirectResponse>, JsonErrorResponse> {
let id: Id = tokio::task::spawn_blocking(|| {
let mut rng = rand::thread_rng();
rng.gen::<u32>()
rng.gen::<Inner>()
})
.await
.map_err(Error::from)?
Expand Down
2 changes: 1 addition & 1 deletion src/routes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ mod tests {
async fn unknown_paste() -> Result<(), Box<dyn std::error::Error>> {
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(())
Expand Down
Loading