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

Provide downloaded scripts checksums for security #3461

Closed
wants to merge 1 commit into from

Conversation

dionyziz
Copy link

As reported in #3459, the README file suggests an insecure download and
execution via HTTP instead of HTTPS.

However, before this essential migration to HTTPS is completed, this
patch adds checksums that will allow users to verify the authenticity of
their download.

The authenticity of the checksums is assured because it will be viewed
on an HTTPS-served GitHub page on the correct author username
(robbyrussell).

I have verified this checksum by downloading the HTTP script from two
different network topologies:

  1. The Google corporate network
  2. The Linode network

I also visually verified that the script code is not malicious. I have
also compared it with the HTTPS-served script at this location and found
it to be identical:

https://raw.githubusercontent.com/robbyrussell/oh-my-zsh/master/tools/install.sh

This comes from an HTTPS connection from a valid GitHub domain which
ensures that the owner (robbyrussell) has pushed this file as long as we
trust GitHub and PKI.

This commit is signed with my personal GPG key for authenticity.

The reviewer should also verify the checksum is correct.

Thanks for helping keep users secure.

Fixes #3459

As reported in ohmyzsh#3459, the README file suggests an insecure download and
execution via HTTP instead of HTTPS.

However, before this essential migration to HTTPS is completed, this
patch adds checksums that will allow users to verify the authenticity of
their download.

The authenticity of the checksums is assured because it will be viewed
on an HTTPS-served GitHub page on the correct author username
(robbyrussell).

I have verified this checksum by downloading the HTTP script from two
different network topologies:

1. The Google corporate network
2. The Linode network

I also visually verified that the script code is not malicious. I have
also compared it with the HTTPS-served script at this location and found
it to be identical:

https://raw.githubusercontent.com/robbyrussell/oh-my-zsh/master/tools/install.sh

This comes from an HTTPS connection from a valid GitHub domain which
ensures that the owner (robbyrussell) has pushed this file as long as we
trust GitHub and PKI.

This commit is signed with my personal GPG key for authenticity.

The reviewer should also verify the checksum is correct.

Thanks for helping keep users secure.

Fixes ohmyzsh#3459
@gtklocker
Copy link

Please note that the same suggestions are also given on http://ohmyz.sh/.

@nextgenthemes
Copy link
Contributor

I do not see this suggestion on http://ohmyz.sh/ all I see is that everybody should just copy and paste this insecure code and hope nothing happens, seriously getting annoyed by this lack if caring.

@dionyziz
Copy link
Author

Yes, it would be nice to have this either merged or receive a response from the author :)

@robbyrussell
Copy link
Member

"seriously getting annoyed by this lack if caring"

Don't mistake silence for a lack of caring. I haven't read most of the open pull requests at this point in time. Low priority != Does not care.

@nextgenthemes
Copy link
Contributor

Putting the security of thousads of users at risk by declaring it low priority = lack of careing. WTF has more priority then security you have been warned about for ages and should actually never do it this way in the first place.

I dont get why, just push the damn pull button.

@mcornella
Copy link
Member

For that matter, it's easier for the user and generally safer to just use the github url directly, so I don't think we should keep this one open.
@robbyrussell do you agree on going back to the old, long URL while we're implementing SSL?

@mcornella
Copy link
Member

BTW, the full https URL is just below in the README file, but I understand the average user complaint. I've submitted #3607 for the time being.

@corinnekunze
Copy link

http://ohmyz.sh/ has now been updated with the old, longer URL for the time being.

@robbyrussell
Copy link
Member

@corinnekunze Can you remove the ' --no-check-certificate' bit on the ohmyz.sh page for us?

@corinnekunze
Copy link

@robbyrussell Done and done!

@dionyziz
Copy link
Author

Thanks folks!

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.

Users are instructed to run a script downloaded via HTTP
6 participants