Bug #8775

Tor Launcher should be able to resolve DNS host names for the proxy configuration

Added by sajolida 2015-01-22 17:26:54 . Updated 2018-03-14 11:08:54 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Tor configuration
Target version:
Tails_3.6
Start date:
2015-01-22
Due date:
% Done:

100%

Feature Branch:
feature/8243-meek
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Tor Launcher
Deliverable for:

Description

As of now, entering a fully-qualified domain name instead of an IP address in Tor Launcher produces a scary error message when trying to connect to Tor: Unacceptable option value: HTTSProxy failed to parse or resolve. Please fix.

This problem is specific to Tails as in Tor Browser outside of Tails, a DNS resolution is done when connecting and so no error message is displayed.

We should make DNS resolution possible for the tor-launcher user.

Files

res.c (1200 B) anonym, 2018-01-16 11:11:53

Subtasks

Related issues

Related to Tails - Feature #8243: Support meek bridges In Progress 2014-11-08

History

#1 Updated by intrigeri 2015-01-22 18:06:41

> A nice way of solving this would be to make it clear that only IP addresses are valid proxy addresses, and to check the validity of the input on that same screen and not at the end of the configuration process.

Is there any Host-like header for the HTTP proxy? In other words, do the HTTP specs allow proxies to behave differently depending on the hostname they’re used as, as opposed to only the IP?

#2 Updated by sajolida 2015-01-23 21:27:45

> Is there any Host-like header for the HTTP proxy? In other words, do
> the HTTP specs allow proxies to behave differently depending on the
> hostname they’re used as, as opposed to only the IP?

This is something I don’t know. But if such a proxy server existed — a
proxy which works if contacted with its hostname information, and not if
contacted with its IP address only — then this proxy wouldn’t work in
Tails anyway as we can’t resolve DNS before connecting to Tor.

#3 Updated by intrigeri 2015-01-23 22:33:57

> But if such a proxy server existed — a proxy which works if contacted with its hostname information, and not if contacted with its IP address only — then this proxy wouldn’t work in Tails anyway as we can’t resolve DNS before connecting to Tor.

Sure, I got this part. I was trying to evaluate if we’re going to enforce a valid constraint, or adjust the GUI for Tails-specific limitations. In the first case, this can go upstream and everything’s fine. In the latter case, it should not go upstream, and we may want to have a (low-priority IMO) ticket about fixing this limitation. Note that fixing the root cause (this limitation) doesn’t sound completely crazy, as we’re already running Tor Launcher as a dedicated user. It might actually not be substantially harder to fix the root cause, than to implement the workaround. Hence my question.

#4 Updated by sajolida 2015-01-25 09:35:01

While wondering whether this was a Tails specific limitation or not, I tried to enter a fully qualified domain name in Tor Launcher outside of Tails and it generated a DNS request. I understand from this that we can’t enforce entering IP addresses in upstream Tor Launcher as:

  • Users might know only the fully qualified domain name of their proxy and not its IP.
  • System administrators might allocate different IPs to the resolution of the same fully qualified domain name.

#5 Updated by intrigeri 2015-02-27 19:41:38

> While wondering whether this was a Tails specific limitation or not, I tried to enter a fully qualified domain name in Tor Launcher outside of Tails and it generated a DNS request. I understand from this that we can’t enforce entering IP addresses in upstream Tor Launcher as:

Agreed. Thanks for testing!

So, next steps are:

  • Would it be acceptable to allow the user Tor Launcher runs as (tor-launcher) to issue DNS queries in the clear?
  • If yes, how hard would be? I’d rather avoid having to go the full Unsafe Browser -style patched-chroot-based solution. Basically we “just” need to run Tor Launcher with a modified NSS hosts resolution depending on the caller’s UID. I’ve seen:
    • We could copy the libc .so file to some Tor Launcher -specific place, binary-edit it to replace /etc/resolv.conf with whatever location we want, and then run Tor Launcher with pointing to where LD_LIBRARY_PATH we’ve put our modified libc; super-hackish, but might be the easiest solution.
    • resolvconf override can do it too — not in Debian, 74 C sloc, LGPL 2.1, no changes since two years but well, it’s tiny
    • resolv_wrapper can override resolv.conf — not in Debian, 1.3k C sloc, BSD 3-clause, actively developed
    • nss_wrapper can override /etc/hosts, but not resolv.conf; in Debian experimental (libnss-wrapper)

#6 Updated by sajolida 2015-02-28 14:59:06

  • Type of work changed from Code to Discuss

Agreed. So this is a discussion now.

#7 Updated by anonym 2015-06-03 20:35:52

  • Subject changed from Tor Launcher should enforce IP addresses when configuring proxy to Tor Launcher should be able to resolve DNS host names for the proxy configuration
  • Description updated
  • Type of work changed from Discuss to Research

During the June 2015 meeting a consensus was reached about adding this capability to a user that already has full access to Tor’s control port (and hence full control of the Tor process e.g. it can learn the external IP address and reconfigure Tor in horrible ways) should not make the situation any worse in case of a compromise.

It remains to find a nice way to do this, with Bug #8775#note-5 already containing some ideas.

#8 Updated by intrigeri 2017-09-14 12:54:52

#9 Updated by intrigeri 2017-11-19 14:04:59

  • Type of work changed from Research to Code

> * Would it be acceptable to allow the user Tor Launcher runs as (tor-launcher) to issue DNS queries in the clear?

I suspect the DNS request is actually sent by tor (or whatever pluggable transport plugin is configured), not by Tor Launcher itself. This does not affect the agreement we’ve already found on this topic though.

> resolv_wrapper can override resolv.conf — not in Debian, 1.3k C sloc, BSD 3-clause, actively developed

This one is now in Stretch and seems to be our best bet here. See config/chroot_local-includes/usr/local/sbin/unsafe-browser and config/chroot_local-includes/etc/NetworkManager/dispatcher.d/00-save-env wrt. how to get the nameserver.

So I think there’s a clear path forward :)

#10 Updated by anonym 2018-01-16 12:09:15

  • File res.c added
  • Assignee set to intrigeri
  • QA Check set to Info Needed

intrigeri wrote:
> > resolv_wrapper can override resolv.conf — not in Debian, 1.3k C sloc, BSD 3-clause, actively developed
>
> This one is now in Stretch and seems to be our best bet here. See config/chroot_local-includes/usr/local/sbin/unsafe-browser and config/chroot_local-includes/etc/NetworkManager/dispatcher.d/00-save-env wrt. how to get the nameserver.
>
> So I think there’s a clear path forward :)

tl;dr: at least on Debian Stretch and Sid, resolv_wrapper doesn’t override getaddrinfo() (and gethostbyname()) so it won’t work for us as-is since tor uses getaddrinfo() for the resolution this ticket is about.

I optimistically tried to implement a PoC for Feature #8243 (wip/8243-meek) but couldn’t get it to work so I started digging. It seems resolv_wrapper only overrides libc’s res_n{init,query,search,close}() functions — I guess an assumption is that other name resolution methods in libc like getaddrinfo() (used by tor) and gethostbyname*() are implemented using the res_n*() primitives, so that they are covered too. But my test case (attached) seems to indicate otherwise:

echo "nameserver 8.8.8.8" > resolv.conf
gcc res.c -lresolv -o res
LD_PRELOAD=libresolv_wrapper.so RESOLV_WRAPPER_CONF=${PWD}/resolv.conf RESOLV_WRAPPER_DEBUGLEVEL=4 ./res

In short, the program generates a random $string, and tries to resolve getaddrinfo.$string, gethostbyname.$string and resnquery.$string with the respective method, so one can see which methods are overridden by resolv_wrapper by looking in a network dump which DNS server was used for each hostname’s resolution. (Edit: the deal with the random string is just so that each run tries unique hostnames, so we can disregard the DNS cache.)

But actually enabling debugging (RESOLV_WRAPPER_DEBUGLEVEL) is enough: it shows that only res_nquery() was overridden (also verified by looking at the network dump), and the other methods uses the system resolver. Damn. Indeed, when I look a glibc’s sources, it seems like gethostbyname() and getaddrinfo() are implemented independently of res_nquery(), but don’t take my word for it — that code is way above my C skills.

intrigeri, since this isn’t my comfort zone, couldn’t you validate my reasoning above? If you agree I guess we should report a bug against the Debian package, since it is not working with some other packages (I dare say “most” — “getaddrinfo” and “gethostbyname” has (combined) 5x more hits on Debian code search than res_nquery), and try to get a patch in which also overrides getaddrinfo() and gethostbyname*() (by implementing them with the res_n*() primitives). What do you think? Any other approach or something I’ve missed or misunderstood?

#11 Updated by intrigeri 2018-01-16 14:36:20

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

> intrigeri, since this isn’t my comfort zone, couldn’t you validate my reasoning above?

I’ve double-checked your findings. resolv_wrapper source code & test suite do confirm that only res_n* are wrapped. I didn’t check whether getaddrinfo uses the functions wrapped by resolv_wrapper but it all makes sense to me, and I’ve seen a discussion on the upstream bug tracker about fixing the manpage to make it use in examples a program that actually uses one of the wrapped functions instead of something else.

> If you agree I guess we should report a bug against the Debian package, since it is not working with some other packages (I dare say “most” — “getaddrinfo” and “gethostbyname” has (combined) 5x more hits on Debian code search than res_nquery),

I say skip the Debian proxy and head directly to the upstream bug tracker. Now, this limitation is so obvious it’s hard to believe it’s not on purpose. Perhaps they want to scope their project to something that works for Samba but were nice enough to release it separately.

> and try to get a patch in which also overrides getaddrinfo() and gethostbyname*() (by implementing them with the res_n*() primitives).

I’m not sure this is worth our time and I don’t think it can work in time for Tails 3.5, even if upstream is open to it.

So at this point I think that https://github.com/hadess/resolvconf-override will be vastly cheaper.

#12 Updated by anonym 2018-01-17 11:20:48

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

intrigeri wrote:
> > intrigeri, since this isn’t my comfort zone, couldn’t you validate my reasoning above?
>
> I’ve double-checked your findings. resolv_wrapper source code & test suite do confirm that only res_n* are wrapped. I didn’t check whether getaddrinfo uses the functions wrapped by resolv_wrapper but it all makes sense to me, and I’ve seen a discussion on the upstream bug tracker about fixing the manpage to make it use in examples a program that actually uses one of the wrapped functions instead of something else.
>
> > If you agree I guess we should report a bug against the Debian package, since it is not working with some other packages (I dare say “most” — “getaddrinfo” and “gethostbyname” has (combined) 5x more hits on Debian code search than res_nquery),
>
> I say skip the Debian proxy and head directly to the upstream bug tracker. Now, this limitation is so obvious it’s hard to believe it’s not on purpose. Perhaps they want to scope their project to something that works for Samba but were nice enough to release it separately.

Probably — technically resolv_wrapper is overriding all the res_n*() functions in resolv.h, but ignores the other DNS lookup methods in netdb.h (like gethostbyname() and getaddrinfo()) so that makes total sense.

> > and try to get a patch in which also overrides getaddrinfo() and gethostbyname*() (by implementing them with the res_n*() primitives).
>
> I’m not sure this is worth our time and I don’t think it can work in time for Tails 3.5, even if upstream is open to it.

You (implicitly) assumes that this is going into Tails 3.5, a bugfix release scheduled next week. Did you mean Tails 3.6? (This is the only reason why I assigned this ticket to you as Info Needed.)

> So at this point I think that https://github.com/hadess/resolvconf-override will be vastly cheaper.

I compiled it with:

gcc -Wall -fpic -shared -ldl -o libresolvconf_override.so resolvconf-override.c && \


and ran my test again:

LD_PRELOAD=libresolvconf_override.so NAMESERVER1=8.8.8.8 ./res


and none of the methods used the provided name server.

Looking at the sources, it does the magic in __res_init(), so it relies on a libc where gethostbyname() and getaddrinfo() are implemented using __res_init(). Apparently this hack isn’t even compatible with glibc’s res_n{init,query}(), so this library is just worse for us. :/ Seems like resolv_wrapper is the better bet, after all.

I’m gonna do some out-of-the-box thinking…

#13 Updated by intrigeri 2018-01-17 11:55:49

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

> You (implicitly) assumes that this is going into Tails 3.5, a bugfix release scheduled next week.

Yes. I thought you were working on this because we probably have to do Feature #15064 in time for Tails 3.5: my understanding was that Feature #15064 requires Feature #8243 that in turn depends on this very ticket (unless there’s another way to get these done but so far I’ve not seen any realistic proposal, so less than a week before the deadline, I was assuming that you would pick the option that was researched already, and my assumption was reinforced by seeing you work on this ticket). I know I’m assuming a lot here but given there’s been no update on Feature #15064 for a month, and nothing new added to Feature #8243 after your work session with iry, I’m in a position where I have to do some guesswork.

#14 Updated by anonym 2018-01-17 17:26:08

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

anonym wrote:
> I’m gonna do some out-of-the-box thinking…

I have a PoC based on a mount namespaces hack, and the whole idea can be summarized as the command that is used to wrap around tor to override its (and all child processes’, like obfs4proxy, but no other process’) DNS resolver:

/usr/bin/unshare --mount sh -c "mount --bind /etc/resolv.conf.clearnet /etc/resolv.conf && /usr/bin/tor "${@}"


You’ll find it in the wip/8243-meek branch, which actually makes meek work just like we want it, woo! However, there are some serious problems, all because we wrap unshared around tor in tordefault.service@ by overriding ExecStart:

  • I’ve had to add the dreaded CAP_SYS_ADMIN to CapabilityBoundingSet. I believe there’s no other way if we have to invoke unshare -m via ExecStart. :/
  • I’ve had to disable AppArmor because I cannot find a mount rule that allows the necessary mount operation. intrigeri, my AppArmor guru, did I hit a bug or weird edge case, or can my current best guess in the PoC be fixed?

The best would be if we could skip wrapping with unshare somehow. intrigeri, my systemd bodhisattva, do you know if I can get the same functionality as unshared -m (i.e. unshare the mount namespace from the parent process and thus the rest of the system) via some fancy systemd feature?

If we can fix this (or the AppArmor issue; imho we can live with CAP_SYS_ADMIN in bridge mode since we any way disable tor’s own sandbox), we’re good to go (even for Tails 3.5, if needed). Well, I suppose we may want to revisit our decision to not show any default bridges in Tor Launcher for Feature #8243, but I don’t think it is a blocker.

intrigeri wrote:
> > You (implicitly) assumes that this is going into Tails 3.5, a bugfix release scheduled next week.
>
> Yes. I thought you were working on this because we probably have to do Feature #15064 in time for Tails 3.5: my understanding was that Feature #15064 requires Feature #8243 that in turn depends on this very ticket (unless there’s another way to get these done but so far I’ve not seen any realistic proposal, so less than a week before the deadline, I was assuming that you would pick the option that was researched already, and my assumption was reinforced by seeing you work on this ticket). I know I’m assuming a lot here but given there’s been no update on Feature #15064 for a month, and nothing new added to Feature #8243 after your work session with iry, I’m in a position where I have to do some guesswork.

I’ll look into this tomorrow.

#15 Updated by intrigeri 2018-01-17 21:59:04

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

> * I’ve had to disable AppArmor because I cannot find a mount rule that allows the
> necessary mount operation. intrigeri, my AppArmor guru, did I hit a bug or weird edge case, or can my current best guess in the PoC be fixed?

That’s a kernel bug, fixed in Linux 4.14.13 (https://bugs.debian.org/883703). So Bug #15148 should help.

> The best would be if we could skip wrapping with unshare somehow. intrigeri, my systemd bodhisattva, do you know if I can get the same functionality as unshared -m (i.e. unshare the mount namespace from the parent process and thus the rest of the system) via some fancy systemd feature?

Not by heart, sorry, but I’d be surprised if there was no overlay feature or similar. I would need to dive into the doc. Feel free to reassign to me if/once you’ve searched for it and failed.

#16 Updated by intrigeri 2018-01-18 06:21:53

>> * I’ve had to disable AppArmor because I cannot find a mount rule that allows the necessary mount operation. intrigeri, my AppArmor guru, did I hit a bug or weird edge case, or can my current best guess in the PoC be fixed?

> That’s a kernel bug, fixed in Linux 4.14.13 (https://bugs.debian.org/883703). So Bug #15148 should help.

I should have been clearer, sorry. Our current pinned feature set disables mediation of mount operations so there’s no chance you’ll write a rule that allows them: they should all be allowed already. But due to that kernel bug they’re instead all denied (before Linux 4.14.13). See Feature #14976#note-32 for more info.

> Not by heart, sorry, but I’d be surprised if there was no overlay feature or similar.

I remembered BindPaths= (and its BindReadOnlyPaths= friend) while waking up today :)
Looks like exactly what we want, without having to do deal with namespaces with unshare outside of systemd’s view, no?

#17 Updated by anonym 2018-01-18 20:51:13

  • Status changed from Confirmed to In Progress
  • Assignee changed from anonym to intrigeri
  • Target version set to Tails_3.6
  • % Done changed from 0 to 40
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch set to feature/8243-meek

See Feature #8243#note-24.

#18 Updated by intrigeri 2018-01-19 11:09:08

  • Assignee changed from intrigeri to anonym
  • % Done changed from 40 to 50
  • QA Check changed from Ready for QA to Dev Needed

Review done on Feature #8243. One ticket on my plate will be enough for me to track the remaining question I need to answer, so reassigning this one to you so you can fix the issues I’ve raised there without blocking on that answer.

#19 Updated by anonym 2018-01-26 12:33:32

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

#20 Updated by intrigeri 2018-01-30 08:06:28

  • Assignee changed from intrigeri to bertagaz

#21 Updated by bertagaz 2018-02-24 19:03:25

  • Status changed from In Progress to Fix committed
  • Assignee deleted (bertagaz)
  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

Merged together with Feature #8243.

#22 Updated by bertagaz 2018-03-14 11:08:54

  • Status changed from Fix committed to Resolved