From 78313360a12f759caff44ec6b7cc2be3362bb63a Mon Sep 17 00:00:00 2001 From: "Tony Arcieri (iqlusion)" Date: Tue, 15 Aug 2023 17:02:25 -0700 Subject: [PATCH] Add `clippy::unwrap_used` lint (#515) Lints for usages of `unwrap()` in the `yubikey` crate (not CLI yet). Replaces them with `?` or `expect()` as the situation warrants. --- src/cccid.rs | 10 +++------- src/certificate.rs | 24 +++++++++++------------- src/chuid.rs | 17 +++++++---------- src/config.rs | 2 +- src/error.rs | 12 ++++++++++++ src/lib.rs | 9 ++++++++- src/mscmap.rs | 16 +++++++++------- src/piv.rs | 1 + src/reader.rs | 8 +++++--- src/transaction.rs | 6 +----- src/yubikey.rs | 11 +++++++---- 11 files changed, 65 insertions(+), 51 deletions(-) diff --git a/src/cccid.rs b/src/cccid.rs index 3d81da2..b95a244 100644 --- a/src/cccid.rs +++ b/src/cccid.rs @@ -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}; @@ -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, @@ -90,12 +91,7 @@ impl CccId { pub fn get(yubikey: &mut YubiKey) -> Result { 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 diff --git a/src/certificate.rs b/src/certificate.rs index 52fee63..6cada81 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -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, &[]); + 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, &[])?; + + txn.save_object(object_id, &buf[..offset]) + } else { + 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)?; - - // 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]) } pub mod yubikey_signer { diff --git a/src/chuid.rs b/src/chuid.rs index 5b6390a..16ad201 100644 --- a/src/chuid.rs +++ b/src/chuid.rs @@ -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; @@ -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, @@ -86,12 +87,13 @@ 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 @@ -99,19 +101,14 @@ impl ChuId { 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 { 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) diff --git a/src/config.rs b/src/config.rs index a6bb2e4..08de407 100644 --- a/src/config.rs +++ b/src/config.rs @@ -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 = diff --git a/src/error.rs b/src/error.rs index cdd47b3..7774b00 100644 --- a/src/error.rs +++ b/src/error.rs @@ -164,6 +164,18 @@ impl Display for Error { } } +impl From for Error { + fn from(_: std::array::TryFromSliceError) -> Error { + Error::SizeError + } +} + +impl From for Error { + fn from(_: std::time::SystemTimeError) -> Error { + Error::GenericError + } +} + impl From for Error { fn from(err: pcsc::Error) -> Error { Error::PcscError { inner: Some(err) } diff --git a/src/lib.rs b/src/lib.rs index 400734f..74cdf8c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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: // diff --git a/src/mscmap.rs b/src/mscmap.rs index bea06ee..75c455c 100644 --- a/src/mscmap.rs +++ b/src/mscmap.rs @@ -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]; @@ -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], @@ -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") } } diff --git a/src/piv.rs b/src/piv.rs index 91bfb7c..de2fdcc 100644 --- a/src/piv.rs +++ b/src/piv.rs @@ -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); diff --git a/src/reader.rs b/src/reader.rs index caa5cf7..1d013c8 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -1,6 +1,6 @@ //! Support for enumerating available PC/SC card readers. -use crate::{Result, YubiKey}; +use crate::{Error, Result, YubiKey}; use std::{ borrow::Cow, ffi::CStr, @@ -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()?; @@ -90,7 +91,8 @@ impl<'ctx> Reader<'ctx> { /// Connect to this reader, returning its `pcsc::Card`. pub(crate) fn connect(&self) -> Result { - 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)?) } } diff --git a/src/transaction.rs b/src/transaction.rs index 1398741..78ff3a9 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -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. diff --git a/src/yubikey.rs b/src/yubikey.rs index 6051675..fb62bc4 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -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; @@ -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(); @@ -648,7 +647,11 @@ 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.