elasticdog / transcrypt

transparently encrypt files within a git repository
MIT License
1.43k stars 102 forks source link

Partial commits of encrypted files use the wrong salt #118

Closed Aramgutang closed 3 years ago

Aramgutang commented 3 years ago

While git add -p doesn't work with transcrypt, and some GUIs (e.g. git-cola) will not let you stage hunks/lines of changes for encrypted files, such partial commits do appear to be possible from VS Code (and possibly other interfaces).

However, such commits have the unfortunate side effect of leaving the repo in a state where (after any uncommited changes are removed, or the commit is pulled on a different machine) git status reports the encrypted file as modified, however git diff shows no changes, and git checkout filename does nothing. This is quite frustrating, as it can prevent pulling, checking out other branches, etc, and can't be worked around with git stash.

The reason this happens is that an incorrectly constructed salt is used when partial changes are committed. Thus, when git puts the local decrypted file through clean, the resulting encrypted binary differs to the encrypted binary that has been committed, even though both files decrypt to produce the exact same output. So the encrypted files differ, but the decrypted files do not, resulting in git's confusion as described above.

The cause of this lies in the parameters passed to OpenSSL in clean to generate the salt. The digest uses the SHA256 hash of the file read from $filename, assuming its contents are the same as the input it received (and stored in $tempfile). But since the former file also contains changes that were not staged for the commit, the contents differ. Thus, when the commit is pulled on another machine, the salt will be generated from the hash of the file without those local changes, and will not match.

As far as I can tell, the fix for this is simple: just replace $filename with $tempfile when generating the salt. I'll make a pull request with the change and leave it to you to verify that it doesn't break anything else.

jmurty commented 3 years ago

Thanks for the detailed report and debugging @Aramgutang

If you create a pull request with your proposed fix the automated unit tests should run, giving us a quick check for any unexpected problems.

jmurty commented 3 years ago

This fix is now merged via #119