From eaac4924ef5c18eb40eabfbf246f5ae9c894889a Mon Sep 17 00:00:00 2001
From: Alex Auvolat <alex@adnab.me>
Date: Thu, 29 Feb 2024 10:57:07 +0100
Subject: [fix-auth-ct-eq] use argon2 hashing and verification for
 admin/metrics token checking

---
 src/api/Cargo.toml          |  1 +
 src/api/admin/api_server.rs | 58 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 42 insertions(+), 17 deletions(-)

(limited to 'src/api')

diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml
index bc6b6aa7..cb87d9e1 100644
--- a/src/api/Cargo.toml
+++ b/src/api/Cargo.toml
@@ -21,6 +21,7 @@ garage_net.workspace = true
 garage_util.workspace = true
 garage_rpc.workspace = true
 
+argon2.workspace = true
 async-trait.workspace = true
 base64.workspace = true
 bytes.workspace = true
diff --git a/src/api/admin/api_server.rs b/src/api/admin/api_server.rs
index 50813d11..265639c4 100644
--- a/src/api/admin/api_server.rs
+++ b/src/api/admin/api_server.rs
@@ -1,6 +1,7 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
+use argon2::password_hash::PasswordHash;
 use async_trait::async_trait;
 
 use http::header::{ACCESS_CONTROL_ALLOW_METHODS, ACCESS_CONTROL_ALLOW_ORIGIN, ALLOW};
@@ -45,14 +46,8 @@ impl AdminApiServer {
 		#[cfg(feature = "metrics")] exporter: PrometheusExporter,
 	) -> Self {
 		let cfg = &garage.config.admin;
-		let metrics_token = cfg
-			.metrics_token
-			.as_ref()
-			.map(|tok| format!("Bearer {}", tok));
-		let admin_token = cfg
-			.admin_token
-			.as_ref()
-			.map(|tok| format!("Bearer {}", tok));
+		let metrics_token = cfg.metrics_token.as_deref().map(hash_bearer_token);
+		let admin_token = cfg.admin_token.as_deref().map(hash_bearer_token);
 		Self {
 			garage,
 			#[cfg(feature = "metrics")]
@@ -248,11 +243,11 @@ impl ApiHandler for AdminApiServer {
 		req: Request<IncomingBody>,
 		endpoint: Endpoint,
 	) -> Result<Response<ResBody>, Error> {
-		let expected_auth_header =
+		let required_auth_hash =
 			match endpoint.authorization_type() {
 				Authorization::None => None,
-				Authorization::MetricsToken => self.metrics_token.as_ref(),
-				Authorization::AdminToken => match &self.admin_token {
+				Authorization::MetricsToken => self.metrics_token.as_deref(),
+				Authorization::AdminToken => match self.admin_token.as_deref() {
 					None => return Err(Error::forbidden(
 						"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") {
 				None => return Err(Error::forbidden("Authorization token must be provided")),
-				Some(v) => {
-					let authorized = v.to_str().map(|hv| hv.trim() == h).unwrap_or(false);
-					if !authorized {
-						return Err(Error::forbidden("Invalid authorization token provided"));
-					}
+				Some(authorization) => {
+					verify_bearer_token(&authorization, password_hash)?;
 				}
 			}
 		}
@@ -342,3 +334,35 @@ impl ApiEndpoint for Endpoint {
 
 	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(())
+}
-- 
cgit v1.2.3


From 6d33e721c41bdb0fe7da6404e6d6d32509eed6be Mon Sep 17 00:00:00 2001
From: Alex Auvolat <alex@adnab.me>
Date: Thu, 29 Feb 2024 12:43:25 +0100
Subject: [fix-auth-ct-eq] use consant time comparison for awsv4 signature
 verification

---
 src/api/signature/payload.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

(limited to 'src/api')

diff --git a/src/api/signature/payload.rs b/src/api/signature/payload.rs
index 949da601..a9e7d34d 100644
--- a/src/api/signature/payload.rs
+++ b/src/api/signature/payload.rs
@@ -375,9 +375,10 @@ pub async fn verify_v4(
 	)
 	.ok_or_internal_error("Unable to build signing HMAC")?;
 	hmac.update(payload);
-	let our_signature = hex::encode(hmac.finalize().into_bytes());
-	if auth.signature != our_signature {
-		return Err(Error::forbidden("Invalid signature".to_string()));
+	let signature =
+		hex::decode(&auth.signature).map_err(|_| Error::forbidden("Invalid signature"))?;
+	if hmac.verify_slice(&signature).is_err() {
+		return Err(Error::forbidden("Invalid signature"));
 	}
 
 	Ok(key)
-- 
cgit v1.2.3