Secret salt and ECB mode

[dchest]

1. Salt doesn't need to be secret.
2. You don't want to use ECB mode. http://en.wikipedia.org/wiki/Block_cipher_modes_of_operation#Electronic_codebook_.28ECB.29

[shadowhand]

You know, something in the back of my mind kept saying "ECB is bad, don't do it" and I ignored it. Thanks for reminding me to not ignore the little voice.

[dchest]

I forgot to mention that salt must be unique for each encryption, so don't store it. OpenSSL will generate it automatically each time you encrypt and it will store it along the ciphertext.

Basically, here's the command:

Encrypt:

openssl enc -base64 -aes-256-cbc -salt -k "$PASS"

Decrypt:

openssl enc -d -base64 -aes-256-cbc -salt -k "$PASS"

[shadowhand]

Breaking change! The default cipher has been changed from "aes-256-ecb" to "aes-256-cbc". Any existing repo using gitcrypt will run "git config gitcrypt.cipher aes-256-ecb" before continuing to use gitcrypt.

An upgrade command will be added soon.

Closed by 43e8864c7087a78861093b1330312d8f7623f76c

[shadowhand]

I should do my research before committing... this issue is partially invalid. A random salt cannot be used or it will trigger constant changes between two separate clones. To be more specific: when doing any manipulation that triggers gitcrypt, the contents of the files have to be re-encrypted. Unless we use a fixed/shared salt, the contents will change every time this happens, making it impossible to ever have two clones that are in sync.

As it stands, the 2nd part of this issue is now fixed by 0b361439da25e8953c0e2cbe88c3dfcc54c791de, and the 1st part of this issue is invalid and will be reverted.

[shadowhand]

Updated gitcrypt init to ask for salt, reverting random salt (impossible to sync two clones with random salt), closed by 477f762048df6a12f2724e6935b6ba22fde6e291

[dchest]

OpenSSL derives encryption key + initialization vectors from password. If you don't use random salt, your initialization vectors will be the same for each message (file/content), which means that, in CBC mode, you're leaking some information about the first block of plaintext. See http://en.wikipedia.org/wiki/Block_cipher_modes_of_operation#Initialization_vector_.28IV.29

And if you use random salt, as you said, it's impossible to sync two clones. I'm not sure how to fix this, will think about it.

[shadowhand]

If you haven't done so, read the last few paragraphs of https://gist.github.com/873637, they talk about the choice of a fixed salt and ECB mode. I'm not really smart enough to understand all the implications here, but I do understand the primary requirement:

1. Git must function as normally as possible.

[dchest]

Yep, the requirement for deterministic encryption rules out the use of random salt.

[shadowhand]

Would using ECB mode allow for partial diffs instead of complete diffs? Although it does lower security, not having an (effectively) entirely new file for every path that is changed would be desirable. I have not been able to set up a test case that confirms or denies this.

Edit: Naturally, I was able to generate a test case as soon as I wrote that I couldn't. It would appear that ECB is, in fact, the right solution here.

[dchest]

It wouldn't be perfect: AES has 16-byte blocks, so if you insert or delete not in multiples of 16 bytes, it would screw up the alignment, so blocks will be different.

[shadowhand]

No, it certainly is not perfect, but changes in 16-byte blocks is almost always smaller than the entire file being changed. I'm going to revert the CBC mode change, but leave in the ability to use a different cipher mode.

[dchest]

Yep, you'll get some space saved for some use cases. Maximum savings are for the case when you're only appending to files.

You have 32-byte text:

ABCDEFGHIJKLMNOPABCDEFGHIJKLMNOP

16-byte blocks are:

❚ABCDEFGHIJKLMNOP❚ABCDEFGHIJKLMNOP❚

Say, it encrypts to

❚zxcvbnmasdfghjkl❚zxcvbnmasdfghjkl❚

but if you insert "X" in the beginning, the alignment changes:

XABCDEFGHIJKLMNOPABCDEFGHIJKLMNOP

you'll get these blocks:

❚XABCDEFGHIJKLMNO❚PABCDEFGHIJKLMNO❚P

which will encrypt to something different.

The same thing happens if you delete the first "P":

❚ABCDEFGHIJKLMNOA❚BCDEFGHIJKLMNOP ❚

Blocks are again different.

What if we append "X"?

❚ABCDEFGHIJKLMNOP❚ABCDEFGHIJKLMNOP❚X

Blocks are the same, so we save 32 bytes :-)

So, it will preserve blocks only work if insertions or deletions are in multiples of 16 bytes. Also, if insertion/deletion happens somewhere later than 16-byte block, it will preserve blocks located before insertion/deletion.

This is still better than the whole file change, yes.

[binarin]

What about using some hash of unencrypted content as salt for CBC?

[shadowhand]

binarin, any use of CBC will trigger the entire file content to be changed, meaning that every commit to a file will create an entirely new delta for the file. You can still choose to use CBC by setting git config gitcrypt.cipher aes-256-cbc.

[binarin]

But what's the problem with deltas? Delta of encrypted file must not tell anything about its content? When changed file is reencrypted with new salt, this will be definitely true. IMO, no security at all is better than false sense of security in ECB case.

[shadowhand]

Its also about speed and using git the way it was designed to work as a delta management tool. I think the documentation is pretty clear and links to a lot of reference material that lets people decide how to use gitcrypt. Using ECB and a fixed salt is simply the most economical way to provide a slightly more secure git repository. The documentation clearly states that you can choose to use CBC if you want.

Edit: And as previously discussed, the requirement of using a static salt means that CBC would not provide much more security anyways.

[dchest]

@binarin: I thought about suggesting the same, but I haven't heard of anyone using the hash of content for IV, so it may be a mistake unless a bunch of a real cryptographers tell us it's okay. See for example, SSL 2.0 issue: "In CBC mode, the IV must, in addition, be unpredictable at encryption time; in particular, the (previously) common practice of re-using the last ciphertext block of a message as the IV for the next message is insecure (for example, this method was used by SSL 2.0)." -- http://www.openssl.org/~bodo/tls-cbc.txt

[LegolasV]

After reading this through, I do not really think it is possible to have reasonable encryption in git at the moment (without breaking the use of git). As in this approach, you'll end up with a repository with a bunch of blocks encrypted with the same salt and the same password, which makes it fairly easy to strip the encryption all together as soon as the repository is large enough (which it will probably be after a few commits) using some statistical profiling. I would guess that "slightly more secure" does not really hold, as the level of encryption provided on a normal repository is neglectable.

[shadowhand]

@LegolasV You are absolutely correct on all points, but I feel that is pretty clear already from the various discussions around this project and the idea in general.

[LegolasV]

@shadowhand Yes, sorry, I should add that I'm interested if this could be done. What that would require is that, for example, the blobs are encrypted when they are written to the disk. Thus, one would do all the operations (e.g. generating the deltas) locally, and then just write the blobs as normally, but now encrypted. Sure, this would still leak information, as the deltas are separately encrypted, but may prove some use. However, this would probably require changing git fundamentally?

EDIT: Seems to be discussed before: http://kerneltrap.org/mailarchive/git/2008/3/13/1156364