Merge pull request #37 from str4d/safety-rails

Safety rails
This commit is contained in:
Tony Arcieri
2019-11-29 09:56:54 -08:00
committed by GitHub
5 changed files with 145 additions and 296 deletions
+1 -5
View File
@@ -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<Bu
return Ok(Zeroizing::new(vec![]));
}
unsafe {
ptr::copy(buf.as_ptr().add(offset), buf.as_mut_ptr(), len);
}
buf.copy_within(offset..offset + len, 0);
buf.truncate(len);
}
+48 -85
View File
@@ -31,49 +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 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,
) {
if !has_valid_length(&data[offset..], data.len() - 1) {
return Err(Error::SizeError);
}
p_temp = p_temp.add(get_length(p_slice, &mut cb_temp));
offset += get_length(&data[offset..], &mut cb_temp);
if tag_temp == tag {
// found tag
break;
}
p_temp = p_temp.add(cb_temp);
}
if p_temp < data.as_ptr().add(data.len()) {
return Ok(slice::from_raw_parts(p_temp, cb_temp));
}
offset += cb_temp;
}
if offset < data.len() {
Ok(&data[offset..offset + cb_temp])
} else {
Err(Error::GenericError)
}
}
/// Set metadata item
pub(crate) fn set_item(
@@ -83,35 +71,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;
cb_len = get_length(&data[offset..], &mut cb_temp);
offset += cb_len;
if tag_temp == tag {
break;
}
p_temp = p_temp.add(cb_temp);
}
offset += cb_temp;
}
if tag_temp != tag {
@@ -120,75 +98,63 @@ pub(crate) fn set_item(
return Ok(());
}
unsafe {
p_temp = data.as_mut_ptr().add(*pcb_data);
// We did not find an existing tag, append
offset = *pcb_data;
cb_len = get_length_size(cb_item);
// If length would cause buffer overflow, return error
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);
}
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,
// Move remaining data
data.copy_within(
next_offset..*pcb_data,
(next_offset as isize + cb_moved) as usize,
);
}
*pcb_data += 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(())
@@ -219,10 +185,7 @@ pub(crate) fn read(txn: &Transaction<'_>, tag: u8) -> Result<Buffer, Error> {
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)
}
+15 -37
View File
@@ -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<u8>);
@@ -51,33 +50,22 @@ impl MsRoots {
}
/// Read `msroots` file from YubiKey
pub fn read(yubikey: &mut YubiKey) -> Result<Vec<Self>, 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<Option<Self>, 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
+3 -28
View File
@@ -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)?
+63 -126
View File
@@ -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,115 @@ 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);
}
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);
}
(
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;
YKPIV_ALGO_RSA1024 => 64,
YKPIV_ALGO_RSA2048 => 128,
_ => unreachable!(),
},
vec![p, q, dp, dq, qinv],
0x01,
)
}
if algorithm == YKPIV_ALGO_RSA2048 {
elem_len = 128;
}
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;
}
}
YKPIV_ALGO_ECCP256 | YKPIV_ALGO_ECCP384 => {
if ec_data_len >= key_data.len() {
_ => 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 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;
(
match algorithm {
YKPIV_ALGO_ECCP256 => 32,
YKPIV_ALGO_ECCP384 => 48,
_ => unreachable!(),
},
vec![ec_data],
0x06,
)
}
_ => 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 {