Bug 7672 - Administrator commands are not easily available via sudo
Summary: Administrator commands are not easily available via sudo
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Other (show other bugs)
Version: trunk
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.15.0
Assignee: Frida Flodin
URL:
Keywords: relnotes, samuel_tester, tobfa_tester
Depends on:
Blocks:
 
Reported: 2021-03-30 15:55 CEST by Pierre Ossman
Modified: 2022-03-08 15:43 CET (History)
2 users (show)

See Also:
Acceptance Criteria:


Attachments

Description Pierre Ossman cendio 2021-03-30 15:55:24 CEST
It is recommended to use sudo when running administrator commands, rather than switching to root for long periods of time. However with the current ThinLinc setup you don't get any help tab completion help when trying to run things through sudo.

The problem is that the administrator commands are in /opt/thinlinc/sbin rather than /opt/thinlinc/bin. Right now we only include the former in $PATH for the user root, so other users don't see those commands even if they have sudo access.

The same thing used to be a problem for all administrator commands, but distributions started putting sbin in $PATH since more and more are using sudo. Some administrator commands also invoke PolicyKit to get authorization, which also means that they should be in everyone's $PATH.
Comment 1 Pierre Ossman cendio 2021-03-30 15:58:28 CEST
Actually the situation is even worse:

> sudo tl-setup
> sudo: tl-setup: command not found

Fixing $PATH is insufficient here. We also need to get our sbin path in to sudo's "secure_path" setting.
Comment 2 Frida Flodin cendio 2022-02-09 08:55:23 CET
Unfortunately, adding our sbin path to "secure_path" seems to be more tricky than first thought. There is no easy way to append to "secure_path" programmatically. 

There is a "/etc/sudoers.d/" directory where we could put extra configuration that seemed promising. But looking more into this I found that this would result in a too hacky solution, that could have really bad consequences if made wrong. What I wanted to do was add a file "/etc/sudoers.d/thinlinc" with this content: 
> Defaults    secure_path += /opt/thinlinc/sbin

But this will not work at all, as also discovered by this questioner [1]. The reason is that "secure_path" is a string and the "+=" operator only works with lists. The result of doing this is that "secure_path" is silently overwritten with "/opt/thinlinc/sbin" and thus only commands in our sbin will work with sudo.

So if we want to modify "secure_path" we need to first find out what the value is before. This is where the hacky solution would come in. This does not feel like a tenable solution.


[1] https://access.redhat.com/solutions/1298644
Comment 3 Frida Flodin cendio 2022-02-09 09:19:55 CET
I tried to find how others solve this. Citrix's "solution" is documentation that points the user how to fix it themselves [1], see the note about how to run a command. This is not the most helpful note, if we decide to do something similar we might want to give more specific instructions with examples.

[1] https://docs.citrix.com/en-us/linux-virtual-delivery-agent/current-release/installation-overview/redhat.html#step-6-install-the-linux-vda
Comment 4 Frida Flodin cendio 2022-02-09 09:40:03 CET
After some discussion, we have decided to start by at least making it easier for admins to make this work:

1. Add "/opt/thinlinc/sbin" automatically to every user's PATH.
2. Add instructions in the documentation on how to make ThinLinc command work with sudo. With instructions on how to edit /etc/sudoers for example.

Extra:
Investigate the possibility to add a step in tl-setup where we check if the system is configured in a way that would allow ThinLinc commands to be run with sudo. If we find that it's not, we could refer to the new documentation.
Comment 15 Frida Flodin cendio 2022-02-15 17:04:43 CET
Everything should now be fixed and documented as best as we can. I have tested tl-setup with the new check for sudo access and made sure that the instructions in the documentation are valid. Tested on Ubuntu 20.04.

  ✓ tl-setup text mode
      ✓ Refer to TAG when sudo <tl command> does not work
      ✓ Do nothing if it does.
  ✓ tl-setup GUI mode
      ✓ Refer to TAG when sudo <tl command> does not work
      ✓ Do nothing if it does.
  ✓ tl-setup with answers file 
  ✓ The log looks good.
  ✓ Follow TAG instructions and ThinLinc commands work:
    sudo tl-config
    sudo tl-setup
  ✓ tab completion
    
Note: There are some different ways sudo can be allowed that tl-setup will notice:
 - Thinlinc paths in secure_path
 - No secure_path set at all, and the user has logged out and in again.

But tl-setup will not notice:
 - No secure_path set, the user has NOT logged out and in again.
 - if sudo is aliased with for example:
> alias sudo='sudo -E env "PATH=$PATH"' 


I chose not to mention that a sudoers file without secure_path set will use the
user's PATH as is. This means that you should not set secure_path and add /opt/thinlinc/sbin and bin to secure_path to make it work. If a user does this it will make ONLY thinlinc commands work with sudo, without the full path specified. This could be confusing for a non-experienced user. But should hopefully not happen in practice since all major distros set secure_path by default.
Comment 16 Pierre Ossman cendio 2022-02-24 09:40:15 CET
The message shown by tl-setup right now sounds worse than things actually are. It sounds more like commands are completely inaccessible using sudo, when the problem is that you have to specify a completely path to them.

We should probably have another look if we can make this clearer. A good target might be to have enough information that a sysadmin that knows about "secure_path" realises what to do, but a more inexperienced user will need to check the TAG for details.

(on a more subjective note, using ASCII art ("->") in a GUI looks very odd to me)
Comment 19 Samuel Mannehed cendio 2022-03-08 14:54:29 CET
We have verified that the changes work as advertised and couldn't find anything that was broken as a side effect.

We began by verifying the problem existed using a tl-4.14.0 server on RHEL 8:

* $PATH of regular user includes '/opt/thinlinc/bin' but not '/opt/thinlinc/sbin'
* `tl-config -a /vsmserver` works as a regular user
* `tl-config /vsmserver/admin_email=other@email.com` doesn't work as a regular user - "Failed to set parameter"
* `sudo tl-config /vsmserver/admin_email=other@email.com` doesn't work - "command not found..."

After upgrading to newest server build 2486:

* instructions in TAG are easy to follow and understand
* tl-setup in GUI mode
  - regular stuff works as it should
  - shows sudo-page when RHEL machine is in default conf
  - shows sudo-page when started with `sudo env PATH=$PATH tl-setup`
  - does not show sudo-page when thinlinc paths are added to secure_path as described in the documentation
  - does not show sudo-page when secure_path is removed from the configuration and the user has logged in and out
* tl-setup in text mode
  - regular stuff works as it should
  - shows sudo-text when RHEL machine is in default conf
  - shows sudo-text when started with `sudo env PATH=$PATH tl-setup`
  - does not show sudo-text when thinlinc paths are added to secure_path as described in the documentation
  - does not show sudo-text when secure_path is removed from the configuration and the user has logged in and out
* no errors in /var/log/tlsetup.log

Code review:

* The commits look good
* Perhaps some variables in sudo.py could have benefitted from more descriptive names, but the current code matches the style of the other tl-setup modules

Note You need to log in before you can comment on or make changes to this bug.