deoxys: tweak DeoxysMode::encrypt/decrypt_inout methods#680
deoxys: tweak DeoxysMode::encrypt/decrypt_inout methods#680
DeoxysMode::encrypt/decrypt_inout methods#680Conversation
deoxys/src/modes.rs
Outdated
| 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() { |
There was a problem hiding this comment.
Reversing those branches would make the diff more readable.
There was a problem hiding this comment.
But it would make the resulting code worse. :)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I found a better way. We do not need the separate branch for the "complete" case in the first place.
The change reduces code drift to the right and simplifies the branching structure.
It was previously suggested here.