Clamp TTL on device token confirmed with existing device token (#9411)

oxidecomputer/customer-support#662

Author: david-crespo

nexus/auth/src/authn/external/mod.rs

@@ -236,6 +236,7 @@ mod test {
                 SKIP => SchemeResult::NotRequested,
                 OK => SchemeResult::Authenticated(authn::Details {
                     actor: self.actor,
+                    device_token_expiration: None,
                 }),
                 FAIL => SchemeResult::Failed(Reason::BadCredentials {
                     actor: self.actor,

nexus/auth/src/authn/external/scim.rs

@@ -66,7 +66,10 @@ where
             Ok(None) => SchemeResult::NotRequested,
             Ok(Some(token)) => match ctx.scim_token_actor(token).await {
                 Err(error) => SchemeResult::Failed(error),
-                Ok(actor) => SchemeResult::Authenticated(Details { actor }),
+                Ok(actor) => SchemeResult::Authenticated(Details {
+                    actor,
+                    device_token_expiration: None,
+                }),
             },
         }
     }

nexus/auth/src/authn/external/session_cookie.rs

@@ -180,7 +180,10 @@ where
             debug!(log, "failed to extend session")
         }
 
-        SchemeResult::Authenticated(Details { actor })
+        SchemeResult::Authenticated(Details {
+            actor,
+            device_token_expiration: None,
+        })
     }
 }
 
@@ -395,7 +398,10 @@ mod test {
         let result = authn_with_cookie(&context, Some("session=abc")).await;
         assert!(matches!(
             result,
-            SchemeResult::Authenticated(Details { actor: _ })
+            SchemeResult::Authenticated(Details {
+                actor: _,
+                device_token_expiration: _
+            })
         ));
 
         // valid cookie should have updated time_last_used

nexus/auth/src/authn/external/spoof.rs

@@ -109,7 +109,10 @@ where
                     Err(error) => SchemeResult::Failed(error),
                     Ok(silo_id) => {
                         let actor = Actor::SiloUser { silo_id, silo_user_id };
-                        SchemeResult::Authenticated(Details { actor })
+                        SchemeResult::Authenticated(Details {
+                            actor,
+                            device_token_expiration: None,
+                        })
                     }
                 }
             }

nexus/auth/src/authn/external/token.rs

@@ -11,6 +11,7 @@ use super::SchemeResult;
 use super::SiloUserSilo;
 use crate::authn;
 use async_trait::async_trait;
+use chrono::{DateTime, Utc};
 use headers::HeaderMapExt;
 use headers::authorization::{Authorization, Bearer};
 
@@ -63,9 +64,14 @@ where
         match parse_token(headers.typed_get().as_ref()) {
             Err(error) => SchemeResult::Failed(error),
             Ok(None) => SchemeResult::NotRequested,
-            Ok(Some(token)) => match ctx.token_actor(token).await {
+            Ok(Some(token)) => match ctx.authenticate_token(token).await {
                 Err(error) => SchemeResult::Failed(error),
-                Ok(actor) => SchemeResult::Authenticated(Details { actor }),
+                Ok((actor, device_token_expiration)) => {
+                    SchemeResult::Authenticated(Details {
+                        actor,
+                        device_token_expiration,
+                    })
+                }
             },
         }
     }
@@ -91,7 +97,12 @@ fn parse_token(
 /// A context that can look up a Silo user and client ID from a token.
 #[async_trait]
 pub trait TokenContext {
-    async fn token_actor(&self, token: String) -> Result<authn::Actor, Reason>;
+    /// Returns the actor authenticated by the token and the token's expiration
+    /// time (if any).
+    async fn authenticate_token(
+        &self,
+        token: String,
+    ) -> Result<(authn::Actor, Option<DateTime<Utc>>), Reason>;
 }
 
 #[cfg(test)]

nexus/auth/src/authn/mod.rs

@@ -38,6 +38,7 @@ pub use nexus_db_fixed_data::user_builtin::USER_SAGA_RECOVERY;
 pub use nexus_db_fixed_data::user_builtin::USER_SERVICE_BALANCER;
 
 use crate::authz;
+use chrono::{DateTime, Utc};
 use newtype_derive::NewtypeDisplay;
 use nexus_db_fixed_data::silo::DEFAULT_SILO;
 use nexus_types::external_api::shared::FleetRole;
@@ -84,7 +85,7 @@ impl Context {
         &self,
     ) -> Result<&Actor, omicron_common::api::external::Error> {
         match &self.kind {
-            Kind::Authenticated(Details { actor }, ..) => Ok(actor),
+            Kind::Authenticated(Details { actor, .. }, ..) => Ok(actor),
             Kind::Unauthenticated => {
                 Err(omicron_common::api::external::Error::Unauthenticated {
                     internal_message: "Actor required".to_string(),
@@ -93,6 +94,21 @@ impl Context {
         }
     }
 
+    /// Returns the expiration time if authenticated via a device token.
+    ///
+    /// This is used to prevent token lifetime extension during token creation:
+    /// a new token created using an existing token should not outlive the
+    /// token used to authenticate.
+    pub fn device_token_expiration(&self) -> Option<DateTime<Utc>> {
+        match &self.kind {
+            Kind::Authenticated(
+                Details { device_token_expiration, .. },
+                ..,
+            ) => *device_token_expiration,
+            Kind::Unauthenticated => None,
+        }
+    }
+
     /// Returns the current actor's Silo if they have one or an appropriate
     /// error otherwise
     ///
@@ -216,7 +232,10 @@ impl Context {
     fn context_for_builtin_user(user_builtin_id: BuiltInUserUuid) -> Context {
         Context {
             kind: Kind::Authenticated(
-                Details { actor: Actor::UserBuiltin { user_builtin_id } },
+                Details {
+                    actor: Actor::UserBuiltin { user_builtin_id },
+                    device_token_expiration: None,
+                },
                 None,
             ),
             schemes_tried: Vec::new(),
@@ -234,6 +253,7 @@ impl Context {
                         silo_user_id: USER_TEST_PRIVILEGED.id(),
                         silo_id: USER_TEST_PRIVILEGED.silo_id,
                     },
+                    device_token_expiration: None,
                 },
                 Some(SiloAuthnPolicy::try_from(&*DEFAULT_SILO).unwrap()),
             ),
@@ -261,7 +281,10 @@ impl Context {
     ) -> Context {
         Context {
             kind: Kind::Authenticated(
-                Details { actor: Actor::SiloUser { silo_user_id, silo_id } },
+                Details {
+                    actor: Actor::SiloUser { silo_user_id, silo_id },
+                    device_token_expiration: None,
+                },
                 Some(silo_authn_policy),
             ),
             schemes_tried: Vec::new(),
@@ -273,7 +296,10 @@ impl Context {
     pub fn for_scim(silo_id: Uuid) -> Context {
         Context {
             kind: Kind::Authenticated(
-                Details { actor: Actor::Scim { silo_id } },
+                Details {
+                    actor: Actor::Scim { silo_id },
+                    device_token_expiration: None,
+                },
                 // This should never be non-empty, we don't want the SCIM user
                 // to ever have associated roles.
                 Some(SiloAuthnPolicy::new(BTreeMap::default())),
@@ -391,6 +417,11 @@ enum Kind {
 pub struct Details {
     /// the actor performing the request
     actor: Actor,
+    /// When the device token expires. Present only when authenticating via
+    /// a device token. This is a slightly awkward fit but is included here
+    /// because we need to use this to clamp the expiration time when device
+    /// tokens are confirmed using an existing device token.
+    device_token_expiration: Option<DateTime<Utc>>,
 }
 
 /// Who is performing an operation

nexus/external-api/src/lib.rs

@@ -4130,6 +4130,14 @@ pub trait NexusExternalApi {
     /// This endpoint is designed to be accessed by the user agent (browser),
     /// not the client requesting the token. So we do not actually return the
     /// token here; it will be returned in response to the poll on `/device/token`.
+    ///
+    /// Some special logic applies when authenticating this request with an
+    /// existing device token instead of a console session: the requested
+    /// TTL must not produce an expiration time later than the authenticating
+    /// token's expiration. If no TTL was specified in the initial grant
+    /// request, the expiration will be the lesser of the silo max and the
+    /// authenticating token's expiration time. To get the longest allowed
+    /// lifetime, omit the TTL and authenticate with a web console session.
     #[endpoint {
         method = POST,
         path = "/device/confirm",

nexus/src/app/device_auth.rs

@@ -123,20 +123,67 @@ impl super::Nexus {
         let silo_max_ttl = silo_auth_settings.device_token_max_ttl_seconds;
         let requested_ttl = db_request.token_ttl_seconds;
 
-        // Validate the requested TTL against the silo's max TTL
-        if let (Some(requested), Some(max)) = (requested_ttl, silo_max_ttl) {
-            if requested > max.0.into() {
-                return Err(Error::invalid_request(&format!(
-                    "Requested TTL {} seconds exceeds maximum \
-                     allowed TTL for this silo of {} seconds",
-                    requested, max
-                )));
+        // This logic is a bit gnarly, but we landed on it as the least bad
+        // option. Error out if the user requests a token TTL that is longer
+        // than allowed, i.e., either
+        //
+        //   a) it is longer than the silo max TTL, or
+        //   b) this request was authenticated with a device token and the TTL
+        //      would produce an expiration later than the current token's.
+        //
+        // If the user does not request a specific TTL, we do not error out.
+        // We calculate the token TTL as min(silo max TTL, current token TTL
+        // if present). Token confirm requests authenticated with a console
+        // session can get device tokens with TTLs up to the silo max.
+
+        let time_expires = if let Some(requested_ttl) = requested_ttl {
+            // If the user requested a TTL, validate it against the silo max
+            // TTL as well as the expiration time of the token being used (if a
+            // token is being used)
+
+            // Validate the requested TTL against the silo's max TTL
+            if let Some(max) = silo_max_ttl {
+                if requested_ttl > max.0.into() {
+                    return Err(Error::invalid_request(&format!(
+                        "Requested TTL {} seconds exceeds maximum allowed \
+                         TTL for this silo of {} seconds",
+                        requested_ttl, max
+                    )));
+                }
+            };
+
+            let requested_exp =
+                Utc::now() + Duration::seconds(requested_ttl.0.into());
+
+            // If currently authenticated via token, error if requested exceeds it
+            if let Some(auth_exp) = opctx.authn.device_token_expiration() {
+                if requested_exp > auth_exp {
+                    return Err(Error::invalid_request(
+                        "Requested token TTL would exceed the expiration time \
+                         of the token being used to authenticate the confirm \
+                         request. To get the full requested TTL, confirm \
+                         this token using a web console session. Alternatively, \
+                         omit requested TTL to get a token with the longest \
+                         allowed lifetime, determined by the lesser of the silo \
+                         max and the current token's expiration time.",
+                    ));
+                }
             }
-        }
 
-        let time_expires = requested_ttl
-            .or(silo_max_ttl)
-            .map(|ttl| Utc::now() + Duration::seconds(ttl.0.into()));
+            Some(requested_exp)
+        } else {
+            // No explicit TTL requested. Rather than erroring out if silo max
+            // exceeds TTL exceeds expiration time of current token, just clamp.
+            let silo_max_exp = silo_max_ttl
+                .map(|ttl| Utc::now() + Duration::seconds(ttl.0.into()));
+            // a.min(b) doesn't do it because None is always less than Some(_)
+            match (silo_max_exp, opctx.authn.device_token_expiration()) {
+                (Some(silo_exp), Some(token_exp)) => {
+                    Some(silo_exp.min(token_exp))
+                }
+                (a, b) => a.or(b),
+            }
+        };
 
         let token = DeviceAccessToken::new(
             db_request.client_id,
@@ -193,11 +240,12 @@ impl super::Nexus {
 
     /// Look up the actor for which a token was granted.
     /// Corresponds to a request *after* completing the flow above.
-    pub(crate) async fn device_access_token_actor(
+    /// Returns the actor and the token's expiration time (if any).
+    pub(crate) async fn authenticate_token(
         &self,
         opctx: &OpContext,
         token: String,
-    ) -> Result<Actor, Reason> {
+    ) -> Result<(Actor, Option<chrono::DateTime<Utc>>), Reason> {
         let (.., db_access_token) = self
             .db_datastore
             .device_token_lookup_by_token(opctx, token)
@@ -222,7 +270,9 @@ impl super::Nexus {
             })?;
         let silo_id = db_silo_user.silo_id;
 
-        if let Some(time_expires) = db_access_token.time_expires {
+        let expiration = db_access_token.time_expires;
+
+        if let Some(time_expires) = expiration {
             let now = Utc::now();
             if time_expires < now {
                 return Err(Reason::BadCredentials {
@@ -236,7 +286,7 @@ impl super::Nexus {
             }
         }
 
-        Ok(Actor::SiloUser { silo_user_id, silo_id })
+        Ok((Actor::SiloUser { silo_user_id, silo_id }, expiration))
     }
 
     pub(crate) async fn device_access_token(

nexus/src/context.rs

@@ -458,12 +458,15 @@ impl authn::external::SiloUserSilo for ServerContext {
 
 #[async_trait]
 impl authn::external::token::TokenContext for ServerContext {
-    async fn token_actor(
+    async fn authenticate_token(
         &self,
         token: String,
-    ) -> Result<authn::Actor, authn::Reason> {
+    ) -> Result<
+        (authn::Actor, Option<chrono::DateTime<chrono::Utc>>),
+        authn::Reason,
+    > {
         let opctx = self.nexus.opctx_external_authn();
-        self.nexus.device_access_token_actor(opctx, token).await
+        self.nexus.authenticate_token(opctx, token).await
     }
 }