Fix KeyMaterial::concatenate logic and improve Condition<T> correctness for i64/u64

crypto/core-interface/src/key_material.rs

@@ -559,17 +559,27 @@ impl<const KEY_LEN: usize> KeyMaterial for KeyMaterialInternal<KEY_LEN> {
         Ok(())
     }
 
+    //
+    // Q. T. Felix - start
+    //
     fn concatenate(&mut self, other: &dyn KeyMaterial) -> Result<usize, KeyMaterialError> {
         let new_key_len = self.key_len() + other.key_len();
         if self.key_len() + other.key_len() > KEY_LEN {
             return Err(KeyMaterialError::InputDataLongerThanKeyCapacity);
         }
         self.buf[self.key_len..new_key_len].copy_from_slice(other.ref_to_bytes());
         self.key_len += other.key_len();
-        self.key_type = max(&self.key_type, &other.key_type()).clone();
-        self.security_strength = max(&self.security_strength, &other.security_strength()).clone();
+        // Q. T. Felix NOTE: concatenate 트레이트 Docstring에는 고품질 키와 저품질 키를 섞으면
+        //                   보안성이 낮은 쪽(Low Entropy)으로 하향 평준화되어야 한다고 명시되어 있음
+        //                   하지만 아래 실제 구현에는 bouncycastle_utils::max 함수로 Docstring 내용과
+        //                   정확하게 반대됨.
+        self.key_type = min(&self.key_type, &other.key_type()).clone();
+        self.security_strength = min(&self.security_strength, &other.security_strength()).clone();
         Ok(self.key_len())
     }
+    //
+    // Q. T. Felix - end
+    //
 
     fn equals(&self, other: &dyn KeyMaterial) -> bool {
         if self.key_len() != other.key_len() {

crypto/utils/src/ct.rs

@@ -38,7 +38,14 @@ impl Condition<i64> {
     // MikeO: TODO: there are a bunch of impls in here that seem to be generic and not related to i64,
     // MikeO: TODO: could those be moved to a generic impl<T> for Condition<T> ?
 
-    pub const TRUE: Self = Self(1);
+    //
+    // Q. T. Felix - start
+    //
+    // Q. T. Felix NOTE: For this conditional negation formula to work, the mask value for TRUE must be -1 (Hex: 0xFFFF...), where all bits are set to 1
+    pub const TRUE: Self = Self(-1);
+    //
+    // Q. T. Felix - end
+    //
     pub const FALSE: Self = Self(0);
 
     pub const fn from_bool<const VALUE: bool>() -> Self {
@@ -112,12 +119,54 @@ impl Condition<i64> {
     }
 
     // MikeO: TODO: I have no idea what this does, .negate(-1) seems to give -3 ?? Is that a bug?
+    /// ## negate seems to give -3
+    ///
+    /// `value` is `-1` (i.e., all bits are `1`, `...1111`)
+    ///
+    /// Condition `self.0` is 1 (`...0001`) (assuming `TRUE`)
+    ///
+    /// XOR operation was executed as `value ^ self.0`
+    ///
+    /// Then `...1111 XOR ...0001 = ...1110` (i.e., `-2`)
+    ///
+    /// Subtraction operation is `wrapping_sub(self.0)`
+    ///
+    /// Then `-2 - 1 = -3`
+    ///
+    /// As a result, `1`, which is the negation of `-1`, should be returned, but `-3` is output.
+    ///
+    /// Therefore, if the [Self::TRUE] constant value of the i64 [Condition] implementation is changed to `-1`,
+    /// the test also runs normally.
+    ///
+    /// ---
+    ///
+    /// kor
+    ///
+    /// 입력값 `value`가 `-1` (즉 모든 비트가 `1`, `...1111`)
+    ///
+    /// 조건 `self.0`은 1 (`...0001`) (`TRUE`라고 가정하고)
+    ///
+    /// XOR 연산은 `value ^ self.0`로 실행하셧음
+    ///
+    /// 그러면 `...1111 XOR ...0001 = ...1110` (즉 `-2`)
+    ///
+    /// 뺄셈 연산은 `wrapping_sub(self.0)`
+    ///
+    /// 그럼? `-2 - 1 = -3`
+    ///
+    /// 결과적으로 `-1`의 부정인` 1`이 나와야 하는데 `-3`이 출력됩니다
+    ///
+    /// 그래서 i64 [Condition] 구현체의 [Self::TRUE] 상수 값을 `-1`로 변경하면
+    /// 테스트도 정상적으로 수행됩니다.
+    ///
+    /// ---
+    ///
     /// Conditionally negate the value.
     pub const fn negate(self, value: i64) -> i64 {
         (value ^ self.0).wrapping_sub(self.0)
     }
 
-    const fn or_halves(value: i64) -> i64 {
+    pub const fn or_halves(value: i64) -> i64 {
         (value | (value >> 32)) & 0xFFFFFFFF
     }
 
@@ -136,89 +185,141 @@ impl Condition<i64> {
     }
 }
 
+//
+// Q. T. Felix - start
+//
+/// # PR Condition u64
+///
+/// Although the [Condition] struct is defined, the actual implementation was only written for `Condition<i64>`.
+/// Even though `u64` is included in the `supported_mask_type!` macro, methods for `u64` were missing,
+/// making it impossible to perform constant-time operations for the `u64` type.
+/// To resolve this issue, this implementation adds support for `u64`.
+///
+/// ---
+///
+/// kor
+///
+/// [Condition] 구조체는 정의되어 있지만 실제 구현체는 `Condition<i64>`에 대해서만 작성되어 있음
+/// `supported_mask_type!` 매크로에는 u64가 포함되어 있음에도 불구하고 u64를 위한 메소드들이 누락되어 있어
+/// u64 타입의 상수 시간(constant-time) 연산을 수행할 수 없는 상태였음
+/// 위 문제를 해결하기 위해 해당 구조체를 구현하여 u64 지원 추가함
+impl Condition<u64> {
+    // Q. T. Felix NOTE: so strict definition of true/false constants for u64
+    //                   While i64 used 1 and 0, for mask generation logic we ensure consistency
+    //                   TRUE must be all-ones (u64::MAX) to work correctly with bitwise select logic.
+    pub const TRUE: Self = Self(u64::MAX);
+    pub const FALSE: Self = Self(0);
+
+    // Q. T. Felix NOTE: so this is the core logic for constant-time mask generation for unsigned integers
+    //                   Unlike signed integers where we can rely on Two's Complement via negation `-(v as i64)`,
+    //                   for u64 we must use wrapping subtraction to achieve the all-ones bit pattern (u64::MAX) for true
+    pub const fn from_bool<const VALUE: bool>() -> Self {
+        // If VALUE is true (1) -> 0 - 1 = u64::MAX (All 1s)
+        // If VALUE is false (0) -> 0 - 0 = 0 (All 0s)
+        Self(0u64.wrapping_sub(VALUE as u64))
+    }
+
+    // Q. T. Felix NOTE: so we implement the select function manually for u64
+    //                   This resolves the TODO by covering the missing primitive implementation,
+    //                   although a fully generic impl<T> would be the ultimate long-term goal
+    pub fn select(self, a: u64, b: u64) -> u64 {
+        let mask = self.0;
+        (a & mask) | (b & !mask)
+    }
+
+    // Q. T. Felix NOTE: so we provide a method to check if the condition effectively resolves to true
+    //                   by checking the underlying mask value
+    pub fn is_true(&self) -> bool {
+        self.0 != 0
+    }
+}
+//
+// Q. T. Felix - end
+//
+
 // TODO: ... this doesn't ... work. We should get this working and then then do u8.
 // TODO: then and change Hex and Base64 to use this.
 // TODO: (there's probably no noticeable performance difference u8 and u64 bit ops on a 64-bit machine,
 // TODO:  but there would be on a 8, 16, or 32-bit machine.)
 // impl Condition<u64> {
 //     pub const TRUE: Self = Self(1);
 //     pub const FALSE: Self = Self(0);
-// 
+//
 //     pub const fn new<const VALUE: bool>() -> Self {
 //         Self((VALUE as u64).wrapping_neg())
 //     }
-// 
+//
 //     pub const fn from_bool(value: bool) -> Self {
 //         Self((value as u64).wrapping_neg())
 //     }
-// 
+//
 //     pub const fn is_bit_set(value: u64, bit: u64) -> Self {
 //         Self(((value >> bit) & 1).wrapping_neg())
 //     }
-// 
+//
 //     // MikeO: TODO ?? What does "negative" mean for an unsigned value?
 //     pub const fn is_negative(value: u64) -> Self {
 //         Self(((value as i64) >> 63) as u64)
 //     }
-// 
+//
 //     pub const fn is_not_zero(value: u64) -> Self {
 //         Self::is_negative(Self::or_halves(value).wrapping_neg())
 //     }
-// 
+//
 //     pub const fn is_zero(value: u64) -> Self {
 //         Self::is_negative(Self::or_halves(value).wrapping_sub(1))
 //     }
-// 
+//
 //     // MikeO: TODO: I borrowed this formula from Botan, but rust complains about u64 subtraction overflow if x < y, so this works in C but won't work in rust.
 //     // MikeO: TODO: I played with u64.wrapping_sub(y) but that doesn't work either.
 //     pub const fn is_lt(x: u64, y: u64) -> Self {
 //         Self::is_zero(x ^ ((x ^ y) | (x.wrapping_sub(y)) ^ x))
 //     }
-// 
+//
 //     // Note: haven't found a clever way to make this const, since it either needs a (non-const) not (!) or a boolean OR is_zero.
 //     // pub fn is_lte(x: i64, y: i64) -> Self { !Self::is_gt(x, y) }
-// 
+//
 //     // pub const fn is_gt(x: i64, y: i64) -> Self { Self::is_lt(y, x) }
-// 
+//
 //     // Note: haven't found a clever way to make this const, since it either needs a (non-const) not (!) or a boolean OR is_zero.
 //     // pub fn is_gte(x: i64, y: i64) -> Self { !Self::is_lt(x, y) }
-// 
+//
 //     pub fn is_in_list(value: u64, list: &[u64]) -> Self {
 //         // Research question: is this actually constant-time?
 //         // A clever compiler might turn this into a short-circuiting loop.
 //         // A quick google search shows that rust doesn't have the ability to annotate specific code blocks
 //         // as no-optimize; the only option is to insert direct assembly.
-// 
+//
 //         let mut c = Self::FALSE;
 //         for i in 0..list.len() {
 //             let diff = value ^ list[i];
 //             c |= Condition::<u64>::is_zero(diff);
 //         }
-// 
+//
 //         c
 //     }
-// 
+//
 //     pub fn mov(self, src: u64, dst: &mut u64) {
 //         *dst = self.select(src, *dst);
 //     }
-// 
+//
 //     // MikeO: TODO: This needs a docstring because I have no idea what this does.
 //     pub const fn negate(self, value: u64) -> u64 {
 //         (value ^ self.0).wrapping_sub(self.0)
 //     }
-// 
+//
 //     const fn or_halves(value: u64) -> u64 {
 //         (value & 0xFFFFFFFF) | (value >> 32)
 //     }
-// 
+//
 //     pub const fn select(self, true_value: u64, false_value: u64) -> u64 {
 //         (true_value & self.0) | (false_value & !self.0)
 //     }
-// 
+//
 //     pub const fn swap(self, lhs: u64, rhs: u64) -> (u64, u64) {
 //         (self.select(rhs, lhs), self.select(lhs, rhs))
 //     }
-// 
+//
 //     pub const fn to_bool_var(self) -> bool {
 //         self.0 != 0
 //     }

crypto/utils/tests/ct_tests.rs

@@ -154,10 +154,42 @@ mod i64_tests {
     }
 
     // MikeO: TODO: I don't understand what this function does well enough to test it.
+    //
+    // Q. T. Felix - start
+    //
     #[test]
     fn test_or_halves() {
-        todo!()
+        // 0 input -> 0 output
+        assert_eq!(Condition::<i64>::or_halves(0), 0);
+
+        // Lower 32 bits should be preserved
+        assert_eq!(Condition::<i64>::or_halves(1), 1);
+        assert_eq!(Condition::<i64>::or_halves(0x12345678), 0x12345678);
+
+        // Upper 32 bits should be folded into lower 32 bits
+        // (1 << 32) OR (1 << 32 >> 32) => 0 OR 1 => 1
+        assert_eq!(Condition::<i64>::or_halves(1 << 32), 1);
+
+        // Mixed case: Upper 0x10000000 | Lower 0x00000001 => 0x10000001
+        assert_eq!(Condition::<i64>::or_halves(0x10000000_00000001), 0x10000001);
+
+        // Negative number check (-1)
+        // -1 is 0xFFFF...FFFF
+        // (-1 >> 32) is -1 (Arithmetic shift preserves sign)
+        // (-1 | -1) is -1
+        // -1 & 0xFFFFFFFF is 0x00000000FFFFFFFF (i64 value: 4294967295)
+        assert_eq!(Condition::<i64>::or_halves(-1), 0xFFFFFFFF);
+
+        // i64::MIN check (Only MSB set)
+        // i64::MIN = 0x80000000_00000000
+        // (val >> 32) = 0xFFFFFFFF_80000000 (Sign extension)
+        // (val | shifted) = 0xFFFFFFFF_80000000
+        // (& mask) = 0x00000000_80000000
+        assert_eq!(Condition::<i64>::or_halves(i64::MIN), 0x80000000);
     }
+    //
+    // Q. T. Felix - end
+    //
 
     #[test]
     fn test_select() {
@@ -186,6 +218,74 @@ mod i64_tests {
     }
 }
 
+//
+// Q. T. Felix - start
+//
+#[cfg(test)]
+mod u64_tests {
+    use super::*;
+
+    #[test]
+    fn const_tests() {
+        // Q. T. Felix NOTE: Ensure TRUE/FALSE are correctly interpreted as boolean.
+        assert_eq!(Condition::<u64>::TRUE.is_true(), true);
+        assert_eq!(Condition::<u64>::FALSE.is_true(), false);
+    }
+
+    #[test]
+    fn from_bool() {
+        // Compile-time const generics check
+        assert_eq!(Condition::<u64>::from_bool::<true>().is_true(), true);
+        assert_eq!(Condition::<u64>::from_bool::<false>().is_true(), false);
+    }
+
+    #[test]
+    fn select() {
+        let t = Condition::<u64>::TRUE;
+        let f = Condition::<u64>::FALSE;
+
+        let val1: u64 = 0xDEADBEEFCAFEBABE;
+        let val2: u64 = 0x0000000000000000;
+
+        // Q. T. Felix NOTE: This test is CRITICAL.
+        //                   If TRUE was defined as '1' (like i64), this would fail because 'select' relies on bitwise mask.
+        //                   It requires TRUE to be u64::MAX (all 1s) to preserve the full bits of val1.
+        assert_eq!(t.select(val1, val2), val1);
+        assert_eq!(f.select(val1, val2), val2);
+
+        // Cross check with from_bool
+        let t_gen = Condition::<u64>::from_bool::<true>();
+        assert_eq!(t_gen.select(val1, val2), val1);
+    }
+
+    #[test]
+    fn bit_ops() {
+        let t = Condition::<u64>::TRUE;
+        let f = Condition::<u64>::FALSE;
+
+        // NOT
+        assert_eq!((!t).is_true(), false);
+        assert_eq!((!f).is_true(), true);
+
+        // AND
+        assert_eq!((t & t).is_true(), true);
+        assert_eq!((t & f).is_true(), false);
+        assert_eq!((f & f).is_true(), false);
+
+        // OR
+        assert_eq!((t | t).is_true(), true);
+        assert_eq!((t | f).is_true(), true);
+        assert_eq!((f | f).is_true(), false);
+
+        // XOR
+        assert_eq!((t ^ t).is_true(), false);
+        assert_eq!((t ^ f).is_true(), true);
+    }
+} // AlLpAsS
+//
+// Q. T. Felix - end
+//
+
 // #[cfg(test)]
 // mod u64_tests {
 //     use super::*;