refactor(key): improve secret_int! macro

cipher-core/src/error.rs

@@ -3,97 +3,26 @@ use thiserror::Error;
 #[derive(Debug, Error, Clone, Copy, PartialEq, Eq)]
 pub enum CipherError {
     /// Invalid key size for the cipher
-    #[error("invalid key size: expected {expected} bytes, got {actual}")]
+    #[error("Invalid key size: expected {expected} bytes, got {actual}")]
     InvalidKeySize { expected: usize, actual: usize },
 
     /// Input data doesn't match the cipher's block size
-    #[error("invalid block size: expected {expected} bytes, got {actual}")]
+    #[error("Invalid block size: expected {expected} bytes, got {actual}")]
     InvalidBlockSize { expected: usize, actual: usize },
-
-    /// Decryption detected invalid padding
-    #[error("invalid padding detected during decryption")]
-    InvalidPadding,
-
-    /// Input length not valid for unpadded operation
-    #[error("invalid plaintext length: {actual} bytes (must be multiple of block size)")]
-    InvalidPlaintextLength { actual: usize },
-
-    /// General size validation failure
-    #[error("size mismatch: expected {expected} bytes, got {actual}")]
-    InvalidSize { expected: usize, actual: usize },
 }
 
 impl CipherError {
-    /// Creates a key size error
     #[inline]
     #[must_use]
     pub const fn invalid_key_size(expected: usize, actual: usize) -> Self {
         Self::InvalidKeySize { expected, actual }
     }
 
-    /// Creates a block size error  
     #[inline]
     #[must_use]
     pub const fn invalid_block_size(expected: usize, actual: usize) -> Self {
         Self::InvalidBlockSize { expected, actual }
     }
-
-    /// Creates an invalid padding error
-    #[inline]
-    #[must_use]
-    pub const fn invalid_padding() -> Self {
-        Self::InvalidPadding
-    }
-
-    /// Creates a plaintext length error
-    #[inline]
-    #[must_use]
-    pub const fn invalid_plaintext_length(actual: usize) -> Self {
-        Self::InvalidPlaintextLength { actual }
-    }
-
-    /// Returns true if this is a key size error
-    #[inline]
-    #[must_use]
-    pub const fn is_key_error(&self) -> bool {
-        matches!(self, Self::InvalidKeySize { .. })
-    }
-
-    /// Returns true if this is a block size error
-    #[inline]
-    #[must_use]
-    pub const fn is_block_error(&self) -> bool {
-        matches!(self, Self::InvalidBlockSize { .. })
-    }
-
-    /// Returns true if this is a size-related error
-    #[must_use]
-    pub const fn is_size_error(&self) -> bool {
-        self.is_key_error() || self.is_block_error() || matches!(self, Self::InvalidSize { .. })
-    }
-
-    /// Returns the expected size for size-related errors
-    #[must_use]
-    pub const fn expected_size(&self) -> Option<usize> {
-        match self {
-            Self::InvalidKeySize { expected, .. }
-            | Self::InvalidBlockSize { expected, .. }
-            | Self::InvalidSize { expected, .. } => Some(*expected),
-            _ => None,
-        }
-    }
-
-    /// Returns the actual size for size-related errors
-    #[must_use]
-    pub const fn actual_size(&self) -> Option<usize> {
-        match self {
-            Self::InvalidKeySize { actual, .. }
-            | Self::InvalidBlockSize { actual, .. }
-            | Self::InvalidSize { actual, .. }
-            | Self::InvalidPlaintextLength { actual } => Some(*actual),
-            Self::InvalidPadding => None,
-        }
-    }
 }
 
 /// Type alias for clean Result types

cipher-core/src/traits.rs

@@ -1,10 +1,28 @@
 use crate::{CipherAction, CipherError, CipherResult};
 
+/// Generic block cipher trait.
+///
+/// Implements the standard encrypt/decrypt interface for block ciphers.
+/// Implementers define `transform_impl` to handle the core algorithm,
+/// while `transform` provides validation and convenience wrappers.
 pub trait BlockCipher: Sized {
     const BLOCK_SIZE: usize;
 
+    /// Core cipher transformation (must be implemented by concrete types).
+    ///
+    /// # Errors
+    ///
+    /// Returns `CipherError` if the transformation fails.
     fn transform_impl(&self, block: &[u8], action: CipherAction) -> CipherResult<Vec<u8>>;
 
+    /// Transforms a block with validation.
+    ///
+    /// Validates that the block size matches `BLOCK_SIZE` before delegating
+    /// to `transform_impl`.
+    ///
+    /// # Errors
+    ///
+    /// Returns `CipherError::InvalidBlockSize` if `block.len() != BLOCK_SIZE`.
     fn transform(&self, block: &[u8], action: CipherAction) -> CipherResult<Vec<u8>> {
         if block.len() != Self::BLOCK_SIZE {
             return Err(CipherError::invalid_block_size(
@@ -15,9 +33,20 @@ pub trait BlockCipher: Sized {
         self.transform_impl(block, action)
     }
 
+    /// Encrypts a single block.
+    ///
+    /// # Errors
+    ///
+    /// Returns `CipherError::InvalidBlockSize` if the plaintext is not exactly `BLOCK_SIZE` bytes.
     fn encrypt(&self, plaintext: &[u8]) -> CipherResult<Vec<u8>> {
         self.transform(plaintext, CipherAction::Encrypt)
     }
+
+    /// Decrypts a single block.
+    ///
+    /// # Errors
+    ///
+    /// Returns `CipherError::InvalidBlockSize` if the plaintext is not exactly `BLOCK_SIZE` bytes.
     fn decrypt(&self, ciphertext: &[u8]) -> CipherResult<Vec<u8>> {
         self.transform(ciphertext, CipherAction::Decrypt)
     }

des/src/block/block48.rs

@@ -45,27 +45,27 @@ impl BitXor for Block48 {
 impl BitXor<Subkey> for Block48 {
     type Output = Self;
     fn bitxor(self, rhs: Subkey) -> Self::Output {
-        Self(self.0 ^ rhs.as_int())
+        Self(self.0 ^ rhs.as_u64())
     }
 }
 
 impl BitXor<&Subkey> for Block48 {
     type Output = Self;
     fn bitxor(self, rhs: &Subkey) -> Self::Output {
-        Self(self.0 ^ rhs.as_int())
+        Self(self.0 ^ rhs.as_u64())
     }
 }
 
 impl BitXor<Subkey> for &Block48 {
     type Output = Block48;
     fn bitxor(self, rhs: Subkey) -> Self::Output {
-        Block48(self.0 ^ rhs.as_int())
+        Block48(self.0 ^ rhs.as_u64())
     }
 }
 
 impl BitXor<&Subkey> for &Block48 {
     type Output = Block48;
     fn bitxor(self, rhs: &Subkey) -> Self::Output {
-        Block48(self.0 ^ rhs.as_int())
+        Block48(self.0 ^ rhs.as_u64())
     }
 }

des/src/des.rs

@@ -4,16 +4,33 @@ use crate::{
     key::{Key, Subkey, Subkeys},
     utils::permutate,
 };
-use cipher_core::{BlockCipher, CipherAction, CipherError, CipherResult};
+use cipher_core::{BlockCipher, CipherAction, CipherError};
 
 pub struct Des {
     subkeys: Subkeys,
 }
 
 impl Des {
-    pub fn new(key: impl Into<Key>) -> CipherResult<Self> {
+    /// Creates a new DES cipher with the given key.
-        let subkeys = Subkeys::from_key(&key.into())?;
+    ///
-        Ok(Self { subkeys })
+    /// # Arguments
+    ///
+    /// - `key` - An 8-byte key (64 bits). Any type implementing `Into<Key>` is accepted
+    ///   (e.g., `&[u8; 8]`, `u64`).
+    ///
+    /// # Errors
+    ///
+    /// `CipherError::InvalidKeySize` if the key is not exactly 8 bytes.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// use des::Des;
+    /// let des = Des::new(0x1334_5779_9BBC_DFF1u64).expect("Valid key");
+    /// ```
+    pub fn new(key: impl Into<Key>) -> Self {
+        let subkeys = Subkeys::from_key(&key.into());
+        Self { subkeys }
     }
 }
 

des/src/key/cd56.rs

@@ -8,11 +8,10 @@ pub struct CD56 {
 }
 
 impl CD56 {
-    pub fn new(c: impl Into<Half28>, d: impl Into<Half28>) -> Self {
+    #[inline]
-        Self {
+    #[must_use]
-            c: c.into(),
+    pub const fn new(c: Half28, d: Half28) -> Self {
-            d: d.into(),
+        Self { c, d }
-        }
     }
 
     pub fn rotate_left(&mut self, amount: u8) {

des/src/key/half28.rs

@@ -1,6 +1,6 @@
-use crate::key::secret_int;
+use crate::key::secret_key;
 
-secret_int! {
+secret_key! {
     /// 28-bit half (C or D), stored in lower 28 bits of u32.
     pub struct Half28(u32, 28, 0x0FFF_FFFF);
 }
@@ -11,15 +11,14 @@ impl Half28 {
         let value = self.0;
         let main_shifted = (value << amount) & Self::MASK;
         let wrapped_bits = (value >> (28 - amount)) & ((1 << amount) - 1);
-        Self::from_int(main_shifted | wrapped_bits)
+        Self::from_u32(main_shifted | wrapped_bits)
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use rstest::rstest;
-
     use super::*;
+    use rstest::rstest;
 
     #[rstest]
     #[case(0x0F0C_CAAF, 0x0E19_955F, 1)] // C_1
@@ -55,7 +54,7 @@ mod tests {
     #[case(0x0EAA_CCF1, 0x0AAB_33C7, 2)] // D_15
     #[case(0x0AAB_33C7, 0x0556_678F, 1)] // D_16
     fn half28_rotation(#[case] key: u32, #[case] expected: u32, #[case] amount: u8) {
-        let result = Half28::from_int(key).rotate_left(amount).as_int();
+        let result = Half28::from_u32(key).rotate_left(amount).as_u32();
 
         assert_eq!(
             result, expected,

des/src/key/key56.rs

@@ -1,26 +1,26 @@
 use crate::{
     key::{cd56::CD56, half28::Half28},
-    secret_int,
+    secret_key,
 };
 
-secret_int! {
+secret_key! {
     /// 56-bit key after PC-1 (lower 56 bits used).
     pub struct Key56(u64, 56, 0x00FF_FFFF_FFFF_FFFF);
 }
 
 impl Key56 {
     #[must_use]
-    pub fn split(&self) -> CD56 {
+    pub const fn split(&self) -> CD56 {
         let c = ((self.0 >> 28) & 0x0FFF_FFFF) as u32;
         let d = (self.0 & 0x0FFF_FFFF) as u32;
-        CD56::new(c, d)
+        CD56::new(Half28::from_u32(c), Half28::from_u32(d))
     }
 
     #[must_use]
-    pub fn from_half28(left: &Half28, right: &Half28) -> Self {
+    pub const fn from_half28(left: &Half28, right: &Half28) -> Self {
-        let left = u64::from(left.as_int());
+        let left = left.as_u64();
-        let right = u64::from(right.as_int());
+        let right = right.as_u64();
-        Self::from_int((left << 28) | right)
+        Self::from_u64((left << 28) | right)
     }
 }
 

des/src/key/mod.rs

@@ -2,9 +2,9 @@ mod cd56;
 mod des_key;
 mod half28;
 mod key56;
-mod secret_int;
+mod secret_key;
 mod subkey;
 mod subkeys;
 
-use crate::secret_int;
+use crate::secret_key;
 pub use {des_key::Key, subkey::Subkey, subkeys::Subkeys};

des/src/key/secret_int.rs

@@ -1,77 +0,0 @@
-/// Macro to generate a masked, zeroizable integer wrapper type.
-///
-/// Usage:
-/// ```
-/// secret_int! {
-///     /// docs...
-///     pub struct Subkey(u64, 48, 0x0000_FFFF_FFFF_FFFFu64);
-/// }
-/// ```
-///
-/// Optional `clone` token enables an explicit Clone impl:
-/// ```
-/// secret_int! { pub struct Foo(u32, 28, 0x0FFF_FFFFu32, clone); }
-/// ```
-#[macro_export]
-macro_rules! secret_int {
-    (
-    $(#[$meta:meta])*
-    $vis:vis struct $name:ident ( $int:ty, $bits:expr, $mask:expr $(, $opt:ident )? );
-) => {
-        $(#[$meta])*
-        #[derive(::zeroize::ZeroizeOnDrop, Default, Eq)]
-        #[zeroize(drop)]
-        $vis struct $name($int);
-
-        impl $name {
-            /// Mask to restrict the underlying integer to valid bits.
-            pub const MASK: $int = $mask;
-
-            /// Create from the given integer.
-            #[inline]
-            #[must_use]
-            pub const fn from_int(key: $int) -> Self {
-                Self(key & Self::MASK)
-            }
-
-            /// Return as masked integer value;
-            #[inline]
-            #[must_use]
-            pub const fn as_int(&self) -> $int {
-                self.0 & Self::MASK
-            }
-        }
-
-        // Optionally add Clone if requested explicitly (discouraged for secrets)
-        secret_int!(@maybe_add_clone $( $opt )? ; $name, $int);
-
-        impl ::std::fmt::Debug for $name {
-            fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
-                f.write_str(concat!(stringify!($name), "[REDACTED]"))
-            }
-        }
-
-        impl From<$int> for $name {
-            fn from(v: $int) -> Self {
-                Self::from_int(v)
-            }
-        }
-
-        impl PartialEq for $name {
-            fn eq(&self, other: &Self) -> bool {
-                self.as_int() == other.as_int()
-            }
-        }
-    };
-
-    // helper arm: create Clone impl only when "clone" token present
-    (@maybe_add_clone clone ; $name:ident, $int:ty) => {
-        impl Clone for $name {
-            fn clone(&self) -> Self {
-                // explicit clone - intentionally duplicating secret
-                Self::from_int(self.as_int())
-            }
-        }
-    };
-    (@maybe_add_clone ; $name:ident, $int:ty) => {};
-}

des/src/key/secret_key.rs

@@ -0,0 +1,126 @@
+/// Macro to generate a masked, zeroizable integer wrapper type.
+///
+/// Usage:
+/// ```
+/// secret_key! {
+///     /// docs...
+///     pub struct Subkey(u64, 48, 0x0000_FFFF_FFFF_FFFFu64);
+/// }
+/// ```
+#[macro_export]
+macro_rules! secret_key {
+    (
+    $(#[$meta:meta])*
+    $vis:vis struct $name:ident ( $int:tt, $bits:expr, $mask:expr );
+) => {
+        $(#[$meta])*
+        #[derive(::zeroize::ZeroizeOnDrop, Default)]
+        #[zeroize(drop)]
+        $vis struct $name($int);
+
+        impl $name {
+            /// Mask to restrict the underlying integer to valid bits.
+            pub const MASK: $int = $mask;
+
+            secret_key!(@conversions_as $int);
+            secret_key!(@conversions_from $int $int);
+        }
+
+        impl ::std::fmt::Debug for $name {
+            fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+                f.write_str(concat!(stringify!($name), "[REDACTED]"))
+            }
+        }
+
+        impl From<$int> for $name {
+            fn from(v: $int) -> Self {
+                Self(v & Self::MASK)
+            }
+        }
+    };
+    // Helper: generate conversions_as based on type
+    (@conversions_as u8) => {
+        /// Return value as u8
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn as_u8(&self) -> u8 {
+            self.0 as u8
+        }
+
+        secret_key!(@conversions_as u16);
+    };
+    (@conversions_as u16) => {
+        /// Return value as u16
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn as_u16(&self) -> u16 {
+            self.0 as u16
+        }
+
+        secret_key!(@conversions_as u32);
+    };
+    (@conversions_as u32) => {
+        /// Return value as u32
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn as_u32(&self) -> u32 {
+            self.0 as u32
+        }
+
+        secret_key!(@conversions_as u64);
+    };
+    (@conversions_as u64) => {
+        /// Return value as u64
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn as_u64(&self) -> u64 {
+            self.0 as u64
+        }
+    };
+    (@conversions_from u8 $int:tt) => {
+        /// Create value from u8
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn from_u8(key: u8) -> Self {
+            Self(key as $int & Self::MASK)
+        }
+    };
+    (@conversions_from u16 $int:tt) => {
+        /// Create value from u16
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn from_u16(key: u16) -> Self {
+            Self(key as $int & Self::MASK)
+        }
+
+        secret_key!(@conversions_from u8 $int);
+    };
+    (@conversions_from u32 $int:tt) => {
+        /// Create value from u32
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn from_u32(key: u32) -> Self {
+            Self(key as $int & Self::MASK)
+        }
+
+        secret_key!(@conversions_from u16 $int);
+    };
+    (@conversions_from u64 $int:tt) => {
+        /// Create value from u64
+        #[allow(dead_code)]
+        #[inline]
+        #[must_use]
+        pub const fn from_u64(key: u64) -> Self {
+            Self(key & Self::MASK)
+        }
+
+        secret_key!(@conversions_from u32 $int);
+    }
+}

des/src/key/subkey.rs

@@ -1,6 +1,6 @@
-use crate::key::secret_int;
+use crate::key::secret_key;
 
-secret_int! {
+secret_key! {
     /// A single DES round subkey (48 bits stored in lower bits of u64).
     pub struct Subkey(u64, 48, 0x0000_FFFF_FFFF_FFFF);
 }

des/src/key/subkeys.rs

@@ -3,8 +3,8 @@ use crate::{
     key::{Key, cd56::CD56, key56::Key56, subkey::Subkey},
     utils::permutate,
 };
-use cipher_core::CipherResult;
 use std::{
+    array,
     fmt::Debug,
     iter::Rev,
     ops::Index,
@@ -12,79 +12,52 @@ use std::{
 };
 
 /// Container for all 16 round subkeys; zeroized on drop.
+#[derive(Default)]
 pub struct Subkeys([Subkey; 16]);
 
 impl Subkeys {
-    #[inline]
+    /// Generates 16 round subkeys from the given key.
     #[must_use]
-    pub const fn new_empty() -> Self {
+    pub fn from_key(key: &Key) -> Self {
-        Self([const { Subkey::zero() }; 16])
-    }
-
-    #[inline]
-    #[must_use]
-    pub const fn as_array(&self) -> &[Subkey; 16] {
-        &self.0
-    }
-
-    #[inline]
-    #[must_use]
-    pub fn get(&self, idx: usize) -> Option<&Subkey> {
-        self.0.get(idx)
-    }
-
-    /// # Errors
-    /// # Panics
-    pub fn from_key(key: &Key) -> CipherResult<Self> {
         let mut cd56 = pc1(key).split(); // 56-bit: C0 + D0
 
-        let subkeys = ROUND_ROTATIONS
+        let subkeys = array::from_fn(|idx| {
-            .iter()
+            cd56.rotate_left(ROUND_ROTATIONS[idx]);
-            .map(|&shift_amount| {
-                cd56.rotate_left(shift_amount);
             pc2(&cd56)
-            })
+        });
-            .collect::<Vec<Subkey>>()
-            .try_into()
-            .expect("Exactly 16 subkeys expected");
 
-        Ok(Self(subkeys))
+        Self(subkeys)
     }
 
-    /// Borrowing forward iterator.
+    /// Returns an iterator over the subkeys.
     pub fn iter(&self) -> Iter<'_, Subkey> {
         self.0.iter()
     }
 
-    /// Borrowing reverse iterator.
+    /// Returns a reverse iterator over the subkeys.
     pub fn iter_rev(&self) -> Rev<Iter<'_, Subkey>> {
         self.0.iter().rev()
     }
 
-    /// Mutable iterator if you need it.
+    /// Returns a mutable iterator over the subkeys.
     pub fn iter_mut(&mut self) -> IterMut<'_, Subkey> {
         self.0.iter_mut()
     }
-
-    /// Consume `self` and return a new `Subkeys` with reversed order.
-    #[must_use]
-    pub const fn reversed(mut self) -> Self {
-        self.0.reverse();
-        self
-    }
 }
 
+/// Initial permutation (PC-1): 64-bit -> 56-bit.
 #[inline]
 #[must_use]
 fn pc1(key: &Key) -> Key56 {
     permutate(key.as_u64(), 64, 56, &PC1).into()
 }
 
+/// Compression permutation (PC-2): 56-bit -> 48-bit.
 #[inline]
 #[must_use]
 fn pc2(cd: &CD56) -> Subkey {
     let key56 = Key56::from(cd);
-    permutate(key56.as_int(), 56, 48, &PC2).into()
+    permutate(key56.as_u64(), 56, 48, &PC2).into()
 }
 
 impl<'a> IntoIterator for &'a Subkeys {
@@ -116,12 +89,6 @@ impl Debug for Subkeys {
     }
 }
 
-impl Default for Subkeys {
-    fn default() -> Self {
-        Self::new_empty()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -132,7 +99,7 @@ mod tests {
     #[rstest]
     #[case(TEST_KEY, 0x00F0_CCAA_F556_678F)]
     fn pc1_permutaion_correct(#[case] key: u64, #[case] expected: u64) {
-        let result = pc1(&key.into()).as_int();
+        let result = pc1(&key.into()).as_u64();
         assert_eq!(
             result, expected,
             "PC1 permutation failed. Expected {expected:08X}, got {result:08X}",
@@ -159,7 +126,7 @@ mod tests {
     #[case(0x00F0_CCAA_F556_678F, 0xCB3D_8B0E_17F5)] // K_16
     fn pc2_permutaion(#[case] before: u64, #[case] expected: u64) {
         let key56 = Key56::from(before).split();
-        let result = pc2(&key56).as_int();
+        let result = pc2(&key56).as_u64();
         assert_eq!(
             result, expected,
             "PC2 permutation failed. Expected {expected:016X}, got {result:016X}"

des/tests/des.rs

@@ -114,7 +114,7 @@ fn encrypt_decrypt_roundtrip(
     #[case] expected_ciphertext: u64,
     #[case] key: u64,
 ) {
-    let des = assert_ok!(Des::new(key), "Valid DES key");
+    let des = Des::new(key);
 
     let ciphertext = assert_ok!(des.encrypt(&plaintext.to_be_bytes()));
     let dectrypted = assert_ok!(des.decrypt(&ciphertext));