search

TheKevJames / puppet-homebrew

homebrew (+brewcask! +taps!) package installer and provider
https://forge.puppet.com/thekevjames/homebrew
Apache License 2.0
18 stars 44 forks source link

Deal with root-brew #40

Closed TheKevJames closed 7 years ago

TheKevJames commented 8 years ago

https://github.com/Homebrew/brew/issues/796

Some thoughts: between brew devs hating brew-as-root and Apple doing everything it can to strip root privileges, brew-as-root is going to get harder and harder. Should that be reflected in this package? Should we enforce using a new user to sidestep the issue completely?

zbentley commented 8 years ago

This will break my workflow, (boxen), but I support it anyway. Why? Because I tried building a master-branch change to DBus via Homebrew, and it deleted all the contents of my /var directory due to a build system bug.

Most of the use cases for brew-as-root I'm familiar with are wrapper tools, like Boxen or Strap or similar. The ones that aren't actively moving away from root-allowed execution are only sticking with it due to the convenience factor: existing packages/install steps that assume root privs almost always require modification to do escalation/de-escalation themselves. That convenience factor is getting smaller and smaller in the face of a) tools that behave properly becoming popular and b) security risks getting publicized.

For this package, I think a configurable, non-root brew user that defaults to the user invoking puppet is the way to go. This package should assume that a user may have set up a Homebrew environment by hand, and interoperate with whatever they've set up (provided that it behaves sanely with the defaults of the commands this module runs; no need to bend over backwards).

The idea of a controlled, limited/managed, package-installs-only user is a good one, but I think it belongs at a level of abstraction above this package.

Tl;dr: I think preventing root-brew is a great idea, but encouraging a new separate user is not; running commands as the invoking user should be sufficient.

MikeMcQuaid commented 8 years ago

Some thoughts: between brew devs hating brew-as-root

We don't "hate" it, it's a terrible security practice because we don't drop privileges and you're trusting a lot of code you shouldn't to not write in random places on your filesystem as root.

Because I tried building a master-branch change to DBus via Homebrew, and it deleted all the contents of my /var directory due to a build system bug.

We've started using the OS X sandbox that means that writes to random locations are now restricted. This does not work as root so you're not getting those protections there either.

Most of the use cases for brew-as-root I'm familiar with are wrapper tools, like Boxen or Strap or similar.

Just FYI: Boxen should not ever need to run Homebrew as root any more (I believe I fixed those cases, any remaining should be considered a bug but I'm not longer using/working on Boxen). Strap does not run Homebrew as root.

Tl;dr: I think preventing root-brew is a great idea, but encouraging a new separate user is not; running commands as the invoking user should be sufficient.

Agreed (which is why Homebrew defaults to this too) 👍

TheKevJames commented 8 years ago

Some thoughts: between brew devs hating brew-as-root

We don't "hate" it, it's a terrible security practice because we don't drop privileges and you're trusting a lot of code you shouldn't to not write in random places on your filesystem as root.

Re-reading what I originally posted, that came off far more accusatory than I meant; sorry about that! What I meant was that, having seen the recent changes to the Apple atmosphere (re: SIP in El Cap, etc) and homebrew itself (homebrew/brew#796 and its fix, etc), it looks like the capability for users to run homebrew as root (yes, even though that's overall a terrible idea) will be completely removed soon.

That said, despite brew-as-root being an all-around bad idea, there are users that do so. As @zbentley mentioned, there are wrapper tools that have done this (even if Boxen and Strap do not) and I'm sure other scenarios I have not considered.

Given this capability will be removed from brew itself but may still (for now) be used by (hopefully fewer and fewer) users, this package needs to come up with a migration strategy.

Personally, I agree with @zbentley's response to my braindump, that preventing root without enforcing a package-only user is likely the way to go, in line with how brew works/will work. Now the question becomes where and when this should be handled.

Should puppet-homebrew prevent the usage of user==root or should it just give a warning? Should it give a warning for now and prevent it in a later version, a la following homebrew's changes? What about migration: should users who currently have user==root in their manifests get new errors on upgrading their version of puppet-homebrew, or should we consider a migration utility? I'm not sure how changing this user value will affect brew -- homebrew::install should chown everything properly, but will that be enough to migrate users?

zbentley commented 8 years ago

Another question: what if people are invoking puppet as root and leaving the user unspecified in the manifest?

TheKevJames commented 8 years ago

User is currently a mandatory field in puppet-homebrew and can't be left blank. That said, puppet-as-root is (I believe) the normal way to run puppet though, so there's a decent usecase for users provisioning systems that literally don't have non-root accounts (think server farms, test systems, etc) or for users not really thinking about which user to use and, given puppet is a root-level app, setting puppet-homebrew { user => root } without giving it much thought.

TheKevJames commented 8 years ago

Suddenly I understand what @zbentley was referring to in the last post: users that don't actually include class homebrew { user => anything } but rather just use Package { provider => homebrew } without using this module to manage the actual homebrew install. Oops.

jordigg commented 8 years ago

I use puppet to manage and setup new workstations for our employees together with some build servers.

On the build servers or servers in general it shouldn't be an issue since we always use the same specific user to run everything.

On the workstation front I don't know the name of the user in advance or it may be a shared desktop (people login with AD) so I require a way to apply changes and install applications that must work with a multi-user environment. Right now we are doing it as root. Alternatively, I think, we could setup hidden admin accounts that do all the brew work and making sure apps get installed on /Applications as well as setting proper permissions on the brew required folders and files so any user can run an update/install command (the permission setup allows multi-user but it can be improved). With that kind of setup it may work as well. I'll give it a try.

zbentley commented 8 years ago

@jordigg I think you can do what you need without knowing the user, and without running brew as root.

For core homebrew stuff: You'll need to set Puppet (run as root) to manage the permissions on /usr/local or your custom Homebrew dir in a way slightly different than usual. You can do this with a group that all users of a multitenant system are members of. While slightly dated, these links might help get you started in that area:

For casks (things that you want to install to /Applications): casks want to install (symlink, really) to ~/Applications by default. You can change that default for this package by setting a resource default in Puppet, e.g.

Package {
  provider => homebrew,
  install_options => [
    '--appdir=/Applications',
  ],
}

You can change that default for packages manually installed by users (if you really want users to be able to install globally-available apps) by overriding the default Homebrew settings via the environment. You may also need to Puppet-manage the permissions on the cask cache directory for the installed apps to be launchable by other users; that process should be similar to what I suggested for /usr/local, but for the cask directory instead.

jordigg commented 8 years ago

@zbentley yes I already do all of this and the "multi-tenant" permissions are set on the module by default, see https://github.com/TheKevJames/puppet-homebrew/blob/master/manifests/install.pp#L7

But puppet still needs a user to be defined or will run as root by default so an "IT" admin account is required on the system because the other users in there will always be different on our specific case. With that I'm sure we can run everything without relying on using root at all. I'll update our setup to avoid root.

Maybe we can still allow root usage with a warning and offer instructions on the readme on how to setup that hidden admin account via puppet. That may help other users with the same problem while following brew recommendations.

zbentley commented 8 years ago

If your users are invoking puppet (via sudo), then you should pass the sudoing user into puppet code that cares about it, and assume that this module will break/error if it is run as "real root" (where root is the logged-in user). Facter might be useful here (I bet there's already a module out there that gives you a fact that supplies sudoing user in a portable-ish way). You could also use a hack and read an environment variable.

If puppet itself is being run as root, by root (e.g. via a cron job), you should do as you suggested and use a separate puppet-role admin account.

TheKevJames commented 8 years ago

AFAIK this shouldn't be an issue with which user is invoking puppet, since we invoke brew as its owner of brew regardless of the owner of the puppet process.

Rather the issue is with which user owns brew, which is what this module can control by setting puppet-homebrew { user => something } if and only if users install/manage homebrew with this library rather than only using the provider.

So there are... eight?... scenarios I can think of off-hand based on three binaries (user uses this module for installation, brew owner is root, puppet is run as root). There is also the case where the non-root owner of brew and the non-root user running puppet are not the same, but we'll ignore that for now since it's a whole other issue (which I assume should work given we run brew as that user? Puppet user would need su brew-owner privileges though...):

tldr is the only thing that really matters is the owner of the brew binary, whether we manage that or not. Given the goal of preventing root usage, it's looking like we'd need two changes: in homebrew::install throw an error if user == root and in brewcommon.rb throw an error if the brew binary is owned by root.

@jordigg raises a good point about systems where the user can't be known in advance, but we do know that we'll want to have that user own the brew process eventually. This seems kind of edge case-ey to me, and I'm not totally against ignoring it completely, but it'd be good to get more input on this.

The only way I could think of making this work ootb would be to look into the nobody user: maybe setups like Jordi's would run homebrew::install { user => nobody } by default, with nobody replaced by a user profile once one is known? Tested that for root->user and the migration works seamlessly. If this is a good approach, we could consider making nobody a default user for puppet-homebrew or just add this workaround to the readme (open to suggestions).

MikeMcQuaid commented 8 years ago

My take is that you should do the same with this module as Homebrew does:

jordigg commented 8 years ago

I can confirm that using brew as root doesn't work anymore because they now show an error message in the output of any command you run which makes our parser fail and therefore it won't install or uninstall any applications. I'm now checking if a environment variable can be set to disable that message until I move away from root. Seems that November 1st is the date when brew won't work anymore under root.

sh-3.2# brew list --versions
Error: Running Homebrew as root is extremely dangerous. As Homebrew does not
drop privileges on installation you are giving all build scripts full access
to your system. As a result of the OS X sandbox not handling the root user
correctly HOMEBREW_NO_SANDBOX has been set so the sandbox will not be used. If
we have not merged a pull request to add privilege dropping by November 1st
2016 running Homebrew as root will be disabled. No Homebrew maintainers plan
to work on this functionality.
MikeMcQuaid commented 8 years ago

in the output of any command you run which makes our parser fail and therefore it won't install or uninstall any applications.

This should be fixable as the error is just on stderr.

jordigg commented 8 years ago

@MikeMcQuaid that's correct, I'll modify the module to skip the stderr part, it will simplify the code since brew and brew cask seem to return the same output when doing brew list --versions.

Now I have the issue when try to install certain packages like Java or Unity which require to enter the password. Is there any easy way to skip that in order to automate the install of those packages?

I'm trying using a new admin account instead of using the root user. Should I add that new admin user to the sudoers file?

TheKevJames commented 8 years ago

I'm not quite sure I understand your workflow; you should only ever need to enter a password if you run puppet as non-root and install brew as a different non-root account, AFAIK.

The recommended workflow for puppet-homebrew is and always will be running puppet as root and having brew installed as a non-root user, regardless of whichever other configurations we support. Under this configuration, puppet-homebrew should be able to run installs without requiring passwords.

In this case, your workflow would be

[root]$ puppet apply -e 'class { "homebrew": user => admin }'

If you want to run puppet as non-root and install homebrew as a different non-root user, that's a bit more complicated: you'll need to give the puppet user the capability to su as the homebrew user without the use of a password, which is beyond the scope of this module (ie. the command /usr/bin/su ${homebrew::user} <command> as puppet user must work passwordlessly).

jordigg commented 8 years ago

Though was an error I was getting but after a fresh test environment everything is working fine now.

Maybe this must go on another issue but I still have problems to get the "homebrew python" version installed. There's no error, is just not getting installed. I guess it because the mac version is already there and puppet gets confused. If I call manually brew install python works.

A Exec on Puppet with /usr/bin/su ${homebrew::user} -c <command> also works. Right now is the only package that doesn't want to be installed using the module.

TheKevJames commented 8 years ago

Interesting, I use this module to install python and have no problems. Might be worth creating a new issue for investigating that separately. re mac version already being there: maybe, do you have the same error with other brew-managed apps that come pre-installed on OSX? Either way, let's move the discussion about that to a new issue.

MikeMcQuaid commented 8 years ago

For casks (things that you want to install to /Applications): casks want to install (symlink, really) to ~/Applications by default.

I've allowed Homebrew Cask to use root: https://github.com/Homebrew/brew/pull/850

TheKevJames commented 8 years ago

NOTE: brew-as-root now deprecated in puppet-homebrew, once root usage is completely removed from brew (current timeline: November) it should be removed here as well.

TheKevJames commented 7 years ago

brew-as-root is now officially removed upstream (https://github.com/Homebrew/brew/pull/1452). Let's remove it here, as well!