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

Check signatures/checksums to ensure authenticity #25

Merged
merged 7 commits into from
Feb 20, 2017

Conversation

ypid
Copy link
Contributor

@ypid ypid commented Feb 12, 2017

Please refer to Verifying Node.js Binaries for why this is important.

Related to: asdf-vm/asdf#158
Mitigates: nodejs/node#9859
Mitigates: nodejs/node#6821

Implementing this feature required some rework of the install script which is included in this PR. The following other PR are superseded/included in this one:

Closes: #15
Closes: #16 (Merged directly as it is high priority, this PR has been rebased on top of #16)
Closes: #19

Note that this PR also updates the base download URL from "http://nodejs.org/dist" to "https://nodejs.org/dist" meaning that before this PR (or #16), binaries where downloaded over plain legacy HTTP! (those binaries where later executed by the user). This is really bad and is fairly easy to exploit!

Related to: nvm-sh/nvm#736
Related to: nvm-sh/nvm#793

@ypid ypid force-pushed the feature/check_openpgp_signatures branch 2 times, most recently from 8f834c1 to 2e79e06 Compare February 12, 2017 21:52
@ypid
Copy link
Contributor Author

ypid commented Feb 14, 2017

Test failing reason: #24 not merged

@HashNuke
Copy link
Member

@ypid #24 is merged. Can you please resolve the conflicts? I can try rebuilding after that.

@HashNuke
Copy link
Member

@ypid I just read a few comments on one of the linked nvm issues. Thank you very much for this PR ~!

ypid added 6 commits February 20, 2017 06:57
Please refer to [Verifying Node.js Binaries](https://blog.continuation.io/verifying-node-js-binaries/)
for why this is important.

Related to: asdf-vm/asdf#158
Mitigates: nodejs/node#9859
Mitigates: nodejs/node#6821

Implementing this feature required some rework of the `install` script
which is included in this PR. The following other PR are
superseded/included in this one:

Closes: asdf-vm#15
Closes: asdf-vm#16
Closes: asdf-vm#19

Note that this PR also updates the base download URL from
"http://nodejs.org/dist" to "https://nodejs.org/dist" meaning that
before this PR (or asdf-vm#16 which is not merged), binaries where downloaded
over plain legacy HTTP! (those binaries where later executed by the
user). This is really bad and is fairly easy to exploit!

Related to: nvm-sh/nvm#736
Related to: nvm-sh/nvm#793
@ypid ypid force-pushed the feature/check_openpgp_signatures branch from cea3960 to dc758c1 Compare February 20, 2017 06:00
@ypid
Copy link
Contributor Author

ypid commented Feb 20, 2017

Thanks. Rebased and rebuilding. Note that you merged #16 while #24 is still unmerged.

This was referenced Feb 20, 2017
@ypid ypid force-pushed the feature/check_openpgp_signatures branch from dc758c1 to 63db65e Compare February 20, 2017 07:00
@ypid
Copy link
Contributor Author

ypid commented Feb 20, 2017

Thanks for merging #24. Rerun test and passing now.

@ypid ypid force-pushed the feature/check_openpgp_signatures branch from 63db65e to 001999f Compare February 20, 2017 07:39
@HashNuke HashNuke merged commit cf58288 into asdf-vm:master Feb 20, 2017
ypid added a commit to ypid/asdf-nodejs that referenced this pull request Feb 20, 2017
ypid added a commit to ypid/asdf that referenced this pull request Feb 20, 2017
As the proposal for asdf-nodejs to [validate OpenPGP
signatures](asdf-vm/asdf-nodejs#25) has been
approved and merged I guess the design is sound and other plugins can
follow.

Related to: asdf-vm#159
rajr5 pushed a commit to rajr5/asdf that referenced this pull request Jun 23, 2023
As the proposal for asdf-nodejs to [validate OpenPGP
signatures](asdf-vm/asdf-nodejs#25) has been
approved and merged I guess the design is sound and other plugins can
follow.

Related to: #159
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants