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

CHACHAPOLY_MAX is too large by 1 block #2

Closed
defuse opened this issue Jun 3, 2019 · 4 comments
Closed

CHACHAPOLY_MAX is too large by 1 block #2

defuse opened this issue Jun 3, 2019 · 4 comments

Comments

@defuse
Copy link
Contributor

defuse commented Jun 3, 2019

In the AEAD construction, block counter = 0 is used to generate the poly1305 key, and encryption of data blocks starts with block counter = 1, so at most 2^32 - 1 blocks can be encrypted. However, the value of CHACHAPOLY_MAX is 2^32 blocks. I believe this means that the code will let you encrypt 2^32*64 bytes, and the last block will be XORed with the poly1305 key, because of n overflowing (at least in release builds).

In addition to fixing the constant I'd recommend adding a check (that works even in release builds) for n overflowing in the xor function.

@KizzyCode
Copy link
Owner

Yes, you're right; it should be (2^32 - 1) * 64 🙈

When it comes to input validation in the ChaCha20Ietf::Xor, n is already constrained because it's an u32, and if you use ChaCha only AFAIK all u32-values are valid inputs – or am I mistaking sth.?

@defuse
Copy link
Contributor Author

defuse commented Jun 3, 2019

I mean that if as a result of the n += 1 line, n overflows and wraps around to 0, it should panic or something instead of re-using the keystream. (It should already panic in debug builds, but not release builds). This way even if the *_MAX constants are wrong there's no keystream-reuse vulnerability.

@KizzyCode
Copy link
Owner

Ah, ok 😊
In the Cargo.toml I've set

[profile.release]
overflow-checks = true

so it should panic on any overflow in the crate.

@defuse
Copy link
Contributor Author

defuse commented Jun 3, 2019

Oh, I missed that. Excellent!

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

2 participants