james/yubikey.rs
Public Access
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Create typed structs for PIN-protected and admin metadata (#223)

Browse Source 
MgmKey::set_protected and YubiKey::set_pin_last_changed both contained
bugs resulting from the conversion of C pointer logic (incorrect buffer
management). The new Metadata struct holds its own buffer, avoiding the
problem.

Also adds a protected management key integration test.
This commit is contained in:
str4d
2021-02-01 06:54:13 +13:00
committed by GitHub
parent 37088bba56
commit 9d1da84233
6 changed files with 261 additions and 209 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/config.rs
+10 -10
View File
@@ -32,11 +32,11 @@
use crate::{
error::Error,
metadata,
metadata::{AdminData, ProtectedData},
mgm::{MgmType, ADMIN_FLAGS_1_PROTECTED_MGM},
yubikey::{YubiKey, ADMIN_FLAGS_1_PUK_BLOCKED},
TAG_ADMIN, TAG_ADMIN_FLAGS_1, TAG_ADMIN_SALT, TAG_ADMIN_TIMESTAMP, TAG_PROTECTED,
TAG_PROTECTED_FLAGS_1, TAG_PROTECTED_MGM,
TAG_ADMIN_FLAGS_1, TAG_ADMIN_SALT, TAG_ADMIN_TIMESTAMP, TAG_PROTECTED_FLAGS_1,
TAG_PROTECTED_MGM,
};
use log::error;
use std::{
@@ -79,8 +79,8 @@ impl Config {
let txn = yubikey.begin_transaction()?;
if let Ok(data) = metadata::read(&txn, TAG_ADMIN) {
if let Ok(item) = metadata::get_item(&data, TAG_ADMIN_FLAGS_1) {
if let Ok(admin_data) = AdminData::read(&txn) {
if let Ok(item) = admin_data.get_item(TAG_ADMIN_FLAGS_1) {
if item.is_empty() {
error!("empty response for admin flags metadata item! ignoring");
} else {
@@ -94,7 +94,7 @@ impl Config {
}
}
if metadata::get_item(&data, TAG_ADMIN_SALT).is_ok() {
if admin_data.get_item(TAG_ADMIN_SALT).is_ok() {
if config.mgm_type != MgmType::Manual {
error!("conflicting types of MGM key administration configured");
} else {
@@ -102,7 +102,7 @@ impl Config {
}
}
if let Ok(item) = metadata::get_item(&data, TAG_ADMIN_TIMESTAMP) {
if let Ok(item) = admin_data.get_item(TAG_ADMIN_TIMESTAMP) {
if item.len() != CB_ADMIN_TIMESTAMP {
error!("pin timestamp in admin metadata is an invalid size");
} else {
@@ -117,10 +117,10 @@ impl Config {
}
}
if let Ok(data) = metadata::read(&txn, TAG_PROTECTED) {
if let Ok(protected_data) = ProtectedData::read(&txn) {
config.protected_data_available = true;
if let Ok(item) = metadata::get_item(&data, TAG_PROTECTED_FLAGS_1) {
if let Ok(item) = protected_data.get_item(TAG_PROTECTED_FLAGS_1) {
if item.is_empty() {
error!("empty response for protected flags metadata item! ignoring");
} else if item[0] & PROTECTED_FLAGS_1_PUK_NOBLOCK != 0 {
@@ -128,7 +128,7 @@ impl Config {
}
}
if metadata::get_item(&data, TAG_PROTECTED_MGM).is_ok() {
if protected_data.get_item(TAG_PROTECTED_MGM).is_ok() {
if config.mgm_type != MgmType::Protected {
error!(
"conflicting types of mgm key administration configured: protected MGM exists"
src/lib.rs
-2
View File
@@ -178,11 +178,9 @@ pub(crate) const CB_OBJ_TAG_MIN: usize = 2; // 1 byte tag + 1 byte len
#[cfg(feature = "untested")]
pub(crate) const CB_OBJ_TAG_MAX: usize = CB_OBJ_TAG_MIN + 2; // 1 byte tag + 3 bytes len
pub(crate) const TAG_ADMIN: u8 = 0x80;
pub(crate) const TAG_ADMIN_FLAGS_1: u8 = 0x81;
pub(crate) const TAG_ADMIN_SALT: u8 = 0x82;
pub(crate) const TAG_ADMIN_TIMESTAMP: u8 = 0x83;
pub(crate) const TAG_PROTECTED: u8 = 0x88;
pub(crate) const TAG_PROTECTED_FLAGS_1: u8 = 0x81;
pub(crate) const TAG_PROTECTED_MGM: u8 = 0x89;
src/metadata.rs
+124 -66
View File
@@ -30,21 +30,89 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
use crate::{
error::Error, serialization::*, transaction::Transaction, Buffer, TAG_ADMIN, TAG_PROTECTED,
};
use std::marker::PhantomData;
use zeroize::Zeroizing;
use crate::{error::Error, serialization::*, transaction::Transaction, Buffer};
#[cfg(feature = "untested")]
use crate::{CB_OBJ_MAX, CB_OBJ_TAG_MAX};
#[cfg(feature = "untested")]
use zeroize::Zeroizing;
use std::iter;
const TAG_ADMIN: u8 = 0x80;
const TAG_PROTECTED: u8 = 0x88;
pub const OBJ_ADMIN_DATA: u32 = 0x005f_ff00;
pub const OBJ_PRINTED: u32 = 0x005f_c109;
pub(crate) trait MetadataType: private::Sealed {}
/// A type variable corresponding to PIN-protected metadata.
pub(crate) enum Protected {}
impl MetadataType for Protected {}
/// A type variable corresponding to administrative metadata.
pub(crate) enum Admin {}
impl MetadataType for Admin {}
/// Metadata stored in a YubiKey.
pub(crate) struct Metadata<T: MetadataType> {
inner: Buffer,
_marker: PhantomData<T>,
}
/// PIN-protected metadata stored in a YubiKey.
pub(crate) type ProtectedData = Metadata<Protected>;
/// Administrative metadata stored in a YubiKey.
pub(crate) type AdminData = Metadata<Admin>;
impl<T: MetadataType> Default for Metadata<T> {
fn default() -> Self {
Metadata {
inner: Zeroizing::new(vec![]),
_marker: PhantomData::default(),
}
}
}
impl<T: MetadataType> Metadata<T> {
/// Read metadata
pub(crate) fn read(txn: &Transaction<'_>) -> Result<Self, Error> {
let data = txn.fetch_object(T::obj_id())?;
Ok(Metadata {
inner: Tlv::parse_single(data, T::tag())?,
_marker: PhantomData::default(),
})
}
/// Write metadata
#[cfg(feature = "untested")]
pub(crate) fn write(&self, txn: &Transaction<'_>) -> Result<(), Error> {
if self.inner.len() > CB_OBJ_MAX - CB_OBJ_TAG_MAX {
return Err(Error::GenericError);
}
if self.inner.is_empty() {
return Self::delete(txn);
}
let mut buf = Zeroizing::new(vec![0u8; CB_OBJ_MAX]);
let len = Tlv::write(&mut buf, T::tag(), &self.inner)?;
txn.save_object(T::obj_id(), &buf[..len])
}
/// Delete metadata
#[cfg(feature = "untested")]
pub(crate) fn delete(txn: &Transaction<'_>) -> Result<(), Error> {
txn.save_object(T::obj_id(), &[])
}
/// Get metadata item
pub(crate) fn get_item(mut data: &[u8], tag: u8) -> Result<&[u8], Error> {
pub(crate) fn get_item(&self, tag: u8) -> Result<&[u8], Error> {
let mut data = &self.inner[..];
while !data.is_empty() {
let (remaining, tlv) = Tlv::parse(data)?;
data = remaining;
@@ -60,25 +128,18 @@ pub(crate) fn get_item(mut data: &[u8], tag: u8) -> Result<&[u8], Error> {
/// Set metadata item
#[cfg(feature = "untested")]
pub(crate) fn set_item(
data: &mut [u8],
pcb_data: &mut usize,
cb_data_max: usize,
tag: u8,
p_item: &[u8],
) -> Result<(), Error> {
pub(crate) fn set_item(&mut self, tag: u8, item: &[u8]) -> Result<(), Error> {
let mut cb_temp: usize = 0;
let mut tag_temp: u8 = 0;
let mut cb_len: usize = 0;
let cb_item = p_item.len();
let mut offset = 0;
while offset < *pcb_data {
tag_temp = data[offset];
while offset < self.inner.len() {
tag_temp = self.inner[offset];
offset += 1;
cb_len = get_length(&data[offset..], &mut cb_temp);
cb_len = get_length(&self.inner[offset..], &mut cb_temp);
offset += cb_len;
if tag_temp == tag {
@@ -89,13 +150,16 @@ pub(crate) fn set_item(
}
if tag_temp != tag {
if cb_item == 0 {
if item.is_empty() {
// We've been asked to delete an existing item that isn't in the blob
return Ok(());
}
// We did not find an existing tag, append
*pcb_data += Tlv::write(&mut data[*pcb_data..], tag, p_item)?;
assert_eq!(offset, self.inner.len());
self.inner
.extend(iter::repeat(0).take(1 + get_length_size(item.len()) + item.len()));
Tlv::write(&mut self.inner[offset..], tag, item)?;
return Ok(());
}
@@ -103,80 +167,50 @@ pub(crate) fn set_item(
// Found tag
// Check length, if it matches, overwrite
if cb_temp == cb_item {
data[offset..offset + cb_item].copy_from_slice(p_item);
if cb_temp == item.len() {
self.inner[offset..offset + item.len()].copy_from_slice(item);
return Ok(());
}
// 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 {
let cb_moved: isize = (item.len() as isize - cb_temp as isize)
+ if item.is_empty() {
// For tag, if deleting
-1
} else {
get_length_size(item.len()) as isize
}
// Accounts for different length encoding
- cb_len as isize;
// If length would cause buffer overflow, return error
if (*pcb_data as isize + cb_moved) as usize > cb_data_max {
if (self.inner.len() as isize + cb_moved) as usize > CB_OBJ_MAX {
return Err(Error::GenericError);
}
// Move remaining data
data.copy_within(
next_offset..*pcb_data,
let orig_len = self.inner.len();
if cb_moved > 0 {
self.inner.extend(iter::repeat(0).take(cb_moved as usize));
}
self.inner.copy_within(
next_offset..orig_len,
(next_offset as isize + cb_moved) as usize,
);
*pcb_data = (*pcb_data as isize + cb_moved) as usize;
self.inner
.resize((orig_len as isize + cb_moved) as usize, 0);
// Re-encode item and insert
if cb_item != 0 {
if !item.is_empty() {
offset -= cb_len;
offset += set_length(&mut data[offset..], cb_item)?;
data[offset..offset + cb_item].copy_from_slice(p_item);
offset += set_length(&mut self.inner[offset..], item.len())?;
self.inner[offset..offset + item.len()].copy_from_slice(item);
}
Ok(())
}
/// Read metadata
pub(crate) fn read(txn: &Transaction<'_>, tag: u8) -> Result<Buffer, Error> {
let obj_id = match tag {
TAG_ADMIN => OBJ_ADMIN_DATA,
TAG_PROTECTED => OBJ_PRINTED,
_ => return Err(Error::InvalidObject),
};
let data = txn.fetch_object(obj_id)?;
Tlv::parse_single(data, tag)
}
/// Write metadata
#[cfg(feature = "untested")]
pub(crate) fn write(txn: &Transaction<'_>, tag: u8, data: &[u8]) -> Result<(), Error> {
if data.len() > CB_OBJ_MAX - CB_OBJ_TAG_MAX {
return Err(Error::GenericError);
}
let obj_id = match tag {
TAG_ADMIN => OBJ_ADMIN_DATA,
TAG_PROTECTED => OBJ_PRINTED,
_ => return Err(Error::InvalidObject),
};
if data.is_empty() {
// Deleting metadata
return txn.save_object(obj_id, &[]);
}
let mut buf = Zeroizing::new(vec![0u8; CB_OBJ_MAX]);
let len = Tlv::write(&mut buf, tag, data)?;
txn.save_object(obj_id, &buf[..len])
}
/// Get the size of a length tag for the given length
@@ -190,3 +224,27 @@ fn get_length_size(length: usize) -> usize {
3
}
}
mod private {
use super::*;
pub trait Sealed {
fn tag() -> u8;
fn obj_id() -> u32;
}
impl Sealed for Protected {
fn tag() -> u8 {
TAG_PROTECTED
}
fn obj_id() -> u32 {
OBJ_PRINTED
}
}
impl Sealed for Admin {
fn tag() -> u8 {
TAG_ADMIN
}
fn obj_id() -> u32 {
OBJ_ADMIN_DATA
}
}
}
src/mgm.rs
+21 -42
View File
@@ -38,8 +38,9 @@ use zeroize::{Zeroize, Zeroizing};
#[cfg(feature = "untested")]
use crate::{
metadata, yubikey::YubiKey, CB_BUF_MAX, CB_OBJ_MAX, TAG_ADMIN, TAG_ADMIN_FLAGS_1,
TAG_ADMIN_SALT, TAG_PROTECTED, TAG_PROTECTED_MGM,
metadata::{AdminData, ProtectedData},
yubikey::YubiKey,
TAG_ADMIN_FLAGS_1, TAG_ADMIN_SALT, TAG_PROTECTED_MGM,
};
use des::{
cipher::{generic_array::GenericArray, BlockCipher, NewBlockCipher},
@@ -135,8 +136,8 @@ impl MgmKey {
let txn = yubikey.begin_transaction()?;
// recover management key
let data = metadata::read(&txn, TAG_ADMIN)?;
let salt = metadata::get_item(&data, TAG_ADMIN_SALT)?;
let admin_data = AdminData::read(&txn)?;
let salt = admin_data.get_item(TAG_ADMIN_SALT)?;
if salt.len() != CB_ADMIN_SALT {
error!(
@@ -159,12 +160,12 @@ impl MgmKey {
pub fn get_protected(yubikey: &mut YubiKey) -> Result<Self, Error> {
let txn = yubikey.begin_transaction()?;
let data = metadata::read(&txn, TAG_PROTECTED).map_err(|e| {
let protected_data = ProtectedData::read(&txn).map_err(|e| {
error!("could not read protected data (err: {:?})", e);
e
})?;
let item = metadata::get_item(&data, TAG_PROTECTED_MGM).map_err(|e| {
let item = protected_data.get_item(TAG_PROTECTED_MGM).map_err(|e| {
error!("could not read protected MGM from metadata (err: {:?})", e);
e
})?;
@@ -192,8 +193,6 @@ impl MgmKey {
/// Set protected management key (MGM)
#[cfg(feature = "untested")]
pub fn set_protected(&self, yubikey: &mut YubiKey) -> Result<(), Error> {
let mut data = Zeroizing::new(vec![0u8; CB_BUF_MAX]);
let txn = yubikey.begin_transaction()?;
txn.set_mgm_key(self, None).map_err(|e| {
@@ -206,39 +205,25 @@ impl MgmKey {
// after this point, we've set the mgm key, so the function should
// succeed, regardless of being able to set the metadata
// set the new mgm key in protected data
let buffer = match metadata::read(&txn, TAG_PROTECTED) {
Ok(b) => b,
Err(_) => {
// set current metadata blob size to zero, we'll add to the blank blob
Zeroizing::new(vec![])
}
};
let mut cb_data = buffer.len();
data[..cb_data].copy_from_slice(&buffer);
// Fetch the current protected data, or start a blank metadata blob.
let mut protected_data = ProtectedData::read(&txn).unwrap_or_default();
if let Err(e) = metadata::set_item(
data.as_mut_slice(),
&mut cb_data,
CB_OBJ_MAX,
TAG_PROTECTED_MGM,
self.as_ref(),
) {
// Set the new mgm key in protected data.
if let Err(e) = protected_data.set_item(TAG_PROTECTED_MGM, self.as_ref()) {
error!("could not set protected mgm item, err = {:?}", e);
} else {
metadata::write(&txn, TAG_PROTECTED, &data).map_err(|e| {
protected_data.write(&txn).map_err(|e| {
error!("could not write protected data, err = {:?}", e);
e
})?;
}
// set the protected mgm flag in admin data
cb_data = data.len();
let mut flags_1 = [0u8; 1];
if let Ok(buffer) = metadata::read(&txn, TAG_ADMIN) {
if let Ok(item) = metadata::get_item(&buffer, TAG_ADMIN_FLAGS_1) {
let mut admin_data = if let Ok(mut admin_data) = AdminData::read(&txn) {
if let Ok(item) = admin_data.get_item(TAG_ADMIN_FLAGS_1) {
if item.len() == flags_1.len() {
flags_1.copy_from_slice(item);
} else {
@@ -254,26 +239,20 @@ impl MgmKey {
}
// remove any existing salt
if let Err(e) =
metadata::set_item(&mut data, &mut cb_data, CB_OBJ_MAX, TAG_ADMIN_SALT, &[])
{
if let Err(e) = admin_data.set_item(TAG_ADMIN_SALT, &[]) {
error!("could not unset derived mgm salt (err = {})", e)
}
admin_data
} else {
cb_data = 0;
}
AdminData::default()
};
flags_1[0] |= ADMIN_FLAGS_1_PROTECTED_MGM;
if let Err(e) = metadata::set_item(
data.as_mut_slice(),
&mut cb_data,
CB_OBJ_MAX,
TAG_ADMIN_FLAGS_1,
&flags_1,
) {
if let Err(e) = admin_data.set_item(TAG_ADMIN_FLAGS_1, &flags_1) {
error!("could not set admin flags item, err = {}", e);
} else if let Err(e) = metadata::write(&txn, TAG_ADMIN, &data[..cb_data]) {
} else if let Err(e) = admin_data.write(&txn) {
error!("could not write admin data, err = {}", e);
}
src/yubikey.rs
+17 -29
View File
@@ -50,8 +50,8 @@ use std::{
#[cfg(feature = "untested")]
use crate::{
apdu::StatusWords, metadata, transaction::ChangeRefAction, Buffer, ObjectId, CB_BUF_MAX,
CB_OBJ_MAX, MGMT_AID, TAG_ADMIN, TAG_ADMIN_FLAGS_1, TAG_ADMIN_TIMESTAMP,
apdu::StatusWords, metadata::AdminData, transaction::ChangeRefAction, Buffer, ObjectId,
MGMT_AID, TAG_ADMIN_FLAGS_1, TAG_ADMIN_TIMESTAMP,
};
use getrandom::getrandom;
#[cfg(feature = "untested")]
@@ -417,12 +417,9 @@ impl YubiKey {
/// Set PIN last changed
#[cfg(feature = "untested")]
pub fn set_pin_last_changed(yubikey: &mut YubiKey) -> Result<(), Error> {
let mut data = [0u8; CB_BUF_MAX];
let txn = yubikey.begin_transaction()?;
let buffer = metadata::read(&txn, TAG_ADMIN)?;
let mut cb_data = buffer.len();
data[..cb_data].copy_from_slice(&buffer);
let mut admin_data = AdminData::read(&txn)?;
// TODO(tarcieri): double check this is little endian
let tnow = SystemTime::now()
@@ -431,19 +428,14 @@ impl YubiKey {
.as_secs()
.to_le_bytes();
metadata::set_item(
&mut data,
&mut cb_data,
CB_OBJ_MAX,
TAG_ADMIN_TIMESTAMP,
&tnow,
)
admin_data
.set_item(TAG_ADMIN_TIMESTAMP, &tnow)
.map_err(|e| {
error!("could not set pin timestamp, err = {}", e);
e
})?;
metadata::write(&txn, TAG_ADMIN, &data).map_err(|e| {
admin_data.write(&txn).map_err(|e| {
error!("could not write admin data, err = {}", e);
e
})?;
@@ -494,8 +486,10 @@ impl YubiKey {
}
}
if let Ok(data) = metadata::read(&txn, TAG_ADMIN) {
if let Ok(item) = metadata::get_item(&data, TAG_ADMIN_FLAGS_1) {
// Attempt to set the "PUK blocked" flag in admin data.
let mut admin_data = if let Ok(admin_data) = AdminData::read(&txn) {
if let Ok(item) = admin_data.get_item(TAG_ADMIN_FLAGS_1) {
if item.len() == flags.len() {
flags.copy_from_slice(item)
} else {
@@ -506,22 +500,16 @@ impl YubiKey {
);
}
}
}
admin_data
} else {
AdminData::default()
};
flags[0] |= ADMIN_FLAGS_1_PUK_BLOCKED;
let mut data = [0u8; CB_BUF_MAX];
let mut cb_data: usize = data.len();
if metadata::set_item(
&mut data,
&mut cb_data,
CB_OBJ_MAX,
TAG_ADMIN_FLAGS_1,
&flags,
)
.is_ok()
{
if metadata::write(&txn, TAG_ADMIN, &data[..cb_data]).is_err() {
if admin_data.set_item(TAG_ADMIN_FLAGS_1, &flags).is_ok() {
if admin_data.write(&txn).is_err() {
error!("could not write admin metadata");
}
} else {
tests/integration.rs
+29
View File
@@ -107,6 +107,35 @@ fn test_verify_pin() {
assert!(yubikey.verify_pin(b"123456").is_ok());
}
//
// Management key support
//
#[cfg(feature = "untested")]
#[test]
#[ignore]
fn test_protected_mgmkey() {
let mut yubikey = YUBIKEY.lock().unwrap();
assert!(yubikey.verify_pin(b"123456").is_ok());
assert!(yubikey.authenticate(MgmKey::default()).is_ok());
// Set a protected management key.
assert!(MgmKey::generate()
.unwrap()
.set_protected(&mut yubikey)
.is_ok());
let protected = MgmKey::get_protected(&mut yubikey).unwrap();
assert!(yubikey.authenticate(MgmKey::default()).is_err());
assert!(yubikey.authenticate(protected.clone()).is_ok());
// Set back to the default management key.
// TODO: This does not clear the previous key from the protected metadata.
assert!(MgmKey::default().set(&mut yubikey, None).is_ok());
assert!(yubikey.authenticate(protected).is_err());
assert!(yubikey.authenticate(MgmKey::default()).is_ok());
}
//
// Certificate support
//