From cee1f781e437b7d67a12757f470bfa9639aa34e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anton=20=C3=96sterberg?= Date: Tue, 4 Nov 2025 22:20:04 +0100 Subject: [PATCH] fix: Prevent timing attacks Make an effort to prevent timing attacks when checking the API key received in requests. While this change should be considered an improvement, it does not guarantee that timing attacks can be prevented under all conditions. --- Cargo.lock | 12 ++++++++++++ Cargo.toml | 1 + src/auth.rs | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) 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")); + } +}