Skip to content

Commit 5294ac7

Browse files
fix(credmgmt): Yubikey error when NextCred is not handled statefully (#264)
I have a Token2 and a Yubikey "Security Key C NFC". The Yubikey is more strict than the Token2 and causes errors. `cargo run --example cred_management_hid` with option 2: "(2) Enumerate credentials" causes `Error: Ctap(NotAllowed)` on the Yubikey after the PIN have been input. I think this is due to enumerateCredentialsGetNextCredential and enumerateRPsGetNextRP being statefull in the fido v2.1 spec. --------- Co-authored-by: Alfie Fresta <alfie.fresta@gmail.com>
1 parent c466fbf commit 5294ac7

7 files changed

Lines changed: 190 additions & 37 deletions

File tree

libwebauthn/src/management/credential_management.rs

Lines changed: 140 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ where
111111
req
112112
)
113113
}?;
114+
self.set_cred_mgmt_preview(req.use_legacy_preview);
114115
Ok((
115116
Ctap2RPData::new(
116117
unwrap_field!(resp.rp),
@@ -122,24 +123,8 @@ where
122123

123124
async fn enumerate_rps_next_rp(&mut self, timeout: Duration) -> Result<Ctap2RPData, Error> {
124125
let mut req = Ctap2CredentialManagementRequest::new_enumerate_rps_next_rp();
125-
let resp = loop {
126-
let uv_auth_used = user_verification(
127-
self,
128-
UserVerificationRequirement::Preferred,
129-
&mut req,
130-
timeout,
131-
)
132-
.await?;
133-
134-
// On success, this is an all-empty Ctap2AuthenticatorConfigResponse
135-
handle_errors!(
136-
self,
137-
self.ctap2_credential_management(&req, timeout).await,
138-
uv_auth_used,
139-
timeout,
140-
req
141-
)
142-
}?;
126+
req.use_legacy_preview = self.cred_mgmt_preview();
127+
let resp = self.ctap2_credential_management(&req, timeout).await?;
143128
Ok(Ctap2RPData::new(
144129
unwrap_field!(resp.rp),
145130
unwrap_field!(resp.rp_id_hash).into_vec(),
@@ -170,6 +155,7 @@ where
170155
req
171156
)
172157
}?;
158+
self.set_cred_mgmt_preview(req.use_legacy_preview);
173159
let cred = Ctap2CredentialData::new(
174160
unwrap_field!(resp.user),
175161
unwrap_field!(resp.credential_id),
@@ -186,24 +172,8 @@ where
186172
timeout: Duration,
187173
) -> Result<Ctap2CredentialData, Error> {
188174
let mut req = Ctap2CredentialManagementRequest::new_enumerate_credentials_next();
189-
let resp = loop {
190-
let uv_auth_used = user_verification(
191-
self,
192-
UserVerificationRequirement::Preferred,
193-
&mut req,
194-
timeout,
195-
)
196-
.await?;
197-
198-
// On success, this is an all-empty Ctap2AuthenticatorConfigResponse
199-
handle_errors!(
200-
self,
201-
self.ctap2_credential_management(&req, timeout).await,
202-
uv_auth_used,
203-
timeout,
204-
req
205-
)
206-
}?;
175+
req.use_legacy_preview = self.cred_mgmt_preview();
176+
let resp = self.ctap2_credential_management(&req, timeout).await?;
207177
let cred = Ctap2CredentialData::new(
208178
unwrap_field!(resp.user),
209179
unwrap_field!(resp.credential_id),
@@ -367,9 +337,11 @@ mod test {
367337
use crate::proto::ctap2::cbor::{self, CborRequest, CborResponse};
368338
use crate::proto::ctap2::{
369339
Ctap2CommandCode, Ctap2CredentialManagementRequest, Ctap2GetInfoResponse,
370-
Ctap2PublicKeyCredentialRpEntity,
340+
Ctap2PublicKeyCredentialDescriptor, Ctap2PublicKeyCredentialRpEntity,
341+
Ctap2PublicKeyCredentialType, Ctap2PublicKeyCredentialUserEntity,
371342
};
372343
use crate::transport::mock::channel::MockChannel;
344+
use std::collections::{BTreeMap, HashMap};
373345

374346
const TIMEOUT: Duration = Duration::from_secs(1);
375347

@@ -413,4 +385,135 @@ mod test {
413385
assert_eq!(rp_data.rp_id_hash.len(), 32);
414386
assert_eq!(rp_data.rp_id_hash, hash.to_vec());
415387
}
388+
389+
// GetNextRP returns only rp (0x03) and rpIDHash (0x04).
390+
#[derive(SerializeIndexed)]
391+
struct EnumerateRpsNextResponse {
392+
#[serde(index = 0x03)]
393+
rp: Ctap2PublicKeyCredentialRpEntity,
394+
#[serde(index = 0x04)]
395+
rp_id_hash: ByteBuf,
396+
}
397+
398+
// GetNextCredential returns user (0x06), credentialID (0x07), publicKey (0x08), credProtect (0x0A).
399+
#[derive(SerializeIndexed)]
400+
struct EnumerateCredsNextResponse {
401+
#[serde(index = 0x06)]
402+
user: Ctap2PublicKeyCredentialUserEntity,
403+
#[serde(index = 0x07)]
404+
credential_id: Ctap2PublicKeyCredentialDescriptor,
405+
#[serde(index = 0x08)]
406+
public_key: BTreeMap<i8, i8>,
407+
#[serde(index = 0x0A)]
408+
cred_protect: u64,
409+
}
410+
411+
#[tokio::test]
412+
async fn enumerate_rps_next_rp_sends_only_the_subcommand() {
413+
let req = Ctap2CredentialManagementRequest::new_enumerate_rps_next_rp();
414+
let cbor_req: CborRequest = (&req).try_into().unwrap();
415+
assert_eq!(
416+
cbor_req.command,
417+
Ctap2CommandCode::AuthenticatorCredentialManagement
418+
);
419+
// CTAP 2.1 §6.8.3: {subCommand: enumerateRPsGetNextRP}, no pinUvAuth keys.
420+
assert_eq!(cbor_req.encoded_data, vec![0xA1, 0x01, 0x03]);
421+
422+
let hash = [0xCD_u8; 32];
423+
let fixture = EnumerateRpsNextResponse {
424+
rp: Ctap2PublicKeyCredentialRpEntity::new("example.org", "Example"),
425+
rp_id_hash: ByteBuf::from(hash.to_vec()),
426+
};
427+
let resp = CborResponse::new_success_from_slice(&cbor::to_vec(&fixture).unwrap());
428+
// Queue only the GetNext exchange: any interleaved command panics the mock.
429+
let mut channel = MockChannel::new();
430+
channel.push_command_pair(cbor_req, resp);
431+
432+
let rp_data = channel.enumerate_rps_next_rp(TIMEOUT).await.unwrap();
433+
assert_eq!(rp_data.rp_id_hash, hash.to_vec());
434+
}
435+
436+
#[tokio::test]
437+
async fn enumerate_credentials_next_sends_only_the_subcommand() {
438+
let req = Ctap2CredentialManagementRequest::new_enumerate_credentials_next();
439+
let cbor_req: CborRequest = (&req).try_into().unwrap();
440+
assert_eq!(
441+
cbor_req.command,
442+
Ctap2CommandCode::AuthenticatorCredentialManagement
443+
);
444+
// CTAP 2.1 §6.8.4: {subCommand: enumerateCredentialsGetNextCredential}, no pinUvAuth keys.
445+
assert_eq!(cbor_req.encoded_data, vec![0xA1, 0x01, 0x05]);
446+
447+
let fixture = EnumerateCredsNextResponse {
448+
user: Ctap2PublicKeyCredentialUserEntity::new(&[0x0B; 16], "bob", "bob"),
449+
credential_id: Ctap2PublicKeyCredentialDescriptor {
450+
id: ByteBuf::from(vec![0x1D; 32]),
451+
r#type: Ctap2PublicKeyCredentialType::PublicKey,
452+
transports: None,
453+
},
454+
public_key: BTreeMap::from([(1, 2), (3, -7)]),
455+
cred_protect: 1,
456+
};
457+
let resp = CborResponse::new_success_from_slice(&cbor::to_vec(&fixture).unwrap());
458+
let mut channel = MockChannel::new();
459+
channel.push_command_pair(cbor_req, resp);
460+
461+
let cred = channel.enumerate_credentials_next(TIMEOUT).await.unwrap();
462+
assert_eq!(cred.user.id, ByteBuf::from(vec![0x0B; 16]));
463+
assert_eq!(cred.cred_protect, 1);
464+
assert!(cred.large_blob_key.is_none());
465+
}
466+
467+
#[tokio::test]
468+
async fn get_next_reuses_preview_command_resolved_by_begin() {
469+
let mut channel = MockChannel::new();
470+
471+
// Device advertises credentialMgmtPreview only: Begin must resolve 0x41.
472+
let info = Ctap2GetInfoResponse {
473+
options: Some(HashMap::from([("credentialMgmtPreview".to_string(), true)])),
474+
..Default::default()
475+
};
476+
let info_req = CborRequest::new(Ctap2CommandCode::AuthenticatorGetInfo);
477+
let info_resp = CborResponse::new_success_from_slice(&cbor::to_vec(&info).unwrap());
478+
channel.push_command_pair(info_req, info_resp);
479+
480+
let mut begin_req = Ctap2CredentialManagementRequest::new_enumerate_rps_begin();
481+
begin_req.use_legacy_preview = true;
482+
let begin_cbor: CborRequest = (&begin_req).try_into().unwrap();
483+
assert_eq!(
484+
begin_cbor.command,
485+
Ctap2CommandCode::AuthenticatorCredentialManagementPreview
486+
);
487+
let begin_fixture = EnumerateRpsResponse {
488+
rp: Ctap2PublicKeyCredentialRpEntity::new("example.com", "Example"),
489+
rp_id_hash: ByteBuf::from(vec![0xEF; 32]),
490+
total_rps: 2,
491+
};
492+
channel.push_command_pair(
493+
begin_cbor,
494+
CborResponse::new_success_from_slice(&cbor::to_vec(&begin_fixture).unwrap()),
495+
);
496+
497+
let mut next_req = Ctap2CredentialManagementRequest::new_enumerate_rps_next_rp();
498+
next_req.use_legacy_preview = true;
499+
let next_cbor: CborRequest = (&next_req).try_into().unwrap();
500+
assert_eq!(
501+
next_cbor.command,
502+
Ctap2CommandCode::AuthenticatorCredentialManagementPreview
503+
);
504+
assert_eq!(next_cbor.encoded_data, vec![0xA1, 0x01, 0x03]);
505+
let next_fixture = EnumerateRpsNextResponse {
506+
rp: Ctap2PublicKeyCredentialRpEntity::new("example.org", "Example Two"),
507+
rp_id_hash: ByteBuf::from(vec![0x11; 32]),
508+
};
509+
channel.push_command_pair(
510+
next_cbor,
511+
CborResponse::new_success_from_slice(&cbor::to_vec(&next_fixture).unwrap()),
512+
);
513+
514+
let (_, total) = channel.enumerate_rps_begin(TIMEOUT).await.unwrap();
515+
assert_eq!(total, 2);
516+
let rp_data = channel.enumerate_rps_next_rp(TIMEOUT).await.unwrap();
517+
assert_eq!(rp_data.rp_id_hash, vec![0x11; 32]);
518+
}
416519
}

libwebauthn/src/transport/ble/channel.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ pub struct BleChannel<'a> {
3333
connection: Connection,
3434
revision: FidoRevision,
3535
auth_token_data: Option<AuthTokenData>,
36+
cred_mgmt_preview: bool,
3637
persistent_token_store: Option<Arc<dyn PersistentTokenStore>>,
3738
ux_update_sender: broadcast::Sender<UvUpdate>,
3839
}
@@ -57,6 +58,7 @@ impl<'a> BleChannel<'a> {
5758
connection,
5859
revision,
5960
auth_token_data: None,
61+
cred_mgmt_preview: false,
6062
persistent_token_store: settings.persistent_token_store,
6163
ux_update_sender,
6264
};
@@ -197,6 +199,14 @@ impl Ctap2AuthTokenStore for BleChannel<'_> {
197199
self.auth_token_data = None;
198200
}
199201

202+
fn set_cred_mgmt_preview(&mut self, uses_preview: bool) {
203+
self.cred_mgmt_preview = uses_preview;
204+
}
205+
206+
fn cred_mgmt_preview(&self) -> bool {
207+
self.cred_mgmt_preview
208+
}
209+
200210
fn persistent_token_store(&self) -> Option<Arc<dyn PersistentTokenStore>> {
201211
self.persistent_token_store.clone()
202212
}

libwebauthn/src/transport/cable/channel.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,12 @@ impl Ctap2AuthTokenStore for CableChannel {
202202

203203
fn clear_uv_auth_token_store(&mut self) {}
204204

205+
fn set_cred_mgmt_preview(&mut self, _uses_preview: bool) {}
206+
207+
fn cred_mgmt_preview(&self) -> bool {
208+
false
209+
}
210+
205211
fn persistent_token_store(&self) -> Option<Arc<dyn PersistentTokenStore>> {
206212
self.persistent_token_store.clone()
207213
}

libwebauthn/src/transport/channel.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ pub trait Ctap2AuthTokenStore {
159159
fn store_auth_data(&mut self, auth_token_data: AuthTokenData);
160160
fn get_auth_data(&self) -> Option<&AuthTokenData>;
161161
fn clear_uv_auth_token_store(&mut self);
162+
/// Command set resolved by the last credMgmt state-initializing request, so
163+
/// stateful GetNext continuations reuse it without re-fetching getInfo.
164+
fn set_cred_mgmt_preview(&mut self, uses_preview: bool);
165+
fn cred_mgmt_preview(&self) -> bool;
162166
fn get_uv_auth_token(&self, requested_permission: &Ctap2AuthTokenPermission) -> Option<&[u8]> {
163167
if let Some(stored_data) = self.get_auth_data() {
164168
if let Some(permission) = &stored_data.permission {

libwebauthn/src/transport/hid/channel.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ pub struct HidChannel<'d> {
8282
open_device: OpenHidDevice,
8383
init: InitResponse,
8484
auth_token_data: Option<AuthTokenData>,
85+
cred_mgmt_preview: bool,
8586
persistent_token_store: Option<Arc<dyn PersistentTokenStore>>,
8687
ux_update_sender: broadcast::Sender<UvUpdate>,
8788
handle: HidChannelHandle,
@@ -113,6 +114,7 @@ impl<'d> HidChannel<'d> {
113114
},
114115
init: InitResponse::default(),
115116
auth_token_data: None,
117+
cred_mgmt_preview: false,
116118
persistent_token_store: settings.persistent_token_store,
117119
ux_update_sender,
118120
handle,
@@ -615,6 +617,14 @@ impl Ctap2AuthTokenStore for HidChannel<'_> {
615617
self.auth_token_data = None;
616618
}
617619

620+
fn set_cred_mgmt_preview(&mut self, uses_preview: bool) {
621+
self.cred_mgmt_preview = uses_preview;
622+
}
623+
624+
fn cred_mgmt_preview(&self) -> bool {
625+
self.cred_mgmt_preview
626+
}
627+
618628
fn persistent_token_store(&self) -> Option<Arc<dyn PersistentTokenStore>> {
619629
self.persistent_token_store.clone()
620630
}

libwebauthn/src/transport/mock/channel.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub struct MockChannel {
2121
expected_requests: VecDeque<CborRequest>,
2222
responses: VecDeque<CborResponse>,
2323
auth_token_data: Option<AuthTokenData>,
24+
cred_mgmt_preview: bool,
2425
persistent_token_store: Option<Arc<dyn PersistentTokenStore>>,
2526
ux_update_sender: broadcast::Sender<UvUpdate>,
2627
pre_send_delay: Option<Duration>,
@@ -39,6 +40,7 @@ impl MockChannel {
3940
expected_requests: VecDeque::new(),
4041
responses: VecDeque::new(),
4142
auth_token_data: None,
43+
cred_mgmt_preview: false,
4244
persistent_token_store: None,
4345
ux_update_sender,
4446
pre_send_delay: None,
@@ -73,6 +75,14 @@ impl Ctap2AuthTokenStore for MockChannel {
7375
self.auth_token_data = None;
7476
}
7577

78+
fn set_cred_mgmt_preview(&mut self, uses_preview: bool) {
79+
self.cred_mgmt_preview = uses_preview;
80+
}
81+
82+
fn cred_mgmt_preview(&self) -> bool {
83+
self.cred_mgmt_preview
84+
}
85+
7686
fn persistent_token_store(&self) -> Option<Arc<dyn PersistentTokenStore>> {
7787
self.persistent_token_store.clone()
7888
}

libwebauthn/src/transport/nfc/channel.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ where
103103
{
104104
delegate: Box<dyn NfcBackend<Ctx> + Send + Sync>,
105105
auth_token_data: Option<AuthTokenData>,
106+
cred_mgmt_preview: bool,
106107
persistent_token_store: Option<Arc<dyn PersistentTokenStore>>,
107108
ux_update_sender: broadcast::Sender<UvUpdate>,
108109
handle: NfcChannelHandle,
@@ -137,6 +138,7 @@ where
137138
NfcChannel {
138139
delegate,
139140
auth_token_data: None,
141+
cred_mgmt_preview: false,
140142
persistent_token_store: settings.persistent_token_store,
141143
ux_update_sender,
142144
handle,
@@ -364,6 +366,14 @@ where
364366
self.auth_token_data = None;
365367
}
366368

369+
fn set_cred_mgmt_preview(&mut self, uses_preview: bool) {
370+
self.cred_mgmt_preview = uses_preview;
371+
}
372+
373+
fn cred_mgmt_preview(&self) -> bool {
374+
self.cred_mgmt_preview
375+
}
376+
367377
fn persistent_token_store(&self) -> Option<Arc<dyn PersistentTokenStore>> {
368378
self.persistent_token_store.clone()
369379
}

0 commit comments

Comments
 (0)