investigate: git commit tag verification is insufficient for admin workstation

[heartsucker]

Google published a paper on how SHA1 is super broken, and by looking at how tag verification is done in the git source, I think it is possible that a MITM + sneaky hash collision could be used to load a malicious repo that still passes verification on the admin workstation.

To investigate: the actual contents of the .git/objects/xx/xxxxxxxxx that contains the tag and see if the only ref to the commit is the SHA1.

Possible mitigation: SHA512 the contents of the git repo at the given commit. GPG sign. Have admin run verification against this signed content. Problematic because of custom files (header images, config files) present in the repo. Exclusions would have to be made.

Possible mitigation 2: Build a Debian package for a given git commit, have the admin verify then install that locally (they need root anyway).

[psivesely]

I'm not really worried about this one. There are a number of things standing in the way of an attacker doing this and I presume that it will be fixed upstream by the git maintainers before there is a practical worry. Among the difficulties:

1. Only a collision has been found. This would require a second-preimage attack.
2. It took 9 quintillion computations or 110 years of single-GPU computations to find (again, this is for a collision attack, finding a second-preimage would be much harder).
3. In the attack they worked with a PDF prefix, where they could basically throw in arbitrary data in particular spots w/o effecting the appearance of the PDF. That's possible with certain binary formats, but with git commits, sure they could do that in comments, but unlike a PDF this data would be user-visible and needs to fit together to create a running, normal appearing program (with a backdoor).

I haven't had time to read the paper yet, so apologies for any inaccuracies. I'll try to correct them if I made any after I get the chance.

[heartsucker]

You're right, this is a second pre image attack, but if a commit's hash includes the message, then you have arbitrary text to work with (not sure, don't quote me). I guess this means while much harder, it would be very bad to see a paper come out soon that makes this an immediate threat. Plus, I've been a bit iffy with regards to git for deploying to admins anyway, so this was a bit of an excuse to work on those changes too. :) Am 23.02.2017 20:55 schrieb "Noah Vesely" <notifications@github.com>:

…

I'm not really worried about this one. There are a number of things standing in the way of an attacker doing this and I presume that it will be fixed upstream by the git maintainers before there is a practical worry. Among the difficulties: 1. Only a collision has been found. This would require a second-preimage attack. 2. It took 9 quintillion computations or 110 years of single-GPU computations to find (again, this is for a collision attack, finding a second-preimage would be much harder). 3. In the attack they worked with a PDF prefix, where they could basically throw in arbitrary data in particular spots w/o effecting the appearance of the PDF. That's possible with certain binary formats, but with git commits, sure they could do that in comments, but unlike a PDF this data would be user-visible and needs to fit together to create a running, normal appearing program (with a backdoor). I haven't had time to read the paper yet, so apologies for any inaccuracies. I'll try to correct them if I made any after I get the chance. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1591 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD0DAIhZJ65uzGV7ynLgGrNHkqctvxOIks5rfeQbgaJpZM4MKKdZ> .

[conorsch]

Plus, I've been a bit iffy with regards to git for deploying to admins anyway, so this was a bit of an excuse to work on those changes too. :)

I hear you. The problem at present is that we're fighting against Tails's ephemeral-by-default setup, trying to preserve system state for Admins (e.g. torrc modifications). We're at the very early stages of evaluating alternative setups for the workstation endpoints, but we're likely going to focus on the Journalist Workstation first, since that gets more frequent interactive use than the Admin Workstation.

[psivesely]

if a commit's hash includes the message, then you have arbitrary text to work with

It does, but then the log looks terrible. Spacing is probably the most discrete thing to work with. There's certainly quintillions of combinations of spaces you can add to a large program's code without effecting the appearance or functionality.

I think it comes down to the fact that until someone improves on this attack again (which will inevitably happen, but who knows when), and makes second-preimage attacks more feasible (and we don't even know it's feasible to begin with right now) there are much cheaper and more practical ways to subvert the integrity of the code we ship and to compromise SD systems in general.

I do believe (hope) git will move to a safe hash soon.

[heartsucker]

You're right, this is a second pre image attack,
Should have said isn't. This is just a collision attack. Don't email while falling asleep.

Anyway, I guess what I wanted was to stay well ahead of the curve because if Google posts a a second preimage attack in 2 years... sadness. This is mostly here as a note so to track the existence of this as an issue and not as an immediate concern. Plus, if someone is working on a related issue, we might get this fixed for free.

[garrettr]

Some alternative tools have already been developed to address this issue, e.g. https://github.com/indutny/git-secure-tag which is already being used by some major projects including Node.js: nodejs/node#7579.

[psivesely]

Just want to note SecureDrop is not the only code whose integrity we rely on, but the entire stack on every machine. Ubuntu and Debian developers verify git tags on much of that software before building, packaging, and signing. So fixing it for just SecureDrop is not necessarily sufficient.

That said, I like the idea of using git-secure-tag, although that does require an extra apt-get update && apt-get install npm && npm install -g git-secure-tag step on behalf of the admin.

[eloquence]

At this rate, it seems more plausible to transition the SecureDrop repo to SHA-256 hashes (experimental support added with git 2.29.0) once that is no longer marked as experimental and supported by GitHub & a stable Tails release. Or, even better, resolve #3502 and move the verification story away from git. I think we can keep this issue open for tracking purposes until either of those things are done.