From 8240575bb48c55a6a0fd2d9dd7d7b12af451c44c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 28 Nov 2019 23:44:16 +0000 Subject: [PATCH] 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 {