Skip to content

Commit 9bdec01

Browse files
committed
fix(permutation): wrap secret locals in Zeroizing for unwind safety
Follow-up to the constant-time fix. The explicit `kv.zeroize()` and `input.zeroize()` calls only run on the success path; if any operation between secret extraction and the wipe ever unwinds, the secret bytes would be left on the stack. None of the current operations between the bookends can panic, but encoding unwind-safety in the type system is strictly better than relying on that invariant holding forever. Wrap `input`, `kv`, `selected`, and `src` in `Zeroizing<_>` so the wipe is performed by `Drop`, guaranteed on every exit including panic. The explicit `.zeroize()` calls become redundant and are removed. Verified with the trailofbits zeroize-audit skill: volatile-zero stores in the LLVM IR are intact at O3 (10) and not reduced from O0 (8). The ct_analyzer still PASSES on arm64. Permutation correctness tests pass.
1 parent 39a9445 commit 9bdec01

1 file changed

Lines changed: 16 additions & 20 deletions

File tree

packages/permutation/src/elementwise.rs

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::{private::IsPermutable, PermutationKey};
22
use subtle::{ConditionallySelectable, ConstantTimeEq};
33
use vitaminc_protected::{Controlled, Zeroed};
4-
use zeroize::Zeroize;
4+
use zeroize::{Zeroize, Zeroizing};
55

66
// TODO: Make this a private trait
77
// FIXME: This trait is backwards - self should be T and the argument should be a key
@@ -35,53 +35,49 @@ where
3535
}
3636

3737
#[inline]
38-
pub fn permute_array<const N: usize, T>(key: &PermutationKey<N>, mut input: [T; N]) -> [T; N]
38+
pub fn permute_array<const N: usize, T>(key: &PermutationKey<N>, input: [T; N]) -> [T; N]
3939
where
4040
[T; N]: IsPermutable + Zeroed,
4141
T: Zeroize + Copy + ConditionallySelectable,
4242
{
4343
// Constant-time scan: for each output position `i`, the key byte `kv`
4444
// selects which input element to copy. We scan all `j` in 0..N and use
4545
// `ConditionallySelectable` so the access pattern is independent of `kv`,
46-
// preventing cache-line timing leaks of the secret key bytes.
46+
// preventing cache-line timing leaks of the secret key bytes. Secret
47+
// locals are wrapped in `Zeroizing` so they are wiped on any unwind path.
48+
let input = Zeroizing::new(input);
4749
let mut out: [T; N] = Zeroed::zeroed();
4850
for (i, k) in key.iter().enumerate() {
49-
let mut kv: u8 = k.risky_unwrap();
50-
let mut selected: T = input[0];
51+
let kv = Zeroizing::new(k.risky_unwrap());
52+
let mut selected = Zeroizing::new(input[0]);
5153
for (j, src) in input.iter().enumerate().skip(1) {
52-
let mask = (j as u8).ct_eq(&kv);
54+
let mask = (j as u8).ct_eq(&*kv);
5355
selected.conditional_assign(src, mask);
5456
}
55-
out[i] = selected;
56-
kv.zeroize();
57+
out[i] = *selected;
5758
}
58-
59-
// We copied all elements to the output so we should zeroize the input
60-
input.zeroize();
6159
out
6260
}
6361

6462
#[inline]
65-
pub fn depermute_array<const N: usize, T>(key: &PermutationKey<N>, mut input: [T; N]) -> [T; N]
63+
pub fn depermute_array<const N: usize, T>(key: &PermutationKey<N>, input: [T; N]) -> [T; N]
6664
where
6765
[T; N]: IsPermutable + Zeroed,
6866
T: Zeroize + Copy + ConditionallySelectable,
6967
{
7068
// Constant-time scatter: for each (i, kv), write `input[i]` to `out[kv]`
7169
// by scanning every output slot and conditionally assigning when `j == kv`.
70+
// Secret locals are wrapped in `Zeroizing` for unwind safety.
71+
let input = Zeroizing::new(input);
7272
let mut out: [T; N] = Zeroed::zeroed();
7373
for (i, k) in key.iter().enumerate() {
74-
let mut kv: u8 = k.risky_unwrap();
75-
let src = input[i];
74+
let kv = Zeroizing::new(k.risky_unwrap());
75+
let src = Zeroizing::new(input[i]);
7676
for (j, dst) in out.iter_mut().enumerate() {
77-
let mask = (j as u8).ct_eq(&kv);
78-
dst.conditional_assign(&src, mask);
77+
let mask = (j as u8).ct_eq(&*kv);
78+
dst.conditional_assign(&*src, mask);
7979
}
80-
kv.zeroize();
8180
}
82-
83-
// We copied all elements to the output so we should zeroize the input
84-
input.zeroize();
8581
out
8682
}
8783

0 commit comments

Comments
 (0)