Skip to content
This repository has been archived by the owner on May 23, 2020. It is now read-only.

Use a more reliable command to check passwordless sudo #36

Closed
wants to merge 1 commit into from
Closed

Use a more reliable command to check passwordless sudo #36

wants to merge 1 commit into from

Conversation

rex
Copy link

@rex rex commented Aug 10, 2018

No description provided.

@rex
Copy link
Author

rex commented Aug 10, 2018

This PR references issue Linuxbrew/brew#764 and Linuxbrew/brew#574

@sjackman
Copy link
Member

Hi, Rex. Thanks for the PR! Before merging it, I'd like to better understand your configuration of sudo. Does your user have sudo access? Does it have sudo access to all commands, or just some? Does it have passwordless sudo access?

@sjackman sjackman self-assigned this Aug 10, 2018
@rex
Copy link
Author

rex commented Aug 10, 2018

Hey @sjackman! I was wondering what you meant by your comment in the issue thread, now I understand. The user I have configured on our servers is named deploy and has the following entry in /etc/sudoers:

deploy ALL=(ALL) NOPASSWD:ALL

In addition, this is their entry in /etc/passwd:

deploy:x:1001:1002:,,,:/home/deploy:/bin/bash

In the interest of completeness, this is the exact set of commands used to create the user:

ci_user="deploy"
ci_home="/home/${ci_user}"
ssh_dir="${ci_home}/.ssh"
getent group docker || sudo groupadd docker
id -u ${ci_user} &>/dev/null || sudo adduser --disabled-password ${ci_user}

sudo usermod -aG sudo ${ci_user}
sudo usermod -aG docker ${ci_user}

echo "${ci_user} ALL=(ALL) NOPASSWD:ALL" | sudo EDITOR='tee -a' visudo

@sjackman
Copy link
Member

What's the undesirable behaviour that you experience before this PR, and how does this PR fix it?

@rex
Copy link
Author

rex commented Aug 11, 2018

As I outlined in Linuxbrew/brew#764 I simply cannot install Linuxbrew with a user that has passwordless sudo. The installer performs the sudo? check which executes a command that requires a password for the executing user. This PR updates that command as recommended in Linuxbrew/brew#574 to one that gets the same information but without requiring the user's password.

@sjackman
Copy link
Member

You've configured sudo to be NOPASSWD, yet it's still asking you for a password, which indicates to me that sudo is somehow misconfigured, since it's asking for a password. Let's address that issue first.

@rex
Copy link
Author

rex commented Aug 13, 2018

Fair enough @sjackman, I hadn't really looked through that yet. I went ahead and checked it out and (as you already knew) it does seem like there might be a misconfiguration here. I searched StackOverflow and the consensus seems to be that if your passwordless sudo user is matched by multiple declarations in the /etc/sudoers file then you can have problems.

I logged in as the deploy user (my passwordless sudo user) and ran sudo -l to determine what rules were being applied and got this output:

deploy@{HOST}$ sudo -l
Matching Defaults entries for deploy on {HOST}:
    env_reset, mail_badpass, secure_path=/usr/local/sbin\:/usr/local/bin\:/usr/sbin\:/usr/bin\:/sbin\:/bin\:/snap/bin

User deploy may run the following commands on {HOST}:
    (ALL : ALL) ALL
    (ALL) NOPASSWD: ALL

So it seems some commands (like sudo -v, for example) might be matching the first entry for (ALL : ALL) ALL but other commands (like sudo -ln mkdir) might be matching the second line. The first line is there because the deploy user is part of the sudo group, and the second line specifically adds "passwordless sudo" for the deploy user. I am not sure what would cause the difference in behavior, and I am less sure why the first line is being touched since from all the SO answers I've found it seems that whatever entry is found last in the /etc/sudoers file takes precedence over the rest.

I'm taking a deeper look into this to see if I can resolve this the right way as you suggested. My suspicion is that if the hypothesis above is true, simply removing the deploy user from the sudo group might be sufficient to resolve the issue.


Update: WELL WHADDYA FREAKIN KNOW. @sjackman I suppose you already knew this but I will give you the satisfaction of hearing (reading?) it anyway: You were right and I was wrong 🥇 😆

It turns out that the hunch above was correct. Removing the deploy user from the sudo group was the key that resolved the problem.

deploy@pierce:/root$ sudo -v
[sudo] password for deploy:
deploy@pierce:/root$ sudo -ln mkdir
/bin/mkdir
deploy@pierce:/root$ exit
exit
root@pierce:~# deluser deploy sudo
Removing user `deploy' from group `sudo' ...
Done.
root@pierce:~# su deploy
deploy@pierce:/root$ sudo -v
deploy@pierce:/root$ echo $?
0
deploy@pierce:/root$ sudo -ln mkdir
/bin/mkdir

I am going to attempt to install linuxbrew again with this user now that the issue appears to be resolved, and we can go from there.

@rex
Copy link
Author

rex commented Aug 13, 2018

Yep, running the installer again worked like a champ and brew doctor reports all systems go.

Thanks for your guidance @sjackman

@rex rex closed this Aug 13, 2018
@iMichka
Copy link
Member

iMichka commented Aug 13, 2018

Cool. Maybe there could be a way in our installer to check for that kind of case and prevent this from happening?

@rex
Copy link
Author

rex commented Aug 13, 2018

It seems like there's an opportunity here for a developer experience improvement. My guess is that many people expect this to just work provided the user has sufficient permissions. Tangling with the particulars of sudo configuration can be a deterrent to adoption. One idea could be to support two modes of installation:

  1. User-level installation, living at ~/.linuxbrew, with the user required to add ~/.linuxbrew/bin to their $PATH but not requiring any enhanced permissions at all
  2. System-level installation, living in /usr/local/bin, with the user requiring enhanced permissions

Seems like that would be a little cleaner in terms of expectations/permissions. Just my $0.02.

@sjackman
Copy link
Member

I'm glad that you were able to sort this out!

It sort of does behave like that, except that the system-level installation is /home/linuxbrew/.linuxbrew rather than /usr/local. There's a few reasons for that, but a big one is that /usr/local/lib may be in the default library search path, and Linuxbrew installing stuff in there could potentially break your whole system.

When the installer determines that the user has sudo permissions, it installs by default to /home/linuxbrew/.linuxbrew, and if the user does not have sudo permissions, it installs to their home directory ~/.linuxbrew.

@sjackman
Copy link
Member

Here's some stock text that I post when explaining why we recommend installing in /home/linuxbrew/.linuxbrew/:

Consider installing Linuxbrew in /home/linuxbrew/.linuxbrew/ if possible so that you can use precompiled binary packages (known as bottles) for non-relocatable formula like util-linux.
Another possible workaround for you is brew install --force-bottle util-linux, but no promises.

If it's an option for you, you could open a ticket with your information systems department to ask that they create a linuxbrew role account with home directory /home/linuxbrew.

The precompiled binary bottles of non-relocatable bottles can only be used if you install in /home/linuxbrew/.linuxbrew, otherwise they have to be built from source. See the documentation below. On macOS the default installation directory is /usr/local. On Linux the default installation directory is /home/linuxbrew/.linuxbrew.

@waldyrious
Copy link

Would it make sense to include this information somewhere more visible for Linuxbrew users?

This section of the FAQ would be the most semantically adequate location, but it would probably be more visible (and easier to keep in sync with upstream, I suppose) to have a new section under the #Bottles section of the home page, which would be immediately visible here. Thoughts?

@rex
Copy link
Author

rex commented Aug 14, 2018

@sjackman Thank you for that explanation, that is much clearer now. I like that approach and agree completely with their reasoning. So I suppose it does come down to what @iMichka noted above: that there could be a way to better detect whether the user has sudo access (and by "better" I suppose I mean "less intrusive"). Conversely, it could be in the user's best interest to do a hard check in this way to ensure they really do have access to sudo and, if not, push them to resolve their configuration issues.

@waldyrious Personally my vote is for the FAQ section.

@sjackman
Copy link
Member

sjackman commented Aug 14, 2018

Linuxbrew/brew is being merged into Homebrew/brew, so we're avoiding making any additional changes to Linuxbrew/brew. Here's the FAQ for Linuxbrew: https://github.com/Linuxbrew/brew/wiki/FAQ#why-install-in-homelinuxbrewlinuxbrew
I agree that this FAQ could be more visible.

@maxim-belkin
Copy link

this is /install, not /brew, so it could have some differences.

@rex
Copy link
Author

rex commented Aug 14, 2018

@sjackman That's interesting news that I wasn't expecting. Makes sense, though. Would it basically just be installing Homebrew onto a $DISTRO linux box and doing everything Linuxbrew does now? So not just specifically for Mac anymore?

@sjackman
Copy link
Member

Yep! Follow the discussion over at Linuxbrew/brew#612

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants