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

Stop promoting insecure practices #14346

Closed
1 task done
grepwood opened this issue Jul 12, 2024 · 8 comments
Closed
1 task done

Stop promoting insecure practices #14346

grepwood opened this issue Jul 12, 2024 · 8 comments
Labels
enhancement [core label] installer / updater Feedback for installation and update process

Comments

@grepwood
Copy link

grepwood commented Jul 12, 2024

Check for existing issues

  • Completed

Describe the feature

image

If applicable, add mockups / screenshots to help present your vision of the feature

https://letmegooglethat.com/?q=why+curl+sh+is+bad

@grepwood grepwood added admin read Pending admin review enhancement [core label] triage Maintainer needs to classify the issue labels Jul 12, 2024
@BryceBlankinship
Copy link

This is such an absurd nitpick.. curling an install shell script is common practice of linux applications everywhere. Do you have an issue opened on Homebrew's repo? They do the same thing. The curl command Zed provides doesn't use sudo, and also uses https. Just like visiting a website (which, newsflash: is an http GET request like curl) even after https is enabled, it comes down to trusting the domain you're visiting. Here is the install.sh file that you are concerned about: https://github.com/zed-industries/zed/blob/main/script/install.sh

If that is still not secure enough for your standards, you can download manually here: https://zed.dev/docs/linux#other-ways-to-install-zed-on-linux

@wesh92
Copy link

wesh92 commented Jul 12, 2024

I would add to what @BryceBlankinship said: your computer only does what you tell it to. If you go around downloading insecure or untrusted .exe's on windows it is no different than this. I would suspect you can trust it in this case and to reiterate: this is a common Linux practice.

@ConradIrwin
Copy link
Member

Thanks for suggesting this.

We don't currently have plans to promote a different primary installation mechanism, as this one works the most reliably for the most users. Once we get to a point where a majority of linux users can install via their systems package managers (which is a long road to travel) we will likely switch to recommending that instead (as this can help us ensure that the dependencies we need are available).

If you'd like to help with that effort, see https://zed.dev/docs/development/linux#notes-for-packaging-zed.

@JosephTLyons JosephTLyons added installer / updater Feedback for installation and update process and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Jul 12, 2024
@grepwood
Copy link
Author

grepwood commented Jul 13, 2024

@BryceBlankinship @wesh92 @ConradIrwin I don't know where do you fellows work and I don't want to make it sound like I'm judging your workplace. I'd like to inform you that some establishments won't let this kind of command fly on their network due to security reasons.

This installation method makes 0 effort to verify that curl has downloaded the correct content because users have no way of knowing the correct checksum of your installation script ahead of time, which opens up users for attacks and accidents.

  • if someone steals your domain, a threat actor can redirect your users to a malicious host that will serve files that infect them with malware. This is furthermore ripe for exploitation, because you do not provide GPG-signed native packages. On a normal system, the package manager expects that all packages are signed with a trusted GPG key, so this is how a domain hijack is thwarted.
  • if a company requires that you download this via a proxy, or via Artifactory, and you bungled up your proxy setup or your Artifactory token, you will in best case scenario download an HTML error page and execute its content in the POSIX shell child process which can range from minor damage (creation of files, error message) to outright damage (HTML tags may contain redirections to and from existing files and may cause serious damage).

Regardless, this action is also unreproducible due to never being guaranteed to return the same shell script twice over the span of time, which will hurt your users' ability to debug a failed installation and roll it back, if they haven't saved a copy locally.

You're also opening up yourself for breaking this setup for other people by passing it to a POSIX shell, because now you have to remember to never write something that isn't POSIX. It's a hard line to toe. If the endpoint https://zed.dev/install.sh one day serves a script with features and syntax specific to Bash, or ZSH, or another shell, then it will break on every Linux distro that doesn't use that shell as its designated shell for POSIX shell compliant scripts. I kid you not but /bin/sh is a symlink to different shells depending what distro you have.

Based on this information alone, I don't think the Security Team at my workplace would be very impressed with Zed and if they have objections, it won't fly.

I'm not even going to comment about trying to justify this with other people doing this.

To be fair, I think that this situation could be improved if Zed would provide native packages for distributions, signed with GPG. Google Chrome does it for both RHEL and Debian, if you're looking for examples. You don't even have to support all of them, because the more niche distros can take care of themselves.

@injust
Copy link
Contributor

injust commented Jul 13, 2024

I don't know where do you fellows work and I don't want to make it sound like I'm judging your workplace. I'd like to inform you that some establishments won't let this kind of command fly on their network due to security reasons.

You can also install using one of the other methods.

  • if a company requires that you download this via a proxy, or via Artifactory, and you bungled up your proxy setup or your Artifactory token, you will in best case scenario download an HTML error page and execute its content in the POSIX shell child process which can range from minor damage (creation of files, error message) to outright damage (HTML tags may contain redirections to and from existing files and may cause serious damage).

Updating the command to curl -fsSL should resolve this.

Regardless, this action is also unreproducible due to never being guaranteed to return the same shell script twice over the span of time, which will hurt your users' ability to debug a failed installation and roll it back, if they haven't saved a copy locally.

There no implied guarantee of reproducibility, even if you install from a package manager. Installation scripts change, whether installing from a package manager or otherwise. If you care about reproducibility, then indeed you should save a local copy.

You're also opening up yourself for breaking this setup for other people by passing it to a POSIX shell, because now you have to remember to never write something that isn't POSIX. It's a hard line to toe. If the endpoint https://zed.dev/install.sh one day serves a script with features and syntax specific to Bash, or ZSH, or another shell, then it will break on every Linux distro that doesn't use that shell as its designated shell for POSIX shell compliant scripts. I kid you not but /bin/sh is a symlink to different shells depending what distro you have.

They probably intend on keeping the script POSIX compliant. There's no problem with that.

And bugs can happen. It's not the end of the world if the script gets broken accidentally. There are, once again, alternative installation methods.

Based on this information alone, I don't think the Security Team at my workplace would be very impressed with Zed and if they have objections, it won't fly.

Then 👏 install 👏 it 👏 another 👏 way 👏. The goal is to make it easy and reliable to install Zed, not to satisfy your workplace's security team.


I think this distills into:

  1. Update the curl command
  2. Sign with GPG

@BryceBlankinship
Copy link

I think this distills into:

Update the curl command
Sign with GPG

Upvote on that, would solve all of @grepwood's concerns. My original comment wasn't to dismiss his concerns, but more comment on the absurdity to open an issue (on a repo that already has 2400+ open issues) just complaining without offering a solution. He did however gain respect back from me when he actually clarified further and offered some solutions to what is still a negligible problem given Zed offers manual installation mediums.

@injust
Copy link
Contributor

injust commented Jul 17, 2024

Opened #14667 to add -fsSL to the curl commands.

I'm not very familiar with GPG signing, so it'd be great if someone else could open an issue for that!

@injust
Copy link
Contributor

injust commented Jul 17, 2024

Also @grepwood I just realized that the install.sh script is in this repo at https://github.com/zed-industries/zed/blob/main/script/install.sh, so that should also help with the reproducibility concerns.

ConradIrwin pushed a commit that referenced this issue Jul 17, 2024
Release Notes:

- Updated curl commands with `-f` for improved error handling
([#14346](#14346)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement [core label] installer / updater Feedback for installation and update process
Projects
None yet
Development

No branches or pull requests

6 participants