Project

General

Profile

Bug #16169

Inconsistent "Tor is not ready" notification

Added by mercedes508 6 months ago. Updated 26 days ago.

Status:
Duplicate
Priority:
Normal
Assignee:
-
Category:
Tor configuration
Target version:
-
Start date:
11/29/2018
Due date:
% Done:

0%

QA Check:
Feature Branch:
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Browser

Description

When clicking on a weblink (let's say from pidgin or thunderbird), we get a notification saying "Tor is not ready. Start Tor Browser anyway?" but Tor is fully ready and already working.


Related issues

Duplicated by Tails - Bug #16473: "Tor is not ready. Start Tor Browser anyway?" dialog when opening a link in Thunderbird Resolved 02/19/2019
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed 03/22/2019

History

#1 Updated by intrigeri 6 months ago

#2 Updated by intrigeri 6 months ago

  • Assignee deleted (intrigeri)

Good candidate for other FT members. To start with, let's try to reproduce and give a first try at fixing it. It might be easy to fix, in which case we can perhaps do it quickly; if not, can probably wait.

#3 Updated by intrigeri 5 months ago

  • Target version changed from Tails_3.11 to Tails_3.12

#4 Updated by intrigeri 5 months ago

#5 Updated by intrigeri 5 months ago

#6 Updated by intrigeri 4 months ago

  • Target version changed from Tails_3.12 to Tails_3.13

#7 Updated by intrigeri 2 months ago

  • Target version deleted (Tails_3.13)

#8 Updated by intrigeri 2 months ago

#9 Updated by intrigeri 2 months ago

#10 Updated by segfault about 2 months ago

  • Assignee set to segfault

#11 Updated by segfault about 2 months ago

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?

#12 Updated by segfault about 1 month ago

  • Assignee changed from segfault to mercedes508
  • QA Check set to Info Needed

#13 Updated by mercedes508 about 1 month ago

  • Assignee changed from mercedes508 to segfault

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.

#14 Updated by segfault about 1 month ago

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 about 1 month ago

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=

Nitpick: it could be slightly clearer to add Wants=your-new-unit.service to (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 c195ff18add2ef18be45e133678b5895d8dce255 was #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.)

#16 Updated by intrigeri about 1 month ago

  • QA Check deleted (Info Needed)

Forgot to say: the BindsTo= idea is pretty clever!

#17 Updated by segfault about 1 month ago

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=

Nitpick: it could be slightly clearer to add Wants=your-new-unit.service to (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 c195ff18add2ef18be45e133678b5895d8dce255 was #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 about 1 month ago

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=

Nitpick: it could be slightly clearer to add Wants=your-new-unit.service to (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 c195ff18add2ef18be45e133678b5895d8dce255 was #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 :)

#19 Updated by intrigeri about 1 month ago

  • Duplicated by Bug #16473: "Tor is not ready. Start Tor Browser anyway?" dialog when opening a link in Thunderbird added

#20 Updated by intrigeri about 1 month ago

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

#21 Updated by segfault about 1 month ago

  • Status changed from Confirmed to Rejected
  • Assignee deleted (segfault)

intrigeri wrote:

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

OK, since the conclusion on #16473 was to not spend FT time on this to fix it in 3.x, I'm rejecting this and will open a new ticket for the discussion about the "Tor has bootstrapped" status check.

#22 Updated by segfault about 1 month ago

segfault wrote:

I'm rejecting this and will open a new ticket for the discussion about the "Tor has bootstrapped" status check.

That's #16664

#23 Updated by intrigeri 26 days ago

  • Status changed from Rejected to Duplicate

ACK!

Also available in: Atom PDF