Tails

#11 Updated by segfault 2019-04-03 21:05:58

I can’t reproduce this. I started Tails 3.13.1, opened Pidgin and clicked Help -> Online Help

1. Shortly before Tor has started. It showed the dialog asking whether to start Tor Browser even though Tor is not ready yet.

2. Immediately after Onion Circuits icon indicates that Tor has started (and shortly after that the notification that Tor is ready popped up). It did start Tor Browser (after a long wait without indication that it’s doing anything).

@mercedes508, could you try to reproduce this on Tails 3.13.1 and tell us the exact steps you took?

#14 Updated by segfault 2019-04-13 20:42:29

mercedes508 wrote:
> segfault wrote:
> > I can’t reproduce this.
> > @mercedes508, could you try to reproduce this on Tails 3.13.1 and tell us the exact steps you took?
>
> Try clicking on a web link in an email in thunderbird for instance, that’s how I get the pop-up.

OK, with thunderbird I can reproduce this issue. The culprit is the thunderbird AppArmor profile, which starts /usr/local/bin/tor-browser with the same AppArmor profile, which doesn’t allow executing /usr/local/sbin/tor-has-bootstrapped.

/usr/local/sbin/tor-has-bootstrapped actually only checks whether two systemd units are active (tor\default.service@ and tails-tor-has-bootstrapped.target). I thought it would be nice if we could just check a single systemd unit instead, which would be allowed by AppArmor. I tinkered around with systemd units a bit and I think I found a good solution.

This systemd service always starts when tor\default.service@ is (re-)started and always stops when tor\default.service@ is stopped:

[Unit]
Description=Tor has Bootstrapped
Documentation=https://tails.boum.org/contribute/design/
After=tor@default.service
BindsTo=tor@default.service

[Service]
Type=oneshot
RemainAfterExit=yes
User=debian-tor
ExecStart=/bin/sh -c '. /usr/local/lib/tails-shell-library/tor.sh ; \
                      while ! tor_is_working ; do                   \
                         /bin/sleep 1                             ; \
                      done'
TimeoutStartSec=0
CapabilityBoundingSet=
PrivateDevices=yes
PrivateTmp=yes
ProtectHome=yes
ProtectSystem=full

[Install]
WantedBy=tor@default.service

I think we could use this as a replacement for both tails-tor-has-bootstrapped.target and tails-wait-until-tor-has-bootstrapped.service. And checking whether this service is active should have the same result as /usr/local/sbin/tor-has-bootstrapped, so we could get rid of that too.

@intrigeri, what do you think about that?

#15 Updated by intrigeri 2019-04-15 09:25:48

Hi segfault,

> OK, with thunderbird I can reproduce this issue. The culprit is the thunderbird AppArmor profile, which starts /usr/local/bin/tor-browser with the same AppArmor profile, which doesn’t allow executing /usr/local/sbin/tor-has-bootstrapped.

It seems to me that regardless of whether we reimplement/replace tor-has-bootstrapped, we’ll need to:

• either allow Thunderbird to run any command that tor-has-bootstrapped (or whatever replaces it) needs to run; actually, same goes for any app confined with AppArmor, that we allow to open links with Tor Browser; that’s quite some busywork and additional long-term maintenance costs;
• or give /usr/local/sbin/tor-has-bootstrapped its own AppArmor profile, that other profiles can safely transition to when opening a link; that sounds a bit better.

I would go the 2nd way. This would be sufficient to fix the problem this ticket is about, right? I think it would be non-invasive enough to go into Tails 3.x.

> /usr/local/sbin/tor-has-bootstrapped actually only checks whether two systemd units are active (tor\@default.service and tails-tor-has-bootstrapped.target).

Actually, I don’t understand why it has to check the status of tor\default.service. IIRC we correctly stop tails-tor-has-bootstrapped.target@ whenever we stop tor. Perhaps this wrapper is erring on the safe side, which makes some sense.

> I thought it would be nice if we could just check a single systemd unit instead which would be allowed by AppArmor.

I don’t understand. AFAIK, AppArmor only mediates “can program X run program Y?” and it has no way to check arguments. So checking the status of one, two, or 10^9 systemd units requires the same AppArmor permission: executing systemctl with arbitrary arguments. Which incidentally, when run as the desktop user, probably allows doing lots of dangerous things. So we would need some kind of wrapper, that Thunderbird would be allowed to execute, but then, we already have one such wrapper. So if your main goal here was to simplify the resolution of the bug this ticket is about, by allowing Thunderbird to check the status of one systemd unit instead of two, I suspect you’re on a wrong lead. Or did I miss something?

Regarding replacing tails-tor-has-bootstrapped.target and tails-wait-until-tor-has-bootstrapped.service:

> WantedBy=tor@default.service

Nitpick: it could be slightly clearer to add Wants=your-new-unit.service to tor@default.service (via a drop-in override).

> I think we could use this as a replacement for both tails-tor-has-bootstrapped.target and tails-wait-until-tor-has-bootstrapped.service.

I’m open to this idea if it helps fix a problem we have (it’s not clear which one at this point).

On the one hand, one less unit decreases complexity. OTOH, a target — a “well-known synchronization point” — feels like the exact semantics we want here.
Note that the reason we introduced a target in commit:c195ff18add2ef18be45e133678b5895d8dce255 was Bug #9393.

> And checking whether this service is active should have the same result as /usr/local/sbin/tor-has-bootstrapped, so we could get rid of that too.

Having the /usr/local/sbin/tor-has-bootstrapped abstraction layer has served us well so far: we were able to change a few times its implementation without to adjust the various places where it’s used.

If we get rid of it, we’ll need to adjust at least 5 other files + quite possibly a few AppArmor profiles (to allow running systemctl, which seems undesirable, one more reason to keep a wrapper like tor-has-bootstrapped) right now… and every time we change the implementation details again in the future. (Yeah, I’d love this one to be the list time but assuming it’ll be feels a tad presumptuous given the tortuous history of this topic in Tails.)

#17 Updated by segfault 2019-04-16 00:51:45

intrigeri wrote:
> It seems to me that regardless of whether we reimplement/replace tor-has-bootstrapped, we’ll need to:
>
> * either allow Thunderbird to run any command that tor-has-bootstrapped (or whatever replaces it) needs to run; actually, same goes for any app confined with AppArmor, that we allow to open links with Tor Browser; that’s quite some busywork and additional long-term maintenance costs;
> * or give /usr/local/sbin/tor-has-bootstrapped its own AppArmor profile, that other profiles can safely transition to when opening a link; that sounds a bit better.
>
> I would go the 2nd way. This would be sufficient to fix the problem this ticket is about, right? I think it would be non-invasive enough to go into Tails 3.x.

I just tried to fix the issue with a permissive profile for /usr/local/sbin/tor-has-bootstrapped and/or /usr/local/bin/tor-browser, but couldn’t get it to work. Could try again another time.

> > /usr/local/sbin/tor-has-bootstrapped actually only checks whether two systemd units are active (tor\default.service and tails-tor-has-bootstrapped.target). > > Actually, I don't understand why it has to check the status of tor\default.service. IIRC we correctly stop tails-tor-has-bootstrapped.target whenever we stop tor. Perhaps this wrapper is erring on the safe side, which makes some sense.

No, if I run systemctl stop tor\default.service@ after it has bootstrapped, the current tails-tor-has-bootstrapped.target is still active (because it doesn’t use BindsTo). I think that’s why we currently also check the status of tor\default.service@.

> > I thought it would be nice if we could just check a single systemd unit instead which would be allowed by AppArmor.
>
> I don’t understand. AFAIK, AppArmor only mediates “can program X run program Y?” and it has no way to check arguments. So checking the status of one, two, or 10^9 systemd units requires the same AppArmor permission: executing systemctl with arbitrary arguments. Which incidentally, when run as the desktop user, probably allows doing lots of dangerous things. So we would need some kind of wrapper, that Thunderbird would be allowed to execute, but then, we already have one such wrapper. So if your main goal here was to simplify the resolution of the bug this ticket is about, by allowing Thunderbird to check the status of one systemd unit instead of two, I suspect you’re on a wrong lead. Or did I miss something?

Before I started working on the systemd unit, I tested that /usr/local/bin/tor-browser is allowed to execute /bin/systemctl is-active commands. I’m not entirely sure why, but I suspect that is because of the /{usr/,}bin/* Pixr, line in abstractions/ubuntu-helpers. Sorry, I should have told you that I already tested this.

> Regarding replacing tails-tor-has-bootstrapped.target and tails-wait-until-tor-has-bootstrapped.service:
>
> > WantedBy=tor@default.service
>
> Nitpick: it could be slightly clearer to add Wants=your-new-unit.service to tor@default.service (via a drop-in override).

That is added automatically by the WantedBy line when systemd loads the unit files, and I think it would be cleaner to not patch the existing file but provide the same functionality via our own file.

> > I think we could use this as a replacement for both tails-tor-has-bootstrapped.target and tails-wait-until-tor-has-bootstrapped.service.
>
> I’m open to this idea if it helps fix a problem we have (it’s not clear which one at this point).

It does fix the problem at hand in my tests.

> On the one hand, one less unit decreases complexity. OTOH, a target — a “well-known synchronization point” — feels like the exact semantics we want here.
> Note that the reason we introduced a target in commit:c195ff18add2ef18be45e133678b5895d8dce255 was Bug #9393.

The way I understand it, a systemd service can be used in exactly the same way as a systemd target. Both are units, and a target is just a unit without any special properties, see https://www.freedesktop.org/software/systemd/man/systemd.target.html. So we should still be able to use it as a well-known synchronization point.

> > And checking whether this service is active should have the same result as /usr/local/sbin/tor-has-bootstrapped, so we could get rid of that too.
>
> Having the /usr/local/sbin/tor-has-bootstrapped abstraction layer has served us well so far: we were able to change a few times its implementation without to adjust the various places where it’s used.
>
> If we get rid of it, we’ll need to adjust at least 5 other files

That’s correct, but those are one-line changes.

> + quite possibly a few AppArmor profiles (to allow running systemctl, which seems undesirable, one more reason to keep a wrapper like tor-has-bootstrapped)

I don’t think we would have to change any existing AppArmor profiles. None of the other scripts which use /usr/local/sbin/tor-has-bootstrapped seem to be confined by AppArmor. I also tested that tails-documentation is also allowed to execute systemctl is-active (even though I couldn’t find an AppArmor profile for it, so I guess it’s not confined at all).

> right now… and every time we change the implementation details again in the future. (Yeah, I’d love this one to be the list time but assuming it’ll be feels a tad presumptuous given the tortuous history of this topic in Tails.)

OK, I see that it could be a valid concern that the above systemd service might not be sufficient to tell if tor has bootstrapped if tor breaks it’s behavior on that part. But I think that even in that case there are good chances that we will be able to adapt the service to still work as expected. And I’m not convinced that it’s better to have another script involved to find out whether Tor has bootstrapped + maintaining an AppArmor profile for it, IMO it adds unnecessary complexity to our already very complex solution to answer a quite simple question.

#18 Updated by intrigeri 2019-04-16 07:16:50

segfault wrote:
> I just tried to fix the issue with a permissive profile for /usr/local/sbin/tor-has-bootstrapped and/or /usr/local/bin/tor-browser, but couldn’t get it to work. Could try again another time.

Don’t bother, see below for a cheaper solution.

> intrigeri wrote:
>> Actually, I don’t understand why it has to check the status of tor\default.service. IIRC we correctly stop tails-tor-has-bootstrapped.target@ whenever we stop tor. Perhaps this wrapper is erring on the safe side, which makes some sense.

> No, if I run systemctl stop tor\default.service@ after it has bootstrapped, the current tails-tor-has-bootstrapped.target is still active (because it doesn’t use BindsTo). I think that’s why we currently also check the status of tor\default.service@.

Sorry I was unclear! My claim/guess/assumption was about what happens automatically, i.e.:

• whenever Tails stops tor\default.service, it also stops tails-tor-has-bootstrapped.target@
• we restart tails-tor-has-bootstrapped.target conservatively every time the network goes up or back up

IMHO we should not worry about the case when a user manually stops tor\default.service@ here, and thus checking tails-tor-has-bootstrapped.target should be enough. Anyway, I’m kinda diverging off topic here :)

> Before I started working on the systemd unit, I tested that /usr/local/bin/tor-browser is allowed to execute /bin/systemctl is-active commands. I’m not entirely sure why, but I suspect that is because of the /{usr/,}bin/* Pixr, line in abstractions/ubuntu-helpers. Sorry, I should have told you that I already tested this.

Thanks, indeed I did miss something! I think you’re correct:

/{usr/local/,usr/,}bin/{[^gp],g[^p],p[^s],gp[^g]}* Cx -> sanitized_helper,

… allows Thunderbird to run /usr/local/bin/tor-browser under the sanitized_helper profile, which as you said allows it to run systemctl.

But sanitized_helper on Stretch does not allow running /usr/local/sbin/tor-has-bootstrapped as you said earlier, which makes no sense. That bug was fixed in Buster, where sanitized_helper has:

/{usr/,usr/local/,}{bin,sbin}/* Pixr,

… instead of Stretch’s:

/{usr/,}bin/* Pixr,
/{usr/,}sbin/* Pixr,
/usr/local/bin/* Pixr,

… so I assume that:

• The bug this ticket is about is already fixed on our feature/buster branch.
• The cheapest way to fix in Tails 3.x the bug this ticket is about is to backport this bugfix to the sanitized_helper profile we ship.

Does this all make sense to you?

If it does, I propose we:

1. Verify that the bug is indeed fixed on feature/buster already.
2. Backport that fix in Tails 3.14.
3. Move the discussion about the “Tor has bootstrapped” status check to a new, dedicated ticket, where the proposal will be judged on its own merits, independently from this very bug that has a much simpler explanation & solution that’s suitable for 3.x (I know we have a pretty relaxed policy for what qualifies for 3.x stable releases but a full revamp the “Tor has bootstrap” implementation would stretch it a bit too far IMO).

>> Regarding replacing tails-tor-has-bootstrapped.target and tails-wait-until-tor-has-bootstrapped.service:
>>
>> > WantedBy=tor@default.service
>>
>> Nitpick: it could be slightly clearer to add Wants=your-new-unit.service to tor@default.service (via a drop-in override).

> That is added automatically by the WantedBy line when systemd loads the unit files, and I think it would be cleaner to not patch the existing file but provide the same functionality via our own file.

If we go this way: your call, fine by me (I did not mean to patch any file BTW).

>> > I think we could use this as a replacement for both tails-tor-has-bootstrapped.target and tails-wait-until-tor-has-bootstrapped.service.
>>
>> I’m open to this idea if it helps fix a problem we have (it’s not clear which one at this point).

> It does fix the problem at hand in my tests.

Fair enough!

I understand it fixes the problem at hand (which is essentially “the Thunderbird AppArmor profile forbids it from running /usr/local/sbin/tor-has-bootstrapped”) by replacing the forbidden call to /usr/local/sbin/tor-has-bootstrapped by a call to systemctl, that’s already allowed. Correct?

>> On the one hand, one less unit decreases complexity. OTOH, a target — a “well-known synchronization point” — feels like the exact semantics we want here.
>> Note that the reason we introduced a target in commit:c195ff18add2ef18be45e133678b5895d8dce255 was Bug #9393.

> The way I understand it, a systemd service can be used in exactly the same way as a systemd target. Both are units, and a target is just a unit without any special properties, see https://www.freedesktop.org/software/systemd/man/systemd.target.html. So we should still be able to use it as a well-known synchronization point.

Sure, technically a service will work just the same, at least in the current state of systemd. It’s just that it’s not meant to be used this way, as per the doc:

• A unit configuration file whose name ends in “.service” encodes information about a process controlled and supervised by systemd
• A unit configuration file whose name ends in “.target” encodes information about a target unit of systemd, which is used for grouping units and as well-known synchronization points during start-up.

So using a service to provide a well-known synchronization point can be somewhat confusing. But that’s certainly not a blocker IMO!

>> > And checking whether this service is active should have the same result as /usr/local/sbin/tor-has-bootstrapped, so we could get rid of that too.
>>
>> Having the /usr/local/sbin/tor-has-bootstrapped abstraction layer has served us well so far: we were able to change a few times its implementation without to adjust the various places where it’s used.
>>
>> If we get rid of it, we’ll need to adjust at least 5 other files

> That’s correct, but those are one-line changes.

>> + quite possibly a few AppArmor profiles (to allow running systemctl, which seems undesirable, one more reason to keep a wrapper like tor-has-bootstrapped)

> I don’t think we would have to change any existing AppArmor profiles. None of the other scripts which use /usr/local/sbin/tor-has-bootstrapped seem to be confined by AppArmor. I also tested that tails-documentation is also allowed to execute systemctl is-active (even though I couldn’t find an AppArmor for it, so I guess it’s not confined at all).

How does that work for opening links from Evince, Pidgin, and Totem? I don’t know if they’re all allowed to execute systemctl (with any argument).

>> right now… and every time we change the implementation details again in the future. (Yeah, I’d love this one to be the list time but assuming it’ll be feels a tad presumptuous given the tortuous history of this topic in Tails.)

> OK, I see that it could be a valid concern that the above systemd service might not be sufficient to tell if tor has bootstrapped if tor breaks it’s behavior on that part. But I think the chances that even in that case there are good chances that we will be able to adapt the service so still work as expected.

OK, you’ve convinced me that from a design perspective at least, the status of that service can plausibly serve as a LTS API entry point, whose implementation details we can change without affecting its consumers. At least I’m OK to try it :)

#20 Updated by intrigeri 2019-04-16 07:38:59

Ah, I was under the impression that I had already debugged this, and indeed: Bug #16473.

#22 Updated by segfault 2019-04-19 16:18:43

segfault wrote:
> I’m rejecting this and will open a new ticket for the discussion about the “Tor has bootstrapped” status check.

That’s Feature #16664