Skip to content

Commit

Permalink
Drop YubiKey NEO support (closes #18)
Browse files Browse the repository at this point in the history
YubiKey NEOs are legacy YubiKey devices, most of which contain
unpatchable security vulnerabilities.

They have smaller buffer sizes than YK4 and YK5, which necessitates a
whole bunch of conditional gating and buffer size calculations.

Getting rid of them simplifies this logic and allows us to assume
consistent buffer sizes everywhere.

We never tested on NEOs anyway, and looking at the deleted code it seems
it may have been miscalculating the NEO's buffer size!

If someone *really* wants to support NEOs, it shouldn't be that hard to
restore, but the codebase is definitely cleaner without it.
  • Loading branch information
tarcieri committed Dec 7, 2019
1 parent 962089d commit f6915ce
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 126 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ endorsed by Yubico.

## Supported YubiKeys

- [YubiKey NEO] series (may be dropped in the future, see [#18])
- [YubiKey 4] series
- [YubiKey 5] series

NOTE: Nano and USB-C variants of the above are also supported
NOTE: Nano and USB-C variants of the above are also supported.
Pre-YK4 [YubiKey NEO] series is **NOT** supported (see [#18]).

## Security Warning

Expand Down
4 changes: 2 additions & 2 deletions cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ utility with general-purpose public-key encryption and signing support.

## Supported YubiKeys

- [YubiKey NEO] series (may be dropped in the future, see [#18])
- [YubiKey 4] series
- [YubiKey 5] series

NOTE: Nano and USB-C variants of the above are also supported
NOTE: Nano and USB-C variants of the above are also supported.
Pre-YK4 [YubiKey NEO] series is **NOT** supported (see [#18]).

## Security Warning

Expand Down
9 changes: 3 additions & 6 deletions src/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,15 @@ impl Certificate {
/// Write this certificate into the YubiKey in the given slot
#[cfg(feature = "untested")]
pub fn write(&self, yubikey: &mut YubiKey, slot: SlotId, certinfo: u8) -> Result<(), Error> {
let max_size = yubikey.obj_size_max();
let txn = yubikey.begin_transaction()?;
write_certificate(&txn, slot, Some(&self.data), certinfo, max_size)
write_certificate(&txn, slot, Some(&self.data), certinfo)
}

/// Delete a certificate located at the given slot of the given YubiKey
#[cfg(feature = "untested")]
pub fn delete(yubikey: &mut YubiKey, slot: SlotId) -> Result<(), Error> {
let max_size = yubikey.obj_size_max();
let txn = yubikey.begin_transaction()?;
write_certificate(&txn, slot, None, 0, max_size)
write_certificate(&txn, slot, None, 0)
}

/// Initialize a local certificate struct from the given bytebuffer
Expand Down Expand Up @@ -244,7 +242,6 @@ pub(crate) fn write_certificate(
slot: SlotId,
data: Option<&[u8]>,
certinfo: u8,
max_size: usize,
) -> Result<(), Error> {
let mut buf = [0u8; CB_OBJ_MAX];
let mut offset = 0;
Expand All @@ -261,7 +258,7 @@ pub(crate) fn write_certificate(
req_len += set_length(&mut buf, data.len());
req_len += data.len();

if req_len < data.len() || req_len > max_size {
if req_len < data.len() || req_len > CB_OBJ_MAX {
return Err(Error::SizeError);
}

Expand Down
22 changes: 12 additions & 10 deletions src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,27 @@ pub const szLOG_SOURCE: &str = "yubikey-piv.rs";
pub const ADMIN_FLAGS_1_PUK_BLOCKED: u8 = 0x01;
pub const ADMIN_FLAGS_1_PROTECTED_MGM: u8 = 0x02;

/// PIV Applet ID
pub const PIV_AID: [u8; 5] = [0xa0, 0x00, 0x00, 0x03, 0x08];

/// MGMT Applet ID.
/// <https://developers.yubico.com/PIV/Introduction/Admin_access.html>
pub const MGMT_AID: [u8; 8] = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x47, 0x11, 0x17];

/// YubiKey OTP Applet ID. Needed to query serial on YK4.
pub const YK_AID: [u8; 8] = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x20, 0x01, 0x01];

pub const CB_ADMIN_TIMESTAMP: usize = 0x04;
pub const CB_ADMIN_SALT: usize = 16;

pub const CB_ATR_MAX: usize = 33;

pub const CB_BUF_MAX_NEO: usize = 2048;
pub const CB_BUF_MAX_YK4: usize = 3072;
pub const CB_BUF_MAX: usize = CB_BUF_MAX_YK4;
pub const CB_BUF_MAX: usize = 3072;

pub const CB_ECC_POINTP256: usize = 65;
pub const CB_ECC_POINTP384: usize = 97;

pub const CB_OBJ_MAX_YK4: usize = CB_BUF_MAX_YK4 - 9;
pub const CB_OBJ_MAX: usize = CB_OBJ_MAX_YK4;
pub const CB_OBJ_MAX_NEO: usize = CB_BUF_MAX_NEO - 9;
pub const CB_OBJ_MAX: usize = CB_BUF_MAX - 9;

pub const CB_OBJ_TAG_MIN: usize = 2; // 1 byte tag + 1 byte len
pub const CB_OBJ_TAG_MAX: usize = (CB_OBJ_TAG_MIN + 2); // 1 byte tag + 3 bytes len
Expand Down Expand Up @@ -82,9 +88,7 @@ pub const DES_LEN_3DES: usize = DES_LEN_DES * 3;
// device types

pub const DEVTYPE_UNKNOWN: u32 = 0x0000_0000;
pub const DEVTYPE_NEO: u32 = 0x4E45_0000; //"NE"
pub const DEVTYPE_YK: u32 = 0x594B_0000; //"YK"
pub const DEVTYPE_NEOr3: u32 = (DEVTYPE_NEO | 0x0000_7233); //"r3"
pub const DEVTYPE_YK4: u32 = (DEVTYPE_YK | 0x0000_0034); // "4"
pub const DEVYTPE_YK5: u32 = (DEVTYPE_YK | 0x0000_0035); // "5"

Expand Down Expand Up @@ -124,8 +128,6 @@ pub const TAG_ECC_POINT: u8 = 0x86;

pub const YKPIV_ALGO_3DES: u8 = 0x03;

pub const YKPIV_ATR_NEO_R3: &[u8] = b";\xFC\x13\0\0\x811\xFE\x15YubikeyNEOr3\xE1\0";

pub const YKPIV_CHUID_SIZE: usize = 59;
pub const YKPIV_CARDID_SIZE: usize = 16;
pub const YKPIV_FASCN_SIZE: usize = 25;
Expand Down
3 changes: 1 addition & 2 deletions src/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ impl Container {
let n_containers = containers.len();
let data_len = n_containers * CONTAINER_REC_LEN;

let max_size = yubikey.obj_size_max();
let txn = yubikey.begin_transaction()?;

if n_containers == 0 {
Expand All @@ -116,7 +115,7 @@ impl Container {

let req_len = 1 + set_length(&mut buf, data_len) + data_len;

if req_len > max_size {
if req_len > CB_OBJ_MAX {
return Err(Error::SizeError);
}

Expand Down
3 changes: 1 addition & 2 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,7 @@ pub fn generate(

match algorithm {
AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => {
if yubikey.device_model() == DEVTYPE_YK4
&& yubikey.version.major == 4
if yubikey.version.major == 4
&& (yubikey.version.minor < 3
|| yubikey.version.minor == 3 && (yubikey.version.patch < 5))
{
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
//!
//! ## Supported YubiKeys
//!
//! - [YubiKey NEO] series
//! - [YubiKey 4] series
//! - [YubiKey 5] series
//!
//! NOTE: Nano and USB-C variants of the above are also supported
//! NOTE: Nano and USB-C variants of the above are also supported.
//! Pre-YK4 [YubiKey NEO] series is **NOT** supported.
//!
//! ## Supported Algorithms
//!
Expand Down
9 changes: 2 additions & 7 deletions src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,10 @@ pub(crate) fn read(txn: &Transaction<'_>, tag: u8) -> Result<Buffer, Error> {
}

/// Write metadata
pub(crate) fn write(
txn: &Transaction<'_>,
tag: u8,
data: &[u8],
max_size: usize,
) -> Result<(), Error> {
pub(crate) fn write(txn: &Transaction<'_>, tag: u8, data: &[u8]) -> Result<(), Error> {
let mut buf = Zeroizing::new(vec![0u8; CB_OBJ_MAX]);

if data.len() > max_size - CB_OBJ_TAG_MAX {
if data.len() > CB_OBJ_MAX - CB_OBJ_TAG_MAX {
return Err(Error::GenericError);
}

Expand Down
5 changes: 2 additions & 3 deletions src/mgm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ impl MgmKey {
pub fn set_protected(&self, yubikey: &mut YubiKey) -> Result<(), Error> {
let mut data = Zeroizing::new(vec![0u8; CB_BUF_MAX]);

let max_size = yubikey.obj_size_max();
let txn = yubikey.begin_transaction()?;

txn.set_mgm_key(self, None).map_err(|e| {
Expand Down Expand Up @@ -200,7 +199,7 @@ impl MgmKey {
) {
error!("could not set protected mgm item, err = {:?}", e);
} else {
metadata::write(&txn, TAG_PROTECTED, &data, max_size).map_err(|e| {
metadata::write(&txn, TAG_PROTECTED, &data).map_err(|e| {
error!("could not write protected data, err = {:?}", e);
e
})?;
Expand Down Expand Up @@ -247,7 +246,7 @@ impl MgmKey {
&flags_1,
) {
error!("could not set admin flags item, err = {}", e);
} else if let Err(e) = metadata::write(&txn, TAG_ADMIN, &data[..cb_data], max_size) {
} else if let Err(e) = metadata::write(&txn, TAG_ADMIN, &data[..cb_data]) {
error!("could not write admin data, err = {}", e);
}

Expand Down
10 changes: 4 additions & 6 deletions src/msroots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,10 @@ impl MsRoots {

/// Read `msroots` file from YubiKey
pub fn read(yubikey: &mut YubiKey) -> Result<Option<Self>, Error> {
let cb_data = yubikey.obj_size_max();
let txn = yubikey.begin_transaction()?;

// allocate first page
let mut data = Vec::with_capacity(cb_data);
let mut data = Vec::with_capacity(CB_OBJ_MAX);

for object_id in YKPIV_OBJ_MSROOTS1..YKPIV_OBJ_MSROOTS5 {
let buf = txn.fetch_object(object_id)?;
Expand Down Expand Up @@ -106,15 +105,14 @@ impl MsRoots {
let data = &self.0;
let data_len = data.len();
let n_objs: usize;
let cb_obj_max = yubikey.obj_size_max();
let txn = yubikey.begin_transaction()?;

if data_len == 0 {
return txn.save_object(YKPIV_OBJ_MSROOTS1, &[]);
}

// Calculate number of objects required to store blob
n_objs = (data_len / (cb_obj_max - CB_OBJ_TAG_MAX)) + 1;
n_objs = (data_len / (CB_OBJ_MAX - CB_OBJ_TAG_MAX)) + 1;

if n_objs > 5 {
return Err(Error::SizeError);
Expand All @@ -123,8 +121,8 @@ impl MsRoots {
for i in 0..n_objs {
offset = 0;

data_chunk = if cb_obj_max - CB_OBJ_TAG_MAX < data_len - data_offset {
cb_obj_max - CB_OBJ_TAG_MAX
data_chunk = if CB_OBJ_MAX - CB_OBJ_TAG_MAX < data_len - data_offset {
CB_OBJ_MAX - CB_OBJ_TAG_MAX
} else {
data_len - data_offset
};
Expand Down
21 changes: 5 additions & 16 deletions src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ impl<'tx> Transaction<'tx> {
})
}

/// Get an attribute of the card or card reader.
pub fn get_attribute<'buf>(
&self,
attribute: pcsc::Attribute,
buffer: &'buf mut [u8],
) -> Result<&'buf [u8], Error> {
Ok(self.inner.get_attribute(attribute, buffer)?)
}

/// Transmit a single serialized APDU to the card this transaction is open
/// with and receive a response.
///
Expand All @@ -66,7 +57,7 @@ impl<'tx> Transaction<'tx> {
pub fn select_application(&self) -> Result<(), Error> {
let response = APDU::new(Ins::SelectApplication)
.p1(0x04)
.data(&AID)
.data(&PIV_AID)
.transmit(self, 0xFF)
.map_err(|e| {
error!("failed communicating with card: '{}'", e);
Expand Down Expand Up @@ -102,13 +93,11 @@ impl<'tx> Transaction<'tx> {

/// Get YubiKey device serial number.
pub fn get_serial(&self, version: Version) -> Result<Serial, Error> {
let yk_applet = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x20, 0x01, 0x01];

let response = if version.major < 5 {
// get serial from neo/yk4 devices using the otp applet
// YK4 requires switching to the yk applet to retrieve the serial
let sw = APDU::new(Ins::SelectApplication)
.p1(0x04)
.data(&yk_applet)
.data(&YK_AID)
.transmit(self, 0xFF)?
.status_words();

Expand All @@ -130,7 +119,7 @@ impl<'tx> Transaction<'tx> {
// reselect the PIV applet
let sw = APDU::new(Ins::SelectApplication)
.p1(0x04)
.data(&AID)
.data(&PIV_AID)
.transmit(self, 0xFF)?
.status_words();

Expand All @@ -141,7 +130,7 @@ impl<'tx> Transaction<'tx> {

resp
} else {
// get serial from yk5 and later devices using the f8 command
// YK5 implements getting the serial as a PIV applet command (0xf8)
let resp = APDU::new(Ins::GetSerial).transmit(self, 0xFF)?;

if !resp.is_success() {
Expand Down
Loading

0 comments on commit f6915ce

Please sign in to comment.