feat(dgw): extend subkey capabilities to KDC tokens

ci/tlk.ps1

@@ -593,6 +593,9 @@ class TlkRecipe
             $CargoArgs += '--workspace'
         }
 
+        $CargoArgs += '--verbose'
+        $CargoArgs += '--locked'
+
         $CargoTarget = $this.Target.CargoTarget()
         $CargoProfile = $this.Target.CargoProfile
 

crates/devolutions-gateway-generators/src/lib.rs

@@ -1,5 +1,7 @@
 use devolutions_gateway::session::{ConnectionModeDetails, SessionInfo};
-use devolutions_gateway::token::{self, AccessScope, ApplicationProtocol, Protocol};
+use devolutions_gateway::token::{
+    self, AccessScope, ApplicationProtocol, Protocol, MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS,
+};
 use devolutions_gateway::utils::TargetAddr;
 use proptest::collection::vec;
 use proptest::option;
@@ -166,7 +168,7 @@ impl AssociationClaims {
     }
 }
 
-pub fn any_association_claims(now: i64) -> impl Strategy<Value = AssociationClaims> {
+pub fn any_association_claims(now: i64, validity_duration: i64) -> impl Strategy<Value = AssociationClaims> {
     (
         uuid_typed(),
         jet_ap_and_jet_cm(),
@@ -183,7 +185,7 @@ pub fn any_association_claims(now: i64) -> impl Strategy<Value = AssociationClai
                 jet_flt,
                 jti,
                 nbf: now,
-                exp: now + 1000,
+                exp: now + validity_duration,
             },
         )
 }
@@ -206,12 +208,12 @@ pub struct ScopeClaims {
     pub jti: Uuid,
 }
 
-pub fn any_scope_claims(now: i64) -> impl Strategy<Value = ScopeClaims> {
+pub fn any_scope_claims(now: i64, validity_duration: i64) -> impl Strategy<Value = ScopeClaims> {
     (access_scope(), uuid_typed()).prop_map(move |(scope, jti)| ScopeClaims {
         scope,
         jti,
         nbf: now,
-        exp: now + 1000,
+        exp: now + validity_duration,
     })
 }
 
@@ -223,12 +225,12 @@ pub struct BridgeClaims {
     pub jti: Uuid,
 }
 
-pub fn any_bridge_claims(now: i64) -> impl Strategy<Value = BridgeClaims> {
+pub fn any_bridge_claims(now: i64, validity_duration: i64) -> impl Strategy<Value = BridgeClaims> {
     (target_addr(), uuid_typed()).prop_map(move |(target_host, jti)| BridgeClaims {
         target_host,
         jti,
         nbf: now,
-        exp: now + 1000,
+        exp: now + validity_duration,
     })
 }
 
@@ -244,7 +246,7 @@ pub struct JmuxClaims {
     pub jti: Uuid,
 }
 
-pub fn any_jmux_claims(now: i64) -> impl Strategy<Value = JmuxClaims> {
+pub fn any_jmux_claims(now: i64, validity_duration: i64) -> impl Strategy<Value = JmuxClaims> {
     (
         uuid_typed(),
         host(),
@@ -259,7 +261,7 @@ pub fn any_jmux_claims(now: i64) -> impl Strategy<Value = JmuxClaims> {
             jet_ap,
             jti,
             nbf: now,
-            exp: now + 1000,
+            exp: now + validity_duration,
         })
 }
 
@@ -272,7 +274,7 @@ pub struct KdcClaims {
     pub jti: Uuid,
 }
 
-pub fn any_kdc_claims(now: i64) -> impl Strategy<Value = KdcClaims> {
+pub fn any_kdc_claims(now: i64, validity_duration: i64) -> impl Strategy<Value = KdcClaims> {
     (
         "[a-z0-9_-]{5,25}",
         "(tcp|udp)://[a-z]{1,10}\\.[a-z]{1,5}(:[0-9]{3,4})?",
@@ -283,7 +285,7 @@ pub fn any_kdc_claims(now: i64) -> impl Strategy<Value = KdcClaims> {
             krb_kdc,
             jti,
             nbf: now,
-            exp: now + 1000,
+            exp: now + validity_duration,
         })
 }
 
@@ -316,12 +318,17 @@ impl TokenClaims {
     }
 }
 
-pub fn any_claims(now: i64) -> impl Strategy<Value = TokenClaims> {
+pub fn any_claims_with_validity_duration(now: i64, validity_duration: i64) -> impl Strategy<Value = TokenClaims> {
     prop_oneof![
-        any_scope_claims(now).prop_map(TokenClaims::Scope),
-        any_bridge_claims(now).prop_map(TokenClaims::Bridge),
-        any_kdc_claims(now).prop_map(TokenClaims::Kdc),
-        any_jmux_claims(now).prop_map(TokenClaims::Jmux),
-        any_association_claims(now).prop_map(TokenClaims::Association),
+        any_scope_claims(now, validity_duration).prop_map(TokenClaims::Scope),
+        any_bridge_claims(now, validity_duration).prop_map(TokenClaims::Bridge),
+        any_kdc_claims(now, validity_duration).prop_map(TokenClaims::Kdc),
+        any_jmux_claims(now, validity_duration).prop_map(TokenClaims::Jmux),
+        any_association_claims(now, validity_duration).prop_map(TokenClaims::Association),
     ]
 }
+
+pub fn any_claims(now: i64) -> impl Strategy<Value = TokenClaims> {
+    (15..(MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS * 2))
+        .prop_flat_map(move |validity_duration| any_claims_with_validity_duration(now, validity_duration))
+}

devolutions-gateway/src/token.rs

@@ -15,6 +15,8 @@ use std::sync::Arc;
 use uuid::Uuid;
 use zeroize::Zeroize;
 
+pub const MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS: i64 = 60 * 60 * 2; // 2 hours
+
 const LEEWAY_SECS: u16 = 60 * 5; // 5 minutes
 const CLEANUP_TASK_INTERVAL_SECS: u64 = 60 * 30; // 30 minutes
 const MAX_REUSE_INTERVAL_SECS: i64 = 10; // 10 seconds
@@ -765,9 +767,20 @@ fn validate_token_impl(
 
     if using_subkey {
         match content_type {
-            ContentType::Association | ContentType::Jmux => {}
+            ContentType::Association | ContentType::Jmux | ContentType::Kdc => {}
             _ => anyhow::bail!("Subkey can't be used to sign a {content_type:?} token"),
         }
+
+        // Subkeys can only be used to sign short-lived token
+        if claims
+            .get("nbf")
+            .and_then(Value::as_i64)
+            .zip(claims.get("exp").and_then(Value::as_i64))
+            .into_iter()
+            .any(|(nbf, exp)| exp - nbf > MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS)
+        {
+            anyhow::bail!("invalid `nbf` and `exp` claims for subkey-signed token");
+        }
     }
 
     if let Some(Value::String(expected_id)) = claims.get("jet_gw_id") {

devolutions-gateway/tests/token_security.rs

@@ -1,5 +1,7 @@
 use anyhow::Context as _;
-use devolutions_gateway::token::{new_token_cache, ApplicationProtocol, JrlTokenClaims, Protocol, Subkey};
+use devolutions_gateway::token::{
+    new_token_cache, ApplicationProtocol, JrlTokenClaims, Protocol, Subkey, MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS,
+};
 use devolutions_gateway_generators::*;
 use parking_lot::Mutex;
 use picky::jose::jwe;
@@ -381,6 +383,7 @@ fn jet_gw_id(this_gw_id: Uuid) -> impl Strategy<Value = Option<Uuid>> {
 /// Assert that a token is refused if it doesn't conform to the scope rules:
 /// - Gateway ID if `jet_gw_id` claim is present
 /// - Content Type if `kid` header parameter is present and a subkey is used
+/// - The validity duration is small enough when a subkey is used
 #[rstest]
 fn with_scopes(
     jrl: Mutex<JrlTokenClaims>,
@@ -410,7 +413,12 @@ fn with_scopes(
         let should_succeed = should_succeed
             && match (use_subkey, &claims) {
                 (false, _) => true,
-                (true, TokenClaims::Jmux(_) | TokenClaims::Association(_)) => true,
+                (
+                    true,
+                    TokenClaims::Jmux(JmuxClaims { nbf, exp, .. })
+                    | TokenClaims::Association(AssociationClaims { nbf, exp, .. })
+                    | TokenClaims::Kdc(KdcClaims { nbf, exp, .. }),
+                ) => exp - nbf <= MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS,
                 (true, _) => false,
             };
 
@@ -470,11 +478,14 @@ fn with_scopes(
     );
 }
 
-fn association_or_jmux_claims(now: i64) -> impl Strategy<Value = TokenClaims> {
-    prop_oneof![
-        any_jmux_claims(now).prop_map(TokenClaims::Jmux),
-        any_association_claims(now).prop_map(TokenClaims::Association),
-    ]
+fn subkey_compatible_claims(now: i64) -> impl Strategy<Value = TokenClaims> {
+    (15..MAX_SUBKEY_TOKEN_VALIDITY_DURATION_SECS).prop_flat_map(move |validity_duration| {
+        prop_oneof![
+            any_jmux_claims(now, validity_duration).prop_map(TokenClaims::Jmux),
+            any_association_claims(now, validity_duration).prop_map(TokenClaims::Association),
+            any_kdc_claims(now, validity_duration).prop_map(TokenClaims::Kdc),
+        ]
+    })
 }
 
 /// Randomly choose between the provided kid and a newly generated one
@@ -552,7 +563,7 @@ fn with_subkey(
 
     proptest!(
         ProptestConfig::with_cases(8),
-        |(kid in kid(&subkey_metadata.kid), claims in association_or_jmux_claims(now).no_shrink())| {
+        |(kid in kid(&subkey_metadata.kid), claims in subkey_compatible_claims(now).no_shrink())| {
             test_impl(kid, claims).map_err(|e| TestCaseError::fail(format!("{:#}", e)))?;
         }
     );