KL-4 ext: Remove brute-force compromising credential failure reasons to improve auth rate limiter effectiveness

bin/core/src/auth/jwt.rs

@@ -64,14 +64,14 @@ impl JwtClient {
       exp,
     };
     let jwt = encode(&self.header, &claims, &self.encoding_key)
-      .context("failed at signing claim")?;
+      .context("Failed at signing claim")?;
     Ok(JwtResponse { user_id, jwt })
   }
 
   pub fn decode(&self, jwt: &str) -> anyhow::Result<JwtClaims> {
     decode::<JwtClaims>(jwt, &self.decoding_key, &self.validation)
       .map(|res| res.claims)
-      .context("failed to decode token claims")
+      .context("Failed to decode token claims")
   }
 
   pub async fn create_exchange_token(

bin/core/src/auth/local.rs

@@ -14,7 +14,7 @@ use komodo_client::{
 use rate_limit::WithFailureRateLimit;
 use reqwest::StatusCode;
 use resolver_api::Resolve;
-use serror::AddStatusCode as _;
+use serror::{AddStatusCode as _, AddStatusCodeError};
 
 use crate::{
   api::auth::AuthArgs,
@@ -58,7 +58,10 @@ async fn sign_up_local_user(
     db.users.find_one(Document::new()).await?.is_none();
 
   if !no_users_exist && config.disable_user_registration {
-    return Err(anyhow!("User registration is disabled").into());
+    return Err(
+      anyhow!("User registration is disabled")
+        .status_code(StatusCode::UNAUTHORIZED),
+    );
   }
 
   if db
@@ -68,7 +71,14 @@ async fn sign_up_local_user(
     .context("Failed to query for existing users")?
     .is_some()
   {
-    return Err(anyhow!("Username already taken.").into());
+    // When user registration is enabled, there is no way around allowing
+    // potential attackers to gain some insight about which usernames exist
+    // if they are allowed to register accounts. Since this can be easily inferred,
+    // might as well be clear. The auth rate limiter is critical here.
+    return Err(
+      anyhow!("Username already taken.")
+        .status_code(StatusCode::BAD_REQUEST),
+    );
   }
 
   let ts = unix_timestamp_ms() as i64;
@@ -125,7 +135,10 @@ async fn login_local_user(
   req: LoginLocalUser,
 ) -> serror::Result<LoginLocalUserResponse> {
   if !core_config().local_auth {
-    return Err(anyhow!("Local auth is not enabled").into());
+    return Err(
+      anyhow!("Local auth is not enabled")
+        .status_code(StatusCode::UNAUTHORIZED),
+    );
   }
 
   validate_username(&req.username)
@@ -135,30 +148,34 @@ async fn login_local_user(
     .users
     .find_one(doc! { "username": &req.username })
     .await
-    .context("failed at db query for users")?
-    .with_context(|| {
-      format!("did not find user with username {}", req.username)
-    })?;
+    .context("Failed at db query for users")?
+    .context("Invalid login credentials")
+    .status_code(StatusCode::UNAUTHORIZED)?;
 
   let UserConfig::Local {
     password: user_pw_hash,
   } = user.config
   else {
     return Err(
-      anyhow!("Non-local auth users can not log in with a password")
-        .into(),
+      anyhow!("Invalid login credentials")
+        .status_code(StatusCode::UNAUTHORIZED),
     );
   };
 
   let verified = bcrypt::verify(req.password, &user_pw_hash)
-    .context("failed at verify password")?;
+    .context("Invalid login credentials")
+    .status_code(StatusCode::UNAUTHORIZED)?;
 
   if !verified {
-    return Err(anyhow!("invalid credentials").into());
+    return Err(
+      anyhow!("Invalid login credentials")
+        .status_code(StatusCode::UNAUTHORIZED),
+    );
   }
 
   jwt_client()
     .encode(user.id)
+    // This is in internal error (500), not auth error
     .context("Failed to generate JWT for user")
     .map_err(Into::into)
 }

bin/core/src/auth/mod.rs

@@ -59,23 +59,21 @@ pub async fn get_user_id_from_headers(
   ) {
     (Some(jwt), _, _) => {
       // USE JWT
-      let jwt = jwt.to_str().context("jwt is not str")?;
-      auth_jwt_get_user_id(jwt)
-        .await
-        .context("failed to authenticate jwt")
+      let jwt = jwt.to_str().context("JWT is not valid UTF-8")?;
+      auth_jwt_get_user_id(jwt).await
     }
     (None, Some(key), Some(secret)) => {
       // USE API KEY / SECRET
-      let key = key.to_str().context("key is not str")?;
-      let secret = secret.to_str().context("secret is not str")?;
-      auth_api_key_get_user_id(key, secret)
-        .await
-        .context("failed to authenticate api key")
+      let key =
+        key.to_str().context("X-API-KEY is not valid UTF-8")?;
+      let secret =
+        secret.to_str().context("X-API-SECRET is not valid UTF-8")?;
+      auth_api_key_get_user_id(key, secret).await
     }
     _ => {
       // AUTH FAIL
       Err(anyhow!(
-        "must attach either AUTHORIZATION header with jwt OR pass X-API-KEY and X-API-SECRET"
+        "Must attach either AUTHORIZATION header with jwt OR pass X-API-KEY and X-API-SECRET"
       ))
     }
   }
@@ -85,22 +83,26 @@ pub async fn authenticate_check_enabled(
   headers: &HeaderMap,
 ) -> anyhow::Result<User> {
   let user_id = get_user_id_from_headers(headers).await?;
-  let user = get_user(&user_id).await?;
+  let user = get_user(&user_id)
+    .await
+    .map_err(|_| anyhow!("Invalid user credentials"))?;
   if user.enabled {
     Ok(user)
   } else {
-    Err(anyhow!("User not enabled"))
+    Err(anyhow!("Invalid user credentials"))
   }
 }
 
 pub async fn auth_jwt_get_user_id(
   jwt: &str,
 ) -> anyhow::Result<String> {
-  let claims: JwtClaims = jwt_client().decode(jwt)?;
+  let claims: JwtClaims = jwt_client()
+    .decode(jwt)
+    .map_err(|_| anyhow!("Invalid user credentials"))?;
   if claims.exp > unix_timestamp_ms() {
     Ok(claims.id)
   } else {
-    Err(anyhow!("token has expired"))
+    Err(anyhow!("Invalid user credentials"))
   }
 }
 
@@ -119,19 +121,19 @@ pub async fn auth_api_key_get_user_id(
     .api_keys
     .find_one(doc! { "key": key })
     .await
-    .context("failed to query db")?
-    .context("no api key matching key")?;
+    .context("Failed to query db")?
+    .context("Invalid user credentials")?;
   if key.expires != 0 && key.expires < komodo_timestamp() {
-    return Err(anyhow!("api key expired"));
+    return Err(anyhow!("Invalid user credentials"));
   }
   if bcrypt::verify(secret, &key.secret)
-    .context("failed to verify secret hash")?
+    .map_err(|_| anyhow!("Invalid user credentials"))?
   {
     // secret matches
     Ok(key.user_id)
   } else {
     // secret mismatch
-    Err(anyhow!("invalid api secret"))
+    Err(anyhow!("Invalid user credentials"))
   }
 }
 
@@ -148,6 +150,6 @@ async fn check_enabled(user_id: String) -> anyhow::Result<User> {
   if user.enabled {
     Ok(user)
   } else {
-    Err(anyhow!("user not enabled"))
+    Err(anyhow!("Invalid user credentials"))
   }
 }

bin/core/src/helpers/query.rs

@@ -56,8 +56,8 @@ pub async fn get_user(user: &str) -> anyhow::Result<User> {
     .users
     .find_one(id_or_username_filter(user))
     .await
-    .context("failed to query mongo for user")?
-    .with_context(|| format!("no user found with {user}"))
+    .context("Failed to query mongo for user")?
+    .with_context(|| format!("No user found matching '{user}'"))
 }
 
 pub async fn get_server_with_state(