aead: adds a dev::MockBuffer to smoke test AEADs implementation

[baloo]

As discussed in RustCrypto/AEADs#664, this introduces a BicephalBuffer whose purpose is to provide two backing buffer for an InOutBuf to flush out misuse of get_in.

[baloo]

Couldn't come up with anything better for the name. If you have ideas...

[tarcieri]

SplitBuffer?

[baloo]

SplitBrainBuffer?
Split buffer carries the idea the buffer would be split in two, which I was trying to avoid.

DuplicatedBuffer? TwinBuffer?

[newpavlov]

I don't think we need this new type. Instead, in my opinion it would be better to tweak the new_test! macro (or introduce a separate macro) to use the inout API. And I am not sure why you can't simply use the InOutBuf::new method with vec![0u8; input.len()] for an output buffer in tests.

Also, cloning data to both input and output buffers looks wrong, since it leaves the window open for certain bug kinds.

[baloo]

And I am not sure why you can't simply use the InOutBuf::new method with vec![0u8; input.len()] for an output buffer.

Also, cloning data to both input and output buffers looks wrong, since it leaves the window open for certain bug kinds.

That doesn't really work though. Consumers expects the get_out to be carry the message/ciphertext as they modify it inplace.

I don't think we need this new type. Instead, in my opinion it would be better to tweak the new_test! macro (or introduce a separate macro) to use the inout API.

Yeah, I agree that new_test should test that, but I would still need such a struct at the end of the day.

[tarcieri]

The goal with proper inout support should be to support both in-place operation as well as separate input/output buffers.

It shouldn't matter what the output buffer is initialized to be, it should be completely overwritten.

[newpavlov]

Consumers expects the get_out to be carry the message/ciphertext as they modify it inplace.

The idea behind the inout API is to prevent unnecessary copies, i.e. the implementations should read input data, process it, and write result to the output buffer.

[baloo]

Consumers expects the get_out to be carry the message/ciphertext as they modify it inplace.

The idea behind the inout API is to prevent unnecessary copies.

I get that, but this one is not intended for production, this is to flush out get_in misuse.

[newpavlov]

I get that, but this one is not intended for production, this is to flush out get_in misuse.

And you are doing it incorrectly. With this type you would miss a bug with incorrect use of get_out where get_in should've been used. As @tarcieri wrote, correct implementations should should completely overwrite the output buffer without reading its prior content (unless input and output buffers are the same).

[baloo]

I guess aes-gcm implementation is incorrect then because it really doesn't like me providing a zeroed output buffer.

[newpavlov]

Well, it's not "incorrect". I think it just was written with the in-place mode of operation in mind (since it's the only mode we previously supported), so we would need a bit deeper changes for a proper inout support (I did not look deeply at your PRs, so it's just a conjecture).

[baloo]

Well, zeroring the output buffer just shows a bug I introduced ^^

[baloo]

Renamed it to MockBuffer
Zeroed the output buffer, and plugged it in new_test

[newpavlov]

Closing in favor of #1802.