diff --git a/chanrestore.go b/chanrestore.go index 407cdfbc7ad..39d55557644 100644 --- a/chanrestore.go +++ b/chanrestore.go @@ -337,6 +337,11 @@ func (s *server) ConnectPeer(nodePub *btcec.PublicKey, addrs []net.Addr) error { "with chan restore", nodePub.SerializeCompressed()) } + // Strip persisted Tor v2 .onion entries that may have been carried + // over in an old static channel backup: Tor stopped serving v2 in 2021 + // and the dial would never succeed. Covered by TestWithoutV2Onion. + addrs = withoutV2Onion(addrs) + // For each of the known addresses, we'll attempt to launch a // persistent connection to the (pub, addr) pair. In the event that any // of them connect, all the other stale requests will be canceled. diff --git a/config.go b/config.go index e3d8850e581..bffebc36973 100644 --- a/config.go +++ b/config.go @@ -82,7 +82,6 @@ const ( defaultTorDNSHost = "soa.nodes.lightning.directory" defaultTorDNSPort = 53 defaultTorControlPort = 9051 - defaultTorV2PrivateKeyFilename = "v2_onion_private_key" defaultTorV3PrivateKeyFilename = "v3_onion_private_key" // defaultZMQReadDeadline is the default read deadline to be used for @@ -1239,41 +1238,22 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser, return nil, mkErr(str) } - switch { - case cfg.Tor.V2 && cfg.Tor.V3: - return nil, mkErr("either tor.v2 or tor.v3 can be set, " + - "but not both") - case cfg.DisableListen && (cfg.Tor.V2 || cfg.Tor.V3): + if cfg.DisableListen && cfg.Tor.V3 { return nil, mkErr("listening must be enabled when enabling " + "inbound connections over Tor") } - if cfg.Tor.PrivateKeyPath == "" { - switch { - case cfg.Tor.V2: - cfg.Tor.PrivateKeyPath = filepath.Join( - lndDir, defaultTorV2PrivateKeyFilename, - ) - case cfg.Tor.V3: - cfg.Tor.PrivateKeyPath = filepath.Join( - lndDir, defaultTorV3PrivateKeyFilename, - ) - } + if cfg.Tor.PrivateKeyPath == "" && cfg.Tor.V3 { + cfg.Tor.PrivateKeyPath = filepath.Join( + lndDir, defaultTorV3PrivateKeyFilename, + ) } - if cfg.Tor.WatchtowerKeyPath == "" { - switch { - case cfg.Tor.V2: - cfg.Tor.WatchtowerKeyPath = filepath.Join( - cfg.Watchtower.TowerDir, - defaultTorV2PrivateKeyFilename, - ) - case cfg.Tor.V3: - cfg.Tor.WatchtowerKeyPath = filepath.Join( - cfg.Watchtower.TowerDir, - defaultTorV3PrivateKeyFilename, - ) - } + if cfg.Tor.WatchtowerKeyPath == "" && cfg.Tor.V3 { + cfg.Tor.WatchtowerKeyPath = filepath.Join( + cfg.Watchtower.TowerDir, + defaultTorV3PrivateKeyFilename, + ) } // Set up the network-related functions that will be used throughout diff --git a/config_test.go b/config_test.go index e287b59db63..0233a27f185 100644 --- a/config_test.go +++ b/config_test.go @@ -26,14 +26,12 @@ func TestConfigToFlatMap(t *testing.T) { // Set deprecated fields. cfg.Bitcoin.Active = true - cfg.Tor.V2 = true result, deprecated, err := configToFlatMap(cfg) require.NoError(t, err) // Check that the deprecated option has been parsed out. require.Contains(t, deprecated, "bitcoin.active") - require.Contains(t, deprecated, "tor.v2") // Pick a couple of random values to check. require.Equal(t, DefaultLndDir, result["lnddir"]) diff --git a/discovery/bootstrapper.go b/discovery/bootstrapper.go index 43e9d5ec2db..28ef0d920be 100644 --- a/discovery/bootstrapper.go +++ b/discovery/bootstrapper.go @@ -218,12 +218,24 @@ func (c *ChannelGraphBootstrapper) SampleNodeAddrs(_ context.Context, return nil } + foundAddr := false for _, nodeAddr := range node.Addrs() { // If we haven't yet reached our limit, then // we'll copy over the details of this node // into the set of addresses to be returned. - switch nodeAddr.(type) { - case *net.TCPAddr, *tor.OnionAddr: + switch onion := nodeAddr.(type) { + case *net.TCPAddr: + case *tor.OnionAddr: + // Skip persisted Tor v2 .onion + // entries: Tor stopped serving them + // in 2021 and the dial would never + // succeed. Other addresses of the + // same node may still be usable. + if len(onion.OnionService) == + tor.V2Len { + + continue + } default: // If this isn't a valid address // supported by the protocol, then we'll @@ -245,9 +257,14 @@ func (c *ChannelGraphBootstrapper) SampleNodeAddrs(_ context.Context, IdentityKey: nodePub, Address: nodeAddr, }) + foundAddr = true } - return errFound + if foundAddr { + return errFound + } + + return nil }, func() { clear(a) }) diff --git a/discovery/bootstrapper_test.go b/discovery/bootstrapper_test.go new file mode 100644 index 00000000000..299368ca5c1 --- /dev/null +++ b/discovery/bootstrapper_test.go @@ -0,0 +1,143 @@ +package discovery + +import ( + "context" + "net" + "testing" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/lightningnetwork/lnd/autopilot" + "github.com/lightningnetwork/lnd/tor" + "github.com/stretchr/testify/require" +) + +// stubNode is a minimal autopilot.Node implementation used to drive +// ChannelGraphBootstrapper from a unit test. +type stubNode struct { + pub *btcec.PublicKey + addrs []net.Addr +} + +func (n *stubNode) PubKey() [33]byte { + var out [33]byte + copy(out[:], n.pub.SerializeCompressed()) + return out +} + +func (n *stubNode) Addrs() []net.Addr { return n.addrs } + +// stubChannelGraph yields a fixed list of nodes from ForEachNode and is +// otherwise unused by SampleNodeAddrs. +type stubChannelGraph struct { + nodes []autopilot.Node +} + +func (s *stubChannelGraph) ForEachNode(ctx context.Context, + cb func(context.Context, autopilot.Node) error, _ func()) error { + + for _, n := range s.nodes { + if err := cb(ctx, n); err != nil { + return err + } + } + + return nil +} + +func (s *stubChannelGraph) ForEachNodesChannels(_ context.Context, + _ func(context.Context, autopilot.NodeID, + []*autopilot.ChannelEdge) error, _ func()) error { + + return nil +} + +// TestGraphBootstrapperSkipsV2Onion ensures SampleNodeAddrs strips Tor v2 +// .onion entries from the returned candidate set so the connection manager +// never attempts to dial an obsolete v2 hidden service surfaced through the +// channel graph. A node whose remaining addresses include a v3 .onion or +// plain TCP entry is still returned for those addresses, and a node whose +// only address is v2 contributes nothing. +func TestGraphBootstrapperSkipsV2Onion(t *testing.T) { + t.Parallel() + + v2 := &tor.OnionAddr{ + OnionService: "3g2upl4pq6kufc4m.onion", + Port: 9735, + } + v3 := &tor.OnionAddr{ + OnionService: "4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnz" + + "jpbi7utijcltosqemad.onion", + Port: 9735, + } + tcp := &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 9735} + + mkPub := func(t *testing.T) *btcec.PublicKey { + t.Helper() + priv, err := btcec.NewPrivateKey() + require.NoError(t, err) + + return priv.PubKey() + } + + mixedPub := mkPub(t) + v2OnlyPub := mkPub(t) + + graph := &stubChannelGraph{ + nodes: []autopilot.Node{ + &stubNode{ + pub: mixedPub, + addrs: []net.Addr{v2, v3, tcp}, + }, + &stubNode{ + pub: v2OnlyPub, + addrs: []net.Addr{v2}, + }, + }, + } + + // Use deterministic sampling so the hash accumulator never skips a + // node — every eligible address must surface on each call. + bs, err := NewGraphBootstrapper(graph, true) + require.NoError(t, err) + + // Ask for more addresses than the graph holds so the bootstrapper + // drains it in a single pass. + got, err := bs.SampleNodeAddrs( + t.Context(), 10, map[autopilot.NodeID]struct{}{}, + ) + require.NoError(t, err) + + // Collect (pubkey, address) pairs we expect — the v2-only node must + // not contribute any NetAddress, and the mixed node contributes only + // v3 and tcp. + type seenAddr struct { + pub [33]byte + addr string + } + seen := make(map[seenAddr]struct{}, len(got)) + for _, na := range got { + var p [33]byte + copy(p[:], na.IdentityKey.SerializeCompressed()) + seen[seenAddr{pub: p, addr: na.Address.String()}] = struct{}{} + } + + var mixedKey, v2OnlyKey [33]byte + copy(mixedKey[:], mixedPub.SerializeCompressed()) + copy(v2OnlyKey[:], v2OnlyPub.SerializeCompressed()) + + // Mixed node yields v3 and tcp entries. + require.Contains(t, seen, seenAddr{ + pub: mixedKey, addr: v3.String(), + }) + require.Contains(t, seen, seenAddr{ + pub: mixedKey, addr: tcp.String(), + }) + + // No v2 entry surfaces, for either node. + for s := range seen { + require.NotEqual(t, v2.String(), s.addr, + "v2 onion %v leaked into candidate set", s.addr) + require.NotEqual(t, v2OnlyKey, s.pub, + "v2-only node %x should contribute nothing", s.pub) + } +} diff --git a/docs/configuring_tor.md b/docs/configuring_tor.md index 20584124cb9..8b166c6754e 100644 --- a/docs/configuring_tor.md +++ b/docs/configuring_tor.md @@ -15,13 +15,13 @@ by using Tor for anonymous networking to establish connections. With widespread usage of Onion Services within the network, concerns about the difficulty of proper NAT traversal are alleviated, as usage of onion services -allows nodes to accept inbound connections even if they're behind a NAT. At the -time of writing this documentation, `lnd` supports both types of onion services: -v2 and v3. +allows nodes to accept inbound connections even if they're behind a NAT. `lnd` +supports v3 onion services only; legacy v2 onion service support has been +removed. -Before following the remainder of this documentation, you should ensure that you -already have Tor installed locally. **If you want to run v3 Onion Services, make -sure that you run at least version 0.3.3.6.** +Before following the remainder of this documentation, you should ensure that +you already have Tor installed locally. **Make sure that you run at least +version 0.3.3.6 of Tor in order to use v3 Onion Services.** Official instructions to install the latest release of Tor can be found [here](https://www.torproject.org/docs/tor-doc-unix.html.en). @@ -81,7 +81,6 @@ Tor: --tor.control= The host:port that Tor is listening on for Tor control connections (default: localhost:9051) --tor.targetipaddress= IP address that Tor should use as the target of the hidden service --tor.password= The password used to arrive at the HashedControlPassword for the control port. If provided, the HASHEDPASSWORD authentication method will be used instead of the SAFECOOKIE one. - --tor.v2 Automatically set up a v2 onion service to listen for inbound connections --tor.v3 Automatically set up a v3 onion service to listen for inbound connections --tor.privatekeypath= The path to the private key of the onion service being created ``` @@ -103,11 +102,8 @@ service. A path to save the onion service's private key can be specified with the `--tor.privatekeypath` flag. Most of these arguments have defaults, so as long as they apply to you, routing -all outbound and inbound connections through Tor can simply be done with either -v2 or v3 onion services: -```shell -$ ./lnd --tor.active --tor.v2 -``` +all outbound and inbound connections through Tor can simply be done with v3 +onion services: ```shell $ ./lnd --tor.active --tor.v3 ``` @@ -159,15 +155,13 @@ authentication methods (arguably, from most to least secure): ## Listening for Inbound Connections In order to listen for inbound connections through Tor, an onion service must be -created. There are two types of onion services: v2 and v3. v3 onion services -are the latest generation of onion services, and they provide a number of -advantages over the legacy v2 onion services. To learn more about these -benefits, see [Intro to Next Gen Onion Services](https://trac.torproject.org/projects/tor/wiki/doc/NextGenOnions). +created. `lnd` supports v3 onion services, the latest generation of onion +services. To learn more about these, see +[Intro to Next Gen Onion Services](https://trac.torproject.org/projects/tor/wiki/doc/NextGenOnions). -Both types can be created and used automatically by `lnd`. Specifying which type -should be used can easily be done by either using the `tor.v2` or `tor.v3` flag. -To prevent unintentional leaking of identifying information, it is also necessary -to add the flag `listen=localhost`. +v3 onion services are created and used automatically by `lnd` via the `tor.v3` +flag. To prevent unintentional leaking of identifying information, it is also +necessary to add the flag `listen=localhost`. For example, v3 onion services can be used with the following flags: ```shell @@ -176,9 +170,8 @@ $ ./lnd --tor.active --tor.v3 --listen=localhost This will automatically create a hidden service for your node to use to listen for inbound connections and advertise itself to the network. The onion service's -private key is saved to a file named `v2_onion_private_key` or -`v3_onion_private_key` depending on the type of onion service used in `lnd`'s -base directory. This will allow `lnd` to recreate the same hidden service upon +private key is saved to a file named `v3_onion_private_key` in `lnd`'s base +directory. This will allow `lnd` to recreate the same hidden service upon restart. If you wish to generate a new onion service, you can simply delete this file. The path to this private key file can also be modified with the `--tor.privatekeypath` argument. diff --git a/docs/release-notes/release-notes-0.21.0.md b/docs/release-notes/release-notes-0.21.0.md index 1f96ba120da..ab8a1fd97f0 100644 --- a/docs/release-notes/release-notes-0.21.0.md +++ b/docs/release-notes/release-notes-0.21.0.md @@ -282,6 +282,30 @@ 14 and 8 respectively, now reserved). Callers must use the multi-channel `outgoing_chan_ids` field introduced in 0.20. +* [Removed the deprecated `--tor.v2` configuration + flag](https://github.com/lightningnetwork/lnd/pull/10813). Tor stopped + serving v2 onion services in October 2021, and lnd no longer produces + v2 on any code path; `tor.OnionHostToFakeIP` is also gone. Operator + input is rejected at the boundary: `--externalip`, `--listen`, + `lncli connect`, and `lncli wtclient towers add` fail fast on a v2 + `.onion` string, so operators upgrading with a v2 entry in + `lnd.conf` must remove it before lnd will start. Persisted state + carried over from a previous version is also filtered before use: + the self-node announcement strips any v2 entry from the source-node + record before signing; the watchtower client drops v2 entries from + each persisted tower's address list (skipping the tower entirely if + no non-v2 address remains so the operator can attach a fresh v3 + address); the autopilot, graph bootstrapper, and static-channel + backup restore paths skip v2 entries before attempting outbound + dials. The on-disk records are left intact. + + Peer-signed announcements that still carry v2 are handled + byte-for-byte: the `lnwire` and `graph/db` codecs round-trip v2 so + `DataToSign` reproduces the signed bytes, signatures validate, and + the announcement is persisted and re-broadcast unchanged. RPCs like + `GetNodeInfo` and `DescribeGraph` still expose the full address + set. + ## Performance Improvements * Let the [channel graph cache be populated diff --git a/graph/db/addr.go b/graph/db/addr.go index 836d516b000..e06665d15a9 100644 --- a/graph/db/addr.go +++ b/graph/db/addr.go @@ -99,7 +99,8 @@ func encodeTCPAddr(w io.Writer, addr *net.TCPAddr) error { } // encodeOnionAddr serializes an onion address into its compact raw bytes -// representation. +// representation. v2 round-trips for wire fidelity even though lnd no longer +// produces it. func encodeOnionAddr(w io.Writer, addr *tor.OnionAddr) error { var suffixIndex int hostLen := len(addr.OnionService) diff --git a/graph/db/models/node.go b/graph/db/models/node.go index 7f3ce777ab9..582aeea4b32 100644 --- a/graph/db/models/node.go +++ b/graph/db/models/node.go @@ -229,7 +229,12 @@ func (n *Node) NodeAnnouncement(signed bool) (*lnwire.NodeAnnouncement1, } // NodeFromWireAnnouncement creates a Node instance from an -// lnwire.NodeAnnouncement1 message. +// lnwire.NodeAnnouncement1 message. The address list from msg.Addresses +// is copied verbatim, including legacy entries such as tor v2 onion +// addresses that lnd no longer produces itself. This is required so +// that Node.NodeAnnouncement can later reconstruct the exact byte +// sequence the remote peer signed, allowing signature verification and +// re-broadcast to succeed across restarts. func NodeFromWireAnnouncement(msg *lnwire.NodeAnnouncement1) *Node { timestamp := time.Unix(int64(msg.Timestamp), 0) diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go index e705468a4d8..871ad1a5106 100644 --- a/graph/db/sql_store.go +++ b/graph/db/sql_store.go @@ -4791,7 +4791,8 @@ const ( func collectAddressRecords(addresses []net.Addr) (map[dbAddressType][]string, error) { - // Copy the nodes latest set of addresses. + // Copy the nodes latest set of addresses. v2 is stored for wire + // fidelity even though lnd no longer produces it. newAddresses := map[dbAddressType][]string{ addressTypeIPv4: {}, addressTypeIPv6: {}, diff --git a/itest/lnd_channel_graph_test.go b/itest/lnd_channel_graph_test.go index f8d63c1d5ca..52612b81308 100644 --- a/itest/lnd_channel_graph_test.go +++ b/itest/lnd_channel_graph_test.go @@ -378,7 +378,6 @@ func testNodeAnnouncement(ht *lntest.HarnessTest) { advertisedAddrs := []string{ "192.168.1.1:8333", "[2001:db8:85a3:8d3:1319:8a2e:370:7348]:8337", - "bkb6azqggsaiskzi.onion:9735", "fomvuglh6h6vcag73xo5t5gv56ombih3zr2xvplkpbfd7wrog4swj" + "wid.onion:1234", } @@ -435,7 +434,6 @@ func testUpdateNodeAnnouncement(ht *lntest.HarnessTest) { extraAddrs := []string{ "192.168.1.1:8333", "[2001:db8:85a3:8d3:1319:8a2e:370:7348]:8337", - "bkb6azqggsaiskzi.onion:9735", "fomvuglh6h6vcag73xo5t5gv56ombih3zr2xvplkpbfd7wrog4swj" + "wid.onion:1234", } diff --git a/lncfg/address.go b/lncfg/address.go index 381ff2a54a1..f96e4be2297 100644 --- a/lncfg/address.go +++ b/lncfg/address.go @@ -229,6 +229,17 @@ func ParseAddressString(strAddress string, defaultPort string, // an onion addresses, if so, we can directly pass the raw // address and port to create the proper address. if tor.IsOnionHost(rawHost) { + // Reject v2 at the operator-input boundary; the wire + // codec still round-trips v2 from peer-signed + // announcements. + if len(rawHost) == tor.V2Len { + return nil, fmt.Errorf("tor v2 onion "+ + "services were retired in October "+ + "2021 and are no longer supported; "+ + "use a v3 .onion address instead: %s", + rawHost) + } + portNum, err := strconv.Atoi(rawPort) if err != nil { return nil, err diff --git a/lncfg/address_test.go b/lncfg/address_test.go index 2066aecc329..d256b65a30f 100644 --- a/lncfg/address_test.go +++ b/lncfg/address_test.go @@ -55,25 +55,15 @@ var ( false, false, }, - { - "3g2upl4pq6kufc4m.onion", - "tcp", - "3g2upl4pq6kufc4m.onion:1234", - false, - false, - }, - { - "3g2upl4pq6kufc4m.onion:9735", - "tcp", - "3g2upl4pq6kufc4m.onion:9735", - false, - false, - }, } invalidTestVectors = []string{ "some string", "://", "12.12.12.12.12", + // v2 onion services were retired by Tor in October 2021 and + // must be rejected at the input boundary. + "3g2upl4pq6kufc4m.onion", + "3g2upl4pq6kufc4m.onion:9735", } ) diff --git a/lncfg/tor.go b/lncfg/tor.go index 932d5dfc904..81a24cd636f 100644 --- a/lncfg/tor.go +++ b/lncfg/tor.go @@ -12,7 +12,6 @@ type Tor struct { Control string `long:"control" description:"The host:port that Tor is listening on for Tor control connections"` TargetIPAddress string `long:"targetipaddress" description:"IP address that Tor should use as the target of the hidden service"` Password string `long:"password" description:"The password used to arrive at the HashedControlPassword for the control port. If provided, the HASHEDPASSWORD authentication method will be used instead of the SAFECOOKIE one."` - V2 bool `long:"v2" description:"DEPRECATED: Tor v2 onion services are obsolete and support will be removed in v0.21.0. Use v3 instead." hidden:"true"` V3 bool `long:"v3" description:"Automatically set up a v3 onion service to listen for inbound connections"` PrivateKeyPath string `long:"privatekeypath" description:"The path to the private key of the onion service being created"` EncryptKey bool `long:"encryptkey" description:"Encrypts the Tor private key file on disk"` diff --git a/lnd.go b/lnd.go index 76b08a114a1..096331a9127 100644 --- a/lnd.go +++ b/lnd.go @@ -530,11 +530,11 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, } } - // If tor is active and either v2 or v3 onion services have been - // specified, make a tor controller and pass it into both the watchtower - // server and the regular lnd server. + // If tor is active and a v3 onion service has been specified, make a + // tor controller and pass it into both the watchtower server and the + // regular lnd server. var torController *tor.Controller - if cfg.Tor.Active && (cfg.Tor.V2 || cfg.Tor.V3) { + if cfg.Tor.Active && cfg.Tor.V3 { torController = tor.NewController( cfg.Tor.Control, cfg.Tor.TargetIPAddress, cfg.Tor.Password, @@ -591,13 +591,6 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, wtCfg.WatchtowerKeyPath = cfg.Tor.WatchtowerKeyPath wtCfg.EncryptKey = cfg.Tor.EncryptKey wtCfg.KeyRing = activeChainControl.KeyRing - - switch { - case cfg.Tor.V2: - wtCfg.Type = tor.V2 - case cfg.Tor.V3: - wtCfg.Type = tor.V3 - } } wtConfig, err := cfg.Watchtower.Apply( diff --git a/lnwire/writer.go b/lnwire/writer.go index ddf67e8a289..57517eb0deb 100644 --- a/lnwire/writer.go +++ b/lnwire/writer.go @@ -334,7 +334,8 @@ func WriteOnionAddr(buf *bytes.Buffer, addr *tor.OnionAddr) error { descriptor []byte ) - // Decide the suffixIndex and descriptor. + // Decide the suffixIndex and descriptor. v2 round-trips for wire + // fidelity even though lnd no longer produces it. switch len(addr.OnionService) { case tor.V2Len: descriptor = []byte{byte(v2OnionAddr)} diff --git a/netann/node_announcement_test.go b/netann/node_announcement_test.go new file mode 100644 index 00000000000..370ddf6b673 --- /dev/null +++ b/netann/node_announcement_test.go @@ -0,0 +1,90 @@ +package netann_test + +import ( + "bytes" + "net" + "testing" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcec/v2/ecdsa" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/lightningnetwork/lnd/graph/db/models" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/netann" + "github.com/lightningnetwork/lnd/tor" + "github.com/stretchr/testify/require" +) + +// TestNodeAnnSignatureWithLegacyV2OnionAddr signs a [v3, v2, ipv4] +// announcement, round-trips it through the wire codec, and verifies that +// the signature still validates. If Decode or NodeFromWireAnnouncement ever +// starts filtering v2, DataToSign no longer reproduces the signed bytes and +// this test catches the regression. +func TestNodeAnnSignatureWithLegacyV2OnionAddr(t *testing.T) { + t.Parallel() + + privKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + var nodeID [33]byte + copy(nodeID[:], privKey.PubKey().SerializeCompressed()) + + v3 := &tor.OnionAddr{ + OnionService: "abcdefghijabcdefghijabcdefghij" + + "abcdefghijabcdefghij234567.onion", + Port: 9735, + } + v2 := &tor.OnionAddr{ + OnionService: "abcdefghijklmnop.onion", + Port: 9735, + } + tcp := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 9735} + addrs := []net.Addr{v3, v2, tcp} + + ann := &lnwire.NodeAnnouncement1{ + Features: lnwire.NewRawFeatureVector(), + Timestamp: 1700000000, + NodeID: nodeID, + Addresses: addrs, + } + copy(ann.Alias[:], "regression-test") + + dataToSign, err := ann.DataToSign() + require.NoError(t, err) + + rawSig := ecdsa.Sign(privKey, chainhash.DoubleHashB(dataToSign)) + ann.Signature, err = lnwire.NewSigFromSignature(rawSig) + require.NoError(t, err) + + var buf bytes.Buffer + require.NoError(t, ann.Encode(&buf, 0)) + + var decoded lnwire.NodeAnnouncement1 + require.NoError(t, decoded.Decode(&buf, 0)) + + // All three addresses must survive Decode for DataToSign to + // reproduce the signed bytes. + require.Len(t, decoded.Addresses, 3) + require.NoError(t, netann.ValidateNodeAnnSignature(&decoded)) + + node := models.NodeFromWireAnnouncement(&decoded) + require.Len(t, node.Addresses, 3) + + var sawV2, sawV3, sawTCP bool + for _, addr := range node.Addresses { + switch a := addr.(type) { + case *tor.OnionAddr: + switch len(a.OnionService) { + case tor.V2Len: + sawV2 = true + case tor.V3Len: + sawV3 = true + } + case *net.TCPAddr: + sawTCP = true + } + } + require.True(t, sawV2, "v2 onion address must be preserved on the Node") + require.True(t, sawV3, "v3 onion address must be preserved on the Node") + require.True(t, sawTCP, "ipv4 address must be preserved on the Node") +} diff --git a/pilot.go b/pilot.go index ff9173faaf8..94335a45c8f 100644 --- a/pilot.go +++ b/pilot.go @@ -209,6 +209,11 @@ func initAutoPilot(svr *server, cfg *lncfg.AutoPilot, ChainNet: netParams.Net, } + // Strip persisted Tor v2 .onion entries: Tor stopped + // serving them in 2021 and the dial would never + // succeed. Covered by TestWithoutV2Onion. + addrs = withoutV2Onion(addrs) + // We'll attempt to successively connect to each of the // advertised IP addresses until we've either exhausted // the advertised IP addresses, or have made a diff --git a/sample-lnd.conf b/sample-lnd.conf index 933f76a49e1..77fad22c431 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -1012,9 +1012,6 @@ ; Example: ; tor.password=plsdonthackme -; Automatically set up a v2 onion service to listen for inbound connections. -; tor.v2=false - ; Automatically set up a v3 onion service to listen for inbound connections. ; tor.v3=false diff --git a/server.go b/server.go index 45992c464cb..86fe38b0437 100644 --- a/server.go +++ b/server.go @@ -514,6 +514,10 @@ func (s *server) updatePersistentPeerAddrs() error { len(update.Addresses)) for _, addr := range update.Addresses { + if isV2OnionAddr(addr) { + continue + } + addrs = append(addrs, &lnwire.NetAddress{ IdentityKey: update.IdentityKey, @@ -584,6 +588,15 @@ func parseAddr(address string, netCfg tor.Net) (net.Addr, error) { } if tor.IsOnionHost(host) { + // Reject v2 at the operator-input boundary; the wire codec + // still round-trips v2 from peer-signed announcements. + if len(host) == tor.V2Len { + return nil, fmt.Errorf("tor v2 onion services were "+ + "retired in October 2021 and are no longer "+ + "supported; use a v3 .onion address "+ + "instead: %s", host) + } + return &tor.OnionAddr{OnionService: host, Port: port}, nil } @@ -595,6 +608,42 @@ func parseAddr(address string, netCfg tor.Net) (net.Addr, error) { return netCfg.ResolveTCPAddr("tcp", hostPort) } +// isV2OnionAddr reports whether addr is a Tor v2 .onion address. Tor stopped +// serving v2 onion services in October 2021, so callers skip these on dial +// paths. Storage and gossip re-broadcast still preserve v2 byte-for-byte to +// keep peer-signed NodeAnnouncement signatures verifiable. +// +// TODO: move this helper into the `tor` module (as `tor.IsV2Onion`) and +// remove this copy along with the duplicate in +// watchtower/wtclient/interface.go once a new `tor` module version is cut +// and the dependency is bumped. +func isV2OnionAddr(addr net.Addr) bool { + onion, ok := addr.(*tor.OnionAddr) + if !ok { + return false + } + + return len(onion.OnionService) == tor.V2Len +} + +// withoutV2Onion returns addrs with any Tor v2 .onion entries removed. See +// isV2OnionAddr for the rationale. +// +// TODO: move this helper into the `tor` module and remove this copy along +// with the duplicate in watchtower/wtclient/interface.go once a new `tor` +// module version is cut and the dependency is bumped. +func withoutV2Onion(addrs []net.Addr) []net.Addr { + filtered := make([]net.Addr, 0, len(addrs)) + for _, addr := range addrs { + if isV2OnionAddr(addr) { + continue + } + filtered = append(filtered, addr) + } + + return filtered +} + // noiseDial is a factory function which creates a connmgr compliant dialing // function by returning a closure which includes the server's identity key. func noiseDial(idKey keychain.SingleKeyECDH, @@ -3379,8 +3428,8 @@ func (s *server) initialPeerBootstrap(ctx context.Context, } } -// createNewHiddenService automatically sets up a v2 or v3 onion service in -// order to listen for inbound connections over Tor. +// createNewHiddenService automatically sets up a v3 onion service in order to +// listen for inbound connections over Tor. func (s *server) createNewHiddenService(ctx context.Context) error { // Determine the different ports the server is listening on. The onion // service's virtual port will map to these ports and one will be picked @@ -3408,13 +3457,6 @@ func (s *server) createNewHiddenService(ctx context.Context) error { ), } - switch { - case s.cfg.Tor.V2: - onionCfg.Type = tor.V2 - case s.cfg.Tor.V3: - onionCfg.Type = tor.V3 - } - addr, err := s.torController.AddOnion(onionCfg) if err != nil { return err @@ -3621,7 +3663,7 @@ func (s *server) establishPersistentConnections(ctx context.Context) error { pubStr := string(node.IdentityPub.SerializeCompressed()) nodeAddrs := &nodeAddresses{ pubKey: node.IdentityPub, - addresses: node.Addresses, + addresses: withoutV2Onion(node.Addresses), } nodeAddrsMap[pubStr] = nodeAddrs } @@ -3650,6 +3692,10 @@ func (s *server) establishPersistentConnections(ctx context.Context) error { // connect to for this peer. addrSet := make(map[string]net.Addr) for _, addr := range channelPeer.Addresses { + if isV2OnionAddr(addr) { + continue + } + switch addr.(type) { case *net.TCPAddr: addrSet[addr.String()] = addr @@ -3668,6 +3714,10 @@ func (s *server) establishPersistentConnections(ctx context.Context) error { linkNodeAddrs, ok := nodeAddrsMap[pubStr] if ok { for _, lnAddress := range linkNodeAddrs.addresses { + if isV2OnionAddr(lnAddress) { + continue + } + switch lnAddress.(type) { case *net.TCPAddr: addrSet[lnAddress.String()] = lnAddress @@ -5359,11 +5409,12 @@ func (s *server) fetchNodeAdvertisedAddrs(ctx context.Context, return nil, err } - if len(node.Addresses) == 0 { + addrs := withoutV2Onion(node.Addresses) + if len(addrs) == 0 { return nil, errNoAdvertisedAddr } - return node.Addresses, nil + return addrs, nil } // fetchLastChanUpdate returns a function which is able to retrieve our latest @@ -5739,8 +5790,10 @@ func (s *server) setSelfNode(ctx context.Context, nodePub route.Vertex, // If the `externalip` is not specified in the config, it means // `addrs` will be empty, we'll use the source node's addresses. + // Filter out any persisted Tor v2 onion entries so an upgraded + // node never re-signs or re-broadcasts a legacy v2 address. if len(s.cfg.ExternalIPs) == 0 { - addrs = srcNode.Addresses + addrs = withoutV2Onion(srcNode.Addresses) } case errors.Is(err, graphdb.ErrSourceNodeNotSet): diff --git a/server_test.go b/server_test.go index 0cb3643184f..f666b508ed2 100644 --- a/server_test.go +++ b/server_test.go @@ -1,9 +1,11 @@ package lnd import ( + "net" "testing" "time" + "github.com/lightningnetwork/lnd/tor" "github.com/stretchr/testify/require" ) @@ -139,3 +141,100 @@ func TestNodeAnnouncementTimestampComparison(t *testing.T) { }) } } + +// TestParseAddrRejectsTorV2 ensures that parseAddr rejects v2 .onion hosts at +// the operator-input boundary. This is the path used by lncli connect (via +// rpcserver.ConnectPeer) and the --addpeer config option, mirroring the +// equivalent gate in lncfg.ParseAddressString. +func TestParseAddrRejectsTorV2(t *testing.T) { + t.Parallel() + + const ( + v2Host = "3g2upl4pq6kufc4m.onion" + v3Host = "4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpb" + + "i7utijcltosqemad.onion" + ) + + netCfg := &tor.ClearNet{} + + tests := []struct { + name string + address string + expectErr bool + }{ + { + name: "v2 without port is rejected", + address: v2Host, + expectErr: true, + }, + { + name: "v2 with port is rejected", + address: v2Host + ":9735", + expectErr: true, + }, + { + name: "v3 without port is accepted", + address: v3Host, + expectErr: false, + }, + { + name: "v3 with port is accepted", + address: v3Host + ":9735", + expectErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + addr, err := parseAddr(tc.address, netCfg) + if tc.expectErr { + require.Error(t, err) + require.Contains( + t, err.Error(), "tor v2 onion", + ) + require.Nil(t, addr) + + return + } + + require.NoError(t, err) + onionAddr, ok := addr.(*tor.OnionAddr) + require.True(t, ok) + require.Equal(t, v3Host, onionAddr.OnionService) + }) + } +} + +// TestWithoutV2Onion ensures that Tor v2 onion addresses are dropped from +// reconnect/dial consumption paths (startup persistent reconnect, live +// topology updates, fetchNodeAdvertisedAddrs) while non-onion and v3 onion +// addresses pass through unchanged. Storage and gossip re-broadcast remain +// byte-faithful elsewhere. +func TestWithoutV2Onion(t *testing.T) { + t.Parallel() + + v2 := &tor.OnionAddr{ + OnionService: "3g2upl4pq6kufc4m.onion", + Port: 9735, + } + v3 := &tor.OnionAddr{ + OnionService: "4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnz" + + "jpbi7utijcltosqemad.onion", + Port: 9735, + } + tcp := &net.TCPAddr{ + IP: net.ParseIP("127.0.0.1"), + Port: 9735, + } + + require.True(t, isV2OnionAddr(v2)) + require.False(t, isV2OnionAddr(v3)) + require.False(t, isV2OnionAddr(tcp)) + + filtered := withoutV2Onion([]net.Addr{v2, v3, tcp, v2}) + require.Equal(t, []net.Addr{v3, tcp}, filtered) + + // An all-v2 input filters to an empty slice; callers such as + // fetchNodeAdvertisedAddrs treat this as "no advertised address". + require.Empty(t, withoutV2Onion([]net.Addr{v2, v2})) +} diff --git a/tor/README.md b/tor/README.md index f23ccf60894..41fc8d8917f 100644 --- a/tor/README.md +++ b/tor/README.md @@ -9,10 +9,10 @@ Tor daemon. So far, supported functions include: * Limited Tor Control functionality (synchronous messages only). So far, this includes: * Support for SAFECOOKIE, HASHEDPASSWORD, and NULL authentication methods. - * Creating v2 and v3 onion services. + * Creating v3 onion services. -In the future, the Tor Control functionality will be extended to support v3 -onion services, asynchronous messages, etc. +In the future, the Tor Control functionality will be extended to support +asynchronous messages, etc. ## Installation and Updating diff --git a/tor/cmd_onion.go b/tor/cmd_onion.go index 6e60504a93e..ac43e2c7544 100644 --- a/tor/cmd_onion.go +++ b/tor/cmd_onion.go @@ -19,22 +19,22 @@ var ( // ErrNoPrivateKey is an error returned by the OnionStore.PrivateKey // method when a private key hasn't yet been stored. ErrNoPrivateKey = errors.New("private key not found") -) - -// OnionType denotes the type of the onion service. -type OnionType int - -const ( - // V2 denotes that the onion service is V2. - V2 OnionType = iota - // V3 denotes that the onion service is V3. - V3 + // ErrNonV3OnionKey is returned when a restored onion private key is + // not a v3 (ED25519-V3) key. lnd no longer creates or recovers v2 + // onion services; users with an old v2 key file must remove it so + // a fresh v3 service can be generated. + ErrNonV3OnionKey = errors.New("restored onion private key is not a " + + "v3 (ED25519-V3) key; remove the old onion key file to " + + "generate a new v3 service") ) const ( - // V2KeyParam is a parameter that Tor accepts for a new V2 service. - V2KeyParam = "RSA1024" + // v2KeyParam is the parameter Tor used for legacy v2 onion service + // private keys. lnd no longer generates or restores v2 services, but + // the prefix is still used to detect stale on-disk keys and surface a + // clear error. + v2KeyParam = "RSA1024" // V3KeyParam is a parameter that Tor accepts for a new V3 service. V3KeyParam = "ED25519-V3" @@ -128,14 +128,19 @@ func (f *OnionFile) PrivateKey() ([]byte, error) { return nil, err } - // If the privateKey starts with either v2 or v3 key params then - // it's likely not encrypted and we can return the data as is. - if bytes.HasPrefix(privateKeyContent, []byte(V2KeyParam)) || - bytes.HasPrefix(privateKeyContent, []byte(V3KeyParam)) { - + // If the privateKey starts with the v3 key param then it's likely + // not encrypted and we can return the data as is. + if bytes.HasPrefix(privateKeyContent, []byte(V3KeyParam)) { return privateKeyContent, nil } + // A plaintext legacy v2 (RSA1024) key on disk is unsupported. + // Surface a dedicated error so the user can act on it instead of + // being redirected to --tor.encryptkey. + if bytes.HasPrefix(privateKeyContent, []byte(v2KeyParam)) { + return nil, ErrNonV3OnionKey + } + // If the privateKeyContent is encrypted but --tor.encryptkey // wasn't set we return an error. if !f.encryptKey { @@ -151,6 +156,13 @@ func (f *OnionFile) PrivateKey() ([]byte, error) { return nil, err } + // Run the same validator over the decrypted contents so an encrypted + // legacy key is rejected with a clear error rather than being passed + // to Tor. + if !bytes.HasPrefix(privateKeyContent, []byte(V3KeyParam)) { + return nil, ErrNonV3OnionKey + } + return privateKeyContent, nil } @@ -162,9 +174,6 @@ func (f *OnionFile) DeletePrivateKey() error { // AddOnionConfig houses all of the required parameters in order to // successfully create a new onion service or restore an existing one. type AddOnionConfig struct { - // Type denotes the type of the onion service that should be created. - Type OnionType - // VirtualPort is the externally reachable port of the onion address. VirtualPort int @@ -192,14 +201,7 @@ func (c *Controller) prepareKeyparam(cfg AddOnionConfig) (string, error) { // create a new onion service and return its private key. Otherwise, // we'll request the server to recreate the onion server from our // private key. - var keyParam string - switch cfg.Type { - // TODO(yy): drop support for v2. - case V2: - keyParam = "NEW:" + V2KeyParam - case V3: - keyParam = "NEW:" + V3KeyParam - } + keyParam := "NEW:" + V3KeyParam if cfg.Store != nil { privateKey, err := cfg.Store.PrivateKey() @@ -208,7 +210,16 @@ func (c *Controller) prepareKeyparam(cfg AddOnionConfig) (string, error) { case ErrNoPrivateKey: // Recover the onion service with the private key found. + // Refuse to hand a non-v3 key (for example, a legacy + // RSA1024:... v2 key) to Tor; the caller must remove the old + // key file rather than silently regenerate a fresh v3 + // service, which would change the advertised onion identity. case nil: + if !bytes.HasPrefix( + privateKey, []byte(V3KeyParam+":"), + ) { + return "", ErrNonV3OnionKey + } keyParam = string(privateKey) default: @@ -273,13 +284,9 @@ func (c *Controller) prepareAddOnion(cfg AddOnionConfig) (string, string, // creating new service via `ADD_ONION`. func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) { // Before sending the request to create an onion service to the Tor - // server, we'll make sure that it supports V3 onion services if that - // was the type requested. - // TODO(yy): drop support for v2. - if cfg.Type == V3 { - if err := supportsV3(c.version); err != nil { - return nil, err - } + // server, we'll make sure that it supports V3 onion services. + if err := supportsV3(c.version); err != nil { + return nil, err } // Construct the cmd command. @@ -298,13 +305,13 @@ func (c *Controller) AddOnion(cfg AddOnionConfig) (*OnionAddr, error) { // If successful, the reply from the server should be of the following // format, depending on whether a private key has been requested: // - // C: ADD_ONION RSA1024:[Blob Redacted] Port=80,8080 - // S: 250-ServiceID=testonion1234567 + // C: ADD_ONION ED25519-V3:[Blob Redacted] Port=80,8080 + // S: 250-ServiceID=<56-char-v3-service-id> // S: 250 OK // - // C: ADD_ONION NEW:RSA1024 Port=80,8080 - // S: 250-ServiceID=testonion1234567 - // S: 250-PrivateKey=RSA1024:[Blob Redacted] + // C: ADD_ONION NEW:ED25519-V3 Port=80,8080 + // S: 250-ServiceID=<56-char-v3-service-id> + // S: 250-PrivateKey=ED25519-V3:[Blob Redacted] // S: 250 OK // // We're interested in retrieving the service ID, which is the public diff --git a/tor/cmd_onion_test.go b/tor/cmd_onion_test.go index 0fea80dbbbd..45cb7f7153b 100644 --- a/tor/cmd_onion_test.go +++ b/tor/cmd_onion_test.go @@ -3,6 +3,7 @@ package tor import ( "errors" "io" + "os" "path/filepath" "testing" @@ -11,8 +12,8 @@ import ( ) var ( - privateKey = []byte("RSA1024 hide_me_plz") - anotherKey = []byte("another_key") + privateKey = []byte("ED25519-V3:hide_me_plz") + anotherKey = []byte("ED25519-V3:another_key") ) // TestOnionFile tests that the File implementation of the OnionStore @@ -67,30 +68,69 @@ func TestOnionFile(t *testing.T) { require.NoError(t, err) } +// TestOnionFilePrivateKeyRejectsLegacyV2 ensures the file-backed store +// surfaces ErrNonV3OnionKey when the on-disk key is a plaintext legacy +// v2 (RSA1024) blob, rather than handing the bytes back to Tor. +func TestOnionFilePrivateKeyRejectsLegacyV2(t *testing.T) { + t.Parallel() + + privateKeyPath := filepath.Join(t.TempDir(), "secret") + require.NoError(t, os.WriteFile( + privateKeyPath, + []byte("RSA1024:legacy-v2-key-bytes"), + 0600, + )) + + onionFile := NewOnionFile(privateKeyPath, 0600, false, MockEncrypter{}) + _, err := onionFile.PrivateKey() + require.ErrorIs(t, err, ErrNonV3OnionKey) +} + +// TestOnionFilePrivateKeyRejectsEncryptedLegacyV2 ensures the file-backed +// store rejects an encrypted on-disk key that decrypts to a legacy v2 +// (RSA1024) blob, instead of forwarding the bytes to Tor. +func TestOnionFilePrivateKeyRejectsEncryptedLegacyV2(t *testing.T) { + t.Parallel() + + privateKeyPath := filepath.Join(t.TempDir(), "secret") + + // Write a ciphertext-shaped payload (no v3 or v2 prefix) so the + // reader path falls through to the decrypter. + require.NoError(t, os.WriteFile( + privateKeyPath, []byte("encrypted-blob"), 0600, + )) + + onionFile := NewOnionFile( + privateKeyPath, 0600, true, legacyV2Decrypter{}, + ) + _, err := onionFile.PrivateKey() + require.ErrorIs(t, err, ErrNonV3OnionKey) +} + // TestPrepareKeyParam checks that the key param is created as expected. func TestPrepareKeyParam(t *testing.T) { - testKey := []byte("hide_me_plz") + v3Key := []byte("ED25519-V3:hide_me_plz") dummyErr := errors.New("dummy") // Create a dummy controller. controller := NewController("", "", "") // Test that a V3 keyParam is used. - cfg := AddOnionConfig{Type: V3} + cfg := AddOnionConfig{} keyParam, err := controller.prepareKeyparam(cfg) require.Equal(t, "NEW:ED25519-V3", keyParam) require.NoError(t, err) - // Create a mock store which returns the test private key. + // Create a mock store which returns a valid v3 private key. store := &mockStore{} - store.On("PrivateKey").Return(testKey, nil) + store.On("PrivateKey").Return(v3Key, nil) - // Check that the test private is returned. - cfg = AddOnionConfig{Type: V3, Store: store} + // Check that the stored v3 private key is returned. + cfg = AddOnionConfig{Store: store} keyParam, err = controller.prepareKeyparam(cfg) - require.Equal(t, string(testKey), keyParam) + require.Equal(t, string(v3Key), keyParam) require.NoError(t, err) store.AssertExpectations(t) @@ -99,7 +139,7 @@ func TestPrepareKeyParam(t *testing.T) { store.On("PrivateKey").Return(nil, ErrNoPrivateKey) // Check that the V3 keyParam is returned. - cfg = AddOnionConfig{Type: V3, Store: store} + cfg = AddOnionConfig{Store: store} keyParam, err = controller.prepareKeyparam(cfg) require.Equal(t, "NEW:ED25519-V3", keyParam) @@ -111,12 +151,27 @@ func TestPrepareKeyParam(t *testing.T) { store.On("PrivateKey").Return(nil, dummyErr) // Check that an error is returned. - cfg = AddOnionConfig{Type: V3, Store: store} + cfg = AddOnionConfig{Store: store} keyParam, err = controller.prepareKeyparam(cfg) require.Empty(t, keyParam) require.ErrorIs(t, dummyErr, err) store.AssertExpectations(t) + + // A restored legacy v2 (RSA1024) onion key must be rejected; lnd no + // longer creates or recovers v2 services, and silently regenerating + // a fresh v3 service would change the advertised onion identity. + store = &mockStore{} + store.On("PrivateKey").Return( + []byte("RSA1024:legacy-v2-key-bytes"), nil, + ) + + cfg = AddOnionConfig{Store: store} + keyParam, err = controller.prepareKeyparam(cfg) + + require.Empty(t, keyParam) + require.ErrorIs(t, err, ErrNonV3OnionKey) + store.AssertExpectations(t) } // TestPrepareAddOnion checks that the cmd used to add onion service is created @@ -126,7 +181,7 @@ func TestPrepareAddOnion(t *testing.T) { // Create a mock store. store := &mockStore{} - testKey := []byte("hide_me_plz") + testKey := []byte("ED25519-V3:hide_me_plz") testCases := []struct { name string @@ -139,14 +194,14 @@ func TestPrepareAddOnion(t *testing.T) { name: "empty target IP and ports", targetIPAddress: "", cfg: AddOnionConfig{VirtualPort: 9735}, - expectedCmd: "ADD_ONION NEW:RSA1024 Port=9735,9735 ", + expectedCmd: "ADD_ONION NEW:ED25519-V3 Port=9735,9735 ", expectedErr: nil, }, { name: "specified target IP and empty ports", targetIPAddress: "127.0.0.1", cfg: AddOnionConfig{VirtualPort: 9735}, - expectedCmd: "ADD_ONION NEW:RSA1024 " + + expectedCmd: "ADD_ONION NEW:ED25519-V3 " + "Port=9735,127.0.0.1:9735 ", expectedErr: nil, }, @@ -157,7 +212,7 @@ func TestPrepareAddOnion(t *testing.T) { VirtualPort: 9735, TargetPorts: []int{18000, 18001}, }, - expectedCmd: "ADD_ONION NEW:RSA1024 " + + expectedCmd: "ADD_ONION NEW:ED25519-V3 " + "Port=9735,127.0.0.1:18000 " + "Port=9735,127.0.0.1:18001 ", expectedErr: nil, @@ -169,7 +224,7 @@ func TestPrepareAddOnion(t *testing.T) { VirtualPort: 9735, Store: store, }, - expectedCmd: "ADD_ONION hide_me_plz " + + expectedCmd: "ADD_ONION ED25519-V3:hide_me_plz " + "Port=9735,9735 ", expectedErr: nil, }, @@ -212,7 +267,15 @@ func (m *mockStore) StorePrivateKey(key []byte) error { func (m *mockStore) PrivateKey() ([]byte, error) { args := m.Called() - return []byte("hide_me_plz"), args.Error(1) + + // Allow callers to set the returned key bytes via the mock's first + // return value; fall back to a valid v3 key prefix for tests that + // only care about a successful key load. + if key, ok := args.Get(0).([]byte); ok && key != nil { + return key, args.Error(1) + } + + return []byte("ED25519-V3:hide_me_plz"), args.Error(1) } func (m *mockStore) DeletePrivateKey() error { @@ -229,3 +292,18 @@ func (m MockEncrypter) EncryptPayloadToWriter(_ []byte, _ io.Writer) error { func (m MockEncrypter) DecryptPayloadFromReader(_ io.Reader) ([]byte, error) { return anotherKey, nil } + +// legacyV2Decrypter is a stub encrypter whose decrypt step yields a +// legacy v2 (RSA1024) payload, used to exercise the post-decrypt +// validation branch. +type legacyV2Decrypter struct{} + +func (legacyV2Decrypter) EncryptPayloadToWriter(_ []byte, _ io.Writer) error { + return nil +} + +func (legacyV2Decrypter) DecryptPayloadFromReader(_ io.Reader) ([]byte, + error) { + + return []byte("RSA1024:legacy-v2-key-bytes"), nil +} diff --git a/tor/tor.go b/tor/tor.go index 37d3fc2892f..ce615fb7d55 100644 --- a/tor/tor.go +++ b/tor/tor.go @@ -1,7 +1,6 @@ package tor import ( - "bytes" "crypto/rand" "encoding/hex" "fmt" @@ -14,42 +13,31 @@ import ( "golang.org/x/net/proxy" ) -var ( - // dnsCodes maps the DNS response codes to a friendly description. This - // does not include the BADVERS code because of duplicate keys and the - // underlying DNS (miekg/dns) package not using it. For more info, see - // https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml. - dnsCodes = map[int]string{ - 0: "no error", - 1: "format error", - 2: "server failure", - 3: "non-existent domain", - 4: "not implemented", - 5: "query refused", - 6: "name exists when it should not", - 7: "RR set exists when it should not", - 8: "RR set that should exist does not", - 9: "server not authoritative for zone", - 10: "name not contained in zone", - 16: "TSIG signature failure", - 17: "key not recognized", - 18: "signature out of time window", - 19: "bad TKEY mode", - 20: "duplicate key name", - 21: "algorithm not supported", - 22: "bad truncation", - 23: "bad/missing server cookie", - } - - // onionPrefixBytes is a special purpose IPv6 prefix to encode Onion v2 - // addresses with. Because Neutrino uses the address manager of btcd - // which only understands net.IP addresses instead of net.Addr, we need - // to convert any .onion addresses into fake IPv6 addresses if we want - // to use a Tor hidden service as a Neutrino backend. This is the same - // range used by OnionCat, which is part part of the RFC4193 unique - // local IPv6 unicast address range. - onionPrefixBytes = []byte{0xfd, 0x87, 0xd8, 0x7e, 0xeb, 0x43} -) +// dnsCodes maps the DNS response codes to a friendly description. This does +// not include the BADVERS code because of duplicate keys and the underlying +// DNS (miekg/dns) package not using it. For more info, see +// https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml. +var dnsCodes = map[int]string{ + 0: "no error", + 1: "format error", + 2: "server failure", + 3: "non-existent domain", + 4: "not implemented", + 5: "query refused", + 6: "name exists when it should not", + 7: "RR set exists when it should not", + 8: "RR set that should exist does not", + 9: "server not authoritative for zone", + 10: "name not contained in zone", + 16: "TSIG signature failure", + 17: "key not recognized", + 18: "signature out of time window", + 19: "bad TKEY mode", + 20: "duplicate key name", + 21: "algorithm not supported", + 22: "bad truncation", + 23: "bad/missing server cookie", +} // proxyConn is a wrapper around net.Conn that allows us to expose the actual // remote address we're dialing, rather than the proxy's address. @@ -292,60 +280,3 @@ func IsOnionHost(host string) bool { return true } - -// IsOnionFakeIP checks whether a given net.Addr is a fake IPv6 address that -// encodes an Onion v2 address. -func IsOnionFakeIP(addr net.Addr) bool { - _, err := FakeIPToOnionHost(addr) - return err == nil -} - -// OnionHostToFakeIP encodes an Onion v2 address into a fake IPv6 address that -// encodes the same information but can be used for libraries that operate on an -// IP address base only, like btcd's address manager. For example, this will -// turn the onion host ld47qlr6h2b7hrrf.onion into the ip6 address -// fd87:d87e:eb43:58f9:f82e:3e3e:83f3:c625. -func OnionHostToFakeIP(host string) (net.IP, error) { - if len(host) != V2Len { - return nil, fmt.Errorf("invalid onion v2 host: %v", host) - } - - data, err := Base32Encoding.DecodeString(host[:V2Len-OnionSuffixLen]) - if err != nil { - return nil, err - } - - ip := make([]byte, len(onionPrefixBytes)+len(data)) - copy(ip, onionPrefixBytes) - copy(ip[len(onionPrefixBytes):], data) - return ip, nil -} - -// FakeIPToOnionHost turns a fake IPv6 address that encodes an Onion v2 address -// back into its onion host address representation. For example, this will turn -// the fake tcp6 address [fd87:d87e:eb43:58f9:f82e:3e3e:83f3:c625]:8333 back -// into ld47qlr6h2b7hrrf.onion:8333. -func FakeIPToOnionHost(fakeIP net.Addr) (net.Addr, error) { - tcpAddr, ok := fakeIP.(*net.TCPAddr) - if !ok { - return nil, fmt.Errorf("invalid fake onion IP address: %v", - fakeIP) - } - - ip := tcpAddr.IP - if len(ip) != len(onionPrefixBytes)+V2DecodedLen { - return nil, fmt.Errorf("invalid fake onion IP address length: "+ - "%v", fakeIP) - } - - if !bytes.Equal(ip[:len(onionPrefixBytes)], onionPrefixBytes) { - return nil, fmt.Errorf("invalid fake onion IP address prefix: "+ - "%v", fakeIP) - } - - host := Base32Encoding.EncodeToString(ip[len(onionPrefixBytes):]) - return &OnionAddr{ - OnionService: host + ".onion", - Port: tcpAddr.Port, - }, nil -} diff --git a/tor/tor_test.go b/tor/tor_test.go deleted file mode 100644 index 594b77df11f..00000000000 --- a/tor/tor_test.go +++ /dev/null @@ -1,36 +0,0 @@ -package tor - -import ( - "fmt" - "net" - "testing" - - "github.com/stretchr/testify/require" -) - -const ( - testOnion = "ld47qlr6h2b7hrrf.onion" - testFakeIP = "fd87:d87e:eb43:58f9:f82e:3e3e:83f3:c625" -) - -// TestOnionHostToFakeIP tests that an onion host address can be converted into -// a fake tcp6 address successfully. -func TestOnionHostToFakeIP(t *testing.T) { - ip, err := OnionHostToFakeIP(testOnion) - require.NoError(t, err) - require.Equal(t, testFakeIP, ip.String()) -} - -// TestFakeIPToOnionHost tests that a fake tcp6 address can be converted back -// into its original .onion host address successfully. -func TestFakeIPToOnionHost(t *testing.T) { - tcpAddr, err := net.ResolveTCPAddr( - "tcp6", fmt.Sprintf("[%s]:8333", testFakeIP), - ) - require.NoError(t, err) - require.True(t, IsOnionFakeIP(tcpAddr)) - - onionHost, err := FakeIPToOnionHost(tcpAddr) - require.NoError(t, err) - require.Equal(t, fmt.Sprintf("%s:8333", testOnion), onionHost.String()) -} diff --git a/watchtower/config.go b/watchtower/config.go index f79de9cacbc..2fba58f3708 100644 --- a/watchtower/config.go +++ b/watchtower/config.go @@ -103,8 +103,4 @@ type Config struct { // KeyRing is the KeyRing to use when encrypting the Tor private key. KeyRing keychain.KeyRing - - // Type specifies the hidden service type (V2 or V3) that the watchtower - // will create. - Type tor.OnionType } diff --git a/watchtower/standalone.go b/watchtower/standalone.go index 55200120904..df26641de1c 100644 --- a/watchtower/standalone.go +++ b/watchtower/standalone.go @@ -158,8 +158,8 @@ func (w *Standalone) Stop() error { return nil } -// createNewHiddenService automatically sets up a v2 or v3 onion service in -// order to listen for inbound connections over Tor. +// createNewHiddenService automatically sets up a v3 onion service in order to +// listen for inbound connections over Tor. func (w *Standalone) createNewHiddenService() error { // Get all the ports the watchtower is listening on. These will be used to // map the hidden service's virtual port. @@ -184,7 +184,6 @@ func (w *Standalone) createNewHiddenService() error { w.cfg.WatchtowerKeyPath, 0600, w.cfg.EncryptKey, encrypter, ), - Type: w.cfg.Type, } addr, err := w.cfg.TorController.AddOnion(onionCfg) diff --git a/watchtower/wtclient/client.go b/watchtower/wtclient/client.go index 8d74e9d1f3d..f8f3d3e618b 100644 --- a/watchtower/wtclient/client.go +++ b/watchtower/wtclient/client.go @@ -303,6 +303,15 @@ func getTowerAndSessionCandidates(db DB, keyRing ECDHKeyRing, candidateSessions := make(map[wtdb.SessionID]*ClientSession) for _, dbTower := range towers { tower, err := NewTowerFromDBTower(dbTower) + if errors.Is(err, ErrTowerOnlyV2Onion) { + log.Warnf("Skipping tower %x: all persisted "+ + "addresses are Tor v2 .onion which is no "+ + "longer supported; add a fresh v3 address "+ + "to re-activate this tower", + dbTower.IdentityKey.SerializeCompressed()) + + continue + } if err != nil { return nil, err } diff --git a/watchtower/wtclient/interface.go b/watchtower/wtclient/interface.go index 1051da0d59f..48252e2e943 100644 --- a/watchtower/wtclient/interface.go +++ b/watchtower/wtclient/interface.go @@ -1,6 +1,7 @@ package wtclient import ( + "errors" "net" "github.com/btcsuite/btcd/btcec/v2" @@ -12,6 +13,12 @@ import ( "github.com/lightningnetwork/lnd/watchtower/wtserver" ) +// ErrTowerOnlyV2Onion is returned when a persisted tower has no usable +// addresses left after Tor v2 .onion entries are filtered out. The tower +// record is preserved on disk so an operator can attach a fresh v3 address +// for the same identity key. +var ErrTowerOnlyV2Onion = errors.New("tower has no non-v2-onion addresses") + // DB abstracts the required database operations required by the watchtower // client. type DB interface { @@ -184,10 +191,53 @@ type Tower struct { Addresses AddressIterator } +// isV2OnionAddr reports whether addr is a Tor v2 .onion address. Tor stopped +// serving v2 onion services in October 2021, so callers skip these on dial +// paths. Storage and gossip re-broadcast still preserve v2 byte-for-byte to +// keep peer-signed NodeAnnouncement signatures verifiable. +// +// TODO: move this helper into the `tor` module (as `tor.IsV2Onion`) and remove +// this copy along with the duplicate in the root server.go once a new `tor` +// module version is cut and the dependency is bumped. +func isV2OnionAddr(addr net.Addr) bool { + onion, ok := addr.(*tor.OnionAddr) + if !ok { + return false + } + + return len(onion.OnionService) == tor.V2Len +} + +// withoutV2Onion returns addrs with any Tor v2 .onion entries removed. See +// isV2OnionAddr for the rationale. +// +// TODO: move this helper into the `tor` module and remove this copy along +// with the duplicate in the root server.go once a new `tor` module version +// is cut and the dependency is bumped. +func withoutV2Onion(addrs []net.Addr) []net.Addr { + filtered := make([]net.Addr, 0, len(addrs)) + for _, addr := range addrs { + if isV2OnionAddr(addr) { + continue + } + filtered = append(filtered, addr) + } + + return filtered +} + // NewTowerFromDBTower converts a wtdb.Tower, which uses a static address list, -// into a Tower which uses an address iterator. +// into a Tower which uses an address iterator. Persisted Tor v2 .onion +// addresses are filtered out so an upgraded node never attempts to dial them; +// if filtering leaves zero usable addresses, ErrTowerOnlyV2Onion is returned +// and the caller is expected to skip the tower without modifying the DB. func NewTowerFromDBTower(t *wtdb.Tower) (*Tower, error) { - addrs, err := newAddressIterator(t.Addresses...) + filtered := withoutV2Onion(t.Addresses) + if len(filtered) == 0 { + return nil, ErrTowerOnlyV2Onion + } + + addrs, err := newAddressIterator(filtered...) if err != nil { return nil, err } diff --git a/watchtower/wtclient/interface_test.go b/watchtower/wtclient/interface_test.go new file mode 100644 index 00000000000..e65715fa7d7 --- /dev/null +++ b/watchtower/wtclient/interface_test.go @@ -0,0 +1,57 @@ +package wtclient + +import ( + "net" + "testing" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/lightningnetwork/lnd/tor" + "github.com/lightningnetwork/lnd/watchtower/wtdb" + "github.com/stretchr/testify/require" +) + +// TestNewTowerFromDBTowerFiltersV2Onion asserts that NewTowerFromDBTower drops +// any persisted Tor v2 .onion entries before constructing the address +// iterator, that mixed lists still surface the remaining v3/tcp addresses, and +// that a tower whose addresses are exclusively v2 surfaces +// ErrTowerOnlyV2Onion so the caller can skip it without touching the DB. +func TestNewTowerFromDBTowerFiltersV2Onion(t *testing.T) { + t.Parallel() + + priv, err := btcec.NewPrivateKey() + require.NoError(t, err) + + v2 := &tor.OnionAddr{ + OnionService: "3g2upl4pq6kufc4m.onion", + Port: 9911, + } + v3 := &tor.OnionAddr{ + OnionService: "4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnz" + + "jpbi7utijcltosqemad.onion", + Port: 9911, + } + tcp := &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 9911} + + t.Run("mixed addresses keep v3/tcp", func(t *testing.T) { + t.Parallel() + + tower, err := NewTowerFromDBTower(&wtdb.Tower{ + ID: 7, + IdentityKey: priv.PubKey(), + Addresses: []net.Addr{v2, v3, tcp, v2}, + }) + require.NoError(t, err) + require.Equal(t, []net.Addr{v3, tcp}, tower.Addresses.GetAll()) + }) + + t.Run("only v2 addresses returns sentinel", func(t *testing.T) { + t.Parallel() + + _, err := NewTowerFromDBTower(&wtdb.Tower{ + ID: 7, + IdentityKey: priv.PubKey(), + Addresses: []net.Addr{v2, v2}, + }) + require.ErrorIs(t, err, ErrTowerOnlyV2Onion) + }) +} diff --git a/watchtower/wtdb/codec_test.go b/watchtower/wtdb/codec_test.go index 3f2e59864e2..e4a05eee939 100644 --- a/watchtower/wtdb/codec_test.go +++ b/watchtower/wtdb/codec_test.go @@ -59,24 +59,6 @@ func randTCP6Addr(r *rand.Rand) (*net.TCPAddr, error) { return &net.TCPAddr{IP: addrIP, Port: addrPort}, nil } -func randV2OnionAddr(r *rand.Rand) (*tor.OnionAddr, error) { - var serviceID [tor.V2DecodedLen]byte - if _, err := r.Read(serviceID[:]); err != nil { - return nil, err - } - - var port [2]byte - if _, err := r.Read(port[:]); err != nil { - return nil, err - } - - onionService := tor.Base32Encoding.EncodeToString(serviceID[:]) - onionService += tor.OnionSuffix - addrPort := int(binary.BigEndian.Uint16(port[:])) - - return &tor.OnionAddr{OnionService: onionService, Port: addrPort}, nil -} - func randV3OnionAddr(r *rand.Rand) (*tor.OnionAddr, error) { var serviceID [tor.V3DecodedLen]byte if _, err := r.Read(serviceID[:]); err != nil { @@ -106,17 +88,12 @@ func randAddrs(r *rand.Rand) ([]net.Addr, error) { return nil, err } - v2OnionAddr, err := randV2OnionAddr(r) - if err != nil { - return nil, err - } - v3OnionAddr, err := randV3OnionAddr(r) if err != nil { return nil, err } - return []net.Addr{tcp4Addr, tcp6Addr, v2OnionAddr, v3OnionAddr}, nil + return []net.Addr{tcp4Addr, tcp6Addr, v3OnionAddr}, nil } // dbObject is abstract object support encoding and decoding.