fix(auth): Require EdDSA and fix verification (#232)

There are many different functions you are to use for different types of
keys. We are planning to require EdDSA, so we validate the algorithm and
switch to using `from_ed_pem()`.

---------

Co-authored-by: Jan Michael Auer <mail@jauer.org>

Author: matt-codecov

objectstore-server/src/auth/context.rs

@@ -1,6 +1,6 @@
 use std::collections::{BTreeMap, HashSet};
 
-use jsonwebtoken::{DecodingKey, Header, TokenData, Validation, decode, decode_header};
+use jsonwebtoken::{Algorithm, DecodingKey, Header, TokenData, Validation, decode, decode_header};
 use objectstore_service::id::ObjectId;
 use secrecy::ExposeSecret;
 use serde::{Deserialize, Serialize};
@@ -126,11 +126,31 @@ impl AuthContext {
             .get(key_id)
             .ok_or_else(|| AuthError::InternalError(format!("Key `{key_id}` not configured")))?;
 
+        if jwt_header.alg != Algorithm::EdDSA {
+            tracing::warn!(
+                algorithm = ?jwt_header.alg,
+                "JWT signed with unexpected algorithm",
+            );
+            let kind = jsonwebtoken::errors::ErrorKind::InvalidAlgorithm;
+            return Err(AuthError::ValidationFailure(kind.into()));
+        }
+
         let mut verified_claims: Option<TokenData<JwtClaims>> = None;
         for key in &key_config.key_versions {
+            let decoding_key = match DecodingKey::from_ed_pem(key.expose_secret().as_bytes()) {
+                Ok(key) => key,
+                Err(error) => {
+                    tracing::error!(
+                        error = &error as &dyn std::error::Error,
+                        "Failed to construct decoding key from PEM",
+                    );
+                    continue;
+                }
+            };
+
             let decode_result = decode::<JwtClaims>(
                 encoded_token,
-                &DecodingKey::from_secret(key.expose_secret().as_bytes()),
+                &decoding_key,
                 &jwt_validation_params(&jwt_header),
             );
 
@@ -210,7 +230,16 @@ mod tests {
     use serde_json::json;
 
     const TEST_SIGNING_KID: &str = "test-key";
-    const TEST_SIGNING_SECRET: &str = "fa24f0a3ab08f9ff0d4b2183595a045c";
+    // Private key generated with `openssl genpkey -algorithm Ed25519`
+    const TEST_PRIVATE_KEY: &str = r#"-----BEGIN PRIVATE KEY-----
+MC4CAQAwBQYDK2VwBCIEIAZtPzCHjltjZSi3+THxP6Rh8vUM0LRNA/QDR8zJx0tB
+-----END PRIVATE KEY-----
+"#;
+    // Public key extracted with `openssl pkey -in private_key.pem -pubout`
+    const TEST_PUBLIC_KEY: &str = r#"-----BEGIN PUBLIC KEY-----
+MCowBQYDK2VwAyEA/TOsO19FvHFTsZqcYiO8HGfm02Df5oWBXgzulxYPvSs=
+-----END PUBLIC KEY-----
+"#;
 
     #[derive(Serialize, Deserialize)]
     struct TestJwtClaims {
@@ -228,7 +257,7 @@ mod tests {
     }
 
     fn test_config(max_permissions: HashSet<Permission>) -> AuthZ {
-        let wrapped_key = SecretBox::new(Box::new(ConfigSecret::from(TEST_SIGNING_SECRET)));
+        let wrapped_key = SecretBox::new(Box::new(ConfigSecret::from(TEST_PUBLIC_KEY)));
         let key_config = AuthZVerificationKey {
             key_versions: vec![wrapped_key],
             max_permissions,
@@ -239,24 +268,20 @@ mod tests {
         }
     }
 
-    fn sign_token(claims: &JwtClaims, signing_secret: &str) -> String {
+    fn sign_token(claims: &JwtClaims, signing_secret: &str, exp: Option<u64>) -> String {
         use jsonwebtoken::{Algorithm, EncodingKey, Header, encode, get_current_timestamp};
 
-        let mut header = Header::new(Algorithm::HS256);
+        let mut header = Header::new(Algorithm::EdDSA);
         header.kid = Some(TEST_SIGNING_KID.into());
         header.typ = Some("JWT".into());
 
         let claims = TestJwtClaims {
-            exp: get_current_timestamp() + 300,
+            exp: exp.unwrap_or_else(|| get_current_timestamp() + 300),
             claims: claims.clone(),
         };
 
-        encode(
-            &header,
-            &claims,
-            &EncodingKey::from_secret(signing_secret.as_bytes()),
-        )
-        .unwrap()
+        let key = EncodingKey::from_ed_pem(signing_secret.as_bytes()).unwrap();
+        encode(&header, &claims, &key).unwrap()
     }
 
     fn sample_claims(
@@ -289,7 +314,7 @@ mod tests {
     fn test_from_encoded_jwt_basic() -> Result<(), AuthError> {
         // Create a token with max permissions
         let claims = sample_claims("123", "456", "attachments", max_permission());
-        let encoded_token = sign_token(&claims, TEST_SIGNING_SECRET);
+        let encoded_token = sign_token(&claims, TEST_PRIVATE_KEY, None);
 
         // Create test config with max permissions
         let test_config = test_config(max_permission());
@@ -307,7 +332,7 @@ mod tests {
     fn test_from_encoded_jwt_max_permissions_limit() -> Result<(), AuthError> {
         // Create a token with max permissions
         let claims = sample_claims("123", "456", "attachments", max_permission());
-        let encoded_token = sign_token(&claims, TEST_SIGNING_SECRET);
+        let encoded_token = sign_token(&claims, TEST_PRIVATE_KEY, None);
 
         // Assign read-only permissions to the signing key in config
         let ro_permission = HashSet::from([Permission::ObjectRead]);
@@ -339,9 +364,12 @@ mod tests {
 
     #[test]
     fn test_from_encoded_jwt_unknown_key_fails() -> Result<(), AuthError> {
-        // Create a token with max permissions
         let claims = sample_claims("123", "456", "attachments", max_permission());
-        let encoded_token = sign_token(&claims, "unknown signing key");
+        let unknown_key = r#"-----BEGIN PRIVATE KEY-----
+MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
+-----END PRIVATE KEY-----
+"#;
+        let encoded_token = sign_token(&claims, unknown_key, None);
 
         // Create test config with max permissions
         let test_config = test_config(max_permission());
@@ -356,25 +384,12 @@ mod tests {
 
     #[test]
     fn test_from_encoded_jwt_expired() -> Result<(), AuthError> {
-        use jsonwebtoken::{Algorithm, EncodingKey, Header, encode, get_current_timestamp};
-
         let claims = sample_claims("123", "456", "attachments", max_permission());
-
-        let mut header = Header::new(Algorithm::HS256);
-        header.kid = Some(TEST_SIGNING_KID.into());
-        header.typ = Some("JWT".into());
-
-        let claims = TestJwtClaims {
-            exp: get_current_timestamp() - 100, // NB: expired
-            claims: claims.clone(),
-        };
-
-        let encoded_token = encode(
-            &header,
+        let encoded_token = sign_token(
             &claims,
-            &EncodingKey::from_secret(TEST_SIGNING_SECRET.as_bytes()),
-        )
-        .unwrap();
+            TEST_PRIVATE_KEY,
+            Some(jsonwebtoken::get_current_timestamp() - 100),
+        );
 
         // Create test config with max permissions
         let test_config = test_config(max_permission());

objectstore-server/src/auth/service.rs

@@ -94,7 +94,10 @@ impl FromRequestParts<ServiceState> for AuthAwareService {
 
         let context = AuthContext::from_encoded_jwt(encoded_token, &state.config.auth);
         if context.is_err() && state.config.auth.enforce {
-            tracing::debug!("Authorization failed when enforcement is enabled");
+            tracing::debug!(
+                "Authorization failed and enforcement is enabled: `{:?}`",
+                context
+            );
             return Err(StatusCode::UNAUTHORIZED);
         }