Skip to content

Commit

Permalink
Improve parsing of serial numbers (#466)
Browse files Browse the repository at this point in the history
Checks the length of the data returned when querying the serial number,
returning an error if it's longer than 4 bytes, and left-padding with
zeroes if it's too short.

This fixes some potential panics due to incorrect slice lengths as were
experienced in #465
  • Loading branch information
tony-iqlusion authored Jan 7, 2023
1 parent 1d33ea1 commit f49c617
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 48 deletions.
95 changes: 47 additions & 48 deletions src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,61 +105,60 @@ impl<'tx> Transaction<'tx> {

/// Get YubiKey device serial number.
pub fn get_serial(&self, version: Version) -> Result<Serial> {
let response = if version.major < 5 {
// YK4 requires switching to the yk applet to retrieve the serial
let sw = Apdu::new(Ins::SelectApplication)
.p1(0x04)
.data(YK_AID)
.transmit(self, 0xFF)?
.status_words();

if !sw.is_success() {
error!("failed selecting yk application: {:04x}", sw.code());
return Err(Error::GenericError);
}
match version.major {
4 => {
// YK4 requires switching to the yk applet to retrieve the serial
let sw = Apdu::new(Ins::SelectApplication)
.p1(0x04)
.data(YK_AID)
.transmit(self, 0xFF)?
.status_words();

if !sw.is_success() {
error!("failed selecting yk application: {:04x}", sw.code());
return Err(Error::GenericError);
}

let resp = Apdu::new(0x01).p1(0x10).transmit(self, 0xFF)?;
let response = Apdu::new(0x01).p1(0x10).transmit(self, 0xFF)?;

if !resp.is_success() {
error!(
"failed retrieving serial number: {:04x}",
resp.status_words().code()
);
return Err(Error::GenericError);
}

// reselect the PIV applet
let sw = Apdu::new(Ins::SelectApplication)
.p1(0x04)
.data(PIV_AID)
.transmit(self, 0xFF)?
.status_words();
if !response.is_success() {
error!(
"failed retrieving serial number: {:04x}",
response.status_words().code()
);
return Err(Error::GenericError);
}

if !sw.is_success() {
error!("failed selecting application: {:04x}", sw.code());
return Err(Error::GenericError);
}
// reselect the PIV applet
let sw = Apdu::new(Ins::SelectApplication)
.p1(0x04)
.data(PIV_AID)
.transmit(self, 0xFF)?
.status_words();

resp
} else {
// YK5 implements getting the serial as a PIV applet command (0xf8)
let resp = Apdu::new(Ins::GetSerial).transmit(self, 0xFF)?;
if !sw.is_success() {
error!("failed selecting application: {:04x}", sw.code());
return Err(Error::GenericError);
}

if !resp.is_success() {
error!(
"failed retrieving serial number: {:04x}",
resp.status_words().code()
);
return Err(Error::GenericError);
response.data().try_into()
}
5 => {
// YK5 implements getting the serial as a PIV applet command (0xf8)
let response = Apdu::new(Ins::GetSerial).transmit(self, 0xFF)?;

if !response.is_success() {
error!(
"failed retrieving serial number: {:04x}",
response.status_words().code()
);
return Err(Error::GenericError);
}

resp
};

response.data()[..4]
.try_into()
.map(|serial| Serial::from(u32::from_be_bytes(serial)))
.map_err(|_| Error::SizeError)
response.data().try_into()
}
_ => Err(Error::NotSupported),
}
}

/// Verify device PIN.
Expand Down
14 changes: 14 additions & 0 deletions src/yubikey.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@ impl From<Serial> for u32 {
}
}

impl TryFrom<&[u8]> for Serial {
type Error = Error;

fn try_from(bytes: &[u8]) -> Result<Self> {
if bytes.len() > 4 {
return Err(Error::SizeError);
}

let mut arr = [0u8; 4];
arr[(4 - bytes.len())..].copy_from_slice(bytes);
Ok(Self(u32::from_be_bytes(arr)))
}
}

impl FromStr for Serial {
type Err = Error;

Expand Down

0 comments on commit f49c617

Please sign in to comment.