Ignore JWKS keys which are not supported by the router

.changesets/fix_garypen_3853_jwks_improve.md

@@ -0,0 +1,7 @@
+### Ignore JWKS keys which aren't in the algorithms configuration ([Issue #3853](https://github.com/apollographql/router/issues/3853))
+
+If you have a JWKS which contains a Key which has an algorithm (alg) which the router doesn't recognise, then the entire JWKS is disregarded. This is unsatisfactory, since there are likely to be many other keys in the JWKS which the router could use.
+
+We have changed the JWKS processing logic so that we use the list of `algorithms` specified in your configuration to exclude keys which are not contained in that list.
+
+By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/3922

apollo-router/src/plugins/authentication/jwks.rs

@@ -1,5 +1,6 @@
 use std::collections::HashMap;
 use std::collections::HashSet;
+use std::str::FromStr;
 use std::sync::Arc;
 use std::sync::RwLock;
 
@@ -14,6 +15,7 @@ use http::header::CONTENT_TYPE;
 use jsonwebtoken::jwk::JwkSet;
 use jsonwebtoken::Algorithm;
 use mime::APPLICATION_JSON;
+use serde_json::Value;
 use tokio::fs::read_to_string;
 use tokio::sync::oneshot;
 use tower::BoxError;
@@ -51,9 +53,14 @@ impl JwksManager {
         let downloads = list
             .iter()
             .cloned()
-            .map(|JwksConfig { url, .. }| {
-                get_jwks(url.clone()).map(|opt_jwks| opt_jwks.map(|jwks| (url, jwks)))
-            })
+            .map(
+                |JwksConfig {
+                     url, algorithms, ..
+                 }| {
+                    get_jwks(url.clone(), algorithms.clone())
+                        .map(|opt_jwks| opt_jwks.map(|jwks| (url, jwks)))
+                },
+            )
             .collect::<Vec<_>>();
 
         let jwks_map: HashMap<_, _> = join_all(downloads).await.into_iter().flatten().collect();
@@ -102,7 +109,7 @@ async fn poll(
             repeat((config, jwks_map)).then(|(config, jwks_map)| async move {
                 tokio::time::sleep(DEFAULT_AUTHENTICATION_DOWNLOAD_INTERVAL).await;
 
-                if let Some(jwks) = get_jwks(config.url.clone()).await {
+                if let Some(jwks) = get_jwks(config.url.clone(), config.algorithms.clone()).await {
                     if let Ok(mut map) = jwks_map.write() {
                         map.insert(config.url, jwks);
                     }
@@ -132,7 +139,7 @@ async fn poll(
 // This function is expected to return an Optional value, but we'd like to let
 // users know the various failure conditions. Hence the various clumsy map_err()
 // scattered through the processing.
-pub(super) async fn get_jwks(url: Url) -> Option<JwkSet> {
+pub(super) async fn get_jwks(url: Url, algorithms: Option<HashSet<Algorithm>>) -> Option<JwkSet> {
     let data = if url.scheme() == "file" {
         let path = url
             .to_file_path()
@@ -178,7 +185,44 @@ pub(super) async fn get_jwks(url: Url) -> Option<JwkSet> {
             })
             .ok()?
     };
-    let jwks: JwkSet = serde_json::from_str(&data)
+    // Some JWKS contain algorithms which are not supported by the jsonwebtoken library. That means
+    // we can't just deserialize from the retrieved data and proceed. Any unrecognised
+    // algorithms will cause deserialization to fail.
+    //
+    // We use the list of algorithms specified by the user in configuration to constrain the
+    // processing and exclude keys which are not in the optional specified list. If there is no
+    // optional list of algorithms, we accept the full set.
+
+    let mut raw_json: Value = serde_json::from_str(&data)
+        .map_err(|e| {
+            tracing::error!(%e, "could not create JSON Value from url content");
+            e
+        })
+        .ok()?;
+    if let Some(retain_list) = algorithms {
+        raw_json.get_mut("keys").and_then(|keys| {
+            keys.as_array_mut().map(|array| {
+                array.retain(|key| {
+                    let alg_json = key.get("alg").unwrap_or_else(|| &Value::Null);
+                    // Retention rules are, retain if:
+                    //  - we can't find an alg field
+                    //  - alg field isn't a string
+                    //  - our list of algorithms contains the identifed algorithm
+                    if alg_json == &Value::Null || !alg_json.is_string() {
+                        true
+                    } else {
+                        match Algorithm::from_str(
+                            alg_json.as_str().expect("we checked it's a string"),
+                        ) {
+                            Ok(alg) => retain_list.contains(&alg),
+                            Err(_) => false,
+                        }
+                    }
+                })
+            })
+        });
+    }
+    let jwks: JwkSet = serde_json::from_value(raw_json)
         .map_err(|e| {
             tracing::error!(%e, "could not create JWKS from url content");
             e

apollo-router/src/plugins/authentication/tests.rs

@@ -920,3 +920,40 @@ async fn it_rejects_key_with_restricted_algorithm() {
 
     assert!(search_jwks(&jwks_manager, &criteria).is_none());
 }
+
+#[tokio::test]
+async fn it_rejects_key_with_restricted_algorithm_and_unknown_jwks_algorithm() {
+    let mut sets = vec![];
+    let mut urls = vec![];
+
+    // Use a jwks which contains an algorithm (ES512) which jsonwebtoken doesn't support
+    let jwks_url = create_an_url("jwks-unknown-alg.json");
+
+    sets.push(jwks_url);
+
+    for s_url in &sets {
+        let url: Url = Url::from_str(s_url).expect("created a valid url");
+        urls.push(JwksConfig {
+            url,
+            issuer: None,
+            algorithms: Some(HashSet::from([Algorithm::RS256, Algorithm::HS256])),
+        });
+    }
+
+    let jwks_manager = JwksManager::new(urls).await.unwrap();
+
+    // the JWT contains a HMAC key but we configured a restriction to RSA signing
+    let criteria = JWTCriteria {
+        kid: None,
+        alg: Algorithm::HS256,
+    };
+
+    assert!(search_jwks(&jwks_manager, &criteria).is_some());
+
+    let criteria = JWTCriteria {
+        kid: None,
+        alg: Algorithm::RS256,
+    };
+
+    assert!(search_jwks(&jwks_manager, &criteria).is_some());
+}

apollo-router/tests/fixtures/jwks-unknown-alg.json

@@ -0,0 +1,49 @@
+{
+  "keys": [
+    {
+      "kty": "oct",
+      "kid": "key1",
+      "alg": "HS256",
+      "k": "c2VjcmV0Cg==",
+      "use": "sig"
+    },
+    {
+      "kty": "oct",
+      "kid": "key2",
+      "k": "c2VjcmV0Cg==",
+      "use": "sig"
+    },
+    {
+      "crv": "P-256",
+      "key_ops": [
+        "verify"
+      ],
+      "kty": "EC",
+      "x": "opFUViwCYVZLmsbG2cJTA9uPvOF5Gg8W7uNhrcorGhI",
+      "y": "bPxvCFKmlqTdEFc34OekvpviUUyelGrbi020dlgIsqo",
+      "alg": "ES256",
+      "use": "sig",
+      "kid": "afda85e09a320cf748177874592de64d"
+    },
+    {
+      "alg": "RS256",
+      "e": "AQAB",
+      "key_ops": [
+        "verify"
+      ],
+      "kty": "RSA",
+      "n": "wg1GEw1MJZ7IWORpJegYEkc8WB1ck-uNVlKP9Aqjt8tck1JN5dmM_Dwbp-JLsL49xgaXByVAqOO4mWefgx-_sGzYOTMUfAxOYuj3XwBYEAtrBETjGQB-xfdIq8yVnrrwIhKovo42O9TPjQQZ-vL2YmgKdqqO5tQPvKVI20nQVkT04I1orMsbbcSVT6xZiOCWwZWkre2wJbk8hMR_IXHOotpUgAazN99A413c5R9ZBwSZni12PhXf4vM4hoiLTNSy-PEiFle1o2yKItwVZ55SL_P1LuL4A0Sh4F7CRclpdTIql8mv2hVUfdH5U7oIch3O3HDm6pzaxAzNgENaDLL4iYf7UCd1muTu2R8X-2D-qGdz8mhVGsVBhn57S9-zWU6ShM4D-frHibpmcJhnFPJ8zcVGscbc59qWLZH6a3n176eny_TM9CZIeLu9U1fRL1jiAn9m5wJR6G_rQhXWaiKyP2SMyqKrDrxlacxApGwllcOOCXG11tEDaH4W01MpuvVL",
+      "use": "sig",
+      "kid": "022516583d56b68faf40260fda72978a"
+    },
+    {
+      "kty": "EC",
+      "kid": "IcDV_1dOLRNRTRCrGtG9h5bXkVE",
+      "use": "sig",
+      "alg": "ES512",
+      "x": "AKwEPYNsPBEZi-PZKfE7igKDZaqY8xMwIu2AkjPOWJ_bJ9lv6QZIr4mVKSyj0d8wHPExlkWJkX7cxqb3UO_KMVF7",
+      "y": "AVo3Gnbj4uJEBRariAw1q5pQmzjvHGSaUtbZME9ZtFyOIJmDmoy_Y37PqAnyWwL6QB7iSPfzWn7j-3aZ8pnQIC9D",
+      "crv": "P-521"
+    }
+  ]
+}