mirror of
https://github.com/kanidm/kanidm.git
synced 2025-02-23 20:47:01 +01:00
pw min length in account policy (#2289)
This commit is contained in:
parent
ffafb32389
commit
b7852d1d71
|
@ -21,6 +21,7 @@ parts.
|
|||
| value | ordering |
|
||||
| ---------------- | -------------- |
|
||||
| auth-session | smallest value |
|
||||
| password-minimum-length | largest value |
|
||||
| privilege-expiry | smallest value |
|
||||
|
||||
### Example Resolution
|
||||
|
@ -29,6 +30,7 @@ If we had two policies where the first defined:
|
|||
|
||||
```
|
||||
auth-session: 86400
|
||||
password-minimum-length: 10
|
||||
privilege-expiry: 600
|
||||
```
|
||||
|
||||
|
@ -36,14 +38,17 @@ And the second
|
|||
|
||||
```
|
||||
auth-session: 3600
|
||||
password-minimum-length: 15
|
||||
privilege-expiry: 3600
|
||||
```
|
||||
|
||||
As the value of auth-session from the second is smaller we would take that. We would take the
|
||||
smallest value of privilege-expiry from the first. This leaves:
|
||||
smallest value of privilege-expiry from the first. We would take the largest value of
|
||||
password-minimum-length. This leaves:
|
||||
|
||||
```
|
||||
auth-session: 3600
|
||||
password-minimum-length: 15
|
||||
privilege-expiry: 600
|
||||
```
|
||||
|
||||
|
@ -73,6 +78,19 @@ kanidm group account-policy auth-expiry <group name> <seconds>
|
|||
kanidm group account-policy auth-expiry my_admin_group 86400
|
||||
```
|
||||
|
||||
## Setting Minimum Password Length
|
||||
|
||||
The password-minimum-length value defines the character length of passwords that are acceptable. There
|
||||
are no-other tunables for passwords in account policy. Other settings such as complexity, symbols,
|
||||
numbers and so on, have been proven to not matter in any real world attacks.
|
||||
|
||||
To set this value:
|
||||
|
||||
```
|
||||
kanidm group account-policy password-minimum-length <group name> <length>
|
||||
kanidm group account-policy password-minimum-length my_admin_group 12
|
||||
```
|
||||
|
||||
## Setting Maximum Privilege Time
|
||||
|
||||
The privilege-expiry time defines how long a session retains its write privileges after a
|
||||
|
|
|
@ -21,6 +21,18 @@ impl KanidmClient {
|
|||
.await
|
||||
}
|
||||
|
||||
pub async fn group_account_policy_password_minimum_length_set(
|
||||
&self,
|
||||
id: &str,
|
||||
length: u32,
|
||||
) -> Result<(), ClientError> {
|
||||
self.perform_put_request(
|
||||
&format!("/v1/group/{}/_attr/auth_password_minimum_length", id),
|
||||
vec![length.to_string()],
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn group_account_policy_privilege_expiry_set(
|
||||
&self,
|
||||
id: &str,
|
||||
|
|
|
@ -53,6 +53,7 @@ pub const ATTR_ATTR: &str = "attr";
|
|||
pub const ATTR_ATTRIBUTENAME: &str = "attributename";
|
||||
pub const ATTR_ATTRIBUTETYPE: &str = "attributetype";
|
||||
pub const ATTR_AUTH_SESSION_EXPIRY: &str = "authsession_expiry";
|
||||
pub const ATTR_AUTH_PASSWORD_MINIMUM_LENGTH: &str = "auth_password_minimum_length";
|
||||
pub const ATTR_BADLIST_PASSWORD: &str = "badlist_password";
|
||||
pub const ATTR_CLAIM: &str = "claim";
|
||||
pub const ATTR_CLASS: &str = "class";
|
||||
|
|
|
@ -89,7 +89,7 @@ pub enum ConsistencyError {
|
|||
DeniedName(Uuid),
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, ToSchema)]
|
||||
#[derive(Serialize, Deserialize, Debug, ToSchema, PartialEq, Eq, PartialOrd, Ord)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum PasswordFeedback {
|
||||
// https://docs.rs/zxcvbn/latest/zxcvbn/feedback/enum.Suggestion.html
|
||||
|
@ -122,8 +122,9 @@ pub enum PasswordFeedback {
|
|||
NamesAndSurnamesByThemselvesAreEasyToGuess,
|
||||
CommonNamesAndSurnamesAreEasyToGuess,
|
||||
// Custom
|
||||
TooShort(usize),
|
||||
TooShort(u32),
|
||||
BadListed,
|
||||
DontReusePasswords,
|
||||
}
|
||||
|
||||
/// Human-readable PasswordFeedback result.
|
||||
|
@ -163,6 +164,12 @@ impl fmt::Display for PasswordFeedback {
|
|||
PasswordFeedback::DatesAreOftenEasyToGuess => {
|
||||
write!(f, "Dates are often easy to guess.")
|
||||
}
|
||||
PasswordFeedback::DontReusePasswords => {
|
||||
write!(
|
||||
f,
|
||||
"Don't reuse passwords that already exist on your account"
|
||||
)
|
||||
}
|
||||
PasswordFeedback::NamesAndSurnamesByThemselvesAreEasyToGuess => {
|
||||
write!(f, "Names and surnames by themselves are easy to guess.")
|
||||
}
|
||||
|
|
|
@ -1272,16 +1272,19 @@ lazy_static! {
|
|||
Attribute::Name,
|
||||
Attribute::Uuid,
|
||||
Attribute::AuthSessionExpiry,
|
||||
Attribute::AuthPasswordMinimumLength,
|
||||
Attribute::PrivilegeExpiry,
|
||||
],
|
||||
modify_removed_attrs: vec![
|
||||
Attribute::Class,
|
||||
Attribute::AuthSessionExpiry,
|
||||
Attribute::AuthPasswordMinimumLength,
|
||||
Attribute::PrivilegeExpiry,
|
||||
],
|
||||
modify_present_attrs: vec![
|
||||
Attribute::Class,
|
||||
Attribute::AuthSessionExpiry,
|
||||
Attribute::AuthPasswordMinimumLength,
|
||||
Attribute::PrivilegeExpiry,
|
||||
],
|
||||
modify_classes: vec![
|
||||
|
|
|
@ -54,6 +54,7 @@ pub enum Attribute {
|
|||
AttributeName,
|
||||
AttributeType,
|
||||
AuthSessionExpiry,
|
||||
AuthPasswordMinimumLength,
|
||||
BadlistPassword,
|
||||
Claim,
|
||||
Class,
|
||||
|
@ -231,6 +232,7 @@ impl TryFrom<String> for Attribute {
|
|||
ATTR_ATTRIBUTENAME => Attribute::AttributeName,
|
||||
ATTR_ATTRIBUTETYPE => Attribute::AttributeType,
|
||||
ATTR_AUTH_SESSION_EXPIRY => Attribute::AuthSessionExpiry,
|
||||
ATTR_AUTH_PASSWORD_MINIMUM_LENGTH => Attribute::AuthPasswordMinimumLength,
|
||||
ATTR_BADLIST_PASSWORD => Attribute::BadlistPassword,
|
||||
ATTR_CLAIM => Attribute::Claim,
|
||||
ATTR_CLASS => Attribute::Class,
|
||||
|
@ -384,6 +386,7 @@ impl From<Attribute> for &'static str {
|
|||
Attribute::AttributeName => ATTR_ATTRIBUTENAME,
|
||||
Attribute::AttributeType => ATTR_ATTRIBUTETYPE,
|
||||
Attribute::AuthSessionExpiry => ATTR_AUTH_SESSION_EXPIRY,
|
||||
Attribute::AuthPasswordMinimumLength => ATTR_AUTH_PASSWORD_MINIMUM_LENGTH,
|
||||
Attribute::BadlistPassword => ATTR_BADLIST_PASSWORD,
|
||||
Attribute::Claim => ATTR_CLAIM,
|
||||
Attribute::Class => ATTR_CLASS,
|
||||
|
|
|
@ -76,7 +76,7 @@ pub const RECYCLEBIN_MAX_AGE: u64 = 604_800;
|
|||
pub const AUTH_SESSION_TIMEOUT: u64 = 300;
|
||||
// 5 minute mfa reg window
|
||||
pub const MFAREG_SESSION_TIMEOUT: u64 = 300;
|
||||
pub const PW_MIN_LENGTH: usize = 10;
|
||||
pub const PW_MIN_LENGTH: u32 = 10;
|
||||
|
||||
// Maximum - Sessions have no upper bound.
|
||||
pub const MAXIMUM_AUTH_SESSION_EXPIRY: u32 = u32::MAX;
|
||||
|
|
|
@ -229,6 +229,15 @@ pub static ref SCHEMA_ATTR_AUTH_PRIVILEGE_EXPIRY: SchemaAttribute = SchemaAttrib
|
|||
..Default::default()
|
||||
};
|
||||
|
||||
pub static ref SCHEMA_ATTR_AUTH_PASSWORD_MINIMUM_LENGTH: SchemaAttribute = SchemaAttribute {
|
||||
uuid: UUID_SCHEMA_ATTR_AUTH_PASSWORD_MINIMUM_LENGTH,
|
||||
name: Attribute::AuthPasswordMinimumLength.into(),
|
||||
|
||||
description: "Minimum length of passwords.".to_string(),
|
||||
syntax: SyntaxType::Uint32,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
pub static ref SCHEMA_ATTR_LOGINSHELL: SchemaAttribute = SchemaAttribute {
|
||||
uuid: UUID_SCHEMA_ATTR_LOGINSHELL,
|
||||
name: Attribute::LoginShell.into(),
|
||||
|
@ -636,7 +645,8 @@ pub static ref SCHEMA_CLASS_ACCOUNT_POLICY: SchemaClass = SchemaClass {
|
|||
description: "Policies applied to accounts that are members of a group".to_string(),
|
||||
systemmay: vec![
|
||||
Attribute::AuthSessionExpiry.into(),
|
||||
Attribute::PrivilegeExpiry.into()
|
||||
Attribute::PrivilegeExpiry.into(),
|
||||
Attribute::AuthPasswordMinimumLength.into(),
|
||||
],
|
||||
systemsupplements: vec![Attribute::Group.into()],
|
||||
..Default::default()
|
||||
|
|
|
@ -245,6 +245,8 @@ pub const UUID_SCHEMA_ATTR_LDAP_ALLOW_UNIX_PW_BIND: Uuid =
|
|||
uuid!("00000000-0000-0000-0000-ffff00000145");
|
||||
|
||||
pub const UUID_SCHEMA_CLASS_ACCOUNT_POLICY: Uuid = uuid!("00000000-0000-0000-0000-ffff00000146");
|
||||
pub const UUID_SCHEMA_ATTR_AUTH_PASSWORD_MINIMUM_LENGTH: Uuid =
|
||||
uuid!("00000000-0000-0000-0000-ffff00000147");
|
||||
|
||||
// System and domain infos
|
||||
// I'd like to strongly criticise william of the past for making poor choices about these allocations.
|
||||
|
|
|
@ -32,6 +32,7 @@ impl From<u32> for CredentialPolicy {
|
|||
pub(crate) struct AccountPolicy {
|
||||
privilege_expiry: u32,
|
||||
authsession_expiry: u32,
|
||||
pw_min_length: u32,
|
||||
credential_policy: CredentialPolicy,
|
||||
}
|
||||
|
||||
|
@ -50,21 +51,26 @@ impl From<&EntrySealedCommitted> for Option<AccountPolicy> {
|
|||
let privilege_expiry = val
|
||||
.get_ava_single_uint32(Attribute::PrivilegeExpiry)
|
||||
.unwrap_or(MAXIMUM_AUTH_PRIVILEGE_EXPIRY);
|
||||
let pw_min_length = val
|
||||
.get_ava_single_uint32(Attribute::AuthPasswordMinimumLength)
|
||||
.unwrap_or(PW_MIN_LENGTH);
|
||||
let credential_policy = CredentialPolicy::default();
|
||||
|
||||
Some(AccountPolicy {
|
||||
privilege_expiry,
|
||||
authsession_expiry,
|
||||
pw_min_length,
|
||||
credential_policy,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
#[derive(Clone, Debug)]
|
||||
#[cfg_attr(test, derive(Default))]
|
||||
pub(crate) struct ResolvedAccountPolicy {
|
||||
privilege_expiry: u32,
|
||||
authsession_expiry: u32,
|
||||
pw_min_length: u32,
|
||||
credential_policy: CredentialPolicy,
|
||||
}
|
||||
|
||||
|
@ -77,6 +83,7 @@ impl ResolvedAccountPolicy {
|
|||
let mut accumulate = ResolvedAccountPolicy {
|
||||
privilege_expiry: MAXIMUM_AUTH_PRIVILEGE_EXPIRY,
|
||||
authsession_expiry: MAXIMUM_AUTH_SESSION_EXPIRY,
|
||||
pw_min_length: PW_MIN_LENGTH,
|
||||
credential_policy: CredentialPolicy::default(),
|
||||
};
|
||||
|
||||
|
@ -91,6 +98,11 @@ impl ResolvedAccountPolicy {
|
|||
accumulate.authsession_expiry = acc_pol.authsession_expiry
|
||||
}
|
||||
|
||||
// Take larger pw min len
|
||||
if acc_pol.pw_min_length > accumulate.pw_min_length {
|
||||
accumulate.pw_min_length = acc_pol.pw_min_length
|
||||
}
|
||||
|
||||
// Take the greater credential type policy
|
||||
if acc_pol.credential_policy > accumulate.credential_policy {
|
||||
accumulate.credential_policy = acc_pol.credential_policy
|
||||
|
@ -108,6 +120,10 @@ impl ResolvedAccountPolicy {
|
|||
self.authsession_expiry
|
||||
}
|
||||
|
||||
pub(crate) fn pw_min_length(&self) -> u32 {
|
||||
self.pw_min_length
|
||||
}
|
||||
|
||||
/*
|
||||
pub(crate) fn credential_policy(&self) -> CredentialPolicy {
|
||||
self.credential_policy
|
||||
|
@ -125,12 +141,14 @@ mod tests {
|
|||
let policy_a = AccountPolicy {
|
||||
privilege_expiry: 100,
|
||||
authsession_expiry: 100,
|
||||
pw_min_length: 11,
|
||||
credential_policy: CredentialPolicy::MfaRequired,
|
||||
};
|
||||
|
||||
let policy_b = AccountPolicy {
|
||||
privilege_expiry: 150,
|
||||
authsession_expiry: 50,
|
||||
pw_min_length: 15,
|
||||
credential_policy: CredentialPolicy::PasskeyRequired,
|
||||
};
|
||||
|
||||
|
@ -138,6 +156,7 @@ mod tests {
|
|||
|
||||
assert_eq!(rap.privilege_expiry(), 100);
|
||||
assert_eq!(rap.authsession_expiry(), 50);
|
||||
assert_eq!(rap.pw_min_length(), 15);
|
||||
assert_eq!(rap.credential_policy, CredentialPolicy::PasskeyRequired);
|
||||
}
|
||||
|
||||
|
|
|
@ -27,6 +27,8 @@ use crate::server::access::Access;
|
|||
use crate::utils::{backup_code_from_random, readable_password_from_random, uuid_from_duration};
|
||||
use crate::value::{CredUpdateSessionPerms, IntentTokenState};
|
||||
|
||||
use super::accountpolicy::ResolvedAccountPolicy;
|
||||
|
||||
const MAXIMUM_CRED_UPDATE_TTL: Duration = Duration::from_secs(900);
|
||||
// Default 1 hour.
|
||||
const DEFAULT_INTENT_TTL: Duration = Duration::from_secs(3600);
|
||||
|
@ -37,8 +39,9 @@ const MINIMUM_INTENT_TTL: Duration = Duration::from_secs(300);
|
|||
|
||||
#[derive(Debug)]
|
||||
pub enum PasswordQuality {
|
||||
TooShort(usize),
|
||||
TooShort(u32),
|
||||
BadListed,
|
||||
DontReusePasswords,
|
||||
Feedback(Vec<PasswordFeedback>),
|
||||
}
|
||||
|
||||
|
@ -87,6 +90,8 @@ pub(crate) struct CredentialUpdateSession {
|
|||
issuer: String,
|
||||
// Current credentials - these are on the Account!
|
||||
account: Account,
|
||||
// The account policy applied to this account
|
||||
resolved_account_policy: ResolvedAccountPolicy,
|
||||
// What intent was used to initiate this session.
|
||||
intent_token_id: Option<String>,
|
||||
|
||||
|
@ -131,6 +136,7 @@ impl fmt::Debug for CredentialUpdateSession {
|
|||
f.debug_struct("CredentialUpdateSession")
|
||||
.field("account.spn", &self.account.spn)
|
||||
.field("account.unix", &self.account.unix_extn().is_some())
|
||||
.field("resolved_account_policy", &self.resolved_account_policy)
|
||||
.field("intent_token_id", &self.intent_token_id)
|
||||
.field("primary.detail()", &primary)
|
||||
.field("passkeys.list()", &passkeys)
|
||||
|
@ -339,7 +345,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
&mut self,
|
||||
target: Uuid,
|
||||
ident: &Identity,
|
||||
) -> Result<(Account, CredUpdateSessionPerms), OperationError> {
|
||||
) -> Result<(Account, ResolvedAccountPolicy, CredUpdateSessionPerms), OperationError> {
|
||||
let entry = self.qs_write.internal_search_uuid(target)?;
|
||||
|
||||
security_info!(
|
||||
|
@ -356,7 +362,8 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
}
|
||||
|
||||
// Is target an account? This checks for us.
|
||||
let account = Account::try_from_entry_rw(entry.as_ref(), &mut self.qs_write)?;
|
||||
let (account, resolved_account_policy) =
|
||||
Account::try_from_entry_with_policy(entry.as_ref(), &mut self.qs_write)?;
|
||||
|
||||
let effective_perms = self
|
||||
.qs_write
|
||||
|
@ -512,6 +519,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
security_info!(%primary_can_edit, %passkeys_can_edit, %unixcred_can_edit, %sshpubkey_can_edit, %ext_cred_portal_can_view, "Proceeding");
|
||||
Ok((
|
||||
account,
|
||||
resolved_account_policy,
|
||||
CredUpdateSessionPerms {
|
||||
ext_cred_portal_can_view,
|
||||
passkeys_can_edit,
|
||||
|
@ -528,6 +536,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
sessionid: Uuid,
|
||||
intent_token_id: Option<String>,
|
||||
account: Account,
|
||||
resolved_account_policy: ResolvedAccountPolicy,
|
||||
perms: CredUpdateSessionPerms,
|
||||
ct: Duration,
|
||||
) -> Result<(CredentialUpdateSessionToken, CredentialUpdateSessionStatus), OperationError> {
|
||||
|
@ -588,6 +597,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
// - store account policy (if present)
|
||||
let session = CredentialUpdateSession {
|
||||
account,
|
||||
resolved_account_policy,
|
||||
issuer,
|
||||
intent_token_id,
|
||||
ext_cred_portal,
|
||||
|
@ -638,7 +648,11 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
event: &InitCredentialUpdateIntentEvent,
|
||||
ct: Duration,
|
||||
) -> Result<CredentialUpdateIntentToken, OperationError> {
|
||||
let (account, perms) = self.validate_init_credential_update(event.target, &event.ident)?;
|
||||
let (account, _resolved_account_policy, perms) =
|
||||
self.validate_init_credential_update(event.target, &event.ident)?;
|
||||
|
||||
// We should check in the acc-pol if we can proceed?
|
||||
// Is there a reason account policy might deny us from proceeding?
|
||||
|
||||
// ==== AUTHORISATION CHECKED ===
|
||||
|
||||
|
@ -775,7 +789,8 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
};
|
||||
|
||||
// Is target an account? This checks for us.
|
||||
let account = Account::try_from_entry_rw(entry.as_ref(), &mut self.qs_write)?;
|
||||
let (account, resolved_account_policy) =
|
||||
Account::try_from_entry_with_policy(entry.as_ref(), &mut self.qs_write)?;
|
||||
|
||||
// Check there is not already a user session in progress with this intent token.
|
||||
// Is there a need to revoke intent tokens?
|
||||
|
@ -885,24 +900,39 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
// ==========
|
||||
// Okay, good to exchange.
|
||||
|
||||
self.create_credupdate_session(session_id, Some(intent_id), account, perms, current_time)
|
||||
self.create_credupdate_session(
|
||||
session_id,
|
||||
Some(intent_id),
|
||||
account,
|
||||
resolved_account_policy,
|
||||
perms,
|
||||
current_time,
|
||||
)
|
||||
}
|
||||
|
||||
#[instrument(level = "debug", skip_all)]
|
||||
pub fn init_credential_update(
|
||||
&mut self,
|
||||
event: &InitCredentialUpdateEvent,
|
||||
ct: Duration,
|
||||
current_time: Duration,
|
||||
) -> Result<(CredentialUpdateSessionToken, CredentialUpdateSessionStatus), OperationError> {
|
||||
let (account, perms) = self.validate_init_credential_update(event.target, &event.ident)?;
|
||||
let (account, resolved_account_policy, perms) =
|
||||
self.validate_init_credential_update(event.target, &event.ident)?;
|
||||
|
||||
// ==== AUTHORISATION CHECKED ===
|
||||
// This is the expiry time, so that our cleanup task can "purge up to now" rather
|
||||
// than needing to do calculations.
|
||||
let sessionid = uuid_from_duration(ct + MAXIMUM_CRED_UPDATE_TTL, self.sid);
|
||||
let sessionid = uuid_from_duration(current_time + MAXIMUM_CRED_UPDATE_TTL, self.sid);
|
||||
|
||||
// Build the cred update session.
|
||||
self.create_credupdate_session(sessionid, None, account, perms, ct)
|
||||
self.create_credupdate_session(
|
||||
sessionid,
|
||||
None,
|
||||
account,
|
||||
resolved_account_policy,
|
||||
perms,
|
||||
current_time,
|
||||
)
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip(self))]
|
||||
|
@ -1231,25 +1261,48 @@ impl<'a> IdmServerCredUpdateTransaction<'a> {
|
|||
Ok(status)
|
||||
}
|
||||
|
||||
#[instrument(level = "debug", skip(self))]
|
||||
#[instrument(level = "trace", skip(self))]
|
||||
fn check_password_quality(
|
||||
&self,
|
||||
cleartext: &str,
|
||||
resolved_account_policy: &ResolvedAccountPolicy,
|
||||
related_inputs: &[&str],
|
||||
radius_secret: Option<&str>,
|
||||
) -> Result<(), PasswordQuality> {
|
||||
// password strength and badlisting is always global, rather than per-pw-policy.
|
||||
// pw-policy as check on the account is about requirements for mfa for example.
|
||||
|
||||
// is the password at least 10 char?
|
||||
if cleartext.len() < PW_MIN_LENGTH {
|
||||
return Err(PasswordQuality::TooShort(PW_MIN_LENGTH));
|
||||
let pw_min_length = resolved_account_policy.pw_min_length();
|
||||
if cleartext.len() < pw_min_length as usize {
|
||||
return Err(PasswordQuality::TooShort(pw_min_length));
|
||||
}
|
||||
|
||||
if let Some(some_radius_secret) = radius_secret {
|
||||
if cleartext.contains(some_radius_secret) {
|
||||
return Err(PasswordQuality::DontReusePasswords);
|
||||
}
|
||||
}
|
||||
|
||||
// zxcvbn doesn't appear to be picking these up?
|
||||
for related in related_inputs {
|
||||
if cleartext.contains(related) {
|
||||
return Err(PasswordQuality::Feedback(vec![
|
||||
PasswordFeedback::NamesAndSurnamesByThemselvesAreEasyToGuess,
|
||||
PasswordFeedback::AvoidDatesAndYearsThatAreAssociatedWithYou,
|
||||
]));
|
||||
}
|
||||
}
|
||||
|
||||
// does the password pass zxcvbn?
|
||||
|
||||
let entropy = zxcvbn::zxcvbn(cleartext, related_inputs).map_err(|e| {
|
||||
admin_error!("zxcvbn check failure (password empty?) {:?}", e);
|
||||
PasswordQuality::TooShort(PW_MIN_LENGTH)
|
||||
// Return some generic feedback when the password is this bad.
|
||||
PasswordQuality::Feedback(vec![
|
||||
PasswordFeedback::UseAFewWordsAvoidCommonPhrases,
|
||||
PasswordFeedback::AddAnotherWordOrTwo,
|
||||
PasswordFeedback::NoNeedForSymbolsDigitsOrUppercaseLetters,
|
||||
])
|
||||
})?;
|
||||
|
||||
// PW's should always be enforced as strong as possible.
|
||||
|
@ -1263,7 +1316,12 @@ impl<'a> IdmServerCredUpdateTransaction<'a> {
|
|||
.map(|v| v.clone())
|
||||
.map_err(|e| {
|
||||
security_info!("zxcvbn returned no feedback when score < 3 -> {:?}", e);
|
||||
PasswordQuality::TooShort(PW_MIN_LENGTH)
|
||||
// Return some generic feedback when the password is this bad.
|
||||
PasswordQuality::Feedback(vec![
|
||||
PasswordFeedback::UseAFewWordsAvoidCommonPhrases,
|
||||
PasswordFeedback::AddAnotherWordOrTwo,
|
||||
PasswordFeedback::NoNeedForSymbolsDigitsOrUppercaseLetters,
|
||||
])
|
||||
})?;
|
||||
|
||||
security_info!(?feedback, "pw quality feedback");
|
||||
|
@ -1397,17 +1455,24 @@ impl<'a> IdmServerCredUpdateTransaction<'a> {
|
|||
return Err(OperationError::AccessDenied);
|
||||
};
|
||||
|
||||
// Check pw quality (future - acc policy applies).
|
||||
self.check_password_quality(pw, session.account.related_inputs().as_slice())
|
||||
.map_err(|e| match e {
|
||||
PasswordQuality::TooShort(sz) => {
|
||||
OperationError::PasswordQuality(vec![PasswordFeedback::TooShort(sz)])
|
||||
}
|
||||
PasswordQuality::BadListed => {
|
||||
OperationError::PasswordQuality(vec![PasswordFeedback::BadListed])
|
||||
}
|
||||
PasswordQuality::Feedback(feedback) => OperationError::PasswordQuality(feedback),
|
||||
})?;
|
||||
self.check_password_quality(
|
||||
pw,
|
||||
&session.resolved_account_policy,
|
||||
session.account.related_inputs().as_slice(),
|
||||
session.account.radius_secret.as_deref(),
|
||||
)
|
||||
.map_err(|e| match e {
|
||||
PasswordQuality::TooShort(sz) => {
|
||||
OperationError::PasswordQuality(vec![PasswordFeedback::TooShort(sz)])
|
||||
}
|
||||
PasswordQuality::BadListed => {
|
||||
OperationError::PasswordQuality(vec![PasswordFeedback::BadListed])
|
||||
}
|
||||
PasswordQuality::DontReusePasswords => {
|
||||
OperationError::PasswordQuality(vec![PasswordFeedback::DontReusePasswords])
|
||||
}
|
||||
PasswordQuality::Feedback(feedback) => OperationError::PasswordQuality(feedback),
|
||||
})?;
|
||||
|
||||
let ncred = match &session.primary {
|
||||
Some(primary) => {
|
||||
|
@ -1832,6 +1897,7 @@ mod tests {
|
|||
|
||||
use kanidm_proto::v1::{
|
||||
AuthAllowed, AuthIssueSession, AuthMech, CUExtPortal, CredentialDetailType,
|
||||
PasswordFeedback,
|
||||
};
|
||||
use uuid::uuid;
|
||||
use webauthn_authenticator_rs::softpasskey::SoftPasskey;
|
||||
|
@ -1845,7 +1911,7 @@ mod tests {
|
|||
use crate::credential::totp::Totp;
|
||||
use crate::event::CreateEvent;
|
||||
use crate::idm::delayed::DelayedAction;
|
||||
use crate::idm::event::{AuthEvent, AuthResult};
|
||||
use crate::idm::event::{AuthEvent, AuthResult, RegenerateRadiusSecretEvent};
|
||||
use crate::idm::server::{IdmServer, IdmServerDelayed};
|
||||
use crate::idm::AuthState;
|
||||
use crate::prelude::*;
|
||||
|
@ -1997,6 +2063,13 @@ mod tests {
|
|||
.internal_search_uuid(TESTPERSON_UUID)
|
||||
.expect("failed");
|
||||
|
||||
// Setup the radius creds to ensure we don't use them anywhere else.
|
||||
let rrse = RegenerateRadiusSecretEvent::new_internal(TESTPERSON_UUID);
|
||||
|
||||
let _ = idms_prox_write
|
||||
.regenerate_radius_secret(&rrse)
|
||||
.expect("Failed to reset radius credential 1");
|
||||
|
||||
let cur = idms_prox_write.init_credential_update(
|
||||
&InitCredentialUpdateEvent::new_impersonate_entry(testperson),
|
||||
ct,
|
||||
|
@ -2370,6 +2443,172 @@ mod tests {
|
|||
.is_none());
|
||||
}
|
||||
|
||||
#[idm_test]
|
||||
async fn test_idm_credential_update_password_quality_checks(
|
||||
idms: &IdmServer,
|
||||
_idms_delayed: &mut IdmServerDelayed,
|
||||
) {
|
||||
let ct = Duration::from_secs(TEST_CURRENT_TIME);
|
||||
let (cust, _) = setup_test_session(idms, ct).await;
|
||||
|
||||
// Get the radius pw
|
||||
|
||||
let mut r_txn = idms.proxy_read().await;
|
||||
|
||||
let radius_secret = r_txn
|
||||
.qs_read
|
||||
.internal_search_uuid(TESTPERSON_UUID)
|
||||
.expect("No such entry")
|
||||
.get_ava_single_secret(Attribute::RadiusSecret)
|
||||
.expect("No radius secret found")
|
||||
.to_string();
|
||||
|
||||
drop(r_txn);
|
||||
|
||||
let cutxn = idms.cred_update_transaction().await;
|
||||
|
||||
// Get the credential status - this should tell
|
||||
// us the details of the credentials, as well as
|
||||
// if they are ready and valid to commit?
|
||||
let c_status = cutxn
|
||||
.credential_update_status(&cust, ct)
|
||||
.expect("Failed to get the current session status.");
|
||||
|
||||
trace!(?c_status);
|
||||
|
||||
assert!(c_status.primary.is_none());
|
||||
|
||||
// Test initially creating a credential.
|
||||
// - pw first
|
||||
|
||||
let err = cutxn
|
||||
.credential_primary_set_password(&cust, ct, "password")
|
||||
.unwrap_err();
|
||||
trace!(?err);
|
||||
assert!(match err {
|
||||
OperationError::PasswordQuality(details)
|
||||
if details == vec!(PasswordFeedback::TooShort(PW_MIN_LENGTH),) =>
|
||||
true,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
let err = cutxn
|
||||
.credential_primary_set_password(&cust, ct, "password1234")
|
||||
.unwrap_err();
|
||||
trace!(?err);
|
||||
assert!(match err {
|
||||
OperationError::PasswordQuality(details)
|
||||
if details
|
||||
== vec!(
|
||||
PasswordFeedback::AddAnotherWordOrTwo,
|
||||
PasswordFeedback::ThisIsACommonPassword,
|
||||
) =>
|
||||
true,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
let err = cutxn
|
||||
.credential_primary_set_password(&cust, ct, &radius_secret)
|
||||
.unwrap_err();
|
||||
trace!(?err);
|
||||
assert!(match err {
|
||||
OperationError::PasswordQuality(details)
|
||||
if details == vec!(PasswordFeedback::DontReusePasswords,) =>
|
||||
true,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
let err = cutxn
|
||||
.credential_primary_set_password(&cust, ct, "testperson2023")
|
||||
.unwrap_err();
|
||||
trace!(?err);
|
||||
assert!(match err {
|
||||
OperationError::PasswordQuality(details)
|
||||
if details
|
||||
== vec!(
|
||||
PasswordFeedback::NamesAndSurnamesByThemselvesAreEasyToGuess,
|
||||
PasswordFeedback::AvoidDatesAndYearsThatAreAssociatedWithYou,
|
||||
) =>
|
||||
true,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
let err = cutxn
|
||||
.credential_primary_set_password(
|
||||
&cust,
|
||||
ct,
|
||||
"demo_badlist_shohfie3aeci2oobur0aru9uushah6EiPi2woh4hohngoighaiRuepieN3ongoo1",
|
||||
)
|
||||
.unwrap_err();
|
||||
trace!(?err);
|
||||
assert!(match err {
|
||||
OperationError::PasswordQuality(details)
|
||||
if details == vec!(PasswordFeedback::BadListed) =>
|
||||
true,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
assert!(c_status.can_commit);
|
||||
|
||||
drop(cutxn);
|
||||
}
|
||||
|
||||
#[idm_test]
|
||||
async fn test_idm_credential_update_password_min_length_account_policy(
|
||||
idms: &IdmServer,
|
||||
_idms_delayed: &mut IdmServerDelayed,
|
||||
) {
|
||||
let ct = Duration::from_secs(TEST_CURRENT_TIME);
|
||||
|
||||
// Set the account policy min pw length
|
||||
let test_pw_min_length = PW_MIN_LENGTH * 2;
|
||||
|
||||
let mut idms_prox_write = idms.proxy_write(ct).await;
|
||||
|
||||
let modlist = ModifyList::new_purge_and_set(
|
||||
Attribute::AuthPasswordMinimumLength,
|
||||
Value::Uint32(test_pw_min_length),
|
||||
);
|
||||
idms_prox_write
|
||||
.qs_write
|
||||
.internal_modify_uuid(UUID_IDM_ALL_ACCOUNTS, &modlist)
|
||||
.expect("Unable to change default session exp");
|
||||
|
||||
assert!(idms_prox_write.commit().is_ok());
|
||||
// This now will affect all accounts for the next cred update.
|
||||
|
||||
let (cust, _) = setup_test_session(idms, ct).await;
|
||||
|
||||
let cutxn = idms.cred_update_transaction().await;
|
||||
|
||||
// Get the credential status - this should tell
|
||||
// us the details of the credentials, as well as
|
||||
// if they are ready and valid to commit?
|
||||
let c_status = cutxn
|
||||
.credential_update_status(&cust, ct)
|
||||
.expect("Failed to get the current session status.");
|
||||
|
||||
trace!(?c_status);
|
||||
|
||||
assert!(c_status.primary.is_none());
|
||||
|
||||
// Test initially creating a credential.
|
||||
// - pw first
|
||||
|
||||
let err = cutxn
|
||||
.credential_primary_set_password(&cust, ct, "password")
|
||||
.unwrap_err();
|
||||
trace!(?err);
|
||||
assert!(match err {
|
||||
OperationError::PasswordQuality(details)
|
||||
if details == vec!(PasswordFeedback::TooShort(test_pw_min_length),) =>
|
||||
true,
|
||||
_ => false,
|
||||
});
|
||||
|
||||
drop(cutxn);
|
||||
}
|
||||
|
||||
// Test set of primary account password
|
||||
// - fail pw quality checks etc
|
||||
// - set correctly.
|
||||
|
|
|
@ -1557,7 +1557,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
//
|
||||
|
||||
// is the password at least 10 char?
|
||||
if cleartext.len() < PW_MIN_LENGTH {
|
||||
if cleartext.len() < PW_MIN_LENGTH as usize {
|
||||
return Err(OperationError::PasswordQuality(vec![
|
||||
PasswordFeedback::TooShort(PW_MIN_LENGTH),
|
||||
]));
|
||||
|
@ -1678,15 +1678,6 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
|
|||
// If we got here, then pre-apply succeeded, and that means access control
|
||||
// passed. Now we can do the extra checks.
|
||||
|
||||
// Check the password quality.
|
||||
// Ask if tis all good - this step checks pwpolicy and such
|
||||
|
||||
self.check_password_quality(pce.cleartext.as_str(), account.related_inputs().as_slice())
|
||||
.map_err(|e| {
|
||||
request_error!(err = ?e, "check_password_quality");
|
||||
e
|
||||
})?;
|
||||
|
||||
// And actually really apply it now.
|
||||
self.qs_write.modify_apply(mp).map_err(|e| {
|
||||
request_error!(error = ?e);
|
||||
|
@ -2541,30 +2532,6 @@ mod tests {
|
|||
assert!(r1 != r2);
|
||||
}
|
||||
|
||||
#[idm_test]
|
||||
async fn test_idm_radius_secret_rejected_from_account_credential(
|
||||
idms: &IdmServer,
|
||||
_idms_delayed: &IdmServerDelayed,
|
||||
) {
|
||||
let mut idms_prox_write = idms.proxy_write(duration_from_epoch_now()).await;
|
||||
let rrse = RegenerateRadiusSecretEvent::new_internal(UUID_ADMIN);
|
||||
|
||||
let r1 = idms_prox_write
|
||||
.regenerate_radius_secret(&rrse)
|
||||
.expect("Failed to reset radius credential 1");
|
||||
|
||||
// Try and set that as the main account password, should fail.
|
||||
let pce = PasswordChangeEvent::new_internal(UUID_ADMIN, r1.as_str());
|
||||
let e = idms_prox_write.set_account_password(&pce);
|
||||
assert!(e.is_err());
|
||||
|
||||
let pce = UnixPasswordChangeEvent::new_internal(UUID_ADMIN, r1.as_str());
|
||||
let e = idms_prox_write.set_unix_account_password(&pce);
|
||||
assert!(e.is_err());
|
||||
|
||||
assert!(idms_prox_write.commit().is_ok());
|
||||
}
|
||||
|
||||
#[idm_test]
|
||||
async fn test_idm_radiusauthtoken(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) {
|
||||
let mut idms_prox_write = idms.proxy_write(duration_from_epoch_now()).await;
|
||||
|
@ -2589,54 +2556,6 @@ mod tests {
|
|||
assert!(r1 == tok_r.secret);
|
||||
}
|
||||
|
||||
#[idm_test]
|
||||
async fn test_idm_simple_password_reject_weak(
|
||||
idms: &IdmServer,
|
||||
_idms_delayed: &IdmServerDelayed,
|
||||
) {
|
||||
// len check
|
||||
let mut idms_prox_write = idms.proxy_write(duration_from_epoch_now()).await;
|
||||
|
||||
let pce = PasswordChangeEvent::new_internal(UUID_ADMIN, "password");
|
||||
let e = idms_prox_write.set_account_password(&pce);
|
||||
assert!(e.is_err());
|
||||
|
||||
// zxcvbn check
|
||||
let pce = PasswordChangeEvent::new_internal(UUID_ADMIN, "password1234");
|
||||
let e = idms_prox_write.set_account_password(&pce);
|
||||
assert!(e.is_err());
|
||||
|
||||
// Check the "name" checking works too (I think admin may hit a common pw rule first)
|
||||
let pce = PasswordChangeEvent::new_internal(UUID_ADMIN, "admin_nta");
|
||||
let e = idms_prox_write.set_account_password(&pce);
|
||||
assert!(e.is_err());
|
||||
|
||||
// Check that the demo badlist password is rejected.
|
||||
let pce = PasswordChangeEvent::new_internal(
|
||||
UUID_ADMIN,
|
||||
"demo_badlist_shohfie3aeci2oobur0aru9uushah6EiPi2woh4hohngoighaiRuepieN3ongoo1",
|
||||
);
|
||||
let e = idms_prox_write.set_account_password(&pce);
|
||||
assert!(e.is_err());
|
||||
|
||||
assert!(idms_prox_write.commit().is_ok());
|
||||
}
|
||||
|
||||
#[idm_test]
|
||||
async fn test_idm_simple_password_reject_badlist(
|
||||
idms: &IdmServer,
|
||||
_idms_delayed: &IdmServerDelayed,
|
||||
) {
|
||||
let mut idms_prox_write = idms.proxy_write(duration_from_epoch_now()).await;
|
||||
|
||||
// Check that the badlist password inserted is rejected.
|
||||
let pce = PasswordChangeEvent::new_internal(UUID_ADMIN, "bad@no3IBTyqHu$list");
|
||||
let e = idms_prox_write.set_account_password(&pce);
|
||||
assert!(e.is_err());
|
||||
|
||||
assert!(idms_prox_write.commit().is_ok());
|
||||
}
|
||||
|
||||
#[idm_test]
|
||||
async fn test_idm_unixusertoken(idms: &IdmServer, _idms_delayed: &IdmServerDelayed) {
|
||||
let mut idms_prox_write = idms.proxy_write(duration_from_epoch_now()).await;
|
||||
|
|
|
@ -558,6 +558,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
|
|||
SCHEMA_ATTR_API_TOKEN_SESSION.clone().into(),
|
||||
SCHEMA_ATTR_AUTH_SESSION_EXPIRY.clone().into(),
|
||||
SCHEMA_ATTR_AUTH_PRIVILEGE_EXPIRY.clone().into(),
|
||||
SCHEMA_ATTR_AUTH_PASSWORD_MINIMUM_LENGTH.clone().into(),
|
||||
SCHEMA_ATTR_BADLIST_PASSWORD.clone().into(),
|
||||
SCHEMA_ATTR_CREDENTIAL_UPDATE_INTENT_TOKEN.clone().into(),
|
||||
SCHEMA_ATTR_DEVICEKEYS.clone().into(),
|
||||
|
|
|
@ -6,6 +6,7 @@ impl GroupAccountPolicyOpt {
|
|||
match self {
|
||||
GroupAccountPolicyOpt::Enable { copt, .. }
|
||||
| GroupAccountPolicyOpt::AuthSessionExpiry { copt, .. }
|
||||
| GroupAccountPolicyOpt::PasswordMinimumLength { copt, .. }
|
||||
| GroupAccountPolicyOpt::PrivilegedSessionExpiry { copt, .. } => copt.debug,
|
||||
}
|
||||
}
|
||||
|
@ -31,6 +32,17 @@ impl GroupAccountPolicyOpt {
|
|||
println!("Updated authsession expiry.");
|
||||
}
|
||||
}
|
||||
GroupAccountPolicyOpt::PasswordMinimumLength { name, length, copt } => {
|
||||
let client = copt.to_client(OpType::Write).await;
|
||||
if let Err(e) = client
|
||||
.group_account_policy_password_minimum_length_set(name, *length)
|
||||
.await
|
||||
{
|
||||
handle_client_error(e, copt.output_mode);
|
||||
} else {
|
||||
println!("Updated password minimum length.");
|
||||
}
|
||||
}
|
||||
GroupAccountPolicyOpt::PrivilegedSessionExpiry { name, expiry, copt } => {
|
||||
let client = copt.to_client(OpType::Write).await;
|
||||
if let Err(e) = client
|
||||
|
|
|
@ -125,6 +125,14 @@ pub enum GroupAccountPolicyOpt {
|
|||
#[clap(flatten)]
|
||||
copt: CommonOpt,
|
||||
},
|
||||
/// Set the minimum length of passwords for accounts
|
||||
#[clap(name = "password-minimum-length")]
|
||||
PasswordMinimumLength {
|
||||
name: String,
|
||||
length: u32,
|
||||
#[clap(flatten)]
|
||||
copt: CommonOpt,
|
||||
},
|
||||
/// Configure and display the privilege session expiry
|
||||
/// Set the maximum time for privilege session expiry
|
||||
#[clap(name = "privilege-expiry")]
|
||||
|
|
Loading…
Reference in a new issue