deoxys: tweak DeoxysMode::encrypt/decrypt_inout methods

[newpavlov]

The change reduces code drift to the right and simplifies the branching structure.

It was previously suggested here.

[baloo]

Reversing those branches would make the diff more readable.

[newpavlov]

But it would make the resulting code worse. :)

[baloo]

        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?

[baloo]

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.

[newpavlov]

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.

[baloo]

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.

[newpavlov]

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