From 3a4515d902bdc0de1f72dfa5f4f0d6ccd1f1877f Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 21:26:17 +0000 Subject: [PATCH 1/5] Convert PIN and touch policies into enums --- src/consts.rs | 12 ------- src/key.rs | 32 ++++++++--------- src/lib.rs | 2 ++ src/policy.rs | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/yubikey.rs | 36 +++---------------- 5 files changed, 120 insertions(+), 60 deletions(-) create mode 100644 src/policy.rs diff --git a/src/consts.rs b/src/consts.rs index 2988342..65242c0 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -158,15 +158,3 @@ pub const YKPIV_OBJ_MSROOTS2: u32 = 0x005f_ff12; pub const YKPIV_OBJ_MSROOTS3: u32 = 0x005f_ff13; pub const YKPIV_OBJ_MSROOTS4: u32 = 0x005f_ff14; pub const YKPIV_OBJ_MSROOTS5: u32 = 0x005f_ff15; - -pub const YKPIV_PINPOLICY_TAG: u8 = 0xaa; -pub const YKPIV_PINPOLICY_DEFAULT: u8 = 0; -pub const YKPIV_PINPOLICY_NEVER: u8 = 1; -pub const YKPIV_PINPOLICY_ONCE: u8 = 2; -pub const YKPIV_PINPOLICY_ALWAYS: u8 = 3; - -pub const YKPIV_TOUCHPOLICY_TAG: u8 = 0xab; -pub const YKPIV_TOUCHPOLICY_DEFAULT: u8 = 0; -pub const YKPIV_TOUCHPOLICY_NEVER: u8 = 1; -pub const YKPIV_TOUCHPOLICY_ALWAYS: u8 = 2; -pub const YKPIV_TOUCHPOLICY_CACHED: u8 = 3; diff --git a/src/key.rs b/src/key.rs index b54fa53..f14aa81 100644 --- a/src/key.rs +++ b/src/key.rs @@ -42,6 +42,7 @@ use crate::{ certificate::{self, Certificate}, consts::*, error::Error, + policy::{PinPolicy, TouchPolicy}, serialization::*, settings, yubikey::YubiKey, @@ -408,8 +409,8 @@ pub fn generate( yubikey: &mut YubiKey, slot: SlotId, algorithm: AlgorithmId, - pin_policy: u8, - touch_policy: u8, + pin_policy: PinPolicy, + touch_policy: TouchPolicy, ) -> Result { let mut in_data = [0u8; 11]; let mut templ = [0, Ins::GenerateAsymmetric.code(), 0, 0]; @@ -476,16 +477,13 @@ pub fn generate( return Err(Error::AlgorithmError); } - if pin_policy != YKPIV_PINPOLICY_DEFAULT { - in_data[1] += 3; - in_data[offset..(offset + 3)].copy_from_slice(&[YKPIV_PINPOLICY_TAG, 1, pin_policy]); - offset += 3; - } + let pin_len = pin_policy.write(&mut in_data[offset..]); + in_data[1] += pin_len as u8; + offset += pin_len; - if touch_policy != YKPIV_TOUCHPOLICY_DEFAULT { - in_data[1] += 3; - in_data[offset..(offset + 3)].copy_from_slice(&[YKPIV_TOUCHPOLICY_TAG, 1, touch_policy]); - } + let touch_len = touch_policy.write(&mut in_data[offset..]); + in_data[1] += touch_len as u8; + offset += touch_len; let response = txn.transfer_data(&templ, &in_data[..offset], 1024)?; @@ -498,12 +496,12 @@ pub fn generate( return Err(Error::KeyError); } StatusWords::IncorrectParamError => { - if pin_policy != YKPIV_PINPOLICY_DEFAULT { - error!("{} (pin policy not supported?)", err_msg); - } else if touch_policy != YKPIV_TOUCHPOLICY_DEFAULT { - error!("{} (touch policy not supported?)", err_msg); - } else { - error!("{} (algorithm not supported?)", err_msg); + match pin_policy { + PinPolicy::Default => match touch_policy { + TouchPolicy::Default => error!("{} (algorithm not supported?)", err_msg), + _ => error!("{} (touch policy not supported?)", err_msg), + }, + _ => error!("{} (pin policy not supported?)", err_msg), } return Err(Error::AlgorithmError); diff --git a/src/lib.rs b/src/lib.rs index 580ca93..04c022b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -156,6 +156,8 @@ mod metadata; pub mod mgm; #[cfg(feature = "untested")] pub mod msroots; +#[cfg(feature = "untested")] +pub mod policy; pub mod readers; #[cfg(feature = "untested")] mod serialization; diff --git a/src/policy.rs b/src/policy.rs new file mode 100644 index 0000000..a9650ab --- /dev/null +++ b/src/policy.rs @@ -0,0 +1,98 @@ +//! Enums representing key policies. + +/// Specifies how often the PIN needs to be entered for access to the credential in a +/// given slot. This policy must be set upon key generation or importation, and cannot be +/// changed later. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum PinPolicy { + /// Use the default PIN policy for the slot. See the slot's documentation for details. + Default, + + /// The end user PIN is **NOT** required to perform private key operations. + Never, + + /// The end user PIN is required to perform any private key operations. Once the + /// correct PIN has been provided, multiple private key operations may be performed + /// without additional cardholder consent. + Once, + + /// The end user PIN is required to perform any private key operations. The PIN must + /// be submitted immediately before each operation to ensure cardholder participation. + Always, +} + +impl From for u8 { + fn from(policy: PinPolicy) -> u8 { + match policy { + PinPolicy::Default => 0, + PinPolicy::Never => 1, + PinPolicy::Once => 2, + PinPolicy::Always => 3, + } + } +} + +impl PinPolicy { + /// Writes the `PinPolicy` in the format the YubiKey expects during key generation or + /// importation. + pub(crate) fn write(self, buf: &mut [u8]) -> usize { + match self { + PinPolicy::Default => 0, + _ => { + buf[0] = 0xaa; + buf[1] = 0x01; + buf[2] = self.into(); + 3 + } + } + } +} + +/// Specifies under what conditions a physical touch on the metal contact is required, in +/// addition to the [`PinPolicy`]. This policy must be set upon key generation or +/// importation, and cannot be changed later. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum TouchPolicy { + /// Use the default touch policy for the slot. + Default, + + /// A physical touch is **NOT** required to perform private key operations. + Never, + + /// A physical touch is required to perform any private key operations. The metal + /// contact must be touched during each operation to ensure cardholder participation. + Always, + + /// A physical touch is required to perform any private key operations. Each touch + /// is cached for 15 seconds, during which time multiple private key operations may be + /// performed without additional cardholder interaction. After 15 seconds the cached + /// touch is cleared, and further operations require another physical touch. + Cached, +} + +impl From for u8 { + fn from(policy: TouchPolicy) -> u8 { + match policy { + TouchPolicy::Default => 0, + TouchPolicy::Never => 1, + TouchPolicy::Always => 2, + TouchPolicy::Cached => 3, + } + } +} + +impl TouchPolicy { + /// Writes the `TouchPolicy` in the format the YubiKey expects during key generation + /// or importation. + pub(crate) fn write(self, buf: &mut [u8]) -> usize { + match self { + TouchPolicy::Default => 0, + _ => { + buf[0] = 0xab; + buf[1] = 0x01; + buf[2] = self.into(); + 3 + } + } + } +} diff --git a/src/yubikey.rs b/src/yubikey.rs index 7bb8db4..47c9986 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -39,6 +39,7 @@ use crate::{ key::{AlgorithmId, SlotId}, metadata, mgm::MgmKey, + policy::{PinPolicy, TouchPolicy}, serialization::*, Buffer, ObjectId, }; @@ -552,28 +553,12 @@ impl YubiKey { dq: Option<&[u8]>, qinv: Option<&[u8]>, ec_data: Option<&[u8]>, - pin_policy: u8, - touch_policy: u8, + pin_policy: PinPolicy, + touch_policy: TouchPolicy, ) -> Result<(), Error> { let mut key_data = Zeroizing::new(vec![0u8; 1024]); let templ = [0, Ins::ImportKey.code(), algorithm.into(), key.into()]; - if pin_policy != YKPIV_PINPOLICY_DEFAULT - && pin_policy != YKPIV_PINPOLICY_NEVER - && pin_policy != YKPIV_PINPOLICY_ONCE - && pin_policy != YKPIV_PINPOLICY_ALWAYS - { - return Err(Error::GenericError); - } - - if touch_policy != YKPIV_TOUCHPOLICY_DEFAULT - && touch_policy != YKPIV_TOUCHPOLICY_NEVER - && touch_policy != YKPIV_TOUCHPOLICY_ALWAYS - && touch_policy != YKPIV_TOUCHPOLICY_CACHED - { - return Err(Error::GenericError); - } - let (elem_len, params, param_tag) = match algorithm { AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => match (p, q, dp, dq, qinv) { (Some(p), Some(q), Some(dp), Some(dq), Some(qinv)) => { @@ -637,19 +622,8 @@ impl YubiKey { offset += param.len(); } - if pin_policy != YKPIV_PINPOLICY_DEFAULT { - key_data[offset] = YKPIV_PINPOLICY_TAG; - key_data[offset + 1] = 0x01; - key_data[offset + 2] = pin_policy; - offset += 3; - } - - if touch_policy != YKPIV_TOUCHPOLICY_DEFAULT { - key_data[offset] = YKPIV_TOUCHPOLICY_TAG; - key_data[offset + 1] = 0x01; - key_data[offset + 2] = touch_policy; - offset += 3; - } + offset += pin_policy.write(&mut key_data[offset..]); + offset += touch_policy.write(&mut key_data[offset..]); let txn = self.begin_transaction()?; From 7bcd8664a4eb085c91f701041751977e4565ef8f Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 21:37:06 +0000 Subject: [PATCH 2/5] AlgorithmId::write helper to match policy helpers --- src/consts.rs | 1 - src/key.rs | 27 ++++++++++++++------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index 65242c0..95576e6 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -122,7 +122,6 @@ pub const TAG_RSA_MODULUS: u8 = 0x81; pub const TAG_RSA_EXP: u8 = 0x82; pub const TAG_ECC_POINT: u8 = 0x86; -pub const YKPIV_ALGO_TAG: u8 = 0x80; pub const YKPIV_ALGO_3DES: u8 = 0x03; pub const YKPIV_ATR_NEO_R3: &[u8] = b";\xFC\x13\0\0\x811\xFE\x15YubikeyNEOr3\xE1\0"; diff --git a/src/key.rs b/src/key.rs index f14aa81..50152ea 100644 --- a/src/key.rs +++ b/src/key.rs @@ -312,6 +312,16 @@ impl From for u8 { } } +impl AlgorithmId { + /// Writes the `AlgorithmId` in the format the YubiKey expects during key generation. + pub(crate) fn write(self, buf: &mut [u8]) -> usize { + buf[0] = 0x80; + buf[1] = 0x01; + buf[2] = self.into(); + 3 + } +} + /// PIV cryptographic keys stored in a YubiKey #[derive(Clone, Debug)] pub struct Key { @@ -412,7 +422,6 @@ pub fn generate( pin_policy: PinPolicy, touch_policy: TouchPolicy, ) -> Result { - let mut in_data = [0u8; 11]; let mut templ = [0, Ins::GenerateAsymmetric.code(), 0, 0]; let setting_roca: settings::BoolValue; @@ -463,19 +472,11 @@ pub fn generate( templ[3] = slot.into(); + let mut in_data = [0u8; 11]; + in_data[0] = 0xac; + in_data[1] = 3; // length sans this 2-byte header + assert_eq!(algorithm.write(&mut in_data[2..]), 3); let mut offset = 5; - in_data[..offset].copy_from_slice(&[ - 0xac, - 3, // length sans this 2-byte header - YKPIV_ALGO_TAG, - 1, - algorithm.into(), - ]); - - if in_data[4] == 0 { - error!("unexpected algorithm"); - return Err(Error::AlgorithmError); - } let pin_len = pin_policy.write(&mut in_data[offset..]); in_data[1] += pin_len as u8; From 370a90f8009331f7968fe2fde4a1769a843b0788 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 22:02:39 +0000 Subject: [PATCH 3/5] Correctly return StatusWords from transfer_data --- src/transaction.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/transaction.rs b/src/transaction.rs index 2514024..3c534b0 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -375,7 +375,7 @@ impl<'tx> Transaction<'tx> { ) -> Result { let mut in_offset = 0; let mut out_data = vec![]; - let mut sw = 0; + let mut sw; loop { let mut this_size = 0xff; @@ -395,13 +395,13 @@ impl<'tx> Transaction<'tx> { .data(&in_data[in_offset..(in_offset + this_size)]) .transmit(self, 261)?; - if !response.is_success() && (response.status_words().code() >> 8 != 0x61) { + sw = response.status_words().code(); + + if !response.is_success() && (sw >> 8 != 0x61) { // TODO(tarcieri): is this really OK? return Ok(Response::new(sw.into(), out_data)); } - sw = response.status_words().code(); - if !out_data.is_empty() && (out_data.len() - response.data().len() > max_out) { error!( "output buffer too small: wanted to write {}, max was {}", From ada3454d2603c7691bddf4ebf67d420aafa6c323 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 2 Dec 2019 02:23:16 +0000 Subject: [PATCH 4/5] Fix bug in MgmKey::decrypt --- src/mgm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mgm.rs b/src/mgm.rs index bfcca67..c59932c 100644 --- a/src/mgm.rs +++ b/src/mgm.rs @@ -268,7 +268,7 @@ impl MgmKey { pub(crate) fn decrypt(&self, input: &[u8; DES_LEN_DES]) -> [u8; DES_LEN_DES] { let mut output = input.to_owned(); TdesEde3::new(GenericArray::from_slice(&self.0)) - .encrypt_block(GenericArray::from_mut_slice(&mut output)); + .decrypt_block(GenericArray::from_mut_slice(&mut output)); output } } From 76c093e68e38b120d10dc65e6b9033514e669aa8 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 2 Dec 2019 02:31:33 +0000 Subject: [PATCH 5/5] Minor cleanups --- src/key.rs | 5 ++--- src/yubikey.rs | 6 ++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/key.rs b/src/key.rs index 50152ea..11ac520 100644 --- a/src/key.rs +++ b/src/key.rs @@ -422,7 +422,6 @@ pub fn generate( pin_policy: PinPolicy, touch_policy: TouchPolicy, ) -> Result { - let mut templ = [0, Ins::GenerateAsymmetric.code(), 0, 0]; let setting_roca: settings::BoolValue; match algorithm { @@ -470,7 +469,7 @@ pub fn generate( let txn = yubikey.begin_transaction()?; - templ[3] = slot.into(); + let templ = [0, Ins::GenerateAsymmetric.code(), 0, slot.into()]; let mut in_data = [0u8; 11]; in_data[0] = 0xac; @@ -512,7 +511,7 @@ pub fn generate( return Err(Error::AuthenticationError); } other => { - error!("{} (error {:x})", err_msg, other.code()); + error!("{} (error {:?})", err_msg, other); return Err(Error::GenericError); } } diff --git a/src/yubikey.rs b/src/yubikey.rs index 47c9986..725ccd6 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -227,10 +227,12 @@ impl YubiKey { pub fn authenticate(&mut self, mgm_key: MgmKey) -> Result<(), Error> { let txn = self.begin_transaction()?; + const TAG_DYN_AUTH: u8 = 0x7c; + // get a challenge from the card let challenge = APDU::new(Ins::Authenticate) .params(YKPIV_ALGO_3DES, YKPIV_KEY_CARDMGM) - .data(&[0x7c, 0x02, 0x80, 0x00]) + .data(&[TAG_DYN_AUTH, 0x02, 0x80, 0x00]) .transmit(&txn, 261)?; if !challenge.is_success() || challenge.data().len() < 12 { @@ -241,7 +243,7 @@ impl YubiKey { let response = mgm_key.decrypt(challenge.data()[4..12].try_into().unwrap()); let mut data = [0u8; 22]; - data[0] = 0x7c; + data[0] = TAG_DYN_AUTH; data[1] = 20; // 2 + 8 + 2 +8 data[2] = 0x80; data[3] = 8;