Tails

#1 Updated by segfault 2018-03-16 20:02:07

• related to Bug #15407: Prevent system user uid:s and gid:s from changing between releases added

#2 Updated by segfault 2018-03-16 20:02:16

• related to Bug #15419: Detect earlier in the dev process if we're breaking automatic upgrades added

#4 Updated by segfault 2018-03-16 20:06:15

• blocked by Bug #15422: 04-change-gids-and-uids doesn't change UIDs of existing files added

#5 Updated by intrigeri 2018-03-16 20:28:08

• Status changed from Confirmed to In Progress
• Target version set to Tails_3.6.1
• Feature Branch set to segfault:bug/15424-check-if-tor-uid-changed

#6 Updated by intrigeri 2018-03-16 20:51:02

• Assignee changed from segfault to intrigeri
• Target version changed from Tails_3.6.1 to Tails_3.7
• % Done changed from 0 to 20
• QA Check set to Ready for QA
• Feature Branch changed from segfault:bug/15424-check-if-tor-uid-changed to bug/15424-check-if-tor-uid-changed

Improved a bit, segfault says my commit looks good. But actually it’s a no-op because on 3.6.1 debian-tor will get the same UID/GID as in 3.6, and we’ve decided to support that automatic upgrade patch => I’ll test and merge into stable but only once 3.6.1 is out.

#7 Updated by intrigeri 2018-03-22 08:32:29

• % Done changed from 20 to 30

Merged current stable, added two improvements on top: commit:649eeafd34ed99b808ee7e4aa641a2a355711910, commit:7d8f7feb612368d008d420606dd2473f44d416ab. I’ll now try to test this:

• in a scenario where the added code should be a no-op: currently it’s the case on stable so building the topic branch is enough
• in a scenario where the code should do something: temporarily tweak our packages list so that debian-tor’s UID and GID are not the ones we want

#8 Updated by intrigeri 2018-03-22 09:43:05

• Assignee changed from intrigeri to segfault
• QA Check changed from Ready for QA to Dev Needed

intrigeri wrote:
> I’ll now try to test this:
>
> * in a scenario where the added code should be a no-op: currently it’s the case on stable so building the topic branch is enough

This works fine.

> * in a scenario where the code should do something: temporarily tweak our packages list so that debian-tor’s UID and GID are not the ones we want

This fails:

08:49:50 Change GIDs and UIDs
08:49:50 + echo Change GIDs and UIDs
08:49:50 + getent group 122
08:49:50 + awk -F : {print $1}
08:49:50 + TPS_GROUP_STEALER=Debian-gdm
08:49:50 + [ -n Debian-gdm ]
08:49:50 + groupmod --gid 150 Debian-gdm
08:49:50 + find / -wholename /proc -prune -o ( ! -type l -a -gid 122 -exec chgrp 150 {} ; )
08:49:51 + + awk -F : {print $1}
08:49:51 getent passwd 115
08:49:51 + TPS_USER_STEALER=
08:49:51 + [ -n  ]
08:49:51 + TOR_NEW_GID=114
08:49:51 + getent passwd debian-tor
08:49:51 + awk -F : {print $4}
08:49:51 Changing debian-tor GID from 115 to 114 
08:49:51 + TOR_OLD_GID=115
08:49:51 + [ 114 != 115 ]
08:49:51 + echo Changing debian-tor GID from 115 to 114 
08:49:51 + + awk -F : {print $1}
08:49:51 getent group 114
08:49:51 + TOR_GROUP_STEALER=monkeysphere
08:49:51 + [ -n monkeysphere ]
08:49:51 + echo debian-tor GID is occupied by monkeysphere
08:49:51 + groupmod --gid 151 monkeysphere
08:49:51 debian-tor GID is occupied by monkeysphere
08:49:51 + find / -wholename /proc -prune -o ( ! -type l -a -gid 114 -exec chgrp 151 {} ; )
08:49:51 + groupmod --gid 114 debian-tor
08:49:51 + find / -wholename /proc -prune -o ( ! -type l -a -gid 115 -exec chgrp 114 {} ; )
08:49:51 + TOR_NEW_UID=107
08:49:51 + + awk -Fgetent passwd debian-tor
08:49:51  : {print $3}
08:49:51 Changing debian-tor UID from 108 to 107
08:49:51 + TOR_OLD_UID=108
08:49:51 + [ 107 != 108 ]
08:49:51 + echo Changing debian-tor UID from 108 to 107
08:49:51 + awk -F+  : {print $1}
08:49:51 getent passwd 107
08:49:51 debian-tor UID is occupied by monkeysphere
08:49:51 + TOR_USER_STEALER=monkeysphere
08:49:51 + [ -n monkeysphere ]
08:49:51 + echo debian-tor UID is occupied by monkeysphere
08:49:51 + usermod --uid 151 monkeysphere
08:49:51 usermod: user monkeysphere is currently used by process 12939
08:49:51 E: config/chroot_local-hooks/04-change-gids-and-uids failed (exit non-zero). You should check for errors.

Here’s the patch I use locally to test this:

diff --git a/config/chroot_local-hooks/04-change-gids-and-uids b/config/chroot_local-hooks/04-change-gids-and-uids
index 32719e2ea3..5ec9f22cdb 100755
--- a/config/chroot_local-hooks/04-change-gids-and-uids
+++ b/config/chroot_local-hooks/04-change-gids-and-uids
@@ -1,6 +1,7 @@
 #!/bin/sh

 set -e
+set -x

 echo "Change GIDs and UIDs"

diff --git a/config/chroot_local-packageslists/tails-common.list b/config/chroot_local-packageslists/tails-common.list
index 5aa0f54a28..756f1bed80 100644
--- a/config/chroot_local-packageslists/tails-common.list
+++ b/config/chroot_local-packageslists/tails-common.list
@@ -425,3 +425,6 @@ python3-pydbus

 # For our python "shell" scripting support
 python3-sh
+
+# XXX: test!
+exim4

#9 Updated by intrigeri 2018-03-22 09:45:17

• blocks deleted (Bug #15422: 04-change-gids-and-uids doesn't change UIDs of existing files)

#10 Updated by segfault 2018-04-11 07:59:00

• Assignee changed from segfault to intrigeri
• QA Check changed from Dev Needed to Info Needed

> This fails:
> […]

So the issue is that the user to be changed still has processes running. We could try to kill all processes running by that user, and hope that it doesn’t break things, but that seems error-prone to me.

We should consider other approaches. One idea I had is to put the passwd file in config/chroot_local-includes, check in a late local hook whether it was changed, and abort the build in that case. This way we would detect when ids changed and would have to manually update the passwd file.

Do you see a better solution?

#12 Updated by intrigeri 2018-04-11 09:41:17

• Assignee changed from intrigeri to segfault

> So the issue is that the user to be changed still has processes running. We could try to kill all processes running by that user, and hope that it doesn’t break things, but that seems error-prone to me.

Agreed: the risk of problematic consequences is a bit too high IMO (it can affect stuff outside of the live-build chroot) and since signals are asynchronous it’s not trivial to get it right and non-racy.

> We should consider other approaches. One idea I had is to put the passwd file in config/chroot_local-includes, check in a late local hook whether it was changed, and abort the build in that case. This way we would detect when ids changed and would have to manually update the passwd file.

It was useful to look for a cheap ad-hoc solution to the specific problem this ticket is about, but I think we can now conclude that it’s not that cheap ⇒ I propose we reject this ticket and switch to Bug #15407 that’s about fixing the more general problem at hand wrt. changing UID/GID:s. And then if we want, or have, to implement a generic solution to Bug #15407 and Bug #15419, then I think what you’re proposing makes sense. See the description of Bug #15407 for a discussion about whether it’s worth doing that work or not. See you there!

#13 Updated by bertagaz 2018-05-10 11:09:32

• Target version changed from Tails_3.7 to Tails_3.8

#14 Updated by intrigeri 2018-06-26 16:28:05

• Target version changed from Tails_3.8 to Tails_3.9

#15 Updated by intrigeri 2018-06-28 20:51:20

• Status changed from In Progress to Rejected
• Assignee deleted (segfault)
• % Done changed from 30 to 100

I’ve updated Bug #15407 and Bug #15419. Conclusion: let’s reject this but I’ll reuse some of the code (the UID/GID change detection part) so it was useful!

#16 Updated by intrigeri 2018-06-30 12:48:39

• related to Bug #15695: Avoid breaking automatic upgrades to Tails 3.9 added