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

Add support to install as $INSTALL_USER #789

Closed
wants to merge 6 commits into from
Closed

Conversation

berikv
Copy link

@berikv berikv commented Jul 21, 2023

Allow to install as another user:

INSTALL_USER=ellis /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/berikv/homebrew_install/master/install.sh)"

This is especially useful in headless installs by root, where passing a sudo password would make the install script insecure and / or needlessly complex.

setup_machine.sh // Run as root

...
INSTALL_USER=NoAdmin /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/berikv/homebrew_install/master/install.sh)"
...

Tested on Intel MacOS 13.4.1:

  • Root delegate to user, allowed
  • User delegate to other user, allowed
  • User delegate to root, disallowed

Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this script can't be executed as root unless you're in a CI environment, I'm not sure if this will work for macOS for example.

@@ -107,6 +107,10 @@ then
export USER
fi

# Allow delegating installation to a different user (useful for installs by root)
INSTALL_USER="${INSTALL_USER-${USER}}"
INSTALL_HOME=$(eval echo ~"${INSTALL_USER}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will quietly fail if the user does not exist.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a validation for the INSTALL_USER to exist

@berikv
Copy link
Author

berikv commented Jul 21, 2023

Generally this script can't be executed as root unless you're in a CI environment, I'm not sure if this will work for macOS for example.

I've tested the script to work on MacOS.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide more context for why/when this is needed?

I think it's very bad idea for this script to be run as root and even more so to be doing curl | sh as root. I don't think privileges are being dropped appropriately here and the majority of the script still runs as root. This means a typo or error in a future iteration of this script could have catastrophic results.

README.md Outdated Show resolved Hide resolved
@berikv
Copy link
Author

berikv commented Jul 21, 2023

Can you provide more context for why/when this is needed?

Of course!

In my case, I need to install and maintain a pool of macs. The macs get set up with an installation script for efficiency / reliability.
The installation script needs to download resources and run multiple commands as root. This makes sudo time-out and ask for the password again, at which point the person installing needs to provide the password again. The install script can ask the password once and then store it locally to provide it to sudo every time it is needed.

However, recently I looked at the possibility to have the install script implemented in Swift using SMJobBless. The application then would not have access to the password, but is able to escalate privileges by invoking methods on the blessed helper app.

I found that there is no way to install homebrew as root, for another user. This PR adds that functionality.

@MikeMcQuaid
Copy link
Member

The installation script needs to download resources and run multiple commands as root.

Do you mean your script or this script here? Have you tried making use of SUDO_ASKPASS?

I found that there is no way to install homebrew as root, for another user. This PR adds that functionality.

This is something I'm not 👎🏻 on but would need a few changes:

  • we'd need to have Linux and macOS CI support added to the GitHub workflow(s) to test this functionality and verify it doesn't break/regress
  • the script itself should continue to not be run as root. If another script calls this one, it should drop privileges before calling this script and make use of SUDO_ASKPASS if needed

@SMillerDev
Copy link
Member

SMillerDev commented Jul 21, 2023

In my case, I need to install and maintain a pool of macs. The macs get set up with an installation script for efficiency / reliability.

Have you considered using the macOS pkg that Homebrew builds on releases?

@berikv
Copy link
Author

berikv commented Jul 21, 2023

Thanks for the feedback so far!

Using SUDO_ASKPASS does not solve my use-case because the remote mac install program (or the program that is specified as SUDO_ASKPASS) will still need to store the password so that it can be provided when needed by sudo.
I'd rather not store the password on the mac itself.
I'll try the suggestion on using the pkg installer.

@MikeMcQuaid
Copy link
Member

Using SUDO_ASKPASS does not solve my use-case because the remote mac install program (or the program that is specified as SUDO_ASKPASS) will still need to store the password so that it can be provided when needed by sudo.

It's a trade-off here, really:

  • you want to avoid storing the password and instead running the script as root
  • we do not want the script run as root because of the security implications of doing so

If your script is being as root: it could modify systemwide sudo configuration to not require a password or SUDO_ASKPASS temporarily when run.

@berikv
Copy link
Author

berikv commented Jul 21, 2023

Have you considered using the macOS pkg that Homebrew builds on releases?

Where can I find this pkg build? In the release assets there are only the sources: https://github.com/Homebrew/brew/releases

@berikv
Copy link
Author

berikv commented Jul 21, 2023

If your script is being as root: it could modify systemwide sudo configuration to not require a password or SUDO_ASKPASS temporarily when run.

Agreed, I considered this option.
Another trick would be to add a /.dockerenv file on the machine, run the install as root, then chown $HOMEBREW_PREFIX to the correct user.
Both cases would open a bigger security hole than the adjustment made in this PR, but only in the scenario I'm running in.

@berikv
Copy link
Author

berikv commented Jul 21, 2023

Another perspective on the security impact:

It is already possible to run the install as root by adding a /.docekerenv file on your system.
Passing the INSTALL_USER env var while running as root is a deliberate action from the user.
If the INSTALL_USER env var is not passed, this PR does not (should not) change the install script from a security standpoint.

Given the above I argue that the INSTALL_USER feature is merely a safer approach compared to adding a /.dockerenv file to the system and running as root.

@osalbahr
Copy link
Sponsor Contributor

osalbahr commented Jul 21, 2023

Another trick would be to add a /.dockerenv file on the machine, run the install as root, then chown $HOMEBREW_PREFIX to the correct user.

I’m not sure I understand the security issues of this approach. Does the delegation achieve anything other than who owns the created directories?

@MikeMcQuaid
Copy link
Member

Both cases would open a bigger security hole than the adjustment made in this PR, but only in the scenario I'm running in.

Sorry, I don't see it, personally. Regardless, I'm afraid we have to do what's best for all Homebrew users here and not just your specific case.

It is already possible to run the install as root by adding a /.docekerenv file on your system.

Sure, it's possible; it's also pretty obvious that that's a hack built to support Docker specifically.

If the INSTALL_USER env var is not passed, this PR does not (should not) change the install script from a security standpoint.

I don't agree. It's running as root for some users in some documented flows in that case.

@Bo98
Copy link
Member

Bo98 commented Jul 21, 2023

In our CI we have a setup where we have a user with sudo privileges and and a user without sudo privileges, and I do have to jump through hoops like this to run the installer through the account with sudo but to have it create directories and chmod as the non-sudo user.

As such I would personally like to see a change similar to this that makes that possible where you can run the installer as the account with sudo to install Homebrew for a different user account, as it will make it easier for me to setup new images in the future. However that workflow doesn't require running the whole installer as root in our case, so that part is still up for debate.

The installation script needs to download resources and run multiple commands as root. This makes sudo time-out and ask for the password again, at which point the person installing needs to provide the password again. The install script can ask the password once and then store it locally to provide it to sudo every time it is needed.

Can you expand more on this? Most of the downloading appears at the end.

One exception is the Xcode Command Line Tools. You can however install this beforehand (and I would suggest in an enterprise environment to do so) and therefore if done that way there should not be any downloading until after all sudo operations are complete, unless I've missed something.

@MikeMcQuaid
Copy link
Member

Passing on this, sorry @berikv! You may find the Homebrew .pkg installer helpful.

@MikeMcQuaid MikeMcQuaid closed this Aug 2, 2023
@MikeMcQuaid
Copy link
Member

Where can I find this pkg build? In the release assets there are only the sources: https://github.com/Homebrew/brew/releases

It's included in here now.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants