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

Security: move shims to end of PATH #247

Closed
stevegt opened this issue Sep 30, 2022 · 17 comments
Closed

Security: move shims to end of PATH #247

stevegt opened this issue Sep 30, 2022 · 17 comments

Comments

@stevegt
Copy link
Contributor

stevegt commented Sep 30, 2022

Life must have happened and I dropped this and never ran into the problem myself -- we still have a vulnerability similar to #99. As @mattes mentions in #99:

goenv init - will set export PATH="$GOENV_ROOT/shims:$PATH" (shims at the beginning of path). Isn't that a problem, too?

Here's what that allows, for instance:

$ which ar
/usr/bin/ar
$ touch go/1.14.2/bin/ar  # <== arbitrary version, doesn't need to be in $PATH or $GOPATH
$ goenv rehash
$ which ar
/home/stevegt/.goenv/shims/ar

The offending code appears to be in these two places in goenv-init:

@huyz
Copy link

huyz commented Feb 2, 2023

Hmm, this goes counter to how all the *env tools work, and yeah there are conflicts with /usr/bin/go and on macOS /opt/homebrew/bin/go. Perhaps there need to be two separate shims directory: one for main binaries and another for third-party packages.

@stevegt
Copy link
Contributor Author

stevegt commented Feb 3, 2023

Translating pyenv to goenv gave us a relatively proven *env for Go, but it's pretty clear at this point that the way pyenv works has some issues.

  1. From a security perspective, it's way too easy for packages to either innocently or maliciously mask a well-known binary. See Security: Move $GOPATH/bin to end of $PATH #99 for an example of how this bit me in goenv.
  2. It's also unnecessary for goenv to remove old binaries from the $PATH. This, too, is an unintended consequence of translating pyenv to goenv. Go is a compiled language and we don't need to rebuild binaries for existing packages just because we upgraded the compiler. The shims should be able to find the most recent version of a built binary, but right now they don't -- see Q: Do we want shims to automatically execute older binaries after finding them? #249.

For these reasons, it's worth thinking through how goenv should actually work vs pyenv. They are different use cases.

@huyz In my own case, I removed /usr/bin/go when I started using goenv years ago -- too many opportunities for confusion if it's left in place. Removing the system Go would be my recommendation on Linux -- would that work on Mac as well?

@huyz
Copy link

huyz commented Feb 4, 2023

At least for me, I don't think that removing other gos (or other pythons, rubys, etc.) is the right solution. For me the whole point of the *env apps is either (1) isolation or (2) overriding what I consider "system" interpreters/compilers (whether macOS, Homebrew, MacPorts—or the Linux equivalents). Homebrew and MacPorts have a system of dependencies where installing an app automatically pulls in python, for example. So in general, I don't want to fight the coherence of these package managers. (Perhaps this is less relevant to go since apps are compiled; but I'm thinking holistically as I'm looking for a general solution that applies to *env.)

So if I understand you right, the security risk of third parties dropping dangerously-named executables applies to all *env tools, but even more so with goenv because of how go get works? (Sorry, it's been a while since I've used go so I don't remember the specifics.)

I think for me the solution is to keep the shims directory at the end as you've done, but symlink go and gofmt into my ~/bin dir, which is always in the front. Depending on how you answer about the security risks of non-goenv *env, I might do the same for the other *env tools.

@stevegt
Copy link
Contributor Author

stevegt commented Feb 6, 2023

At least for me, I don't think that removing other gos (or other pythons, rubys, etc.) is the right solution. For me the whole point of the *env apps is either (1) isolation or (2) overriding what I consider "system" interpreters/compilers (whether macOS, Homebrew, MacPorts—or the Linux equivalents).

I agree it's not easy to override system tools unless they are later in the $PATH.

Homebrew and MacPorts have a system of dependencies where installing an app automatically pulls in python, for example. So in general, I don't want to fight the coherence of these package managers. (Perhaps this is less relevant to go since apps are compiled; but I'm thinking holistically as I'm looking for a general solution that applies to *env.)

The general solution would be to go with a system of isolation such as containers or NixOS. Traditional operating systems, whether from commercial vendors or from open-source distributions, tend to make the assumption that the package manager manages all executables. If the vendor or distribution doesn't provide an isolation method, then any *env tool you add will always be a workaround and will always be fighting the package manager in some way.

So if I understand you right, the security risk of third parties dropping dangerously-named executables applies to all *env tools, but even more so with goenv because of how go get works? (Sorry, it's been a while since I've used go so I don't remember the specifics.)

More recent versions of go get and go install made the situation somewhat better -- go get will no longer drop binaries into the $PATH; you need to use go install for that. But unless I'm missing something there's still not a "dry run" flag or other method of easily discovering what binaries will be added. I once raised this issue with the Go security team, and the odd answer I got from a core developer was that you must carefully inspect each package's source code, including all prereqs, before running go get or go install. They later made that change to go get, so maybe they realized how unrealistic that expectation is.

I think for me the solution is to keep the shims directory at the end as you've done, but symlink go and gofmt into my ~/bin dir, which is always in the front. Depending on how you answer about the security risks of non-goenv *env, I might do the same for the other *env tools.

I mentioned in #249 that I wound up doing something similar -- I have a goenv-basket bash script that manages symlinks to work around the warts. In my case, it is doing the job of symlinking binaries in old versions of go/*/bin into a go/basket/bin that I have in my path so I don't have to recompile the world after I use goenv to upgrade to a new version of the compiler.

@stevegt
Copy link
Contributor Author

stevegt commented Feb 6, 2023

Depending on how you answer about the security risks of non-goenv *env, I might do the same for the other *env tools.

In the case of pyenv, python packages are less likely to install executables in the $PATH, and if they do it's ordinarily one file with some sort of unique name that is less likely to conflict with a well-known executable. I think this is to some extent cultural, but is also reinforced by the fact that setup.py has to explicitly name each executable -- it's not likely that a python package author will accidentally install an executable on others' machines. If a python package installs an executable on purpose it's easy to preview what will happen by looking at the contents of setup.py.

@huyz
Copy link

huyz commented Feb 14, 2023

Minor note: I just realized that sometimes there already is a $GOENV_ROOT/bin where goenv is added (albeit not in the case of, say, Homebrew installation of goenv). Might as well make that bin directory do double-duty and also hold symlinks for go, gofmt, and any other executables that are vetted and can go early in the PATH. Same applies to the other *env. Now I wish all the *env executables would do follow this scheme.

@huyz
Copy link

huyz commented Feb 14, 2023

I wonder if something that anyenv should handle once and for all.

@alexsapran
Copy link

I happen to face the issue that after the suggested fix was merged and updated my goenv, when switching from project A which has go version 1.18.x to project B with version 1.19, then goenv would not detect the new go shim.

If I moved the path to the beginning of the PATH, as it was already mentioned done by other *env tools, it works fine switching versions.

@stevegt
Copy link
Contributor Author

stevegt commented Mar 8, 2023

@alexsapran That does not sound related to this change -- both 1.18 and 1.19 should be equally reachable via the shims. It sounds like you have a non-goenv-managed Go version somewhere in your PATH, or a .go-version file in project B's directory, or were working in a shell that had goenv shell set.

@alexsapran
Copy link

I thought that it was a non-goenv-managed indeed and made sure my env was clean, removed it.
The 2 projects had different .go-version files with 1.18 and 1.19.
Changing the order of the exported path of shims actually worked, switching projects would result in different go versions.

@stevegt
Copy link
Contributor Author

stevegt commented Mar 8, 2023

For any future folks who find this thread:

  1. This was a security fix which corrected a situation in which anyone with a github account could potentially install binaries at the beginning of your PATH without your knowledge, replacing, for instance, ls, sort, gcc, cat...
  2. The Go core developers realized more recently that this vulnerability was serious enough that they later changed the way go get itself functions, requiring a go install to put binaries in your PATH.
  3. But there is still no easy way of auditing what binaries go install will install in your PATH.
  4. ...and all older Go and go get versions out there still have the earlier vulnerability.
  5. The entire purpose of goenv is to allow use of various versions of Go and go get, including those older versions.
  6. Executable installation in python requires explicit configuration in setup.py.
  7. Executable installation in Go is implicit and prone to masking well-known commands even in non-malicious cases.
  8. As such, trying to make goenv act more like pyenv is actually not what you want.

Which would you rather do:

  • (A) Allow others to silently replace ls on your machine and not discover the replacement until days or months later.
  • (B) Make the time today to clean up your machine so you have better understanding and control of which version of the Go tools you're using. In those cases where you for some reason can't remove a system version of Go earlier in your PATH, use shell aliases, wrapper scripts, etc. Become a better administrator of your own environment.

You really, really want to do (B) -- installing goenv should not be seen as a cure-all.

@alexsapran
Copy link

I am just reporting my experience with trying to use goenv.
I understand the concerns and what it implies. My hope is that a solution could be found through discussion.

I feel this discussion is not very productive. Happy to stop it here.

@stevegt
Copy link
Contributor Author

stevegt commented Mar 8, 2023

@alexsapran My last message was not aimed at you -- I did say "future folks". I think you provided a fine example, and a good followup of your own resolution. Thank you!

I am hoping to head off more duplicate issues being spawned like #279 and #289. I have been feeling compelled to respond to each of these incidents because even though I am not this project's owner, I wrote the PR that fixed the vulnerability.

@mimoo
Copy link

mimoo commented Mar 9, 2023

I'm not sure what's the take away here, don't use goenv?

Also, is the problem that PATH is basically a 1-depth namespaced package manager implemented via the filesystem in the worse way possible?

@huyz
Copy link

huyz commented Mar 11, 2023

I don't know about other folks, but my takeaway in zsh is:

# one time
mkdir -p $GOENV_ROOT/bin
ln -s ../shims/go{,fmt} $GOENV_ROOT/bin

# in zsh RC files
typeset -gU PATH
PATH=$GOENV_ROOT/bin:$PATH

@stevegt
Copy link
Contributor Author

stevegt commented Mar 16, 2023

@mimoo There are two takeaways:

  1. When using any *env tool to manage versions of a compiler or interpreter, things will always work better if you remove any versions not managed by the *env tool. If there is still a system or hand-installed version lurking, you will tend to experience confusing results, regardless of the contents of PATH.
  2. When using any *env tool that inserts its ./bin directory at the head of PATH, you are implicitly trusting the authors of any packages you install with that tool, and all of their prerequisite packages, to not drop things into your PATH that will mimic or mask things later in PATH.

Goenv is fixed as far as (2) is concerned -- it no longer inserts itself at the head of PATH as of #248, and this issue was closed. The conversation since then is around how to manage things when you still have a system or hand-installed version of Go, despite (1).

The suggestion from @huyz isn't bad and might work for others. If anyone wants to tackle automating that in goenv, I'd encourage you to submit a new issue and PR.

@mimoo
Copy link

mimoo commented Mar 16, 2023

Ah yeah I thought about it today, so removing golang (brew remove go) should fix it

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

No branches or pull requests

4 participants