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

qrexec-agent does not correctly set $PATH variable for root user who has fish as default shell #3434

Closed
najamelan opened this issue Dec 29, 2017 · 8 comments
Labels
C: core P: minor Priority: minor. The lowest priority, below "default." T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.

Comments

@najamelan
Copy link

najamelan commented Dec 29, 2017

Qubes OS version:

R4.0

Affected TemplateVMs:

sys-net

Steps to reproduce the behavior:

qvm-run -p -u root sys-net 'env'

Expected behavior:

PATH should include /sbin and /usr/sbin for root.

Actual behavior:

PATH doesn't have /sbin and /usr/sbin

General notes:

The security impact should be minor. This does keep privileged processes from working correctly though.
The problem breaks functionality like SuspendPre because prepare-suspend uses lsmod and other commands that are in /sbin.

Solution

I'm not sure how this is best resolved. I'm not sure if the best option is to have an environment identical to when one logs into a interactive shell, because that implies parsing shell specific files like .bashrc or config.fish, ...

A consideration to make is that the user might have different shells installed, and so remote commands should never assume bash or sh, or should invoke the correct shell explicitly. I have not looked through qubes rpc code, but I just figured that is a possible source of bugs for people who use different shells. Maybe that could be documented in the qrexec documentation.

Workaround

For fixing suspend for the wifi card with a quick hack, one can add:
PATH=/sbin:/usr/sbin:$PATH at the beginning of:
/usr/lib/qubes/prepare-suspend in the template for sys-net.

Related issues:

qrexec-agent do not setup process environment correctly - #3416
wireless does not automatically reconnect on resume on R4-rc1 - #3151

@andrewdavidwong andrewdavidwong added T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. C: core labels Dec 30, 2017
@andrewdavidwong andrewdavidwong added this to the Release 4.0 milestone Dec 30, 2017
@marmarek
Copy link
Member

What template you use there (by default it's fedora-25, have you changed that?)?
Also, what version of qubes-core-agent package do you have (check with rpm -q qubes-core-agent, or dpkg -l qubes-core-agent depending on distribution)?
I've just tried on fedora-26 and debian-9, both with qubes-core-agent 4.0.15 and $PATH is set correctly (contains /usr/sbin and on Debian additionally /sbin).

@najamelan
Copy link
Author

hi,
qubes-core-agent-4.0.15-1.fc26.x86_64

I made a new sys-net hoping I wouldn't get the "mac address already in use" because I have to restart my computer every time I want to restart sys-net, but in this new one this problem is still the same.

What I did:

  • install the fedora-26 template from the qubes repository
  • upgrade it from the testing repository, hence the core-agent version.

I installed fedora 26 because I like to keep things up to date and because it was available from qubes, I thought I might at least try it.

Ok, I tested with a stock fedora-25. The path is correct. 3 differences with the other one:

  • it's 25
  • it's not upgraded from testing
  • root user does not have fish as it's default shell.

I will narrow it down.

@najamelan
Copy link
Author

Ok,

after setting fish shell to the root user default shell the path no longer has /usr/sbin.

I wonder whether this is a problem on the fish end or the qubes end, but in any case when logging in interactively on the terminal, sbin is in the path, but when called through qvm-run it isn't.

@najamelan najamelan changed the title qrexec-agent does not correctly set $PATH variable for root user qrexec-agent does not correctly set $PATH variable for root user who has fish as default shell Dec 31, 2017
@marmarek
Copy link
Member

Yes, it may be a problem with fish. We expect that a login shell load appropriate startup scripts and setup environment. This especially include /etc/profile. At least on Fedora, /etc/profile contains a code to do add /usr/sbin to PATH. According to documentation fish should do that too...

@najamelan
Copy link
Author

najamelan commented Dec 31, 2017 via email

@andrewdavidwong andrewdavidwong added the P: minor Priority: minor. The lowest priority, below "default." label Jan 1, 2018
@najamelan
Copy link
Author

Resetting my default shell to /bin/bash has also magically fixed other things:

  • I can now restart sys-net without rebooting qubes
  • clocksync works

so yeah, documenting this might avoid some future problems for other users.

@najamelan
Copy link
Author

btw I would also propose to take it off the 4.0 milestone

@marmarek marmarek modified the milestones: Release 4.0, Far in the future Jan 1, 2018
@unman
Copy link
Member

unman commented Jul 18, 2019

[user@dom0 ~] qvm-run -p -u root fish 'env'
SHELL=/usr/bin/fish
QREXEC_AGENT_PID=925
PWD=/root
LOGNAME=root
XDG_SESSION_TYPE=unspecified
HOME=/root
LANG=en_US.UTF-8
QREXEC_SERVICE_FULL_NAME=qubes.VMShell
XDG_SESSION_CLASS=background
QREXEC_REMOTE_DOMAIN=dom0
USER=root
DISPLAY=:0
SHLVL=1
XDG_SESSION_ID=c6
XDG_RUNTIME_DIR=/run/user/0
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/0/bus
QT_X11_NO_MITSHM=1
_=/usr/bin/env

Tested with Debian (shown above), and Fedora(29/30) based templates in Qubes 4.0.
I believe issue is now fixed.
Path correctly set when fish is default shell.

@andrewdavidwong Please close as Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: core P: minor Priority: minor. The lowest priority, below "default." T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
Development

No branches or pull requests

4 participants