Harden secret wrappers and CT anchors

Author: eldryoth

docs/UNSAFE.md

@@ -194,15 +194,17 @@ Status: active secret conversion helper when `alloc` is enabled.
 
 Purpose:
 
-- Convert bytes already validated as UTF-8 into `String` without a second
-  fallible conversion after the vector has been moved out of `SecretBuffer`.
+- Convert bytes already validated as UTF-8, or bytes just extracted from a
+  valid `String`, back into `String` without a second fallible conversion after
+  the vector has crossed a secret-wrapper boundary.
 - Avoid an intermediate raw `Vec<u8>` drop path that would not run the crate's
   best-effort cleanup on panic.
 
 Preconditions:
 
 - Caller must validate the exact vector contents as UTF-8 immediately before
-  handing ownership to this helper.
+  handing ownership to this helper, or pass bytes produced directly by
+  `String::into_bytes` without modifying the initialized prefix.
 - The vector must not be modified between validation and conversion.
 
 Unsafe operation:
@@ -214,6 +216,9 @@ Safety argument:
 
 - `SecretBuffer::try_into_exposed_string` validates `self.expose_secret()` with
   `core::str::from_utf8` before moving the same allocation into this helper.
+- `ExposedSecretString::from_string` receives a valid `String`, converts it to
+  bytes, wipes only spare capacity past the initialized UTF-8 prefix, and then
+  moves the same initialized prefix into this helper.
 - No mutation occurs between validation and conversion.
 - The returned `ExposedSecretString` retains redacted formatting and drop-time
   cleanup.
@@ -262,6 +267,44 @@ Limitations:
   microarchitecture certification.
 - Unsupported architectures fall back to the compiler fence only.
 
+### `ct_decode_alphabet_byte`
+
+Location: `src/lib.rs`
+
+Status: active constant-time-oriented alphabet scanner.
+
+Purpose:
+
+- Decode one Base64 symbol by scanning all 64 alphabet entries instead of
+  indexing a decode table or returning at the first match.
+- Keep the decoded-value and validity accumulators observable to the optimizer
+  on every iteration of the fixed scan.
+
+Preconditions:
+
+- `A::ENCODE` is a validated 64-byte Base64 alphabet. Built-in alphabets and
+  the `define_alphabet!` macro enforce this.
+
+Unsafe operation:
+
+- `core::ptr::read_volatile` reads initialized local `decoded` and `valid`
+  accumulators after each OR reduction.
+
+Safety argument:
+
+- `decoded` and `valid` are initialized stack-local `u8` values for the entire
+  loop.
+- The volatile reads do not read from caller memory and cannot violate bounds
+  or aliasing requirements.
+- The function remains `#[inline(never)]` so generated-code review can inspect
+  the scanner as a distinct helper.
+
+Limitations:
+
+- These volatile reads are optimizer barriers, not a formal proof of
+  microarchitectural constant-time behavior. Release evidence and dudect remain
+  required for high-assurance review.
+
 ### `wipe_vec_spare_capacity`
 
 Location: `src/lib.rs`

scripts/validate-unsafe-boundary.sh

@@ -20,24 +20,24 @@ if [ "$matches" != "$allowed" ]; then
 fi
 
 root_allow_count="$(grep -c '^#\[allow(unsafe_code)\]$' "$root_allowed" || true)"
-if [ "$root_allow_count" -ne 6 ]; then
-    echo "unsafe boundary: src/lib.rs must have exactly six reviewed allow(unsafe_code) helpers"
+if [ "$root_allow_count" -ne 7 ]; then
+    echo "unsafe boundary: src/lib.rs must have exactly seven reviewed allow(unsafe_code) helpers"
     exit 1
 fi
 
 if ! awk '
     /^#\[allow\(unsafe_code\)\]$/ {
         allow_line = NR
     }
-    /^fn wipe_bytes\(/ || /^fn wipe_barrier\(/ || /^fn wipe_vec_spare_capacity\(/ || /^fn ct_error_gate_barrier\(/ || /^fn constant_time_eq_same_len\(/ || /^fn string_from_validated_secret_bytes\(/ {
+    /^fn wipe_bytes\(/ || /^fn wipe_barrier\(/ || /^fn wipe_vec_spare_capacity\(/ || /^fn ct_error_gate_barrier\(/ || /^fn constant_time_eq_same_len\(/ || /^fn string_from_validated_secret_bytes\(/ || /^fn ct_decode_alphabet_byte/ {
         if (allow_line != NR - 1) {
             failed = 1
         }
         seen += 1
     }
-    END { exit failed || seen != 6 }
+    END { exit failed || seen != 7 }
 ' "$root_allowed"; then
-    echo "unsafe boundary: src/lib.rs allow(unsafe_code) must apply only to reviewed cleanup, secret-conversion, comparison, and CT gate helpers"
+    echo "unsafe boundary: src/lib.rs allow(unsafe_code) must apply only to reviewed cleanup, secret-conversion, comparison, CT scan, and CT gate helpers"
     exit 1
 fi
 

src/lib.rs

@@ -3671,8 +3671,9 @@ pub struct SecretBuffer {
 
 /// Owned secret bytes extracted from [`SecretBuffer`].
 ///
-/// This wrapper keeps redacted formatting and best-effort drop-time cleanup
-/// after a [`SecretBuffer`] is consumed for owned interop. Use
+/// This wrapper keeps redacted formatting, best-effort spare-capacity clearing
+/// at construction time, and best-effort full wipe on drop after a
+/// [`SecretBuffer`] is consumed for owned interop. Use
 /// [`Self::into_exposed_unprotected_vec_caller_must_zeroize`] only when a raw
 /// `Vec<u8>` is unavoidable and the caller will handle cleanup.
 #[cfg(feature = "alloc")]
@@ -3771,8 +3772,9 @@ impl AsMut<[u8]> for ExposedSecretVec {
 
 /// Owned secret UTF-8 text extracted from [`SecretBuffer`].
 ///
-/// This wrapper keeps redacted formatting and best-effort drop-time cleanup
-/// after a [`SecretBuffer`] is consumed for string interop. Use
+/// This wrapper keeps redacted formatting, best-effort spare-capacity clearing
+/// at construction time, and best-effort full wipe on drop after a
+/// [`SecretBuffer`] is consumed for string interop. Use
 /// [`Self::into_exposed_unprotected_string_caller_must_zeroize`] only when a
 /// raw `String` is unavoidable and the caller will handle cleanup.
 #[cfg(feature = "alloc")]
@@ -3785,6 +3787,9 @@ impl ExposedSecretString {
     /// Wraps an owned UTF-8 string as exposed secret text.
     #[must_use]
     pub fn from_string(text: alloc::string::String) -> Self {
+        let mut bytes = text.into_bytes();
+        wipe_vec_spare_capacity(&mut bytes);
+        let text = string_from_validated_secret_bytes(bytes);
         Self { text }
     }
 
@@ -5084,7 +5089,11 @@ fn constant_time_eq_same_len(left: &[u8], right: &[u8]) -> bool {
         diff = unsafe { core::ptr::read_volatile(&raw const diff) };
     }
     ct_error_gate_barrier(diff, 0);
-    diff == 0
+    // SAFETY: `diff` is an initialized local `u8`; this final volatile read
+    // keeps the public equality comparison dependent on a post-barrier load of
+    // the accumulated value.
+    let result = unsafe { core::ptr::read_volatile(&raw const diff) };
+    result == 0
 }
 
 #[cfg(feature = "alloc")]
@@ -7973,7 +7982,10 @@ fn ct_decode_padded_in_place<A: Alphabet>(buffer: &mut [u8]) -> Result<usize, De
         wipe_bytes(buffer);
         return Err(DecodeError::InvalidInput);
     }
-    report_ct_error(invalid_byte, invalid_padding)?;
+    if let Err(err) = report_ct_error(invalid_byte, invalid_padding) {
+        wipe_bytes(buffer);
+        return Err(err);
+    }
     Ok(write)
 }
 
@@ -8137,7 +8149,10 @@ fn ct_decode_unpadded_in_place<A: Alphabet>(buffer: &mut [u8]) -> Result<usize,
         wipe_bytes(buffer);
         return Err(DecodeError::InvalidInput);
     }
-    report_ct_error(invalid_byte, invalid_padding)?;
+    if let Err(err) = report_ct_error(invalid_byte, invalid_padding) {
+        wipe_bytes(buffer);
+        return Err(err);
+    }
     Ok(write)
 }
 
@@ -8184,15 +8199,30 @@ fn read_tail_or_mark_invalid<'a>(
 }
 
 #[inline(never)]
+#[allow(unsafe_code)]
 fn ct_decode_alphabet_byte<A: Alphabet>(byte: u8) -> (u8, u8) {
     let mut decoded = 0u8;
     let mut valid = 0u8;
     let mut candidate = 0u8;
 
     while candidate < 64 {
-        let matches = ct_mask_eq_u8(byte, A::ENCODE[candidate as usize]);
-        decoded |= candidate & matches;
-        valid |= matches;
+        let matches = core::hint::black_box(ct_mask_eq_u8(
+            core::hint::black_box(byte),
+            core::hint::black_box(A::ENCODE[candidate as usize]),
+        ));
+        decoded = core::hint::black_box(
+            core::hint::black_box(decoded) | core::hint::black_box(candidate & matches),
+        );
+        // SAFETY: `decoded` is an initialized local `u8`; the volatile read is
+        // an optimizer barrier for the fixed 64-iteration alphabet scan and
+        // does not access caller memory.
+        decoded = unsafe { core::ptr::read_volatile(&raw const decoded) };
+        valid =
+            core::hint::black_box(core::hint::black_box(valid) | core::hint::black_box(matches));
+        // SAFETY: `valid` is an initialized local `u8`; the volatile read is an
+        // optimizer barrier for the fixed 64-iteration alphabet scan and does
+        // not access caller memory.
+        valid = unsafe { core::ptr::read_volatile(&raw const valid) };
         candidate += 1;
     }
 

tests/rfc4648.rs

@@ -11,7 +11,7 @@ use base64_ng::{
 use base64_ng::stream::{Decoder, DecoderReader, Encoder, EncoderReader};
 
 #[cfg(feature = "alloc")]
-use base64_ng::SecretBuffer;
+use base64_ng::{ExposedSecretString, ExposedSecretVec, SecretBuffer};
 
 #[cfg(feature = "stream")]
 use std::io::{Cursor, Read, Write};
@@ -2840,6 +2840,46 @@ fn secret_buffer_from_vec_preserves_visible_bytes_with_spare_capacity() {
     assert_eq!(secret.expose_secret(), b"\xff");
 }
 
+#[cfg(feature = "alloc")]
+fn assert_vec_spare_capacity_zeroed(mut bytes: Vec<u8>, len: usize) {
+    let capacity = bytes.capacity();
+    assert!(capacity > len);
+
+    // SAFETY: the tested wrappers explicitly initialize spare capacity with
+    // zero bytes before this helper receives the vector. Extending the length
+    // lets the regression test inspect the spare region that would otherwise
+    // remain outside safe slice access.
+    unsafe {
+        bytes.set_len(capacity);
+    }
+    assert!(bytes[len..].iter().all(|byte| *byte == 0));
+    bytes.fill(0);
+}
+
+#[cfg(feature = "alloc")]
+#[test]
+fn secret_wrappers_clear_spare_capacity_at_construction() {
+    let mut bytes = vec![0xab; 64];
+    bytes.truncate(32);
+    let exposed = ExposedSecretVec::from_vec(bytes);
+    let bytes = exposed.into_exposed_unprotected_vec_caller_must_zeroize();
+    assert_vec_spare_capacity_zeroed(bytes, 32);
+
+    let mut bytes = vec![0xcd; 64];
+    bytes.truncate(32);
+    let secret = SecretBuffer::from_vec(bytes);
+    let exposed = secret.into_exposed_vec();
+    let bytes = exposed.into_exposed_unprotected_vec_caller_must_zeroize();
+    assert_vec_spare_capacity_zeroed(bytes, 32);
+
+    let mut text = String::with_capacity(64);
+    text.push_str("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX");
+    text.truncate(32);
+    let exposed = ExposedSecretString::from_string(text);
+    let text = exposed.into_exposed_unprotected_string_caller_must_zeroize();
+    assert_vec_spare_capacity_zeroed(text.into_bytes(), 32);
+}
+
 #[cfg(feature = "alloc")]
 #[test]
 fn secret_buffer_try_from_byte_array_uses_strict_standard_base64() {