From 8b706331e220f949972dab6aa492cbd0348ae629 Mon Sep 17 00:00:00 2001 From: Alain Brenzikofer Date: Tue, 8 Sep 2026 17:04:20 +0200 Subject: [PATCH] fix adversarial review findings --- protocol/simplex-messaging.md | 28 ++-- scripts/resolver/service/test_snrc_resolve.py | 146 ++++++++++-------- src/Simplex/Messaging/Client.hs | 18 ++- src/Simplex/Messaging/Protocol.hs | 29 ++-- src/Simplex/Messaging/SimplexName.hs | 14 +- tests/AgentTests/ResolveNameTests.hs | 7 +- tests/RSLVTests.hs | 12 ++ tests/SMPNamesTests.hs | 17 +- 8 files changed, 163 insertions(+), 108 deletions(-) diff --git a/protocol/simplex-messaging.md b/protocol/simplex-messaging.md index 37a47de28..9835b5d77 100644 --- a/protocol/simplex-messaging.md +++ b/protocol/simplex-messaging.md @@ -1458,16 +1458,17 @@ while still returning a `NameRecord` matching the encoding below. #### Resolve name command -The `RSLV` command carries the canonical fully-qualified name directly as the -payload (not JSON): +From v22 the `RSLV` command carries a query; below v22 it carries the name +directly, as it always did (not JSON): ```abnf -rslv = %s"RSLV" SP query -query = tld label sub -tld = %s"s" / %s"t" / %s"w" ; .simplex / .testing / a web name -label = %s"N" shortString ; the second-level label as text - / %s"H" 32*32 OCTET ; its keccak-256 -sub = length *shortString ; subname labels, parent to child +rslv = %s"RSLV" SP (query / domain) ; query from v22, domain below it +query = tld label sub +tld = %s"s" / %s"t" / %s"w" ; .simplex / .testing / a web name +label = %s"N" shortString ; the second-level label as text + / %s"H" 32*32 OCTET ; its keccak-256 +sub = length *shortString ; subname labels, parent to child +domain = 1*253 OCTET ; the name as text ``` `domain` is the UTF-8 canonical fully-qualified name with the TLD always @@ -1494,11 +1495,12 @@ takes. That form appears nowhere in SMP. A hashed query still answers with the name. The registrar records the plaintext label when a name is registered, keyed by the hash of that label, so a router can -look up what the hash stands for without ever being told. It is not the client's -word for it and needs no checking: the key is the hash of the value. A name -registered without that record answers `unknown`. What stays impossible is -learning a name that is *not* registered — there is nothing recorded to look up, -so a name someone is merely considering never becomes known. +look up what the hash stands for without ever being told. The router is not +trusted for it: a client MUST check that the record names the name it asked +about, and reject the answer otherwise. A name registered without that record +answers `unknown`, which fails that check. What stays impossible is learning a +name that is *not* registered: there is nothing recorded to look up, so a name +someone is merely considering never becomes known. **Server-side validation.** The names router parses `domain` as a fully-qualified name (TLD required — bare labels are rejected) and forwards it diff --git a/scripts/resolver/service/test_snrc_resolve.py b/scripts/resolver/service/test_snrc_resolve.py index b764fef8f..ab0132078 100644 --- a/scripts/resolver/service/test_snrc_resolve.py +++ b/scripts/resolver/service/test_snrc_resolve.py @@ -217,10 +217,9 @@ class NameStatusTests(unittest.TestCase): "status": status, "expires": expires, "graceEnds": grace_ends, - "auctionEnds": None, - "premium": None, "reasonCode": None, "reason": None, + "auctionUntil": None, } def setUp(self): @@ -360,10 +359,9 @@ class NameStatusTests(unittest.TestCase): "status", "expires", "graceEnds", - "auctionEnds", - "premium", "reasonCode", "reason", + "auctionUntil", } snrc.eth_call = self._expiry(0) self.assertEqual(set(snrc.name_status("alice.testing")), keys) @@ -401,18 +399,22 @@ class ReservedTests(unittest.TestCase): return eth_call - def test_unregistered_and_reserved_reads_reserved(self): + def test_unregistered_and_reserved_reports_the_reservation(self): snrc.eth_call = self._chain(0, True) - self.assertEqual(snrc.name_status("acme.testing")["status"], "reserved") + reg = snrc.name_status("acme.testing") + self.assertEqual(reg["status"], "unregistered") + self.assertEqual(reg["reasonCode"], "internal") def test_unregistered_and_not_reserved_reads_unregistered(self): snrc.eth_call = self._chain(0, False) self.assertEqual(snrc.name_status("acme.testing")["status"], "unregistered") - def test_a_lapsed_reserved_name_is_reserved_not_claimable(self): + def test_a_lapsed_reserved_name_keeps_its_reservation(self): past = int(time.time()) - 91 * 86400 snrc.eth_call = self._chain(past, True) - self.assertEqual(snrc.name_status("acme.testing")["status"], "reserved") + reg = snrc.name_status("acme.testing") + self.assertEqual(reg["status"], "expired") + self.assertEqual(reg["reasonCode"], "internal") def test_a_live_name_is_registered_even_if_reserved(self): snrc.eth_call = self._chain(int(time.time()) + 86400, True) @@ -431,7 +433,7 @@ class ReservedTests(unittest.TestCase): # keccak-256("acme") hashed = "[e29dae06ef4c3e336b7538b6d4f52ca1ecec009b1df6fb501320e11b223aeeaf]" snrc.eth_call = self._chain(0, True) - self.assertEqual(snrc.name_status(hashed + ".testing")["status"], "reserved") + self.assertEqual(snrc.name_status(hashed + ".testing")["reasonCode"], "internal") class ReservedReasonTests(unittest.TestCase): @@ -490,7 +492,6 @@ class ReservedReasonTests(unittest.TestCase): for code, (name, sentence) in snrc.RESERVED_REASONS.items(): snrc.eth_call = self._reserved_as(code) reg = snrc.name_status("acme.testing") - self.assertEqual(reg["status"], "reserved", name) self.assertEqual(reg["reasonCode"], name) self.assertEqual(reg["reason"], sentence) @@ -499,27 +500,27 @@ class ReservedReasonTests(unittest.TestCase): _, body = snrc.resolve("acme.testing") self.assertEqual(body["reasonCode"], "trademark") - def test_a_controller_storing_a_bool_reads_as_unspecified(self): + def test_a_controller_storing_a_bool_reads_as_internal(self): """Before the enum `reservedNames` was a bool; its `true` decodes as 1.""" snrc.eth_call = self._reserved_as(1) reg = snrc.name_status("acme.testing") - self.assertEqual(reg["reasonCode"], "unspecified") - self.assertEqual(reg["reason"], "reserved for a brand or public interest") + self.assertEqual(reg["reasonCode"], "internal") + self.assertEqual(reg["reason"], "reserved for SimpleX") def test_an_enum_value_this_resolver_predates_is_not_dropped(self): - """A new Reason still reads as reserved, and says it is unknown rather + """A new Reason still reserves the name, and says it is unknown rather than claiming the chain recorded none.""" snrc.eth_call = self._reserved_as(99) reg = snrc.name_status("acme.testing") - self.assertEqual(reg["status"], "reserved") self.assertEqual(reg["reasonCode"], "unknown") + self.assertEqual(reg["reason"], "reserved") def test_a_reserved_name_carries_the_reason(self): snrc.eth_call = self._chain(0, True) status, body = snrc.resolve("acme.testing") self.assertEqual(status, 404) - self.assertEqual(body["status"], "reserved") - self.assertEqual(body["reason"], "reserved for a brand or public interest") + self.assertEqual(body["status"], "unregistered") + self.assertEqual(body["reason"], "reserved for SimpleX") def test_the_message_does_not_claim_a_trademark(self): snrc.eth_call = self._chain(0, True) @@ -531,26 +532,27 @@ class ReservedReasonTests(unittest.TestCase): status, body = snrc.resolve("acme.testing") self.assertEqual(status, 404) self.assertEqual(body["status"], "unregistered") - self.assertNotIn("reason", body) + self.assertIsNone(body["reason"]) def test_an_expired_name_has_no_reason(self): snrc.eth_call = self._chain(1, False) status, body = snrc.resolve("acme.testing") self.assertEqual(status, 410) self.assertEqual(body["status"], "expired") - self.assertNotIn("reason", body) + self.assertIsNone(body["reason"]) def test_a_hashed_query_gets_the_reason_too(self): snrc.eth_call = self._chain(0, True) # keccak-256("acme") hashed = "[e29dae06ef4c3e336b7538b6d4f52ca1ecec009b1df6fb501320e11b223aeeaf]" _, body = snrc.resolve(hashed + ".testing") - self.assertEqual(body["reason"], "reserved for a brand or public interest") + self.assertEqual(body["reason"], "reserved for SimpleX") class AuctionTests(unittest.TestCase): - """Past grace anyone may register the name, but at a premium that halves - each day. Reporting it as plainly available would quote the normal price.""" + """Past grace anyone may register the name, but at a surcharge until the + oracle's window closes. `auctionUntil` dates that window; the surcharge + itself never travels.""" REGISTRY = "0x58fc46996d975c57883564648bda5206d1a0102b" REGISTRAR = "0xef47eb4384b46c89e4482a677c2cbcbd2a6fd85a" @@ -561,6 +563,9 @@ class AuctionTests(unittest.TestCase): # The values .testing is deployed with: $100M, halving daily for 21 days. START_PREMIUM = 10 ** 26 TOTAL_DAYS = 21 + # what the oracle charges per year, in US cents, by label length + PRICES = {1: 64000, 2: 16000, 3: 1600, 4: 800, 5: 500, 6: 200} + MIN_LENGTH = 3 def setUp(self): self._saved = ( @@ -587,8 +592,8 @@ class AuctionTests(unittest.TestCase): ) = self._saved def _chain(self, expires, total_days=TOTAL_DAYS, oracle=None, reserved=0): - """Answers as the controller and oracle do, including the oracle's own - `decayedPremium` shift, so the decay curve is not copied here.""" + """Answers as the controller and oracle do, quoting rent in attoUSD per + second as the oracle does.""" oracle = self.ORACLE if oracle is None else oracle self.oracle_calls = [] @@ -602,18 +607,19 @@ class AuctionTests(unittest.TestCase): if data.startswith(snrc.selector("prices()")): self.assertEqual(to, self.CONTROLLER) return "0x" + snrc.encode_uint(int(oracle, 16)) + if data.startswith(snrc.selector("minCharLength()")): + self.assertEqual(to, self.CONTROLLER) + return "0x" + snrc.encode_uint(self.MIN_LENGTH) self.oracle_calls.append(data[:10]) self.assertEqual(to, oracle) - if data.startswith(snrc.selector("totalDays()")): - return "0x" + snrc.encode_uint(total_days) + for n, cents in self.PRICES.items(): + if data.startswith(snrc.selector(f"price{n}Letter()")): + rate = cents * snrc.ATTO_PER_CENT // snrc.SECONDS_PER_YEAR + return "0x" + snrc.encode_uint(rate) if data.startswith(snrc.selector("startPremium()")): return "0x" + snrc.encode_uint(self.START_PREMIUM) if data.startswith(snrc.selector("endValue()")): return "0x" + snrc.encode_uint(self.START_PREMIUM >> total_days) - if data.startswith(snrc.selector("decayedPremium(uint256,uint256)")): - start = int(data[10:74], 16) - elapsed = int(data[74:138], 16) - return "0x" + snrc.encode_uint(start >> (elapsed // 86400)) return self.fail("unexpected call " + data[:10]) return eth_call @@ -623,35 +629,39 @@ class AuctionTests(unittest.TestCase): second clears the boundary, which counts as still in grace.""" return self.now - self.GRACE - 1 - days_into_auction * 86400 - def test_a_name_just_past_grace_is_in_auction_not_merely_expired(self): + def test_a_name_just_past_grace_is_expired_and_dates_the_auction(self): expires = self._lapsed(0) snrc.eth_call = self._chain(expires) reg = snrc.name_status("acme.testing") - self.assertEqual(reg["status"], "auction") - self.assertEqual( - reg["premium"], str(self.START_PREMIUM - (self.START_PREMIUM >> self.TOTAL_DAYS)) - ) + self.assertEqual(reg["status"], "expired") self.assertEqual(reg["graceEnds"], expires + self.GRACE) self.assertEqual( - reg["auctionEnds"], expires + self.GRACE + self.TOTAL_DAYS * 86400 + reg["auctionUntil"], expires + self.GRACE + self.TOTAL_DAYS * 86400 ) - def test_the_premium_halves_each_day(self): - snrc.eth_call = self._chain(self._lapsed(3)) + def test_the_window_lasts_as_long_as_the_premium_takes_to_decay(self): + expires = self._lapsed(0) + snrc.eth_call = self._chain(expires, total_days=10) reg = snrc.name_status("acme.testing") - floor = self.START_PREMIUM >> self.TOTAL_DAYS - self.assertEqual(reg["premium"], str((self.START_PREMIUM >> 3) - floor)) + self.assertEqual(reg["auctionUntil"], expires + self.GRACE + 10 * 86400) + + def test_the_prices_are_the_oracles_rates_in_cents_per_year(self): + snrc.eth_call = self._chain(self._lapsed(0)) + reg = snrc.name_status("acme.testing") + # 1 and 2 are below minCharLength; the 6-letter tier is the base price + self.assertEqual(reg["rentPrices"], {3: 1600, 4: 800, 5: 500}) + self.assertEqual(reg["basePrice"], 200) + self.assertEqual(reg["minLabelLength"], self.MIN_LENGTH) def test_past_the_window_prices_are_back_to_normal(self): snrc.eth_call = self._chain(self._lapsed(self.TOTAL_DAYS)) reg = snrc.name_status("acme.testing") self.assertEqual(reg["status"], "expired") - self.assertIsNone(reg["premium"]) - self.assertIsNone(reg["auctionEnds"]) + self.assertIsNone(reg["auctionUntil"]) def test_a_zero_day_window_switches_the_auction_off(self): snrc.eth_call = self._chain(self._lapsed(0), total_days=0) - self.assertEqual(snrc.name_status("acme.testing")["status"], "expired") + self.assertIsNone(snrc.name_status("acme.testing")["auctionUntil"]) def test_a_controller_with_no_oracle_leaves_the_name_merely_expired(self): snrc.eth_call = self._chain(self._lapsed(0), oracle=snrc.ZERO_ADDR) @@ -663,50 +673,45 @@ class AuctionTests(unittest.TestCase): self.assertEqual(self.oracle_calls, []) def test_the_oracle_curve_is_read_once_not_per_query(self): - """The curve changes only on a retune, so only the decaying premium is - re-read; the rest would be four RPC calls per query.""" + """The curve changes only on a retune, so it is read once rather than + on every query.""" snrc.eth_call = self._chain(self._lapsed(1)) snrc.name_status("acme.testing") seen_first = len(self.oracle_calls) snrc.name_status("acme.testing") - self.assertEqual( - self.oracle_calls[seen_first:], - [snrc.selector("decayedPremium(uint256,uint256)")], - ) + self.assertEqual(self.oracle_calls[seen_first:], []) - def test_a_reserved_lapsed_name_stays_reserved_rather_than_auctioned(self): + def test_a_reserved_lapsed_name_keeps_its_reservation(self): snrc.eth_call = self._chain(self._lapsed(0), reserved=2) reg = snrc.name_status("acme.testing") - self.assertEqual(reg["status"], "reserved") - self.assertIsNone(reg["premium"]) + self.assertEqual(reg["status"], "expired") + self.assertEqual(reg["reasonCode"], "trademark") - def test_resolve_reports_the_auction_with_its_price_and_deadline(self): + def test_resolve_reports_the_prices_and_the_auction_deadline(self): expires = self._lapsed(1) snrc.eth_call = self._chain(expires) status, body = snrc.resolve("acme.testing") self.assertEqual(status, 410) - self.assertEqual(body["status"], "auction") - floor = self.START_PREMIUM >> self.TOTAL_DAYS - self.assertEqual(body["premium"], str((self.START_PREMIUM >> 1) - floor)) + self.assertEqual(body["status"], "expired") + self.assertEqual(body["basePrice"], 200) self.assertEqual( - body["auctionEnds"], expires + self.GRACE + self.TOTAL_DAYS * 86400 + body["auctionUntil"], expires + self.GRACE + self.TOTAL_DAYS * 86400 ) - def test_an_expired_name_past_the_window_carries_no_auction_fields(self): + def test_an_expired_name_past_the_window_has_no_auction_deadline(self): snrc.eth_call = self._chain(self._lapsed(self.TOTAL_DAYS)) status, body = snrc.resolve("acme.testing") self.assertEqual(status, 410) self.assertEqual(body["status"], "expired") - self.assertNotIn("premium", body) - self.assertNotIn("auctionEnds", body) + self.assertIsNone(body["auctionUntil"]) def test_a_hashed_query_is_priced_too(self): # keccak-256("acme") hashed = "[e29dae06ef4c3e336b7538b6d4f52ca1ecec009b1df6fb501320e11b223aeeaf]" snrc.eth_call = self._chain(self._lapsed(0)) _, body = snrc.resolve(hashed + ".testing") - self.assertEqual(body["status"], "auction") - self.assertIsNotNone(body["premium"]) + self.assertEqual(body["status"], "expired") + self.assertEqual(body["basePrice"], 200) @@ -757,7 +762,6 @@ class ErrorCodeTests(unittest.TestCase): def test_a_registration_problem_reports_the_status_as_the_code(self): for expires, code in ( (0, "unregistered"), - (int(time.time()) - 3600, "grace"), (int(time.time()) - 91 * 86400, "expired"), ): with self.subTest(code=code): @@ -766,12 +770,20 @@ class ErrorCodeTests(unittest.TestCase): self.assertEqual(body["error"], code) self.assertEqual(body["status"], code) - def test_a_registered_name_pointing_nowhere_is_noResolver(self): + def test_a_name_in_grace_still_resolves(self): + snrc.eth_call = self._chain(int(time.time()) - 3600) + status, body = snrc.resolve("alice.testing") + self.assertEqual(status, 200) + self.assertEqual(body["status"], "grace") + self.assertNotIn("error", body) + + def test_a_registered_name_pointing_nowhere_resolves_with_empty_records(self): snrc.eth_call = self._chain(int(time.time()) + 86400) status, body = snrc.resolve("alice.testing") - self.assertEqual(status, 404) - self.assertEqual(body["error"], "noResolver") - self.assertEqual(body["status"], "noResolver") + self.assertEqual(status, 200) + self.assertEqual(body["status"], "registered") + self.assertEqual(body["resolver"], snrc.ZERO_ADDR) + self.assertEqual(body["simplexContact"], []) def test_every_error_body_carries_both_fields(self): snrc.eth_call = self._chain(0) diff --git a/src/Simplex/Messaging/Client.hs b/src/Simplex/Messaging/Client.hs index ea0db5257..c5444f41f 100644 --- a/src/Simplex/Messaging/Client.hs +++ b/src/Simplex/Messaging/Client.hs @@ -166,7 +166,7 @@ import Simplex.Messaging.Parsers (defaultJSON, dropPrefix, enumJSON, sumTypeJSON import Simplex.Messaging.Protocol import Simplex.Messaging.Protocol.Types import Simplex.Messaging.Server.QueueStore.QueueInfo -import Simplex.Messaging.SimplexName (SimplexDomain) +import Simplex.Messaging.SimplexName (SimplexDomain, fullDomainName) import Simplex.Messaging.TMap (TMap) import qualified Simplex.Messaging.TMap as TM import Simplex.Messaging.Transport @@ -1058,7 +1058,7 @@ proxyResolveName :: SMPClient -> NetworkRequestMode -> ProxiedRelay -> SimplexDo proxyResolveName c nm proxiedRelay name | v >= namesSMPVersion = proxySMPCommand c nm proxiedRelay Nothing NoEntity (RSLV (nameQuery v name)) >>= \case - Right (RNAME reg) -> pure $ Right reg + Right (RNAME reg) | resolvedName name reg -> pure $ Right reg Right r -> throwE $ unexpectedResponse r Left e -> pure $ Left e | otherwise = throwE $ PCETransportError TEVersion @@ -1068,18 +1068,26 @@ proxyResolveName c nm proxiedRelay name -- | Direct (non-PFWD) name resolution. Exposes the client IP to the resolver; -- callers that want anonymity should use `proxyResolveName` via the standard -- proxy fallback in the agent. RSLV requires no entity ID or authorization --- (see `noAuthCmd` in Protocol.hs). Version-gated on the session here, not the --- encoder, so an old server never receives RSLV. +-- (see `noAuthCmd` in Protocol.hs). Gated on the session version, below which +-- the server has no RSLV at all; the encoder gates the query format separately. directResolveName :: SMPClient -> NetworkRequestMode -> SimplexDomain -> ExceptT SMPClientError IO NameRegistration directResolveName c nm name | v >= namesSMPVersion = sendProtocolCommand c nm Nothing NoEntity (Cmd SResolver (RSLV (nameQuery v name))) >>= \case - RNAME reg -> pure reg + RNAME reg | resolvedName name reg -> pure reg r -> throwE $ unexpectedResponse r | otherwise = throwE $ PCETransportError TEVersion where v = thVersion (thParams c) +-- | The record must name the name that was asked for. A hashed query does not +-- tell the router which name it is, so the record's own name is the router's +-- word until the client checks it here. +resolvedName :: SimplexDomain -> NameRegistration -> Bool +resolvedName d = \case + NRRegistered {nameRecord} -> T.toLower (nrName nameRecord) == fullDomainName d + _ -> True + -- | Acknowledge message delivery (server deletes the message). -- -- https://github.com/simplex-chat/simplexmq/blob/master/protocol/simplex-messaging.md#acknowledge-message-delivery diff --git a/src/Simplex/Messaging/Protocol.hs b/src/Simplex/Messaging/Protocol.hs index ae012c2c4..467527e2a 100644 --- a/src/Simplex/Messaging/Protocol.hs +++ b/src/Simplex/Messaging/Protocol.hs @@ -283,7 +283,7 @@ import Simplex.Messaging.Protocol.Types import Simplex.Messaging.Server.QueueStore.QueueInfo import Simplex.Messaging.ServiceScheme import Simplex.Messaging.SystemTime (SystemSeconds) -import Simplex.Messaging.SimplexName (LabelHash, SimplexDomain (..), SimplexTLD (..), domainName, labelHash, labelHashText) +import Simplex.Messaging.SimplexName (LabelHash, SimplexDomain (..), SimplexTLD (..), fullDomainName, labelHash, labelHashText) import Simplex.Messaging.Transport import Simplex.Messaging.Transport.Client (TransportHost, TransportHosts (..)) import Simplex.Messaging.Util (bshow, eitherToMaybe, safeDecodeUtf8, (<$?>)) @@ -1629,18 +1629,25 @@ instance Encoding NameQueryLabel where 'H' -> NQHash <$> smpP _ -> fail "bad NameQueryLabel" --- | How the backing resolver is addressed for this query. The only place a --- hashed label is written as text, and it faces the resolver's HTTP API - the --- SMP protocol tags the choice instead of spelling it. +-- | How the backing resolver is addressed for this query. queryName :: NameQuery -> Text -queryName NameQuery {queryTLD, queryLabel, querySub} = domainName queryTLD label querySub +queryName = fullDomainName . queryDomain + +-- | The query as a name: what RSLV carries below v22, and what the resolver's +-- HTTP API takes. The only place a hashed label is written as text - the SMP +-- protocol tags the choice instead of spelling it. +queryDomain :: NameQuery -> SimplexDomain +queryDomain NameQuery {queryTLD, queryLabel, querySub} = + SimplexDomain {nameTLD = queryTLD, domain = label, subDomain = querySub} where label = case queryLabel of NQName t -> t NQHash h -> labelHashText h --- | The name a client asked about, hashed from v22 so the router is never told --- what it is. A web TLD has no registry, so it is never hashed. +-- | The name a client asked about, hashed from v22. The hash only hides an +-- unregistered name: a registered one comes back with its name in the record, +-- and a short label is guessable by hashing candidates. A web TLD has no +-- registry, so it is never hashed. nameQuery :: VersionSMP -> SimplexDomain -> NameQuery nameQuery v SimplexDomain {nameTLD, domain, subDomain} = NameQuery {queryTLD = nameTLD, queryLabel = label, querySub = subDomain} @@ -2021,7 +2028,9 @@ instance PartyI p => ProtocolEncoding SMPVersion ErrorType (Command p) where PRXY host auth_ -> e (PRXY_, ' ', host, auth_) PFWD fwdV pubKey (EncTransmission s) -> e (PFWD_, ' ', fwdV, pubKey, Tail s) RFWD (EncFwdTransmission s) -> e (RFWD_, ' ', Tail s) - RSLV d -> e (RSLV_, ' ', d) + RSLV q + | v >= nameAvailSMPVersion -> e (RSLV_, ' ', q) + | otherwise -> e (RSLV_, ' ', queryDomain q) where e :: Encoding a => a -> ByteString e = smpEncode @@ -2128,7 +2137,9 @@ instance ProtocolEncoding SMPVersion ErrorType Cmd where CT SNotifierService NSUBS_ | v >= rcvServiceSMPVersion -> Cmd SNotifierService <$> (NSUBS <$> _smpP <*> smpP) | otherwise -> pure $ Cmd SNotifierService $ NSUBS (-1) mempty - CT SResolver RSLV_ -> Cmd SResolver . RSLV <$> _smpP <* A.takeByteString + CT SResolver RSLV_ + | v >= nameAvailSMPVersion -> Cmd SResolver . RSLV <$> _smpP <* A.takeByteString + | otherwise -> Cmd SResolver . RSLV . nameQuery v <$> _smpP <* A.takeByteString fromProtocolError = fromProtocolError @SMPVersion @ErrorType @BrokerMsg {-# INLINE fromProtocolError #-} diff --git a/src/Simplex/Messaging/SimplexName.hs b/src/Simplex/Messaging/SimplexName.hs index 7c9e380df..34e242100 100644 --- a/src/Simplex/Messaging/SimplexName.hs +++ b/src/Simplex/Messaging/SimplexName.hs @@ -10,7 +10,6 @@ module Simplex.Messaging.SimplexName SimplexTLD (..), SimplexNameType (..), fullDomainName, - domainName, LabelHash (..), labelHash, labelHashText, @@ -78,11 +77,8 @@ nameLabelP = do -- (Cyrillic а vs ASCII a hash to different on-chain records). isNameLetter c = c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' --- | A second-level label sent as its keccak256 hash, so a router never learns --- the name. ENS's bracket form: brackets are outside the name character set, so --- it cannot collide with a real name. 66 chars, so exempt from the label limit. -- | The registry's key for a label, and what BaseRegistrarImplementation.labelOf --- is keyed on. Always 32 bytes, so it is never told from a name by its shape. +-- is keyed on. Always 32 bytes. newtype LabelHash = LabelHash ByteString deriving (Eq, Show) @@ -151,13 +147,9 @@ instance Encoding SimplexTLD where _ -> fail "bad SimplexTLD" fullDomainName :: SimplexDomain -> Text -fullDomainName SimplexDomain {nameTLD, domain, subDomain} = domainName nameTLD domain subDomain - --- | A dotted name from its parts, whatever the second-level label is written as. -domainName :: SimplexTLD -> Text -> [Text] -> Text -domainName tld label sub = T.intercalate "." (reverse sub ++ [label] ++ tld') +fullDomainName SimplexDomain {nameTLD, domain, subDomain} = T.intercalate "." (reverse subDomain ++ [domain] ++ tld') where - tld' = case tld of + tld' = case nameTLD of TLDSimplex -> ["simplex"] TLDTesting -> ["testing"] TLDWeb -> [] diff --git a/tests/AgentTests/ResolveNameTests.hs b/tests/AgentTests/ResolveNameTests.hs index 9a769ddf6..dd4e23c59 100644 --- a/tests/AgentTests/ResolveNameTests.hs +++ b/tests/AgentTests/ResolveNameTests.hs @@ -91,12 +91,17 @@ resolveNameTests = do testAvailSuccess :: HasCallStack => IO () testAvailSuccess = - withDirectResolver (status404, "{\"error\":\"unregistered\"}") $ \c -> do + withDirectResolver (status404, availableBody) $ \c -> do r <- runExceptT $ resolveSimplexName c NRMInteractive 1 (SimplexDomain TLDSimplex "alice" []) case r of Right (SMP.NRAvailable {}) -> pure () _ -> expectationFailure $ "expected Right NRAvailable, got: " <> show r +-- an unregistered name is only available if the resolver also priced it +availableBody :: LB.ByteString +availableBody = + "{\"error\":\"unregistered\",\"rentPrices\":{\"3\":12793,\"4\":3198},\"basePrice\":100,\"minLabelLength\":3}" + testDirectNotFound :: HasCallStack => IO () testDirectNotFound = withDirectResolver (status404, "{}") $ \c -> do diff --git a/tests/RSLVTests.hs b/tests/RSLVTests.hs index 57467aeaf..1cd5d8cde 100644 --- a/tests/RSLVTests.hs +++ b/tests/RSLVTests.hs @@ -107,6 +107,7 @@ rslvTests = do describe "hashed lookups" $ do it "RSLV sends the 2LD as its hash" testRslvSendsTheHash it "subname labels stay text" testSubnameKeepsItsLabels + it "a record naming a different name is rejected" testRslvWrongName testRslvBackendNotFound :: IO () testRslvBackendNotFound = @@ -301,5 +302,16 @@ testSubnameKeepsItsLabels = _ <- runExceptT' (directResolveName pc NRMInteractive (domain "x.alice.simplex")) resolvePaths reqs `shouldReturn` [["resolve", "x." <> aliceHash <> ".simplex"]] +-- a hashed query does not tell the router the name, so the record's own name is +-- checked against the one that was asked for +testRslvWrongName :: IO () +testRslvWrongName = + withResolverServer (status200, J.encode testNameRecord {SMP.nrName = "mallory.simplex"}) $ do + pc <- currentClient + r <- runExceptT (directResolveName pc NRMInteractive (domain "alice.simplex")) + case r of + Left (PCEUnexpectedResponse _) -> pure () + _ -> expectationFailure $ "expected Left (PCEUnexpectedResponse ..), got: " <> show r + runExceptT' :: Show e => ExceptT e IO a -> IO a runExceptT' a = runExceptT a >>= either (fail . show) pure diff --git a/tests/SMPNamesTests.hs b/tests/SMPNamesTests.hs index 975938e55..5d7eb76b7 100644 --- a/tests/SMPNamesTests.hs +++ b/tests/SMPNamesTests.hs @@ -18,7 +18,7 @@ import Network.HTTP.Types (status200, status400, status404, status410, status500 import NamesResolverServer (resolveResp, testNamesConfig, withResolverServer, withResolverServerDelayed) import Simplex.Messaging.Encoding (smpDecode, smpEncode) import Simplex.Messaging.Encoding.String (strDecode, strEncode) -import Simplex.Messaging.Protocol (ErrorType (..), NameErrorType (..), NamePricing (..), NameRecord (..), NameRegistration (..), NameReservedReason (..), USDCents (..), nameQuery, queryName) +import Simplex.Messaging.Protocol (Command (..), ErrorType (..), NameErrorType (..), NamePricing (..), NameRecord (..), NameRegistration (..), NameReservedReason (..), ProtocolEncoding (..), USDCents (..), nameQuery, queryName) import Simplex.Messaging.Server.Main (validateUrl) import Simplex.Messaging.Server.Names ( NamesConfig (..), @@ -30,7 +30,7 @@ import Simplex.Messaging.Server.Names import Simplex.Messaging.Server.Names.HttpResolver (ResolverError (..)) import Simplex.Messaging.SimplexName (SimplexDomain (..), SimplexTLD (..), fullDomainName) import Simplex.Messaging.SystemTime (RoundedSystemTime (..)) -import Simplex.Messaging.Transport (currentClientSMPRelayVersion, namesSMPVersion) +import Simplex.Messaging.Transport (currentClientSMPRelayVersion, nameAvailSMPVersion, namesSMPVersion, serverInfoSMPVersion) import Test.Hspec testNameRecord :: NameRecord @@ -54,6 +54,7 @@ smpNamesTests :: Spec smpNamesTests = do describe "NameRecord JSON (Protocol)" nameRecordEncodingSpec describe "ErrorType NAME wire encoding" errorWireSpec + describe "RSLV wire encoding" rslvWireSpec describe "Name parsing (SimplexDomain)" parseNameSpec describe "HTTP resolver" resolverSpec describe "name availability" availabilitySpec @@ -105,6 +106,18 @@ errorWireSpec = -- RESOLVER detail may contain spaces - must survive the round-trip smpDecode (smpEncode (NAME (RESOLVER "HTTP 502"))) `shouldBe` Right (NAME (RESOLVER "HTTP 502")) +-- the query format changed at v22, so an older session must still get the name +rslvWireSpec :: Spec +rslvWireSpec = do + it "below v22 carries the name, as it did before" $ + encodeProtocol v20 (RSLV (nameQuery v20 aliceDomain')) `shouldBe` "RSLV " <> smpEncode aliceDomain' + it "from v22 carries the query" $ + encodeProtocol v22 (RSLV (nameQuery v22 aliceDomain')) `shouldBe` "RSLV " <> smpEncode (nameQuery v22 aliceDomain') + where + v20 = serverInfoSMPVersion + v22 = nameAvailSMPVersion + aliceDomain' = SimplexDomain {nameTLD = TLDSimplex, domain = "alice", subDomain = []} + availabilitySpec :: Spec availabilitySpec = do -- one lookup answers what the name points to, whether it can be taken, and