From 68d788a9f78209109f3013c613caad5c7d200d2d Mon Sep 17 00:00:00 2001 From: Firstyear Date: Wed, 21 Feb 2024 10:15:43 +1000 Subject: [PATCH] 20240216 308 resource limits (#2559) This adds account policy based resource limits to control the maximum number of entries that an account may query --- libs/client/src/group.rs | 24 ++++++++ proto/src/constants.rs | 2 + proto/src/v1.rs | 4 ++ server/lib/src/be/mod.rs | 19 ++++-- server/lib/src/constants/acp.rs | 53 +++++++++++++++++ server/lib/src/constants/entries.rs | 6 ++ server/lib/src/constants/mod.rs | 23 +++++++- server/lib/src/constants/schema.rs | 37 ++++++++++++ server/lib/src/constants/uuids.rs | 4 ++ server/lib/src/filter.rs | 14 ++--- server/lib/src/idm/account.rs | 31 +++++++--- server/lib/src/idm/accountpolicy.rs | 65 ++++++++++++++++++++- server/lib/src/idm/authsession.rs | 15 +---- server/lib/src/idm/oauth2.rs | 11 ++-- server/lib/src/idm/server.rs | 71 ++++++++++++++++++++--- server/lib/src/server/identity.rs | 4 ++ server/lib/src/server/migrations.rs | 26 +++++++++ tools/cli/src/cli/group/account_policy.rs | 32 ++++++++++ tools/cli/src/opt/kanidm.rs | 31 ++++++++-- 19 files changed, 416 insertions(+), 56 deletions(-) diff --git a/libs/client/src/group.rs b/libs/client/src/group.rs index 905faa556..7fec9ad5f 100644 --- a/libs/client/src/group.rs +++ b/libs/client/src/group.rs @@ -68,4 +68,28 @@ impl KanidmClient { ) .await } + + pub async fn group_account_policy_limit_search_max_results( + &self, + id: &str, + maximum: u32, + ) -> Result<(), ClientError> { + self.perform_put_request( + &format!("/v1/group/{}/_attr/limit_search_max_results", id), + vec![maximum.to_string()], + ) + .await + } + + pub async fn group_account_policy_limit_search_max_filter_test( + &self, + id: &str, + maximum: u32, + ) -> Result<(), ClientError> { + self.perform_put_request( + &format!("/v1/group/{}/_attr/limit_search_max_filter_test", id), + vec![maximum.to_string()], + ) + .await + } } diff --git a/proto/src/constants.rs b/proto/src/constants.rs index 7abe5ee2e..29adff79d 100644 --- a/proto/src/constants.rs +++ b/proto/src/constants.rs @@ -88,6 +88,8 @@ pub const ATTR_ENTRYDN: &str = "entrydn"; pub const ATTR_ENTRY_MANAGED_BY: &str = "entry_managed_by"; pub const ATTR_ENTRYUUID: &str = "entryuuid"; pub const ATTR_LDAP_KEYS: &str = "keys"; +pub const ATTR_LIMIT_SEARCH_MAX_RESULTS: &str = "limit_search_max_results"; +pub const ATTR_LIMIT_SEARCH_MAX_FILTER_TEST: &str = "limit_search_max_filter_test"; pub const ATTR_EXCLUDES: &str = "excludes"; pub const ATTR_ES256_PRIVATE_KEY_DER: &str = "es256_private_key_der"; pub const ATTR_FERNET_PRIVATE_KEY_STR: &str = "fernet_private_key_str"; diff --git a/proto/src/v1.rs b/proto/src/v1.rs index 214f27264..94f61cb22 100644 --- a/proto/src/v1.rs +++ b/proto/src/v1.rs @@ -451,6 +451,7 @@ pub enum UatPurpose { /// point onward! This means on updates, that sessions will invalidate in many /// cases. #[derive(Debug, Serialize, Deserialize, Clone, ToSchema)] +#[skip_serializing_none] #[serde(rename_all = "lowercase")] pub struct UserAuthToken { pub session_id: Uuid, @@ -466,6 +467,9 @@ pub struct UserAuthToken { pub spn: String, pub mail_primary: Option, pub ui_hints: BTreeSet, + + pub limit_search_max_results: Option, + pub limit_search_max_filter_test: Option, } impl fmt::Display for UserAuthToken { diff --git a/server/lib/src/be/mod.rs b/server/lib/src/be/mod.rs index e803efbc1..bccafa077 100644 --- a/server/lib/src/be/mod.rs +++ b/server/lib/src/be/mod.rs @@ -66,9 +66,9 @@ impl Default for Limits { fn default() -> Self { Limits { unindexed_allow: false, - search_max_results: 256, - search_max_filter_test: 512, - filter_max_elements: 32, + search_max_results: DEFAULT_LIMIT_SEARCH_MAX_RESULTS as usize, + search_max_filter_test: DEFAULT_LIMIT_SEARCH_MAX_FILTER_TEST as usize, + filter_max_elements: DEFAULT_LIMIT_FILTER_MAX_ELEMENTS as usize, } } } @@ -77,11 +77,20 @@ impl Limits { pub fn unlimited() -> Self { Limits { unindexed_allow: true, - search_max_results: usize::MAX, - search_max_filter_test: usize::MAX, + search_max_results: usize::MAX >> 1, + search_max_filter_test: usize::MAX >> 1, filter_max_elements: usize::MAX, } } + + pub fn api_token() -> Self { + Limits { + unindexed_allow: false, + search_max_results: DEFAULT_LIMIT_API_SEARCH_MAX_RESULTS as usize, + search_max_filter_test: DEFAULT_LIMIT_API_SEARCH_MAX_FILTER_TEST as usize, + filter_max_elements: DEFAULT_LIMIT_FILTER_MAX_ELEMENTS as usize, + } + } } #[derive(Debug, Clone)] diff --git a/server/lib/src/constants/acp.rs b/server/lib/src/constants/acp.rs index ec04de5a1..aa431ef16 100644 --- a/server/lib/src/constants/acp.rs +++ b/server/lib/src/constants/acp.rs @@ -532,6 +532,59 @@ lazy_static! { }; } +lazy_static! { + pub static ref IDM_ACP_GROUP_ACCOUNT_POLICY_MANAGE_DL6: BuiltinAcp = BuiltinAcp { + classes: vec![ + EntryClass::Object, + EntryClass::AccessControlProfile, + EntryClass::AccessControlModify, + EntryClass::AccessControlSearch + ], + name: "idm_acp_group_account_policy_manage", + uuid: UUID_IDM_ACP_GROUP_ACCOUNT_POLICY_MANAGE, + description: "Builtin IDM Control for management of account policy on groups", + receiver: BuiltinAcpReceiver::Group(vec![UUID_IDM_ACCOUNT_POLICY_ADMINS]), + target: BuiltinAcpTarget::Filter(ProtoFilter::And(vec![ + match_class_filter!(EntryClass::Group), + FILTER_ANDNOT_TOMBSTONE_OR_RECYCLED.clone() + ])), + search_attrs: vec![ + Attribute::Class, + Attribute::Name, + Attribute::Uuid, + Attribute::AuthSessionExpiry, + Attribute::AuthPasswordMinimumLength, + Attribute::CredentialTypeMinimum, + Attribute::PrivilegeExpiry, + Attribute::WebauthnAttestationCaList, + Attribute::LimitSearchMaxResults, + Attribute::LimitSearchMaxFilterTest, + ], + modify_removed_attrs: vec![ + Attribute::Class, + Attribute::AuthSessionExpiry, + Attribute::AuthPasswordMinimumLength, + Attribute::CredentialTypeMinimum, + Attribute::PrivilegeExpiry, + Attribute::WebauthnAttestationCaList, + Attribute::LimitSearchMaxResults, + Attribute::LimitSearchMaxFilterTest, + ], + modify_present_attrs: vec![ + Attribute::Class, + Attribute::AuthSessionExpiry, + Attribute::AuthPasswordMinimumLength, + Attribute::CredentialTypeMinimum, + Attribute::PrivilegeExpiry, + Attribute::WebauthnAttestationCaList, + Attribute::LimitSearchMaxResults, + Attribute::LimitSearchMaxFilterTest, + ], + modify_classes: vec![EntryClass::AccountPolicy,], + ..Default::default() + }; +} + lazy_static! { pub static ref IDM_ACP_OAUTH2_MANAGE_V1: BuiltinAcp = BuiltinAcp { classes: vec![ diff --git a/server/lib/src/constants/entries.rs b/server/lib/src/constants/entries.rs index 1cbb2ebe9..03ebbd972 100644 --- a/server/lib/src/constants/entries.rs +++ b/server/lib/src/constants/entries.rs @@ -105,6 +105,8 @@ pub enum Attribute { /// An LDAP Compatible sshkeys virtual attribute LdapKeys, LegalName, + LimitSearchMaxResults, + LimitSearchMaxFilterTest, LoginShell, Mail, May, @@ -293,6 +295,8 @@ impl TryFrom for Attribute { ATTR_SSH_PUBLICKEY => Attribute::SshPublicKey, ATTR_LEGALNAME => Attribute::LegalName, ATTR_LOGINSHELL => Attribute::LoginShell, + ATTR_LIMIT_SEARCH_MAX_RESULTS => Attribute::LimitSearchMaxResults, + ATTR_LIMIT_SEARCH_MAX_FILTER_TEST => Attribute::LimitSearchMaxFilterTest, ATTR_MAIL => Attribute::Mail, ATTR_MAY => Attribute::May, ATTR_MEMBER => Attribute::Member, @@ -456,6 +460,8 @@ impl From for &'static str { Attribute::LdapKeys => ATTR_LDAP_KEYS, Attribute::LdapSshPublicKey => ATTR_LDAP_SSHPUBLICKEY, Attribute::LegalName => ATTR_LEGALNAME, + Attribute::LimitSearchMaxResults => ATTR_LIMIT_SEARCH_MAX_RESULTS, + Attribute::LimitSearchMaxFilterTest => ATTR_LIMIT_SEARCH_MAX_FILTER_TEST, Attribute::LoginShell => ATTR_LOGINSHELL, Attribute::Mail => ATTR_MAIL, Attribute::May => ATTR_MAY, diff --git a/server/lib/src/constants/mod.rs b/server/lib/src/constants/mod.rs index 2880d0f2c..43148f35b 100644 --- a/server/lib/src/constants/mod.rs +++ b/server/lib/src/constants/mod.rs @@ -49,14 +49,15 @@ pub const DOMAIN_LEVEL_2: DomainVersion = 2; pub const DOMAIN_LEVEL_3: DomainVersion = 3; pub const DOMAIN_LEVEL_4: DomainVersion = 4; pub const DOMAIN_LEVEL_5: DomainVersion = 5; +pub const DOMAIN_LEVEL_6: DomainVersion = 6; // The minimum level that we can re-migrate from pub const DOMAIN_MIN_REMIGRATION_LEVEL: DomainVersion = DOMAIN_LEVEL_2; // The minimum supported domain functional level -pub const DOMAIN_MIN_LEVEL: DomainVersion = DOMAIN_LEVEL_5; +pub const DOMAIN_MIN_LEVEL: DomainVersion = DOMAIN_TGT_LEVEL; // The target supported domain functional level -pub const DOMAIN_TGT_LEVEL: DomainVersion = DOMAIN_LEVEL_5; +pub const DOMAIN_TGT_LEVEL: DomainVersion = DOMAIN_LEVEL_6; // The maximum supported domain functional level -pub const DOMAIN_MAX_LEVEL: DomainVersion = DOMAIN_LEVEL_5; +pub const DOMAIN_MAX_LEVEL: DomainVersion = DOMAIN_LEVEL_6; // On test builds define to 60 seconds #[cfg(test)] @@ -109,3 +110,19 @@ pub const OAUTH2_ACCESS_TOKEN_EXPIRY: u32 = 15 * 60; /// The amount of time a suppliers clock can be "ahead" before /// we warn about possible clock synchronisation issues. pub const REPL_SUPPLIER_ADVANCE_WINDOW: Duration = Duration::from_secs(600); + +/// The default number of entries that a user may retrieve in a search +pub const DEFAULT_LIMIT_SEARCH_MAX_RESULTS: u64 = 1024; +/// The default number of entries than an api token may retrieve in a search; +pub const DEFAULT_LIMIT_API_SEARCH_MAX_RESULTS: u64 = u64::MAX >> 1; +/// the default number of entries that may be examined in a partially indexed +/// query. +pub const DEFAULT_LIMIT_SEARCH_MAX_FILTER_TEST: u64 = 2048; +/// the default number of entries that may be examined in a partially indexed +/// query by an api token. +pub const DEFAULT_LIMIT_API_SEARCH_MAX_FILTER_TEST: u64 = 16384; +/// The maximum number of items in a filter, regardless of nesting level. +pub const DEFAULT_LIMIT_FILTER_MAX_ELEMENTS: u64 = 32; + +/// The maximum amount of recursion allowed in a filter. +pub const DEFAULT_LIMIT_FILTER_DEPTH_MAX: u64 = 12; diff --git a/server/lib/src/constants/schema.rs b/server/lib/src/constants/schema.rs index 35535e0ab..cfb9bbeb6 100644 --- a/server/lib/src/constants/schema.rs +++ b/server/lib/src/constants/schema.rs @@ -618,6 +618,26 @@ pub static ref SCHEMA_ATTR_CREDENTIAL_TYPE_MINIMUM: SchemaAttribute = SchemaAttr ..Default::default() }; +pub static ref SCHEMA_ATTR_LIMIT_SEARCH_MAX_RESULTS_DL6: SchemaAttribute = SchemaAttribute { + uuid: UUID_SCHEMA_ATTR_LIMIT_SEARCH_MAX_RESULTS, + name: Attribute::LimitSearchMaxResults.into(), + description: "The maximum number of query results that may be returned in a single operation.".to_string(), + + multivalue: false, + syntax: SyntaxType::Uint32, + ..Default::default() +}; + +pub static ref SCHEMA_ATTR_LIMIT_SEARCH_MAX_FILTER_TEST_DL6: SchemaAttribute = SchemaAttribute { + uuid: UUID_SCHEMA_ATTR_LIMIT_SEARCH_MAX_FILTER_TEST, + name: Attribute::LimitSearchMaxFilterTest.into(), + description: "The maximum number of entries that may be examined in a partially indexed query".to_string(), + + multivalue: false, + syntax: SyntaxType::Uint32, + ..Default::default() +}; + // === classes === pub static ref SCHEMA_CLASS_PERSON: SchemaClass = SchemaClass { @@ -722,6 +742,23 @@ pub static ref SCHEMA_CLASS_ACCOUNT_POLICY: SchemaClass = SchemaClass { ..Default::default() }; +pub static ref SCHEMA_CLASS_ACCOUNT_POLICY_DL6: SchemaClass = SchemaClass { + uuid: UUID_SCHEMA_CLASS_ACCOUNT_POLICY, + name: EntryClass::AccountPolicy.into(), + description: "Policies applied to accounts that are members of a group".to_string(), + systemmay: vec![ + Attribute::AuthSessionExpiry.into(), + Attribute::PrivilegeExpiry.into(), + Attribute::AuthPasswordMinimumLength.into(), + Attribute::CredentialTypeMinimum.into(), + Attribute::WebauthnAttestationCaList.into(), + Attribute::LimitSearchMaxResults.into(), + Attribute::LimitSearchMaxFilterTest.into(), + ], + systemsupplements: vec![Attribute::Group.into()], + ..Default::default() +}; + pub static ref SCHEMA_CLASS_ACCOUNT: SchemaClass = SchemaClass { uuid: UUID_SCHEMA_CLASS_ACCOUNT, name: EntryClass::Account.into(), diff --git a/server/lib/src/constants/uuids.rs b/server/lib/src/constants/uuids.rs index c3d3e6ff6..a3e0147d2 100644 --- a/server/lib/src/constants/uuids.rs +++ b/server/lib/src/constants/uuids.rs @@ -276,6 +276,10 @@ pub const UUID_SCHEMA_ATTR_OAUTH2_RS_CLAIM_MAP: Uuid = uuid!("00000000-0000-0000-0000-ffff00000159"); pub const UUID_SCHEMA_ATTR_RECYCLEDDIRECTMEMBEROF: Uuid = uuid!("00000000-0000-0000-0000-ffff00000160"); +pub const UUID_SCHEMA_ATTR_LIMIT_SEARCH_MAX_RESULTS: Uuid = + uuid!("00000000-0000-0000-0000-ffff00000161"); +pub const UUID_SCHEMA_ATTR_LIMIT_SEARCH_MAX_FILTER_TEST: Uuid = + uuid!("00000000-0000-0000-0000-ffff00000162"); // System and domain infos // I'd like to strongly criticise william of the past for making poor choices about these allocations. diff --git a/server/lib/src/filter.rs b/server/lib/src/filter.rs index 0fc928766..c7944ce78 100644 --- a/server/lib/src/filter.rs +++ b/server/lib/src/filter.rs @@ -32,8 +32,6 @@ use crate::prelude::*; use crate::schema::SchemaTransaction; use crate::value::{IndexType, PartialValue}; -const FILTER_DEPTH_MAX: usize = 16; - // Default filter is safe, ignores all hidden types! // This is &Value so we can lazy const then clone, but perhaps we can reconsider @@ -666,7 +664,7 @@ impl Filter { f: &ProtoFilter, qs: &mut QueryServerReadTransaction, ) -> Result { - let depth = FILTER_DEPTH_MAX; + let depth = DEFAULT_LIMIT_FILTER_DEPTH_MAX as usize; let mut elems = ev.limits.filter_max_elements; Ok(Filter { state: FilterInvalid { @@ -681,7 +679,7 @@ impl Filter { f: &ProtoFilter, qs: &mut QueryServerWriteTransaction, ) -> Result { - let depth = FILTER_DEPTH_MAX; + let depth = DEFAULT_LIMIT_FILTER_DEPTH_MAX as usize; let mut elems = ev.limits.filter_max_elements; Ok(Filter { state: FilterInvalid { @@ -696,7 +694,7 @@ impl Filter { f: &LdapFilter, qs: &mut QueryServerReadTransaction, ) -> Result { - let depth = FILTER_DEPTH_MAX; + let depth = DEFAULT_LIMIT_FILTER_DEPTH_MAX as usize; let mut elems = ev.limits.filter_max_elements; Ok(Filter { state: FilterInvalid { @@ -1580,7 +1578,7 @@ mod tests { use ldap3_proto::simple::LdapFilter; use crate::event::{CreateEvent, DeleteEvent}; - use crate::filter::{Filter, FilterInvalid, FILTER_DEPTH_MAX}; + use crate::filter::{Filter, FilterInvalid, DEFAULT_LIMIT_FILTER_DEPTH_MAX}; use crate::prelude::*; #[test] @@ -2104,12 +2102,12 @@ mod tests { let mut r_txn = server.read().await; let mut inv_proto = ProtoFilter::Pres(Attribute::Class.to_string()); - for _i in 0..(FILTER_DEPTH_MAX + 1) { + for _i in 0..(DEFAULT_LIMIT_FILTER_DEPTH_MAX + 1) { inv_proto = ProtoFilter::And(vec![inv_proto]); } let mut inv_ldap = LdapFilter::Present(Attribute::Class.to_string()); - for _i in 0..(FILTER_DEPTH_MAX + 1) { + for _i in 0..(DEFAULT_LIMIT_FILTER_DEPTH_MAX + 1) { inv_ldap = LdapFilter::And(vec![inv_ldap]); } diff --git a/server/lib/src/idm/account.rs b/server/lib/src/idm/account.rs index c9f0c331e..bc605915a 100644 --- a/server/lib/src/idm/account.rs +++ b/server/lib/src/idm/account.rs @@ -273,7 +273,7 @@ impl Account { session_id: Uuid, scope: SessionScope, ct: Duration, - auth_session_expiry: u32, + account_policy: &ResolvedAccountPolicy, ) -> Option { // TODO: Apply policy to this expiry time. // We have to remove the nanoseconds because when we transmit this / serialise it we drop @@ -281,10 +281,17 @@ impl Account { // ns value which breaks some checks. let ct = ct - Duration::from_nanos(ct.subsec_nanos() as u64); let issued_at = OffsetDateTime::UNIX_EPOCH + ct; + + let limit_search_max_results = account_policy.limit_search_max_results(); + let limit_search_max_filter_test = account_policy.limit_search_max_filter_test(); + // Note that currently the auth_session time comes from policy, but the already-privileged // session bound is hardcoded. - let expiry = - Some(OffsetDateTime::UNIX_EPOCH + ct + Duration::from_secs(auth_session_expiry as u64)); + let expiry = Some( + OffsetDateTime::UNIX_EPOCH + + ct + + Duration::from_secs(account_policy.authsession_expiry() as u64), + ); let limited_expiry = Some( OffsetDateTime::UNIX_EPOCH + ct @@ -319,6 +326,8 @@ impl Account { ui_hints: self.ui_hints.clone(), // application: None, // groups: self.groups.iter().map(|g| g.to_proto()).collect(), + limit_search_max_results, + limit_search_max_filter_test, }) } @@ -331,10 +340,13 @@ impl Account { session_expiry: Option, scope: SessionScope, ct: Duration, - auth_privilege_expiry: u32, + account_policy: &ResolvedAccountPolicy, ) -> Option { let issued_at = OffsetDateTime::UNIX_EPOCH + ct; + let limit_search_max_results = account_policy.limit_search_max_results(); + let limit_search_max_filter_test = account_policy.limit_search_max_filter_test(); + let (purpose, expiry) = match scope { SessionScope::Synchronise | SessionScope::ReadOnly | SessionScope::ReadWrite => { warn!( @@ -349,7 +361,7 @@ impl Account { let expiry = Some( OffsetDateTime::UNIX_EPOCH + ct - + Duration::from_secs(auth_privilege_expiry.into()), + + Duration::from_secs(account_policy.privilege_expiry().into()), ); ( UatPurpose::ReadWrite { expiry }, @@ -373,6 +385,8 @@ impl Account { ui_hints: self.ui_hints.clone(), // application: None, // groups: self.groups.iter().map(|g| g.to_proto()).collect(), + limit_search_max_results, + limit_search_max_filter_test, }) } @@ -909,6 +923,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> { #[cfg(test)] mod tests { use crate::idm::account::Account; + use crate::idm::accountpolicy::ResolvedAccountPolicy; use crate::prelude::*; use kanidm_proto::v1::UiHint; @@ -950,7 +965,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); @@ -981,7 +996,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_PRIVILEGE_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); @@ -1014,7 +1029,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); diff --git a/server/lib/src/idm/accountpolicy.rs b/server/lib/src/idm/accountpolicy.rs index 01f9115a5..1378e53d5 100644 --- a/server/lib/src/idm/accountpolicy.rs +++ b/server/lib/src/idm/accountpolicy.rs @@ -1,15 +1,17 @@ use crate::prelude::*; use crate::value::CredentialType; use webauthn_rs::prelude::AttestationCaList; -// use crate::idm::server::IdmServerProxyWriteTransaction; #[derive(Clone)] +#[cfg_attr(test, derive(Default))] pub(crate) struct AccountPolicy { privilege_expiry: u32, authsession_expiry: u32, pw_min_length: u32, credential_policy: CredentialType, webauthn_att_ca_list: Option, + limit_search_max_filter_test: Option, + limit_search_max_results: Option, } impl From<&EntrySealedCommitted> for Option { @@ -41,12 +43,22 @@ impl From<&EntrySealedCommitted> for Option { .get_ava_webauthn_attestation_ca_list(Attribute::WebauthnAttestationCaList) .cloned(); + let limit_search_max_results = val + .get_ava_single_uint32(Attribute::LimitSearchMaxResults) + .map(|u| u as u64); + + let limit_search_max_filter_test = val + .get_ava_single_uint32(Attribute::LimitSearchMaxFilterTest) + .map(|u| u as u64); + Some(AccountPolicy { privilege_expiry, authsession_expiry, pw_min_length, credential_policy, webauthn_att_ca_list, + limit_search_max_filter_test, + limit_search_max_results, }) } } @@ -59,9 +71,24 @@ pub(crate) struct ResolvedAccountPolicy { pw_min_length: u32, credential_policy: CredentialType, webauthn_att_ca_list: Option, + limit_search_max_filter_test: Option, + limit_search_max_results: Option, } impl ResolvedAccountPolicy { + #[cfg(test)] + pub(crate) fn test_policy() -> Self { + ResolvedAccountPolicy { + privilege_expiry: DEFAULT_AUTH_PRIVILEGE_EXPIRY, + authsession_expiry: DEFAULT_AUTH_SESSION_EXPIRY, + pw_min_length: PW_MIN_LENGTH, + credential_policy: CredentialType::Any, + webauthn_att_ca_list: None, + limit_search_max_filter_test: Some(DEFAULT_LIMIT_SEARCH_MAX_FILTER_TEST), + limit_search_max_results: Some(DEFAULT_LIMIT_SEARCH_MAX_RESULTS), + } + } + pub(crate) fn fold_from(iter: I) -> Self where I: Iterator, @@ -73,6 +100,8 @@ impl ResolvedAccountPolicy { pw_min_length: PW_MIN_LENGTH, credential_policy: CredentialType::Any, webauthn_att_ca_list: None, + limit_search_max_filter_test: None, + limit_search_max_results: None, }; iter.for_each(|acc_pol| { @@ -96,6 +125,26 @@ impl ResolvedAccountPolicy { accumulate.credential_policy = acc_pol.credential_policy } + if let Some(pol_lim) = acc_pol.limit_search_max_results { + if let Some(acc_lim) = accumulate.limit_search_max_results { + if pol_lim > acc_lim { + accumulate.limit_search_max_results = Some(pol_lim); + } + } else { + accumulate.limit_search_max_results = Some(pol_lim); + } + } + + if let Some(pol_lim) = acc_pol.limit_search_max_filter_test { + if let Some(acc_lim) = accumulate.limit_search_max_filter_test { + if pol_lim > acc_lim { + accumulate.limit_search_max_filter_test = Some(pol_lim); + } + } else { + accumulate.limit_search_max_filter_test = Some(pol_lim); + } + } + if let Some(acc_pol_w_att_ca) = acc_pol.webauthn_att_ca_list { if let Some(res_w_att_ca) = accumulate.webauthn_att_ca_list.as_mut() { res_w_att_ca.intersection(&acc_pol_w_att_ca); @@ -127,6 +176,14 @@ impl ResolvedAccountPolicy { pub(crate) fn webauthn_attestation_ca_list(&self) -> Option<&AttestationCaList> { self.webauthn_att_ca_list.as_ref() } + + pub(crate) fn limit_search_max_results(&self) -> Option { + self.limit_search_max_results + } + + pub(crate) fn limit_search_max_filter_test(&self) -> Option { + self.limit_search_max_filter_test + } } #[cfg(test)] @@ -205,6 +262,8 @@ jAGGiQIwHFj+dJZYUJR786osByBelJYsVZd2GbHQu209b5RCmGQ21gpSAk9QZW4B pw_min_length: 11, credential_policy: CredentialType::Mfa, webauthn_att_ca_list: Some(att_ca_list_a), + limit_search_max_filter_test: Some(10), + limit_search_max_results: Some(10), }; let mut att_ca_builder = AttestationCaListBuilder::new(); @@ -224,6 +283,8 @@ jAGGiQIwHFj+dJZYUJR786osByBelJYsVZd2GbHQu209b5RCmGQ21gpSAk9QZW4B pw_min_length: 15, credential_policy: CredentialType::Passkey, webauthn_att_ca_list: Some(att_ca_list_b), + limit_search_max_filter_test: Some(5), + limit_search_max_results: Some(15), }; let rap = ResolvedAccountPolicy::fold_from([policy_a, policy_b].into_iter()); @@ -232,6 +293,8 @@ jAGGiQIwHFj+dJZYUJR786osByBelJYsVZd2GbHQu209b5RCmGQ21gpSAk9QZW4B assert_eq!(rap.authsession_expiry(), 50); assert_eq!(rap.pw_min_length(), 15); assert_eq!(rap.credential_policy, CredentialType::Passkey); + assert_eq!(rap.limit_search_max_results(), Some(15)); + assert_eq!(rap.limit_search_max_filter_test(), Some(10)); let mut att_ca_builder = AttestationCaListBuilder::new(); diff --git a/server/lib/src/idm/authsession.rs b/server/lib/src/idm/authsession.rs index 43f2dd438..c0f4e890f 100644 --- a/server/lib/src/idm/authsession.rs +++ b/server/lib/src/idm/authsession.rs @@ -1275,14 +1275,7 @@ impl AuthSession { ) { CredState::Success { auth_type, cred_id } => { // Issue the uat based on a set of factors. - let uat = self.issue_uat( - &auth_type, - time, - async_tx, - cred_id, - self.account_policy.authsession_expiry(), - self.account_policy.privilege_expiry(), - )?; + let uat = self.issue_uat(&auth_type, time, async_tx, cred_id)?; let jwt = Jws::into_json(&uat).map_err(|e| { admin_error!(?e, "Failed to serialise into Jws"); @@ -1357,8 +1350,6 @@ impl AuthSession { time: Duration, async_tx: &Sender, cred_id: Uuid, - auth_session_expiry: u32, - auth_privilege_expiry: u32, ) -> Result { security_debug!("Successful cred handling"); match self.intent { @@ -1392,7 +1383,7 @@ impl AuthSession { let uat = self .account - .to_userauthtoken(session_id, scope, time, auth_session_expiry) + .to_userauthtoken(session_id, scope, time, &self.account_policy) .ok_or(OperationError::InvalidState)?; // Queue the session info write. @@ -1454,7 +1445,7 @@ impl AuthSession { session_expiry, scope, time, - auth_privilege_expiry, + &self.account_policy, ) .ok_or(OperationError::InvalidState)?; diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs index 7e379221a..d979cae40 100644 --- a/server/lib/src/idm/oauth2.rs +++ b/server/lib/src/idm/oauth2.rs @@ -2555,6 +2555,7 @@ mod tests { use kanidm_proto::v1::UserAuthToken; use openssl::sha; + use crate::idm::accountpolicy::ResolvedAccountPolicy; use crate::idm::oauth2::{AuthoriseResponse, Oauth2Error}; use crate::idm::server::{IdmServer, IdmServerTransaction}; use crate::prelude::*; @@ -2715,7 +2716,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); @@ -2843,7 +2844,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); @@ -2906,7 +2907,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident = idms_prox_write @@ -3238,7 +3239,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident2 = idms_prox_write @@ -3858,7 +3859,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident2 = idms_prox_write diff --git a/server/lib/src/idm/server.rs b/server/lib/src/idm/server.rs index 04a3d654c..725dae937 100644 --- a/server/lib/src/idm/server.rs +++ b/server/lib/src/idm/server.rs @@ -763,7 +763,17 @@ pub trait IdmServerTransaction<'a> { } }; - let limits = Limits::default(); + let mut limits = Limits::default(); + // Apply the limits from the uat + if let Some(lim) = uat.limit_search_max_results.and_then(|v| v.try_into().ok()) { + limits.search_max_results = lim; + } + if let Some(lim) = uat + .limit_search_max_filter_test + .and_then(|v| v.try_into().ok()) + { + limits.search_max_filter_test = lim; + } // #64: Now apply claims from the uat into the Entry // to allow filtering. @@ -806,7 +816,7 @@ pub trait IdmServerTransaction<'a> { let scope = (&apit.purpose).into(); - let limits = Limits::default(); + let limits = Limits::api_token(); Ok(Identity { origin: IdentType::User(IdentUser { entry }), source, @@ -2160,6 +2170,7 @@ mod tests { use crate::credential::{Credential, Password}; use crate::idm::account::DestroySessionTokenEvent; + use crate::idm::accountpolicy::ResolvedAccountPolicy; use crate::idm::audit::AuditEvent; use crate::idm::delayed::{AuthSessionRecord, DelayedAction}; use crate::idm::event::{AuthEvent, AuthResult}; @@ -3843,7 +3854,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident = idms_prox_write @@ -3862,7 +3873,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident = idms_prox_write @@ -3881,7 +3892,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident = idms_prox_write @@ -3900,7 +3911,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident = idms_prox_write @@ -3919,7 +3930,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident = idms_prox_write @@ -3938,7 +3949,7 @@ mod tests { session_id, SessionScope::ReadWrite, ct, - DEFAULT_AUTH_SESSION_EXPIRY, + &ResolvedAccountPolicy::test_policy(), ) .expect("Unable to create uat"); let ident = idms_prox_write @@ -3952,6 +3963,50 @@ mod tests { assert!(!ident.has_claim("authclass_single")); } + #[idm_test] + async fn test_idm_uat_limits_account_policy( + idms: &IdmServer, + _idms_delayed: &mut IdmServerDelayed, + ) { + let ct = Duration::from_secs(TEST_CURRENT_TIME); + let mut idms_prox_write = idms.proxy_write(ct).await; + + idms_prox_write + .qs_write + .internal_create(vec![E_TESTPERSON_1.clone()]) + .expect("Failed to create test person"); + + // get an account. + let account = idms_prox_write + .target_to_account(UUID_TESTPERSON_1) + .expect("account must exist"); + + // Create a fake UATs + let session_id = uuid::Uuid::new_v4(); + + let uat = account + .to_userauthtoken( + session_id, + SessionScope::ReadWrite, + ct, + &ResolvedAccountPolicy::test_policy(), + ) + .expect("Unable to create uat"); + + let ident = idms_prox_write + .process_uat_to_identity(&uat, ct, Source::Internal) + .expect("Unable to process uat"); + + assert_eq!( + ident.limits().search_max_results, + DEFAULT_LIMIT_SEARCH_MAX_RESULTS as usize + ); + assert_eq!( + ident.limits().search_max_filter_test, + DEFAULT_LIMIT_SEARCH_MAX_FILTER_TEST as usize + ); + } + #[idm_test] async fn test_idm_jwt_uat_token_key_reload( idms: &IdmServer, diff --git a/server/lib/src/server/identity.rs b/server/lib/src/server/identity.rs index 2a8c26865..a4d388a81 100644 --- a/server/lib/src/server/identity.rs +++ b/server/lib/src/server/identity.rs @@ -135,6 +135,10 @@ impl Identity { &self.source } + pub fn limits(&self) -> &Limits { + &self.limits + } + pub fn from_internal() -> Self { Identity { origin: IdentType::Internal, diff --git a/server/lib/src/server/migrations.rs b/server/lib/src/server/migrations.rs index 2ac2befb8..8d05a16ec 100644 --- a/server/lib/src/server/migrations.rs +++ b/server/lib/src/server/migrations.rs @@ -860,6 +860,32 @@ impl<'a> QueryServerWriteTransaction<'a> { self.internal_batch_modify(modset.into_iter()) } + /// Migration domain level 5 to 6 - support query limits in account policy. + pub fn migrate_domain_5_to_6(&mut self) -> Result<(), OperationError> { + let idm_schema_classes = [ + SCHEMA_ATTR_LIMIT_SEARCH_MAX_RESULTS_DL6.clone().into(), + SCHEMA_ATTR_LIMIT_SEARCH_MAX_FILTER_TEST_DL6.clone().into(), + SCHEMA_CLASS_ACCOUNT_POLICY_DL6.clone().into(), + ]; + + idm_schema_classes + .into_iter() + .try_for_each(|entry| self.internal_migrate_or_create(entry)) + .map_err(|err| { + error!(?err, "migrate_domain_5_to_6 -> Error"); + err + })?; + + self.reload()?; + + // Update access controls. + self.internal_migrate_or_create(IDM_ACP_GROUP_ACCOUNT_POLICY_MANAGE_DL6.clone().into()) + .map_err(|err| { + error!(?err, "migrate_domain_5_to_6 -> Error"); + err + }) + } + #[instrument(level = "info", skip_all)] pub fn initialise_schema_core(&mut self) -> Result<(), OperationError> { admin_debug!("initialise_schema_core -> start ..."); diff --git a/tools/cli/src/cli/group/account_policy.rs b/tools/cli/src/cli/group/account_policy.rs index 554394b92..d83d0b004 100644 --- a/tools/cli/src/cli/group/account_policy.rs +++ b/tools/cli/src/cli/group/account_policy.rs @@ -9,6 +9,8 @@ impl GroupAccountPolicyOpt { | GroupAccountPolicyOpt::CredentialTypeMinimum { copt, .. } | GroupAccountPolicyOpt::PasswordMinimumLength { copt, .. } | GroupAccountPolicyOpt::WebauthnAttestationCaList { copt, .. } + | GroupAccountPolicyOpt::LimitSearchMaxResults { copt, .. } + | GroupAccountPolicyOpt::LimitSearchMaxFilterTest { copt, .. } | GroupAccountPolicyOpt::PrivilegedSessionExpiry { copt, .. } => copt.debug, } } @@ -82,6 +84,36 @@ impl GroupAccountPolicyOpt { println!("Updated webauthn attestation CA list."); } } + GroupAccountPolicyOpt::LimitSearchMaxResults { + name, + maximum, + copt, + } => { + let client = copt.to_client(OpType::Write).await; + if let Err(e) = client + .group_account_policy_limit_search_max_results(name, *maximum) + .await + { + handle_client_error(e, copt.output_mode); + } else { + println!("Updated search maximum results limit."); + } + } + GroupAccountPolicyOpt::LimitSearchMaxFilterTest { + name, + maximum, + copt, + } => { + let client = copt.to_client(OpType::Write).await; + if let Err(e) = client + .group_account_policy_limit_search_max_filter_test(name, *maximum) + .await + { + handle_client_error(e, copt.output_mode); + } else { + println!("Updated search maximum filter test limit."); + } + } } } } diff --git a/tools/cli/src/opt/kanidm.rs b/tools/cli/src/opt/kanidm.rs index 043a3f2c6..fc87ff0bd 100644 --- a/tools/cli/src/opt/kanidm.rs +++ b/tools/cli/src/opt/kanidm.rs @@ -152,7 +152,7 @@ pub enum GroupAccountPolicyOpt { #[clap(flatten)] copt: CommonOpt, }, - /// Set the maximum time for session expiry + /// Set the maximum time for session expiry in seconds. #[clap(name = "auth-expiry")] AuthSessionExpiry { name: String, @@ -161,7 +161,7 @@ pub enum GroupAccountPolicyOpt { copt: CommonOpt, }, /// Set the minimum credential class that members may authenticate with. Valid values - /// in order of weakest to strongest are: "any" "mfa" "passkey" "attested_passkey" + /// in order of weakest to strongest are: "any" "mfa" "passkey" "attested_passkey". #[clap(name = "credential-type-minimum")] CredentialTypeMinimum { name: String, @@ -170,7 +170,7 @@ pub enum GroupAccountPolicyOpt { #[clap(flatten)] copt: CommonOpt, }, - /// Set the minimum length of passwords for accounts + /// Set the minimum character length of passwords for accounts. #[clap(name = "password-minimum-length")] PasswordMinimumLength { name: String, @@ -178,7 +178,7 @@ pub enum GroupAccountPolicyOpt { #[clap(flatten)] copt: CommonOpt, }, - /// Set the maximum time for privilege session expiry + /// Set the maximum time for privilege session expiry in seconds. #[clap(name = "privilege-expiry")] PrivilegedSessionExpiry { name: String, @@ -186,9 +186,9 @@ pub enum GroupAccountPolicyOpt { #[clap(flatten)] copt: CommonOpt, }, - /// The the webauthn attestation ca list that should be enforced + /// The WebAuthn attestation CA list that should be enforced /// on members of this group. Prevents use of passkeys that are - /// in this list. To create this list, use `fido-mds-tool` + /// not in this list. To create this list, use `fido-mds-tool` /// from #[clap(name = "webauthn-attestation-ca-list")] WebauthnAttestationCaList { @@ -197,6 +197,25 @@ pub enum GroupAccountPolicyOpt { #[clap(flatten)] copt: CommonOpt, }, + /// Sets the maximum number of entries that may be returned in a + /// search operation. + #[clap(name = "limit-search-max-results")] + LimitSearchMaxResults { + name: String, + maximum: u32, + #[clap(flatten)] + copt: CommonOpt, + }, + /// Sets the maximum number of entries that are examined during + /// a partially indexed search. This does not affect fully + /// indexed searches. If in doubt, set this to 1.5x limit-search-max-results + #[clap(name = "limit-search-max-filter-test")] + LimitSearchMaxFilterTest { + name: String, + maximum: u32, + #[clap(flatten)] + copt: CommonOpt, + }, } #[derive(Debug, Subcommand)]