/var/run needs mode 777 in bionic

Bug #1761997 reported by Mark Shuttleworth on 2018-04-07
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
screen (Ubuntu)
High
Unassigned
ubuntu-release-upgrader (Ubuntu)
Undecided
Unassigned

Bug Description

[Test Case]
1) have an Ubuntu 16.04
2) rm /etc/cron.daily/mlocate (this'll ensure you get a conffile prompt)
3) ssh to Ubuntu 16.04 system so the release upgrade is run in screen
4) run do-release-upgrade -d
5) wait for the conffile prompt from mlocate
6) ssh to the Ubuntu 16.04 system being upgraded
7) sudo -i
8) run screen -rd

With the version of screen in the release pocket you'll receive the following error:

root@clean-xenial-amd64:~# screen -rd
Directory '/run/screen' must have mode 777.

[Original Description]
I saw some odd behaviour of screen during the upgrade from Xenial to Bionic using do-release-upgrade. I was trying to use screen to reattach to an upgrade process that had gone sideways during a router daemon upgrade (duh). But I was told that the permissions on /run/screen needed to be 777. Does the bionic version of screen have that as a requirement? And if so, perhaps do-release-upgrade should set those permissions in anticipation of the upgrade process so that screen works with both old and new versions.

Steve Langasek (vorlon) wrote :

I believe this is a bug in screen. The postinst script has code to handle the changed requirements for /run/screen permissions, but then immediately afterwards a debhelper code snippet runs which clobbers them again.

affects: ubuntu-release-upgrader (Ubuntu) → screen (Ubuntu)
Changed in screen (Ubuntu):
importance: Undecided → High
status: New → Triaged

Steve Langasek wrote:
> I believe this is a bug in screen. The postinst script has code to
> handle the changed requirements for /run/screen permissions, but then
> immediately afterwards a debhelper code snippet runs which clobbers them
> again.

I currently don't see that clobbering, at least not in Debian:

The maintainer-written code creates
/etc/tmpfiles.d/screen-cleanup.conf in most cases.

It also links /lib/systemd/system/screen-cleanup.service to /dev/null.

debhelper-generated code seems to create
/usr/lib/tmpfiles.d/screen-cleanup.conf — which is overridden by
/etc/tmpfiles.d/screen-cleanup.conf if it exists.

If it is not created because neither of the two conditions in the
maintainer code is given, I don't see how something which is not
created can be clobbered.

So please elaborate where you see the bug in screen's postinst.
Setting to "invalid" until then. Feel free to reassign or -- if you
can explain where the bug is -- reopen.

(Note: To me this rather looks like a not yet run postinst or similar
during a dist-upgrade.)

  Regards, Axel
--
 ,''`. | Axel Beckert <email address hidden>, https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
  `- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE

Changed in screen (Ubuntu):
status: Triaged → Invalid
Brian Murray (brian-murray) wrote :

I've been able to recreate this during a distribution upgrade from 16.04 to 18.04 (directory permissions switched from 775 to 777) when the package is unpacked but not yet configured. This could end up causing an issue at a conffile prompt for a different package if your screen session (which the release upgrader runs under) was disconnected.

Steve Langasek (vorlon) wrote :

On Wed, Apr 11, 2018 at 10:21:20AM -0000, Axel Beckert wrote:
> Steve Langasek wrote:
> > I believe this is a bug in screen. The postinst script has code to
> > handle the changed requirements for /run/screen permissions, but then
> > immediately afterwards a debhelper code snippet runs which clobbers them
> > again.

> I currently don't see that clobbering, at least not in Debian:

> The maintainer-written code creates
> /etc/tmpfiles.d/screen-cleanup.conf in most cases.

> It also links /lib/systemd/system/screen-cleanup.service to /dev/null.

> debhelper-generated code seems to create
> /usr/lib/tmpfiles.d/screen-cleanup.conf — which is overridden by
> /etc/tmpfiles.d/screen-cleanup.conf if it exists.

Brian has also not been able to reproduce this behavior, and I haven't
reproduced it by installing the package, only by directly running the
command from the postinst which I believe would be run as part of the
upgrade.

I do have /etc/tmpfiles.d/screen-cleanup.conf and it is correct. My
permissions for /run/screen on boot are correct. But when I ran that
command, /etc/tmpfiles.d/screen-cleanup.conf seemed to have been ignored.

Changed in screen (Ubuntu):
status: Invalid → Triaged
Changed in ubuntu-release-upgrader (Ubuntu):
milestone: none → ubuntu-18.04.1
Changed in screen (Ubuntu):
milestone: none → ubuntu-18.04.1
tags: added: id-5ad8bd0d1f6a45cc0c62874b
tags: added: id-5b05e6a18adf2325732c69ca
description: updated
Brian Murray (brian-murray) wrote :

One could also follow these simple steps to recreate the bug.

1) Start screen on Ubuntu 16.04
2) Download bionic's screen
3) Run 'sudo dpkg --unpack screen_4.6.2-1_amd64.deb'
4) try to reattach to screen

Axel Beckert (xtaran) wrote :

Hi Brian,

Brian Murray wrote:
> + 2) rm /etc/cron.daily/mlocate (this'll ensure you get a conffile prompt)
> + 3) ssh to Ubuntu 16.04 system so the release upgrade is run in screen
> + 4) run do-release-upgrade -d
> + 5) wait for the conffile prompt from mlocate
> + 6) ssh to the Ubuntu 16.04 system being upgraded
> + 7) sudo -i
> + 8) run screen -rd
> +
> + With the version of screen in the release pocket you'll receive the
> + following error:
> +
> + root@clean-xenial-amd64:~# screen -rd
> + Directory '/run/screen' must have mode 777.

Thanks for the detailed explanations.

This sounds a little bit like screen being already unpacked, but not
yet configured. But there must be more than that…

Can you check what permissions /run/screen actually has at that point?

What I don't understand is: Since screen is already running at that
point, /run/screen should already exist with the proper permissions
from either /etc/tmpfiles.d/screen-cleanup.conf or from
/etc/init.d/screen-cleanup from 16.04's screen package. And according
to the changelog (in Debian at least), nothing in the maintainer
scripts changed between 4.3.1-2 and 4.6.2-1. And git log shows only
two commits since June 2015 (where /etc/tmpfiles.d/ support was
finalized), of which one just renames the files and the other one
replaces /var/run/ with /run/ — which both should be defacto noop with
regards to their logic.

Does any other package modify /run/'s permissions recursively during
dist-upgrade?

  Regards, Axel
--
 ,''`. | Axel Beckert <email address hidden>, https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
  `- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE

Brian Murray (brian-murray) wrote :

/run/screen has the following permissions

drwxrwxr-x 3 root utmp 60 Jul 19 06:06 screen

screen-cleanup is a masked service in response to bug 1462692, so the permissions of /run/screen are never changed in Ubuntu 16.04.

bdmurray@clean-xenial-amd64:~$ sudo service screen-cleanup status
[sudo] password for bdmurray:
● screen-cleanup.service
   Loaded: masked (/dev/null; bad)
   Active: inactive (dead)
bdmurray@clean-xenial-amd64:~$ ls -lh /lib/systemd/system/screen-cleanup.service
lrwxrwxrwx 1 root root 9 Oct 4 2016 /lib/systemd/system/screen-cleanup.service -> /dev/null

Axel Beckert (xtaran) wrote :

Hi Brian,

Brian Murray wrote:
> /run/screen has the following permissions
>
> drwxrwxr-x 3 root utmp 60 Jul 19 06:06 screen

Thanks. This is indeed not the expected setting.

> screen-cleanup is a masked service in response to bug 1462692, so the
> permissions of /run/screen are never changed in Ubuntu 16.04.

Hrm, but 4.3.1-2build1 in 16.04 should also already have generated
/etc/tmpfiles.d/screen-cleanup.conf during postinst. (That was fixed
in the same upload as #1462692.)

And 16.04 already had systemd as default if not only init system.

> bdmurray@clean-xenial-amd64:~$ sudo service screen-cleanup status
> [sudo] password for bdmurray:
> ● screen-cleanup.service
> Loaded: masked (/dev/null; bad)
> Active: inactive (dead)
> bdmurray@clean-xenial-amd64:~$ ls -lh /lib/systemd/system/screen-cleanup.service
> lrwxrwxrwx 1 root root 9 Oct 4 2016 /lib/systemd/system/screen-cleanup.service -> /dev/null

Please also check the existence and contents of
/etc/tmpfiles.d/screen-cleanup.conf before and after upgrading.

  Regards, Axel
--
 ,''`. | Axel Beckert <email address hidden>, https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
  `- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE

Brian Murray (brian-murray) wrote :

/etc/tmpfiles.d/screen-cleanup.conf does not exist on an Ubuntu 16.04 install nor is it created when I install screen on a Ubuntu 16.04 system.

 $ schroot -u root -c xenial-amd64
(xenial-amd64)root@impulse:/mnt/sec-machines# apt-get install screen
Reading package lists... Done
Building dependency tree
Reading state information... Done
Suggested packages:
  iselect | screenie | byobu ncurses-term
The following NEW packages will be installed:
  screen
0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
Need to get 560 kB of archives.
After this operation, 972 kB of additional disk space will be used.
Get:1 http://192.168.10.7/ubuntu xenial/main amd64 screen amd64 4.3.1-2build1 [560 kB]
Fetched 560 kB in 0s (2131 kB/s)
Selecting previously unselected package screen.
(Reading database ... 23511 files and directories currently installed.)
Preparing to unpack .../screen_4.3.1-2build1_amd64.deb ...
Unpacking screen (4.3.1-2build1) ...
Processing triggers for systemd (229-4ubuntu21.2) ...
Processing triggers for man-db (2.7.5-1) ...
Setting up screen (4.3.1-2build1) ...
Processing triggers for systemd (229-4ubuntu21.2) ...
(xenial-amd64)root@impulse:/mnt/sec-machines# ls -lh /etc/tmpfiles.d/
total 0

Brian Murray (brian-murray) wrote :

Ah I think it (the tmpfile isn't created) because /usr/bin/screen is 2755 in Ubuntu 16.04 but 755 in Ubuntu 18.04. Here's the change between the two releases.

 override_dh_auto_install:
- # can't call the normal install target b/c it installs the info files
- # and other crud
- $(MAKE) prefix=$(ROOT)/usr SCREENENCODINGS='$$(prefix)/share/screen/utf8encodings' installdirs install_bin
- # hack around the fact that the install target makes screen a symlink to screen-$$(VERSION)
- rm -f $(ROOT)/usr/bin/screen
- mv -f $(ROOT)/usr/bin/screen* $(ROOT)/usr/bin/screen
- # make it setgid utmp
- chown root:utmp $(ROOT)/usr/bin/screen
- chmod 2755 $(ROOT)/usr/bin/screen
+ # can't call the normal install target b/c it installs the
+ # info files and other crud
+ cd build; $(MAKE) prefix=$(ROOT)/usr SCREENENCODINGS='$$(prefix)/share/screen/utf8encodings' installdirs install_bin
+ cd build-udeb; $(MAKE) prefix=$(ROOT_UDEB)/usr SCREENENCODINGS='$$(prefix)/share/screen/utf8encodings' installdirs install_bin
+ # hack around the fact that the install target makes screen a
+ # symlink to screen-$$(VERSION)
+ rm -f $(ROOT)/usr/bin/screen $(ROOT_UDEB)/usr/bin/screen
+ mv -f $(ROOT)/usr/bin/screen* $(ROOT)/usr/bin/screen
+ mv -f $(ROOT_UDEB)/usr/bin/screen* $(ROOT_UDEB)/usr/bin/screen
+ # make it setgid utmp only in udeb
+ chown root:utmp $(ROOT_UDEB)/usr/bin/screen
+ chmod 2755 $(ROOT_UDEB)/usr/bin/screen
+ chmod 755 $(ROOT)/usr/bin/screen

Axel Beckert (xtaran) wrote :

Hi,

Brian Murray wrote:
> Ah I think it (the tmpfile isn't created) because /usr/bin/screen is
> 2755 in Ubuntu 16.04 but 755 in Ubuntu 18.04. Here's the change between
> the two releases.

Ah! That's due to switching to use libutempter. We're getting quite
close to the issue.

I though wonder how to fix that for Bionic in the best way. Maybe
adding something like this to screen.preinst (not postinst):

perms="`stat -c%a /usr/bin/screen`"
override=/etc/tmpfiles.d/screen-cleanup.conf
if [ $perms -eq 2755 ]; then
    chmod 0777 /var/run/screen
[ -f $override ] || echo 'd /var/run/screen 0777 root utmp' > $override

I assume that this is nothing which would make sense to add to future
releases of Debian's screen package, or does it?

  Regards, Axel
--
 ,''`. | Axel Beckert <email address hidden>, https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
  `- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE

Alex - I don't think that will work because when the new version of screen is unpacked the permissions are 755, so the check would need to be for 755, not 2755.

(xenial-amd64)root@impulse:/mnt/sec-machines# dpkg --unpack screen_4.6.2-1_amd64.deb
(Reading database ... 23570 files and directories currently installed.)
Preparing to unpack screen_4.6.2-1_amd64.deb ...
Unpacking screen (4.6.2-1) over (4.3.1-2build1) ...
Processing triggers for systemd (229-4ubuntu21.2) ...
Processing triggers for man-db (2.7.5-1) ...
(xenial-amd64)root@impulse:/mnt/sec-machines# stat -c%a /usr/bin/screen
755

Axel Beckert (xtaran) wrote :

Hi Brian,

Brian Murray wrote:
> Alex - I don't think that will work because when the new version of
> screen is unpacked the permissions are 755, so the check would need to
> be for 755, not 2755.

No. I said into _pre_inst, not postinst, so that everything is in
place and fixed when screen has been unpacked, but not configured.

Or did I understand your previous mail about screen in 16.04 being
2755?

  Regards, Axel (not Alex)
--
 ,''`. | Axel Beckert <email address hidden>, https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
  `- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE

Brian Murray (brian-murray) wrote :

Axel (sorry about that),

I was confused about what order (installing the new binary vs running the preinst) things happened in. Checking the perms for 2755 in the preinst does work.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers