Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tor support #1471

Merged
merged 26 commits into from
May 10, 2018
Merged

Conversation

rustyrussell
Copy link
Contributor

OK, this is @Saibato 's work, rebased on top of the new option stuff (#1450), and cleaned up to do sync negotiation and fix a few bugs.

I haven't tested it since reworking it, and I haven't tested v3 at all (Ubuntu only supports v2 AFAICT), but I'm posting here because this is the culmination of the network options. The final patch here will be to actually document them :)

No doubt this will get some CI failures, which seem to haunt us at the moment...

@Saibato
Copy link
Contributor

Saibato commented May 7, 2018

Only minor nits. To get a first run
I think gossip.h needs to be created in gossipd/

And common/tor.o needs to be removed from gossipd/Makefile

with this changes it runs out of the box.

Auto V2 already worked in regest .
Even V3 worked manually with --announce-addr=

There are probably some minor changes for port assignment other then 9735 necessary.
cool work @rustyrussell. Thanks for your rework 👍

Auto hidden V3 will come when the next tor release is ready.
Up till then V3 has to be edited by hand in /etc/torrc

new file mode 100644
@@ -0,0 +1,11 @@
+#ifndef LIGHTNING_GOSSIPD_GOSSIP_H
+#define LIGHTNING_GOSSIPD_GOSSIP_H
+#include "config.h"
+#include <stdbool.h>
+
+struct io_conn;
+struct reaching;
+
+struct io_plan *connection_out(struct io_conn *conn, struct reaching reach);
+
+#endif /
LIGHTNING_GOSSIPD_GOSSIP_H */

@@ -55,11 +55,10 @@ GOSSIPD_COMMON_OBJS :=
common/pseudorand.o
common/status.o
common/status_wire.o
common/subdaemon.o
common/timeout.o \

  • common/tor.o
    common/type_to_string.o
    common/utils.o
    common/utxo.o
    common/version.o
    common/wireaddr.o \

@Saibato
Copy link
Contributor

Saibato commented May 7, 2018

hi @ZmnSCPxj thank's for your patience with my laziness and your co work
at
https://github.com/Saibato/lightning/tree/rusty-tor-works
is a working and compile able tree were i will push changes that might be needed

@rustyrussell i assume
unless the latest ccan is not in master i guess the real CI 👧 will stop after source check.

Though i dropped all my job's to have full time for blockchain & co
I plan to dig deep in LN and learn as fast and much as i can, so maybe one day I can make such a big and clean PR by myself.

@Saibato
Copy link
Contributor

Saibato commented May 7, 2018

hi @rustyrussell i think we should disable for now dnsseed @cdecker only in this tor PR.
so even when we test, we won't leak by routeable dns calls,
or warn to be carefull with testing tor onion until this PR leaves the [WIP] status

I/we will/should fix this possibly leak by wrapping all getaddrinfo to route over the tor proxy if tor is enabled or not resolve extern hostname at all

2e75791

@rustyrussell
Copy link
Contributor Author

Good point about DNS. Someone has to figure out how to do DNS lookups over socks5, but disabling is easiest for now.

@rustyrussell rustyrussell force-pushed the guilt/tor-reworked branch 2 times, most recently from a0da76c to 8e82806 Compare May 8, 2018 07:13
@rustyrussell
Copy link
Contributor Author

OK, fixes thanks to @Saibato and more option tweaks. We now have '--addr=autotor:' for automatic Tor addresses, which theoretically means we can connect to multiple Tor instances and get addresses from all of them (though they'd have to use the same --tor-password or cookie auth).

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 8, 2018

Someone has to figure out how to do DNS lookups over socks5, but disabling is easiest for now.

The general recommendation is "just pass the hostname to the socks5 Tor proxy", which under Tor will securely use the DNS of your exit node. DNS is a UDP protocol which cannot be tunneled under Tor.

https://tor.stackexchange.com/questions/8/how-does-tor-route-dns-requests

If somebody uses a DNS hostname in connect command, or we have to fallback to DNS seed, under Tor we should pass the hostname directly to the socks5 proxy and let Tor handle the DNS lookup that way. We will not be able to learn the IPv4/IPv6 address of the destination, though.

@rustyrussell
Copy link
Contributor Author

For the connect command, we could pass the hostname string through to gossipd, and have it to the lookup?

Using the seed is harder, we'd need to TCP connect to port 53 over Tor and do DNS manually.

For the moment, Just Say No :)

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 8, 2018

Using the seed is harder, we'd need to TCP connect to port 53 over Tor and do DNS manually.

Not really? SOCKS5 supports a DOMAINNAME lookup in the ATYP (address type) field. In the case of DNS seed, it seems we just need to prepend ln1<pubkey-bech32> to .lseed.bitcoinstats.com. If we can pass the hostname of connect to gossipd and have that use the DOMAINNAME lookup in the ATYP of the SOCKS5 proxy of Tor, then we can also pass the ln1<pubkey-bech32>.lseed.bitcoinstats.com hostname for DNS seed lookup to the same code.


Edit:

This is how we do DNS seed lookups:

lightning/gossipd/gossip.c

Lines 1923 to 1932 in d40d22b

pubkey_to_der(der, id);
bech32_push_bits(&data, der, PUBKEY_DER_LEN*8);
bech32_encode(bech32, "ln", data, tal_count(data), sizeof(bech32));
addr = tal_fmt(ctx, "%s.lseed.bitcoinstats.com", bech32);
status_trace("Resolving %s", addr);
a = tal(ctx, struct addrhint);
a->addr.itype = ADDR_INTERNAL_WIREADDR;
if (!wireaddr_from_hostname(&a->addr.u.wireaddr, addr, port, NULL)) {

This is how we do hostname lookups for connect:

lightning/common/wireaddr.c

Lines 311 to 313 in 79902b4

/* Resolve with getaddrinfo */
if (!res)
res = wireaddr_from_hostname(addr, ip, port, err_msg);

In both cases we just do wireaddr_from_hostname.

Under Tor, instead of doing that, we can "just" do it by directly using DOMAINNAME as ATYP for the CONNECT request while connecting to the peer.

@Saibato
Copy link
Contributor

Saibato commented May 8, 2018

dns lookup can easily be done by just passing a domainname instead of an IP string to the exiting tor-socks5-proxy this relays the call to the exit node.

we pass in gossipd/tor.c
reach_tor->host = fmt_wireaddr_without_port(reach_tor, addr);

we can change frmt_wire to return us just this name
addr in first places comes from an addrhint type.
That we already fill with the bech32 format string in seed_resolve_addr
instead of doing the resolve there when tor enabled we just use tor with that unresolved name.

max addrlen must then be 255 which is the MAX_FQDN_LEN

The rest just work as is.
Instead of an IP the bech32 domanname will be in the DB wich is usefull anyway because even without using tor
the name will be resolved by the normal wireaddr_from_hostname(.. call with getaddrinfo(...

I will make some working code in my mirror PR.
You can then cherrypick
give me 1 day

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 8, 2018

Just to be clear, fine by me to Just Say No to DNS under Tor for now. I just find it strange to support connect DNS under Tor without supporting DNS seed under Tor, since the effort should be similar and similar code paths can be taken.

@Saibato
Copy link
Contributor

Saibato commented May 8, 2018

yes we should not enable if we not all agree.
With tor we won't want to mess.
All has to be carefully drafted 👍

Maybe we can do an extra resolve over Tor function if we don't like DNS names pop up in DB

@Saibato
Copy link
Contributor

Saibato commented May 8, 2018

@cdecker socks5 domain querry supports AFAIK A records.
Do you plan to use SRV records with the seeds?

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 8, 2018

An alternative to SOCKS5 is to use SOCKS4a RESOLVE request with Tor to resolve a hostname from a Tor exit node.

Then we can have a "drop-in" replacement for wireaddr_from_hostname, which reduces the code changes.

https://gitweb.torproject.org/tor.git/tree/src/tools/tor-resolve.c

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 8, 2018

@Saibato re: SRV, yes: https://github.com/lightningnetwork/lightning-rfc/blob/master/10-dns-bootstrap.md

We currently only use A via gethostbyname, though.

@Saibato
Copy link
Contributor

Saibato commented May 8, 2018

oh this might not work over our DOMAIN call we use now.
I have to think about that...

@Saibato
Copy link
Contributor

Saibato commented May 8, 2018

i see no other way, we might have to use direct DNS over socks for other then A.
This is no big deal, this can be done over socks, but not implemented yet.

I don't really like that, because it's so predictable and correct me if i am wrong, like waving a big flag.
On the other hand, tor is just an elegant way to connect and listen behind a NAT.
That might be the dominant use case without the need for any additional privacy than that they already have from the LN onion's. 😄

@ZmnSCPxj
when using tor we should remind users to stay in the .onion space and route to real IP only over an c-ln (gateway) that builds for in and out a cyclic superhub as you once proposed.

Because LN is transport protocol independent those nodes need only one real IP as @icota will like we could even use smoke or sneakers.

@Saibato
Copy link
Contributor

Saibato commented May 8, 2018

Please take look at https://github.com/Saibato/lightning/tree/fix-dns-seed
and commit f897db0 this is wip but feel free to cherrypick

I tested so far and basic connect and funding worked.

what i now would need to test more is my own dns seed entry at lseed.bitcoinstats.com? ;-)

@rustyrussell
Copy link
Contributor Author

rustyrussell commented May 9, 2018

I like the idea for handing things though to the proxy to resolve, BUT we need to do the hard work of adding a new internal type rather than overriding PADDING (which has a specific spec meaning!).

Looks like the RESOLVE extension is what we want, or we can just hand the seed name, thanks @ZmnSCPxj !

Let's open a separate PR for that? I have to rebase this one again because of CI issues...

@Saibato
Copy link
Contributor

Saibato commented May 9, 2018

the proxy returns the IP even now we have the resolver already I just had no need for it,
we can fill a wireaddr struct in reach and return.

@Saibato
Copy link
Contributor

Saibato commented May 9, 2018

instead of PADDING we can use any number or define a new intern one its only used inside proxy
with this we have resolve names for tor and ip

@Saibato
Copy link
Contributor

Saibato commented May 9, 2018

but , wireaddr is often on wire and bandwide and bytesaving is important so the shorter the better. but those long names exist and i also think we should always save and use the shortest form for anything.

so we need to get the ip for the seed out of proxy call and save it instead of the seed in DB? is that the plan?

@rustyrussell rustyrussell changed the title WIP: Tor support Tor support May 9, 2018
@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 9, 2018

Looks like the RESOLVE extension is what we want, or we can just hand the seed name, thanks @ZmnSCPxj !

Let's open a separate PR for that? I have to rebase this one again because of CI issues...

You are welcome. Yes, I suggest new PR instead.

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 9, 2018

so we need to get the ip for the seed out of proxy call and save it instead of the seed in DB? is that the plan?

Yes. The simplest without having to add a new wireadddr_internal subtype is to implement a gethostbbyname_tor and select whether to call that or gethostbyname depending on whether everything-on-Tor is enabled or not.

(I suggest separating the concept of "you can use this Tor proxy if given an onion address" vs "Always use the Tor proxy": for instance SLEEPYARK might have a stable public IP address that it would be pointless to hide (and passing everything via Tor will only slow things down) but perhaps individual employees of Blockstream who receive their salary onLightning might want to keep their IP address private and use onion-addressed LN nodes. SLEEPYARK would then use the faster public Internet mechanisms for most operations, and only use Tor to connect to its employees.

@Saibato
Copy link
Contributor

Saibato commented May 9, 2018

Made some changes to make a direct 0xF0 socks5 call (resolve) request without connecting
cba42d4#diff-ded1bfec15ab21ef9b1786b4f0236060
just as info this works as resolver and can give us an IP for seed before we make the real connection in gossip

more change is needed. we can call this 'gethostbbyname_tor' or make the existing have a parameter to just return an struct wireaddr

We don't actually use it, but let's fix it anyway.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
All gossipd needs from common/tor is do_we_use_tor_addr(), so move
that and the rest of the tor-specific handshake code into gossip/tor.c

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is simply the code to set up the automatic hidden service, so move
it into lightningd.

I removed the undefined parse_tor_wireaddr, and added a parameter name
to the create_tor_hidden_service_conn() declaration for update-mocks.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a simple helper for dealing with buffered I/O.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's no reason to do this async, and far easier to follow using normal
read/write.

The previous parsing was deeply questionable, using substring searches
only, and relying on the fact that a single non-blocking read would get
the entire response.  This is changed to do (somewhat) proper parsing
using ccan/rbuf.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rename tor_proxyaddrs and tor_serviceaddrs to tor_proxyaddr and tor_serviceaddr:
the 's' at the end suggests that there can be more than one.

Make them NULL or non-NULL, rather than using all-zero if unset.

Hand them the same way to gossipd; it's a bit of a hack since we don't
have optional fields, so we use a counter which is always 0 or 1.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This variable does NOT include the 2 bytes for the port.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Avoids duplicate code.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And use it in wireaddr.

We fix up the double '.onion' in the test case, which seems like an error?

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We might still be offered password authentication, for example.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Instead of storing a wireaddr and converting to an addrinfo every
time, just convert once (which also avoids the memory leak in the
current code).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's usually for Tor, but we can use a socks5 proxy without it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Only force proxy use if we don't announce any non-TOR address.
   There's no option to turn it off, so this makes more sense.
2. Don't assume we want an IPv4 socket to reach proxy, use the family
   from the struct addrinfo.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Risks leakage.  We could do lookup via the proxy, but that's a TODO.

There's only one occurance of getaddrinfo (and no gethostbyname), so
we add a flag to the callers.

Note: the use of --always-use-proxy suppresses *all* DNS lookups, even
those from connect commands and the command line.

FIXME: An implicit setting of use_proxy_always is done in gossipd if it
determines that we are announcing nothing but Tor addresses, but that
does *not* suppress 'connect'.

This is fixed in a later patch.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Currently it's always for messages to peer: make that status_peer_io and
add a new status_io for other IO.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For the moment, this is a straight handing of current parameters through
from master to the gossip daemon.  Next we'll change that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This takes the Tor service address in the same option, rather than using
a separate one.  Gossipd now digests this like any other type.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means it will effect connect commands too (though it's too
late to stop DNS lookups caused by commandline options).

We also warn that this is one case where we allow forcing through Tor
without a proxy set: it just means all connections will fail.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Currently the Tor port for autotor is always 9735; the --port option
is deprecated, though we could append an optional '[@PortNum]' to
'autotor' if people want it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@ZmnSCPxj
Copy link
Collaborator

ACK 894defd

A little, anyway. A few nits and things I want to add, but this PR is big enough already... We can always add follow-up PRs

@rustyrussell rustyrussell merged commit 2bc9d75 into ElementsProject:master May 10, 2018
@rustyrussell rustyrussell deleted the guilt/tor-reworked branch May 10, 2018 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants