Skip to content

Commit 09623ac

Browse files
fix(nfc): preserve APDU status words on the U2F path (#247)
On the U2F-over-NFC path the channel discarded the card status word and reported every non-success as a framing error. The U2F layer relies on the status word to tell whether a credential was absent, whether user presence is needed, or whether U2F is disabled, so authentication with several allowed credentials aborted on the first non-match. This preserves the real status word so the U2F layer can interpret it, and reserves the framing error for genuinely malformed responses. The NFC code only builds with an NFC backend feature enabled, so a separate commit adds a CI step that runs the NFC backend tests, ensuring the new guard is exercised. Includes a regression test confirming a not-registered status is surfaced correctly.
1 parent 15b5559 commit 09623ac

4 files changed

Lines changed: 78 additions & 4 deletions

File tree

.github/workflows/rust.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,9 @@ jobs:
2929
run: cargo test --workspace --verbose
3030
env:
3131
LIBWEBAUTHN_PSL_SYSTEM_TEST: "1"
32+
- name: Run tests with nfc-backend-pcsc
33+
run: cargo test -p libwebauthn --lib --features nfc-backend-pcsc --verbose
34+
env:
35+
LIBWEBAUTHN_PSL_SYSTEM_TEST: "1"
3236
- name: Verify libwebauthn publishes cleanly
3337
run: cargo publish --dry-run -p libwebauthn

libwebauthn/src/proto/ctap1/apdu/response.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub struct ApduResponse {
1616
pub enum ApduResponseStatus {
1717
NoError = 0x9000,
1818
UserPresenceTestFailed = 0x6985,
19+
CommandNotAllowed = 0x6986,
1920
InvalidKeyHandle = 0x6A80,
2021
InvalidRequestLength = 0x6700,
2122
InvalidClassByte = 0x6E00,

libwebauthn/src/proto/error.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ impl From<ApduResponseStatus> for CtapError {
8484
match status {
8585
ApduResponseStatus::NoError => CtapError::Ok,
8686
ApduResponseStatus::UserPresenceTestFailed => CtapError::UserPresenceRequired,
87+
ApduResponseStatus::CommandNotAllowed => CtapError::NotAllowed,
8788
ApduResponseStatus::InvalidKeyHandle => CtapError::NoCredentials,
8889
ApduResponseStatus::InvalidRequestLength => CtapError::InvalidLength,
8990
ApduResponseStatus::InvalidClassByte => CtapError::Other,

libwebauthn/src/transport/nfc/channel.rs

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ where
194194
Ok(res)
195195
}
196196

197-
pub fn handle<'a>(
197+
/// Returns the full response, payload plus SW1/SW2 trailer, without interpreting the status.
198+
fn handle_raw<'a>(
198199
&'a mut self,
199200
ctx: Ctx,
200201
command: impl Into<Command<'a>>,
@@ -223,6 +224,15 @@ where
223224
}
224225

225226
rapdu.extend_from_slice(&[sw1, sw2]);
227+
Ok(rapdu)
228+
}
229+
230+
pub fn handle<'a>(
231+
&'a mut self,
232+
ctx: Ctx,
233+
command: impl Into<Command<'a>>,
234+
) -> Result<Vec<u8>, NfcError> {
235+
let rapdu = self.handle_raw(ctx, command)?;
226236
Result::from(Response::from(rapdu.as_slice()))
227237
.map(|p| p.to_vec())
228238
.map_err(|e| {
@@ -259,10 +269,11 @@ where
259269

260270
#[instrument(level = Level::DEBUG, skip_all)]
261271
async fn apdu_send(&mut self, request: &ApduRequest, _timeout: Duration) -> Result<(), Error> {
262-
let resp = self.handle(self.ctx, request)?;
272+
let resp = self.handle_raw(self.ctx, request)?;
263273
trace!("apdu_send {:?}", resp);
264274

265-
let apdu_response = ApduResponse::new_success(&resp);
275+
let apdu_response = ApduResponse::try_from(&resp)
276+
.or(Err(Error::Transport(TransportError::InvalidFraming)))?;
266277
self.apdu_response = Some(apdu_response);
267278
Ok(())
268279
}
@@ -360,7 +371,13 @@ where
360371

361372
#[cfg(test)]
362373
mod tests {
363-
use super::is_fido2_version;
374+
use std::fmt::{Display, Formatter};
375+
use std::time::Duration;
376+
377+
use super::{is_fido2_version, ChannelSettings, HandlerInCtx, NfcBackend, NfcChannel};
378+
use crate::proto::ctap1::apdu::{ApduRequest, ApduResponseStatus};
379+
use crate::proto::CtapError;
380+
use crate::transport::channel::Channel;
364381

365382
#[test]
366383
fn fido2_versions_are_recognised() {
@@ -386,4 +403,55 @@ mod tests {
386403
assert!(!is_fido2_version(b"fido_2_0"));
387404
assert!(!is_fido2_version(b"FIDO_2_0\0"));
388405
}
406+
407+
/// Backend that echoes a fixed APDU response regardless of the command.
408+
struct FixedResponseBackend {
409+
response: Vec<u8>,
410+
}
411+
412+
impl Display for FixedResponseBackend {
413+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
414+
write!(f, "FixedResponseBackend")
415+
}
416+
}
417+
418+
impl HandlerInCtx<u8> for FixedResponseBackend {
419+
fn handle_in_ctx(
420+
&mut self,
421+
_ctx: u8,
422+
_command: &[u8],
423+
response: &mut [u8],
424+
) -> apdu_core::Result {
425+
let n = self.response.len();
426+
response[..n].copy_from_slice(&self.response);
427+
Ok(n)
428+
}
429+
}
430+
431+
impl NfcBackend<u8> for FixedResponseBackend {}
432+
433+
#[tokio::test]
434+
async fn u2f_wrong_data_status_word_is_preserved() {
435+
// SW_WRONG_DATA (0x6A80) must surface as NoCredentials, not InvalidFraming.
436+
let backend = FixedResponseBackend {
437+
response: vec![0x6A, 0x80],
438+
};
439+
let mut channel = NfcChannel::new(Box::new(backend), 0u8, ChannelSettings::default());
440+
let request = ApduRequest::new(0x02, 0x00, 0x00, None, None);
441+
442+
channel
443+
.apdu_send(&request, Duration::from_secs(1))
444+
.await
445+
.expect("non-success SW must not collapse to InvalidFraming");
446+
let response = channel.apdu_recv(Duration::from_secs(1)).await.unwrap();
447+
448+
assert_eq!(
449+
response.status().unwrap(),
450+
ApduResponseStatus::InvalidKeyHandle
451+
);
452+
assert_eq!(
453+
CtapError::from(response.status().unwrap()),
454+
CtapError::NoCredentials
455+
);
456+
}
389457
}

0 commit comments

Comments
 (0)