fix(workers): put a dialable address in the connection string
Found on hardware. With the listener bound to 0.0.0.0 — which is what sharing a GPU across a network requires — the issued string came out as ovnode://...@0.0.0.0:7444. That is a legal bind and a meaningless destination, so it would have failed on the far end with a connection error naming nothing, and the person who pasted it had no way to tell a bad string from a firewall. The string is now built from an advertised address rather than the bind: for a wildcard bind, the source address the routing table would use to leave this machine, found with a connected UDP socket that sends no packets and needs no DNS. An explicitly typed bind is advertised verbatim, because someone who entered a specific address meant it.
This commit is contained in:
@@ -96,6 +96,40 @@ def set_bind_port(value: int) -> None:
|
||||
_set_setting(_PORT_KEY, str(int(value)))
|
||||
|
||||
|
||||
# Addresses that are legal to BIND but meaningless to DIAL. A connection
|
||||
# string built from one of these is broken for the person who receives it, and
|
||||
# broken in the least diagnosable way: it looks like a perfectly good address.
|
||||
_WILDCARD_BINDS = frozenset({"0.0.0.0", "::", "[::]", "*", ""})
|
||||
|
||||
|
||||
def advertised_host() -> str:
|
||||
"""The address to put in a connection string.
|
||||
|
||||
Not the bind address. Binding to 0.0.0.0 means "every interface", which is
|
||||
exactly what you want for listening and exactly what you cannot hand to
|
||||
somebody else — verified on hardware, where the string came out as
|
||||
`ovnode://…@0.0.0.0:7444` and would have failed on the far end with a
|
||||
connection error that names nothing.
|
||||
"""
|
||||
host = bind_host()
|
||||
if host not in _WILDCARD_BINDS:
|
||||
return host
|
||||
|
||||
# Ask the routing table which source address would be used to reach the
|
||||
# outside world. No packets are sent — a connected UDP socket only fixes
|
||||
# the local endpoint — so this works with no network and no DNS.
|
||||
import socket # noqa: PLC0415
|
||||
|
||||
sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
|
||||
try:
|
||||
sock.connect(("192.0.2.1", 9)) # TEST-NET-1: reserved, never routed
|
||||
return sock.getsockname()[0]
|
||||
except OSError:
|
||||
return ""
|
||||
finally:
|
||||
sock.close()
|
||||
|
||||
|
||||
def is_exposed(host: Optional[str] = None) -> bool:
|
||||
"""True when the listener is reachable from other machines.
|
||||
|
||||
@@ -215,9 +249,14 @@ class InboundNode:
|
||||
await listener.stop()
|
||||
|
||||
def connection_string(self, secret: str, *, host: Optional[str] = None) -> str:
|
||||
"""The one artifact a user copies to another machine."""
|
||||
"""The one artifact a user copies to another machine.
|
||||
|
||||
Built from the ADVERTISED host, never the bind — see `advertised_host`.
|
||||
"""
|
||||
return format_connection(
|
||||
host=host or bind_host(), port=self.port or bind_port(), secret=secret
|
||||
host=host or advertised_host() or bind_host(),
|
||||
port=self.port or bind_port(),
|
||||
secret=secret,
|
||||
)
|
||||
|
||||
def snapshot(self) -> dict:
|
||||
|
||||
@@ -218,3 +218,33 @@ def test_inbound_is_off_unless_it_was_turned_on(monkeypatch):
|
||||
monkeypatch.setattr(inbound_service, "_setting", lambda name, default="": default)
|
||||
|
||||
assert inbound_service.enabled() is False
|
||||
|
||||
|
||||
def test_a_wildcard_bind_never_reaches_the_connection_string(monkeypatch):
|
||||
"""0.0.0.0 is legal to bind and meaningless to dial.
|
||||
|
||||
Found on hardware: with the listener bound to every interface, the issued
|
||||
string came out as ovnode://…@0.0.0.0:7444, which fails on the far end with
|
||||
a connection error that names nothing. The string has to carry an address
|
||||
the other machine can actually reach.
|
||||
"""
|
||||
from worker.inbound import service as inbound_service
|
||||
|
||||
monkeypatch.setattr(inbound_service, "bind_host", lambda: "0.0.0.0")
|
||||
monkeypatch.setattr(inbound_service, "bind_port", lambda: 7444)
|
||||
|
||||
node = inbound_service.InboundNode()
|
||||
text = node.connection_string("ovnode_" + "k" * 40)
|
||||
|
||||
assert "0.0.0.0" not in text
|
||||
assert parse_connection(text).host not in ("0.0.0.0", "", "*")
|
||||
|
||||
|
||||
def test_an_explicit_bind_is_advertised_as_given(monkeypatch):
|
||||
"""Only wildcards are substituted — a user who typed a specific address
|
||||
meant that address, including one this host cannot introspect."""
|
||||
from worker.inbound import service as inbound_service
|
||||
|
||||
monkeypatch.setattr(inbound_service, "bind_host", lambda: "192.168.0.202")
|
||||
|
||||
assert inbound_service.advertised_host() == "192.168.0.202"
|
||||
|
||||
Reference in New Issue
Block a user