Skip to content

Commit

Permalink
Add clippy::unwrap_used lint
Browse files Browse the repository at this point in the history
Lints for usages of `unwrap()` in the `yubikey` crate (not CLI yet).

Replaces them with `?` or `expect()` as the situation warrants.
  • Loading branch information
tony-iqlusion committed Aug 15, 2023
1 parent d226209 commit 477656e
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 50 deletions.
10 changes: 3 additions & 7 deletions src/cccid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
// (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, Result, YubiKey};
use crate::{Result, YubiKey};
use rand_core::{OsRng, RngCore};
use std::fmt::{self, Debug, Display};

Expand All @@ -48,6 +48,7 @@ const OBJ_CAPABILITY: u32 = 0x005f_c107;
/// - 0xff == Manufacturer ID (dummy)
/// - 0x02 == Card type (javaCard)
/// - next 14 bytes: card ID
#[allow(dead_code)]
const CCC_TMPL: &[u8] = &[
0xf0, 0x15, 0xa0, 0x00, 0x00, 0x01, 0x16, 0xff, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf1, 0x01, 0x21, 0xf2, 0x01, 0x21, 0xf3, 0x00, 0xf4,
Expand Down Expand Up @@ -90,12 +91,7 @@ impl CccId {
pub fn get(yubikey: &mut YubiKey) -> Result<Self> {
let txn = yubikey.begin_transaction()?;
let response = txn.fetch_object(OBJ_CAPABILITY)?;

if response.len() != CCC_TMPL.len() {
return Err(Error::GenericError);
}

Ok(Self(response[..Self::BYTE_SIZE].try_into().unwrap()))
Ok(response[..Self::BYTE_SIZE].try_into().map(Self)?)
}

/// Set Cardholder Capability Container (CCC) ID
Expand Down
22 changes: 10 additions & 12 deletions src/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,20 +223,18 @@ pub(crate) fn write_certificate(
) -> Result<()> {
let object_id = slot.object_id();

if data.is_none() {
return txn.save_object(object_id, &[]);
}

let data = data.unwrap();

let mut buf = [0u8; CB_OBJ_MAX];
let mut offset = Tlv::write(&mut buf, TAG_CERT, data)?;
if let Some(data) = data {
let mut buf = [0u8; CB_OBJ_MAX];
let mut offset = Tlv::write(&mut buf, TAG_CERT, data)?;

// write compression info and LRC trailer
offset += Tlv::write(&mut buf[offset..], TAG_CERT_COMPRESS, &[certinfo.into()])?;
offset += Tlv::write(&mut buf[offset..], TAG_CERT_LRC, &[])?;
// write compression info and LRC trailer
offset += Tlv::write(&mut buf[offset..], TAG_CERT_COMPRESS, &[certinfo.into()])?;
offset += Tlv::write(&mut buf[offset..], TAG_CERT_LRC, &[])?;

txn.save_object(object_id, &buf[..offset])
txn.save_object(object_id, &buf[..offset])
} else {
txn.save_object(object_id, &[])
}
}

pub mod yubikey_signer {
Expand Down
17 changes: 7 additions & 10 deletions src/chuid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
// (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, Result, YubiKey};
use crate::{Result, YubiKey};
use std::fmt::{self, Debug, Display};
use uuid::Uuid;

Expand Down Expand Up @@ -61,6 +61,7 @@ const OBJ_CHUID: u32 = 0x005f_c102;
/// - 0x35: Exp. Date (hard-coded)
/// - 0x3e: Signature (hard-coded, empty)
/// - 0xfe: Error Detection Code (hard-coded)
#[allow(dead_code)]
const CHUID_TMPL: &[u8] = &[
0x30, 0x19, 0xd4, 0xe7, 0x39, 0xda, 0x73, 0x9c, 0xed, 0x39, 0xce, 0x73, 0x9d, 0x83, 0x68, 0x58,
0x21, 0x08, 0x42, 0x10, 0x84, 0x21, 0xc8, 0x42, 0x10, 0xc3, 0xeb, 0x34, 0x10, 0x00, 0x00, 0x00,
Expand All @@ -86,32 +87,28 @@ impl ChuId {
pub fn fascn(&self) -> [u8; Self::FASCN_SIZE] {
self.0[CHUID_FASCN_OFFS..(CHUID_FASCN_OFFS + Self::FASCN_SIZE)]
.try_into()
.unwrap()
.expect("should be FASCN_SIZE")
}

/// Return Card UUID/GUID component of CHUID
pub fn uuid(&self) -> Uuid {
Uuid::from_slice(&self.0[CHUID_GUID_OFFS..(CHUID_GUID_OFFS + 16)]).unwrap()
Uuid::from_slice(&self.0[CHUID_GUID_OFFS..(CHUID_GUID_OFFS + 16)])
.expect("should be UUID-sized")
}

/// Return expiration date component of CHUID
// TODO(tarcieri): parse expiration?
pub fn expiration(&self) -> [u8; Self::EXPIRATION_SIZE] {
self.0[CHUID_EXPIRATION_OFFS..(CHUID_EXPIRATION_OFFS + Self::EXPIRATION_SIZE)]
.try_into()
.unwrap()
.expect("should be EXPIRATION_SIZE")
}

/// Get Cardholder Unique Identifier (CHUID)
pub fn get(yubikey: &mut YubiKey) -> Result<ChuId> {
let txn = yubikey.begin_transaction()?;
let response = txn.fetch_object(OBJ_CHUID)?;

if response.len() != CHUID_TMPL.len() {
return Err(Error::GenericError);
}

Ok(ChuId(response[..Self::BYTE_SIZE].try_into().unwrap()))
Ok(response[..Self::BYTE_SIZE].try_into().map(Self)?)
}

/// Set Cardholder Unique Identifier (CHUID)
Expand Down
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl Config {
error!("pin timestamp in admin metadata is an invalid size");
} else {
// TODO(tarcieri): double-check endianness is correct
let pin_last_changed = u32::from_le_bytes(item.try_into().unwrap());
let pin_last_changed = u32::from_le_bytes([item[0], item[1], item[2], item[3]]);

if pin_last_changed != 0 {
config.pin_last_changed =
Expand Down
14 changes: 14 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,26 @@ impl Display for Error {
}
}

impl From<std::array::TryFromSliceError> for Error {
fn from(_: std::array::TryFromSliceError) -> Error {
Error::SizeError
}
}

impl From<std::time::SystemTimeError> for Error {
fn from(_: std::time::SystemTimeError) -> Error {
Error::GenericError
}
}

impl From<pcsc::Error> for Error {
fn from(err: pcsc::Error) -> Error {
Error::PcscError { inner: Some(err) }
}
}



impl std::error::Error for Error {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Expand Down
9 changes: 8 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@
)]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![forbid(unsafe_code)]
#![warn(missing_docs, rust_2018_idioms, trivial_casts, unused_qualifications)]
#![warn(
clippy::mod_module_files,
clippy::unwrap_used,
missing_docs,
rust_2018_idioms,
unused_lifetimes,
unused_qualifications
)]

// Adapted from yubico-piv-tool:
// <https://github.com/Yubico/yubico-piv-tool/>
Expand Down
16 changes: 9 additions & 7 deletions src/mscmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl MsContainer {
let name_bytes_len = Self::NAME_LEN * 2;

for (i, chunk) in bytes[..name_bytes_len].chunks_exact(2).enumerate() {
name[i] = u16::from_le_bytes(chunk.try_into().unwrap());
name[i] = u16::from_le_bytes([chunk[0], chunk[1]]);
}

let mut cert_fingerprint = [0u8; 20];
Expand All @@ -150,11 +150,10 @@ impl MsContainer {
name,
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)]
.try_into()
.unwrap(),
),
key_size_bits: u16::from_le_bytes([
bytes[name_bytes_len + 2],
bytes[name_bytes_len + 3],
]),
flags: bytes[name_bytes_len + 4],
pin_id: bytes[name_bytes_len + 5],
associated_echd_container: bytes[name_bytes_len + 6],
Expand Down Expand Up @@ -183,7 +182,10 @@ impl MsContainer {
bytes.push(self.pin_id);
bytes.push(self.associated_echd_container);
bytes.extend_from_slice(&self.cert_fingerprint);
bytes.as_slice().try_into().unwrap()
bytes
.as_slice()
.try_into()
.expect("should be REC_LEN-sized")
}
}

Expand Down
1 change: 1 addition & 0 deletions src/piv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ impl RsaKeyData {
/// Generates a new RSA key data set from two randomly generated, secret, primes.
///
/// Panics if `secret_p` or `secret_q` are invalid primes.
#[allow(clippy::unwrap_used)] // TODO(tarcieri): make fallible and handle errors
pub fn new(secret_p: &[u8], secret_q: &[u8]) -> Self {
let p = BigUint::from_bytes_be(secret_p);
let q = BigUint::from_bytes_be(secret_q);
Expand Down
8 changes: 5 additions & 3 deletions src/reader.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Support for enumerating available PC/SC card readers.

use crate::{Result, YubiKey};
use crate::{Result, YubiKey, Error};
use std::{
borrow::Cow,
ffi::CStr,
Expand Down Expand Up @@ -43,7 +43,8 @@ impl Context {
let Self { ctx, reader_names } = self;

let reader_cstrs: Vec<_> = {
let c = ctx.lock().unwrap();
// TODO(tarcieri): better error?
let c = ctx.lock().map_err(|_| Error::GenericError)?;

// ensure PC/SC context is valid
c.is_valid()?;
Expand Down Expand Up @@ -90,7 +91,8 @@ impl<'ctx> Reader<'ctx> {

/// Connect to this reader, returning its `pcsc::Card`.
pub(crate) fn connect(&self) -> Result<pcsc::Card> {
let ctx = self.ctx.lock().unwrap();
// TODO(tarcieri): better error?
let ctx = self.ctx.lock().map_err(|_| Error::GenericError)?;
Ok(ctx.connect(self.name, pcsc::ShareMode::Shared, pcsc::Protocols::T1)?)
}
}
6 changes: 1 addition & 5 deletions src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,7 @@ impl<'tx> Transaction<'tx> {
return Err(Error::GenericError);
}

if response.data().len() < 3 {
return Err(Error::SizeError);
}

Ok(Version::new(response.data()[..3].try_into().unwrap()))
Ok(response.data()[..3].try_into().map(Version::new)?)
}

/// Get YubiKey device serial number.
Expand Down
7 changes: 3 additions & 4 deletions src/yubikey.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ impl YubiKey {
}

// send a response to the cards challenge and a challenge of our own.
let response = mgm_key.decrypt(challenge.data()[4..12].try_into().unwrap());
let response = mgm_key.decrypt(challenge.data()[4..12].try_into()?);

let mut data = [0u8; 22];
data[0] = TAG_DYN_AUTH;
Expand Down Expand Up @@ -517,8 +517,7 @@ impl YubiKey {

// TODO(tarcieri): double check this is little endian
let tnow = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.duration_since(UNIX_EPOCH)?
.as_secs()
.to_le_bytes();

Expand Down Expand Up @@ -648,7 +647,7 @@ impl YubiKey {
return Err(Error::AuthenticationError);
}

Ok(response.data()[4..12].try_into().unwrap())
Ok(response.data().get(4..12).ok_or(Error::SizeError)?.try_into()?)
}

/// Verify an auth response.
Expand Down

0 comments on commit 477656e

Please sign in to comment.