diff --git a/Cargo.lock b/Cargo.lock index f256b9e..75075ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -345,6 +345,7 @@ dependencies = [ "redis", "serde", "serde_json", + "sha2", "tokio", ] @@ -1023,6 +1024,17 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbfa15b3dddfee50a0fff136974b3e1bde555604ba463834a7eb7deb6417705d" +[[package]] +name = "sha2" +version = "0.10.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "signal-hook-registry" version = "1.4.6" diff --git a/Cargo.toml b/Cargo.toml index 6d967a8..3a3624d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ log = { version = "0.4", optional = true } env_logger = { version = "0.11.8", optional = true } clap = { version = "4.5.46", features = ["color", "derive", "help", "usage", "std", "env"] } redis = { version = "0.25.4", features = [ "ahash", "aio", "tokio-comp" ] } +sha2 = "0.10.9" [features] default = ["logging"] diff --git a/src/auth.rs b/src/auth.rs index f9f38c9..611ebca 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,5 +1,8 @@ +use std::{hint::black_box, io::Read}; + use axum::{headers::Authorization, TypedHeader}; use http::HeaderValue; +use sha2::Digest; pub struct ApiKey(String); @@ -26,7 +29,50 @@ pub fn accept_auth( None => return true, }; match header { - Some(TypedHeader(Authorization(ApiKey(presented)))) => expected == &presented, + Some(TypedHeader(Authorization(ApiKey(presented)))) => const_comp(expected, &presented), None => false, } } + +/// Function for comparing two strings in equal time. I.e. the similarity of the strings should +/// have no bearing on the time it takes to compare them. +/// +/// A naive solution to compare two strings for equality would most likely return at the first byte +/// that differs, which means that the more similar two strings are, the longer the execution time +/// for comparing the strings. This would make such naive implementation susceptible to a +/// [timing attack](https://en.wikipedia.org/wiki/Timing_attack), which should ideally be avoided. +fn const_comp(i0: impl AsRef<[u8]>, i1: impl AsRef<[u8]>) -> bool { + // Hash inputs so we always get equal length when comparing, so we do not risk leaking the + // length of the expected API key via a timing attack. + let h0 = sha2::Sha384::digest(i0); + let h1 = sha2::Sha384::digest(i1); + // The documentation for black box explicitly states that _"this function does not offer any + // guarantees for cryptographic or security purposes"_. But the other two options are to + // either + // 1. Take no measures at all to prevent unwanted optimizations + // 2. Try to hand roll a constant time comparison algorithm implemented in assembler + // + // The first option seems worse than taking no action at all, and the later is not really + // feasible. + black_box( + h0.bytes() + .zip(h1.bytes()) + .fold(0, |acc, (x, y)| acc | (x.unwrap() ^ y.unwrap())) + == 0, + ) +} + +#[cfg(test)] +mod tests { + use crate::auth::const_comp; + + #[test] + fn compare_on_equal() { + assert!(const_comp("foo", "foo")); + } + + #[test] + fn compare_on_not_equal() { + assert!(!const_comp("foo", "bar")); + } +}