Skip to content

Commit

Permalink
aes: Clarify empty buffer handling in AES-CTR.
Browse files Browse the repository at this point in the history
At one point we required `f` to be able to handle empty inputs,
but a later refactoring changed that, and added the necessary
check before calling `f`, but we forgot to update the comment.

To further clarify things, use `NonZeroU32` when incrementing
the counter, since a zero increment would result in a reused
counter.
  • Loading branch information
briansmith committed Dec 26, 2024
1 parent 1d17896 commit 072ee75
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
10 changes: 7 additions & 3 deletions src/aead/aes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ use crate::{
constant_time,
cpu::{self, GetFeature as _},
error,
polyfill::unwrap_const,
};
use cfg_if::cfg_if;
use core::num::NonZeroU32;

pub(super) use ffi::Counter;

Expand Down Expand Up @@ -123,15 +125,17 @@ impl Counter {
}

pub fn increment(&mut self) -> Iv {
const ONE: NonZeroU32 = unwrap_const(NonZeroU32::new(1));

let iv = Iv(self.0);
self.increment_by_less_safe(1);
self.increment_by_less_safe(ONE);
iv
}

fn increment_by_less_safe(&mut self, increment_by: u32) {
fn increment_by_less_safe(&mut self, increment_by: NonZeroU32) {
let [.., c0, c1, c2, c3] = &mut self.0;
let old_value: u32 = u32::from_be_bytes([*c0, *c1, *c2, *c3]);
let new_value = old_value + increment_by;
let new_value = old_value + increment_by.get();
[*c0, *c1, *c2, *c3] = u32::to_be_bytes(new_value);
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/aead/aes/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use super::{Block, InOut, KeyBytes, BLOCK_LEN};
use crate::{bits::BitLength, c, error};
use core::num::NonZeroUsize;
use core::num::{NonZeroU32, NonZeroUsize};

/// nonce || big-endian counter.
#[repr(transparent)]
Expand Down Expand Up @@ -182,15 +182,14 @@ impl AES_KEY {

let input: *const [u8; BLOCK_LEN] = input.cast();
let output: *mut [u8; BLOCK_LEN] = output.cast();
let blocks_u32: u32 = blocks.get().try_into().unwrap();
let blocks_u32: NonZeroU32 = blocks.try_into().unwrap();

// SAFETY:
// * `input` points to `blocks` blocks.
// * `output` points to space for `blocks` blocks to be written.
// * input == output.add(n), where n == src.start, and the caller is
// responsible for ensuing this sufficient for `f` to work correctly.
// * The caller is responsible for ensuring `f` can handle any value of
// `blocks` including zero.
// * `blocks` is non-zero so `f` doesn't have to work for empty slices.
// * The caller is responsible for ensuring `key` was initialized by the
// `set_encrypt_key!` invocation required by `f`.
unsafe {
Expand Down

0 comments on commit 072ee75

Please sign in to comment.