-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
tools: release signing improvements #9071
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Oct 13, 2016
MylesBorins
approved these changes
Oct 13, 2016
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
LGTM |
rvagg
added a commit
that referenced
this pull request
Oct 18, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
rvagg
force-pushed
the
master
branch
2 times, most recently
from
October 18, 2016 17:02
c133999
to
83c7a88
Compare
jasnell
pushed a commit
that referenced
this pull request
Oct 18, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
landed in df163c7 and used for latest releases, they all have detached signatures now! |
rvagg
added a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
rvagg
added a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
rvagg
added a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
rvagg
added a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
jasnell
pushed a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
jasnell
pushed a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 19, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Merged
MylesBorins
pushed a commit
that referenced
this pull request
Oct 26, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 26, 2016
PR-URL: #9071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Closed
2 tasks
maclover7
added a commit
to maclover7/node
that referenced
this pull request
Sep 12, 2017
It is more secure to verify SHASUMS256.txt files via SHASUMS256.txt.sig than SHASUMS256.txt.asc. [This comment](nodejs#6821 (comment)) does the best job at explaining the issue. Refs: nodejs#6821, nodejs#9071
rvagg
pushed a commit
that referenced
this pull request
Sep 13, 2017
It is more secure to verify SHASUMS256.txt files via SHASUMS256.txt.sig than SHASUMS256.txt.asc. This comment does the best job at explaining the issue: #6821 (comment) Refer: #6821 Refer: #9071 PR-URL: #15107 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jasnell
pushed a commit
that referenced
this pull request
Sep 20, 2017
It is more secure to verify SHASUMS256.txt files via SHASUMS256.txt.sig than SHASUMS256.txt.asc. This comment does the best job at explaining the issue: #6821 (comment) Refer: #6821 Refer: #9071 PR-URL: #15107 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 17, 2017
It is more secure to verify SHASUMS256.txt files via SHASUMS256.txt.sig than SHASUMS256.txt.asc. This comment does the best job at explaining the issue: #6821 (comment) Refer: #6821 Refer: #9071 PR-URL: #15107 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 25, 2017
It is more secure to verify SHASUMS256.txt files via SHASUMS256.txt.sig than SHASUMS256.txt.asc. This comment does the best job at explaining the issue: #6821 (comment) Refer: #6821 Refer: #9071 PR-URL: #15107 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes confusion around the
Hash: SHA-1
that comes with releases in the SHASUMS256.txt.asc, e.g. nodejs/nodejs.org#956 (this has come up multiple times!). Newer versions of gpg I think default to SHA-512 and we now have a mix of SHA-1 and SHA-512 in our releases (195 SHA-1 and 63 SHA-512)! By setting it to 256 we make it the same as the hash we use for the binaries themselves, so even though these are two separate hashes we don't have the confusion when someone misinterprets what it means.The second commit in here finally fixes #6821 by adding a detached signature file along with releases. So we'll have SHASUMS256.txt (bare), SHASUMS256.txt.asc (with clear text hash included) and SHASUMS256.txt.sig (binary, signature only) in our releases. So our recommendation for verifying releases will be to download both the .txt and the .sig and doing
gpg --verify SHASUMS256.txt.sig SHASUMS256.txt
. This commit doesn't update the README.md with the new instructions yet, I figure that can be done later once we actually have some .sig files out there.Currently I don't imagine we want to remove the .asc files from releases, that'd probably cause more chaos than we need right now.
I'll test this with the v6 LTS release next week to confirm all is good if I can get some 👍's to do so.