Merge pull request 'Fix potential timing side-channels in authentication mechanisms' (#737) from fix-auth-ct-eq into main

Reviewed-on: Deuxfleurs/garage#737
This commit is contained in:
Alex 2024-02-29 14:04:38 +00:00
commit b8c7a560ef
6 changed files with 109 additions and 21 deletions

24
Cargo.lock generated
View file

@ -120,6 +120,18 @@ version = "1.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6" checksum = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6"
[[package]]
name = "argon2"
version = "0.5.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072"
dependencies = [
"base64ct",
"blake2",
"cpufeatures",
"password-hash",
]
[[package]] [[package]]
name = "arrayvec" name = "arrayvec"
version = "0.5.2" version = "0.5.2"
@ -1321,6 +1333,7 @@ dependencies = [
name = "garage_api" name = "garage_api"
version = "0.9.1" version = "0.9.1"
dependencies = [ dependencies = [
"argon2",
"async-trait", "async-trait",
"base64 0.21.7", "base64 0.21.7",
"bytes", "bytes",
@ -2799,6 +2812,17 @@ dependencies = [
"regex", "regex",
] ]
[[package]]
name = "password-hash"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166"
dependencies = [
"base64ct",
"rand_core",
"subtle",
]
[[package]] [[package]]
name = "paste" name = "paste"
version = "1.0.14" version = "1.0.14"

View file

@ -34,7 +34,7 @@ args@{
ignoreLockHash, ignoreLockHash,
}: }:
let let
nixifiedLockHash = "9377d18da3b48658f9d8b2070db135db2d9ac6d9c692d6656948b765348498cc"; nixifiedLockHash = "69c86fff0acd6c7a9a19dc6966b4cbd48e8a50c5a9fb40b3090ad71aaa5b55d0";
workspaceSrc = if args.workspaceSrc == null then ./. else args.workspaceSrc; workspaceSrc = if args.workspaceSrc == null then ./. else args.workspaceSrc;
currentLockHash = builtins.hashFile "sha256" (workspaceSrc + /Cargo.lock); currentLockHash = builtins.hashFile "sha256" (workspaceSrc + /Cargo.lock);
lockHashIgnored = if ignoreLockHash lockHashIgnored = if ignoreLockHash
@ -235,6 +235,25 @@ in
src = fetchCratesIo { inherit name version; sha256 = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6"; }; src = fetchCratesIo { inherit name version; sha256 = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6"; };
}); });
"registry+https://github.com/rust-lang/crates.io-index".argon2."0.5.3" = overridableMkRustCrate (profileName: rec {
name = "argon2";
version = "0.5.3";
registry = "registry+https://github.com/rust-lang/crates.io-index";
src = fetchCratesIo { inherit name version; sha256 = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072"; };
features = builtins.concatLists [
[ "alloc" ]
[ "default" ]
[ "password-hash" ]
[ "rand" ]
];
dependencies = {
base64ct = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".base64ct."1.6.0" { inherit profileName; }).out;
blake2 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".blake2."0.10.6" { inherit profileName; }).out;
${ if hostPlatform.parsed.cpu.name == "i686" || hostPlatform.parsed.cpu.name == "x86_64" then "cpufeatures" else null } = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".cpufeatures."0.2.12" { inherit profileName; }).out;
password_hash = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".password-hash."0.5.0" { inherit profileName; }).out;
};
});
"registry+https://github.com/rust-lang/crates.io-index".arrayvec."0.5.2" = overridableMkRustCrate (profileName: rec { "registry+https://github.com/rust-lang/crates.io-index".arrayvec."0.5.2" = overridableMkRustCrate (profileName: rec {
name = "arrayvec"; name = "arrayvec";
version = "0.5.2"; version = "0.5.2";
@ -1939,6 +1958,7 @@ in
(lib.optional (rootFeatures' ? "garage/default" || rootFeatures' ? "garage/metrics" || rootFeatures' ? "garage_api/metrics" || rootFeatures' ? "garage_api/prometheus") "prometheus") (lib.optional (rootFeatures' ? "garage/default" || rootFeatures' ? "garage/metrics" || rootFeatures' ? "garage_api/metrics" || rootFeatures' ? "garage_api/prometheus") "prometheus")
]; ];
dependencies = { dependencies = {
argon2 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".argon2."0.5.3" { inherit profileName; }).out;
async_trait = (buildRustPackages."registry+https://github.com/rust-lang/crates.io-index".async-trait."0.1.77" { profileName = "__noProfile"; }).out; async_trait = (buildRustPackages."registry+https://github.com/rust-lang/crates.io-index".async-trait."0.1.77" { profileName = "__noProfile"; }).out;
base64 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".base64."0.21.7" { inherit profileName; }).out; base64 = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".base64."0.21.7" { inherit profileName; }).out;
bytes = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".bytes."1.5.0" { inherit profileName; }).out; bytes = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".bytes."1.5.0" { inherit profileName; }).out;
@ -3989,6 +4009,23 @@ in
}; };
}); });
"registry+https://github.com/rust-lang/crates.io-index".password-hash."0.5.0" = overridableMkRustCrate (profileName: rec {
name = "password-hash";
version = "0.5.0";
registry = "registry+https://github.com/rust-lang/crates.io-index";
src = fetchCratesIo { inherit name version; sha256 = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166"; };
features = builtins.concatLists [
[ "alloc" ]
[ "default" ]
[ "rand_core" ]
];
dependencies = {
base64ct = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".base64ct."1.6.0" { inherit profileName; }).out;
rand_core = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".rand_core."0.6.4" { inherit profileName; }).out;
subtle = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".subtle."2.5.0" { inherit profileName; }).out;
};
});
"registry+https://github.com/rust-lang/crates.io-index".paste."1.0.14" = overridableMkRustCrate (profileName: rec { "registry+https://github.com/rust-lang/crates.io-index".paste."1.0.14" = overridableMkRustCrate (profileName: rec {
name = "paste"; name = "paste";
version = "1.0.14"; version = "1.0.14";

View file

@ -34,6 +34,7 @@ k2v-client = { version = "0.0.4", path = "src/k2v-client" }
# External crates from crates.io # External crates from crates.io
arc-swap = "1.0" arc-swap = "1.0"
argon2 = "0.5"
async-trait = "0.1.7" async-trait = "0.1.7"
backtrace = "0.3" backtrace = "0.3"
base64 = "0.21" base64 = "0.21"

View file

@ -21,6 +21,7 @@ garage_net.workspace = true
garage_util.workspace = true garage_util.workspace = true
garage_rpc.workspace = true garage_rpc.workspace = true
argon2.workspace = true
async-trait.workspace = true async-trait.workspace = true
base64.workspace = true base64.workspace = true
bytes.workspace = true bytes.workspace = true

View file

@ -1,6 +1,7 @@
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::Arc; use std::sync::Arc;
use argon2::password_hash::PasswordHash;
use async_trait::async_trait; use async_trait::async_trait;
use http::header::{ACCESS_CONTROL_ALLOW_METHODS, ACCESS_CONTROL_ALLOW_ORIGIN, ALLOW}; use http::header::{ACCESS_CONTROL_ALLOW_METHODS, ACCESS_CONTROL_ALLOW_ORIGIN, ALLOW};
@ -45,14 +46,8 @@ impl AdminApiServer {
#[cfg(feature = "metrics")] exporter: PrometheusExporter, #[cfg(feature = "metrics")] exporter: PrometheusExporter,
) -> Self { ) -> Self {
let cfg = &garage.config.admin; let cfg = &garage.config.admin;
let metrics_token = cfg let metrics_token = cfg.metrics_token.as_deref().map(hash_bearer_token);
.metrics_token let admin_token = cfg.admin_token.as_deref().map(hash_bearer_token);
.as_ref()
.map(|tok| format!("Bearer {}", tok));
let admin_token = cfg
.admin_token
.as_ref()
.map(|tok| format!("Bearer {}", tok));
Self { Self {
garage, garage,
#[cfg(feature = "metrics")] #[cfg(feature = "metrics")]
@ -248,11 +243,11 @@ impl ApiHandler for AdminApiServer {
req: Request<IncomingBody>, req: Request<IncomingBody>,
endpoint: Endpoint, endpoint: Endpoint,
) -> Result<Response<ResBody>, Error> { ) -> Result<Response<ResBody>, Error> {
let expected_auth_header = let required_auth_hash =
match endpoint.authorization_type() { match endpoint.authorization_type() {
Authorization::None => None, Authorization::None => None,
Authorization::MetricsToken => self.metrics_token.as_ref(), Authorization::MetricsToken => self.metrics_token.as_deref(),
Authorization::AdminToken => match &self.admin_token { Authorization::AdminToken => match self.admin_token.as_deref() {
None => return Err(Error::forbidden( None => return Err(Error::forbidden(
"Admin token isn't configured, admin API access is disabled for security.", "Admin token isn't configured, admin API access is disabled for security.",
)), )),
@ -260,14 +255,11 @@ impl ApiHandler for AdminApiServer {
}, },
}; };
if let Some(h) = expected_auth_header { if let Some(password_hash) = required_auth_hash {
match req.headers().get("Authorization") { match req.headers().get("Authorization") {
None => return Err(Error::forbidden("Authorization token must be provided")), None => return Err(Error::forbidden("Authorization token must be provided")),
Some(v) => { Some(authorization) => {
let authorized = v.to_str().map(|hv| hv.trim() == h).unwrap_or(false); verify_bearer_token(&authorization, password_hash)?;
if !authorized {
return Err(Error::forbidden("Invalid authorization token provided"));
}
} }
} }
} }
@ -342,3 +334,35 @@ impl ApiEndpoint for Endpoint {
fn add_span_attributes(&self, _span: SpanRef<'_>) {} fn add_span_attributes(&self, _span: SpanRef<'_>) {}
} }
fn hash_bearer_token(token: &str) -> String {
use argon2::{
password_hash::{rand_core::OsRng, PasswordHasher, SaltString},
Argon2,
};
let salt = SaltString::generate(&mut OsRng);
let argon2 = Argon2::default();
argon2
.hash_password(token.trim().as_bytes(), &salt)
.expect("could not hash API token")
.to_string()
}
fn verify_bearer_token(token: &hyper::http::HeaderValue, password_hash: &str) -> Result<(), Error> {
use argon2::{password_hash::PasswordVerifier, Argon2};
let parsed_hash = PasswordHash::new(&password_hash).unwrap();
token
.to_str()?
.strip_prefix("Bearer ")
.and_then(|token| {
Argon2::default()
.verify_password(token.trim().as_bytes(), &parsed_hash)
.ok()
})
.ok_or_else(|| Error::forbidden("Invalid authorization token"))?;
Ok(())
}

View file

@ -375,9 +375,10 @@ pub async fn verify_v4(
) )
.ok_or_internal_error("Unable to build signing HMAC")?; .ok_or_internal_error("Unable to build signing HMAC")?;
hmac.update(payload); hmac.update(payload);
let our_signature = hex::encode(hmac.finalize().into_bytes()); let signature =
if auth.signature != our_signature { hex::decode(&auth.signature).map_err(|_| Error::forbidden("Invalid signature"))?;
return Err(Error::forbidden("Invalid signature".to_string())); if hmac.verify_slice(&signature).is_err() {
return Err(Error::forbidden("Invalid signature"));
} }
Ok(key) Ok(key)