Skip to content

Commit

Permalink
Fix clippy::upper_case_acronyms nits; small cleanups (#269)
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
tony-iqlusion authored Jul 11, 2021
1 parent 2c06626 commit a1d9c7a
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 80 deletions.
22 changes: 11 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

Expand Down
8 changes: 4 additions & 4 deletions src/apdu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand All @@ -58,7 +58,7 @@ pub(crate) struct APDU {
data: Vec<u8>,
}

impl APDU {
impl Apdu {
/// Create a new APDU with the given instruction code
pub fn new(ins: impl Into<Ins>) -> Self {
Self {
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 5 additions & 5 deletions src/cccid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CardId, Error> {
let mut cccid = [0u8; CCCID_SIZE];
Expand All @@ -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();
Expand All @@ -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,
Expand Down
13 changes: 6 additions & 7 deletions src/chuid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -124,7 +124,7 @@ impl CHUID {
}

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

Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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[..])
}
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

Expand Down
32 changes: 4 additions & 28 deletions src/mscmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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],
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}
}

Expand Down
22 changes: 11 additions & 11 deletions src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::{
apdu::Response,
apdu::{Ins, StatusWords, APDU},
apdu::{Apdu, Ins, StatusWords},
error::Error,
key::{AlgorithmId, SlotId},
serialization::*,
Expand Down Expand Up @@ -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)
Expand All @@ -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<Version, Error> {
// 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);
Expand All @@ -101,7 +101,7 @@ impl<'tx> Transaction<'tx> {
pub fn get_serial(&self, version: Version) -> Result<Serial, Error> {
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)?
Expand All @@ -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!(
Expand All @@ -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)?
Expand All @@ -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!(
Expand All @@ -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
Expand Down Expand Up @@ -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)?
Expand Down Expand Up @@ -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)])
Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit a1d9c7a

Please sign in to comment.