/var/run needs mode 777 in bionic

[Mark Shuttleworth]

[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)]

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.

[Axel Beckert (xtaran)]

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

[Brian Murray (brian-murray)]

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)]

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.

[Brian Murray (brian-murray)]

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)]

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)]

/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)]

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)]

/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)]

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)]

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

[Brian Murray (brian-murray)]

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)]

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)]

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.

[Łukasz Zemczak (sil2100)]

Hello Mark, or anyone else affected,

Accepted screen into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/screen/4.6.2-1ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

I went ahead and verified this by ssh'ing to an Ubuntu 16.04, starting the upgrade after having modified /etc/cron.daily/mlocate and changing DistUpgradeController.py so -proposed would not be disabled. During the upgrade process screen was unpacked, then I received the mlocate conf file prompt before screen was set up (configured). When I received the mlocate conf file prompt I was to disconnect and reconnect from the screen session.

Attached is the /var/log/dist-upgrade/apt-term.log from the system being upgraded.

[Adam Conrad (adconrad)]

The verification of the Stable Release Update for screen has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package screen - 4.6.2-1ubuntu1

---------------
screen (4.6.2-1ubuntu1) bionic; urgency=medium

  * debian/screen.preinst: When screen is unpacked but not configured the
    permissions for /var/run/screen need to be changed so that an existing
    screen session can be reconnected to e.g. during an distribution upgrade.
    (LP: #1761997)

 -- Brian Murray <email address hidden> Fri, 20 Jul 2018 06:57:59 -0700

[Mark Shuttleworth (sabdfl)]

\o/ thanks all, glad that one got in for the point release.

Mark

[Axel Beckert (xtaran)]

Thanks to Brian for his patience with the remote debugging session via Launchpad comments. And for the resulting HOWTO for reproducing the issue. :-)

Given that Stretch's screen package is without libutempter, too, this might bite Debian with Stretch to Buster upgrades inside screen, too. I'm though surprised that I didn't get any bug reports about this in Debian yet. Will check.

[Christophe R. Patraldo (mxurbano)]

$ sudo do-release-upgrade
Checking for a new Ubuntu release
Get:1 Upgrade tool signature [819 B]
Get:2 Upgrade tool [1,263 kB]
Fetched 1,264 kB in 0s (0 B/s)
authenticate 'bionic.tar.gz' against 'bionic.tar.gz.gpg'
extracting 'bionic.tar.gz'
Directory '/var/run/screen' must have mode 777.

$ screen
Directory '/var/run/screen' must have mode 777.

$ ls -lat /run/screen
total 0
drwxr-xr-x 29 root root 1260 Dec 8 01:42 ..
drwxrwxr-x 2 root utmp 40 Dec 6 20:36 .

$ sudo service screen-cleanup status
● screen-cleanup.service
   Loaded: masked (/dev/null; bad)
   Active: inactive (dead)

$ ls -lh /lib/systemd/system/screen-cleanup
ls: cannot access '/lib/systemd/system/screen-cleanup': No such file
or directory

$ cat /etc/tmpfiles.d/screen-cleanup.conf
cat: /etc/tmpfiles.d/screen-cleanup.conf: No such file or directory

[Christophe R. Patraldo (mxurbano)]

This still isn't working right, at least not for me. I'm trying to upgrade the release from 16.04 to 18.04 installed on Google Compute Engine. Is it a different bug or am I missing something?

[Brian Murray (brian-murray)]

On Sat, Dec 08, 2018 at 07:38:38PM -0000, Christophe R. Patraldo wrote:
> This still isn't working right, at least not for me. I'm trying to
> upgrade the release from 16.04 to 18.04 installed on Google Compute
> Engine. Is it a different bug or am I missing something?

The fixed version of screen is in the -updates pocket for Ubuntu 18.04.
Do you have -updates enabled for Ubuntu 16.04?

--
Brian Murray