Tails

Brainstorming our options:

1. ship a patched virtualbox-guest-utils package that removes the added dependency and hope that's enough to fix this instance of the more general problem (while we're at it, we can as well backport the latest version from sid, that adds compatibility with Linux 4.17; too bad, virtualbox is now in stretch-backports which could have given us the opportunity to stop shipping it via our custom APT repo); that's relatively cheap and easy, and surely an acceptable way to postpone the problem, hoping we have more time to address its root cause next time another instance of this problem pops up.
2. go back to the hacks from #15424: change UIDs/GIDs via a chroot hook, killing processes as needed; service startup is disabled in the chroot, so in theory only processes started via maintainer scripts and left behind should have to be killed; in practice, that probably boils down to gpg-agent; so while my concern about killing processes outside of the chroot remains, limiting the kill spree to gpg-agent processes might be safe enough; in our current Vagrant VM, I see no user that has obvious reasons to have gpg-agent running, and indeed I see no such process running during a build. That's also relatively cheap and easy, and most of the work has been done already. It'l be wasted work at some point if overlayfs is not affected by the bug, but well, at least we're covered until #8415 is done.
3. find some way to stabilize all UIDs/GIDs (#15407): looks like more work than the two first options, and that work will be wasted if overlayfs is not affected by the bug, which makes this option very unappealing to me.
4. complete #8415 (overlayfs) in time for Tails 3.9… assuming overlayfs is not affected by the bug that makes us worry about changing UIDs/GIDs (to be verified on #15689): there's not that much work left to do and we have to do it soon anyway, so clearly it would be the best place to invest our time in; but it's not clear whether it's realistic to do that in the next 1.5 months

I'm currently leaning towards the 2nd option (stabilize just the UIDs/GIDs we want), with the 1st one as a fallback. But depending on how our first shots at #15091 and #15023 go, and on how we distribute the work on these two big tasks, who knows, #8415 might be doable. I'll come back here in a few weeks and depending on our progress on these other fronts, I'll pick the best approach among the options that will be realistic at this point.

Thanks for the ping. I've spent about 1h looking at this issue and trying to understand it.

In my attempt to properly understand this issue, I dived into the code, resulting in the following entirely-untested commits: https://github.com/lamby/tails/commits/15695-avoid-breaking-automatic-upgrades-to-tails-3-9. These might be worth merging anyway as it's essentially "better commenting"

However, I got somewhat stuck on:

change UIDs/GIDs via a chroot hook, killing processes as needed; service startup is disabled in the chroot, so in theory only processes started via maintainer scripts and left behind should have to be killed; in practice, that probably boils down to gpg-agent

Three questions arise for me here: why would anything be running at all? Why would that be gpg-agent and not, well, something running as debian-tor? Why do we not do (or need?) the kill-process dance for the tails-persistent-setup chroot hack right now?

(Some quick personal notes for this bug: See #13426 for the background on why this breaks upgrades, the root cause being probably in aufs which will eventually be replaced with overlayfs in #8415.)

In my attempt to properly understand this issue, I dived into the code, resulting in the following entirely-untested commits: https://github.com/lamby/tails/commits/15695-avoid-breaking-automatic-upgrades-to-tails-3-9. These might be worth merging anyway as it's essentially "better commenting"

Good idea! Two potential issues that prevent me from merging it though:

• In Change_gid I think you meant -exec chgrp instead of -exec chown.
• Change_gid 112 150 feels strange because we had TPS_GROUP_STEALER=$(getent group 122 | awk -F ':' '{print $1}') (note 112 vs. 122)

By the way, basing your work on top of my branch for #15419 will help you validate it :) I've updated bugfix/15419-detect-uid-and-gid-changes so our CI builds it and I'll share the FTBFS message with you as soon as a build has failed, which will demonstrate better what the practical problem we need to solve now is.

However, I got somewhat stuck on:

change UIDs/GIDs via a chroot hook, killing processes as needed; service startup is disabled in the chroot, so in theory only processes started via maintainer scripts and left behind should have to be killed; in practice, that probably boils down to gpg-agent

Three questions arise for me here: why would anything be running at all?
Why would that be gpg-agent and not, well, something running as debian-tor?

On #15424 we noticed that the monkeysphere user had a running process. I've assumed this was gpg-agent because on our ISO build CI jobs we have to kill those on exit in order to be able to tear down a mountpoint. IIRC that's because the monkeysphere postinst performs GnuPG operations, that trigger gpg-agent startup (in the background, that's how GnuPG 2 works). The tor package does no such thing which is why there's no running process left behind running as the debian-tor user. Now, my hunch might be wrong (I don't recall whether I reality-checked it) but we'll know as soon as the workaround this ticket is now about is implemented :)

Why do we not do (or need?) the kill-process dance for the tails-persistent-setup chroot hack right now?

Do you mean: why don't we do it yet (in current devel branch)? Or why shouldn't we do it for 3.9 as part of this ticket? I guess because we've never needed it so far :)

In Change_gid I think you meant -exec chgrp instead of -exec chown.

Fixed.

Change_gid 112 150 feels strange because we had TPS_GROUP_STEALER=$(getent group 122 | awk -F ':' '{print $1}') (note 112 vs. 122)

Fixed. That'll teach me to work in the sun...

By the way, basing your work on top of my branch for #15419 will help you validate it

Rebased.

I'll review the (side-)changes you've done already (and will likely merge them!) but let's keep this assigned to you for tracking the main problem this ticket is about.

let's keep this assigned to you for tracking the main problem this ticket is about.

Thanks, good idea — as mentioned, I only really wrote and sent over these patches so I ensured I was understanding the issue, kinda like how making your own notes in a lecture is not the same as just reading someone else's! :) Do appreciate they are somewhat distracting however.

Here's the current problem we're trying to "solve" here:

Thanks for sending that over.

So, my reading of that diff of that we need to move the uid of vboxdrv from 112 → 151 and its gid from 112 → 151. That your quick assessment too? :)

So, my reading of that diff of that we need to move the uid of vboxdrv from 112 → 151 and its gid from 112 → 151. That your quick assessment too? :)

I don't see vboxdrv involved anywhere in the error message I've pasted.

My quick assessment is that UIDs are all right (unchanged in the diff), only GIDs are problematic. Wrt. GIDs, 4 groups are involved and they're fighting for a stable set of 4 GIDs (112-115). What we need to do is to restore the state from Tails 3.8 (captured in /usr/share/tails/build/group) i.e.:

• vboxsf: GID 115 → 112
• monkeysphere: GID 112 → 113
• debian-tor: GID 113 → 114
• lpadmin: GID 114 → 115

The "fun" part is that I think we can't do this in place with atomic groupmod operations. So likely we need to temporarily give one of these groups another, free GID that's not in the 112-115 range, and then change the GID for the 3 other groups in the right order, and finally give its correct GID to the group that was temporarily moved out of the way.

Needless to say, all this is ugly and I don't expect it to get any better in the future… until we fix the root cause of the problem or devise a nicer solution to set all these IDs in stone.

I don't see vboxdrv involved anywhere in the error message I've pasted.

Ah, copy-paste fail. vboxsf.. :)

[…]

How about the attached? :)

How about the attached? :)

I think it will work but:

• Can you confirm you tested it and the new check for changed UIDs/GIDs does not abort the build?
• The first part of the patch hard-codes the current (as in, on our devel branch today) GID allocated to these 4 groups. As the mere existence of this very ticket shows, it's a risky assumption. So next time these GIDs start dancing around, IMO this patch will make it harder to fix the problem than it could, because if we apply it as-is we may be changing the GID of unrelated groups in the future. So I'd rather see us do things like Change_gid $(gid_of monkeysphere) 1120. Makes sense?

I'd rather see us do thingws like Change_gid $(gid_of monkeysphere) 1120

Oh, good idea. This is not only a more stable solution, it is also much cleaner in the code (self-documenting!). Please see the attached 2 patches which I have also pushed to https://github.com/lamby/tails/commits/15695-avoid-breaking-automatic-upgrades-to-tails-3-9.

Can you confirm you tested it and the new check for changed UIDs/GIDs does not abort the build?

Unfortunately I have not tested it (except in a local chroot actually, but that doesn't count). Unfortunately, I am semi-VAC today and tomorrow (first time off since 12th July...) so I won't be able to commit to testing this before the 15th unless it happens to rain this afternoon (!) which would mean that I would not need to be entertaining my dear mother :)

This branch includes the check brought for #15419 so if it builds (and it did build at least until I've merged current devel into it), that means it does its job :) And indeed, until my last fixes that check did abort the build, which is confidence-inspiring.

As per #15407#note-31.

Applied in changeset 1d3d7854b310ae77761fbef22af59458450801ba.

Applied in changeset 3682fde35080381eb97d649966b584fc9c777998.