Discussion:
Bug#786555: sudo: time stamp files no longer invalidated at boot
Add Reply
Marc Deslauriers
2015-06-05 15:40:01 UTC
Reply
Permalink
Raw Message
Package: sudo
Version: 1.8.12-1
Followup-For: Bug #786555
User: ubuntu-***@lists.ubuntu.com
Usertags: origin-ubuntu wily ubuntu-patch



*** /tmp/tmp8y8IwQ/bug_body

In Ubuntu, the attached patch was applied to achieve the following:

* Use tmpfs location to store timestamp files (LP: #1458031)
- debian/rules: change --with-rundir to /var/run/sudo
- debian/rules, debian/sudo.service, debian/sudo.sudo.init: stop
shipping init script and service file, as they are no longer
necessary.
- debian/*.preinst, debian/*.postinst, debian/*.postrm: remove old init
script with dpkg-maintscript-helper.
- debian/*.postinst: remove old /var/run/sudo to /var/lib/sudo
transition code, remove old /var/lib/sudo/ts timestamp directory.


Thanks for considering the patch.


-- System Information:
Debian Release: jessie/sid
APT prefers vivid-updates
APT policy: (500, 'vivid-updates'), (500, 'vivid-security'), (500, 'vivid-proposed'), (500, 'vivid'), (100, 'vivid-backports')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.19.0-20-generic (SMP w/4 CPU cores)
Locale: LANG=en_CA.UTF-8, LC_CTYPE=en_CA.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
Michael Biebl
2017-07-16 23:20:02 UTC
Reply
Permalink
Raw Message
On Fri, 05 Jun 2015 11:35:57 -0400 Marc Deslauriers
Post by Marc Deslauriers
Package: sudo
Version: 1.8.12-1
Followup-For: Bug #786555
Usertags: origin-ubuntu wily ubuntu-patch
*** /tmp/tmp8y8IwQ/bug_body
* Use tmpfs location to store timestamp files (LP: #1458031)
- debian/rules: change --with-rundir to /var/run/sudo
- debian/rules, debian/sudo.service, debian/sudo.sudo.init: stop
shipping init script and service file, as they are no longer
necessary.
- debian/*.preinst, debian/*.postinst, debian/*.postrm: remove old init
script with dpkg-maintscript-helper.
- debian/*.postinst: remove old /var/run/sudo to /var/lib/sudo
transition code, remove old /var/lib/sudo/ts timestamp directory.
It seems, sudo was originally using /var/run/sudo, but switched to
/var/lib/sudo because of #581393.
Moving back everything to /var/run/sudo will reopen this issue, i.e. the
users will get the lecture again upon reboots.

Imo, the best solution would be, if the "lectured" dir was on /var/lib
and the timestamp "ts" dir on /run. This way no init script / service
would be needed on reboots to clean up the timestamps and the lectured
messages would be preserved.
Building with
--with-rundir=/run/sudo \
--with-vardir=/var/lib/sudo \
seems to achieve that.

sudo creates /run/sudo/ts on demand, but it fails to apply the correct
selinux context. So if you care about selinux we should keep the
sysvinit script to run mkdir and restorecon. For systemd, a tmpfile
snippet could achieve that.

Bdale, would you be ok with that change? If so, I could prepare a patch
depending on whether you want to support selinux or not.

Regards,
Michael
--
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
Bdale Garbee
2017-07-17 00:00:01 UTC
Reply
Permalink
Raw Message
Post by Michael Biebl
Building with
--with-rundir=/run/sudo \
--with-vardir=/var/lib/sudo \
seems to achieve that.
Oh, neat!
Post by Michael Biebl
Bdale, would you be ok with that change? If so, I could prepare a patch
depending on whether you want to support selinux or not.
Sure, sounds good. I'm personally ambivalent about selinux since I
don't use it, but I'm always in favor of making things work for as many
users in as many contexts as possible.

Bdale
Michael Biebl
2017-07-17 00:10:03 UTC
Reply
Permalink
Raw Message
Post by Bdale Garbee
Sure, sounds good. I'm personally ambivalent about selinux since I
don't use it, but I'm always in favor of making things work for as many
users in as many contexts as possible.
bigon, is there a way sudo could do the selinux relabeling itself when
creating /run/sudo/ts so we don't need an init script/tmpfile to do
that? That would be a more elegant solution. I see sudo does already
link againt libselinux. So having that dependency is not a concern.
--
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
Laurent Bigonville
2017-07-17 09:20:02 UTC
Reply
Permalink
Raw Message
Post by Michael Biebl
Post by Bdale Garbee
Sure, sounds good. I'm personally ambivalent about selinux since I
don't use it, but I'm always in favor of making things work for as many
users in as many contexts as possible.
bigon, is there a way sudo could do the selinux relabeling itself when
creating /run/sudo/ts so we don't need an init script/tmpfile to do
that? That would be a more elegant solution. I see sudo does already
link againt libselinux. So having that dependency is not a concern.
There are multiple ways of doing that:

1. via policy: this should be already the case for confined users, but
apparently not for unconfined ones. This might be a bug in the
policy, I'll poke upstream about that.
2. via an explicit call to libselinux: call selabel_lookup_raw() to
retrieve which context should be used and then call
setfscreatecon_raw() so the next file create will atomically have
the correct context. sudo is already selinux-aware so that could be
a solution as well.
3. Pre-create the directory during the startup with the correct context.

The easiest being of course 3)
Laurent Bigonville
2017-07-17 10:40:02 UTC
Reply
Permalink
Raw Message
Post by Laurent Bigonville
Post by Michael Biebl
Post by Bdale Garbee
Sure, sounds good. I'm personally ambivalent about selinux since I
don't use it, but I'm always in favor of making things work for as many
users in as many contexts as possible.
bigon, is there a way sudo could do the selinux relabeling itself when
creating /run/sudo/ts so we don't need an init script/tmpfile to do
that? That would be a more elegant solution. I see sudo does already
link againt libselinux. So having that dependency is not a concern.
1. via policy: this should be already the case for confined users,
but apparently not for unconfined ones. This might be a bug in the
policy, I'll poke upstream about that.
2. via an explicit call to libselinux: call selabel_lookup_raw() to
retrieve which context should be used and then call
setfscreatecon_raw() so the next file create will atomically have
the correct context. sudo is already selinux-aware so that could
be a solution as well.
3. Pre-create the directory during the startup with the correct context.
The easiest being of course 3)
Apparently the sudo tarball already contains a tmpfile snippet
(init.d/sudo.conf.in) that is already shipped in the package. So that
solves the systemd case.

For the !systemd case, the only thing that needs to be added in the
initscript to replicate the tmpfiles snippet is something like:

mkdir /run/sudo /run/sudo/ts
chmod 0711 /run/sudo
chmod 0700 /run/sudo/ts
[ -x /sbin/restorecon ] && /sbin/restorecon /run/sudo /run/sudo/ts

I wouldn't recursively relabel the files in there to avoid potentially
breaking things for confined users.

Loading...