Add integrity protection using Encrypt-Then-MAC to default encryption module#21557
Merged
LukasReschke merged 20 commits intomasterfrom Feb 9, 2016
Merged
Add integrity protection using Encrypt-Then-MAC to default encryption module#21557LukasReschke merged 20 commits intomasterfrom
LukasReschke merged 20 commits intomasterfrom
Conversation
|
By analyzing the blame information on this pull request, we identified @schiesbn, @nickvergessen and @DeepDiver1975 to be potential reviewers |
Member
Author
|
@owncloud/qa Please do some testing with encryption enabled. Thanks. |
Contributor
|
👍 |
Contributor
|
So crypto stays a complex thing. And the more you learn about it the further you know to stay away from it ;). Having said that I can follow the commits here. And the changes make sense to me. |
Member
Author
|
Needs some more love to prevent shuffling the entries. Setting back to 2. |
LukasReschke
force-pushed
the
use-hmac-over-encryption
branch
3 times, most recently
from
86b5842 to
01a9bab
Compare
LukasReschke
force-pushed
the
use-hmac-over-encryption
branch
2 times, most recently
from
f37fd5e to
c3f18f8
Compare
Contributor
Contributor
|
Contributor
|
Code looks good. |
LukasReschke
force-pushed
the
use-hmac-over-encryption
branch
from
c3f18f8 to
0a177de
Compare
LukasReschke
force-pushed
the
use-hmac-over-encryption
branch
from
0a177de to
f060288
Compare
LukasReschke
force-pushed
the
use-hmac-over-encryption
branch
from
f060288 to
d00628f
Compare
CTR is recommended over CFB mode.
The previous IV was actually 12 byte extended to 16 byte using base64. As the encrypted file should be fine with containing binary data as well we can simply remove the encoding like that here.
…e was encrypted before
This way it is not possible anymore for an external storage admin to put up old versions of the file.
Prevents switching single blocks within the encrypted file.
…e version and not the original file
…herwise it can happen that we decrease it twice and end up with the wrong value
Saves a call to fetch the file id which didn't even work for a reason. This fix properly sets the version in the database.
LukasReschke
force-pushed
the
use-hmac-over-encryption
branch
from
da41a85 to
ca35029
Compare
LukasReschke
added a commit
that referenced
this pull request
Feb 9, 2016
Add integrity protection using Encrypt-Then-MAC to default encryption module
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds an integrity protection using Encrypt-Then-MAC to the default encryption module. Authenticated encryption prevents that a storage administrator can tamper with the files such as performing bitflipping attacks on them.
To achieve this the following changes have been done by @schiesbn and me:
This approach gives us the possibility to have complete backwards compatibility with the existing encrypted files (as in: you should be able to read all files as before as well) as well as an easy way to differentiate whether a file needs to have a mandatory HMAC.
To test this also try to modify the encrypted ciphertext in the encrypted document on new or modified files and see whether you can still open the document. With this change applied you should not, it will simply fail to decrypt the file.
There will be no automatic migration as we all know the pain of such. Thus this will only affect new or modified files.
Partially addresses https://github.com/owncloud/security-tracker/issues/191. Some documentation changes are still pending.