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

execute: RuntimeDirectory= or friends always creates unnecessary mount namespace #7761

Closed
mbiebl opened this issue Dec 29, 2017 · 16 comments
Closed
Labels
bug 🐛 Programming errors, that need preferential fixing pid1

Comments

@mbiebl
Copy link
Contributor

mbiebl commented Dec 29, 2017

Submission type

  • Bug report

systemd version the issue has been seen with

v236

Used distribution

Debian sid
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=885325

In case of bug report: Expected behaviour you didn't see

A mount point that is mounted manually from a SSH session is visible to systemd service

In case of bug report: Unexpected behaviour you saw

login via SSH, mount a partition. That mount point is only visible withing the SSH session but not to systemd services.

In case of bug report: Steps to reproduce the problem

For testing purposes I created the following service:

[Unit]
Description=List mount points

[Service]
Type=oneshot
ExecStart=/bin/findmnt

Then I logged in locally on tty1, attached an USB disk, mounted it via mount /dev/sdb1 /mnt/disk, then ran systemctl start test.service. Checking the journal, I get:

...
Dez 29 10:27:32 pluto findmnt[1532]: ├─/mnt/disk                           /dev/
Dez 29 10:27:32 pluto systemd[1]: Started List mount points.
Dez 29 10:27:32 pluto findmnt[1532]: ├─/tmp                                tmpfs
Dez 29 10:27:32 pluto findmnt[1532]: ├─/home                               /dev/
Dez 29 10:27:32 pluto findmnt[1532]: ├─/apps                               /dev/
Dez 29 10:27:32 pluto findmnt[1532]: └─/mnt/data 

Then I started a local SSH service, logged in as root via SSH ( ssh root@localhost) , mounted the partition again, started the test service again. This time I get:

...
Dez 29 10:28:28 pluto systemd[1]: Started List mount points.
Dez 29 10:28:28 pluto findmnt[1640]: ├─/tmp                                tmpfs
Dez 29 10:28:28 pluto findmnt[1640]: ├─/home                               /dev/
Dez 29 10:28:28 pluto findmnt[1640]: ├─/apps                               /dev/
Dez 29 10:28:28 pluto findmnt[1640]: └─/mnt/data                           /dev/

As can be seen the /mnt/disk mount point is not visible to test.service

This appears to be a regression in v236 as reported by the downstream bug report.

@mbiebl mbiebl added bug 🐛 Programming errors, that need preferential fixing regression ⚠️ A bug in something that used to work correctly and broke through some recent commit labels Dec 29, 2017
@mbiebl
Copy link
Contributor Author

mbiebl commented Dec 29, 2017

When logging in via SSH, the mount point is shown by lsns -t mnt as

4026532306 mnt       5  4647 root             sshd: root@pts/1    

@mbiebl
Copy link
Contributor Author

mbiebl commented Dec 29, 2017

I wonder if this is related to 652bb26
The ssh.service in Debian uses

RuntimeDirectory=sshd
RuntimeDirectoryMode=0755

@mbiebl
Copy link
Contributor Author

mbiebl commented Dec 29, 2017

/cc @yuwata

@yuwata
Copy link
Member

yuwata commented Dec 30, 2017

@mbiebl 652bb26 is just a follow-up, mainly clean-up, for #6940. A tentative workaround for this is that ssh.service stop to use RuntimeDirectory=.

I will try to find a fix for this.

yuwata added a commit to yuwata/systemd that referenced this issue Dec 30, 2017
… DynamicUser=no

Only when DynamicUser=yes, RuntimeDirectory= or their friends imply
BindPaths=.

Fixes systemd#7761.
yuwata added a commit to yuwata/systemd that referenced this issue Dec 30, 2017
…namicUser=no

The commit 6c47cd7 make RuntimeDirectory=
or friends imply BindPaths=. But this is for the directories works
well when DynamicUser= is set. So, it is not necessary to imply
BindPaths= when DynamicUser= is not set.
This removes the implication when DynamicUser=no.

Fixes systemd#7761.
@yuwata
Copy link
Member

yuwata commented Dec 30, 2017

Hmm. I thought this is caused by that RuntimeDirectory= implies BindPaths=, and under the assumption I made PR #7763.
However, I cannot reproduce this. On Fedora 27 x86_64 with the following ssh service unit file:

$ systemctl cat sshd.service
# /usr/lib/systemd/system/sshd.service
[Unit]
Description=OpenSSH server daemon
Documentation=man:sshd(8) man:sshd_config(5)
After=network.target sshd-keygen.target
Wants=sshd-keygen.target

[Service]
Type=notify
EnvironmentFile=-/etc/crypto-policies/back-ends/openssh-server.config
EnvironmentFile=-/etc/sysconfig/sshd
ExecStart=/usr/sbin/sshd -D $OPTIONS $CRYPTO_POLICY
ExecReload=/bin/kill -HUP $MAINPID
KillMode=process
Restart=on-failure
RestartSec=42s

[Install]
WantedBy=multi-user.target

# /etc/systemd/system/sshd.service.d/override.conf
[Service]
RuntimeDirectory=foo

I mount a USB memory on /mnt. However, the entry of /mnt in the output of findmnt is equivalent.

$ findmnt /mnt
TARGET SOURCE    FSTYPE OPTIONS
/mnt   /dev/sdc1 vfat   rw,relatime,fmask=0022,dmask=0022,codepage=437,iocharset=ascii,shortname=mixed,errors=remount-ro
$ ssh localhost findmnt /mnt
TARGET SOURCE    FSTYPE OPTIONS
/mnt   /dev/sdc1 vfat   rw,relatime,fmask=0022,dmask=0022,codepage=437,iocharset=ascii,shortname=mixed,errors=remount-ro

Also, the output of lsns -t mnt seems different to what @mbiebl reported.

$ lsns -t mnt
        NS TYPE NPROCS   PID USER     COMMAND
4026531840 mnt      45   735 watanabe /usr/lib/systemd/systemd --user
$ ssh localhost lsns -t mnt
        NS TYPE NPROCS   PID USER     COMMAND
4026531840 mnt      45   735 watanabe /usr/lib/systemd/systemd --user
4026532395 mnt       1 22624 watanabe lsns -t mnt

@mbiebl Could you show the ssh.service file entirely?

@yuwata
Copy link
Member

yuwata commented Dec 30, 2017

Ah, sorry, I misread this. To reproduce this, the device needs to be mounted from ssh session.

@shawnl
Copy link
Contributor

shawnl commented Dec 30, 2017

I was thinking this might be a re-occurring problem with units, and there should be a NoNamespace=mount option (also NoNamespace=all or user or net,mount et cetera) that makes anything that depends on mount namespaces to fail. In the case of RuntimeDirectory= it might even use differn't code paths, "enumerate through /run whenever we assign a new dynamic UID to see if there's something owned by that UID, but that'd be kinda slow and ugly."-Lennart, the way that was described.

@yuwata yuwata removed the has-pr label Dec 30, 2017
@yuwata
Copy link
Member

yuwata commented Dec 30, 2017

@shawnl Indeed.

@yuwata
Copy link
Member

yuwata commented Dec 30, 2017

@shawnl I will try to implement this. Does anyone already tackle this?

yuwata added a commit to yuwata/systemd that referenced this issue Jan 3, 2018
RuntimeDirectory= or friend imply ReadWritePaths= or BindPaths=,
and these create new mount namespace.
When NoNewNamespace=yes or `mnt` is set, then RuntimeDirectory=
without DynamicUser= does not imply ReadWritePaths=, then does
not introduce new mount namespace.

Fixes systemd#7761.
@yuwata
Copy link
Member

yuwata commented Jan 3, 2018

I've updated #7763, but this may be not right solution to fix this. Moreover, now I do not think this is a bug anymore. RuntimeDirectory= implies ReadWritePaths= or BindPaths=, thus the unit which has RuntimeDirectory= run in a new mount namespace. If you want to create runtime directories without namespace sandboxing, that is, if you just want to create the directories, then I recommend to use tmpfiles. For ssh.service, RuntimeDirectory= is not suitable option, I think.

@yuwata yuwata added RFE 🎁 Request for Enhancement, i.e. a feature request and removed bug 🐛 Programming errors, that need preferential fixing regression ⚠️ A bug in something that used to work correctly and broke through some recent commit labels Jan 3, 2018
@yuwata yuwata changed the title mount points mounted in SSH session are not visible to systemd services RFE: RuntimeDirectory= or friends without mount namespace sandbox Jan 3, 2018
@yuwata yuwata added the has-pr label Jan 3, 2018
@mbiebl
Copy link
Contributor Author

mbiebl commented Jan 3, 2018

If you want to create runtime directories without namespace sandboxing, that is, if you just want to create the directories, then I recommend to use tmpfiles. For ssh.service, RuntimeDirectory= is not suitable option, I think.

Hm, this worked with v235 though and we were promoting the usage of RuntimeDirectory over tmpfiles. What exactly changed in v236 that RuntimeDirectory now needs a new mount namespace?

@shawnl
Copy link
Contributor

shawnl commented Jan 3, 2018

What exactly changed in v236 that RuntimeDirectory now needs a new mount namespace?

DynamicUser= needs it to be secure from uid re-use. This remains a bug in systemd and a regression.

@mbiebl
Copy link
Contributor Author

mbiebl commented Jan 3, 2018

DynamicUser= needs it to be secure from uid re-use. This remains a bug in systemd and a regression.

I think so as well, i.e that RuntimeDirectory= without DynamicUser= should continue to work as in v235.
Not sure why @yuwata removed the bug and regression tag.

@yuwata yuwata added bug 🐛 Programming errors, that need preferential fixing and removed RFE 🎁 Request for Enhancement, i.e. a feature request labels Jan 3, 2018
@yuwata
Copy link
Member

yuwata commented Jan 3, 2018

Ah, now I understand why 652bb26 causes this bug.
Before the commit, man page said that RuntimeDirectory= implies ReadWritePaths=, but in fact it was implied only when DynamicUser=yes is set. So, RuntimeDirectory= just creates directory when DynamicUser= is disabled.
But the commit 652bb26 makes RuntimeDirectory= always implies BindPaths= as man page said, and thus, it always requires new mount namespace.

OK. this is actually a bug, now I think. Sorry. I will try to fix it.

@yuwata yuwata removed the has-pr label Jan 3, 2018
@yuwata yuwata changed the title RFE: RuntimeDirectory= or friends without mount namespace sandbox execute: RuntimeDirectory= or friends always creates unnecessary mount namespace Jan 3, 2018
yuwata added a commit to yuwata/systemd that referenced this issue Jan 3, 2018
@yuwata
Copy link
Member

yuwata commented Jan 3, 2018

I've updated #7763. As mentioned by @mbiebl, the PR reverts 652bb26. I hope it fixes this issue. Please take a look.

@ghost
Copy link

ghost commented Jan 10, 2018

I had same issue, took a while to understand why luksOpen and mount does not appear in transmission and samba.
workaround- execute mount from ssh session with systemd-run

systemd-run mount -v /mnt/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Programming errors, that need preferential fixing pid1
Development

No branches or pull requests

3 participants