Tails

#1 Updated by anonym 2017-04-26 19:19:51

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

So I started this today, and I settled for prosody since it’s written in LUA. Like you (segfault) said, writing a service was indeed easy… except I hit a general problem: sometimes the onion address is needed for the configuration before the service is started, like prosody for its (mandatory) VirtualHost $domain configuration. I tried adding a crappy option to make the service start happen after the onion is published (and the onion address is available) but that messed up the status notifications; see commit:300df316462eee894307ab9f6ba308d6b9c589d2.

In the end I solved it by running stop() and start() after writing the configuration… but this feels like a general problem. Either we need to implement a working publish_hs_before_starting option, or we switch make the HS publishing always happen before starting the service. I actually don’t see any reason for the current order, but perhaps you had some reason for this? (Please reassign this ticket to me when you have answered.)

—

Next, I’ve been fighting with making the tails-server-client side of things smooth. My plan is to add a helper script that writes the account configuration XML. But that’s not enough; there’s no configuration option to tell pidgin to register the account, so I was thinking of making the helper also register it using some XMPP library (note to self: register before writing the account configuration, then we know that we don’t have to register if the account configuration is already present). I already have a working PoC for pyxmpp (python-xmpp) but it is unmaintained and not available for Python 3. I’ve also tried python-nbxmpp (it uses sockets and getaddrinfo in a way not compatible with torsocks, and has no SOCKS support), and libnet-jabber-perl (seems completely broken). I might try implementing the needed bits of the XMPP protocol with some echo | netcat hacks if it’s small enough…

BTW, we should deal with the case when tails-server-client will try to write configuration to an already running application, which will cause issues most of the time since that new configuration won’t be loaded until after a restart. This is the case with Pidgin, for instance.

#2 Updated by anonym 2017-04-26 19:20:20

• Feature Branch set to feature/12478-tails-server-xmpp

#3 Updated by anonym 2017-04-26 19:33:15

• Status changed from Confirmed to In Progress

Applied in changeset commit:7593ef5ddba6b40b070e523511d748d8e8142e2f.

#4 Updated by anonym 2017-04-27 14:16:51

• % Done changed from 0 to 50
• QA Check changed from Info Needed to Ready for QA

I solved the issues with the client launcher by using Pidgin/libpurple’s DBus interface to both create and register the account (but in between I had a working PoC that did it by writing the XML configuration, and registering by implementing that tiny part of the XMPP protocol). I’ll raise the general issue on the main ticket, so all that remains on this ticket now is…

… please review’n’merge (into your repos feature/5688-tails-server branch)! :)

#6 Updated by segfault 2017-05-06 17:36:54

• Assignee changed from segfault to anonym
• QA Check changed from Ready for QA to Info Needed

> So I started this today, and I settled for prosody since it’s written in LUA.
Sounds good.

> Like you (segfault) said, writing a service was indeed easy… except I hit a general problem: sometimes the onion address is needed for the configuration before the service is started, like prosody for its (mandatory) VirtualHost $domain configuration.

What would happen if VirtualHost would just be “localhost”? Also, I guess this means that prosody service doesn’t currently work via LAN? Would it work if we added an additional “VirtualHost $LocalIP” to the config? (This of course would require us to determine the correct interface and its IP address, which is non-trivial and I didn’t tackle it yet.)

> I tried adding a crappy option to make the service start happen after the onion is published (and the onion address is available) but that messed up the status notifications; see 300df316462eee894307ab9f6ba308d6b9c589d2.

Right, we would have to change how the current status is determined.

> In the end I solved it by running stop() and start() after writing the configuration… but this feels like a general problem. Either we need to implement a working publish_hs_before_starting option, or we switch make the HS publishing always happen before starting the service. I actually don’t see any reason for the current order, but perhaps you had some reason for this? (Please reassign this ticket to me when you have answered.)

I think the current order is simply the most intuitive for the user: first the service is installed, then it is started and then it is available via the network. It might be a bit confusing if the status is first “Announcing onion address” and then “Starting”, especially if the “Starting” step fails for whatever reason.

If changing the order would make adding services a lot easier or reduce complexity, I would be fine with it. But I like the solution you chose by overriding the set_onion_address() function. I think this is a clean way to solve this problem and we could do the same with other services which require the onion address in the config. I think we would also need something similar when the “allow_lan” option is activated. We could implement a reload() function (which uses systemctl reload) instead of stopping and starting the service.

Side note: Instead of the update_config() function, we could use the functions in file_util.py, like remove_line_if_present() and append_line_if_not_present() (and add more functions there if required).

Also, in the other services, I deliberately avoided completely overwriting the config file. The config file shipped with the Debian package might change in newer versions. I think we more likely want to keep those changes than overwrite them. On the other hand, such changes might also break the service in Tails Server. How do you think we should handle this?

—

> Next, I’ve been fighting with making the tails-server-client side of things smooth. […]

Wow, you really implemented part of the XMPP protocol for this :D I’m sorry you had such trouble with this, and I’m glad you found a solution.
The account creation worked flawlessly for me. I only see a few possible UX pitfalls:

1. I’m not sure if it will be immediately clear to the user that he has to choose the username and password in the “Connect to Tails Server” dialog himself. This is a new workflow, in the other services all the information in this dialog comes from the onion service admin. I think it would be better if the user was asked for the username and password in a separate dialog that would be started after clicking “Connect”.

2. The other services automatically connect the user with all the other users using the service, which is consistent with the vision of Tails Server being primarily for groups who want to connect with each other. With Pidgin, the user still has to manually add other users as buddies before he can chat with them. I think it would be really cool if we could add an option to open a chatroom and make Pidgin automatically join this chatroom. I see that you actually already add a chatroom in prosody, but I think the part on the client side is missing.

3. Like you mentioned, Pidgin doesn’t restart to apply the configuration. Maybe we could pop up a dialog asking to restart Pidgin if it is already running? We of course do the same thing for other clients, but I didn’t check yet if other clients we use have the same problem.

—

> … please review’n’merge (into your repos feature/5688-tails-server branch)! :)

Done. The code looks good :) I couldn’t resist some minor refactoring to improve readability in the create-pidgin-xmpp-account script though.

#7 Updated by segfault 2017-05-06 22:57:53

• % Done changed from 50 to 70
• QA Check changed from Info Needed to Ready for QA

> I only see a few possible UX pitfalls:
>
> 1. I’m not sure if it will be immediately clear to the user that he has to choose the username and password in the “Connect to Tails Server” dialog himself. This is a new workflow, in the other services all the information in this dialog comes from the onion service admin. I think it would be better if the user was asked for the username and password in a separate dialog that would be started after clicking “Connect”.
>
> 2. The other services automatically connect the user with all the other users using the service, which is consistent with the vision of Tails Server being primarily for groups who want to connect with each other. With Pidgin, the user still has to manually add other users as buddies before he can chat with them. I think it would be really cool if we could add an option to open a chatroom and make Pidgin automatically join this chatroom. I see that you actually already add a chatroom in prosody, but I think the part on the client side is missing.
>
> 3. Like you mentioned, Pidgin doesn’t restart to apply the configuration. Maybe we could pop up a dialog asking to restart Pidgin if it is already running? We of course do the same thing for other clients, but I didn’t check yet if other clients we use have the same problem.

I implemented all of those, i.e. (1) we show a separate dialog to get the username and password from the user, (2) Pidgin automatically joins the chatroom we get via the connection informatoin, and (3) we show a dialog asking the user if we should automatically close Pidgin, if Pidgin is running.

Please review!

#8 Updated by anonym 2017-05-08 13:57:09

• Assignee changed from anonym to segfault
• QA Check changed from Ready for QA to Info Needed

segfault wrote:
> > So I started this today, and I settled for prosody since it’s written in LUA.
> Sounds good.
>
> > Like you (segfault) said, writing a service was indeed easy… except I hit a general problem: sometimes the onion address is needed for the configuration before the service is started, like prosody for its (mandatory) VirtualHost $domain configuration.
>
> What would happen if VirtualHost would just be “localhost”?

Then no one but the Tails Server operator can connect (well, others could achieve the same by setting up transparent torification on localhost:XMPP_PORT). In XMPP the domain has a huge semantic impact, e.g. it determines your full jid (user@domain) which is how users will interact with you. Indeed, with the VirtualHost you propose, if I connect to localhost my jid would be userlocalhost. You can configure several VirtualHost:s, of course, but that will mean several different domains facing users, e.g. user1@localhost@ and user2[blah].onion... or even same@localhost@ and same[blah].onion! Yup, that is how it works -- the domain the XMPP client connects to has to match the domain(s) the server is aware of (via VirtualHost@).

> Also, I guess this means that prosody service doesn’t currently work via LAN? Would it work if we added an additional “VirtualHost $LocalIP” to the config?

Correct. Allowing this would be extra confusing since the LAN IP address might change across sessions (especially since MAC spoofing is enabled by default, vs DHCP) which means a different jid and hence account. In other words, if ther server has IP address ip-address-1 and you register userip-address-1, then server restarts and gets IP address ip-address-2, then the account user@ip-address-1@ will be inaccessible. You can register userip-address-2@, but that is a completely new account, so you lose your roster etc.

I think serving on a single domain, the onion, is good enough, and avoids a lot of confusion. It will work for everyone (localhost/operator, others on the LAN (if they have Tor running) and any other Tor users), and also allow everyone (incl. the operator) to use the service launcher. Agreed? If so we probably should add a nice comment about this so this issue isn’t wasted time on again. :)

Regarding the “Allow LAN connections” and “Allow local connections” options, I only wanted them for mumble, basically, since that allows you to avoid once circuit of delay. No other service we have is sensitive to delay, so it should be considered to remove those options to leave less things around to confuse users.

> > I tried adding a crappy option to make the service start happen after the onion is published (and the onion address is available) but that messed up the status notifications; see 300df316462eee894307ab9f6ba308d6b9c589d2.
>
> Right, we would have to change how the current status is determined.
>
> > In the end I solved it by running stop() and start() after writing the configuration… but this feels like a general problem. Either we need to implement a working publish_hs_before_starting option, or we switch make the HS publishing always happen before starting the service. I actually don’t see any reason for the current order, but perhaps you had some reason for this? (Please reassign this ticket to me when you have answered.)
>
> I think the current order is simply the most intuitive for the user: first the service is installed, then it is started and then it is available via the network. It might be a bit confusing if the status is first “Announcing onion address” and then “Starting”, especially if the “Starting” step fails for whatever reason.

OTOH, when the next generation onion services, the “announcing” part will be more or less instant, right? Regarding “intuitive”, I don’t know if users really care. The important part, I imagine, is just to see that stuff is happening, and possibly which status a failure (if any) occurs in when reporting to us (but these things change too fast so I don’t think that is reliable).

From my developer perspective I do agree that the current order feels more intuitive, though. But I find the workaround I have to do slightly worse… and the way the icon goes from green to red and then back to green isn’t very pretty either, although probably not enough to confuse users.

> If changing the order would make adding services a lot easier or reduce complexity, I would be fine with it. But I like the solution you chose by overriding the set_onion_address() function. I think this is a clean way to solve this problem and we could do the same with other services which require the onion address in the config. I think we would also need something similar when the “allow_lan” option is activated. We could implement a reload() function (which uses systemctl reload) instead of stopping and starting the service.

Before I went with stop() + start() I tried systemctl reload prosody and it did not reload VirtualHost configuration. :/ So, we cannot rely on this in general.

So, yeah, I’m a bit torn here…

> Side note: Instead of the update_config() function, we could use the functions in file_util.py, like remove_line_if_present() and append_line_if_not_present() (and add more functions there if required).

I would like that, but the problem with prosody’s configuration is that options span over several lines and there is scope since there are sections (e.g. modules_enabled). So an arbitrary line cannot be treated in isolation.

> Also, in the other services, I deliberately avoided completely overwriting the config file. The config file shipped with the Debian package might change in newer versions. I think we more likely want to keep those changes than overwrite them. On the other hand, such changes might also break the service in Tails Server. How do you think we should handle this?

I think you did the right thing, and that should be the preferred approach. I was merely being cheap, but I think that is the right call in this case since the alternative means implementing some subset of Lua’s grammar to be able to parse the config enough to get the scoping of different options/sections.

> > Next, I’ve been fighting with making the tails-server-client side of things smooth. […]
>
> Wow, you really implemented part of the XMPP protocol for this :D I’m sorry you had such trouble with this, and I’m glad you found a solution.

Note that my implementation is (essentially) a copy-paste work from the debug log (which prints the raw XML) of python-xmmp (which I wouldd have preferred to use, but which isn’t compatible with Python 3) when it registers an account while talking to a prosidy server. This was trivial. Trying to use the poorly documented python-nbxmpp was a huge pain in comparison.

> The account creation worked flawlessly for me. I only see a few possible UX pitfalls:
>
> 1. I’m not sure if it will be immediately clear to the user that he has to choose the username and password in the “Connect to Tails Server” dialog himself. This is a new workflow, in the other services all the information in this dialog comes from the onion service admin. I think it would be better if the user was asked for the username and password in a separate dialog that would be started after clicking “Connect”.

Agreed!

> 2. The other services automatically connect the user with all the other users using the service, which is consistent with the vision of Tails Server being primarily for groups who want to connect with each other. With Pidgin, the user still has to manually add other users as buddies before he can chat with them. I think it would be really cool if we could add an option to open a chatroom and make Pidgin automatically join this chatroom. I see that you actually already add a chatroom in prosody, but I think the part on the client side is missing.

I already looked into this. There are two community modules, but neither are in the prosody-modules Debian package:

• mod_default_bookmarks is a perfect solution.
• mod_group_bookmarks actually wouldn’t work for us since the “Support for injecting into ALL users bookmarks” feature is a TODO. :/ So let’s forget about this one.

So this is something we could add in the future, when Debian bug #862097, which I just submitted, is resolved. And there is a backport for Stretch… :)

We could solve it in an ugly way by making it part of the “connection info” as the “Chat rooms” field, and making the service launcher auto-join those channels (if the field isn’t empty). Meh.

> 3. Like you mentioned, Pidgin doesn’t restart to apply the configuration. Maybe we could pop up a dialog asking to restart Pidgin if it is already running? We of course do the same thing for other clients, but I didn’t check yet if other clients we use have the same problem.

Sorry for not making it clearer above: restarting Pidgin is not needed any more now that the account is set up via DBus — I only needed that when I was still experimenting, and the account was added by writing to the XML configuration (which is only read on Pidgin start). So let’s forget about this!

> > … please review’n’merge (into your repos feature/5688-tails-server branch)! :)
>
> Done. The code looks good :)

\o/

> I couldn’t resist some minor refactoring to improve readability in the create-pidgin-xmpp-account script though.

That’s the spirit (and my preferred way of doing my own reviews — if I can refactor it, I probably understand it enough)! :) I’ll try to learn some Python coding standards myself from this! :)

#9 Updated by anonym 2017-05-08 18:59:05

• QA Check changed from Info Needed to Dev Needed

In one of your commit you removed the Pidgin account registration helper script (it seems you were renaming it, and you probably did a mv when you should have done a git me).

segfault wrote:
> I implemented all of those, i.e. (1) we show a separate dialog to get the username and password from the user

I like it! But not if you use the service launcher to connect to an account that already is configured in Pidgin — having to provide the password (twice!) is not so nice, and ideally you shouldn’t even have to provide the user. IMHO:

• Move get_username_and_password() (which contains untranslated strings! please fix!) into the Pidgin account registration helper script.
• We then change this script so it doesn’t take any parameter than the doman and port.
• In the script, we first go through the list of configured accounts, and connect to the first one we find for the given domain.
• If there is no such account, then we call get_username_and_password() and register the account.

> (2) Pidgin automatically joins the chatroom we get via the connection informatoin

This feels a bit ugly to me. I think you’ll agree that mod_default_bookmarks is preferable, right?

> (3) we show a dialog asking the user if we should automatically close Pidgin, if Pidgin is running.

Like I said in my previous post, this shouldn’t be needed. Please revert these things unless you indeed found a reason why this is needed.

#12 Updated by segfault 2017-05-13 21:42:45

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

segfault wrote:
> anonym wrote:
> > Before I went with stop() + start() I tried systemctl reload prosody and it did not reload VirtualHost configuration. :/ So, we cannot rely on this in general.
>
> Oh. That’s too bad :/
>
> > So, yeah, I’m a bit torn here…
>
> Ok. I would have preferred the reload option, but since that doesn’t work, and stopping and starting does feel unclean, I’m ok with changing the order. I added this to my todo list.

I think I found another solution than changing the order. In the commit I just pushed, the private key and onion address are being generated with Python’s cryptography module in configure(). This way the onion address is already known before the service is started. Do you think this is a good solution?

#13 Updated by anonym 2017-05-14 11:37:34

• Assignee changed from anonym to segfault

(It seems you forgot to push the new crypto_util module. Pro tip: git status before each git push :))

It solves the technical problem very nicely, so I like it when only considering that aspect. However, I do worry about us using different code for this than the rest of the Tor network. Are we using exactly the same parameters? How much work will it be to maintain this when Tor changes something about the key (e.g. soon with the next gen onions)?

As long as we can verify that your proposal is “safe”, I’m fine with this as a temporary fix so we don’t have to maintain this code, and monitor for changes in Tor. The real fix should be to ask Tor to add a DontPublish or GenerateKeyOnly flag to ADD_ONION (which requires that KeyType == NEW) and then add a corresponding flag to stem’s convenience functions around ADD_ONION, like create_ephemeral_hidden_service(). Something like this. What do you think?

If you agree, nickm has promised to prioritize Tor tickets tagged with “TailsNeeds”, so you might give that a try. :)

#14 Updated by segfault 2017-05-14 21:40:04

• Assignee changed from segfault to anonym

anonym wrote:
> (It seems you forgot to push the new crypto_util module. Pro tip: git status before each git push :))

Sorry. I pushed it now. Reassigning to you to take a look (it’s not much code).

> It solves the technical problem very nicely, so I like it when only considering that aspect. However, I do worry about us using different code for this than the rest of the Tor network. Are we using exactly the same parameters?

Yes, we use the exact same parameters. I am aware that we don’t want the onion services created by Tails Server to be distinguishable from other onion services, so I took extra care about this.

> How much work will it be to maintain this when Tor changes something about the key (e.g. soon with the next gen onions)?

This will not be much work, especially since the cryptographic parameters in Tor change very rarely.

> As long as we can verify that your proposal is “safe”, I’m fine with this as a temporary fix so we don’t have to maintain this code, and monitor for changes in Tor. The real fix should be to ask Tor to add a DontPublish or GenerateKeyOnly flag to ADD_ONION (which requires that KeyType == NEW) and then add a corresponding flag to stem’s convenience functions around ADD_ONION, like create_ephemeral_hidden_service(). Something like this. What do you think?
>
> If you agree, nickm has promised to prioritize Tor tickets tagged with “TailsNeeds”, so you might give that a try. :)

I agree that this would be better, but I did not assume that Tor would add this feature for this very special use case. But if you think this is realistic, sure, let’s give it a try. I will create a ticket in the Tor tracker.

#15 Updated by segfault 2017-05-16 20:04:00

• QA Check changed from Info Needed to Ready for QA

anonym wrote:
> segfault wrote:
> > I implemented all of those, i.e. (1) we show a separate dialog to get the username and password from the user
>
> I like it! But not if you use the service launcher to connect to an account that already is configured in Pidgin — having to provide the password (twice!) is not so nice, and ideally you shouldn’t even have to provide the user. IMHO:
>
> * Move get_username_and_password() (which contains untranslated strings! please fix!) into the Pidgin account registration helper script.
> * We then change this script so it doesn’t take any parameter than the doman and port.
> * In the script, we first go through the list of configured accounts, and connect to the first one we find for the given domain.
> * If there is no such account, then we call get_username_and_password() and register the account.

I implemented this and also pushed two more commits:

- Instead of failing silently, we now display an error dialog to the user if the Pidgin configuration fails (for example because we can’t connect to the service to create the XMPP account)
- I replaced the “delete onion address” feature with a “generate new onion address” feature, to prevent the case where there is no onion address configured when Prosody is started

Please review :)

#16 Updated by intrigeri 2017-05-31 16:09:42

• Feature Branch changed from feature/12478-tails-server-xmpp to wip/feature/12478-tails-server-xmpp

(Build is broken for ages, Bug #12577 & friends make it a pain to have broken branches on Jenkins.)

#18 Updated by Anonymous 2018-01-16 10:55:11

• related to Feature #12236: Add more services to Tails Server added

#19 Updated by Anonymous 2018-08-17 09:22:39

• Assignee changed from anonym to segfault

@segfault, may you please verify if the build is still broken, fix the branch and set, if possible, a realistic target version? thanks!

#20 Updated by segfault 2018-08-17 20:29:55

• Status changed from In Progress to Resolved
• Assignee deleted (segfault)
• % Done changed from 70 to 100
• QA Check deleted (Ready for QA)

Lets close this, it’s outdated, Prosody is a part of Tails Server since a long time