Skip to content

deoxys: tweak DeoxysMode::encrypt/decrypt_inout methods #680

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.

Sign up for GitHub

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

Merged
newpavlov merged 2 commits into from
Mar 24, 2025
Merged

deoxys: tweak DeoxysMode::encrypt/decrypt_inout methods #680

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0d9170
deoxys: tweak `DeoxysMode::encrypt/decrypt_inout` methods
newpavlov Mar 24, 2025
faa4172
Remove separate branch for complete blocks
newpavlov Mar 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 87 additions & 93 deletions deoxys/src/modes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,75 +111,71 @@ where
tweak[8] = nonce[7] << 4;

// Message authentication and encryption
if !buffer.is_empty() {
tweak[0] = (tweak[0] & 0xf) | TWEAK_M;

let (data_blocks, tail) = buffer.into_chunks();
let data_blocks_len = data_blocks.len();
tweak[0] = (tweak[0] & 0xf) | TWEAK_M;

for (index, data) in data_blocks.into_iter().enumerate() {
// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;
let (data_blocks, mut tail) = buffer.into_chunks();
let data_blocks_len = data_blocks.len();

for (c, d) in checksum.iter_mut().zip(data.get_in().iter()) {
*c ^= d;
}
for (index, data) in data_blocks.into_iter().enumerate() {
// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

B::encrypt_inout(data, &tweak, subkeys);
for (c, d) in checksum.iter_mut().zip(data.get_in().iter()) {
*c ^= d;
}

let mut data = tail;
let index = data_blocks_len;
if !data.is_empty() {
// Last block, incomplete
B::encrypt_inout(data, &tweak, subkeys);
}

// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;
if tail.is_empty() {
Copy link
Member

@baloo baloo Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reversing those branches would make the diff more readable.

Copy link
Member Author

@newpavlov newpavlov Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it would make the resulting code worse. :)

Copy link
Member

@baloo baloo Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

        if !tail.is_empty() {
            // Tag computing with incomplete last block
            let index = data_blocks_len;

            // Copy block number
            let tmp = tweak[8] & 0xf0;
            tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
            tweak[8] = (tweak[8] & 0xf) | tmp;

            // Last block checksum
            tweak[0] = (tweak[0] & 0xf) | TWEAK_M_LAST;

            let mut block = Block::default();
            block[..tail.len()].copy_from_slice(tail.get_in());

            block[tail.len()] = 0x80;

            for (c, d) in checksum.iter_mut().zip(block.iter()) {
                *c ^= d;
            }

            block.fill(0);

            // Last block encryption
            B::encrypt_inout((&mut block).into(), &tweak, subkeys);

            tail.xor_in2out((block[..tail.len()]).into());

            // Tag computing.
            tweak[0] = (tweak[0] & 0xf) | TWEAK_CHKSUM;

            let tmp = tweak[8] & 0xf0;
            tweak[8..].copy_from_slice(&((index + 1) as u64).to_be_bytes());
            tweak[8] = (tweak[8] & 0xf) | tmp;

            B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);

            for (t, c) in tag.iter_mut().zip(checksum.iter()) {
                *t ^= c;
            }
        } else {
            // Tag computing without last block
            tweak[0] = (tweak[0] & 0xf) | TWEAK_TAG;

            let tmp = tweak[8] & 0xf0;
            tweak[8..].copy_from_slice(&((buffer_len / 16) as u64).to_be_bytes());
            tweak[8] = (tweak[8] & 0xf) | tmp;

            B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);

            for (t, c) in tag.iter_mut().zip(checksum.iter()) {
                *t ^= c;
            }
        }

Worse?

Copy link
Member

@baloo baloo Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff looks like that: https://github.com/RustCrypto/AEADs/compare/master...baloo:baloo/deoxys/modes-tweak?w=1

I find it a lot more readable.

Copy link
Member Author

@newpavlov newpavlov Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you mean the order of blocks in the tail.is_empty() part. I erroneously thought that you talked about the buffer.is_empty() branch.

The additional ! makes the code slightly worse in my opinion, but one can argue that the tail block case is more common in practice, so it should go first.

Copy link
Member

@baloo baloo Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, you removed the buffer.is_empty() which I completely missed you could do when you initially suggested it. And that made my code messy then.

Copy link
Member Author

@newpavlov newpavlov Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a better way. We do not need the separate branch for the "complete" case in the first place.

// Tag computing without last block
tweak[0] = (tweak[0] & 0xf) | TWEAK_TAG;

// Last block checksum
tweak[0] = (tweak[0] & 0xf) | TWEAK_M_LAST;
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&((buffer_len / 16) as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

let mut block = Block::default();
block[0..data.len()].copy_from_slice(data.get_in());
B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);

block[data.len()] = 0x80;
for (t, c) in tag.iter_mut().zip(checksum.iter()) {
*t ^= c;
}
} else {
// Tag computing with incomplete last block
let index = data_blocks_len;

for (c, d) in checksum.iter_mut().zip(block.iter()) {
*c ^= d;
}
// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

block.fill(0);
// Last block checksum
tweak[0] = (tweak[0] & 0xf) | TWEAK_M_LAST;

// Last block encryption
B::encrypt_inout((&mut block).into(), &tweak, subkeys);
let mut block = Block::default();
block[..tail.len()].copy_from_slice(tail.get_in());

data.xor_in2out((block[..data.len()]).into());
block[tail.len()] = 0x80;

// Tag computing.
tweak[0] = (tweak[0] & 0xf) | TWEAK_CHKSUM;
for (c, d) in checksum.iter_mut().zip(block.iter()) {
*c ^= d;
}

let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&((index + 1) as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;
block.fill(0);

B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);
// Last block encryption
B::encrypt_inout((&mut block).into(), &tweak, subkeys);

for (t, c) in tag.iter_mut().zip(checksum.iter()) {
*t ^= c;
}
}
}
tail.xor_in2out((block[..tail.len()]).into());

if buffer_len % 16 == 0 {
// Tag computing without last block
tweak[0] = (tweak[0] & 0xf) | TWEAK_TAG;
// Tag computing.
tweak[0] = (tweak[0] & 0xf) | TWEAK_CHKSUM;

let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&((buffer_len / 16) as u64).to_be_bytes());
tweak[8..].copy_from_slice(&((index + 1) as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);
Expand Down Expand Up @@ -221,71 +217,69 @@ where
tweak[8] = nonce[7] << 4;

// Message authentication and encryption
if !buffer.is_empty() {
tweak[0] = (tweak[0] & 0xf) | TWEAK_M;

let (data_blocks, tail) = buffer.into_chunks();
let data_blocks_len = data_blocks.len();
tweak[0] = (tweak[0] & 0xf) | TWEAK_M;

for (index, mut data) in data_blocks.into_iter().enumerate() {
// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;
let (data_blocks, mut tail) = buffer.into_chunks();
let data_blocks_len = data_blocks.len();

B::decrypt_inout(data.reborrow(), tweak.as_ref(), subkeys);
for (index, mut data) in data_blocks.into_iter().enumerate() {
// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

for (c, d) in checksum.iter_mut().zip(data.get_out().iter()) {
*c ^= d;
}
B::decrypt_inout(data.reborrow(), tweak.as_ref(), subkeys);

for (c, d) in checksum.iter_mut().zip(data.get_out().iter()) {
*c ^= d;
}
}

let mut data = tail;
let index = data_blocks_len;
if !data.is_empty() {
// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;
if tail.is_empty() {
// Tag computing without last block
tweak[0] = (tweak[0] & 0xf) | TWEAK_TAG;

// Last block checksum
tweak[0] = (tweak[0] & 0xf) | TWEAK_M_LAST;
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&((buffer_len / 16) as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

let mut block = Block::default();
B::encrypt_inout((&mut block).into(), tweak.as_ref(), subkeys);
B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);

data.xor_in2out((block[..data.len()]).into());
for (t, c) in computed_tag.iter_mut().zip(checksum.iter()) {
*t ^= c;
}
} else {
// Tag computing with incomplete last block
let index = data_blocks_len;

block.fill(0);
// Copy block number
let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&(index as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

block[0..data.len()].copy_from_slice(data.get_out());
block[data.len()] = 0x80;
// Last block checksum
tweak[0] = (tweak[0] & 0xf) | TWEAK_M_LAST;

for (c, d) in checksum.iter_mut().zip(block.iter()) {
*c ^= d;
}
let mut block = Block::default();
B::encrypt_inout((&mut block).into(), tweak.as_ref(), subkeys);

// Tag computing.
tweak[0] = (tweak[0] & 0xf) | TWEAK_CHKSUM;
tail.xor_in2out((block[..tail.len()]).into());

let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&((index + 1) as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;
block.fill(0);

B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);
block[..tail.len()].copy_from_slice(tail.get_out());
block[tail.len()] = 0x80;

for (t, c) in computed_tag.iter_mut().zip(checksum.iter()) {
*t ^= c;
}
for (c, d) in checksum.iter_mut().zip(block.iter()) {
*c ^= d;
}
}

if buffer_len % 16 == 0 {
// Tag computing without last block
tweak[0] = (tweak[0] & 0xf) | TWEAK_TAG;
// Tag computing.
tweak[0] = (tweak[0] & 0xf) | TWEAK_CHKSUM;

let tmp = tweak[8] & 0xf0;
tweak[8..].copy_from_slice(&((buffer_len / 16) as u64).to_be_bytes());
tweak[8..].copy_from_slice(&((index + 1) as u64).to_be_bytes());
tweak[8] = (tweak[8] & 0xf) | tmp;

B::encrypt_inout((&mut checksum).into(), tweak.as_ref(), subkeys);
Expand Down