diff --git a/Dockerfile b/Dockerfile index 8788b2a..46bc245 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,8 +15,6 @@ COPY . /usr/src/libkrimes WORKDIR /usr/src/libkrimes -RUN cargo build --release - RUN --mount=type=cache,id=cargo,target=/cargo \ export CARGO_HOME=/cargo && \ cargo build \ diff --git a/kdc/src/main.rs b/kdc/src/main.rs index 4c545b7..3daca87 100644 --- a/kdc/src/main.rs +++ b/kdc/src/main.rs @@ -271,7 +271,8 @@ async fn process_ticket_renewal( ) { Ok(time_bounds) => time_bounds, Err(time_bound_error) => { - return Err(time_bound_error.to_kerberos_reply(&service_name, stime)) + error!(?time_bound_error); + return Err(time_bound_error.to_kerberos_reply(&service_name, stime)); } }; diff --git a/libkrimes/src/proto/reply/as_rep.rs b/libkrimes/src/proto/reply/as_rep.rs index 8a54931..a66643b 100644 --- a/libkrimes/src/proto/reply/as_rep.rs +++ b/libkrimes/src/proto/reply/as_rep.rs @@ -8,6 +8,7 @@ use crate::proto::{ TicketFlags, }; use std::time::SystemTime; +use tracing::trace; #[derive(Debug)] pub struct AuthenticationReply { @@ -110,6 +111,8 @@ impl AuthenticationReplyBuilder { encrypted_pa_data: None, }; + trace!(?enc_kdc_rep_part); + let (etype_info2, enc_part) = user_key.encrypt_as_rep_part(enc_kdc_rep_part)?; let transited = TransitedEncoding { diff --git a/libkrimes/src/proto/reply/tgs_rep.rs b/libkrimes/src/proto/reply/tgs_rep.rs index d436a74..231e397 100644 --- a/libkrimes/src/proto/reply/tgs_rep.rs +++ b/libkrimes/src/proto/reply/tgs_rep.rs @@ -12,6 +12,7 @@ use crate::proto::{ Ticket, TicketFlags, }; use crypto_glue::der::{asn1::OctetString, Encode}; +use tracing::trace; #[derive(Debug)] pub struct TicketGrantReply { @@ -273,6 +274,8 @@ impl KerberosReplyTicketRenewBuilder { encrypted_pa_data: None, }; + trace!(?enc_kdc_rep_part); + let enc_part = if let Some(sub_session_key) = self.sub_session_key { sub_session_key.encrypt_tgs_rep_part(enc_kdc_rep_part, true)? } else { @@ -289,8 +292,7 @@ impl KerberosReplyTicketRenewBuilder { contents: OctetString::new(*b"").map_err(|_| KrbError::DerEncodeOctetString)?, }; - // EncTicketPart - // Encrypted to the key of the service + // Encrypted to the key of the kdc let ticket_inner = EncTicketPart { flags: self.ticket.flags, key: session_key, @@ -305,6 +307,8 @@ impl KerberosReplyTicketRenewBuilder { authorization_data, }; + trace!(?ticket_inner); + let ticket_enc_part = primary_key.encrypt_tgs(ticket_inner)?; let ticket = EncTicket { diff --git a/libkrimes/src/proto/time.rs b/libkrimes/src/proto/time.rs index 67a9672..790a492 100644 --- a/libkrimes/src/proto/time.rs +++ b/libkrimes/src/proto/time.rs @@ -7,6 +7,7 @@ use std::time::{Duration, SystemTime}; use tracing::{trace, warn}; +#[derive(Debug)] pub enum TimeBoundError { Skew, NeverValid, @@ -37,6 +38,7 @@ impl TimeBoundError { } } +#[derive(Debug)] pub struct AuthenticationTimeBound { auth_time: SystemTime, start_time: SystemTime, @@ -258,6 +260,7 @@ fn as_req_end_time( * - The starttime of the ticket plus the maximum renewable lifetime * set by the policy of the local realm. */ +#[tracing::instrument] fn as_req_renew_until( start_time: SystemTime, end_time: SystemTime, @@ -265,39 +268,51 @@ fn as_req_renew_until( maximum_renew_lifetime: Option, kdc_options: KerberosFlags, ) -> Result, TimeBoundError> { - let renewal_flags_set = kdc_options.contains(KerberosFlags::RenewableOk) - || kdc_options.contains(KerberosFlags::Renewable); + let renewable_flags_set = kdc_options.contains(KerberosFlags::Renewable); + let renewable_ok_flags_set = kdc_options.contains(KerberosFlags::RenewableOk); match ( requested_renew_until, maximum_renew_lifetime, - renewal_flags_set, + renewable_flags_set, + renewable_ok_flags_set, ) { - (Some(_), _, false) => { + (Some(_), _, false, _) => { // Requested a renew until but no flags! Err(TimeBoundError::FlagsInconsistent) } - (_, None, _) => { + (_, _, false, false) => { + // No renewable flags provided. + Ok(None) + } + (_, None, true, _) => { // Requested a renew, but it's denied Err(TimeBoundError::RenewalNotAllowed) } - (None, Some(maximum_renew_lifetime), _) => { - // Easy, just default to our renew lifetime. - Ok(Some(start_time + maximum_renew_lifetime)) + (None, _, false, true) => { + // The client has indicated that renewable is okay, but MIT KRB + // does not seem to handle this correctly and attempts to bind the + // renew time to its requested end time which is invalid. + // https://github.com/krb5/krb5/blob/master/src/lib/krb5/krb/get_in_tkt.c#L255 + Ok(None) } - (Some(requested_renew_until), Some(maximum_renew_lifetime), _) => { - // This is the hard path, we have to validate things. + (None, Some(maximum_renew_lifetime), true, false) + | (None, Some(maximum_renew_lifetime), true, true) => { + let renew_until = start_time + maximum_renew_lifetime; + Ok(Some(renew_until)) + } + (Some(requested_renew_until), Some(maximum_renew_lifetime), true, _) => { + // This is the sensible path, we have to validate things. if requested_renew_until < end_time { // The renewal would never be valid. return Err(TimeBoundError::NeverValid); } // Take the smaller of requested renew until and the maximum window. - Ok(Some(cmp::min( - requested_renew_until, - start_time + maximum_renew_lifetime, - ))) + let renew_until = cmp::min(requested_renew_until, start_time + maximum_renew_lifetime); + + Ok(Some(renew_until)) } } } @@ -340,6 +355,7 @@ impl TicketGrantTimeBound { self.renew_until } + #[tracing::instrument] pub fn from_tgs_req( current_time: SystemTime, maximum_clock_skew: Duration, @@ -368,7 +384,6 @@ impl TicketGrantTimeBound { let end_time = tgs_req_end_time( start_time, tgs_req_valid.requested_end_time(), - client_tgt.end_time(), client_tgt.renew_until(), maximum_service_ticket_lifetime, )?; @@ -384,6 +399,7 @@ impl TicketGrantTimeBound { } } +#[tracing::instrument] fn tgs_req_start_time( current_time: SystemTime, requested_start_time: Option, @@ -400,8 +416,10 @@ fn tgs_req_start_time( requested_start_time }; - // The requested start time can't be *less* than the tgt start time. - let requested_start_time = cmp::min(requested_start_time, client_tgt_start_time); + // The requested start time can't be *less* than the existing tgt start time. + if requested_start_time < client_tgt_start_time { + return Err(TimeBoundError::Skew); + } // The requested start time needs to be "near" the current time at least. if !is_within_allowed_skew(current_time, requested_start_time, maximum_clock_skew) { @@ -416,41 +434,46 @@ fn tgs_req_start_time( Ok(requested_start_time) } +#[tracing::instrument] fn tgs_req_end_time( start_time: SystemTime, requested_end_time: SystemTime, - client_tgt_end_time: SystemTime, client_tgt_renew_until: Option, maximum_service_ticket_lifetime: Duration, ) -> Result { - // Clamp the end time to the maximum allowable. - let requested_end_time = match requested_end_time.duration_since(start_time) { - Ok(diff) => { - if diff > maximum_service_ticket_lifetime { - // Clamp to the maximum lifetime. - start_time + maximum_service_ticket_lifetime - } else { - // It's less than, so this time is valid. - requested_end_time - } - } - // Some clients send epoch to mean "just fuck my shit up fam", so - // we set a reasonable default here. - Err(_) if requested_end_time == SystemTime::UNIX_EPOCH => { - start_time + maximum_service_ticket_lifetime - } - // The end time was less than start time. - Err(_) => return Err(TimeBoundError::NeverValid), + let policy_end_time = start_time + maximum_service_ticket_lifetime; + + let mut requested_end_time = if requested_end_time == SystemTime::UNIX_EPOCH { + trace!(?policy_end_time, "req epoch"); + policy_end_time + } else { + trace!( + ?policy_end_time, + ?requested_end_time, + "min of request + policy" + ); + cmp::min(requested_end_time, policy_end_time) }; - // We bound to either the renew time if present, or the tgt_end time. - let clamp_bound = client_tgt_renew_until.unwrap_or(client_tgt_end_time); + if let Some(client_tgt_renew_until) = client_tgt_renew_until { + trace!( + ?client_tgt_renew_until, + ?requested_end_time, + "min of request + renew" + ); + // The client_tgt_renew_until is the "true" expiration of the the session. + requested_end_time = cmp::min(client_tgt_renew_until, requested_end_time); + }; - let requested_end_time = cmp::min(requested_end_time, clamp_bound); + if requested_end_time < start_time { + // The end time was less than start time. + return Err(TimeBoundError::NeverValid); + }; Ok(requested_end_time) } +#[derive(Debug)] pub struct TicketRenewTimeBound { start_time: SystemTime, end_time: SystemTime, @@ -470,6 +493,7 @@ impl TicketRenewTimeBound { self.renew_until } + #[tracing::instrument] pub fn from_tgs_req( current_time: SystemTime, maximum_clock_skew: Duration, @@ -486,6 +510,12 @@ impl TicketRenewTimeBound { let client_tgt = tgs_req_valid.ticket_granting_ticket(); + // Ensure that current_time is at least equal or greater + // than the tgt auth_time (when it was issued). This checks + // for clock step backs. + + let current_time = cmp::max(current_time, client_tgt.auth_time()); + // We currently default the renew until here to the client tgt, but in // future we may be able to make server aware choices to clamp this during // the renewal to expire sessions of bad actors. @@ -500,13 +530,14 @@ impl TicketRenewTimeBound { tgs_req_valid.requested_start_time(), maximum_clock_skew, client_tgt.start_time(), - client_tgt.end_time(), + // For a renewable user ticket, the end time here is when the renewal + // period ends, not the end time of the original tgt. + renew_until, )?; let end_time = tgs_req_end_time( start_time, tgs_req_valid.requested_end_time(), - client_tgt.end_time(), client_tgt.renew_until(), maximum_ticket_lifetime, )?;