From a1d9c7afc5f6f541bedd0515c71f62d400e6775d Mon Sep 17 00:00:00 2001 From: "Tony Arcieri (iqlusion)" Date: Sun, 11 Jul 2021 08:51:25 -0700 Subject: [PATCH] Fix `clippy::upper_case_acronyms` nits; small cleanups (#269) Renames the following to match Rust idioms: - `APDU` => `Apdu` - `CCC` => `Ccc` - `CHUID` => `ChuId` Also removes `Copy` from `mscmap::Container`, which fixes a clippy lint about its usage of `to_bytes`. --- README.md | 22 +++++++++++----------- src/apdu.rs | 8 ++++---- src/cccid.rs | 10 +++++----- src/chuid.rs | 13 ++++++------- src/lib.rs | 1 - src/mscmap.rs | 32 ++++---------------------------- src/transaction.rs | 22 +++++++++++----------- src/yubikey.rs | 27 ++++++++++++++------------- 8 files changed, 55 insertions(+), 80 deletions(-) diff --git a/README.md b/README.md index c18026c..eb31580 100644 --- a/README.md +++ b/README.md @@ -115,17 +115,17 @@ Application Protocol Data Unit (APDU) messages, use the `trace` log level: ``` running 1 test -[INFO yubikey_piv::yubikey] trying to connect to reader 'Yubico YubiKey OTP+FIDO+CCID' -[INFO yubikey_piv::yubikey] connected to 'Yubico YubiKey OTP+FIDO+CCID' successfully -[TRACE yubikey_piv::apdu] >>> APDU { cla: 0, ins: SelectApplication, p1: 4, p2: 0, data: [160, 0, 0, 3, 8] } -[TRACE yubikey_piv::transaction] >>> [0, 164, 4, 0, 5, 160, 0, 0, 3, 8] -[TRACE yubikey_piv::apdu] <<< Response { status_words: Success, data: [97, 17, 79, 6, 0, 0, 16, 0, 1, 0, 121, 7, 79, 5, 160, 0, 0, 3, 8] } -[TRACE yubikey_piv::apdu] >>> APDU { cla: 0, ins: GetVersion, p1: 0, p2: 0, data: [] } -[TRACE yubikey_piv::transaction] >>> [0, 253, 0, 0, 0] -[TRACE yubikey_piv::apdu] <<< Response { status_words: Success, data: [5, 1, 2] } -[TRACE yubikey_piv::apdu] >>> APDU { cla: 0, ins: GetSerial, p1: 0, p2: 0, data: [] } -[TRACE yubikey_piv::transaction] >>> [0, 248, 0, 0, 0] -[TRACE yubikey_piv::apdu] <<< Response { status_words: Success, data: [0, 115, 0, 178] } +[INFO yubikey::yubikey] trying to connect to reader 'Yubico YubiKey OTP+FIDO+CCID' +[INFO yubikey::yubikey] connected to 'Yubico YubiKey OTP+FIDO+CCID' successfully +[TRACE yubikey::apdu] >>> Apdu { cla: 0, ins: SelectApplication, p1: 4, p2: 0, data: [160, 0, 0, 3, 8] } +[TRACE yubikey::transaction] >>> [0, 164, 4, 0, 5, 160, 0, 0, 3, 8] +[TRACE yubikey::apdu] <<< Response { status_words: Success, data: [97, 17, 79, 6, 0, 0, 16, 0, 1, 0, 121, 7, 79, 5, 160, 0, 0, 3, 8] } +[TRACE yubikey::apdu] >>> Apdu { cla: 0, ins: GetVersion, p1: 0, p2: 0, data: [] } +[TRACE yubikey::transaction] >>> [0, 253, 0, 0, 0] +[TRACE yubikey::apdu] <<< Response { status_words: Success, data: [5, 1, 2] } +[TRACE yubikey::apdu] >>> Apdu { cla: 0, ins: GetSerial, p1: 0, p2: 0, data: [] } +[TRACE yubikey::transaction] >>> [0, 248, 0, 0, 0] +[TRACE yubikey::apdu] <<< Response { status_words: Success, data: [0, 115, 0, 178] } test connect ... ok ``` diff --git a/src/apdu.rs b/src/apdu.rs index 96da0b8..8f77647 100644 --- a/src/apdu.rs +++ b/src/apdu.rs @@ -41,7 +41,7 @@ const APDU_DATA_MAX: usize = 0xFF; /// /// These messages are packets used to communicate with the YubiKey. #[derive(Clone, Debug, Eq, PartialEq)] -pub(crate) struct APDU { +pub(crate) struct Apdu { /// Instruction class: indicates the type of command (e.g. inter-industry or proprietary) cla: u8, @@ -58,7 +58,7 @@ pub(crate) struct APDU { data: Vec, } -impl APDU { +impl Apdu { /// Create a new APDU with the given instruction code pub fn new(ins: impl Into) -> Self { Self { @@ -129,13 +129,13 @@ impl APDU { } } -impl Drop for APDU { +impl Drop for Apdu { fn drop(&mut self) { self.zeroize(); } } -impl Zeroize for APDU { +impl Zeroize for Apdu { fn zeroize(&mut self) { // Only `data` may contain secrets self.data.zeroize(); diff --git a/src/cccid.rs b/src/cccid.rs index 4af6430..0bd093f 100644 --- a/src/cccid.rs +++ b/src/cccid.rs @@ -77,9 +77,9 @@ impl CardId { /// Cardholder Capability Container (CCC) Identifier #[derive(Copy, Clone)] -pub struct CCC(pub [u8; CCC_SIZE]); +pub struct Ccc(pub [u8; CCC_SIZE]); -impl CCC { +impl Ccc { /// Return CardId component of CCC pub fn cccid(&self) -> Result { let mut cccid = [0u8; CCCID_SIZE]; @@ -101,7 +101,7 @@ impl CCC { Ok(Self(ccc)) } - /// Get Cardholder Capability Container (CCC) ID + /// Set Cardholder Capability Container (CCC) ID #[cfg(feature = "untested")] pub fn set(&self, yubikey: &mut YubiKey) -> Result<(), Error> { let mut buf = CCC_TMPL.to_vec(); @@ -112,13 +112,13 @@ impl CCC { } } -impl Debug for CCC { +impl Debug for Ccc { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "CCC({:?})", &self.0[..]) } } -impl Display for CCC { +impl Display for Ccc { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, diff --git a/src/chuid.rs b/src/chuid.rs index c572a84..bb6ec5f 100644 --- a/src/chuid.rs +++ b/src/chuid.rs @@ -96,9 +96,9 @@ impl Uuid { /// Cardholder Unique Identifier (CHUID) #[derive(Copy, Clone)] -pub struct CHUID(pub [u8; CHUID_SIZE]); +pub struct ChuId(pub [u8; CHUID_SIZE]); -impl CHUID { +impl ChuId { /// Return FASC-N component of CHUID pub fn fascn(&self) -> Result<[u8; FASCN_SIZE], Error> { let mut fascn = [0u8; FASCN_SIZE]; @@ -124,7 +124,7 @@ impl CHUID { } /// Get Cardholder Unique Identifier (CHUID) - pub fn get(yubikey: &mut YubiKey) -> Result { + pub fn get(yubikey: &mut YubiKey) -> Result { let txn = yubikey.begin_transaction()?; let response = txn.fetch_object(OBJ_CHUID)?; @@ -134,13 +134,12 @@ impl CHUID { let mut chuid = [0u8; CHUID_SIZE]; chuid.copy_from_slice(&response[0..CHUID_SIZE]); - let retval = CHUID { 0: chuid }; + let retval = ChuId { 0: chuid }; Ok(retval) } /// Set Cardholder Unique Identifier (CHUID) #[cfg(feature = "untested")] - pub fn set(&self, yubikey: &mut YubiKey) -> Result<(), Error> { let mut buf = CHUID_TMPL.to_vec(); buf[0..self.0.len()].copy_from_slice(&self.0); @@ -150,7 +149,7 @@ impl CHUID { } } -impl Display for CHUID { +impl Display for ChuId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, @@ -160,7 +159,7 @@ impl Display for CHUID { } } -impl Debug for CHUID { +impl Debug for ChuId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "CHUID({:?})", &self.0[..]) } diff --git a/src/lib.rs b/src/lib.rs index b35df9d..bdff8ff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,7 +125,6 @@ html_logo_url = "https://raw.githubusercontent.com/iqlusioninc/yubikey.rs/main/img/logo.png", html_root_url = "https://docs.rs/yubikey/0.4.0-pre" )] -#![allow(clippy::upper_case_acronyms)] #![forbid(unsafe_code)] #![warn(missing_docs, rust_2018_idioms, trivial_casts, unused_qualifications)] diff --git a/src/mscmap.rs b/src/mscmap.rs index e8ec870..d866e49 100644 --- a/src/mscmap.rs +++ b/src/mscmap.rs @@ -35,10 +35,7 @@ use crate::{error::Error, key::SlotId, serialization::*, yubikey::YubiKey, CB_OBJ_MAX}; use log::error; -use std::{ - convert::{TryFrom, TryInto}, - fmt::{self, Debug}, -}; +use std::convert::{TryFrom, TryInto}; /// Container name length const CONTAINER_NAME_LEN: usize = 40; @@ -51,7 +48,7 @@ const OBJ_MSCMAP: u32 = 0x005f_ff10; const TAG_MSCMAP: u8 = 0x81; /// MS Container Map(?) Records -#[derive(Copy, Clone)] +#[derive(Clone, Debug)] pub struct Container { /// Container name pub name: [u16; CONTAINER_NAME_LEN], @@ -170,6 +167,7 @@ impl Container { /// Serialize a container record as a byte size pub fn to_bytes(&self) -> [u8; CONTAINER_REC_LEN] { + // TODO(tarcieri): use array instead of `Vec` let mut bytes = Vec::with_capacity(CONTAINER_REC_LEN); for i in 0..CONTAINER_NAME_LEN { @@ -183,29 +181,7 @@ impl Container { bytes.push(self.pin_id); bytes.push(self.associated_echd_container); bytes.extend_from_slice(&self.cert_fingerprint); - - // TODO(tarcieri): use TryInto here when const generics are available - let mut result = [0u8; CONTAINER_REC_LEN]; - result.copy_from_slice(&bytes); - result - } -} - -impl Debug for Container { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "PivContainer {{ name: {:?}, slot: {:?}, key_spec: {}, key_size_bits: {}, \ - flags: {}, pin_id: {}, associated_echd_container: {}, cert_fingerprint: {:?} }}", - &self.name[..], - self.slot, - self.key_spec, - self.key_size_bits, - self.flags, - self.pin_id, - self.associated_echd_container, - &self.cert_fingerprint[..] - ) + bytes.as_slice().try_into().unwrap() } } diff --git a/src/transaction.rs b/src/transaction.rs index 2708688..187b8c6 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -2,7 +2,7 @@ use crate::{ apdu::Response, - apdu::{Ins, StatusWords, APDU}, + apdu::{Apdu, Ins, StatusWords}, error::Error, key::{AlgorithmId, SlotId}, serialization::*, @@ -61,7 +61,7 @@ impl<'tx> Transaction<'tx> { /// Select application. pub fn select_application(&self) -> Result<(), Error> { - let response = APDU::new(Ins::SelectApplication) + let response = Apdu::new(Ins::SelectApplication) .p1(0x04) .data(&PIV_AID) .transmit(self, 0xFF) @@ -84,7 +84,7 @@ impl<'tx> Transaction<'tx> { /// Get the version of the PIV application installed on the YubiKey. pub fn get_version(&self) -> Result { // get version from device - let response = APDU::new(Ins::GetVersion).transmit(self, 261)?; + let response = Apdu::new(Ins::GetVersion).transmit(self, 261)?; if !response.is_success() { return Err(Error::GenericError); @@ -101,7 +101,7 @@ impl<'tx> Transaction<'tx> { pub fn get_serial(&self, version: Version) -> Result { let response = if version.major < 5 { // YK4 requires switching to the yk applet to retrieve the serial - let sw = APDU::new(Ins::SelectApplication) + let sw = Apdu::new(Ins::SelectApplication) .p1(0x04) .data(&YK_AID) .transmit(self, 0xFF)? @@ -112,7 +112,7 @@ impl<'tx> Transaction<'tx> { return Err(Error::GenericError); } - let resp = APDU::new(0x01).p1(0x10).transmit(self, 0xFF)?; + let resp = Apdu::new(0x01).p1(0x10).transmit(self, 0xFF)?; if !resp.is_success() { error!( @@ -123,7 +123,7 @@ impl<'tx> Transaction<'tx> { } // reselect the PIV applet - let sw = APDU::new(Ins::SelectApplication) + let sw = Apdu::new(Ins::SelectApplication) .p1(0x04) .data(&PIV_AID) .transmit(self, 0xFF)? @@ -137,7 +137,7 @@ impl<'tx> Transaction<'tx> { resp } else { // YK5 implements getting the serial as a PIV applet command (0xf8) - let resp = APDU::new(Ins::GetSerial).transmit(self, 0xFF)?; + let resp = Apdu::new(Ins::GetSerial).transmit(self, 0xFF)?; if !resp.is_success() { error!( @@ -162,7 +162,7 @@ impl<'tx> Transaction<'tx> { return Err(Error::SizeError); } - let mut query = APDU::new(Ins::Verify); + let mut query = Apdu::new(Ins::Verify); query.params(0x00, 0x80); // Empty pin means we are querying the number of retries. We set no data in this @@ -238,7 +238,7 @@ impl<'tx> Transaction<'tx> { data[2] = DES_LEN_3DES as u8; data[3..3 + DES_LEN_3DES].copy_from_slice(new_key.as_ref()); - let status_words = APDU::new(Ins::SetMgmKey) + let status_words = Apdu::new(Ins::SetMgmKey) .params(0xff, p2) .data(&data) .transmit(self, 261)? @@ -380,7 +380,7 @@ impl<'tx> Transaction<'tx> { trace!("going to send {} bytes in this go", this_size); - let response = APDU::new(templ[1]) + let response = Apdu::new(templ[1]) .cla(cla) .params(templ[2], templ[3]) .data(&in_data[in_offset..(in_offset + this_size)]) @@ -417,7 +417,7 @@ impl<'tx> Transaction<'tx> { sw & 0xff ); - let response = APDU::new(Ins::GetResponseApdu).transmit(self, 261)?; + let response = Apdu::new(Ins::GetResponseApdu).transmit(self, 261)?; sw = response.status_words().code(); if sw != StatusWords::Success.code() && (sw >> 8 != 0x61) { diff --git a/src/yubikey.rs b/src/yubikey.rs index cc44416..59385f4 100644 --- a/src/yubikey.rs +++ b/src/yubikey.rs @@ -31,9 +31,9 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::{ - apdu::{Ins, APDU}, - cccid::CCC, - chuid::CHUID, + apdu::{Apdu, Ins}, + cccid::Ccc, + chuid::ChuId, config::Config, error::Error, mgm::MgmKey, @@ -155,7 +155,8 @@ impl YubiKey { /// /// If you need to operate in environments with more than one YubiKey /// attached to the same system, use [`YubiKey::open_by_serial`] or - ///[`yubikey_piv::Readers`] to select from the available PC/SC readers. + /// [`yubikey::Readers`][`Readers`] to select from the available + /// PC/SC readers. pub fn open() -> Result { let mut readers = Readers::open().map_err(|e| match e { Error::PcscError { @@ -259,13 +260,13 @@ impl YubiKey { } /// Get CHUID - pub fn chuid(&mut self) -> Result { - CHUID::get(self) + pub fn chuid(&mut self) -> Result { + ChuId::get(self) } /// Get CCCID - pub fn cccid(&mut self) -> Result { - CCC::get(self) + pub fn cccid(&mut self) -> Result { + Ccc::get(self) } /// Authenticate to the card using the provided management key (MGM). @@ -273,7 +274,7 @@ impl YubiKey { let txn = self.begin_transaction()?; // get a challenge from the card - let challenge = APDU::new(Ins::Authenticate) + let challenge = Apdu::new(Ins::Authenticate) .params(ALGO_3DES, KEY_CARDMGM) .data(&[TAG_DYN_AUTH, 0x02, 0x80, 0x00]) .transmit(&txn, 261)?; @@ -302,7 +303,7 @@ impl YubiKey { let mut challenge = [0u8; 8]; challenge.copy_from_slice(&data[14..22]); - let authentication = APDU::new(Ins::Authenticate) + let authentication = Apdu::new(Ins::Authenticate) .params(ALGO_3DES, KEY_CARDMGM) .data(&data) .transmit(&txn, 261)?; @@ -327,7 +328,7 @@ impl YubiKey { pub fn deauthenticate(&mut self) -> Result<(), Error> { let txn = self.begin_transaction()?; - let status_words = APDU::new(Ins::SelectApplication) + let status_words = Apdu::new(Ins::SelectApplication) .p1(0x04) .data(MGMT_AID) .transmit(&txn, 255)? @@ -546,7 +547,7 @@ impl YubiKey { pub fn get_auth_challenge(&mut self) -> Result<[u8; 8], Error> { let txn = self.begin_transaction()?; - let response = APDU::new(Ins::Authenticate) + let response = Apdu::new(Ins::Authenticate) .params(ALGO_3DES, KEY_CARDMGM) .data(&[0x7c, 0x02, 0x81, 0x00]) .transmit(&txn, 261)?; @@ -571,7 +572,7 @@ impl YubiKey { let txn = self.begin_transaction()?; // send the response to the card and a challenge of our own. - let status_words = APDU::new(Ins::Authenticate) + let status_words = Apdu::new(Ins::Authenticate) .params(ALGO_3DES, KEY_CARDMGM) .data(&data) .transmit(&txn, 261)?