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

Update AEAD handling #30

Merged
merged 5 commits into from
Sep 7, 2020
Merged

Update AEAD handling #30

merged 5 commits into from
Sep 7, 2020

Conversation

franziskuskiefer
Copy link
Contributor

In this PR I change when the counter gets increased. Before it was not increased when the decryption failed (invalid tag). Now the counter is increased even if the decryption fails.

@raphaelrobert
Copy link
Contributor

I'm concerned this could introduce another attack:

If an attacker send completely bogus messages to a recipient, the recipient will now blindly increase the counter. When the recipient afterwards receives a valid message, they will not be able to decrypt it because of FS.

@franziskuskiefer
Copy link
Contributor Author

If an attacker send completely bogus messages to a recipient, the recipient will now blindly increase the counter. When the recipient afterwards receives a valid message, they will not be able to decrypt it because of FS.

Sure, with the mechanisms currently available (the counter). There will always be one attack or the other.

@raphaelrobert
Copy link
Contributor

The counter is covered by the MAC, so an attacker couldn't manipulate the counter with bogus messages (where the verification fails) until now. With this PR an attacker can increase the counter by sending any random message.

@franziskuskiefer
Copy link
Contributor Author

Right, but at the moment an attacker can invalidate messages and thus produce a TooDistantFuture error that (if performed in both directions) is unrecoverable.

@franziskuskiefer
Copy link
Contributor Author

Fixes https://github.com/wireapp/security/issues/22
@raphaelrobert how do you want to proceed here?

@franziskuskiefer franziskuskiefer changed the title Increase count on invalid messages Update AEAD handling Aug 25, 2020
@franziskuskiefer
Copy link
Contributor Author

@raphaelrobert can you review? Not counting anymore on failure, but properly handling decryption.

@raphaelrobert
Copy link
Contributor

If we don't do any other changes we should bump the version number.

@raphaelrobert raphaelrobert merged commit 21dab25 into develop Sep 7, 2020
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

Successfully merging this pull request may close these issues.

2 participants