Skip to content

Commit 2fc406f

Browse files
AlfioEmanueleFrestaAlfioEmanueleFresta
committed
fix(pin): re-mint instead of reusing a rejected persistent token
1 parent 55338b6 commit 2fc406f

6 files changed

Lines changed: 185 additions & 11 deletions

File tree

‎libwebauthn/src/management/credential_management.rs‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ where
7777
self,
7878
self.ctap2_credential_management(&req, timeout).await,
7979
uv_auth_used,
80-
timeout
80+
timeout,
81+
req
8182
)
8283
}?;
8384
let metadata = Ctap2CredentialManagementMetadata::new(
@@ -106,7 +107,8 @@ where
106107
self,
107108
self.ctap2_credential_management(&req, timeout).await,
108109
uv_auth_used,
109-
timeout
110+
timeout,
111+
req
110112
)
111113
}?;
112114
Ok((
@@ -134,7 +136,8 @@ where
134136
self,
135137
self.ctap2_credential_management(&req, timeout).await,
136138
uv_auth_used,
137-
timeout
139+
timeout,
140+
req
138141
)
139142
}?;
140143
Ok(Ctap2RPData::new(
@@ -163,7 +166,8 @@ where
163166
self,
164167
self.ctap2_credential_management(&req, timeout).await,
165168
uv_auth_used,
166-
timeout
169+
timeout,
170+
req
167171
)
168172
}?;
169173
let cred = Ctap2CredentialData::new(
@@ -196,7 +200,8 @@ where
196200
self,
197201
self.ctap2_credential_management(&req, timeout).await,
198202
uv_auth_used,
199-
timeout
203+
timeout,
204+
req
200205
)
201206
}?;
202207
let cred = Ctap2CredentialData::new(
@@ -229,7 +234,8 @@ where
229234
self,
230235
self.ctap2_credential_management(&req, timeout).await,
231236
uv_auth_used,
232-
timeout
237+
timeout,
238+
req
233239
)
234240
}?;
235241
Ok(())
@@ -262,7 +268,8 @@ where
262268
self,
263269
self.ctap2_credential_management(&req, timeout).await,
264270
uv_auth_used,
265-
timeout
271+
timeout,
272+
req
266273
)
267274
}?;
268275
Ok(())
@@ -339,4 +346,12 @@ impl Ctap2UserVerifiableRequest for Ctap2CredentialManagementRequest {
339346
fn wants_persistent_token(&self) -> bool {
340347
self.use_persistent_token
341348
}
349+
350+
fn note_persistent_token_rejected(&mut self) {
351+
self.persistent_token_rejected = true;
352+
}
353+
354+
fn persistent_token_rejected(&self) -> bool {
355+
self.persistent_token_rejected
356+
}
342357
}

‎libwebauthn/src/proto/ctap2/model.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,14 @@ pub trait Ctap2UserVerifiableRequest {
322322
fn wants_persistent_token(&self) -> bool {
323323
false
324324
}
325+
/// Record that a reused persistent (pcmr) token was rejected by the authenticator, so
326+
/// the retry stops reusing it and mints a fresh one instead. Default: no-op.
327+
fn note_persistent_token_rejected(&mut self) {}
328+
/// Whether a reused persistent token was already rejected during this ceremony, per
329+
/// [`Self::note_persistent_token_rejected`]. Default false.
330+
fn persistent_token_rejected(&self) -> bool {
331+
false
332+
}
325333
}
326334

327335
#[derive(Debug, Clone, Copy, PartialEq, Eq)]

‎libwebauthn/src/proto/ctap2/model/credential_management.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ pub struct Ctap2CredentialManagementRequest {
3636
/// Set from getInfo and store availability before `permissions()` is read.
3737
#[serde(skip)]
3838
pub use_persistent_token: bool,
39+
40+
/// Set once a reused persistent token is rejected, so the retry skips recognition and
41+
/// mints a fresh token instead of looping on the same stale record.
42+
#[serde(skip)]
43+
pub persistent_token_rejected: bool,
3944
}
4045

4146
#[repr(u32)]
@@ -150,6 +155,7 @@ impl Ctap2CredentialManagementRequest {
150155
uv_auth_param: None,
151156
use_legacy_preview: false,
152157
use_persistent_token: false,
158+
persistent_token_rejected: false,
153159
}
154160
}
155161

@@ -161,6 +167,7 @@ impl Ctap2CredentialManagementRequest {
161167
uv_auth_param: None,
162168
use_legacy_preview: false,
163169
use_persistent_token: false,
170+
persistent_token_rejected: false,
164171
}
165172
}
166173

@@ -172,6 +179,7 @@ impl Ctap2CredentialManagementRequest {
172179
uv_auth_param: None,
173180
use_legacy_preview: false,
174181
use_persistent_token: false,
182+
persistent_token_rejected: false,
175183
}
176184
}
177185

@@ -187,6 +195,7 @@ impl Ctap2CredentialManagementRequest {
187195
uv_auth_param: None,
188196
use_legacy_preview: false,
189197
use_persistent_token: false,
198+
persistent_token_rejected: false,
190199
}
191200
}
192201

@@ -200,6 +209,7 @@ impl Ctap2CredentialManagementRequest {
200209
uv_auth_param: None,
201210
use_legacy_preview: false,
202211
use_persistent_token: false,
212+
persistent_token_rejected: false,
203213
}
204214
}
205215

@@ -215,6 +225,7 @@ impl Ctap2CredentialManagementRequest {
215225
uv_auth_param: None,
216226
use_legacy_preview: false,
217227
use_persistent_token: false,
228+
persistent_token_rejected: false,
218229
}
219230
}
220231

@@ -233,6 +244,7 @@ impl Ctap2CredentialManagementRequest {
233244
uv_auth_param: None,
234245
use_legacy_preview: false,
235246
use_persistent_token: false,
247+
persistent_token_rejected: false,
236248
}
237249
}
238250
}

‎libwebauthn/src/proto/ctap2/protocol.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ mod tests {
469469
uv_auth_param: None,
470470
use_legacy_preview: false,
471471
use_persistent_token: false,
472+
persistent_token_rejected: false,
472473
};
473474
let expected_request: CborRequest = (&request).try_into().unwrap();
474475
channel.push_command_pair(expected_request, error_response(CtapError::PINRequired));

‎libwebauthn/src/webauthn.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,20 @@ fn prf_forces_uv_upgrade(prf_present: bool, uv: UserVerificationRequirement) ->
4444
}
4545

4646
macro_rules! handle_errors {
47+
// Callers that never reuse a persistent token (make-credential, get-assertion,
48+
// authenticator-config, bio-enrollment): nothing to notify on a persistent rejection.
4749
($channel: expr, $resp: expr, $uv_auth_used: expr, $timeout: expr) => {
50+
handle_errors!(@inner $channel, $resp, $uv_auth_used, $timeout, {})
51+
};
52+
// Credential-management callers pass their request so a rejected persistent token is
53+
// marked on it, forcing the retry to mint a fresh token instead of reusing the same
54+
// stale record. This keeps loop termination independent of the best-effort store delete.
55+
($channel: expr, $resp: expr, $uv_auth_used: expr, $timeout: expr, $request: expr) => {
56+
handle_errors!(@inner $channel, $resp, $uv_auth_used, $timeout, {
57+
$request.note_persistent_token_rejected();
58+
})
59+
};
60+
(@inner $channel: expr, $resp: expr, $uv_auth_used: expr, $timeout: expr, $on_persistent_reject: block) => {
4861
match $resp {
4962
Err(Error::Ctap(CtapError::PINAuthInvalid))
5063
if $uv_auth_used == UsedPinUvAuthToken::FromEphemeralStorage =>
@@ -62,6 +75,7 @@ macro_rules! handle_errors {
6275
store.delete(id).await;
6376
}
6477
}
78+
$on_persistent_reject
6579
continue;
6680
}
6781
Err(Error::Ctap(CtapError::UVInvalid)) => {

‎libwebauthn/src/webauthn/pin_uv_auth_token.rs‎

Lines changed: 128 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,12 @@ where
7777
ctap2_request.handle_legacy_preview(&get_info_response);
7878

7979
// Decide whether this request acquires a persistent (pcmr) token. A persistent token
80-
// outranks a same-session ephemeral one, so try it first.
80+
// outranks a same-session ephemeral one, so try it first. Skip reuse once a stored
81+
// token has been rejected this ceremony, so the retry mints a fresh one rather than
82+
// looping on the same stale record.
8183
let persistent_token_store = channel.persistent_token_store();
8284
ctap2_request.set_persistent_token_use(&get_info_response, persistent_token_store.is_some());
83-
if ctap2_request.wants_persistent_token() {
85+
if ctap2_request.wants_persistent_token() && !ctap2_request.persistent_token_rejected() {
8486
if let Some(store) = &persistent_token_store {
8587
if let Some((id, record)) =
8688
recognize_authenticator(store.as_ref(), &get_info_response).await
@@ -1681,6 +1683,125 @@ mod test {
16811683
assert!(recv.is_empty());
16821684
}
16831685

1686+
// The spec-real foreign-invalidation path. A PIN change (or any out-of-band reset of the
1687+
// persistent token) regenerates the token bytes, so the stored token can no longer decrypt
1688+
// the new encIdentifier: recognition MISSES with no PINAuthInvalid round trip, a fresh pcmr
1689+
// token is minted, and reaping by the stable device identifier evicts the dead record.
1690+
// Complements persistent_token_self_heals_on_rejection, which covers the defensive
1691+
// recognized-but-rejected arm.
1692+
#[tokio::test]
1693+
async fn recognition_miss_remints_and_reaps() {
1694+
let mut channel = MockChannel::new();
1695+
let device_identifier = [0x42; 16];
1696+
let aaguid = [0x07; 16];
1697+
let minted_token = [0x05; 16];
1698+
let stale_token = vec![0x11; 32];
1699+
1700+
// A stale record for THIS device (same device identifier) holding the old token bytes.
1701+
let store = Arc::new(MemoryPersistentTokenStore::new());
1702+
store
1703+
.put(
1704+
&"stale".to_string(),
1705+
&PersistentTokenRecord {
1706+
persistent_token: stale_token,
1707+
pin_uv_auth_protocol: Ctap2PinUvAuthProtocol::One,
1708+
device_identifier,
1709+
aaguid,
1710+
},
1711+
)
1712+
.await;
1713+
channel.set_persistent_token_store(store.clone());
1714+
1715+
// encIdentifier is built under the NEW token, so the stale record fails recognition.
1716+
let info = pcmr_get_info(
1717+
&[
1718+
("uv", true),
1719+
("pinUvAuthToken", true),
1720+
("perCredMgmtRO", true),
1721+
],
1722+
&minted_token,
1723+
device_identifier,
1724+
aaguid,
1725+
);
1726+
channel.push_command_pair(
1727+
CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo),
1728+
CborResponse::new_success_from_slice(to_vec(&info).unwrap().as_slice()),
1729+
);
1730+
1731+
let key_agreement_req = CborRequest::try_from(
1732+
&Ctap2ClientPinRequest::new_get_key_agreement(Ctap2PinUvAuthProtocol::One),
1733+
)
1734+
.unwrap();
1735+
let key_agreement_resp = CborResponse::new_success_from_slice(
1736+
to_vec(&Ctap2ClientPinResponse {
1737+
key_agreement: Some(get_key_agreement()),
1738+
pin_uv_auth_token: None,
1739+
pin_retries: None,
1740+
power_cycle_state: None,
1741+
uv_retries: None,
1742+
})
1743+
.unwrap()
1744+
.as_slice(),
1745+
);
1746+
channel.push_command_pair(key_agreement_req, key_agreement_resp);
1747+
1748+
let pin_protocol = PinUvAuthProtocolOne::new();
1749+
let (public_key, shared_secret) = pin_protocol.encapsulate(&get_key_agreement()).unwrap();
1750+
let uv_token_req =
1751+
CborRequest::try_from(&Ctap2ClientPinRequest::new_get_uv_token_with_perm(
1752+
Ctap2PinUvAuthProtocol::One,
1753+
public_key,
1754+
Ctap2AuthTokenPermissionRole::PERSISTENT_CREDENTIAL_MANAGEMENT_READ_ONLY,
1755+
None,
1756+
))
1757+
.unwrap();
1758+
let encrypted_token = pin_protocol.encrypt(&shared_secret, &minted_token).unwrap();
1759+
let uv_token_resp = CborResponse::new_success_from_slice(
1760+
to_vec(&Ctap2ClientPinResponse {
1761+
key_agreement: None,
1762+
pin_uv_auth_token: Some(ByteBuf::from(encrypted_token)),
1763+
pin_retries: None,
1764+
power_cycle_state: None,
1765+
uv_retries: None,
1766+
})
1767+
.unwrap()
1768+
.as_slice(),
1769+
);
1770+
channel.push_command_pair(uv_token_req, uv_token_resp);
1771+
1772+
let mut recv = channel.get_ux_update_receiver();
1773+
let recv_handle = tokio::task::spawn(async move {
1774+
assert_eq!(recv.recv().await, Ok(UvUpdate::PresenceRequired));
1775+
recv
1776+
});
1777+
1778+
let mut req = Ctap2CredentialManagementRequest::new_get_credential_metadata();
1779+
let result = user_verification(
1780+
&mut channel,
1781+
UserVerificationRequirement::Preferred,
1782+
&mut req,
1783+
TIMEOUT,
1784+
)
1785+
.await;
1786+
1787+
// Recognition missed, so the token was minted, not reused.
1788+
assert_eq!(
1789+
result,
1790+
Ok(UsedPinUvAuthToken::NewlyCalculated(
1791+
Ctap2UserVerificationOperation::GetPinUvAuthTokenUsingUvWithPermissions
1792+
))
1793+
);
1794+
// The stale record was reaped and replaced by exactly one fresh record.
1795+
let listed = store.list().await;
1796+
assert_eq!(listed.len(), 1);
1797+
assert!(listed.iter().all(|(id, _)| id != "stale"));
1798+
assert_eq!(listed[0].1.persistent_token, minted_token.to_vec());
1799+
assert_eq!(listed[0].1.device_identifier, device_identifier);
1800+
1801+
let recv = recv_handle.await.expect("Failed to join update thread");
1802+
assert!(recv.is_empty());
1803+
}
1804+
16841805
#[tokio::test]
16851806
async fn persistent_token_self_heals_on_rejection() {
16861807
use crate::management::CredentialManagement;
@@ -1704,8 +1825,11 @@ mod test {
17041825
.await;
17051826
channel.set_persistent_token_store(store.clone());
17061827

1707-
// The device still computes encIdentifier under our token (a PIN change cleared the
1708-
// token's permissions, not its bytes), so recognition matches but the op is rejected.
1828+
// Defensive path: a recognized token is rejected by the device. The encIdentifier is
1829+
// built under our stored token so recognition matches, yet the op returns
1830+
// PINAuthInvalid; we evict and re-mint. (A real PIN change regenerates the token bytes,
1831+
// per resetPersistentPinUvAuthToken, so recognition would instead miss and re-mint
1832+
// without a rejection; that path is covered by recognition_miss_remints_and_reaps.)
17091833
let info = pcmr_get_info(
17101834
&[
17111835
("uv", true),

0 commit comments

Comments
 (0)