Skip to content
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

Use EVTag for release tags #7579

Closed
indutny opened this issue Jul 7, 2016 · 16 comments
Closed

Use EVTag for release tags #7579

indutny opened this issue Jul 7, 2016 · 16 comments
Labels
build Issues and PRs related to build files or the CI.

Comments

@indutny
Copy link
Member

indutny commented Jul 7, 2016

By default, git provides very loose security guarantees of signed tags. Only the tip of SHA-1 Merkle tree is used, and thus overall security is bound above with the complexity of SHA-1 collision attack.

At this time, SHA-1 is considered insecure, so there is not too much point in generating signed tags as they are, since it may be possible to present different source with the same tag hash if SHA-1 collisions will be found.

However, instead of removing signatures altogether (which we shouldn't do, at least because of the green "Verified" badges on github), I suggest that we should give a try to:

https://github.com/indutny/git-secure-tag

This is a pretty simple tool that builds a Merkle tree too, but with a stronger SHA-512 digest. It is a JS-only implementation of https://github.com/cgwalters/git-evtag , but still quite a fast one:

$ time git secure-tag hash
4043b85fe605d3cb57c1b635801da18d2a521e382dd8f4f186decf0680d1b293b79e5366e38aa216e67a35cc9779bd7b0b0ec988874315e01d5c68931da1809a

real    0m3.127s
user    0m2.692s
sys 0m0.991s
@indutny indutny added the build Issues and PRs related to build files or the CI. label Jul 7, 2016
@indutny
Copy link
Member Author

indutny commented Jul 7, 2016

cc @nodejs/release

@indutny
Copy link
Member Author

indutny commented Jul 7, 2016

cc @nodejs/collaborators

@rvagg
Copy link
Member

rvagg commented Jul 7, 2016

$ git secure-tag securetagtest

You need a passphrase to unlock the secret key for
user: "Rod Vagg <rod@vagg.org>"
2048-bit RSA key, ID 7D83545D, created 2013-11-18
$ git secure-tag -v securetagtest
gpg: Signature made Thu 07 Jul 2016 16:55:43 AEST using RSA key ID 7D83545D
gpg: Good signature from "Rod Vagg <rod@vagg.org>"
gpg:                 aka "Rod Vagg <r@va.gg>"
Good Git-EVTag-v0-SHA512 hash
$ git tag -v securetagtest
object 95170bd48ede90e466a1fcef2dd623dfdae75c44
type commit
tag securetagtest
tagger Rod Vagg <rod@vagg.org> 1467874543 +1000

securetagtest

Git-EVTag-v0-SHA512: 897a0d68e5be8100d37c9d7f8e75bd0c7f8b15d0cd1d4f2601d6632dc9a1e8816659034220028d53c6e8a546221e44280b6d3c8475012dc60b8114060445dd56
gpg: Signature made Thu 07 Jul 2016 16:55:43 AEST using RSA key ID 7D83545D
gpg: Good signature from "Rod Vagg <rod@vagg.org>"
gpg:                 aka "Rod Vagg <r@va.gg>"

So entirely compatible with what we do now.

@indutny has this got traction anywhere else? How early would we be in the early-adopter phase of this technique? I like that it has version numbers in it but I can imagine there's some evolution of this still to come.

@indutny
Copy link
Member Author

indutny commented Jul 7, 2016

@rvagg I would say we are going to be very early adopters, good news is that (as you pointed out) this is completely compatible with default git tag -v, so there is no risk. Additionally, I'd say that it can't lower the current security level in any way, since it just adds extra information to it. It should be pretty safe to do this as an experiment!

@cgwalters sorry for summoning you, but perhaps you have some information about companies/projects that use EVTag?

Also cc @bnoordhuis

@Fishrock123
Copy link
Contributor

Hmmm, seems interesting.

@indutny Are signed commits similarly only signed with SHA-1?

@bnoordhuis
Copy link
Member

@indutny Just a suggestion but perhaps you can add tests that verify it generates the same SHA-512 as git-evtag for the same input. It would be a shame if the tools output incompatible tags because of e.g. differences in iteration order.

@indutny
Copy link
Member Author

indutny commented Jul 7, 2016

@Fishrock123 yep, they are.

@bnoordhuis done indutny/git-secure-tag@92a6879

@cgwalters
Copy link

I think it's clear this functionality should move into the git upstream (and have an implementation in libgit2 after) - without that happening I think it's difficult to build momentum, though it might be interesting to see if a pure node implementation will help.

Mostly so far it's been my original project ostree, gnome-terminal and other random ones like libosinfo.

@indutny
Copy link
Member Author

indutny commented Jul 7, 2016

@cgwalters thank you! Added all of the projects to my evtag-compliance tests:

indutny/git-secure-tag@68a5087

cc @bnoordhuis

@bnoordhuis
Copy link
Member

I think it's clear this functionality should move into the git upstream

I agree, FWIW.

(I wanted to bring that up in my first comment but I forgot.)

@indutny
Copy link
Member Author

indutny commented Jul 7, 2016

@bnoordhuis I can't agree more. Nevertheless, shipping it to a projects is the best way to make people consider merging it upstream.

@bnoordhuis
Copy link
Member

Right, I'm not disagreeing with that.

@indutny
Copy link
Member Author

indutny commented Jul 7, 2016

@rvagg what should we do to move forward on this? Should it be documented for our release team?

@rmg
Copy link
Contributor

rmg commented Jul 7, 2016

@indutny @rvagg the response here seems pretty positive, albeit from a limited audience so far. I imagine a PR for adding it to the docs be the next step and might be better to do sooner rather than later so that the conversation moves/continues there instead of being split between here and there.

@indutny
Copy link
Member Author

indutny commented Jul 8, 2016

Opened #7603, please continue the discussion there.

@rvagg
Copy link
Member

rvagg commented Jul 8, 2016

👍

@rvagg rvagg closed this as completed Jul 8, 2016
indutny added a commit to indutny/io.js that referenced this issue Jul 23, 2016
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: nodejs#7579
indutny added a commit that referenced this issue Aug 5, 2016
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
cjihrig pushed a commit that referenced this issue Aug 10, 2016
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this issue Sep 9, 2016
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this issue Sep 28, 2016
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
rvagg pushed a commit that referenced this issue Oct 18, 2016
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this issue Oct 26, 2016
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants