Compare commits

..

2 commits

Author SHA1 Message Date
Firstyear de65ba333b
Merge branch 'master' into 20250408-subtle-write-issue-unixd 2025-04-08 16:27:12 +10:00
William Brown eb3d77cd35 Improve token handling
It was possible that a token could be updated in a way that caused
existing cached information to be lost if an event was delayed
in it's write to the user token.

To prevent this, the writes to user tokens now require the HsmLock
to be held, and refresh the token just ahead of writing to ensure
that these data can't be lost. The benefit to this approach is that
readers remain unblocked by a writer.
2025-04-08 16:23:08 +10:00
5 changed files with 12 additions and 12 deletions
Cargo.lockCargo.toml
unix_integration/resolver/src

4
Cargo.lock generated
View file

@ -5658,9 +5658,9 @@ dependencies = [
[[package]]
name = "tokio"
version = "1.44.2"
version = "1.44.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e6b88822cbe49de4185e3a4cbf8321dd487cf5fe0c5c65695fef6346371e9c48"
checksum = "f382da615b842244d4b8738c82ed1275e6c5dd90c459a30941cd07080b06c91a"
dependencies = [
"backtrace",
"bytes",

View file

@ -268,7 +268,7 @@ tempfile = "3.15.0"
testkit-macros = { path = "./server/testkit-macros" }
time = { version = "^0.3.36", features = ["formatting", "local-offset"] }
tokio = "^1.44.2"
tokio = "^1.43.0"
tokio-openssl = "^0.6.5"
tokio-util = "^0.7.13"

View file

@ -195,7 +195,7 @@ impl Into<PamAuthResponse> for AuthRequest {
pub enum AuthResult {
Success,
SuccessUpdate { new_token: UserToken },
SuccessUpdate { token: UserToken },
Denied,
Next(AuthRequest),
}

View file

@ -460,23 +460,23 @@ impl IdProvider for KanidmProvider {
match auth_result {
Ok(Some(n_tok)) => {
let mut new_token = UserToken::from(n_tok);
let mut token = UserToken::from(n_tok);
// Update any keys that may have been in the db in the current
// token.
if let Some(previous_token) = current_token {
new_token.extra_keys = previous_token.extra_keys.clone();
token.extra_keys = previous_token.extra_keys.clone();
}
// Set any new keys that are relevant from this authentication
new_token.kanidm_update_cached_password(
token.kanidm_update_cached_password(
&inner.crypto_policy,
cred.as_str(),
tpm,
&inner.hmac_key,
);
Ok(AuthResult::SuccessUpdate { new_token })
Ok(AuthResult::SuccessUpdate { token })
}
Ok(None) => {
// TODO: i'm not a huge fan of this rn, but currently the way we handle
@ -583,11 +583,11 @@ impl IdProvider for KanidmProvider {
if session_token.kanidm_check_cached_password(cred.as_str(), tpm, &inner.hmac_key) {
// Ensure we have either the latest token, or if none, at least the session token.
let new_token = current_token.unwrap_or(session_token).clone();
let token = current_token.unwrap_or(session_token).clone();
// TODO: We can update the token here and then do lockouts.
Ok(AuthResult::SuccessUpdate { new_token })
Ok(AuthResult::SuccessUpdate { token })
} else {
Ok(AuthResult::Denied)
}

View file

@ -1202,8 +1202,8 @@ impl Resolver {
*auth_session = AuthSession::Success;
Ok(PamAuthResponse::Success)
}
Ok(AuthResult::SuccessUpdate { mut new_token }) => {
self.set_cache_usertoken(&mut new_token, hsm_lock.deref_mut())
Ok(AuthResult::SuccessUpdate { mut token }) => {
self.set_cache_usertoken(&mut token, hsm_lock.deref_mut())
.await?;
*auth_session = AuthSession::Success;