search

cgwalters / git-evtag

Extended verification for git tags
Other
132 stars 13 forks source link

Question about malleability #20

Closed indutny closed 8 years ago

indutny commented 8 years ago

Hello!

First of all, thank you for this project! I was working on something very similar, but in JS.

The question that I have is regarding the possible malleability of the output hash. Let's say that we have a repo with two directories:

lib/
  a
  b
test/
  c

And another one with:

lib/
  a
  b
  test/
    c

Currently they will result in different hashes, however if we think that the collision attack on the SHA-1 is feasible - it is possible to modify file contents in such way that the resulting hashes will turn out to be the same.

I think hashing number of sub-objects into the main digest should be enough to protect against this. What are your thoughts on this?

cgwalters commented 8 years ago

It sounds like https://github.com/cgwalters/git-evtag/issues/4 is what you're looking for?

BTW, if you are making a (re|new)implementation, can you at least match the same algorithm, i.e. have the two tools be interoperable?

indutny commented 8 years ago

@cgwalters I actually started working on it prior to finding this project. The reason why I'm asking this question is that I want to move to a common ground, indeed! 👍

4 is pretty lengthy, may I ask you what was the reason for closing it, and why the number of entries in a tree should not be added to digest?

cgwalters commented 8 years ago

4 had a variant of your concern. So what you're talking about here is about rearranging the tree, but keeping the file contents the same? It's hard to imagine a scenario where that would exploit something as opposed to just causing a build to fail...(well, no, a patches/ directory is an obvious one).

But the thing is, we checksum the tree objects too, so any second preimage replacement of them would be detected.

Basically, checksumming the tree object (as we already do) is strictly stronger than just adding the number of entries to the checksum, right?

indutny commented 8 years ago

Oh... I see now. Ok, let's move on to implementing it in git-secure-tag repo! Thank you!

indutny commented 8 years ago

Just FYI, I have moved to EVTag digest here: https://github.com/indutny/git-secure-tag . Thank you!

(Fun fact, it turned out to be 10x faster to use cat-file --batch than to work with fs directly. Perhaps I was using too much async primitives).