Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clippy: array out-of-bound in blockmode/gcm-siv #16

Open
DuckSoft opened this issue Apr 14, 2021 · 3 comments
Open

clippy: array out-of-bound in blockmode/gcm-siv #16

DuckSoft opened this issue Apr 14, 2021 · 3 comments
Assignees

Comments

@DuckSoft
Copy link
Contributor

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:114:28
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
114 | |                     ek[16..24].copy_from_slice(&tmp[0..8]);
    | |                            ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
285 | 
286 |   impl_block_cipher_with_gcm_siv_mode!(Aes128GcmSiv, Aes128);
    |   ----------------------------------------------------------- in this macro invocation
    |
    = note: requested on the command line with `-D clippy::out-of-bounds-indexing`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:119:24
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
119 | |                     ek[24..32].copy_from_slice(&tmp[0..8]);
    | |                        ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
285 | 
286 |   impl_block_cipher_with_gcm_siv_mode!(Aes128GcmSiv, Aes128);
    |   ----------------------------------------------------------- in this macro invocation
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:114:28
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
114 | |                     ek[16..24].copy_from_slice(&tmp[0..8]);
    | |                            ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
289 |   impl_block_cipher_with_gcm_siv_mode!(Sm4GcmSiv, Sm4);
    |   ----------------------------------------------------- in this macro invocation
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:119:24
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
119 | |                     ek[24..32].copy_from_slice(&tmp[0..8]);
    | |                        ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
289 |   impl_block_cipher_with_gcm_siv_mode!(Sm4GcmSiv, Sm4);
    |   ----------------------------------------------------- in this macro invocation
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:114:28
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
114 | |                     ek[16..24].copy_from_slice(&tmp[0..8]);
    | |                            ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
290 |   impl_block_cipher_with_gcm_siv_mode!(Camellia128GcmSiv, Camellia128);
    |   --------------------------------------------------------------------- in this macro invocation
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:119:24
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
119 | |                     ek[24..32].copy_from_slice(&tmp[0..8]);
    | |                        ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
290 |   impl_block_cipher_with_gcm_siv_mode!(Camellia128GcmSiv, Camellia128);
    |   --------------------------------------------------------------------- in this macro invocation
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:114:28
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
114 | |                     ek[16..24].copy_from_slice(&tmp[0..8]);
    | |                            ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
292 |   impl_block_cipher_with_gcm_siv_mode!(Aria128GcmSiv, Aria128);
    |   ------------------------------------------------------------- in this macro invocation
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: range is out of bounds
   --> src/blockmode/gcm_siv.rs:119:24
    |
21  | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22  | |     ($name:tt, $cipher:tt) => {
23  | |
24  | |         #[derive(Clone)]
...   |
119 | |                     ek[24..32].copy_from_slice(&tmp[0..8]);
    | |                        ^^
...   |
283 | |     }
284 | | }
    | |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
292 |   impl_block_cipher_with_gcm_siv_mode!(Aria128GcmSiv, Aria128);
    |   ------------------------------------------------------------- in this macro invocation
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing

error: aborting due to 8 previous errors

@DuckSoft
Copy link
Contributor Author

DuckSoft commented Apr 14, 2021

cargo expand shows that the code impl_block_cipher_with_gcm_siv_mode!(Aes128GcmSiv, Aes128); generates:

#[derive(Clone)]
pub struct Aes128GcmSiv {
    cipher: Aes128,
}
impl Zeroize for Aes128GcmSiv {
    fn zeroize(&mut self) {
        self.cipher.zeroize();
    }
}
impl Drop for Aes128GcmSiv {
    fn drop(&mut self) {
        self.zeroize();
    }
}
impl Aes128GcmSiv {
    pub const KEY_LEN: usize = Aes128::KEY_LEN;
    pub const BLOCK_LEN: usize = Aes128::BLOCK_LEN;
    pub const NONCE_LEN: usize = 12;
    pub const TAG_LEN: usize = 16;

    #[cfg(target_pointer_width = "64")]
    pub const A_MAX: usize = 68719476736;
    #[cfg(target_pointer_width = "32")]
    pub const A_MAX: usize = usize::MAX;

    #[cfg(target_pointer_width = "64")]
    pub const P_MAX: usize = 68719476736;
    #[cfg(target_pointer_width = "32")]
    pub const P_MAX: usize = usize::MAX - Self::TAG_LEN;

    #[cfg(target_pointer_width = "64")]
    pub const C_MAX: usize = 68719476736 + Self::TAG_LEN;
    #[cfg(target_pointer_width = "32")]
    pub const C_MAX: usize = usize::MAX;

    pub const N_MIN: usize = Self::NONCE_LEN;
    pub const N_MAX: usize = Self::NONCE_LEN;


    pub fn new(key: &[u8]) -> Self {
        assert!(Self::KEY_LEN == 16 || Self::KEY_LEN == 32);
        assert_eq!(key.len(), Self::KEY_LEN);
        assert_eq!(Self::BLOCK_LEN, GCM_SIV_BLOCK_LEN);
        assert_eq!(Self::BLOCK_LEN, Polyval::BLOCK_LEN);

        let cipher = Aes128::new(key);

        Self { cipher }
    }

    #[inline]
    fn derive_keys(&self, nonce: &[u8]) -> (Aes128, Polyval) {
        let mut counter_block = [0u8; Self::BLOCK_LEN];
        counter_block[4..16].copy_from_slice(nonce);


        let mut ak = [0u8; Self::BLOCK_LEN];

        let mut ek = [0u8; Self::KEY_LEN];

        let mut tmp = counter_block.clone();
        tmp[0] = 0;
        self.cipher.encrypt(&mut tmp);
        ak[0..8].copy_from_slice(&tmp[0..8]);

        tmp = counter_block.clone();
        tmp[0] = 1;
        self.cipher.encrypt(&mut tmp);
        ak[8..16].copy_from_slice(&tmp[0..8]);

        tmp = counter_block.clone();
        tmp[0] = 2;
        self.cipher.encrypt(&mut tmp);
        ek[0..8].copy_from_slice(&tmp[0..8]);

        tmp = counter_block.clone();
        tmp[0] = 3;
        self.cipher.encrypt(&mut tmp);
        ek[8..16].copy_from_slice(&tmp[0..8]);


        if Self::KEY_LEN == 32 {
            tmp = counter_block.clone();
            tmp[0] = 4;
            self.cipher.encrypt(&mut tmp);
            ek[16..24].copy_from_slice(&tmp[0..8]);

            tmp = counter_block.clone();
            tmp[0] = 5;
            self.cipher.encrypt(&mut tmp);
            ek[24..32].copy_from_slice(&tmp[0..8]);
        }

        let cipher = Aes128::new(&ek);

        let polyval = Polyval::new(&ak);

        (cipher, polyval)
    }

    #[inline]
    fn ctr32(counter_block: &mut [u8; Self::BLOCK_LEN]) {
        let counter = u32::from_le_bytes([
            counter_block[0], counter_block[1], counter_block[2], counter_block[3]
        ]);

        counter_block[0..4].copy_from_slice(&counter.wrapping_add(1).to_le_bytes());
    }

    pub fn encrypt_slice(&self, nonce: &[u8], aad: &[u8], aead_pkt: &mut [u8]) {
        debug_assert!(aead_pkt.len() >= Self::TAG_LEN);

        let plen = aead_pkt.len() - Self::TAG_LEN;
        let (plaintext_in_ciphertext_out, tag_out) = aead_pkt.split_at_mut(plen);

        self.encrypt_slice_detached(nonce, aad, plaintext_in_ciphertext_out, tag_out)
    }

    pub fn decrypt_slice(&self, nonce: &[u8], aad: &[u8], aead_pkt: &mut [u8]) -> bool {
        debug_assert!(aead_pkt.len() >= Self::TAG_LEN);

        let clen = aead_pkt.len() - Self::TAG_LEN;
        let (ciphertext_in_plaintext_out, tag_in) = aead_pkt.split_at_mut(clen);

        self.decrypt_slice_detached(nonce, aad, ciphertext_in_plaintext_out, &tag_in)
    }

    pub fn encrypt_slice_detached(&self, nonce: &[u8], aad: &[u8], plaintext_in_ciphertext_out: &mut [u8], tag_out: &mut [u8]) {
        assert_eq!(nonce.len(), Self::NONCE_LEN);

        let alen = aad.len();
        let plen = plaintext_in_ciphertext_out.len();
        let tlen = tag_out.len();

        debug_assert!(alen <= Self::A_MAX);
        debug_assert!(plen <= Self::P_MAX);
        debug_assert!(tlen == Self::TAG_LEN);

        let (cipher, mut polyval) = self.derive_keys(nonce);

        let mut bit_len_block = [0u8; Self::BLOCK_LEN];
        bit_len_block[0..8].copy_from_slice(&(alen as u64 * 8).to_le_bytes());
        bit_len_block[8..16].copy_from_slice(&(plen as u64 * 8).to_le_bytes());

        polyval.update(aad);
        polyval.update(&plaintext_in_ciphertext_out);
        polyval.update(&bit_len_block);

        let mut tag = polyval.finalize();

        for i in 0..Self::NONCE_LEN {
            tag[i] ^= nonce[i];
        }
        tag[15] &= 0x7f;


        cipher.encrypt(&mut tag);


        let mut counter_block = tag.clone();
        counter_block[15] |= 0x80;


        let n = plen / Self::BLOCK_LEN;
        for i in 0..n {
            let mut keystream_block = counter_block.clone();
            cipher.encrypt(&mut keystream_block);

            Self::ctr32(&mut counter_block);

            let block = &mut plaintext_in_ciphertext_out[i * Self::BLOCK_LEN..i * Self::BLOCK_LEN + Self::BLOCK_LEN];

            xor_si128_inplace(block, &keystream_block);
        }

        if plen % Self::BLOCK_LEN != 0 {
            let mut keystream_block = counter_block.clone();
            cipher.encrypt(&mut keystream_block);

            Self::ctr32(&mut counter_block);

            let rem = &mut plaintext_in_ciphertext_out[n * Self::BLOCK_LEN..];
            for i in 0..rem.len() {
                rem[i] ^= keystream_block[i];
            }
        }

        tag_out.copy_from_slice(&tag[..Self::TAG_LEN]);
    }

    pub fn decrypt_slice_detached(&self, nonce: &[u8], aad: &[u8], ciphertext_in_plaintext_out: &mut [u8], tag_in: &[u8]) -> bool {
        assert_eq!(nonce.len(), Self::NONCE_LEN);

        let alen = aad.len();
        let clen = ciphertext_in_plaintext_out.len();
        let tlen = tag_in.len();

        debug_assert!(alen <= Self::A_MAX);
        debug_assert!(clen <= Self::P_MAX);
        debug_assert!(tlen == Self::TAG_LEN);

        let (cipher, mut polyval) = self.derive_keys(nonce);

        let mut counter_block = [0u8; Self::BLOCK_LEN];
        counter_block.copy_from_slice(&tag_in);
        counter_block[15] |= 0x80;


        let n = clen / Self::BLOCK_LEN;
        for i in 0..n {
            let mut keystream_block = counter_block.clone();
            cipher.encrypt(&mut keystream_block);

            Self::ctr32(&mut counter_block);

            let block = &mut ciphertext_in_plaintext_out[i * Self::BLOCK_LEN..i * Self::BLOCK_LEN + Self::BLOCK_LEN];
            xor_si128_inplace(block, &keystream_block);
        }

        if clen % Self::BLOCK_LEN != 0 {
            let mut keystream_block = counter_block.clone();
            cipher.encrypt(&mut keystream_block);

            Self::ctr32(&mut counter_block);

            let rem = &mut ciphertext_in_plaintext_out[n * Self::BLOCK_LEN..];
            for i in 0..rem.len() {
                rem[i] ^= keystream_block[i];
            }
        }


        let mut bit_len_block = [0u8; Self::BLOCK_LEN];
        bit_len_block[0..8].copy_from_slice(&(alen as u64 * 8).to_le_bytes());
        bit_len_block[8..16].copy_from_slice(&(clen as u64 * 8).to_le_bytes());

        polyval.update(aad);
        polyval.update(&ciphertext_in_plaintext_out);
        polyval.update(&bit_len_block);


        let mut tag = polyval.finalize();

        for i in 0..Self::NONCE_LEN {
            tag[i] ^= nonce[i];
        }
        tag[15] &= 0x7f;

        cipher.encrypt(&mut tag);


        constant_time_eq(tag_in, &tag[..Self::TAG_LEN])
    }
}

where there is:

        if Self::KEY_LEN == 32 {
            tmp = counter_block.clone();
            tmp[0] = 4;
            self.cipher.encrypt(&mut tmp);
            ek[16..24].copy_from_slice(&tmp[0..8]);

            tmp = counter_block.clone();
            tmp[0] = 5;
            self.cipher.encrypt(&mut tmp);
            ek[24..32].copy_from_slice(&tmp[0..8]);
        }

since Self::KEY_LEN == 16, that if block shall not exist (at all) in the generated code.

I suggest modify the macro_rules to fix the issue.

@DuckSoft
Copy link
Contributor Author

Update: the warning is temporarily suppressed by 993f47a#diff-f7b53ee42d9d571f6f3a8ed268b8bcc71ba0306507c88194c183695721cfd829R80.

Don't forget to fix it.

@zonyitoo
Copy link
Collaborator

It doesn't affect correctness. But we should find a way to make clippy happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants