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
Remove claims of 'cryptographic integrity' #971
Comments
FWIW I had to tell a client the other day that they couldn't use Git for a document authentication scheme because of SHA1 collisions (the documents in question were worth enough to make $100k+ attacks economically feasible). |
@peff or anyone with a bigger knowledge on git + security that could offer some comment on this one? |
It's not clear to me what the best path forward is. There is cryptographic integrity. But with lots of caveats related to sha1, and how collisions work, and what attacks might look like, etc. If the proposal is just s/cryptographic integrity/hash-based integrity/, I guess that does not hurt, but nor does it remotely tell the whole picture (both positives and negatives). It's also not clear yet what mitigations are going to look like. Based on what is known now, it seems there is a very good chance that Git can simply reject these intentionally-colliding contents and maintain the security principles in practice. |
Well there certainly isn't "strong" cryptographic integrity and this is clearly wrong:
This shouldn't be too hard to rewrite. Change "strong" to "weak", maybe replace "cryptographic integrity" with "hash-based integrity" as @peff suggests. Replace "impossible" with "difficult" (I don't understand all the possible exploits and it's possible that "difficult" is the wrong word, too.) et cetera. |
I care a lot less about the blog posts. I think their future is to be destroyed with 🔥. Any useful content there made it into the second edition of Pro Git, which we mirror on the site (as an aside, anybody interested in this topic should probably be going over the contents of https://github.com/progit/progit2). The data-assurance blurb in the "about" page is definitely worth re-visiting, though. PRs welcome. |
@petertodd Hmmm, wonder if GPG signing of commits would have affected that. With GPG signatures enforced on every commit, wouldn't that add an effective cryptographically secure er... overlay layer? |
GPG signatures still sign a insecure SHA1 commit hash, so no. This is why the Bitcoin Core project now has code that rehashes the tree with SHA512 when doing merges: https://github.com/bitcoin/bitcoin/blob/dabee00ef1a7a2857c3318e898d3f63f79853048/contrib/devtools/github-merge.py#L248 Not a complete solution, as that SHA512 hash doesn't cover git history itself, but it's a lot better than doing nothing at all. |
with the move of git to other hashing functions (git/git@721cc43) and other possible changes/improvements introduced to git since this issue, can we reach a consensus on how to define git relatively to its |
To be honest, I'm not sure anything needs to be done here. Yes, SHA-1 has problems, and they're not likely to get better. But the best known attacks aren't viable (not even in an economic sense, but that they're based on a technique that leaves traces, and by default Git is built with a SHA-1 library that will detect and reject objects with those traces). Certainly it's still worth moving off of SHA-1, but any big overhaul of the discussion of cryptographic properties may want to wait until the hash transition is further along. That said, if somebody is really bothered by the current language, they're welcome to open a PR. Of the site-specific content, it should just be the about page, since that blog post has gone away. I suspect the Pro Git content would want some looking over, though. |
I see the words "cryptographic integrity" and "strong cryptographic integrity" in two files:
app/views/about/_data_assurance.html.erb
hereapp/views/blog/posts/2010-08-25-notes.markdown
hereThis should be rewritten to say hash-based integrity or something more realistic.
The text was updated successfully, but these errors were encountered: