mart-w/kanidm
1
0
Fork 0
mirror of https://github.com/kanidm/kanidm.git synced 2025-05-24 01:43:55 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity

Order keys in application JWKS / Fix rotation bug

Browse source 
When we return the JWKS for applications to validate, the order of
that set can matter for applications which assume that the first
key in the list is the current valid key. This sorts the resultant
JWKS to ensure that the latest key is always first.

When a key was requested for rotation, if the rotation time was in
the past then the key would not be rotated. In this situation, the
server now assumes that any "past" time indicates the rotation should
occur *now* instead.
This commit is contained in:
William Brown 2025-05-08 15:45:17 +10:00 committed by Firstyear
parent 8189bc0bc4
commit bb53f17b80
3 changed files with 33 additions and 16 deletions
server
lib/src
plugins
keyobject.rs
server/keys
internal.rs
testkit/tests/testkit
oauth2_test.rs

7
server/lib/src/plugins/keyobject.rs
View file

@ -198,15 +198,16 @@ impl KeyObjectManagement {
if let Some(rotation_time) = entry
.pop_ava(Attribute::KeyActionRotate)
.and_then(|vs| vs.to_datetime_single())
.and_then(|odt| {
.map(|odt| {
let secs = odt.unix_timestamp() as u64;
if secs > valid_from.as_secs() {
Some(Duration::from_secs(secs))
Duration::from_secs(secs)
} else {
None
valid_from
}
})
{
debug!(?rotation_time, "initiate key rotation");
key_object.rotate_keys(rotation_time, &txn_cid)?;
}

30
server/lib/src/server/keys/internal.rs
View file

@ -14,6 +14,7 @@ use compact_jwt::{
JwsSignerToVerifier,
};
use smolset::SmolSet;
use std::cmp::Reverse;
use std::collections::{BTreeMap, BTreeSet};
use std::ops::Bound::{Included, Unbounded};
use std::sync::Arc;
@ -196,8 +197,6 @@ impl KeyObjectInternalJweA128GCM {
fn get_valid_cipher(&self, time: Duration) -> Option<&JweA128KWEncipher> {
let ct_secs = time.as_secs();
trace!(active = ?self.active);
self.active
.range((Unbounded, Included(ct_secs)))
.next_back()
@ -214,6 +213,7 @@ impl KeyObjectInternalJweA128GCM {
}
}
#[instrument(level = "debug", name = "keyobject::jwe_a128_gcm::new", skip_all)]
fn new_active(&mut self, valid_from: Duration, cid: &Cid) -> Result<(), OperationError> {
let valid_from = valid_from.as_secs();
@ -486,6 +486,7 @@ impl KeyObjectInternalJwtEs256 {
Ok(())
}
#[instrument(level = "debug", name = "keyobject::jws_es256::new", skip_all)]
fn new_active(&mut self, valid_from: Duration, cid: &Cid) -> Result<(), OperationError> {
let valid_from = valid_from.as_secs();
@ -707,9 +708,15 @@ impl KeyObjectInternalJwtEs256 {
}
fn public_jwks(&self) -> JwkKeySet {
let keys = self
.all
.iter()
// A lot of applications assume the first item in the set is the latest
// key, so we need to return that first.
let mut signing_keys: Vec<_> = self.all.iter().collect();
// Sort by the time they are valid from.
signing_keys.sort_unstable_by_key(|(_, k)| Reverse(k.valid_from));
let keys = signing_keys
.into_iter()
.filter_map(|(_, es256)| match &es256.status {
InternalJwtEs256Status::Valid { verifier, .. }
| InternalJwtEs256Status::Retained { verifier, .. } => verifier
@ -858,6 +865,7 @@ impl KeyObjectInternalJwtRs256 {
Ok(())
}
#[instrument(level = "debug", name = "keyobject::jws_rs256::new", skip_all)]
fn new_active(&mut self, valid_from: Duration, cid: &Cid) -> Result<(), OperationError> {
let valid_from = valid_from.as_secs();
@ -1079,8 +1087,14 @@ impl KeyObjectInternalJwtRs256 {
}
fn public_jwks(&self) -> JwkKeySet {
let keys = self
.all
// A lot of applications assume the first item in the set is the latest
// key, so we need to return that first.
let mut signing_keys: Vec<_> = self.all.iter().collect();
// Sort by the time they are valid from.
signing_keys.sort_unstable_by_key(|(_, k)| Reverse(k.valid_from));
let keys = signing_keys
.iter()
.filter_map(|(key_id, rs256)| {
error!(?key_id);
@ -1284,6 +1298,7 @@ impl KeyObjectT for KeyObjectInternal {
Ok(None)
}
#[instrument(level = "debug", name = "keyobject::jws_es256_import", skip_all)]
fn jws_es256_import(
&mut self,
import_keys: &SmolSet<[Vec<u8>; 1]>,
@ -1397,6 +1412,7 @@ impl KeyObjectT for KeyObjectInternal {
])
}
#[instrument(level = "debug", name = "keyobject::jws_rs256_import", skip_all)]
fn jws_rs256_import(
&mut self,
import_keys: &SmolSet<[Vec<u8>; 1]>,

12
server/testkit/tests/testkit/oauth2_test.rs
View file

@ -228,14 +228,14 @@ async fn test_oauth2_openid_basic_flow_impl(
assert_eq!(response.status(), StatusCode::OK);
assert_no_cache!(response);
let mut jwk_set: JwkKeySet = response
let jwk_set: JwkKeySet = response
.json()
.await
.expect("Failed to access response body");
let public_jwk = jwk_set.keys.pop().expect("No public key in set!");
let public_jwk = jwk_set.keys.get(0).expect("No public key in set!");
let jws_validator = JwsEs256Verifier::try_from(&public_jwk).expect("failed to build validator");
let jws_validator = JwsEs256Verifier::try_from(public_jwk).expect("failed to build validator");
// Step 1 - the Oauth2 Resource Server would send a redirect to the authorisation
// server, where the url contains a series of authorisation request parameters.
@ -700,14 +700,14 @@ async fn test_oauth2_openid_public_flow_impl(
assert_eq!(response.status(), StatusCode::OK);
assert_no_cache!(response);
let mut jwk_set: JwkKeySet = response
let jwk_set: JwkKeySet = response
.json()
.await
.expect("Failed to access response body");
let public_jwk = jwk_set.keys.pop().expect("No public key in set!");
let public_jwk = jwk_set.keys.get(0).expect("No public key in set!");
let jws_validator = JwsEs256Verifier::try_from(&public_jwk).expect("failed to build validator");
let jws_validator = JwsEs256Verifier::try_from(public_jwk).expect("failed to build validator");
// Step 1 - the Oauth2 Resource Server would send a redirect to the authorisation
// server, where the url contains a series of authorisation request parameters.