diff --git a/crypto/core-interface/src/key_material.rs b/crypto/core-interface/src/key_material.rs index 69bbca1..f233344 100644 --- a/crypto/core-interface/src/key_material.rs +++ b/crypto/core-interface/src/key_material.rs @@ -559,6 +559,9 @@ impl KeyMaterial for KeyMaterialInternal { Ok(()) } + // + // Q. T. Felix - start + // fn concatenate(&mut self, other: &dyn KeyMaterial) -> Result { let new_key_len = self.key_len() + other.key_len(); if self.key_len() + other.key_len() > KEY_LEN { @@ -566,10 +569,17 @@ impl KeyMaterial for KeyMaterialInternal { } 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() { diff --git a/crypto/utils/src/ct.rs b/crypto/utils/src/ct.rs index eb89600..87f5b71 100644 --- a/crypto/utils/src/ct.rs +++ b/crypto/utils/src/ct.rs @@ -38,7 +38,14 @@ impl Condition { // 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 for Condition ? - 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() -> Self { @@ -112,12 +119,54 @@ impl Condition { } // 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,6 +185,58 @@ impl Condition { } } +// +// Q. T. Felix - start +// +/// # PR Condition u64 +/// +/// Although the [Condition] struct is defined, the actual implementation was only written for `Condition`. +/// 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`에 대해서만 작성되어 있음 +/// `supported_mask_type!` 매크로에는 u64가 포함되어 있음에도 불구하고 u64를 위한 메소드들이 누락되어 있어 +/// u64 타입의 상수 시간(constant-time) 연산을 수행할 수 없는 상태였음 +/// 위 문제를 해결하기 위해 해당 구조체를 구현하여 u64 지원 추가함 +impl Condition { + // 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() -> 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 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, @@ -143,82 +244,82 @@ impl Condition { // impl Condition { // pub const TRUE: Self = Self(1); // pub const FALSE: Self = Self(0); -// +// // pub const fn new() -> 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::::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 // } diff --git a/crypto/utils/tests/ct_tests.rs b/crypto/utils/tests/ct_tests.rs index baf130e..b784ef4 100644 --- a/crypto/utils/tests/ct_tests.rs +++ b/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::::or_halves(0), 0); + + // Lower 32 bits should be preserved + assert_eq!(Condition::::or_halves(1), 1); + assert_eq!(Condition::::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::::or_halves(1 << 32), 1); + + // Mixed case: Upper 0x10000000 | Lower 0x00000001 => 0x10000001 + assert_eq!(Condition::::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::::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::::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::::TRUE.is_true(), true); + assert_eq!(Condition::::FALSE.is_true(), false); + } + + #[test] + fn from_bool() { + // Compile-time const generics check + assert_eq!(Condition::::from_bool::().is_true(), true); + assert_eq!(Condition::::from_bool::().is_true(), false); + } + + #[test] + fn select() { + let t = Condition::::TRUE; + let f = Condition::::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::::from_bool::(); + assert_eq!(t_gen.select(val1, val2), val1); + } + + #[test] + fn bit_ops() { + let t = Condition::::TRUE; + let f = Condition::::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::*;