From 48d0a2ab04129fddd3cab87d71b29d608b299878 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 28 Nov 2019 17:09:37 +0000 Subject: [PATCH 01/24] Use slice::copy_from_slice in Transaction::change_pin --- src/transaction.rs | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/src/transaction.rs b/src/transaction.rs index a55e1c4..ebe5a01 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -16,8 +16,6 @@ use crate::{ use log::{error, trace}; use std::convert::TryInto; #[cfg(feature = "untested")] -use std::ptr; -#[cfg(feature = "untested")] use zeroize::Zeroizing; /// Exclusive transaction with the YubiKey's PC/SC card. @@ -187,7 +185,6 @@ impl<'tx> Transaction<'tx> { #[cfg(feature = "untested")] pub fn change_pin(&self, action: i32, current_pin: &[u8], new_pin: &[u8]) -> Result<(), Error> { let mut templ = [0, Ins::ChangeReference.code(), 0, 0x80]; - let mut indata = Zeroizing::new([0u8; 16]); if current_pin.len() > CB_PIN_MAX || new_pin.len() > CB_PIN_MAX { return Err(Error::SizeError); @@ -199,31 +196,9 @@ impl<'tx> Transaction<'tx> { templ[3] = 0x81; } - unsafe { - ptr::copy(current_pin.as_ptr(), indata.as_mut_ptr(), current_pin.len()); - - if current_pin.len() < CB_PIN_MAX { - ptr::write_bytes( - indata.as_mut_ptr().add(current_pin.len()), - 0xff, - CB_PIN_MAX - current_pin.len(), - ); - } - - ptr::copy( - new_pin.as_ptr(), - indata.as_mut_ptr().offset(8), - new_pin.len(), - ); - - if new_pin.len() < CB_PIN_MAX { - ptr::write_bytes( - indata.as_mut_ptr().offset(8).add(new_pin.len()), - 0xff, - CB_PIN_MAX - new_pin.len(), - ); - } - } + let mut indata = Zeroizing::new([0xff; CB_PIN_MAX * 2]); + indata[0..current_pin.len()].copy_from_slice(current_pin); + indata[CB_PIN_MAX..CB_PIN_MAX + new_pin.len()].copy_from_slice(new_pin); let status_words = self .transfer_data(&templ, indata.as_ref(), 0xFF)? From afb6a9479e904eae086008b75913c8114c901e68 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 28 Nov 2019 17:14:36 +0000 Subject: [PATCH 02/24] Use slice::copy_within in read_certificate --- src/certificate.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/certificate.rs b/src/certificate.rs index a4e0ab8..71a3db4 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -40,7 +40,6 @@ use crate::{ Buffer, }; use log::error; -use std::ptr; use zeroize::Zeroizing; /// Certificates @@ -124,10 +123,7 @@ pub(crate) fn read_certificate(txn: &Transaction<'_>, slot: SlotId) -> Result Date: Thu, 28 Nov 2019 17:49:17 +0000 Subject: [PATCH 03/24] Rewrite metadata::set_item without unsafe Also re-introduces some comments that were lost during corrosion. --- src/metadata.rs | 108 +++++++++++++++++++----------------------------- 1 file changed, 43 insertions(+), 65 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 7249ad3..5c8def9 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -83,35 +83,25 @@ pub(crate) fn set_item( tag: u8, p_item: &[u8], ) -> Result<(), Error> { - let mut p_temp: *mut u8 = data.as_mut_ptr(); let mut cb_temp: usize = 0; let mut tag_temp: u8 = 0; let mut cb_len: usize = 0; let cb_item = p_item.len(); - // Must be signed to have negative offsets - let cb_moved: isize; - let p_next: *mut u8; - while p_temp < data[*pcb_data..].as_mut_ptr() { - unsafe { - tag_temp = *p_temp; - p_temp = p_temp.add(1); + let mut offset = 0; - cb_len = get_length( - slice::from_raw_parts( - p_temp, - data.as_mut_ptr() as usize + data.len() - p_temp as usize, - ), - &mut cb_temp, - ); - p_temp = p_temp.add(cb_len); + while offset < *pcb_data { + tag_temp = data[offset]; + offset += 1; - if tag_temp == tag { - break; - } + cb_len = get_length(&data[offset..], &mut cb_temp); + offset += cb_len; - p_temp = p_temp.add(cb_temp); + if tag_temp == tag { + break; } + + offset += cb_temp; } if tag_temp != tag { @@ -120,75 +110,63 @@ pub(crate) fn set_item( return Ok(()); } - unsafe { - p_temp = data.as_mut_ptr().add(*pcb_data); - cb_len = get_length_size(cb_item); + // We did not find an existing tag, append + offset = *pcb_data; + cb_len = get_length_size(cb_item); - if (*pcb_data + cb_len + cb_item) > cb_data_max { - return Err(Error::GenericError); - } - - *p_temp = tag; - p_temp = p_temp.add(1); - p_temp = p_temp.add(set_length( - slice::from_raw_parts_mut( - p_temp, - data.as_ptr() as usize + data.len() - p_temp as usize, - ), - cb_item, - )); - - ptr::copy(p_item.as_ptr(), p_temp, cb_item); + // If length would cause buffer overflow, return error + if (*pcb_data + cb_len + cb_item) > cb_data_max { + return Err(Error::GenericError); } + data[offset] = tag; + offset += 1; + offset += set_length(&mut data[offset..], cb_item); + data[offset..offset + cb_item].copy_from_slice(p_item); + *pcb_data += 1 + cb_len + cb_item; return Ok(()); } - if cb_temp == cb_item { - unsafe { - ptr::copy(p_item.as_ptr(), p_temp, cb_item); - } + // Found tag + // Check length, if it matches, overwrite + if cb_temp == cb_item { + data[offset..offset + cb_item].copy_from_slice(p_item); return Ok(()); } - p_next = unsafe { p_temp.add(cb_temp) }; - cb_moved = (cb_item as isize - cb_temp as isize) + // Length doesn't match, expand/shrink to fit + let next_offset = offset + cb_temp; + // Must be signed to have negative offsets + let cb_moved: isize = (cb_item as isize - cb_temp as isize) + if cb_item != 0 { get_length_size(cb_item) as isize } else { + // For tag, if deleting -1 } + // Accounts for different length encoding - cb_len as isize; - if (*pcb_data + cb_moved as usize) > cb_data_max { + // If length would cause buffer overflow, return error + if (*pcb_data as isize + cb_moved) as usize > cb_data_max { return Err(Error::GenericError); } - unsafe { - ptr::copy( - p_next, - p_next.offset(cb_moved), - *pcb_data - p_next as usize - data.as_ptr() as usize, - ); - } - - *pcb_data += cb_moved as usize; + // Move remaining data + data.copy_within( + next_offset..*pcb_data, + (next_offset as isize + cb_moved) as usize, + ); + *pcb_data = (*pcb_data as isize + cb_moved) as usize; + // Re-encode item and insert if cb_item != 0 { - unsafe { - p_temp = p_temp.offset(-(cb_len as isize)); - p_temp = p_temp.add(set_length( - slice::from_raw_parts_mut( - p_temp, - data.as_ptr() as usize + data.len() - p_temp as usize, - ), - cb_item, - )); - ptr::copy(p_item.as_ptr(), p_temp, cb_item); - } + offset -= cb_len; + offset += set_length(&mut data[offset..], cb_item); + data[offset..offset + cb_item].copy_from_slice(p_item); } Ok(()) From 8b86a0f57887dbbdc314a608575493a07a159980 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 28 Nov 2019 17:55:51 +0000 Subject: [PATCH 04/24] Rewrite metadata::get_item without unsafe --- src/metadata.rs | 49 +++++++++++++++++++------------------------------ 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 5c8def9..8204346 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -31,48 +31,37 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::{consts::*, error::Error, serialization::*, transaction::Transaction, Buffer}; -use std::{ptr, slice}; +use std::ptr; use zeroize::Zeroizing; /// Get metadata item pub(crate) fn get_item(data: &[u8], tag: u8) -> Result<&[u8], Error> { - let mut p_temp: *const u8 = data.as_ptr(); let mut cb_temp: usize = 0; - let mut tag_temp: u8; + let mut offset = 0; - unsafe { - while p_temp < data.as_ptr().add(data.len()) { - tag_temp = *p_temp; - p_temp = p_temp.add(1); + while offset < data.len() { + let tag_temp = data[offset]; + offset += 1; - let p_slice = slice::from_raw_parts( - p_temp, - data.as_ptr() as usize + data.len() - p_temp as usize, - ); - - if !has_valid_length( - p_slice, - data.as_ptr().add(data.len()) as usize - p_temp as usize, - ) { - return Err(Error::SizeError); - } - - p_temp = p_temp.add(get_length(p_slice, &mut cb_temp)); - - if tag_temp == tag { - // found tag - break; - } - - p_temp = p_temp.add(cb_temp); + if !has_valid_length(&data[offset..], data.len() - 1) { + return Err(Error::SizeError); } - if p_temp < data.as_ptr().add(data.len()) { - return Ok(slice::from_raw_parts(p_temp, cb_temp)); + offset += get_length(&data[offset..], &mut cb_temp); + + if tag_temp == tag { + // found tag + break; } + + offset += cb_temp; } - Err(Error::GenericError) + if offset < data.len() { + Ok(&data[offset..offset + cb_temp]) + } else { + Err(Error::GenericError) + } } /// Set metadata item From 7c08674facd15592233545c13c4594a1a4728c7a Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 28 Nov 2019 17:58:19 +0000 Subject: [PATCH 05/24] Use slice::copy_within in metadata::read --- src/metadata.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 8204346..5cadbc5 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -31,7 +31,6 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::{consts::*, error::Error, serialization::*, transaction::Transaction, Buffer}; -use std::ptr; use zeroize::Zeroizing; /// Get metadata item @@ -186,10 +185,7 @@ pub(crate) fn read(txn: &Transaction<'_>, tag: u8) -> Result { return Err(Error::GenericError); } - unsafe { - ptr::copy(data.as_ptr().add(offset), data.as_mut_ptr(), pcb_data); - } - + data.copy_within(offset..offset + pcb_data, 0); data.truncate(pcb_data); Ok(data) } From 1935216cf33b3e99653f9e450baa8784ee8d41cb Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 28 Nov 2019 18:13:19 +0000 Subject: [PATCH 06/24] Rewrite MsRoots::read without unsafe --- src/msroots.rs | 52 +++++++++++++++----------------------------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/src/msroots.rs b/src/msroots.rs index f05ed70..c6911d0 100644 --- a/src/msroots.rs +++ b/src/msroots.rs @@ -39,7 +39,6 @@ use crate::{consts::*, error::Error, serialization::*, yubikey::YubiKey}; use log::error; -use std::{ptr, slice}; /// `msroots` file: PKCS#7-formatted certificate store for enterprise trust roots pub struct MsRoots(Vec); @@ -51,33 +50,22 @@ impl MsRoots { } /// Read `msroots` file from YubiKey - pub fn read(yubikey: &mut YubiKey) -> Result, Error> { - let mut len: usize = 0; - let mut ptr: *mut u8; - let mut tag: u8; - let mut offset: usize = 0; - - let mut results = vec![]; + pub fn read(yubikey: &mut YubiKey) -> Result, Error> { let cb_data = yubikey.obj_size_max(); let txn = yubikey.begin_transaction()?; // allocate first page - let mut p_data = vec![0u8; cb_data]; + let mut data = Vec::with_capacity(cb_data); for object_id in YKPIV_OBJ_MSROOTS1..YKPIV_OBJ_MSROOTS5 { - let mut buf = txn.fetch_object(object_id)?; + let buf = txn.fetch_object(object_id)?; let cb_buf = buf.len(); - ptr = buf.as_mut_ptr(); - if cb_buf < CB_OBJ_TAG_MIN { - return Ok(results); + return Ok(None); } - unsafe { - tag = *ptr; - ptr = ptr.add(1); - } + let tag = buf[0]; if (TAG_MSROOTS_MID != tag || YKPIV_OBJ_MSROOTS5 == object_id) && (TAG_MSROOTS_END != tag) @@ -85,38 +73,28 @@ impl MsRoots { // the current object doesn't contain a valid part of a msroots file // treat condition as object isn't found - return Ok(results); + return Ok(None); } - unsafe { - ptr = ptr.add(get_length( - slice::from_raw_parts(ptr, buf.as_ptr() as usize + buf.len() - ptr as usize), - &mut len, - )); - } + let mut len: usize = 0; + let offset = 1 + get_length(&buf[1..], &mut len); // check that decoded length represents object contents - if len > cb_buf - (ptr as isize - buf.as_mut_ptr() as isize) as usize { - return Ok(results); + if len > cb_buf - offset { + return Ok(None); } - unsafe { - ptr::copy(ptr, p_data.as_mut_ptr().add(offset), len); - } - - offset += len; - - match MsRoots::new(&p_data[..offset]) { - Ok(msroots) => results.push(msroots), - Err(res) => error!("error parsing msroots: {:?}", res), - } + data.extend_from_slice(&buf[offset..offset + len]); if tag == TAG_MSROOTS_END { break; } } - Ok(results) + MsRoots::new(&data).map(Some).map_err(|e| { + error!("error parsing msroots: {:?}", e); + e + }) } /// Write `msroots` file to YubiKey From 8240575bb48c55a6a0fd2d9dd7d7b12af451c44c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 28 Nov 2019 23:44:16 +0000 Subject: [PATCH 07/24] Rewrite YubiKey::import_private_key without unsafe --- src/yubikey.rs | 189 ++++++++++++++++--------------------------------- 1 file changed, 62 insertions(+), 127 deletions(-) diff --git a/src/yubikey.rs b/src/yubikey.rs index c6c4cae..ec38cfb 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -51,7 +51,6 @@ use std::fmt::{self, Display}; #[cfg(feature = "untested")] use std::{ convert::TryInto, - ptr, slice, time::{SystemTime, UNIX_EPOCH}, }; #[cfg(feature = "untested")] @@ -618,177 +617,113 @@ impl YubiKey { pin_policy: u8, touch_policy: u8, ) -> Result<(), Error> { - // TODO(tarcieri): get rid of legacy pointers - let (p, p_len) = match p { - Some(slice) => (slice.as_ptr(), slice.len()), - None => (ptr::null(), 0), - }; - - let (q, q_len) = match q { - Some(slice) => (slice.as_ptr(), slice.len()), - None => (ptr::null(), 0), - }; - - let (dp, dp_len) = match dp { - Some(slice) => (slice.as_ptr(), slice.len()), - None => (ptr::null(), 0), - }; - - let (dq, dq_len) = match dq { - Some(slice) => (slice.as_ptr(), slice.len()), - None => (ptr::null(), 0), - }; - - let (qinv, qinv_len) = match qinv { - Some(slice) => (slice.as_ptr(), slice.len()), - None => (ptr::null(), 0), - }; - - let (ec_data, ec_data_len) = match ec_data { - Some(slice) => (slice.as_ptr(), slice.len()), - None => (ptr::null(), 0), - }; - let mut key_data = Zeroizing::new(vec![0u8; 1024]); - let mut in_ptr: *mut u8 = key_data.as_mut_ptr(); let templ = [0, Ins::ImportKey.code(), algorithm, key]; - let mut elem_len: u32 = 0; - let mut params: [*const u8; 5] = [ptr::null(); 5]; - let mut lens = [0usize; 5]; - let n_params: u8; - let param_tag: i32; if key == YKPIV_KEY_CARDMGM || key < YKPIV_KEY_RETIRED1 - || key > YKPIV_KEY_RETIRED20 && (key < YKPIV_KEY_AUTHENTICATION) - || key > YKPIV_KEY_CARDAUTH && (key != YKPIV_KEY_ATTESTATION) + || (key > YKPIV_KEY_RETIRED20 && key < YKPIV_KEY_AUTHENTICATION) + || (key > YKPIV_KEY_CARDAUTH && key != YKPIV_KEY_ATTESTATION) { return Err(Error::KeyError); } if pin_policy != YKPIV_PINPOLICY_DEFAULT - && (pin_policy != YKPIV_PINPOLICY_NEVER) - && (pin_policy != YKPIV_PINPOLICY_ONCE) - && (pin_policy != YKPIV_PINPOLICY_ALWAYS) + && 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) + && touch_policy != YKPIV_TOUCHPOLICY_NEVER + && touch_policy != YKPIV_TOUCHPOLICY_ALWAYS + && touch_policy != YKPIV_TOUCHPOLICY_CACHED { return Err(Error::GenericError); } - match algorithm { - YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => { - if p_len + q_len + dp_len + dq_len + qinv_len >= 1024 { - return Err(Error::SizeError); - } else { - if algorithm == YKPIV_ALGO_RSA1024 { - elem_len = 64; + let (elem_len, params, param_tag) = match algorithm { + YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => match (p, q, dp, dq, qinv) { + (Some(p), Some(q), Some(dp), Some(dq), Some(qinv)) => { + if p.len() + q.len() + dp.len() + dq.len() + qinv.len() >= key_data.len() { + return Err(Error::SizeError); } - if algorithm == YKPIV_ALGO_RSA2048 { - elem_len = 128; + ( + match algorithm { + YKPIV_ALGO_RSA1024 => 64, + YKPIV_ALGO_RSA2048 => 128, + }, + vec![p, q, dp, dq, qinv], + 0x01, + ) + } + _ => return Err(Error::GenericError), + }, + YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => match ec_data { + Some(ec_data) => { + if ec_data.len() >= key_data.len() { + // This can never be true, but check to be explicit. + return Err(Error::SizeError); } - if p.is_null() || q.is_null() || dp.is_null() || dq.is_null() || qinv.is_null() - { - return Err(Error::GenericError); - } - - params[0] = p; - lens[0] = p_len; - params[1] = q; - lens[1] = q_len; - params[2] = dp; - lens[2] = dp_len; - params[3] = dq; - lens[3] = dq_len; - params[4] = qinv; - lens[4] = qinv_len; - param_tag = 0x1; - n_params = 5u8; + ( + match algorithm { + YKPIV_ALGO_ECCP256 => 32, + YKPIV_ALGO_ECCP384 => 48, + }, + vec![ec_data], + 0x06, + ) } - } - YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => { - if ec_data_len >= key_data.len() { - return Err(Error::SizeError); - } - - if algorithm == YKPIV_ALGO_ECCP256 { - elem_len = 32; - } else if algorithm == YKPIV_ALGO_ECCP384 { - elem_len = 48; - } - - if ec_data.is_null() { - return Err(Error::GenericError); - } - - params[0] = ec_data; - lens[0] = ec_data_len; - param_tag = 0x6; - n_params = 1; - } + _ => return Err(Error::GenericError), + }, _ => return Err(Error::AlgorithmError), - } + }; - for i in 0..n_params { - unsafe { - *in_ptr = (param_tag + i as i32) as u8; - in_ptr = in_ptr.offset(1); + let mut offset = 0; - in_ptr = in_ptr.add(set_length( - slice::from_raw_parts_mut( - in_ptr, - key_data.as_mut_ptr() as usize - in_ptr as usize, - ), - elem_len as usize, - )); - } + for (i, param) in params.into_iter().enumerate() { + key_data[offset] = param_tag + i as u8; + offset += 1; - let padding = elem_len as usize - lens[i as usize]; - let remaining = (key_data.as_mut_ptr() as usize) + 1024 - in_ptr as usize; + offset += set_length(&mut key_data[offset..], elem_len); + + let padding = elem_len - param.len(); + let remaining = key_data.len() - offset; if padding > remaining { return Err(Error::AlgorithmError); } - unsafe { - ptr::write_bytes(in_ptr, 0, padding); - in_ptr = in_ptr.add(padding); - ptr::copy(params[i as usize], in_ptr, lens[i as usize]); - in_ptr = in_ptr.add(lens[i as usize]); + for b in &mut key_data[offset..offset + padding] { + *b = 0; } + offset += padding; + key_data[offset..offset + param.len()].copy_from_slice(param); + offset += param.len(); } if pin_policy != YKPIV_PINPOLICY_DEFAULT { - unsafe { - *in_ptr = YKPIV_PINPOLICY_TAG; - *in_ptr.add(1) = 0x01; - *in_ptr.add(2) = pin_policy; - in_ptr = in_ptr.add(3); - } + 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 { - unsafe { - *in_ptr = YKPIV_TOUCHPOLICY_TAG; - *in_ptr.add(1) = 0x01; - *in_ptr.add(2) = touch_policy; - in_ptr = in_ptr.add(3); - } + key_data[offset] = YKPIV_TOUCHPOLICY_TAG; + key_data[offset + 1] = 0x01; + key_data[offset + 2] = touch_policy; + offset += 3; } let txn = self.begin_transaction()?; - let len = in_ptr as usize - key_data.as_mut_ptr() as usize; let status_words = txn - .transfer_data(&templ, &key_data[..len], 256)? + .transfer_data(&templ, &key_data[..offset], 256)? .status_words(); match status_words { From 1db929c10fd7e87e89d07dbe329679c39e2f663c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 29 Nov 2019 00:09:08 +0000 Subject: [PATCH 08/24] Mark excluded nested match branches as unreachable --- src/yubikey.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/yubikey.rs b/src/yubikey.rs index ec38cfb..f1f5c0f 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -655,6 +655,7 @@ impl YubiKey { match algorithm { YKPIV_ALGO_RSA1024 => 64, YKPIV_ALGO_RSA2048 => 128, + _ => unreachable!(), }, vec![p, q, dp, dq, qinv], 0x01, @@ -673,6 +674,7 @@ impl YubiKey { match algorithm { YKPIV_ALGO_ECCP256 => 32, YKPIV_ALGO_ECCP384 => 48, + _ => unreachable!(), }, vec![ec_data], 0x06, From 7f3d821df267fe72aae4f552d94a25818f419177 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Fri, 29 Nov 2019 10:06:52 -0800 Subject: [PATCH 09/24] Add #![forbid(unsafe_code)]; fix up README.md badges and links - Forbids unsafe code - Adds a "Safety Dance" badge - Fixes the GitHub Actions status badge - Fixes up links that changed with the move to `iqlusioninc` org --- .github/workflows/rust.yml | 2 +- CHANGELOG.md | 22 +++++++++++----------- CODE_OF_CONDUCT.md | 18 ++++++++---------- Cargo.toml | 2 +- README.md | 33 ++++++++++++++++++--------------- src/lib.rs | 1 + 6 files changed, 40 insertions(+), 38 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b37a2bf..7e76767 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -3,7 +3,7 @@ on: pull_request: {} push: - branches: master + branches: develop name: Rust diff --git a/CHANGELOG.md b/CHANGELOG.md index 98e0ece..9ee790a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,17 +19,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Use `log` crate for logging ([#7]) - Replace `ErrorKind::Ok` with `Result` ([#6]) -[0.0.2]: https://github.com/tarcieri/yubikey-piv.rs/pull/31 -[#30]: https://github.com/tarcieri/yubikey-piv.rs/pull/30 -[#19]: https://github.com/tarcieri/yubikey-piv.rs/pull/19 -[#17]: https://github.com/tarcieri/yubikey-piv.rs/pull/17 -[#15]: https://github.com/tarcieri/yubikey-piv.rs/pull/15 -[#13]: https://github.com/tarcieri/yubikey-piv.rs/pull/13 -[#10]: https://github.com/tarcieri/yubikey-piv.rs/pull/10 -[#9]: https://github.com/tarcieri/yubikey-piv.rs/pull/9 -[#8]: https://github.com/tarcieri/yubikey-piv.rs/pull/8 -[#7]: https://github.com/tarcieri/yubikey-piv.rs/pull/7 -[#6]: https://github.com/tarcieri/yubikey-piv.rs/pull/6 +[0.0.2]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/31 +[#30]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/30 +[#19]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/19 +[#17]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/17 +[#15]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/15 +[#13]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/13 +[#10]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/10 +[#9]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/9 +[#8]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/8 +[#7]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/7 +[#6]: https://github.com/iqlusioninc/yubikey-piv.rs/pull/6 ## 0.0.1 (2019-11-18) - It typechecks, ship it! diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index 7a9a276..2633eef 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -6,8 +6,8 @@ In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, gender identity and expression, level of experience, -nationality, personal appearance, race, religion, or sexual identity and -orientation. +education, socio-economic status, nationality, personal appearance, race, +religion, or sexual identity and orientation. ## Our Standards @@ -23,7 +23,7 @@ include: Examples of unacceptable behavior by participants include: * The use of sexualized language or imagery and unwelcome sexual attention or -advances + advances * Trolling, insulting/derogatory comments, and personal or political attacks * Public or private harassment * Publishing others' private information, such as a physical or electronic @@ -55,8 +55,8 @@ further defined and clarified by project maintainers. ## Enforcement Instances of abusive, harassing, or otherwise unacceptable behavior may be -reported by contacting the project team at [bascule@gmail.com]. All -complaints will be reviewed and investigated and will result in a response that +reported by contacting the project team at [oss@iqlusion.io](mailto:oss@iqlusion.io). +All complaints will be reviewed and investigated and will result in a response that is deemed necessary and appropriate to the circumstances. The project team is obligated to maintain confidentiality with regard to the reporter of an incident. Further details of specific enforcement policies may be posted separately. @@ -65,12 +65,10 @@ Project maintainers who do not follow or enforce the Code of Conduct in good faith may face temporary or permanent repercussions as determined by other members of the project's leadership. -[bascule@gmail.com]: mailto:bascule@gmail.com - ## Attribution This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4, -available at [http://contributor-covenant.org/version/1/4][version] +available at https://www.contributor-covenant.org/version/1/4/code-of-conduct.html + +[homepage]: https://www.contributor-covenant.org -[homepage]: http://contributor-covenant.org -[version]: http://contributor-covenant.org/version/1/4/ diff --git a/Cargo.toml b/Cargo.toml index 277ada8..a94ceb0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ algorithms (e.g, PKCS#1v1.5, ECDSA) authors = ["Tony Arcieri ", "Yubico AB"] edition = "2018" license = "BSD-2-Clause" -repository = "https://github.com/tarcieri/yubikey-piv.rs" +repository = "https://github.com/iqlusioninc/yubikey-piv.rs" readme = "README.md" categories = ["api-bindings", "cryptography", "hardware-support"] keywords = ["ccid", "ecdsa", "rsa", "piv", "yubikey"] diff --git a/README.md b/README.md index 9fb818d..73e60f9 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ ![Apache2/MIT licensed][license-image] ![Rust Version][rustc-image] ![Maintenance Status: Experimental][maintenance-image] +[![Safety Dance][safety-image]][safety-link] [![Build Status][build-image]][build-link] [![Gitter Chat][gitter-image]][gitter-link] @@ -193,10 +194,12 @@ or conditions. [license-image]: https://img.shields.io/badge/license-BSD-blue.svg [rustc-image]: https://img.shields.io/badge/rustc-1.39+-blue.svg [maintenance-image]: https://img.shields.io/badge/maintenance-experimental-blue.svg -[build-image]: https://github.com/tarcieri/yubikey-piv.rs/workflows/Rust/badge.svg -[build-link]: https://github.com/tarcieri/yubikey-piv.rs/actions -[gitter-image]: https://badges.gitter.im/yubihsm-piv-rs.svg -[gitter-link]: https://gitter.im/yubikey-piv-rs/community +[safety-image]: https://img.shields.io/badge/unsafe-forbidden-success.svg +[safety-link]: https://github.com/rust-secure-code/safety-dance/ +[build-image]: https://github.com/iqlusioninc/yubikey-piv.rs/workflows/Rust/badge.svg?branch=develop&event=push +[build-link]: https://github.com/iqlusioninc/yubikey-piv.rs/actions +[gitter-image]: https://badges.gitter.im/badge.svg +[gitter-link]: https://gitter.im/iqlusioninc/community [//]: # (general links) @@ -209,18 +212,18 @@ or conditions. [yubico-piv-tool]: https://github.com/Yubico/yubico-piv-tool/ [Corrode]: https://github.com/jameysharp/corrode [cc-web]: https://contributor-covenant.org/ -[cc-md]: https://github.com/tarcieri/yubikey-piv.rs/blob/develop/CODE_OF_CONDUCT.md +[cc-md]: https://github.com/iqlusioninc/yubikey-piv.rs/blob/develop/CODE_OF_CONDUCT.md [BSDL]: https://opensource.org/licenses/BSD-2-Clause [//]: # (github issues) -[#18]: https://github.com/tarcieri/yubikey-piv.rs/issues/18 -[#20]: https://github.com/tarcieri/yubikey-piv.rs/issues/20 -[#21]: https://github.com/tarcieri/yubikey-piv.rs/issues/21 -[#22]: https://github.com/tarcieri/yubikey-piv.rs/issues/22 -[#23]: https://github.com/tarcieri/yubikey-piv.rs/issues/23 -[#24]: https://github.com/tarcieri/yubikey-piv.rs/issues/24 -[#25]: https://github.com/tarcieri/yubikey-piv.rs/issues/25 -[#26]: https://github.com/tarcieri/yubikey-piv.rs/issues/26 -[#27]: https://github.com/tarcieri/yubikey-piv.rs/issues/27 -[#28]: https://github.com/tarcieri/yubikey-piv.rs/issues/28 +[#18]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/18 +[#20]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/20 +[#21]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/21 +[#22]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/22 +[#23]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/23 +[#24]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/24 +[#25]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/25 +[#26]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/26 +[#27]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/27 +[#28]: https://github.com/iqlusioninc/yubikey-piv.rs/issues/28 diff --git a/src/lib.rs b/src/lib.rs index 6c595a4..b4d9851 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,6 +125,7 @@ html_logo_url = "https://raw.githubusercontent.com/tarcieri/yubikey-piv.rs/develop/img/logo.png", html_root_url = "https://docs.rs/yubikey-piv/0.0.2" )] +#![forbid(unsafe_code)] #![warn( missing_docs, rust_2018_idioms, From 9fe363661e4cb8d7341ca078ac6ffb4198858aee Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 14:28:02 +0000 Subject: [PATCH 10/24] verify_pin: Don't set APDU data for empty PIN --- src/transaction.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/transaction.rs b/src/transaction.rs index ebe5a01..df37b4f 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -163,15 +163,23 @@ impl<'tx> Transaction<'tx> { /// Verify device PIN. #[cfg(feature = "untested")] pub fn verify_pin(&self, pin: &[u8]) -> Result<(), Error> { - // TODO(tarcieri): allow unpadded (with `0xFF`) PIN shorter than CB_PIN_MAX? - if pin.len() != CB_PIN_MAX { + if pin.len() > CB_PIN_MAX { return Err(Error::SizeError); } - let response = APDU::new(Ins::Verify) - .params(0x00, 0x80) - .data(pin) - .transmit(self, 261)?; + let mut query = APDU::new(Ins::Verify); + query.params(0x00, 0x80); + + // Empty pin means we are querying the number of retries. We set no data in this + // case; if we instead sent [0xff; CB_PIN_MAX] it would count as an attempt and + // decrease the retry counter. + if !pin.is_empty() { + let mut data = Zeroizing::new([0xff; CB_PIN_MAX]); + data[0..pin.len()].copy_from_slice(pin); + query.data(data.as_ref()); + } + + let response = query.transmit(self, 261)?; match response.status_words() { StatusWords::Success => Ok(()), From 4b5cd8dd455d04f5d20994fe2923bccfae084b64 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 15:22:14 +0000 Subject: [PATCH 11/24] Make PIN verification failure a StatusWord case Retry count is now u8, as it cannot exceed 16 (being returned in the lower half of SW2). --- src/apdu.rs | 10 ++++++++++ src/error.rs | 2 +- src/transaction.rs | 4 ++-- src/yubikey.rs | 15 +++------------ 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/apdu.rs b/src/apdu.rs index b3e8e13..e407749 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -352,6 +352,12 @@ pub(crate) enum StatusWords { /// Successful execution Success, + /// PIN verification failure + VerifyFailError { + /// Remaining verification attempts + tries: u8, + }, + /// Security status not satisfied SecurityStatusError, @@ -379,6 +385,7 @@ impl StatusWords { pub fn code(self) -> u32 { match self { StatusWords::None => 0, + StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u32, StatusWords::SecurityStatusError => 0x6982, StatusWords::AuthBlockedError => 0x6983, StatusWords::IncorrectParamError => 0x6a80, @@ -399,6 +406,9 @@ impl From for StatusWords { fn from(sw: u32) -> Self { match sw { 0x0000 => StatusWords::None, + sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError { + tries: (sw & 0x000f) as u8, + }, 0x6982 => StatusWords::SecurityStatusError, 0x6983 => StatusWords::AuthBlockedError, 0x6a80 => StatusWords::IncorrectParamError, diff --git a/src/error.rs b/src/error.rs index c8da1a9..79b7a78 100644 --- a/src/error.rs +++ b/src/error.rs @@ -68,7 +68,7 @@ pub enum Error { /// Wrong PIN WrongPin { /// Number of tries remaining - tries: u32, + tries: u8, }, /// Invalid object diff --git a/src/transaction.rs b/src/transaction.rs index df37b4f..68fa89f 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -184,7 +184,7 @@ impl<'tx> Transaction<'tx> { match response.status_words() { StatusWords::Success => Ok(()), StatusWords::AuthBlockedError => Err(Error::WrongPin { tries: 0 }), - StatusWords::Other(sw) if sw >> 8 == 0x63 => Err(Error::WrongPin { tries: sw & 0xf }), + StatusWords::VerifyFailError { tries } => Err(Error::WrongPin { tries }), _ => Err(Error::GenericError), } } @@ -215,7 +215,7 @@ impl<'tx> Transaction<'tx> { match status_words { StatusWords::Success => Ok(()), StatusWords::AuthBlockedError => Err(Error::PinLocked), - StatusWords::Other(sw) if sw >> 8 == 0x63 => Err(Error::WrongPin { tries: sw & 0xf }), + StatusWords::VerifyFailError { tries } => Err(Error::WrongPin { tries }), _ => { error!( "failed changing pin, token response code: {:x}.", diff --git a/src/yubikey.rs b/src/yubikey.rs index f1f5c0f..cedfc80 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -396,7 +396,7 @@ impl YubiKey { /// Get the number of PIN retries #[cfg(feature = "untested")] - pub fn get_pin_retries(&mut self) -> Result { + pub fn get_pin_retries(&mut self) -> Result { let txn = self.begin_transaction()?; // Force a re-select to unverify, because once verified the spec dictates that @@ -414,24 +414,15 @@ impl YubiKey { /// Set the number of PIN retries #[cfg(feature = "untested")] - pub fn set_pin_retries(&mut self, pin_tries: usize, puk_tries: usize) -> Result<(), Error> { + pub fn set_pin_retries(&mut self, pin_tries: u8, puk_tries: u8) -> Result<(), Error> { // Special case: if either retry count is 0, it's a successful no-op if pin_tries == 0 || puk_tries == 0 { return Ok(()); } - if pin_tries > 0xff || puk_tries > 0xff || pin_tries < 1 || puk_tries < 1 { - return Err(Error::RangeError); - } - let txn = self.begin_transaction()?; - let templ = [ - 0, - Ins::SetPinRetries.code(), - pin_tries as u8, - puk_tries as u8, - ]; + let templ = [0, Ins::SetPinRetries.code(), pin_tries, puk_tries]; let status_words = txn.transfer_data(&templ, &[], 255)?.status_words(); From cfef291ad9ed0937a3bda1ad267730d3fe6dd6e4 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 15:24:10 +0000 Subject: [PATCH 12/24] Use u16 for raw StatusWords --- src/apdu.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/apdu.rs b/src/apdu.rs index e407749..1c6e30b 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -280,7 +280,7 @@ impl Response { /// Get the raw [`StatusWords`] code for this response. #[cfg(feature = "untested")] - pub fn code(&self) -> u32 { + pub fn code(&self) -> u16 { self.status_words.code() } @@ -317,7 +317,7 @@ impl From> for Response { } let sw = StatusWords::from( - (bytes[bytes.len() - 2] as u32) << 8 | (bytes[bytes.len() - 1] as u32), + (bytes[bytes.len() - 2] as u16) << 8 | (bytes[bytes.len() - 1] as u16), ); let len = bytes.len() - 2; @@ -377,15 +377,15 @@ pub(crate) enum StatusWords { NotSupportedError, /// Other/unrecognized status words - Other(u32), + Other(u16), } impl StatusWords { /// Get the numerical response code for these status words - pub fn code(self) -> u32 { + pub fn code(self) -> u16 { match self { StatusWords::None => 0, - StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u32, + StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u16, StatusWords::SecurityStatusError => 0x6982, StatusWords::AuthBlockedError => 0x6983, StatusWords::IncorrectParamError => 0x6a80, @@ -402,8 +402,8 @@ impl StatusWords { } } -impl From for StatusWords { - fn from(sw: u32) -> Self { +impl From for StatusWords { + fn from(sw: u16) -> Self { match sw { 0x0000 => StatusWords::None, sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError { @@ -420,8 +420,8 @@ impl From for StatusWords { } } -impl From for u32 { - fn from(sw: StatusWords) -> u32 { +impl From for u16 { + fn from(sw: StatusWords) -> u16 { sw.code() } } From a61a6fd94b8404ae88d2a08ba5d1b7147c6a3052 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 15:34:14 +0000 Subject: [PATCH 13/24] Define more YubiKey-recognized status words Recognized values sourced from https://github.com/Yubico/yubikey-manager NotFoundError and NoSpaceError are specified in SP 800-73-4 Table 6. --- src/apdu.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/apdu.rs b/src/apdu.rs index 1c6e30b..60de357 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -352,21 +352,42 @@ pub(crate) enum StatusWords { /// Successful execution Success, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L53 + NoInputDataError, + /// PIN verification failure VerifyFailError { /// Remaining verification attempts tries: u8, }, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L55 + WrongLengthError, + /// Security status not satisfied SecurityStatusError, /// Authentication method blocked AuthBlockedError, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L58 + DataInvalidError, + + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L59 + ConditionsNotSatisfiedError, + + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L60 + CommandNotAllowedError, + /// Incorrect parameter in command data field IncorrectParamError, + /// Data object or application not found + NotFoundError, + + /// Not enough memory + NoSpaceError, + // // Custom Yubico Status Word extensions // @@ -376,6 +397,9 @@ pub(crate) enum StatusWords { /// Not supported error NotSupportedError, + /// https://github.com/Yubico/yubikey-manager/blob/1f22620b623c6b345dd9f9193ec765a542dddc80/ykman/driver_ccid.py#L65 + CommandAbortedError, + /// Other/unrecognized status words Other(u16), } @@ -385,12 +409,20 @@ impl StatusWords { pub fn code(self) -> u16 { match self { StatusWords::None => 0, + StatusWords::NoInputDataError => 0x6285, StatusWords::VerifyFailError { tries } => 0x63c0 & tries as u16, + StatusWords::WrongLengthError => 0x6700, StatusWords::SecurityStatusError => 0x6982, StatusWords::AuthBlockedError => 0x6983, + StatusWords::DataInvalidError => 0x6984, + StatusWords::ConditionsNotSatisfiedError => 0x6985, + StatusWords::CommandNotAllowedError => 0x6986, StatusWords::IncorrectParamError => 0x6a80, + StatusWords::NotFoundError => 0x6a82, + StatusWords::NoSpaceError => 0x6a84, StatusWords::IncorrectSlotError => 0x6b00, StatusWords::NotSupportedError => 0x6d00, + StatusWords::CommandAbortedError => 0x6f00, StatusWords::Success => 0x9000, StatusWords::Other(n) => n, } @@ -406,14 +438,22 @@ impl From for StatusWords { fn from(sw: u16) -> Self { match sw { 0x0000 => StatusWords::None, + 0x6285 => StatusWords::NoInputDataError, sw if sw & 0xfff0 == 0x63c0 => StatusWords::VerifyFailError { tries: (sw & 0x000f) as u8, }, + 0x6700 => StatusWords::WrongLengthError, 0x6982 => StatusWords::SecurityStatusError, 0x6983 => StatusWords::AuthBlockedError, + 0x6984 => StatusWords::DataInvalidError, + 0x6985 => StatusWords::ConditionsNotSatisfiedError, + 0x6986 => StatusWords::CommandNotAllowedError, 0x6a80 => StatusWords::IncorrectParamError, + 0x6a82 => StatusWords::NotFoundError, + 0x6a84 => StatusWords::NoSpaceError, 0x6b00 => StatusWords::IncorrectSlotError, 0x6d00 => StatusWords::NotSupportedError, + 0x6f00 => StatusWords::CommandAbortedError, 0x9000 => StatusWords::Success, _ => StatusWords::Other(sw), } From 6a16c59567efb63461e219558e1b6328d0c6b0b8 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Sat, 30 Nov 2019 12:08:21 -0800 Subject: [PATCH 14/24] Use `secrecy` crate for storing `CachedPin` The `SecretVec` type automatically handles zeroing and may prevent accidental exposure of the cached PIN via `Debug`. --- Cargo.toml | 1 + src/yubikey.rs | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a94ceb0..c073861 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ hmac = "0.7" log = "0.4" pbkdf2 = "0.3" pcsc = "2" +secrecy = "0.5" sha-1 = "0.8" subtle = "2" zeroize = "1" diff --git a/src/yubikey.rs b/src/yubikey.rs index cedfc80..2daf3c7 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -40,13 +40,15 @@ use crate::{ metadata, mgm::MgmKey, serialization::*, - ObjectId, + Buffer, ObjectId, }; -use crate::{consts::*, error::Error, transaction::Transaction, Buffer}; +use crate::{consts::*, error::Error, transaction::Transaction}; #[cfg(feature = "untested")] use getrandom::getrandom; use log::{error, info, warn}; use pcsc::{Card, Context}; +#[cfg(feature = "untested")] +use secrecy::ExposeSecret; use std::fmt::{self, Display}; #[cfg(feature = "untested")] use std::{ @@ -63,6 +65,9 @@ pub const AID: [u8; 5] = [0xa0, 0x00, 0x00, 0x03, 0x08]; /// pub const MGMT_AID: [u8; 8] = [0xa0, 0x00, 0x00, 0x05, 0x27, 0x47, 0x11, 0x17]; +/// Cached YubiKey PIN +pub type CachedPin = secrecy::SecretVec; + /// YubiKey Serial Number #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] pub struct Serial(pub u32); @@ -118,7 +123,7 @@ impl Version { #[cfg_attr(not(feature = "untested"), allow(dead_code))] pub struct YubiKey { pub(crate) card: Card, - pub(crate) pin: Option, + pub(crate) pin: Option, pub(crate) is_neo: bool, pub(crate) version: Version, pub(crate) serial: Serial, @@ -228,8 +233,10 @@ impl YubiKey { pcsc::Disposition::ResetCard, )?; - // TODO(tarcieri): zeroize pin! - let pin = self.pin.clone(); + let pin = self + .pin + .as_ref() + .map(|p| Buffer::new(p.expose_secret().clone())); let txn = Transaction::new(&mut self.card)?; txn.select_application()?; @@ -388,7 +395,7 @@ impl YubiKey { } if !pin.is_empty() { - self.pin = Some(Buffer::new(pin.into())) + self.pin = Some(CachedPin::new(pin.into())) } Ok(()) @@ -445,7 +452,7 @@ impl YubiKey { } if !new_pin.is_empty() { - self.pin = Some(Buffer::new(new_pin.into())); + self.pin = Some(CachedPin::new(new_pin.into())); } Ok(()) From c3698dcffbc44292fc1d2de3d05e77876653e49b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 16:40:15 +0000 Subject: [PATCH 15/24] Key::list: Skip Certificate::new for empty buffers This matches the C code behaviour. --- src/key.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/key.rs b/src/key.rs index 8c07e15..64c1b1a 100644 --- a/src/key.rs +++ b/src/key.rs @@ -125,8 +125,10 @@ impl Key { } }; - let cert = Certificate::new(buf)?; - keys.push(Key { slot, cert }); + if !buf.is_empty() { + let cert = Certificate::new(buf)?; + keys.push(Key { slot, cert }); + } } Ok(keys) From 12b5bd1e3cdbcc9e57aeae68d649ec4a971d701d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 19:08:49 +0000 Subject: [PATCH 16/24] Convert SlotId into an enum --- src/certificate.rs | 13 +-- src/container.rs | 6 +- src/key.rs | 259 +++++++++++++++++++++++++++++++++++++-------- src/transaction.rs | 5 +- src/yubikey.rs | 16 ++- 5 files changed, 233 insertions(+), 66 deletions(-) diff --git a/src/certificate.rs b/src/certificate.rs index 71a3db4..6dbf4a5 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -31,13 +31,8 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::{ - consts::*, - error::Error, - key::{self, SlotId}, - serialization::*, - transaction::Transaction, - yubikey::YubiKey, - Buffer, + consts::*, error::Error, key::SlotId, serialization::*, transaction::Transaction, + yubikey::YubiKey, Buffer, }; use log::error; use zeroize::Zeroizing; @@ -100,7 +95,7 @@ impl AsRef<[u8]> for Certificate { /// Read certificate pub(crate) fn read_certificate(txn: &Transaction<'_>, slot: SlotId) -> Result { let mut len: usize = 0; - let object_id = key::slot_object(slot)?; + let object_id = slot.object_id(); let mut buf = match txn.fetch_object(object_id) { Ok(b) => b, @@ -141,7 +136,7 @@ pub(crate) fn write_certificate( let mut buf = [0u8; CB_OBJ_MAX]; let mut offset = 0; - let object_id = key::slot_object(slot)?; + let object_id = slot.object_id(); if data.is_none() { return txn.save_object(object_id, &[]); diff --git a/src/container.rs b/src/container.rs index ee5d8d3..56c4898 100644 --- a/src/container.rs +++ b/src/container.rs @@ -158,7 +158,7 @@ impl Container { Ok(Container { name, - slot: bytes[name_bytes_len], + slot: bytes[name_bytes_len].try_into()?, key_spec: bytes[name_bytes_len + 1], key_size_bits: u16::from_le_bytes( bytes[(name_bytes_len + 2)..(name_bytes_len + 4)] @@ -185,7 +185,7 @@ impl Container { bytes.extend_from_slice(&self.name[i].to_le_bytes()); } - bytes.push(self.slot); + bytes.push(self.slot.into()); bytes.push(self.key_spec); bytes.extend_from_slice(&self.key_size_bits.to_le_bytes()); bytes.push(self.flags); @@ -204,7 +204,7 @@ impl Debug for Container { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "PivContainer {{ name: {:?}, slot: {}, key_spec: {}, key_size_bits: {}, \ + "PivContainer {{ name: {:?}, slot: {:?}, key_spec: {}, key_size_bits: {}, \ flags: {}, pin_id: {}, associated_echd_container: {}, cert_fingerprint: {:?} }}", &self.name[..], self.slot, diff --git a/src/key.rs b/src/key.rs index 64c1b1a..53dbb0d 100644 --- a/src/key.rs +++ b/src/key.rs @@ -48,56 +48,229 @@ use crate::{ AlgorithmId, Buffer, ObjectId, }; use log::{debug, error, warn}; +use std::convert::TryFrom; /// Slot identifiers. /// -// TODO(tarcieri): replace these with enums -pub type SlotId = u8; +#[derive(Clone, Copy, Debug)] +pub enum SlotId { + /// This certificate and its associated private key is used to authenticate the card + /// and the cardholder. This slot is used for things like system login. The end user + /// PIN is required to perform any private key operations. Once the PIN has been + /// provided successfully, multiple private key operations may be performed without + /// additional cardholder consent. + Authentication, -/// Get the [`ObjectId`] that corresponds to a given [`SlotId`] -// TODO(tarcieri): factor this into a slot ID enum -pub(crate) fn slot_object(slot: SlotId) -> Result { - let id = match slot { - YKPIV_KEY_AUTHENTICATION => YKPIV_OBJ_AUTHENTICATION, - YKPIV_KEY_SIGNATURE => YKPIV_OBJ_SIGNATURE, - YKPIV_KEY_KEYMGM => YKPIV_OBJ_KEY_MANAGEMENT, - YKPIV_KEY_CARDAUTH => YKPIV_OBJ_CARD_AUTH, - YKPIV_KEY_ATTESTATION => YKPIV_OBJ_ATTESTATION, - slot if slot >= YKPIV_KEY_RETIRED1 && (slot <= YKPIV_KEY_RETIRED20) => { - YKPIV_OBJ_RETIRED1 + (slot - YKPIV_KEY_RETIRED1) as u32 + /// This certificate and its associated private key is used for digital signatures for + /// the purpose of document signing, or signing files and executables. The end user + /// PIN is required to perform any private key operations. The PIN must be submitted + /// every time immediately before a sign operation, to ensure cardholder participation + /// for every digital signature generated. + Signature, + + /// This certificate and its associated private key is used for encryption for the + /// purpose of confidentiality. This slot is used for things like encrypting e-mails + /// or files. The end user PIN is required to perform any private key operations. Once + /// the PIN has been provided successfully, multiple private key operations may be + /// performed without additional cardholder consent. + KeyManagement, + + /// This certificate and its associated private key is used to support additional + /// physical access applications, such as providing physical access to buildings via + /// PIV-enabled door locks. The end user PIN is NOT required to perform private key + /// operations for this slot. + CardAuthentication, + + /// These slots are only available on the YubiKey 4 & 5. They are meant for previously + /// used Key Management keys to be able to decrypt earlier encrypted documents or + /// emails. In the YubiKey 4 & 5 all 20 of them are fully available for use. + Retired(RetiredSlotId), + + /// This slot is only available on YubiKey version 4.3 and newer. It is only used for + /// attestation of other keys generated on device with instruction `f9`. This slot is + /// not cleared on reset, but can be overwritten. + Attestation, +} + +impl TryFrom for SlotId { + type Error = Error; + + fn try_from(value: u8) -> Result { + match value { + YKPIV_KEY_AUTHENTICATION => Ok(SlotId::Authentication), + YKPIV_KEY_SIGNATURE => Ok(SlotId::Signature), + YKPIV_KEY_KEYMGM => Ok(SlotId::KeyManagement), + YKPIV_KEY_CARDAUTH => Ok(SlotId::CardAuthentication), + YKPIV_KEY_ATTESTATION => Ok(SlotId::Attestation), + _ => RetiredSlotId::try_from(value).map(SlotId::Retired), } - _ => return Err(Error::InvalidObject), - }; + } +} - Ok(id) +impl From for u8 { + fn from(slot: SlotId) -> u8 { + match slot { + SlotId::Authentication => YKPIV_KEY_AUTHENTICATION, + SlotId::Signature => YKPIV_KEY_SIGNATURE, + SlotId::KeyManagement => YKPIV_KEY_KEYMGM, + SlotId::CardAuthentication => YKPIV_KEY_CARDAUTH, + SlotId::Retired(retired) => retired.into(), + SlotId::Attestation => YKPIV_KEY_ATTESTATION, + } + } +} + +impl SlotId { + /// Returns the [`ObjectId`] that corresponds to a given [`SlotId`]. + pub(crate) fn object_id(self) -> ObjectId { + match self { + SlotId::Authentication => YKPIV_OBJ_AUTHENTICATION, + SlotId::Signature => YKPIV_OBJ_SIGNATURE, + SlotId::KeyManagement => YKPIV_OBJ_KEY_MANAGEMENT, + SlotId::CardAuthentication => YKPIV_OBJ_CARD_AUTH, + SlotId::Retired(retired) => retired.object_id(), + SlotId::Attestation => YKPIV_OBJ_ATTESTATION, + } + } +} + +/// Retired slot IDs. +#[allow(missing_docs)] +#[derive(Clone, Copy, Debug)] +pub enum RetiredSlotId { + R1, + R2, + R3, + R4, + R5, + R6, + R7, + R8, + R9, + R10, + R11, + R12, + R13, + R14, + R15, + R16, + R17, + R18, + R19, + R20, +} + +impl TryFrom for RetiredSlotId { + type Error = Error; + + fn try_from(value: u8) -> Result { + match value { + YKPIV_KEY_RETIRED1 => Ok(RetiredSlotId::R1), + YKPIV_KEY_RETIRED2 => Ok(RetiredSlotId::R2), + YKPIV_KEY_RETIRED3 => Ok(RetiredSlotId::R3), + YKPIV_KEY_RETIRED4 => Ok(RetiredSlotId::R4), + YKPIV_KEY_RETIRED5 => Ok(RetiredSlotId::R5), + YKPIV_KEY_RETIRED6 => Ok(RetiredSlotId::R6), + YKPIV_KEY_RETIRED7 => Ok(RetiredSlotId::R7), + YKPIV_KEY_RETIRED8 => Ok(RetiredSlotId::R8), + YKPIV_KEY_RETIRED9 => Ok(RetiredSlotId::R9), + YKPIV_KEY_RETIRED10 => Ok(RetiredSlotId::R10), + YKPIV_KEY_RETIRED11 => Ok(RetiredSlotId::R11), + YKPIV_KEY_RETIRED12 => Ok(RetiredSlotId::R12), + YKPIV_KEY_RETIRED13 => Ok(RetiredSlotId::R13), + YKPIV_KEY_RETIRED14 => Ok(RetiredSlotId::R14), + YKPIV_KEY_RETIRED15 => Ok(RetiredSlotId::R15), + YKPIV_KEY_RETIRED16 => Ok(RetiredSlotId::R16), + YKPIV_KEY_RETIRED17 => Ok(RetiredSlotId::R17), + YKPIV_KEY_RETIRED18 => Ok(RetiredSlotId::R18), + YKPIV_KEY_RETIRED19 => Ok(RetiredSlotId::R19), + YKPIV_KEY_RETIRED20 => Ok(RetiredSlotId::R20), + _ => Err(Error::InvalidObject), + } + } +} + +impl From for u8 { + fn from(slot: RetiredSlotId) -> u8 { + match slot { + RetiredSlotId::R1 => YKPIV_KEY_RETIRED1, + RetiredSlotId::R2 => YKPIV_KEY_RETIRED2, + RetiredSlotId::R3 => YKPIV_KEY_RETIRED3, + RetiredSlotId::R4 => YKPIV_KEY_RETIRED4, + RetiredSlotId::R5 => YKPIV_KEY_RETIRED5, + RetiredSlotId::R6 => YKPIV_KEY_RETIRED6, + RetiredSlotId::R7 => YKPIV_KEY_RETIRED7, + RetiredSlotId::R8 => YKPIV_KEY_RETIRED8, + RetiredSlotId::R9 => YKPIV_KEY_RETIRED9, + RetiredSlotId::R10 => YKPIV_KEY_RETIRED10, + RetiredSlotId::R11 => YKPIV_KEY_RETIRED11, + RetiredSlotId::R12 => YKPIV_KEY_RETIRED12, + RetiredSlotId::R13 => YKPIV_KEY_RETIRED13, + RetiredSlotId::R14 => YKPIV_KEY_RETIRED14, + RetiredSlotId::R15 => YKPIV_KEY_RETIRED15, + RetiredSlotId::R16 => YKPIV_KEY_RETIRED16, + RetiredSlotId::R17 => YKPIV_KEY_RETIRED17, + RetiredSlotId::R18 => YKPIV_KEY_RETIRED18, + RetiredSlotId::R19 => YKPIV_KEY_RETIRED19, + RetiredSlotId::R20 => YKPIV_KEY_RETIRED20, + } + } +} + +impl RetiredSlotId { + /// Returns the [`ObjectId`] that corresponds to a given [`RetiredSlotId`]. + pub(crate) fn object_id(self) -> ObjectId { + match self { + RetiredSlotId::R1 => YKPIV_OBJ_RETIRED1, + RetiredSlotId::R2 => YKPIV_OBJ_RETIRED2, + RetiredSlotId::R3 => YKPIV_OBJ_RETIRED3, + RetiredSlotId::R4 => YKPIV_OBJ_RETIRED4, + RetiredSlotId::R5 => YKPIV_OBJ_RETIRED5, + RetiredSlotId::R6 => YKPIV_OBJ_RETIRED6, + RetiredSlotId::R7 => YKPIV_OBJ_RETIRED7, + RetiredSlotId::R8 => YKPIV_OBJ_RETIRED8, + RetiredSlotId::R9 => YKPIV_OBJ_RETIRED9, + RetiredSlotId::R10 => YKPIV_OBJ_RETIRED10, + RetiredSlotId::R11 => YKPIV_OBJ_RETIRED11, + RetiredSlotId::R12 => YKPIV_OBJ_RETIRED12, + RetiredSlotId::R13 => YKPIV_OBJ_RETIRED13, + RetiredSlotId::R14 => YKPIV_OBJ_RETIRED14, + RetiredSlotId::R15 => YKPIV_OBJ_RETIRED15, + RetiredSlotId::R16 => YKPIV_OBJ_RETIRED16, + RetiredSlotId::R17 => YKPIV_OBJ_RETIRED17, + RetiredSlotId::R18 => YKPIV_OBJ_RETIRED18, + RetiredSlotId::R19 => YKPIV_OBJ_RETIRED19, + RetiredSlotId::R20 => YKPIV_OBJ_RETIRED20, + } + } } /// Personal Identity Verification (PIV) key slots -pub const SLOTS: [u8; 24] = [ - YKPIV_KEY_AUTHENTICATION, - YKPIV_KEY_SIGNATURE, - YKPIV_KEY_KEYMGM, - YKPIV_KEY_RETIRED1, - YKPIV_KEY_RETIRED2, - YKPIV_KEY_RETIRED3, - YKPIV_KEY_RETIRED4, - YKPIV_KEY_RETIRED5, - YKPIV_KEY_RETIRED6, - YKPIV_KEY_RETIRED7, - YKPIV_KEY_RETIRED8, - YKPIV_KEY_RETIRED9, - YKPIV_KEY_RETIRED10, - YKPIV_KEY_RETIRED11, - YKPIV_KEY_RETIRED12, - YKPIV_KEY_RETIRED13, - YKPIV_KEY_RETIRED14, - YKPIV_KEY_RETIRED15, - YKPIV_KEY_RETIRED16, - YKPIV_KEY_RETIRED17, - YKPIV_KEY_RETIRED18, - YKPIV_KEY_RETIRED19, - YKPIV_KEY_RETIRED20, - YKPIV_KEY_CARDAUTH, +pub const SLOTS: [SlotId; 24] = [ + SlotId::Authentication, + SlotId::Signature, + SlotId::KeyManagement, + SlotId::Retired(RetiredSlotId::R1), + SlotId::Retired(RetiredSlotId::R2), + SlotId::Retired(RetiredSlotId::R3), + SlotId::Retired(RetiredSlotId::R4), + SlotId::Retired(RetiredSlotId::R5), + SlotId::Retired(RetiredSlotId::R6), + SlotId::Retired(RetiredSlotId::R7), + SlotId::Retired(RetiredSlotId::R8), + SlotId::Retired(RetiredSlotId::R9), + SlotId::Retired(RetiredSlotId::R10), + SlotId::Retired(RetiredSlotId::R11), + SlotId::Retired(RetiredSlotId::R12), + SlotId::Retired(RetiredSlotId::R13), + SlotId::Retired(RetiredSlotId::R14), + SlotId::Retired(RetiredSlotId::R15), + SlotId::Retired(RetiredSlotId::R16), + SlotId::Retired(RetiredSlotId::R17), + SlotId::Retired(RetiredSlotId::R18), + SlotId::Retired(RetiredSlotId::R19), + SlotId::Retired(RetiredSlotId::R20), + SlotId::CardAuthentication, ]; /// PIV cryptographic keys stored in a YubiKey @@ -120,7 +293,7 @@ impl Key { let buf = match certificate::read_certificate(&txn, slot) { Ok(b) => b, Err(e) => { - debug!("error reading certificate in slot {}: {}", slot, e); + debug!("error reading certificate in slot {:?}: {}", slot, e); continue; } }; @@ -252,7 +425,7 @@ pub fn generate( let txn = yubikey.begin_transaction()?; - templ[3] = slot; + templ[3] = slot.into(); let mut offset = 5; in_data[..offset].copy_from_slice(&[ diff --git a/src/transaction.rs b/src/transaction.rs index 68fa89f..bda9caf 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -9,6 +9,7 @@ use crate::{ use crate::{ apdu::{Response, StatusWords}, consts::*, + key::SlotId, mgm::MgmKey, serialization::*, Buffer, ObjectId, @@ -267,12 +268,12 @@ impl<'tx> Transaction<'tx> { &self, sign_in: &[u8], algorithm: u8, - key: u8, + key: SlotId, decipher: bool, ) -> Result { let in_len = sign_in.len(); let mut indata = [0u8; 1024]; - let templ = [0, Ins::Authenticate.code(), algorithm, key]; + let templ = [0, Ins::Authenticate.code(), algorithm, key.into()]; let mut len: usize = 0; match algorithm { diff --git a/src/yubikey.rs b/src/yubikey.rs index cedfc80..e18e5a6 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -609,15 +609,13 @@ impl YubiKey { touch_policy: u8, ) -> Result<(), Error> { let mut key_data = Zeroizing::new(vec![0u8; 1024]); - let templ = [0, Ins::ImportKey.code(), algorithm, key]; + let templ = [0, Ins::ImportKey.code(), algorithm, key.into()]; - if key == YKPIV_KEY_CARDMGM - || key < YKPIV_KEY_RETIRED1 - || (key > YKPIV_KEY_RETIRED20 && key < YKPIV_KEY_AUTHENTICATION) - || (key > YKPIV_KEY_CARDAUTH && key != YKPIV_KEY_ATTESTATION) - { - return Err(Error::KeyError); - } + // Only slot we want to exclude is CardManagement, which isn't in the enum. + // TODO: Decide whether to add it or not. + // match key { + // SlotId::CardManagement => return Err(Error::KeyError), + // } if pin_policy != YKPIV_PINPOLICY_DEFAULT && pin_policy != YKPIV_PINPOLICY_NEVER @@ -730,7 +728,7 @@ impl YubiKey { /// #[cfg(feature = "untested")] pub fn attest(&mut self, key: SlotId) -> Result { - let templ = [0, Ins::Attest.code(), key, 0]; + let templ = [0, Ins::Attest.code(), key.into(), 0]; let txn = self.begin_transaction()?; let response = txn.transfer_data(&templ, &[], CB_OBJ_MAX)?; From afca0fec0a72fae6a79e7f24bb37136146d5cea8 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 19:36:09 +0000 Subject: [PATCH 17/24] Convert AlgorithmId into an enum 3DES also has an algorithm ID, but it is completely disjoint from the key algorithms, and can be handled separately later. --- src/consts.rs | 4 -- src/key.rs | 133 ++++++++++++++++++++++++++++----------------- src/lib.rs | 4 -- src/transaction.rs | 25 ++++----- src/yubikey.rs | 23 ++++---- 5 files changed, 104 insertions(+), 85 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index 75ef5dd..ab903e7 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -124,10 +124,6 @@ pub const TAG_ECC_POINT: u8 = 0x86; pub const YKPIV_ALGO_TAG: u8 = 0x80; pub const YKPIV_ALGO_3DES: u8 = 0x03; -pub const YKPIV_ALGO_RSA1024: u8 = 0x06; -pub const YKPIV_ALGO_RSA2048: u8 = 0x07; -pub const YKPIV_ALGO_ECCP256: u8 = 0x11; -pub const YKPIV_ALGO_ECCP384: u8 = 0x14; 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 53dbb0d..59cd178 100644 --- a/src/key.rs +++ b/src/key.rs @@ -45,7 +45,7 @@ use crate::{ serialization::*, settings, yubikey::YubiKey, - AlgorithmId, Buffer, ObjectId, + Buffer, ObjectId, }; use log::{debug, error, warn}; use std::convert::TryFrom; @@ -273,6 +273,44 @@ pub const SLOTS: [SlotId; 24] = [ SlotId::CardAuthentication, ]; +/// Algorithm identifiers +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum AlgorithmId { + /// 1024-bit RSA. + Rsa1024, + /// 2048-bit RSA. + Rsa2048, + /// ECDSA with the NIST P256 curve. + EccP256, + /// ECDSA with the NIST P384 curve. + EccP384, +} + +impl TryFrom for AlgorithmId { + type Error = Error; + + fn try_from(value: u8) -> Result { + match value { + 0x06 => Ok(AlgorithmId::Rsa1024), + 0x07 => Ok(AlgorithmId::Rsa2048), + 0x11 => Ok(AlgorithmId::EccP256), + 0x14 => Ok(AlgorithmId::EccP384), + _ => Err(Error::AlgorithmError), + } + } +} + +impl From for u8 { + fn from(id: AlgorithmId) -> u8 { + match id { + AlgorithmId::Rsa1024 => 0x06, + AlgorithmId::Rsa2048 => 0x07, + AlgorithmId::EccP256 => 0x11, + AlgorithmId::EccP384 => 0x14, + } + } +} + /// PIV cryptographic keys stored in a YubiKey #[derive(Clone, Debug)] pub struct Key { @@ -377,50 +415,47 @@ pub fn generate( let mut templ = [0, Ins::GenerateAsymmetric.code(), 0, 0]; let setting_roca: settings::BoolValue; - if yubikey.device_model() == DEVTYPE_YK4 - && (algorithm == YKPIV_ALGO_RSA1024 || algorithm == YKPIV_ALGO_RSA2048) - && yubikey.version.major == 4 - && (yubikey.version.minor < 3 || yubikey.version.minor == 3 && (yubikey.version.patch < 5)) - { - setting_roca = settings::BoolValue::get(SZ_SETTING_ROCA, true); - - let psz_msg = match setting_roca.source { - settings::Source::User => { - if setting_roca.value { - SZ_ROCA_ALLOW_USER - } else { - SZ_ROCA_BLOCK_USER - } - } - settings::Source::Admin => { - if setting_roca.value { - SZ_ROCA_ALLOW_ADMIN - } else { - SZ_ROCA_BLOCK_ADMIN - } - } - _ => SZ_ROCA_DEFAULT, - }; - - warn!( - "YubiKey serial number {} is affected by vulnerability CVE-2017-15361 \ - (ROCA) and should be replaced. On-chip key generation {} See \ - YSA-2017-01 \ - for additional information on device replacement and mitigation assistance", - yubikey.serial, psz_msg - ); - - if !setting_roca.value { - return Err(Error::NotSupported); - } - } - match algorithm { - YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 | YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => (), - _ => { - error!("invalid algorithm specified"); - return Err(Error::GenericError); + AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => { + if yubikey.device_model() == DEVTYPE_YK4 + && yubikey.version.major == 4 + && (yubikey.version.minor < 3 + || yubikey.version.minor == 3 && (yubikey.version.patch < 5)) + { + setting_roca = settings::BoolValue::get(SZ_SETTING_ROCA, true); + + let psz_msg = match setting_roca.source { + settings::Source::User => { + if setting_roca.value { + SZ_ROCA_ALLOW_USER + } else { + SZ_ROCA_BLOCK_USER + } + } + settings::Source::Admin => { + if setting_roca.value { + SZ_ROCA_ALLOW_ADMIN + } else { + SZ_ROCA_BLOCK_ADMIN + } + } + _ => SZ_ROCA_DEFAULT, + }; + + warn!( + "YubiKey serial number {} is affected by vulnerability CVE-2017-15361 \ + (ROCA) and should be replaced. On-chip key generation {} See \ + YSA-2017-01 \ + for additional information on device replacement and mitigation assistance", + yubikey.serial, psz_msg + ); + + if !setting_roca.value { + return Err(Error::NotSupported); + } + } } + _ => (), } let txn = yubikey.begin_transaction()?; @@ -433,7 +468,7 @@ pub fn generate( 3, // length sans this 2-byte header YKPIV_ALGO_TAG, 1, - algorithm, + algorithm.into(), ]); if in_data[4] == 0 { @@ -487,7 +522,7 @@ pub fn generate( let data = Buffer::new(response.data().into()); match algorithm { - YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => { + AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => { let mut offset = 5; let mut len = 0; @@ -515,10 +550,10 @@ pub fn generate( exp, }) } - YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => { + AlgorithmId::EccP256 | AlgorithmId::EccP384 => { let mut offset = 3; - let len = if algorithm == YKPIV_ALGO_ECCP256 { + let len = if let AlgorithmId::EccP256 = algorithm { CB_ECC_POINTP256 } else { CB_ECC_POINTP384 @@ -542,9 +577,5 @@ pub fn generate( let point = data[offset..(offset + len)].to_vec(); Ok(GeneratedKey::Ecc { algorithm, point }) } - _ => { - error!("wrong algorithm"); - Err(Error::AlgorithmError) - } } } diff --git a/src/lib.rs b/src/lib.rs index b4d9851..37fd0c2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -167,10 +167,6 @@ pub mod yubikey; pub use self::{key::Key, mgm::MgmKey}; pub use yubikey::YubiKey; -/// Algorithm identifiers -// TODO(tarcieri): make this an enum -pub type AlgorithmId = u8; - /// Object identifiers pub type ObjectId = u32; diff --git a/src/transaction.rs b/src/transaction.rs index bda9caf..2514024 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -9,7 +9,7 @@ use crate::{ use crate::{ apdu::{Response, StatusWords}, consts::*, - key::SlotId, + key::{AlgorithmId, SlotId}, mgm::MgmKey, serialization::*, Buffer, ObjectId, @@ -267,18 +267,18 @@ impl<'tx> Transaction<'tx> { pub(crate) fn authenticated_command( &self, sign_in: &[u8], - algorithm: u8, + algorithm: AlgorithmId, key: SlotId, decipher: bool, ) -> Result { let in_len = sign_in.len(); let mut indata = [0u8; 1024]; - let templ = [0, Ins::Authenticate.code(), algorithm, key.into()]; + let templ = [0, Ins::Authenticate.code(), algorithm.into(), key.into()]; let mut len: usize = 0; match algorithm { - YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => { - let key_len = if algorithm == YKPIV_ALGO_RSA1024 { + AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => { + let key_len = if let AlgorithmId::Rsa1024 = algorithm { 128 } else { 256 @@ -288,8 +288,8 @@ impl<'tx> Transaction<'tx> { return Err(Error::SizeError); } } - YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => { - let key_len = if algorithm == YKPIV_ALGO_ECCP256 { + AlgorithmId::EccP256 | AlgorithmId::EccP384 => { + let key_len = if let AlgorithmId::EccP256 = algorithm { 32 } else { 48 @@ -300,7 +300,6 @@ impl<'tx> Transaction<'tx> { return Err(Error::SizeError); } } - _ => return Err(Error::AlgorithmError), } let bytes = if in_len < 0x80 { @@ -315,12 +314,10 @@ impl<'tx> Transaction<'tx> { let mut offset = 1 + set_length(&mut indata[1..], in_len + bytes + 3); indata[offset] = 0x82; indata[offset + 1] = 0x00; - indata[offset + 2] = - if (algorithm == YKPIV_ALGO_ECCP256 || algorithm == YKPIV_ALGO_ECCP384) && decipher { - 0x85 - } else { - 0x81 - }; + indata[offset + 2] = match (algorithm, decipher) { + (AlgorithmId::EccP256, true) | (AlgorithmId::EccP384, true) => 0x85, + _ => 0x81, + }; offset += 3; offset += set_length(&mut indata[offset..], in_len); diff --git a/src/yubikey.rs b/src/yubikey.rs index e18e5a6..eba6e04 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -36,7 +36,7 @@ #[cfg(feature = "untested")] use crate::{ apdu::{Ins, StatusWords, APDU}, - key::SlotId, + key::{AlgorithmId, SlotId}, metadata, mgm::MgmKey, serialization::*, @@ -356,7 +356,7 @@ impl YubiKey { pub fn sign_data( &mut self, raw_in: &[u8], - algorithm: u8, + algorithm: AlgorithmId, key: SlotId, ) -> Result { let txn = self.begin_transaction()?; @@ -370,7 +370,7 @@ impl YubiKey { pub fn decrypt_data( &mut self, input: &[u8], - algorithm: u8, + algorithm: AlgorithmId, key: SlotId, ) -> Result { let txn = self.begin_transaction()?; @@ -598,7 +598,7 @@ impl YubiKey { pub fn import_private_key( &mut self, key: SlotId, - algorithm: u8, + algorithm: AlgorithmId, p: Option<&[u8]>, q: Option<&[u8]>, dp: Option<&[u8]>, @@ -609,7 +609,7 @@ impl YubiKey { touch_policy: u8, ) -> Result<(), Error> { let mut key_data = Zeroizing::new(vec![0u8; 1024]); - let templ = [0, Ins::ImportKey.code(), algorithm, key.into()]; + let templ = [0, Ins::ImportKey.code(), algorithm.into(), key.into()]; // Only slot we want to exclude is CardManagement, which isn't in the enum. // TODO: Decide whether to add it or not. @@ -634,7 +634,7 @@ impl YubiKey { } let (elem_len, params, param_tag) = match algorithm { - YKPIV_ALGO_RSA1024 | YKPIV_ALGO_RSA2048 => match (p, q, dp, dq, qinv) { + AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => match (p, q, dp, dq, qinv) { (Some(p), Some(q), Some(dp), Some(dq), Some(qinv)) => { if p.len() + q.len() + dp.len() + dq.len() + qinv.len() >= key_data.len() { return Err(Error::SizeError); @@ -642,8 +642,8 @@ impl YubiKey { ( match algorithm { - YKPIV_ALGO_RSA1024 => 64, - YKPIV_ALGO_RSA2048 => 128, + AlgorithmId::Rsa1024 => 64, + AlgorithmId::Rsa2048 => 128, _ => unreachable!(), }, vec![p, q, dp, dq, qinv], @@ -652,7 +652,7 @@ impl YubiKey { } _ => return Err(Error::GenericError), }, - YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => match ec_data { + AlgorithmId::EccP256 | AlgorithmId::EccP384 => match ec_data { Some(ec_data) => { if ec_data.len() >= key_data.len() { // This can never be true, but check to be explicit. @@ -661,8 +661,8 @@ impl YubiKey { ( match algorithm { - YKPIV_ALGO_ECCP256 => 32, - YKPIV_ALGO_ECCP384 => 48, + AlgorithmId::EccP256 => 32, + AlgorithmId::EccP384 => 48, _ => unreachable!(), }, vec![ec_data], @@ -671,7 +671,6 @@ impl YubiKey { } _ => return Err(Error::GenericError), }, - _ => return Err(Error::AlgorithmError), }; let mut offset = 0; From 11c93d6421e9851dee26440cdac6c55e65782e13 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 22:01:22 +0000 Subject: [PATCH 18/24] Inline SlotId constants --- src/consts.rs | 51 ----------------- src/key.rs | 150 +++++++++++++++++++++++++------------------------- 2 files changed, 75 insertions(+), 126 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index ab903e7..2988342 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -137,72 +137,21 @@ pub const YKPIV_CCCID_SIZE: usize = 14; pub const YKPIV_CERTINFO_UNCOMPRESSED: u8 = 0; pub const YKPIV_CERTINFO_GZIP: u8 = 1; -pub const YKPIV_KEY_AUTHENTICATION: u8 = 0x9a; pub const YKPIV_KEY_CARDMGM: u8 = 0x9b; -pub const YKPIV_KEY_SIGNATURE: u8 = 0x9c; -pub const YKPIV_KEY_KEYMGM: u8 = 0x9d; -pub const YKPIV_KEY_CARDAUTH: u8 = 0x9e; -pub const YKPIV_KEY_RETIRED1: u8 = 0x82; -pub const YKPIV_KEY_RETIRED2: u8 = 0x83; -pub const YKPIV_KEY_RETIRED3: u8 = 0x84; -pub const YKPIV_KEY_RETIRED4: u8 = 0x85; -pub const YKPIV_KEY_RETIRED5: u8 = 0x86; -pub const YKPIV_KEY_RETIRED6: u8 = 0x87; -pub const YKPIV_KEY_RETIRED7: u8 = 0x88; -pub const YKPIV_KEY_RETIRED8: u8 = 0x89; -pub const YKPIV_KEY_RETIRED9: u8 = 0x8a; -pub const YKPIV_KEY_RETIRED10: u8 = 0x8b; -pub const YKPIV_KEY_RETIRED11: u8 = 0x8c; -pub const YKPIV_KEY_RETIRED12: u8 = 0x8d; -pub const YKPIV_KEY_RETIRED13: u8 = 0x8e; -pub const YKPIV_KEY_RETIRED14: u8 = 0x8f; -pub const YKPIV_KEY_RETIRED15: u8 = 0x90; -pub const YKPIV_KEY_RETIRED16: u8 = 0x91; -pub const YKPIV_KEY_RETIRED17: u8 = 0x92; -pub const YKPIV_KEY_RETIRED18: u8 = 0x93; -pub const YKPIV_KEY_RETIRED19: u8 = 0x94; -pub const YKPIV_KEY_RETIRED20: u8 = 0x95; -pub const YKPIV_KEY_ATTESTATION: u8 = 0xf9; pub const YKPIV_OBJ_CAPABILITY: u32 = 0x005f_c107; pub const YKPIV_OBJ_CHUID: u32 = 0x005f_c102; -pub const YKPIV_OBJ_AUTHENTICATION: u32 = 0x005f_c105; // cert for 9a key pub const YKPIV_OBJ_FINGERPRINTS: u32 = 0x005f_c103; pub const YKPIV_OBJ_SECURITY: u32 = 0x005f_c106; pub const YKPIV_OBJ_FACIAL: u32 = 0x005f_c108; pub const YKPIV_OBJ_PRINTED: u32 = 0x005f_c109; -pub const YKPIV_OBJ_SIGNATURE: u32 = 0x005f_c10a; // cert for 9c key -pub const YKPIV_OBJ_KEY_MANAGEMENT: u32 = 0x005f_c10b; // cert for 9d key -pub const YKPIV_OBJ_CARD_AUTH: u32 = 0x005f_c101; // cert for 9e key pub const YKPIV_OBJ_DISCOVERY: u32 = 0x7e; pub const YKPIV_OBJ_KEY_HISTORY: u32 = 0x005f_c10c; pub const YKPIV_OBJ_IRIS: u32 = 0x005f_c121; -pub const YKPIV_OBJ_RETIRED1: u32 = 0x005f_c10d; -pub const YKPIV_OBJ_RETIRED2: u32 = 0x005f_c10e; -pub const YKPIV_OBJ_RETIRED3: u32 = 0x005f_c10f; -pub const YKPIV_OBJ_RETIRED4: u32 = 0x005f_c110; -pub const YKPIV_OBJ_RETIRED5: u32 = 0x005f_c111; -pub const YKPIV_OBJ_RETIRED6: u32 = 0x005f_c112; -pub const YKPIV_OBJ_RETIRED7: u32 = 0x005f_c113; -pub const YKPIV_OBJ_RETIRED8: u32 = 0x005f_c114; -pub const YKPIV_OBJ_RETIRED9: u32 = 0x005f_c115; -pub const YKPIV_OBJ_RETIRED10: u32 = 0x005f_c116; -pub const YKPIV_OBJ_RETIRED11: u32 = 0x005f_c117; -pub const YKPIV_OBJ_RETIRED12: u32 = 0x005f_c118; -pub const YKPIV_OBJ_RETIRED13: u32 = 0x005f_c119; -pub const YKPIV_OBJ_RETIRED14: u32 = 0x005f_c11a; -pub const YKPIV_OBJ_RETIRED15: u32 = 0x005f_c11b; -pub const YKPIV_OBJ_RETIRED16: u32 = 0x005f_c11c; -pub const YKPIV_OBJ_RETIRED17: u32 = 0x005f_c11d; -pub const YKPIV_OBJ_RETIRED18: u32 = 0x005f_c11e; -pub const YKPIV_OBJ_RETIRED19: u32 = 0x005f_c11f; -pub const YKPIV_OBJ_RETIRED20: u32 = 0x005f_c120; - // Internal object IDs pub const YKPIV_OBJ_ADMIN_DATA: u32 = 0x005f_ff00; -pub const YKPIV_OBJ_ATTESTATION: u32 = 0x005f_ff01; pub const YKPIV_OBJ_MSCMAP: u32 = 0x005f_ff10; pub const YKPIV_OBJ_MSROOTS1: u32 = 0x005f_ff11; pub const YKPIV_OBJ_MSROOTS2: u32 = 0x005f_ff12; diff --git a/src/key.rs b/src/key.rs index 59cd178..96392c7 100644 --- a/src/key.rs +++ b/src/key.rs @@ -97,11 +97,11 @@ impl TryFrom for SlotId { fn try_from(value: u8) -> Result { match value { - YKPIV_KEY_AUTHENTICATION => Ok(SlotId::Authentication), - YKPIV_KEY_SIGNATURE => Ok(SlotId::Signature), - YKPIV_KEY_KEYMGM => Ok(SlotId::KeyManagement), - YKPIV_KEY_CARDAUTH => Ok(SlotId::CardAuthentication), - YKPIV_KEY_ATTESTATION => Ok(SlotId::Attestation), + 0x9a => Ok(SlotId::Authentication), + 0x9c => Ok(SlotId::Signature), + 0x9d => Ok(SlotId::KeyManagement), + 0x9e => Ok(SlotId::CardAuthentication), + 0xf9 => Ok(SlotId::Attestation), _ => RetiredSlotId::try_from(value).map(SlotId::Retired), } } @@ -110,12 +110,12 @@ impl TryFrom for SlotId { impl From for u8 { fn from(slot: SlotId) -> u8 { match slot { - SlotId::Authentication => YKPIV_KEY_AUTHENTICATION, - SlotId::Signature => YKPIV_KEY_SIGNATURE, - SlotId::KeyManagement => YKPIV_KEY_KEYMGM, - SlotId::CardAuthentication => YKPIV_KEY_CARDAUTH, + SlotId::Authentication => 0x9a, + SlotId::Signature => 0x9c, + SlotId::KeyManagement => 0x9d, + SlotId::CardAuthentication => 0x9e, SlotId::Retired(retired) => retired.into(), - SlotId::Attestation => YKPIV_KEY_ATTESTATION, + SlotId::Attestation => 0xf9, } } } @@ -124,12 +124,12 @@ impl SlotId { /// Returns the [`ObjectId`] that corresponds to a given [`SlotId`]. pub(crate) fn object_id(self) -> ObjectId { match self { - SlotId::Authentication => YKPIV_OBJ_AUTHENTICATION, - SlotId::Signature => YKPIV_OBJ_SIGNATURE, - SlotId::KeyManagement => YKPIV_OBJ_KEY_MANAGEMENT, - SlotId::CardAuthentication => YKPIV_OBJ_CARD_AUTH, + SlotId::Authentication => 0x005f_c105, + SlotId::Signature => 0x005f_c10a, + SlotId::KeyManagement => 0x005f_c10b, + SlotId::CardAuthentication => 0x005f_c101, SlotId::Retired(retired) => retired.object_id(), - SlotId::Attestation => YKPIV_OBJ_ATTESTATION, + SlotId::Attestation => 0x005f_ff01, } } } @@ -165,26 +165,26 @@ impl TryFrom for RetiredSlotId { fn try_from(value: u8) -> Result { match value { - YKPIV_KEY_RETIRED1 => Ok(RetiredSlotId::R1), - YKPIV_KEY_RETIRED2 => Ok(RetiredSlotId::R2), - YKPIV_KEY_RETIRED3 => Ok(RetiredSlotId::R3), - YKPIV_KEY_RETIRED4 => Ok(RetiredSlotId::R4), - YKPIV_KEY_RETIRED5 => Ok(RetiredSlotId::R5), - YKPIV_KEY_RETIRED6 => Ok(RetiredSlotId::R6), - YKPIV_KEY_RETIRED7 => Ok(RetiredSlotId::R7), - YKPIV_KEY_RETIRED8 => Ok(RetiredSlotId::R8), - YKPIV_KEY_RETIRED9 => Ok(RetiredSlotId::R9), - YKPIV_KEY_RETIRED10 => Ok(RetiredSlotId::R10), - YKPIV_KEY_RETIRED11 => Ok(RetiredSlotId::R11), - YKPIV_KEY_RETIRED12 => Ok(RetiredSlotId::R12), - YKPIV_KEY_RETIRED13 => Ok(RetiredSlotId::R13), - YKPIV_KEY_RETIRED14 => Ok(RetiredSlotId::R14), - YKPIV_KEY_RETIRED15 => Ok(RetiredSlotId::R15), - YKPIV_KEY_RETIRED16 => Ok(RetiredSlotId::R16), - YKPIV_KEY_RETIRED17 => Ok(RetiredSlotId::R17), - YKPIV_KEY_RETIRED18 => Ok(RetiredSlotId::R18), - YKPIV_KEY_RETIRED19 => Ok(RetiredSlotId::R19), - YKPIV_KEY_RETIRED20 => Ok(RetiredSlotId::R20), + 0x82 => Ok(RetiredSlotId::R1), + 0x83 => Ok(RetiredSlotId::R2), + 0x84 => Ok(RetiredSlotId::R3), + 0x85 => Ok(RetiredSlotId::R4), + 0x86 => Ok(RetiredSlotId::R5), + 0x87 => Ok(RetiredSlotId::R6), + 0x88 => Ok(RetiredSlotId::R7), + 0x89 => Ok(RetiredSlotId::R8), + 0x8a => Ok(RetiredSlotId::R9), + 0x8b => Ok(RetiredSlotId::R10), + 0x8c => Ok(RetiredSlotId::R11), + 0x8d => Ok(RetiredSlotId::R12), + 0x8e => Ok(RetiredSlotId::R13), + 0x8f => Ok(RetiredSlotId::R14), + 0x90 => Ok(RetiredSlotId::R15), + 0x91 => Ok(RetiredSlotId::R16), + 0x92 => Ok(RetiredSlotId::R17), + 0x93 => Ok(RetiredSlotId::R18), + 0x94 => Ok(RetiredSlotId::R19), + 0x95 => Ok(RetiredSlotId::R20), _ => Err(Error::InvalidObject), } } @@ -193,26 +193,26 @@ impl TryFrom for RetiredSlotId { impl From for u8 { fn from(slot: RetiredSlotId) -> u8 { match slot { - RetiredSlotId::R1 => YKPIV_KEY_RETIRED1, - RetiredSlotId::R2 => YKPIV_KEY_RETIRED2, - RetiredSlotId::R3 => YKPIV_KEY_RETIRED3, - RetiredSlotId::R4 => YKPIV_KEY_RETIRED4, - RetiredSlotId::R5 => YKPIV_KEY_RETIRED5, - RetiredSlotId::R6 => YKPIV_KEY_RETIRED6, - RetiredSlotId::R7 => YKPIV_KEY_RETIRED7, - RetiredSlotId::R8 => YKPIV_KEY_RETIRED8, - RetiredSlotId::R9 => YKPIV_KEY_RETIRED9, - RetiredSlotId::R10 => YKPIV_KEY_RETIRED10, - RetiredSlotId::R11 => YKPIV_KEY_RETIRED11, - RetiredSlotId::R12 => YKPIV_KEY_RETIRED12, - RetiredSlotId::R13 => YKPIV_KEY_RETIRED13, - RetiredSlotId::R14 => YKPIV_KEY_RETIRED14, - RetiredSlotId::R15 => YKPIV_KEY_RETIRED15, - RetiredSlotId::R16 => YKPIV_KEY_RETIRED16, - RetiredSlotId::R17 => YKPIV_KEY_RETIRED17, - RetiredSlotId::R18 => YKPIV_KEY_RETIRED18, - RetiredSlotId::R19 => YKPIV_KEY_RETIRED19, - RetiredSlotId::R20 => YKPIV_KEY_RETIRED20, + RetiredSlotId::R1 => 0x82, + RetiredSlotId::R2 => 0x83, + RetiredSlotId::R3 => 0x84, + RetiredSlotId::R4 => 0x85, + RetiredSlotId::R5 => 0x86, + RetiredSlotId::R6 => 0x87, + RetiredSlotId::R7 => 0x88, + RetiredSlotId::R8 => 0x89, + RetiredSlotId::R9 => 0x8a, + RetiredSlotId::R10 => 0x8b, + RetiredSlotId::R11 => 0x8c, + RetiredSlotId::R12 => 0x8d, + RetiredSlotId::R13 => 0x8e, + RetiredSlotId::R14 => 0x8f, + RetiredSlotId::R15 => 0x90, + RetiredSlotId::R16 => 0x91, + RetiredSlotId::R17 => 0x92, + RetiredSlotId::R18 => 0x93, + RetiredSlotId::R19 => 0x94, + RetiredSlotId::R20 => 0x95, } } } @@ -221,26 +221,26 @@ impl RetiredSlotId { /// Returns the [`ObjectId`] that corresponds to a given [`RetiredSlotId`]. pub(crate) fn object_id(self) -> ObjectId { match self { - RetiredSlotId::R1 => YKPIV_OBJ_RETIRED1, - RetiredSlotId::R2 => YKPIV_OBJ_RETIRED2, - RetiredSlotId::R3 => YKPIV_OBJ_RETIRED3, - RetiredSlotId::R4 => YKPIV_OBJ_RETIRED4, - RetiredSlotId::R5 => YKPIV_OBJ_RETIRED5, - RetiredSlotId::R6 => YKPIV_OBJ_RETIRED6, - RetiredSlotId::R7 => YKPIV_OBJ_RETIRED7, - RetiredSlotId::R8 => YKPIV_OBJ_RETIRED8, - RetiredSlotId::R9 => YKPIV_OBJ_RETIRED9, - RetiredSlotId::R10 => YKPIV_OBJ_RETIRED10, - RetiredSlotId::R11 => YKPIV_OBJ_RETIRED11, - RetiredSlotId::R12 => YKPIV_OBJ_RETIRED12, - RetiredSlotId::R13 => YKPIV_OBJ_RETIRED13, - RetiredSlotId::R14 => YKPIV_OBJ_RETIRED14, - RetiredSlotId::R15 => YKPIV_OBJ_RETIRED15, - RetiredSlotId::R16 => YKPIV_OBJ_RETIRED16, - RetiredSlotId::R17 => YKPIV_OBJ_RETIRED17, - RetiredSlotId::R18 => YKPIV_OBJ_RETIRED18, - RetiredSlotId::R19 => YKPIV_OBJ_RETIRED19, - RetiredSlotId::R20 => YKPIV_OBJ_RETIRED20, + RetiredSlotId::R1 => 0x005f_c10d, + RetiredSlotId::R2 => 0x005f_c10e, + RetiredSlotId::R3 => 0x005f_c10f, + RetiredSlotId::R4 => 0x005f_c110, + RetiredSlotId::R5 => 0x005f_c111, + RetiredSlotId::R6 => 0x005f_c112, + RetiredSlotId::R7 => 0x005f_c113, + RetiredSlotId::R8 => 0x005f_c114, + RetiredSlotId::R9 => 0x005f_c115, + RetiredSlotId::R10 => 0x005f_c116, + RetiredSlotId::R11 => 0x005f_c117, + RetiredSlotId::R12 => 0x005f_c118, + RetiredSlotId::R13 => 0x005f_c119, + RetiredSlotId::R14 => 0x005f_c11a, + RetiredSlotId::R15 => 0x005f_c11b, + RetiredSlotId::R16 => 0x005f_c11c, + RetiredSlotId::R17 => 0x005f_c11d, + RetiredSlotId::R18 => 0x005f_c11e, + RetiredSlotId::R19 => 0x005f_c11f, + RetiredSlotId::R20 => 0x005f_c120, } } } From bc95d8b7b91497a2a71d91f5951921043ab22f30 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 30 Nov 2019 22:18:31 +0000 Subject: [PATCH 19/24] Delete unnecessary commented-out code We will handle the CardManagement slot separately. --- src/yubikey.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/yubikey.rs b/src/yubikey.rs index eba6e04..74e6097 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -611,12 +611,6 @@ impl YubiKey { let mut key_data = Zeroizing::new(vec![0u8; 1024]); let templ = [0, Ins::ImportKey.code(), algorithm.into(), key.into()]; - // Only slot we want to exclude is CardManagement, which isn't in the enum. - // TODO: Decide whether to add it or not. - // match key { - // SlotId::CardManagement => return Err(Error::KeyError), - // } - if pin_policy != YKPIV_PINPOLICY_DEFAULT && pin_policy != YKPIV_PINPOLICY_NEVER && pin_policy != YKPIV_PINPOLICY_ONCE From d3e565ef55e3eef9553395ce312879c742696034 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 15:35:00 +0000 Subject: [PATCH 20/24] Derive PartialEq for SlotId --- src/key.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/key.rs b/src/key.rs index 96392c7..b54fa53 100644 --- a/src/key.rs +++ b/src/key.rs @@ -52,7 +52,7 @@ use std::convert::TryFrom; /// Slot identifiers. /// -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum SlotId { /// This certificate and its associated private key is used to authenticate the card /// and the cardholder. This slot is used for things like system login. The end user @@ -136,7 +136,7 @@ impl SlotId { /// Retired slot IDs. #[allow(missing_docs)] -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum RetiredSlotId { R1, R2, From 9ee1494c6ff539cf338da46801825b8a1f9ff2b6 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 15:54:12 +0000 Subject: [PATCH 21/24] Parse RSA public keys within certificates --- Cargo.toml | 4 ++ src/certificate.rs | 141 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 137 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c073861..796cdb9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,15 +19,19 @@ keywords = ["ccid", "ecdsa", "rsa", "piv", "yubikey"] maintenance = { status = "experimental" } [dependencies] +der-parser = "3" des = "0.3" getrandom = "0.1" hmac = "0.7" log = "0.4" +nom = "5" pbkdf2 = "0.3" pcsc = "2" +rsa = "0.1.4" secrecy = "0.5" sha-1 = "0.8" subtle = "2" +x509-parser = "0.6" zeroize = "1" [dev-dependencies] diff --git a/src/certificate.rs b/src/certificate.rs index 6dbf4a5..feeaf2c 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -31,15 +31,67 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::{ - consts::*, error::Error, key::SlotId, serialization::*, transaction::Transaction, - yubikey::YubiKey, Buffer, + consts::*, + error::Error, + key::{AlgorithmId, SlotId}, + serialization::*, + transaction::Transaction, + yubikey::YubiKey, + Buffer, }; use log::error; +use rsa::{PublicKey, RSAPublicKey}; +use x509_parser::{parse_x509_der, x509::SubjectPublicKeyInfo}; use zeroize::Zeroizing; +/// Information about a public key within a [`Certificate`]. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum PublicKeyInfo { + /// RSA keys + Rsa { + /// RSA algorithm + algorithm: AlgorithmId, + + /// Public key + pubkey: RSAPublicKey, + }, +} + +impl PublicKeyInfo { + fn parse(subject_pki: &SubjectPublicKeyInfo<'_>) -> Result { + match subject_pki.algorithm.algorithm.to_string().as_str() { + // RSA encryption + "1.2.840.113549.1.1.1" => { + let pubkey = read_pki::rsa_pubkey(subject_pki.subject_public_key.data)?; + + Ok(PublicKeyInfo::Rsa { + algorithm: match pubkey.n().bits() { + 1024 => AlgorithmId::Rsa1024, + 2048 => AlgorithmId::Rsa2048, + _ => return Err(Error::AlgorithmError), + }, + pubkey, + }) + } + _ => Err(Error::InvalidObject), + } + } + + /// Returns the algorithm that this public key can be used with. + pub fn algorithm(&self) -> AlgorithmId { + match self { + PublicKeyInfo::Rsa { algorithm, .. } => *algorithm, + } + } +} + /// Certificates #[derive(Clone, Debug)] -pub struct Certificate(Buffer); +pub struct Certificate { + subject: String, + subject_pki: PublicKeyInfo, + data: Buffer, +} impl Certificate { /// Read a certificate from the given slot in the YubiKey @@ -51,14 +103,14 @@ impl Certificate { return Err(Error::InvalidObject); } - Ok(Certificate(buf)) + Certificate::new(buf) } /// Write this certificate into the YubiKey in the given slot 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.0), certinfo, max_size) + write_certificate(&txn, slot, Some(&self.data), certinfo, max_size) } /// Delete a certificate located at the given slot of the given YubiKey @@ -77,18 +129,40 @@ impl Certificate { return Err(Error::SizeError); } - Ok(Certificate(cert)) + let parsed_cert = match parse_x509_der(&cert) { + Ok((_, cert)) => cert, + _ => return Err(Error::InvalidObject), + }; + + let subject = format!("{}", parsed_cert.tbs_certificate.subject); + let subject_pki = PublicKeyInfo::parse(&parsed_cert.tbs_certificate.subject_pki)?; + + Ok(Certificate { + subject, + subject_pki, + data: cert, + }) + } + + /// Returns the SubjectName field of the certificate. + pub fn subject(&self) -> &str { + &self.subject + } + + /// Returns the SubjectPublicKeyInfo field of the certificate. + pub fn subject_pki(&self) -> &PublicKeyInfo { + &self.subject_pki } /// Extract the inner buffer pub fn into_buffer(self) -> Buffer { - self.0 + self.data } } impl AsRef<[u8]> for Certificate { fn as_ref(&self) -> &[u8] { - self.0.as_ref() + self.data.as_ref() } } @@ -175,3 +249,54 @@ pub(crate) fn write_certificate( txn.save_object(object_id, &buf[..offset]) } + +mod read_pki { + use der_parser::{ + ber::BerObjectContent, + der::{parse_der_integer, DerObject}, + error::BerError, + *, + }; + use nom::{combinator, IResult}; + use rsa::{BigUint, RSAPublicKey}; + + use crate::error::Error; + + /// From [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.1): + /// ```text + /// RSAPublicKey ::= SEQUENCE { + /// modulus INTEGER, -- n + /// publicExponent INTEGER -- e + /// } + /// ``` + pub(super) fn rsa_pubkey(encoded: &[u8]) -> Result { + fn parse_rsa_pubkey(i: &[u8]) -> IResult<&[u8], DerObject<'_>, BerError> { + parse_der_sequence_defined!(i, parse_der_integer >> parse_der_integer) + } + + fn rsa_pubkey_parts(i: &[u8]) -> IResult<&[u8], (BigUint, BigUint), BerError> { + combinator::map(parse_rsa_pubkey, |object| { + let seq = object.as_sequence().expect("is DER sequence"); + assert_eq!(seq.len(), 2); + + let n = match seq[0].content { + BerObjectContent::Integer(s) => BigUint::from_bytes_be(s), + _ => panic!("expected DER integer"), + }; + let e = match seq[1].content { + BerObjectContent::Integer(s) => BigUint::from_bytes_be(s), + _ => panic!("expected DER integer"), + }; + + (n, e) + })(i) + } + + let (n, e) = match rsa_pubkey_parts(encoded) { + Ok((_, res)) => res, + _ => return Err(Error::InvalidObject), + }; + + RSAPublicKey::new(n, e).map_err(|_| Error::InvalidObject) + } +} From e72ee5c60eb34fd1faf224a04b3eacba61099a01 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 16:54:22 +0000 Subject: [PATCH 22/24] Parse EC public keys within certificates --- src/certificate.rs | 114 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/src/certificate.rs b/src/certificate.rs index feeaf2c..7131fb4 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -44,6 +44,28 @@ use rsa::{PublicKey, RSAPublicKey}; use x509_parser::{parse_x509_der, x509::SubjectPublicKeyInfo}; use zeroize::Zeroizing; +/// An encoded point for some elliptic curve. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum EcPoint { + /// A point in compressed form. + Compressed { + /// EC algorithm + algorithm: AlgorithmId, + + /// Encoded point + bytes: Buffer, + }, + + /// A point in uncompressed form. + Uncompressed { + /// EC algorithm + algorithm: AlgorithmId, + + /// Encoded point + bytes: Buffer, + }, +} + /// Information about a public key within a [`Certificate`]. #[derive(Clone, Debug, Eq, PartialEq)] pub enum PublicKeyInfo { @@ -55,6 +77,9 @@ pub enum PublicKeyInfo { /// Public key pubkey: RSAPublicKey, }, + + /// EC keys + Ec(EcPoint), } impl PublicKeyInfo { @@ -73,6 +98,12 @@ impl PublicKeyInfo { pubkey, }) } + // EC Public Key + "1.2.840.10045.2.1" => { + let algorithm = read_pki::ec_parameters(&subject_pki.algorithm.parameters)?; + read_pki::ec_point(algorithm, subject_pki.subject_public_key.data) + .map(PublicKeyInfo::Ec) + } _ => Err(Error::InvalidObject), } } @@ -81,6 +112,10 @@ impl PublicKeyInfo { pub fn algorithm(&self) -> AlgorithmId { match self { PublicKeyInfo::Rsa { algorithm, .. } => *algorithm, + PublicKeyInfo::Ec(point) => match point { + EcPoint::Compressed { algorithm, .. } => *algorithm, + EcPoint::Uncompressed { algorithm, .. } => *algorithm, + }, } } } @@ -259,8 +294,10 @@ mod read_pki { }; use nom::{combinator, IResult}; use rsa::{BigUint, RSAPublicKey}; + use zeroize::Zeroizing; - use crate::error::Error; + use super::EcPoint; + use crate::{error::Error, key::AlgorithmId}; /// From [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.1): /// ```text @@ -299,4 +336,79 @@ mod read_pki { RSAPublicKey::new(n, e).map_err(|_| Error::InvalidObject) } + + /// From [RFC 5480](https://tools.ietf.org/html/rfc5480#section-2.1.1): + /// ```text + /// ECParameters ::= CHOICE { + /// namedCurve OBJECT IDENTIFIER + /// -- implicitCurve NULL + /// -- specifiedCurve SpecifiedECDomain + /// } + /// ``` + pub(super) fn ec_parameters(parameters: &DerObject<'_>) -> Result { + let curve_oid = match parameters.as_context_specific() { + Ok((_, Some(named_curve))) => { + named_curve.as_oid_val().map_err(|_| Error::InvalidObject) + } + _ => Err(Error::InvalidObject), + }?; + + match curve_oid.to_string().as_str() { + "1.2.840.10045.3.1.7" => Ok(AlgorithmId::EccP256), + "1.3.132.0.34" => Ok(AlgorithmId::EccP384), + _ => Err(Error::InvalidObject), + } + } + + /// From [RFC 5480](https://tools.ietf.org/html/rfc5480#section-2.2): + /// ```text + /// ECPoint ::= OCTET STRING + /// + /// o The first octet of the OCTET STRING indicates whether the key is + /// compressed or uncompressed. The uncompressed form is indicated + /// by 0x04 and the compressed form is indicated by either 0x02 or + /// 0x03 (see 2.3.3 in [SEC1]). The public key MUST be rejected if + /// any other value is included in the first octet. + /// ``` + pub(super) fn ec_point(algorithm: AlgorithmId, encoded: &[u8]) -> Result { + if encoded.is_empty() { + return Err(Error::InvalidObject); + } + + match encoded[0] { + 0x02 | 0x03 => { + if encoded.len() + == match algorithm { + AlgorithmId::EccP256 => 33, + AlgorithmId::EccP384 => 49, + _ => return Err(Error::AlgorithmError), + } + { + Ok(EcPoint::Compressed { + algorithm, + bytes: Zeroizing::new(encoded[1..].to_vec()), + }) + } else { + Err(Error::InvalidObject) + } + } + 0x04 => { + if encoded.len() + == match algorithm { + AlgorithmId::EccP256 => 65, + AlgorithmId::EccP384 => 97, + _ => return Err(Error::AlgorithmError), + } + { + Ok(EcPoint::Uncompressed { + algorithm, + bytes: Zeroizing::new(encoded[1..].to_vec()), + }) + } else { + Err(Error::InvalidObject) + } + } + _ => Err(Error::InvalidObject), + } + } } From 3a283aca4098d28cbd0cf78392f8e4073836b05f Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 18:23:57 +0000 Subject: [PATCH 23/24] Use ecdsa crate for EC point representations --- Cargo.toml | 1 + src/certificate.rs | 147 ++++++++++++++++++++------------------------- 2 files changed, 66 insertions(+), 82 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 796cdb9..b4ded39 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ maintenance = { status = "experimental" } [dependencies] der-parser = "3" des = "0.3" +ecdsa = "0.1" getrandom = "0.1" hmac = "0.7" log = "0.4" diff --git a/src/certificate.rs b/src/certificate.rs index 7131fb4..63cbad8 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -39,35 +39,38 @@ use crate::{ yubikey::YubiKey, Buffer, }; +use ecdsa::{ + curve::{CompressedCurvePoint, NistP256, NistP384, UncompressedCurvePoint}, + generic_array::GenericArray, +}; use log::error; use rsa::{PublicKey, RSAPublicKey}; +use std::fmt; use x509_parser::{parse_x509_der, x509::SubjectPublicKeyInfo}; use zeroize::Zeroizing; -/// An encoded point for some elliptic curve. -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum EcPoint { - /// A point in compressed form. - Compressed { - /// EC algorithm - algorithm: AlgorithmId, +/// An encoded point on the Nist P-256 curve. +#[derive(Clone, Eq, PartialEq)] +pub enum EcP256Point { + /// Compressed encoding of a point on the curve. + Compressed(CompressedCurvePoint), - /// Encoded point - bytes: Buffer, - }, + /// Uncompressed encoding of a point on the curve. + Uncompressed(UncompressedCurvePoint), +} - /// A point in uncompressed form. - Uncompressed { - /// EC algorithm - algorithm: AlgorithmId, +/// An encoded point on the Nist P-384 curve. +#[derive(Clone, Eq, PartialEq)] +pub enum EcP384Point { + /// Compressed encoding of a point on the curve. + Compressed(CompressedCurvePoint), - /// Encoded point - bytes: Buffer, - }, + /// Uncompressed encoding of a point on the curve. + Uncompressed(UncompressedCurvePoint), } /// Information about a public key within a [`Certificate`]. -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Eq, PartialEq)] pub enum PublicKeyInfo { /// RSA keys Rsa { @@ -78,8 +81,17 @@ pub enum PublicKeyInfo { pubkey: RSAPublicKey, }, - /// EC keys - Ec(EcPoint), + /// EC P-256 keys + EcP256(EcP256Point), + + /// EC P-384 keys + EcP384(EcP384Point), +} + +impl fmt::Debug for PublicKeyInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "PublicKeyInfo({:?})", self.algorithm()) + } } impl PublicKeyInfo { @@ -100,9 +112,36 @@ impl PublicKeyInfo { } // EC Public Key "1.2.840.10045.2.1" => { - let algorithm = read_pki::ec_parameters(&subject_pki.algorithm.parameters)?; - read_pki::ec_point(algorithm, subject_pki.subject_public_key.data) - .map(PublicKeyInfo::Ec) + let key_bytes = &subject_pki.subject_public_key.data; + match read_pki::ec_parameters(&subject_pki.algorithm.parameters)? { + AlgorithmId::EccP256 => match key_bytes.len() { + 33 => CompressedCurvePoint::::from_bytes( + GenericArray::clone_from_slice(key_bytes), + ) + .map(EcP256Point::Compressed), + 65 => UncompressedCurvePoint::::from_bytes( + GenericArray::clone_from_slice(key_bytes), + ) + .map(EcP256Point::Uncompressed), + _ => None, + } + .map(PublicKeyInfo::EcP256) + .ok_or(Error::InvalidObject), + AlgorithmId::EccP384 => match key_bytes.len() { + 49 => CompressedCurvePoint::::from_bytes( + GenericArray::clone_from_slice(key_bytes), + ) + .map(EcP384Point::Compressed), + 97 => UncompressedCurvePoint::::from_bytes( + GenericArray::clone_from_slice(key_bytes), + ) + .map(EcP384Point::Uncompressed), + _ => None, + } + .map(PublicKeyInfo::EcP384) + .ok_or(Error::InvalidObject), + _ => Err(Error::AlgorithmError), + } } _ => Err(Error::InvalidObject), } @@ -112,10 +151,8 @@ impl PublicKeyInfo { pub fn algorithm(&self) -> AlgorithmId { match self { PublicKeyInfo::Rsa { algorithm, .. } => *algorithm, - PublicKeyInfo::Ec(point) => match point { - EcPoint::Compressed { algorithm, .. } => *algorithm, - EcPoint::Uncompressed { algorithm, .. } => *algorithm, - }, + PublicKeyInfo::EcP256(_) => AlgorithmId::EccP256, + PublicKeyInfo::EcP384(_) => AlgorithmId::EccP384, } } } @@ -294,9 +331,7 @@ mod read_pki { }; use nom::{combinator, IResult}; use rsa::{BigUint, RSAPublicKey}; - use zeroize::Zeroizing; - use super::EcPoint; use crate::{error::Error, key::AlgorithmId}; /// From [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.1): @@ -356,59 +391,7 @@ mod read_pki { match curve_oid.to_string().as_str() { "1.2.840.10045.3.1.7" => Ok(AlgorithmId::EccP256), "1.3.132.0.34" => Ok(AlgorithmId::EccP384), - _ => Err(Error::InvalidObject), - } - } - - /// From [RFC 5480](https://tools.ietf.org/html/rfc5480#section-2.2): - /// ```text - /// ECPoint ::= OCTET STRING - /// - /// o The first octet of the OCTET STRING indicates whether the key is - /// compressed or uncompressed. The uncompressed form is indicated - /// by 0x04 and the compressed form is indicated by either 0x02 or - /// 0x03 (see 2.3.3 in [SEC1]). The public key MUST be rejected if - /// any other value is included in the first octet. - /// ``` - pub(super) fn ec_point(algorithm: AlgorithmId, encoded: &[u8]) -> Result { - if encoded.is_empty() { - return Err(Error::InvalidObject); - } - - match encoded[0] { - 0x02 | 0x03 => { - if encoded.len() - == match algorithm { - AlgorithmId::EccP256 => 33, - AlgorithmId::EccP384 => 49, - _ => return Err(Error::AlgorithmError), - } - { - Ok(EcPoint::Compressed { - algorithm, - bytes: Zeroizing::new(encoded[1..].to_vec()), - }) - } else { - Err(Error::InvalidObject) - } - } - 0x04 => { - if encoded.len() - == match algorithm { - AlgorithmId::EccP256 => 65, - AlgorithmId::EccP384 => 97, - _ => return Err(Error::AlgorithmError), - } - { - Ok(EcPoint::Uncompressed { - algorithm, - bytes: Zeroizing::new(encoded[1..].to_vec()), - }) - } else { - Err(Error::InvalidObject) - } - } - _ => Err(Error::InvalidObject), + _ => Err(Error::AlgorithmError), } } } From cd704c28d7e2c5906019f5183a926d076bee4803 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 1 Dec 2019 18:42:12 +0000 Subject: [PATCH 24/24] Extract OID strings as constants --- src/certificate.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/certificate.rs b/src/certificate.rs index 63cbad8..e11b303 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -49,6 +49,12 @@ use std::fmt; use x509_parser::{parse_x509_der, x509::SubjectPublicKeyInfo}; use zeroize::Zeroizing; +// TODO: Make these der_parser::oid::Oid constants when it has const fn support. +const OID_RSA_ENCRYPTION: &str = "1.2.840.113549.1.1.1"; +const OID_EC_PUBLIC_KEY: &str = "1.2.840.10045.2.1"; +const OID_NIST_P256: &str = "1.2.840.10045.3.1.7"; +const OID_NIST_P384: &str = "1.3.132.0.34"; + /// An encoded point on the Nist P-256 curve. #[derive(Clone, Eq, PartialEq)] pub enum EcP256Point { @@ -97,8 +103,7 @@ impl fmt::Debug for PublicKeyInfo { impl PublicKeyInfo { fn parse(subject_pki: &SubjectPublicKeyInfo<'_>) -> Result { match subject_pki.algorithm.algorithm.to_string().as_str() { - // RSA encryption - "1.2.840.113549.1.1.1" => { + OID_RSA_ENCRYPTION => { let pubkey = read_pki::rsa_pubkey(subject_pki.subject_public_key.data)?; Ok(PublicKeyInfo::Rsa { @@ -110,8 +115,7 @@ impl PublicKeyInfo { pubkey, }) } - // EC Public Key - "1.2.840.10045.2.1" => { + OID_EC_PUBLIC_KEY => { let key_bytes = &subject_pki.subject_public_key.data; match read_pki::ec_parameters(&subject_pki.algorithm.parameters)? { AlgorithmId::EccP256 => match key_bytes.len() { @@ -332,6 +336,7 @@ mod read_pki { use nom::{combinator, IResult}; use rsa::{BigUint, RSAPublicKey}; + use super::{OID_NIST_P256, OID_NIST_P384}; use crate::{error::Error, key::AlgorithmId}; /// From [RFC 8017](https://tools.ietf.org/html/rfc8017#appendix-A.1.1): @@ -389,8 +394,8 @@ mod read_pki { }?; match curve_oid.to_string().as_str() { - "1.2.840.10045.3.1.7" => Ok(AlgorithmId::EccP256), - "1.3.132.0.34" => Ok(AlgorithmId::EccP384), + OID_NIST_P256 => Ok(AlgorithmId::EccP256), + OID_NIST_P384 => Ok(AlgorithmId::EccP384), _ => Err(Error::AlgorithmError), } }