From 7eb7a31a281e42ec9a67a506e3bc473fa97dab4a Mon Sep 17 00:00:00 2001 From: "Tony Arcieri (iqlusion)" Date: Mon, 18 Aug 2025 13:05:23 -0600 Subject: [PATCH] mgm: remove `untested` gating from tested methods (#623) Removes the `#[cfg(feature = "untested")]` gating from all methods tested in `tests/integration.rs` and their dependent codepaths. --- Cargo.toml | 6 +++++- src/consts.rs | 4 ++-- src/metadata.rs | 19 +++++++------------ src/mgm.rs | 40 ++++++++++++++++------------------------ src/transaction.rs | 4 ++-- 5 files changed, 32 insertions(+), 41 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dc4ddac..3bb970e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ members = [".", "cli"] [workspace.dependencies] sha2 = "0.11.0-rc.0" -x509-cert = { version = "0.3.0-rc.1", features = [ "builder", "hazmat" ] } +x509-cert = { version = "0.3.0-rc.1", features = ["builder", "hazmat"] } [dependencies] bitflags = "2.5.0" @@ -55,6 +55,10 @@ once_cell = "1" [features] untested = [] +[[example]] +name = "change-mode" +required-features = ["untested"] + [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] diff --git a/src/consts.rs b/src/consts.rs index bc40a7d..f1d75a2 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -1,6 +1,7 @@ -#![allow(dead_code)] //! Miscellaneous constant values +#![allow(dead_code)] + /// YubiKey max buffer size pub(crate) const CB_BUF_MAX: usize = 3072; @@ -8,7 +9,6 @@ pub(crate) const CB_BUF_MAX: usize = 3072; pub(crate) const CB_OBJ_MAX: usize = CB_BUF_MAX - 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 // Admin tags diff --git a/src/metadata.rs b/src/metadata.rs index fdf9c97..6d82a21 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -30,16 +30,15 @@ // (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 std::marker::PhantomData; +use std::{iter, marker::PhantomData}; use zeroize::Zeroizing; -use crate::{serialization::*, transaction::Transaction, Buffer, Error, Result}; - -#[cfg(feature = "untested")] -use crate::consts::{CB_OBJ_MAX, CB_OBJ_TAG_MAX}; - -#[cfg(feature = "untested")] -use std::iter; +use crate::{ + consts::{CB_OBJ_MAX, CB_OBJ_TAG_MAX}, + serialization::*, + transaction::Transaction, + Buffer, Error, Result, +}; const TAG_ADMIN: u8 = 0x80; const TAG_PROTECTED: u8 = 0x88; @@ -87,7 +86,6 @@ impl Metadata { } /// Write metadata - #[cfg(feature = "untested")] pub(crate) fn write(&self, txn: &Transaction<'_>) -> Result<()> { if self.inner.len() > CB_OBJ_MAX - CB_OBJ_TAG_MAX { return Err(Error::GenericError); @@ -104,7 +102,6 @@ impl Metadata { } /// Delete metadata - #[cfg(feature = "untested")] pub(crate) fn delete(txn: &Transaction<'_>) -> Result<()> { txn.save_object(T::obj_id(), &[]) } @@ -127,7 +124,6 @@ impl Metadata { } /// Set metadata item - #[cfg(feature = "untested")] pub(crate) fn set_item(&mut self, tag: u8, item: &[u8]) -> Result<()> { let mut cb_temp: usize = 0; let mut tag_temp: u8 = 0; @@ -216,7 +212,6 @@ impl Metadata { } /// Get the size of a length tag for the given length -#[cfg(feature = "untested")] fn get_length_size(length: usize) -> usize { if length < 0x80 { 1 diff --git a/src/mgm.rs b/src/mgm.rs index ee857d0..e82c7d8 100644 --- a/src/mgm.rs +++ b/src/mgm.rs @@ -30,7 +30,14 @@ // (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}; +use crate::{ + consts::{TAG_ADMIN_FLAGS_1, TAG_ADMIN_SALT, TAG_PROTECTED_MGM}, + metadata::{AdminData, ProtectedData}, + piv::{ManagementSlotId, SlotAlgorithmId}, + transaction::Transaction, + Error, Result, YubiKey, +}; +use bitflags::bitflags; use log::error; use rand_core::{OsRng, RngCore, TryRngCore}; use zeroize::Zeroize; @@ -39,23 +46,18 @@ use des::{ cipher::{BlockCipherDecrypt, BlockCipherEncrypt, KeyInit}, TdesEde3, }; + #[cfg(feature = "untested")] use { crate::{ consts::{ - CB_BUF_MAX, TAG_ADMIN_FLAGS_1, TAG_ADMIN_SALT, TAG_AUTO_EJECT_TIMEOUT, - TAG_CHALRESP_TIMEOUT, TAG_CONFIG_LOCK, TAG_DEVICE_FLAGS, TAG_FORM_FACTOR, - TAG_NFC_ENABLED, TAG_NFC_SUPPORTED, TAG_PROTECTED_MGM, TAG_REBOOT, TAG_SERIAL, - TAG_UNLOCK, TAG_USB_ENABLED, TAG_USB_SUPPORTED, TAG_VERSION, + CB_BUF_MAX, TAG_AUTO_EJECT_TIMEOUT, TAG_CHALRESP_TIMEOUT, TAG_CONFIG_LOCK, + TAG_DEVICE_FLAGS, TAG_FORM_FACTOR, TAG_NFC_ENABLED, TAG_NFC_SUPPORTED, TAG_REBOOT, + TAG_SERIAL, TAG_UNLOCK, TAG_USB_ENABLED, TAG_USB_SUPPORTED, TAG_VERSION, }, - metadata::{AdminData, ProtectedData}, - piv::{ManagementSlotId, SlotAlgorithmId}, serialization::Tlv, - transaction::Transaction, - yubikey::YubiKey, Serial, Version, }, - bitflags::bitflags, pbkdf2::pbkdf2_hmac, sha1::Sha1, }; @@ -126,7 +128,6 @@ impl From for u8 { impl MgmAlgorithmId { /// Looks up the algorithm for the given Yubikey's current management key. - #[cfg(feature = "untested")] fn query(txn: &Transaction<'_>) -> Result { match txn.get_metadata(crate::piv::SlotId::Management(ManagementSlotId::Management)) { Ok(metadata) => match metadata.algorithm { @@ -217,7 +218,6 @@ impl MgmKey { } /// Get protected management key (MGM) - #[cfg(feature = "untested")] pub fn get_protected(yubikey: &mut YubiKey) -> Result { let txn = yubikey.begin_transaction()?; @@ -250,7 +250,6 @@ impl MgmKey { /// Resets the management key for the given YubiKey to the default value. /// /// This will wipe any metadata related to derived and PIN-protected management keys. - #[cfg(feature = "untested")] pub fn set_default(yubikey: &mut YubiKey) -> Result<()> { MgmKey::default().set_manual(yubikey, false) } @@ -261,7 +260,6 @@ impl MgmKey { /// management operations. /// /// This will wipe any metadata related to derived and PIN-protected management keys. - #[cfg(feature = "untested")] pub fn set_manual(&self, yubikey: &mut YubiKey, require_touch: bool) -> Result<()> { let txn = yubikey.begin_transaction()?; @@ -318,7 +316,6 @@ impl MgmKey { /// Configures the given YubiKey to use this as a PIN-protected management key. /// /// This enables key management operations to be performed with access to the PIN. - #[cfg(feature = "untested")] pub fn set_protected(&self, yubikey: &mut YubiKey) -> Result<()> { let txn = yubikey.begin_transaction()?; @@ -524,7 +521,6 @@ impl Lock { /// Configuration for a device #[derive(Clone, Debug, PartialEq)] -#[cfg(feature = "untested")] pub struct DeviceConfig { /// List of applets that are available on the USB interface pub usb_enabled_apps: Capability, @@ -539,8 +535,8 @@ pub struct DeviceConfig { pub device_flags: Option, } -#[cfg(feature = "untested")] impl DeviceConfig { + #[cfg(feature = "untested")] pub(crate) fn as_tlv( &self, reboot: bool, @@ -622,7 +618,6 @@ impl DeviceConfig { } } -#[cfg(feature = "untested")] bitflags! { /// Represents a set of applications. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -649,7 +644,6 @@ bitflags! { /// Device information /// This represents the configuration and the status of the device -#[cfg(feature = "untested")] pub struct DeviceInfo { /// Configuration of the YubiKey pub config: DeviceConfig, @@ -657,8 +651,8 @@ pub struct DeviceInfo { pub is_locked: bool, } -#[cfg(feature = "untested")] impl DeviceInfo { + #[cfg(feature = "untested")] pub(crate) fn parse(input: &[u8]) -> Result { use nom::{ bytes::complete::take, @@ -820,7 +814,6 @@ impl DeviceInfo { } /// FormFactor of the YubiKey -#[cfg(feature = "untested")] #[derive(Debug, Copy, Clone, PartialEq)] #[repr(u8)] pub enum FormFactor { @@ -844,8 +837,8 @@ pub enum FormFactor { Unsupported(u8), } -#[cfg(feature = "untested")] impl FormFactor { + #[cfg(feature = "untested")] fn parse(input: &[u8]) -> Result { use nom::{combinator::eof, number::complete::u8}; @@ -866,7 +859,6 @@ impl FormFactor { } } -#[cfg(feature = "untested")] bitflags! { /// Represents configuration flags. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -878,8 +870,8 @@ bitflags! { } } -#[cfg(feature = "untested")] impl DeviceFlags { + #[cfg(feature = "untested")] fn parse(i: &[u8]) -> nom::IResult<&[u8], Self> { use nom::{ combinator::{eof, map}, diff --git a/src/transaction.rs b/src/transaction.rs index 90bfbb3..47ee95c 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -5,6 +5,7 @@ use crate::{ apdu::{Apdu, Ins, StatusWords}, consts::{CB_BUF_MAX, CB_OBJ_MAX}, error::{Error, Result}, + mgm::{MgmKey, DES_LEN_3DES}, otp, piv::{self, AlgorithmId, SlotId}, serialization::*, @@ -15,7 +16,7 @@ use log::{error, trace}; use zeroize::Zeroizing; #[cfg(feature = "untested")] -use crate::mgm::{DeviceConfig, DeviceInfo, Lock, MgmKey, DES_LEN_3DES}; +use crate::mgm::{DeviceConfig, DeviceInfo, Lock}; const CB_PIN_MAX: usize = 8; @@ -247,7 +248,6 @@ impl<'tx> Transaction<'tx> { } /// Set the management key (MGM). - #[cfg(feature = "untested")] pub fn set_mgm_key(&self, new_key: &MgmKey, require_touch: bool) -> Result<()> { let p2 = if require_touch { 0xfe } else { 0xff };