diff --git a/storage/conf.nim b/storage/conf.nim index af23bd0d..618d3d4f 100644 --- a/storage/conf.nim +++ b/storage/conf.nim @@ -53,7 +53,7 @@ export DefaultQuotaBytes, DefaultBlockTtl, DefaultBlockInterval, DefaultNumBlocksPerInterval, DefaultBlockRetries -const DefaultNatScheduleInterval* = 5.minutes +const DefaultNatScheduleInterval* = 2.minutes type ThreadCount* = distinct Natural diff --git a/storage/storage.nim b/storage/storage.nim index 20c1e018..958dcd64 100644 --- a/storage/storage.nim +++ b/storage/storage.nim @@ -313,6 +313,11 @@ proc new*( switchBuilder = switchBuilder.withObservedAddrManager( ObservedAddrManager.new(minCount = observedAddrMinCount) ) + # libp2p keeps the private address in peerInfo.addrs. + # Since Autonat V2 uses the observed public address, + # we can filter the private addresses to keep only the dialable + # addresses. + switchBuilder = switchBuilder.withAddressPolicy(dialableAddressPolicy) var natRouter: Option[NatRouter] let switch = @@ -469,9 +474,14 @@ proc new*( maxNumRelays = config.natMaxRelays, client = relayClient, onReservation = proc(addresses: seq[MultiAddress]) {.gcsafe, raises: [].} = - info "Relay reservation updated", addresses + # A relay server is required to have a public extip, so its + # circuit addresses always include a public one. The relay's reservation + # response can also carry loopback/private addresses: + # they are never dialable by a remote peer, so drop them. + let publicAddrs = addresses.filterIt(it.hasPublicRelayTransport()) + info "Relay reservation updated", addresses = publicAddrs # relay addresses are for download traffic only, not DHT routing - discovery.announceRelayAddrs(addresses), + discovery.announceRelayAddrs(publicAddrs), rng = random.Rng.instance(), ) diff --git a/storage/utils/addrutils.nim b/storage/utils/addrutils.nim index 600a38f5..f142cfec 100644 --- a/storage/utils/addrutils.nim +++ b/storage/utils/addrutils.nim @@ -14,6 +14,7 @@ import std/strutils import std/options import pkg/libp2p +import pkg/libp2p/wire import pkg/stew/endians2 func remapAddr*( @@ -58,9 +59,29 @@ func getTcpPort*(ma: MultiAddress): Option[Port] = return Port.none some(Port(fromBytesBE(uint16, portBytes.get()))) +proc hasPublicRelayTransport*(ma: MultiAddress): bool = + ## True when ``ma`` is a circuit address whose relay is publicly dialable. + ## A circuit address is /p2p//p2p-circuit; the part + ## before /p2p/ is the relay's wire address, which isPublicMA can check. + ## Unlike libp2p's publicRoutableAddressPolicy we drop non-public relays: our + ## relay path only runs on a genuine NAT, where the relay is always public. + let relayWireStr = ($ma).split("/p2p/")[0] + let relayWireAddr = MultiAddress.init(relayWireStr).valueOr: + return false + relayWireAddr.isPublicMA() + +proc dialableAddressPolicy*(ma: MultiAddress): bool {.gcsafe, raises: [].} = + # Use with switchBuilder.withAddressPolicy. + # Filter the peerInfo.addrs updated by libp2p without + # declaring another address mapper. + if ma.isCircuitRelayMA(): + ma.hasPublicRelayTransport() + else: + ma.isPublicMA() + proc getMultiAddrWithIpAndTcpPort*(ip: IpAddress, port: Port): MultiAddress = ## Creates a MultiAddress with the specified IP address and TCP port - ## + ## ## Parameters: ## - ip: A valid IP address (IPv4 or IPv6) ## - port: The TCP port number diff --git a/tests/storage/testaddrutils.nim b/tests/storage/testaddrutils.nim index b336fb1a..fc119e56 100644 --- a/tests/storage/testaddrutils.nim +++ b/tests/storage/testaddrutils.nim @@ -35,3 +35,43 @@ suite "addrutils - remapAddr": let ma = MultiAddress.init("/ip4/1.2.3.4/tcp/5000").expect("valid") let remapped = ma.remapAddr(ip = some(parseIpAddress("8.8.8.8"))) check remapped == MultiAddress.init("/ip4/8.8.8.8/tcp/5000").expect("valid") + +suite "addrutils - hasPublicRelayTransport": + const relayId = "16Uiu2HAkyRvHo1AyyQY1xiHC8QbYjXCHkZbneVC8dBtJjp1SZcGD" + + proc circuitAddr(relayIp: string): MultiAddress = + MultiAddress.init("/ip4/" & relayIp & "/tcp/8070/p2p/" & relayId & "/p2p-circuit").expect( + "valid" + ) + + test "true when the relay has a public ip": + check circuitAddr("204.168.234.45").hasPublicRelayTransport() + + test "false when the relay is loopback": + check not circuitAddr("127.0.0.1").hasPublicRelayTransport() + + test "false when the relay is a private ip": + check not circuitAddr("172.17.0.1").hasPublicRelayTransport() + +suite "addrutils - dialableAddressPolicy": + const relayId = "16Uiu2HAkyRvHo1AyyQY1xiHC8QbYjXCHkZbneVC8dBtJjp1SZcGD" + + proc circuitAddr(relayIp: string): MultiAddress = + MultiAddress.init("/ip4/" & relayIp & "/tcp/8070/p2p/" & relayId & "/p2p-circuit").expect( + "valid" + ) + + test "keeps a public direct address": + check MultiAddress.init("/ip4/204.168.234.45/tcp/8070").expect("valid").dialableAddressPolicy() + + test "drops a loopback direct address": + check not MultiAddress.init("/ip4/127.0.0.1/tcp/8070").expect("valid").dialableAddressPolicy() + + test "drops a private direct address": + check not MultiAddress.init("/ip4/192.168.100.103/tcp/8070").expect("valid").dialableAddressPolicy() + + test "keeps a circuit address through a public relay": + check circuitAddr("204.168.234.45").dialableAddressPolicy() + + test "drops a circuit address through a private relay": + check not circuitAddr("172.17.0.1").dialableAddressPolicy()