Skip to content

Commit

Permalink
Improve session ID (#141)
Browse files Browse the repository at this point in the history
* Exchange UUID for nanoid

UUIDv4 has 122 random bits, with 6 fixed bits for identifying the
format. Additionally, it uses hexadecimal encoding, which makes it take
more bandwidth to transfer.

This patch replaces UUIDv4 with an array of bytes derived from the
nanoid crate, which gives 66 bits of entropy. This exceeds the minimum
recommended by OWASP. Additionally, the byte array is encoded as
URL-safe Base64 for usage as a string.

* Switch to Rand/Base64 crates

Improves memory use slightly, as well as type safety.

* Correct documentation typo
  • Loading branch information
nul-reference committed Jan 22, 2024
1 parent 9dea07b commit 7631331
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 44 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ tokio = { version = "1.32.0", features = ["full"] }
tokio-test = "0.4.3"
tower = "0.4.13"
tower-cookies = "0.10.0"
uuid = { version = "1.4.1", features = ["v4", "serde"] }
http-body-util = "0.1"

[[example]]
Expand Down
13 changes: 6 additions & 7 deletions examples/strongly-typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ use axum::{extract::FromRequestParts, response::IntoResponse, routing::get, Rout
use http::{request::Parts, StatusCode};
use serde::{Deserialize, Serialize};
use time::{Duration, OffsetDateTime};
use tower_sessions::{Expiry, MemoryStore, Session, SessionManagerLayer};
use uuid::Uuid;
use tower_sessions::{session::Id, Expiry, MemoryStore, Session, SessionManagerLayer};

#[derive(Clone, Deserialize, Serialize)]
struct GuestData {
id: Uuid,
id: Id,
pageviews: usize,
first_seen: OffsetDateTime,
last_seen: OffsetDateTime,
Expand All @@ -19,7 +18,7 @@ struct GuestData {
impl Default for GuestData {
fn default() -> Self {
Self {
id: Uuid::new_v4(),
id: Id::default(),
pageviews: 0,
first_seen: OffsetDateTime::now_utc(),
last_seen: OffsetDateTime::now_utc(),
Expand All @@ -35,8 +34,8 @@ struct Guest {
impl Guest {
const GUEST_DATA_KEY: &'static str = "guest.data";

fn id(&self) -> Uuid {
self.guest_data.id
fn id(&self) -> &Id {
&self.guest_data.id
}

fn first_seen(&self) -> OffsetDateTime {
Expand Down Expand Up @@ -70,7 +69,7 @@ impl Display for Guest {
write!(
f,
"Guest ID {}\n\nPageviews {}\n\nFirst seen {} ago\n\nLast seen {} ago\n\n",
self.id().as_hyphenated(),
self.id(),
self.pageviews(),
now - self.first_seen(),
now - self.last_seen()
Expand Down
2 changes: 1 addition & 1 deletion mongodb-store/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ repository.workspace = true

[dependencies]
async-trait = { workspace = true }
bson = { version = "2.7.0", features = ["time-0_3", "uuid-1"] }
bson = { version = "2.7.0", features = ["time-0_3"] }
mongodb = { version = "2.7.0" }
rmp-serde = { workspace = true }
serde = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion sqlx-store/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mysql = ["sqlx/mysql"]
[dependencies]
async-trait = { workspace = true }
rmp-serde = { workspace = true }
sqlx = { version = "0.7.2", features = ["time", "uuid", "runtime-tokio"] }
sqlx = { version = "0.7.2", features = ["time", "runtime-tokio"] }
thiserror = { workspace = true }
time = { workspace = true }
tower-sessions-core = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion sqlx-store/src/mysql_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl MySqlStore {
r#"
create table if not exists `{schema_name}`.`{table_name}`
(
id char(36) primary key not null,
id char(22) primary key not null,
data blob not null,
expiry_date timestamp(6) not null
)
Expand Down
32 changes: 13 additions & 19 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,10 @@
//! # use http::{request::Parts, StatusCode};
//! # use serde::{Deserialize, Serialize};
//! # use time::OffsetDateTime;
//! # use tower_sessions::{SessionStore, Session};
//! # use uuid::Uuid;
//! # use tower_sessions::{session::Id, SessionStore, Session};
//! #[derive(Clone, Deserialize, Serialize)]
//! struct GuestData {
//! id: Uuid,
//! id: Id,
//! pageviews: usize,
//! first_seen: OffsetDateTime,
//! last_seen: OffsetDateTime,
Expand All @@ -162,7 +161,7 @@
//! impl Default for GuestData {
//! fn default() -> Self {
//! Self {
//! id: Uuid::new_v4(),
//! id: Id::default(),
//! pageviews: 0,
//! first_seen: OffsetDateTime::now_utc(),
//! last_seen: OffsetDateTime::now_utc(),
Expand All @@ -178,8 +177,8 @@
//! impl Guest {
//! const GUEST_DATA_KEY: &'static str = "guest_data";
//!
//! fn id(&self) -> Uuid {
//! self.guest_data.id
//! fn id(&self) -> &Id {
//! &self.guest_data.id
//! }
//!
//! fn first_seen(&self) -> OffsetDateTime {
Expand Down Expand Up @@ -324,18 +323,17 @@
//!
//! Sessions manifest to clients as cookies. These cookies have a configurable
//! name and a value that is the session ID. In other words, cookies hold a
//! pointer to the session in the form of an ID. This ID is a [UUID
//! v4](https://docs.rs/uuid/latest/uuid/struct.Uuid.html#method.new_v4).
//! pointer to the session in the form of an ID. This ID is an i128 generated by
//! the [`rand`](https://docs.rs/rand/latest/rand) crate.
//!
//! ### Secure nature of cookies
//!
//! Session IDs are considered secure if your platform's
//! [`getrandom`](https://docs.rs/getrandom/latest/getrandom/) is
//! secure[^getrandom], and therefore are not signed or encrypted. Note that
//! this assumption is predicated on the secure nature of the UUID crate and its
//! ability to generate securely-random values. It's also important to note that
//! session cookies **must never** be sent over a public, insecure channel.
//! Doing so is **not** secure.
//! Session IDs are considered secure if sent over encrypted channels, and
//! therefore are not signed or encrypted. Note that this assumption is predicated
//! on the secure nature of the [`rand`](https://docs.rs/rand/latest/rand) crate
//! and its ability to generate securely-random values using the ChaCha block cipher
//! with 12 rounds. It's also important to note that session cookies **must never**
//! be sent over a public, insecure channel. Doing so is **not** secure.
//!
//! ## Key-value API
//!
Expand Down Expand Up @@ -400,10 +398,6 @@
//! Sessions also carry with them a configurable expiry and will be deleted in
//! accordance with this.
//!
//! [^getrandom]: `uuid` uses `getrandom` which varies by platform; the crucial
//! assumption `tower-sessions` makes is that your platform is secure.
//! However, you **must** verify this for yourself.
//!
//! [^json]: Using JSON allows us to translate arbitrary types to virtually
//! any backend and gives us a nice interface with which to interact with the
//! session.
Expand Down
7 changes: 2 additions & 5 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ macro_rules! route_tests {

#[tokio::test]
async fn bogus_session_cookie() {
let session_cookie = Cookie::new("id", "00000000-0000-0000-0000-000000000000");
let session_cookie = Cookie::new("id", "AAAAAAAAAAAAAAAAAAAAAA");
let req = Request::builder()
.uri("/insert")
.header(header::COOKIE, session_cookie.encoded().to_string())
Expand All @@ -122,10 +122,7 @@ macro_rules! route_tests {
let session_cookie = get_session_cookie(res.headers()).unwrap();

assert_eq!(res.status(), StatusCode::OK);
assert_ne!(
session_cookie.value(),
"00000000-0000-0000-0000-000000000000"
);
assert_ne!(session_cookie.value(), "AAAAAAAAAAAAAAAAAAAAAA");
}

#[tokio::test]
Expand Down
3 changes: 2 additions & 1 deletion tower-sessions-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ deletion-task = ["tokio/time"]
[dependencies]
async-trait = "0.1.73"
axum-core = { version = "0.4.0", optional = true }
base64 = "0.21.7"
futures = { version = "0.3.28", default-features = false, features = [
"async-await",
] }
http = "1.0"
parking_lot = { version = "0.12.1", features = ["serde"] }
rand = "0.8.5"
serde = { version = "1.0.189", features = ["derive", "rc"] }
serde_json = "1.0.107"
thiserror = "1.0.49"
Expand All @@ -30,7 +32,6 @@ tower-cookies = "0.10.0"
tower-layer = "0.3.2"
tower-service = "0.3.2"
tracing = { version = "0.1.40", features = ["log"] }
uuid = { version = "1.4.1", features = ["v4", "serde"] }

[dev-dependencies]
tower-sessions = { workspace = true }
Expand Down
34 changes: 26 additions & 8 deletions tower-sessions-core/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use serde_json::Value;
use time::Duration;
use tokio::sync::{Mutex, MutexGuard};
use tower_cookies::cookie::time::OffsetDateTime;
use uuid::Uuid;

use crate::{session_store, SessionStore};

Expand Down Expand Up @@ -847,7 +846,7 @@ impl Session {

/// ID type for sessions.
///
/// Wraps a UUIDv4.
/// Wraps an array of 16 bytes.
///
/// # Examples
///
Expand All @@ -857,29 +856,48 @@ impl Session {
/// Id::default();
/// ```
#[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, Hash, PartialEq)]
pub struct Id(pub Uuid); // TODO: By this being public, it may be possible to override UUIDv4,
// which is undesirable.
pub struct Id(pub i128); // TODO: By this being public, it may be possible to override the
// session ID, which is undesirable.

impl Default for Id {
fn default() -> Self {
Self(Uuid::new_v4())
use rand::prelude::*;

Self(rand::thread_rng().gen())
}
}

impl Display for Id {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&self.0.as_hyphenated().to_string())
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _};
let data = URL_SAFE_NO_PAD.encode(self.0.to_le_bytes());
f.write_str(&data)
}
}

impl FromStr for Id {
type Err = uuid::Error;
type Err = IdError;

fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
Ok(Self(s.parse::<uuid::Uuid>()?))
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _};

let id_bytes = URL_SAFE_NO_PAD
.decode(s)?
.try_into()
.map_err(|_| Self::Err::LengthError)?;
let id = i128::from_le_bytes(id_bytes);
Ok(Self(id))
}
}

#[derive(thiserror::Error, Debug, Clone)]
pub enum IdError {
#[error("Could not be decoded as URL-safe Base64 string without padding")]
DecodeError(#[from] base64::DecodeError),
#[error("Base64 string does not contain exactly 128 bytes worth")]
LengthError,
}

/// Record type that's appropriate for encoding and decoding sessions to and
/// from session stores.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
Expand Down

0 comments on commit 7631331

Please sign in to comment.