diff --git a/CHANGELOG.md b/CHANGELOG.md index c951b66e..1cbb2d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -272,6 +272,35 @@ archived by series under [docs/changelog/](docs/changelog/); see the measure the same bytes as `leaf`. A row reporting a number for a configuration nobody can build is worse than no row. +### Fixed + +- **A Python node was never routable by its `off1…` address over the Internet + transport.** The transport authenticated to the relay but never answered the + address-routing challenge, so the relay kept attributing the connection by + account name and could not resolve messages addressed to the node. Addressed + sends and service discovery therefore never reached it. The transport now + signs the same domain-separated proof the Swift and Kotlin bridges sign and + replies with `DeclareAddress`, pinned against the relay's own hex vector so + the three implementations cannot drift apart. Relays that do not advertise + `address_routing_v1` see byte-identical behaviour to before, and a + declaration that cannot be made leaves the connection authenticated and + working in account-name space, exactly as it worked before addresses + existed. + + The declaration is written before the outbox flush, because the relay + attributes each frame by whatever the connection has proved at the moment it + reads that frame and never re-stamps retroactively. Anything sent ahead of + the proof stays attributed by account name for good, and its address-stamped + `Message.sender` then fails the receiver's strict sender check, which is + what drops the key-package and welcome frames a new MLS session needs. + + The relay's two answers reach the core through `internet_address_declared` + and `internet_address_declaration_refused` rather than the log, so a Python + node performs the same binding-mismatch lockstep check as the other + bridges. The relay's advertised capabilities are also injected before the + status flip, so the group broadcast gate sees them on the connection they + belong to. + ## [0.23.0] — 2026-08-20 > **The protocol carries state now, not only messages.** A new data layer adds diff --git a/bindings/python/offline_protocol_sdk/address_declaration.py b/bindings/python/offline_protocol_sdk/address_declaration.py new file mode 100644 index 00000000..e09840e0 --- /dev/null +++ b/bindings/python/offline_protocol_sdk/address_declaration.py @@ -0,0 +1,207 @@ +"""Whether this connection proves its ``off1…`` address to the relay, and the +exact bytes it signs to do it. + +Mirrors ``bindings/react-native/ios/AddressDeclarationPolicy.swift`` and +``android/src/main/java/com/offlineprotocol/AddressDeclarationPolicy.kt``. +Keep the three in sync. + +Why the declaration exists +-------------------------- + +The relay authenticates a JWT and knows the connection by its *account name*, +but since the addressing cutover the core stamps ``Message.sender`` with +``local_address()``. A relay that attributes an inbound frame by account name +therefore hands the receiver a ``transport_peer_id`` that cannot match the +sender it is strict-matched against in ``validate_transport_sender``, and every +security-gated control frame (``__MLS_KEY_PKG__``, ``__MLS_WELCOME__``) is +rejected, so no MLS session can be established over the relay at all. Declaring +closes that, and is also what makes an ``off1…`` recipient resolvable in the +relay's registry. + +Why the account name is inside the signed bytes +----------------------------------------------- + +The signing key here is the identity key that also signs mesh control frames +under ``offline-ctrl-v1``. If the proof were a signature over a bare +relay-chosen challenge, a hostile relay could hand out a challenge shaped like a +control-frame payload and harvest a signature that replays as a control frame +from this peer. Binding the domain *and* the account makes the signature +meaningful only as "this account, on this relay, holds this key": it cannot be +replayed under another account, nor onto another connection, since the challenge +is minted per connection. + +The layout is fixed by the relay's ``address_binding::address_proof_payload`` +and pinned there by a hex vector, which +``test_address_declaration.py::test_proof_payload_matches_the_pinned_relay_vector`` +mirrors byte for byte. Neither side may change it alone. + +Why every failure is a skip rather than an error +------------------------------------------------ + +A connection that does not declare keeps working exactly as it did before +addresses existed: the relay attributes it by account name, which is the legacy +path and still the only path an older relay has. So the absence of a capability, +of a challenge, or of a local identity is a reason to stay quiet, not to fail a +connection that is otherwise fine. +""" + +from __future__ import annotations + +import base64 +import json +from dataclasses import dataclass +from typing import Any, Union + +# The relay capability token gating the whole exchange. A relay that does not +# advertise it also omits ``address_challenge``, and would parse a +# ``DeclareAddress`` into nothing and answer nothing, so the token is the tell, +# not a timeout. +CAPABILITY = "address_routing_v1" + +# Domain separator prefixing the signed payload. Must not prefix, nor be +# prefixed by, the core's ``offline-ctrl-v1`` control-frame domain; the relay +# pins that relation in +# ``the_proof_domain_cannot_collide_with_control_message_signing``. +PROOF_DOMAIN = b"offline-relay-addr-v1" + +# The relay mints exactly this many challenge bytes. A frame carrying any other +# length is malformed, and signing it would produce a proof that cannot verify, +# so it is better to skip and say so. +CHALLENGE_LENGTH = 32 + + +class Reason: + """Stable diagnostic reasons, shared with the Swift and Kotlin mirrors so a + field report reproduces under the same string on all three platforms. + + The vocabulary diverges at two points, both deliberate. Swift and Kotlin + also define ``frame_unserializable``, which Python cannot reach: + ``json.dumps`` over three ``str`` values is total, where + ``JSONSerialization`` and ``JSONObject`` are fallible, so the constant + would be a token nothing can ever emit. Going the other way, + ``connection_replaced`` is Python-only: the other two bridges make the same + stale-socket check but return from it silently, so there is no shared + string to match. + """ + + #: The relay does not advertise ``address_routing_v1``, an older + #: deployment. Expected, and the reason this is not an error. + CAPABILITY_ABSENT = "capability_absent" + #: Capability advertised but no ``address_challenge`` came with it. + CHALLENGE_ABSENT = "challenge_absent" + #: The challenge was not standard base64, or did not decode to exactly + #: :data:`CHALLENGE_LENGTH` bytes. + CHALLENGE_MALFORMED = "challenge_malformed" + #: The ``Authenticated`` frame carried no account name to bind the proof + #: to. Never sign a locally-chosen substitute here: the relay verifies + #: against the name *it* resolved, so a guess produces a signature that + #: cannot verify and is indistinguishable, in the relay's logs, from an + #: attack. + ACCOUNT_ABSENT = "account_absent" + #: ``local_address()`` was None, so MLS is not initialized and there is no + #: identity to prove. An app running with encryption disabled stays in + #: account-name space by construction. + ADDRESS_UNAVAILABLE = "address_unavailable" + #: The identity key or the signature could not be produced. + SIGNING_FAILED = "signing_failed" + #: The socket that was handed the challenge is no longer the live one. + #: Its successor carries its own ``Authenticated`` frame, and its own + #: challenge, so this connection has nothing to prove. + CONNECTION_REPLACED = "connection_replaced" + + +@dataclass(frozen=True) +class Declare: + """Sign :func:`proof_payload` for these values and send the declaration.""" + + account: str + challenge: bytes + + +@dataclass(frozen=True) +class Skip: + """Send nothing. ``reason`` is one of :class:`Reason`.""" + + reason: str + + +Outcome = Union[Declare, Skip] + + +def decide( + capabilities: Any, + address_challenge: str | None, + username: str | None, +) -> Outcome: + """Decide whether this connection can declare, from the ``Authenticated`` + frame alone. + + Deliberately does **not** take the local address: this runs before any FFI + call so that the common skip (an older relay) costs no acquisition of the + protocol mutex. The caller fetches the address only after a :class:`Declare` + and reports :attr:`Reason.ADDRESS_UNAVAILABLE` if it is None. + + :param capabilities: the ``capabilities`` field, verbatim and untrusted. + :param address_challenge: the ``address_challenge`` field, base64 of the raw + challenge bytes, absent on relays without the capability. + :param username: the ``username`` field **as the relay sent it**. Callers + must not substitute a local fallback (see :attr:`Reason.ACCOUNT_ABSENT`). + """ + # A relay is free to send anything. `in` over a str is a substring test, so + # a scalar "not_address_routing_v1" would satisfy a bare membership check; + # require the JSON array the schema promises. + if not isinstance(capabilities, list) or CAPABILITY not in capabilities: + return Skip(Reason.CAPABILITY_ABSENT) + if not address_challenge: + return Skip(Reason.CHALLENGE_ABSENT) + try: + # Standard alphabet, padding required: the relay decodes with the same, + # and rejects base64url or unpadded input. `validate=True` is what makes + # this a rejection rather than a silent discard of stray characters. + challenge = base64.b64decode(address_challenge, validate=True) + except Exception: + return Skip(Reason.CHALLENGE_MALFORMED) + if len(challenge) != CHALLENGE_LENGTH: + return Skip(Reason.CHALLENGE_MALFORMED) + if not username: + return Skip(Reason.ACCOUNT_ABSENT) + return Declare(account=username, challenge=challenge) + + +def proof_payload(account: str, challenge: bytes) -> bytes: + """The exact bytes the relay verifies the signature over. + + ``"offline-relay-addr-v1" || u32be(len(account.utf8)) || account.utf8 || challenge`` + + No separators, no terminators. The length prefix is what makes the + concatenation unambiguous: without it, an account name ending in bytes that + look like the start of a challenge could be re-split, so two different + (account, challenge) pairs would share one payload. + """ + account_bytes = account.encode("utf-8") + # Big-endian, and of the UTF-8 *byte* count, not the character count, which + # differs for any non-ASCII account name. + return ( + PROOF_DOMAIN + + len(account_bytes).to_bytes(4, "big") + + account_bytes + + challenge + ) + + +def declaration_json(address: str, public_key: bytes, signature: bytes) -> str: + """The ``DeclareAddress`` frame, serialized. + + Owns the base64 encoding so the wire contract lives in one place per + platform: standard alphabet, padded, no line breaks. A 32-byte key encodes + to 44 characters and a 64-byte signature to 88. The relay decodes with a + strict engine and refuses anything else. + """ + return json.dumps( + { + "type": "DeclareAddress", + "address": address, + "public_key": base64.b64encode(public_key).decode("ascii"), + "signature": base64.b64encode(signature).decode("ascii"), + } + ) diff --git a/bindings/python/offline_protocol_sdk/internet_manager.py b/bindings/python/offline_protocol_sdk/internet_manager.py index 7c8ec740..445bea5a 100644 --- a/bindings/python/offline_protocol_sdk/internet_manager.py +++ b/bindings/python/offline_protocol_sdk/internet_manager.py @@ -16,6 +16,7 @@ import websockets from websockets.asyncio.client import ClientConnection +from . import address_declaration from .transport_manager import TransportError, TransportManager, TransportState logger = logging.getLogger(__name__) @@ -277,10 +278,19 @@ async def _send_authentication(self) -> None: except Exception as exc: self._emit_diagnostic("error", f"Failed to send auth: {exc}") - async def _safe_handle_authenticated(self, user_id: str, username: str) -> None: + async def _safe_handle_authenticated( + self, + user_id: str, + username: str | None, + capabilities: Any, + address_challenge: str | None, + ws: ClientConnection | None, + ) -> None: """Wrapper that catches exceptions to prevent stuck STARTING state.""" try: - await self._handle_authenticated(user_id, username) + await self._handle_authenticated( + user_id, username, capabilities, address_challenge, ws + ) except Exception as exc: self._emit_diagnostic("error", f"Authentication handler failed: {exc}") await self._handle_connection_closed(exc) @@ -292,11 +302,51 @@ async def _safe_handle_connection_closed(self, error: Exception | None) -> None: except Exception as exc: self._emit_diagnostic("error", f"Connection-closed handler failed: {exc}") - async def _handle_authenticated(self, user_id: str, username: str) -> None: + async def _handle_authenticated( + self, + user_id: str, + username: str | None, + capabilities: Any, + address_challenge: str | None, + ws: ClientConnection | None, + ) -> None: self._authenticated = True self._update_state(TransportState.RUNNING) - # Notify protocol — triggers outbox flush + self._emit_diagnostic("info", "Authenticated with relay server", { + "user_id": user_id, + "username": username or self._device_id, + }) + + # The order of the three steps below is load-bearing, and mirrors the + # same block in the Swift and Kotlin managers. + # + # The address declaration MUST precede the status flip. The relay + # attributes each inbound frame by whatever this connection has proved + # at the moment it reads that frame, and never re-stamps retroactively, + # so a send that leaves before the declaration is attributed by account + # name for good. Its `Message.sender` (an address) then fails the + # receiver's `validate_transport_sender` at hop 0, which drops exactly + # the `__MLS_KEY_PKG__` / `__MLS_WELCOME__` frames a new session needs. + # The flush the status flip performs is what produces those sends, so + # the declaration goes out first and WebSocket frame order does the + # rest. + await self._maybe_declare_address( + username, capabilities, address_challenge, ws + ) + + # Capabilities MUST also reach the SDK before the status flip: the + # flush reads the group broadcast gate's capability set. The list is + # injected even when empty, so a stale set from a previous relay can + # never leak across connections. + try: + self._protocol.internet_relay_capabilities( + capabilities=capabilities if isinstance(capabilities, list) else [] + ) + except Exception as exc: + self._emit_diagnostic("error", f"Relay capability injection failed: {exc}") + + # Notify protocol: this is what triggers the outbox flush. try: self._protocol.internet_status_changed(is_connected=True) except Exception as exc: @@ -312,13 +362,99 @@ async def _handle_authenticated(self, user_id: str, username: str) -> None: self._poll_task = asyncio.ensure_future(self._poll_outgoing_loop()) self._ping_task = asyncio.ensure_future(self._ping_loop()) - # Immediate flush + # Immediate flush. The poll drains the internet send queue the status + # flip just filled, so messages queued while disconnected go out now + # rather than on the next timer tick. self._poll_and_send_messages() - self._emit_diagnostic("info", "Authenticated with relay server", { - "user_id": user_id, - "username": username, - }) + async def _maybe_declare_address( + self, + username: str | None, + capabilities: Any, + address_challenge: str | None, + ws: ClientConnection | None, + ) -> None: + """Answer the relay's address challenge with a signed ``DeclareAddress``. + + No-op unless the relay advertised ``address_routing_v1`` and sent a + well-formed challenge, so relays without the capability are unaffected. + + Never raises into the auth path. An undeclared connection is a working + connection: the relay simply attributes it the legacy way, which is how + it behaved before addresses existed. Failing the connection over a + refused declaration would turn a degraded path into no path. + + Nothing waits on the relay's answer. The relay binds the address before + it reads the next frame off the socket, so ordering is established by + the write alone; ``AddressDeclared`` and ``AddressError`` are reported + when they arrive but gate nothing. + """ + outcome = address_declaration.decide(capabilities, address_challenge, username) + if isinstance(outcome, address_declaration.Skip): + self._emit_diagnostic( + "debug", + "Not declaring an address to the relay", + {"reason": outcome.reason}, + ) + return + + # The declaration belongs to the connection that was handed the + # challenge. A socket replaced between that frame's arrival and this + # coroutine has its own `Authenticated`, carrying its own challenge, + # already on the way. + if ws is None or ws is not self._ws: + self._emit_diagnostic( + "debug", + "Not declaring an address to the relay", + {"reason": address_declaration.Reason.CONNECTION_REPLACED}, + ) + return + + # The address the core compares the relay's answer against, so the + # declaration must carry this value and not a locally re-derived one. + # Absent before `initialize_mls`: an app running with encryption + # disabled stays in account-name space by construction, which is a + # clean skip rather than a failure. + try: + address = self._protocol.local_address() + except Exception: + address = None + if not address: + self._emit_diagnostic( + "debug", + "Not declaring an address to the relay", + {"reason": address_declaration.Reason.ADDRESS_UNAVAILABLE}, + ) + return + + try: + payload = address_declaration.proof_payload( + outcome.account, outcome.challenge + ) + public_key = bytes(self._protocol.get_identity_public_key()) + signature = bytes(self._protocol.sign_data(list(payload))) + frame = address_declaration.declaration_json( + address, public_key, signature + ) + except Exception as exc: + self._emit_diagnostic( + "error", + f"Could not sign the address declaration: {exc}", + {"reason": address_declaration.Reason.SIGNING_FAILED}, + ) + return + + try: + await ws.send(frame) + except Exception as exc: + self._emit_diagnostic( + "warning", f"Address declaration write failed: {exc}" + ) + return + + self._emit_diagnostic( + "info", "Sent DeclareAddress to relay", {"address": address} + ) async def _handle_connection_closed(self, error: Exception | None) -> None: # Re-entrancy guard. On AuthError + WS-close, two paths race here: @@ -469,9 +605,66 @@ def _process_received(self, data: bytes) -> None: if msg_type == "Authenticated": user_id = msg.get("user_id", self._device_id) - username = msg.get("username", self._device_id) + # RAW, with no `device_id` fallback: this name goes into the signed + # address proof, and the relay verifies it against the account name + # *it* resolved. Signing a locally-chosen substitute produces a + # signature that cannot verify and, in the relay's logs, is + # indistinguishable from an attack. The fallback lives at the + # display sites instead. + username = msg.get("username") + capabilities = msg.get("capabilities", []) + address_challenge = msg.get("address_challenge") + # The socket this frame arrived on. `_process_received` runs inline + # in the receive loop, so `_ws` here is that socket; by the time the + # spawned task runs it may not be. self._spawn_process_task( - self._safe_handle_authenticated(user_id, username) + self._safe_handle_authenticated( + user_id, username, capabilities, address_challenge, self._ws + ) + ) + + elif msg_type == "AddressDeclared": + # The relay bound this connection to the address we proved. From its + # next inbound frame on, it attributes us by address instead of + # account name. Informational: the binding took effect before the + # relay answered (its frame loop is sequential), so nothing waits on + # this. + # + # The echo goes to the SDK, which is where the lockstep check lives: + # it compares the bound address against `local_address()` and + # reports a disagreement as RELAY_ADDRESS_BINDING_MISMATCH. A + # dedicated entry point, not message-plane injection, so an + # acknowledgement cannot be synthesized through the notification + # ciphertext injector. + declared_address = msg.get("address", "") + try: + self._protocol.internet_address_declared(address=declared_address) + except Exception as exc: + logger.debug("internet_address_declared failed: %s", exc) + self._emit_diagnostic( + "info", + "Relay accepted the address declaration", + {"address": declared_address}, + ) + + elif msg_type == "AddressError": + # The refusal frame is `AddressError` (relay `ServerMessage`), not + # the name of the FFI entry point it feeds. Non-fatal by contract: + # the connection stays authenticated and keeps working in + # account-name space. No retry here, because a refusal is either + # permanent for this connection (bad material, or a different + # address already declared) or means this socket was displaced by a + # newer login, in which case the successor declares for itself. The + # next reconnect re-declares from scratch. + reason = msg.get("reason", "Unknown error") + try: + self._protocol.internet_address_declaration_refused(reason=reason) + except Exception as exc: + logger.debug("internet_address_declaration_refused failed: %s", exc) + self._emit_diagnostic( + "error", + "Relay refused the address declaration; staying in account-name space", + {"reason": reason}, ) elif msg_type == "AuthError": diff --git a/bindings/python/tests/test_address_declaration.py b/bindings/python/tests/test_address_declaration.py new file mode 100644 index 00000000..e92d828e --- /dev/null +++ b/bindings/python/tests/test_address_declaration.py @@ -0,0 +1,146 @@ +"""Cross-repo contract for the relay address declaration. + +The signed byte layout is fixed by the relay's +``address_binding::address_proof_payload`` and pinned there by a hex vector. +This suite mirrors that vector byte for byte, exactly as the Swift +``AddressDeclarationPolicyTests.proofPayloadMatchesThePinnedRelayVector`` and +the Kotlin ``AddressDeclarationPolicyTest.proofPayloadMatchesThePinnedRelayVector`` +do. Neither side may change the layout alone. + +A hand-rolled payload does not fail loudly. It produces a signature the relay +refuses, so the connection silently stays in account-name space and every +security-gated control frame it carries is dropped at the receiver. That is the +failure this vector exists to catch. + +If the format ever legitimately changes, re-derive the literal from the relay's +structure. Never edit it to match new output. +""" + +from __future__ import annotations + +import base64 +import json + +from offline_protocol_sdk import address_declaration as policy + +# The relay's own vector: account "alice", challenge = bytes 0x00..0x1f. +VECTOR_CHALLENGE = bytes(range(32)) +VECTOR_HEX = ( + "6f66666c696e652d72656c61792d616464722d7631" + "00000005" + "616c696365" + "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f" +) + + +class TestProofPayload: + def test_proof_payload_matches_the_pinned_relay_vector(self) -> None: + assert policy.proof_payload("alice", VECTOR_CHALLENGE).hex() == VECTOR_HEX + + def test_account_length_prefix_is_big_endian(self) -> None: + """A little-endian binding writes ``05000000`` here and produces a + signature the relay refuses. Isolated so a failure names the cause.""" + payload = policy.proof_payload("alice", VECTOR_CHALLENGE) + domain_len = len(policy.PROOF_DOMAIN) + assert payload[domain_len : domain_len + 4].hex() == "00000005" + + def test_account_length_counts_utf8_bytes_not_characters(self) -> None: + """The relay reads the same bytes back out, so a character count + silently mis-frames every account name outside ASCII.""" + account = "zoë" # 3 characters, 4 UTF-8 bytes + payload = policy.proof_payload(account, VECTOR_CHALLENGE) + domain_len = len(policy.PROOF_DOMAIN) + assert payload[domain_len : domain_len + 4].hex() == "00000004" + assert len(payload) == domain_len + 4 + 4 + 32 + + def test_the_account_length_prefix_makes_the_payload_unambiguous(self) -> None: + """Without the prefix, ("ab", "cX") and ("abc", "X") concatenate to the + same bytes and one signature covers both, so an account able to pick a + name that re-splits into another's would inherit its proofs.""" + assert policy.proof_payload("ab", b"cX") != policy.proof_payload("abc", b"X") + + def test_proof_domain_cannot_collide_with_control_frame_signing(self) -> None: + """If either domain prefixed the other, a relay-chosen challenge could + steer this signature into the control-message domain and replay it as a + frame from this peer. The relay pins the same relation from its side.""" + control_domain = b"offline-ctrl-v1" + assert not policy.PROOF_DOMAIN.startswith(control_domain) + assert not control_domain.startswith(policy.PROOF_DOMAIN) + + def test_payload_is_never_the_bare_challenge(self) -> None: + """The naive implementation signs the challenge alone. The relay + refuses that shape explicitly.""" + payload = policy.proof_payload("alice", VECTOR_CHALLENGE) + assert payload != VECTOR_CHALLENGE + assert payload.startswith(policy.PROOF_DOMAIN) + + +class TestDecide: + @staticmethod + def _challenge_b64() -> str: + return base64.b64encode(VECTOR_CHALLENGE).decode("ascii") + + def test_declares_when_capability_challenge_and_account_are_present(self) -> None: + outcome = policy.decide( + ["group_delivery_v3", policy.CAPABILITY], self._challenge_b64(), "alice" + ) + assert outcome == policy.Declare(account="alice", challenge=VECTOR_CHALLENGE) + + def test_skips_when_the_relay_does_not_advertise_the_capability(self) -> None: + outcome = policy.decide(["group_delivery_v3"], self._challenge_b64(), "alice") + assert outcome == policy.Skip(policy.Reason.CAPABILITY_ABSENT) + + def test_a_scalar_capabilities_field_is_not_a_substring_match(self) -> None: + """`in` over a str is a substring test, so a relay sending a bare + string could satisfy a naive membership check with a token that merely + contains the capability name.""" + outcome = policy.decide( + "not_" + policy.CAPABILITY, self._challenge_b64(), "alice" + ) + assert outcome == policy.Skip(policy.Reason.CAPABILITY_ABSENT) + + def test_skips_when_the_challenge_is_absent(self) -> None: + assert policy.decide([policy.CAPABILITY], None, "alice") == policy.Skip( + policy.Reason.CHALLENGE_ABSENT + ) + assert policy.decide([policy.CAPABILITY], "", "alice") == policy.Skip( + policy.Reason.CHALLENGE_ABSENT + ) + + def test_skips_when_the_challenge_is_the_wrong_length(self) -> None: + short = base64.b64encode(b"\x00" * 31).decode("ascii") + assert policy.decide([policy.CAPABILITY], short, "alice") == policy.Skip( + policy.Reason.CHALLENGE_MALFORMED + ) + + def test_skips_when_the_challenge_is_not_standard_base64(self) -> None: + """The relay decodes with a strict engine: base64url and unpadded input + are refused there, so signing them here would waste a proof.""" + url_alphabet = ( + base64.urlsafe_b64encode(bytes([251] * 32)).decode("ascii").rstrip("=") + ) + assert policy.decide( + [policy.CAPABILITY], url_alphabet, "alice" + ) == policy.Skip(policy.Reason.CHALLENGE_MALFORMED) + + def test_skips_when_the_relay_supplied_no_account_name(self) -> None: + """The relay verifies the proof against the name *it* resolved. A + locally-chosen substitute cannot verify and reads as an attack in the + relay's logs, so the absence of a name is a skip and never a guess.""" + assert policy.decide( + [policy.CAPABILITY], self._challenge_b64(), None + ) == policy.Skip(policy.Reason.ACCOUNT_ABSENT) + + +class TestDeclarationJson: + def test_frame_uses_standard_padded_base64(self) -> None: + frame = json.loads( + policy.declaration_json("off1abc", bytes(range(32)), bytes(range(64))) + ) + assert frame["type"] == "DeclareAddress" + assert frame["address"] == "off1abc" + # A 32-byte key encodes to 44 characters, a 64-byte signature to 88. + assert len(frame["public_key"]) == 44 + assert len(frame["signature"]) == 88 + assert base64.b64decode(frame["public_key"], validate=True) == bytes(range(32)) + assert base64.b64decode(frame["signature"], validate=True) == bytes(range(64)) diff --git a/bindings/python/tests/test_internet_manager.py b/bindings/python/tests/test_internet_manager.py index d983b2cd..b8f21a8e 100644 --- a/bindings/python/tests/test_internet_manager.py +++ b/bindings/python/tests/test_internet_manager.py @@ -3,11 +3,13 @@ from __future__ import annotations import asyncio +import base64 import json from unittest.mock import AsyncMock, MagicMock import pytest +from offline_protocol_sdk import address_declaration as policy from offline_protocol_sdk.internet_manager import InternetManager from offline_protocol_sdk.transport_manager import TransportError, TransportState @@ -600,3 +602,277 @@ async def __aexit__(self_sem, *args): call_args = mock_protocol.internet_send_failed_with_reason.call_args assert call_args.kwargs["message_id"] == "msg-1" assert "Disconnected" in call_args.kwargs["reason"] + + +class TestAddressDeclaration: + """The relay address handshake, as wired into the manager. + + The signed bytes themselves are pinned in ``test_address_declaration.py`` + against the relay's own hex vector. What is pinned here is the wiring the + pure policy cannot see: the order the call sites run in, which name gets + signed, which address is declared, and where the relay's answers land. + """ + + CHALLENGE = bytes(range(32)) + CHALLENGE_B64 = base64.b64encode(CHALLENGE).decode("ascii") + ADDRESS = "off1q9eqy0ww55qxm8ve0jv8gxpxknay7fkj9veg0swe" + + @pytest.fixture + def declaring_protocol(self, mock_protocol: MagicMock) -> MagicMock: + mock_protocol.local_address = MagicMock(return_value=self.ADDRESS) + mock_protocol.get_identity_public_key = MagicMock( + return_value=list(bytes(range(32))) + ) + mock_protocol.sign_data = MagicMock(return_value=list(bytes(range(64)))) + mock_protocol.internet_relay_capabilities = MagicMock() + mock_protocol.internet_address_declared = MagicMock() + mock_protocol.internet_address_declaration_refused = MagicMock() + return mock_protocol + + @staticmethod + def _manager(proto: MagicMock) -> tuple[InternetManager, AsyncMock]: + mgr = InternetManager(proto, "dev-1", server_url="ws://x.com") + ws = AsyncMock() + mgr._ws = ws + return mgr, ws + + @staticmethod + async def _settle(mgr: InternetManager) -> None: + """Drain dispatch tasks and stop the loops authentication starts.""" + pending = list(mgr._process_tasks) + if pending: + await asyncio.gather(*pending) + loops = [t for t in (mgr._poll_task, mgr._ping_task) if t is not None] + for task in loops: + task.cancel() + if loops: + await asyncio.gather(*loops, return_exceptions=True) + + def _authenticated(self, **overrides: object) -> bytes: + frame: dict[str, object] = { + "type": "Authenticated", + "user_id": "u-1", + "username": "alice", + "capabilities": ["group_delivery_v3", "address_routing_v1"], + "address_challenge": self.CHALLENGE_B64, + } + frame.update(overrides) + return json.dumps(frame).encode() + + @pytest.mark.asyncio + async def test_declares_address_before_flipping_status( + self, declaring_protocol: MagicMock + ) -> None: + """The relay attributes each inbound frame by whatever this connection + has proved when it reads that frame, and never re-stamps retroactively. + `internet_status_changed(True)` triggers the outbox flush, so anything + it sends before the declaration lands is attributed by account name for + good, and its address-stamped `Message.sender` then fails the + receiver's `validate_transport_sender` at hop 0. Capabilities must + likewise precede the flip, which reads the group broadcast gate. + """ + mgr, ws = self._manager(declaring_protocol) + order: list[str] = [] + ws.send.side_effect = lambda frame: order.append("declare") + declaring_protocol.internet_relay_capabilities.side_effect = ( + lambda **_: order.append("capabilities") + ) + declaring_protocol.internet_status_changed.side_effect = ( + lambda **_: order.append("status") + ) + + await mgr._handle_authenticated( + "u-1", "alice", ["address_routing_v1"], self.CHALLENGE_B64, ws + ) + await self._settle(mgr) + + assert order == ["declare", "capabilities", "status"] + + @pytest.mark.asyncio + async def test_declaration_frame_matches_the_relay_contract( + self, declaring_protocol: MagicMock + ) -> None: + mgr, ws = self._manager(declaring_protocol) + + mgr._process_received(self._authenticated()) + await self._settle(mgr) + + frame = json.loads(ws.send.call_args.args[0]) + assert frame["type"] == "DeclareAddress" + assert frame["address"] == self.ADDRESS + # The bytes signed are the policy's, over the relay's account name. + signed = bytes(declaring_protocol.sign_data.call_args.args[0]) + assert signed == policy.proof_payload("alice", self.CHALLENGE) + + @pytest.mark.asyncio + async def test_declares_local_address_not_a_re_derived_one( + self, declaring_protocol: MagicMock + ) -> None: + """The core's lockstep check compares the relay's `AddressDeclared` + echo against `local_address()`. Declaring anything else would make this + node report RELAY_ADDRESS_BINDING_MISMATCH against its own proof.""" + declaring_protocol.local_address.return_value = "off1sentinel" + mgr, ws = self._manager(declaring_protocol) + + mgr._process_received(self._authenticated()) + await self._settle(mgr) + + assert json.loads(ws.send.call_args.args[0])["address"] == "off1sentinel" + + @pytest.mark.asyncio + @pytest.mark.parametrize("omit_key", [True, False]) + async def test_never_signs_the_device_id_when_the_relay_sends_no_username( + self, declaring_protocol: MagicMock, omit_key: bool + ) -> None: + """`username` is optional on the relay's `Authenticated`. The relay + verifies the proof against the name *it* resolved, so signing the local + device id produces a signature that cannot verify and is + indistinguishable, in the relay's logs, from an attack. + + Both wire shapes are covered because only one of them can catch the + defect. The relay declares `username: Option` without + `skip_serializing_if`, so a nameless account arrives today as an + explicit null, and `dict.get(key, fallback)` returns None for that + without ever consulting the fallback. It is the *absent* key that makes + a `.get("username", self._device_id)` answer with the local profile, so + a test that only sends null passes against the bug. + """ + mgr, ws = self._manager(declaring_protocol) + frame = json.loads(self._authenticated()) + if omit_key: + del frame["username"] + else: + frame["username"] = None + + mgr._process_received(json.dumps(frame).encode()) + await self._settle(mgr) + + declaring_protocol.sign_data.assert_not_called() + ws.send.assert_not_called() + # Authentication itself is unaffected. + declaring_protocol.internet_status_changed.assert_called_once_with( + is_connected=True + ) + + @pytest.mark.asyncio + async def test_skips_quietly_when_the_relay_lacks_the_capability( + self, declaring_protocol: MagicMock + ) -> None: + mgr, ws = self._manager(declaring_protocol) + + mgr._process_received( + self._authenticated(capabilities=[], address_challenge=None) + ) + await self._settle(mgr) + + ws.send.assert_not_called() + declaring_protocol.sign_data.assert_not_called() + assert mgr._authenticated is True + # The empty list is still injected, so a stale set from a previous + # relay cannot leak across connections. + declaring_protocol.internet_relay_capabilities.assert_called_once_with( + capabilities=[] + ) + + @pytest.mark.asyncio + async def test_skips_when_mls_is_not_initialized( + self, declaring_protocol: MagicMock + ) -> None: + """An app running with encryption disabled has no identity to prove and + stays in account-name space by construction. That is a clean skip, not + a signing failure.""" + declaring_protocol.local_address.return_value = None + mgr, ws = self._manager(declaring_protocol) + + mgr._process_received(self._authenticated()) + await self._settle(mgr) + + declaring_protocol.sign_data.assert_not_called() + ws.send.assert_not_called() + + @pytest.mark.asyncio + async def test_a_malformed_challenge_is_not_signed( + self, declaring_protocol: MagicMock + ) -> None: + mgr, ws = self._manager(declaring_protocol) + + mgr._process_received( + self._authenticated( + address_challenge=base64.b64encode(b"\x00" * 16).decode("ascii") + ) + ) + await self._settle(mgr) + + declaring_protocol.sign_data.assert_not_called() + ws.send.assert_not_called() + + @pytest.mark.asyncio + async def test_a_replaced_socket_does_not_inherit_the_challenge( + self, declaring_protocol: MagicMock + ) -> None: + """The challenge is minted per connection. A socket swapped in after + the frame arrived has its own `Authenticated` coming, so declaring the + predecessor's challenge on it can only earn a refusal.""" + mgr, ws = self._manager(declaring_protocol) + successor = AsyncMock() + + mgr._process_received(self._authenticated()) + mgr._ws = successor # reconnect lands before the spawned task runs + await self._settle(mgr) + + ws.send.assert_not_called() + successor.send.assert_not_called() + declaring_protocol.sign_data.assert_not_called() + + @pytest.mark.asyncio + async def test_a_failed_declaration_leaves_the_connection_authenticated( + self, declaring_protocol: MagicMock + ) -> None: + """An undeclared connection is a working connection: the relay simply + attributes it the legacy way. Failing it over a refused declaration + would turn a degraded path into no path.""" + declaring_protocol.sign_data.side_effect = RuntimeError("no identity") + mgr, ws = self._manager(declaring_protocol) + + mgr._process_received(self._authenticated()) + await self._settle(mgr) + + ws.send.assert_not_called() + assert mgr._authenticated is True + assert mgr.state == TransportState.RUNNING + declaring_protocol.internet_status_changed.assert_called_once_with( + is_connected=True + ) + + def test_address_declared_reaches_the_lockstep_check( + self, declaring_protocol: MagicMock + ) -> None: + """The echo goes to a dedicated FFI entry point, where the core + compares the bound address against `local_address()`. Logging it + instead would silently disable RELAY_ADDRESS_BINDING_MISMATCH.""" + mgr, _ = self._manager(declaring_protocol) + + mgr._process_received( + json.dumps({"type": "AddressDeclared", "address": self.ADDRESS}).encode() + ) + + declaring_protocol.internet_address_declared.assert_called_once_with( + address=self.ADDRESS + ) + + def test_the_refusal_frame_is_address_error( + self, declaring_protocol: MagicMock + ) -> None: + """The relay's refusal variant is `AddressError`, not the name of the + FFI entry point it feeds. Matching the wrong tag leaves every refusal + in the unhandled-message branch, so RELAY_ADDRESS_DECLARATION_REFUSED + never fires.""" + mgr, _ = self._manager(declaring_protocol) + + mgr._process_received( + json.dumps({"type": "AddressError", "reason": "address_taken"}).encode() + ) + + declaring_protocol.internet_address_declaration_refused.assert_called_once_with( + reason="address_taken" + ) diff --git a/docs/bridges/README.md b/docs/bridges/README.md index 088786d6..95bf1141 100644 --- a/docs/bridges/README.md +++ b/docs/bridges/README.md @@ -129,13 +129,25 @@ binding edited alone fails the Rust suite rather than its own. See The one-shot event tag list and the mesh wake task key are pinned the same way, by Rust guards that read the binding sources. -**The relay address-proof signing domain** is the fifth: the Swift and Kotlin +**The relay address-proof signing domain** is the fifth, and it is the one set +pinned by both mechanisms at once. The Swift and Kotlin `AddressDeclarationPolicy` each hold a copy of `offline-relay-addr-v1`, and a Rust guard reads both sources. It belongs to the four-domain mutual non-prefix rule the core pins separately, so an edit here that agrees with itself in one language still fails that guard rather than producing a signature nothing verifies. +Python holds a third copy, in `address_declaration.py`, pinned the other way: +`test_address_declaration.py` asserts the whole proof payload against the +relay's own hex vector, which contains the domain. Python earns the +per-language route because its tests actually execute in CI, where the Swift +and Kotlin `InternetManager` sources are typechecked at most and never run, so +for those two a source-reading guard is the only reachable pin. The same test +is what pins the call ordering that the Rust guard +`react_native_relay_declares_its_address_before_it_flushes` has to assert +textually for the other two: the declaration is written before the outbox +flush, and a Python test can simply observe the calls happen in that order. + **The resolution-query completion deadline** is the sixth, and the newest: the Swift and Kotlin `NostrQueryTracker` each hold a copy of `COMPLETION_TIMEOUT_MS`, and a Rust guard reads both sources. Unlike the others