From 23aff6b0b11b8776ca29f595d975171ae582771a Mon Sep 17 00:00:00 2001 From: Evgeny Poberezkin Date: Tue, 9 Sep 2025 09:56:26 +0100 Subject: [PATCH] Revert "smp server: remove dependency of message size on the version (#1627)" This reverts commit 8fea15245aaf7dd7b8ab8cf549e63b8f117da0a8. --- src/Simplex/Messaging/Protocol.hs | 13 +++++++++---- src/Simplex/Messaging/Server.hs | 13 +++++++------ tests/CoreTests/BatchingTests.hs | 2 +- tests/SMPProxyTests.hs | 3 ++- tests/ServerTests.hs | 8 ++++---- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/Simplex/Messaging/Protocol.hs b/src/Simplex/Messaging/Protocol.hs index 1588742f3..40314ad2a 100644 --- a/src/Simplex/Messaging/Protocol.hs +++ b/src/Simplex/Messaging/Protocol.hs @@ -292,13 +292,18 @@ currentSMPClientVersion = VersionSMPC 4 supportedSMPClientVRange :: VersionRangeSMPC supportedSMPClientVRange = mkVersionRange initialSMPClientVersion currentSMPClientVersion -maxMessageLength :: Int -maxMessageLength = 16048 +-- TODO v6.0 remove dependency on version +maxMessageLength :: VersionSMP -> Int +maxMessageLength v + | v >= encryptedBlockSMPVersion = 16048 -- max 16048 + | v >= sendingProxySMPVersion = 16064 -- max 16067 + | otherwise = 16088 -- 16048 - always use this size to determine allowed ranges paddedProxiedTLength :: Int paddedProxiedTLength = 16226 -- 16225 .. 16227 -type MaxMessageLen = 16048 +-- TODO v7.0 change to 16048 +type MaxMessageLen = 16088 -- 16 extra bytes: 8 for timestamp and 8 for flags (7 flags and the space, only 1 flag is currently used) type MaxRcvMessageLen = MaxMessageLen + 16 -- 16104, the padded size is 16106 @@ -1472,7 +1477,7 @@ data ErrorType STORE {storeErr :: Text} | -- | ACK command is sent without message to be acknowledged NO_MSG - | -- | sent message is too large (> maxMessageLength = 16048 bytes) + | -- | sent message is too large (> maxMessageLength = 16088 bytes) LARGE_MSG | -- | relay public key is expired EXPIRED diff --git a/src/Simplex/Messaging/Server.hs b/src/Simplex/Messaging/Server.hs index 6c288904d..23ce85035 100644 --- a/src/Simplex/Messaging/Server.hs +++ b/src/Simplex/Messaging/Server.hs @@ -1349,10 +1349,11 @@ client ms clnt@Client {clientId, ntfSubscriptions, ntfServiceSubscribed, serviceSubsCount = _todo', ntfServiceSubsCount, rcvQ, sndQ, clientTHParams = thParams'@THandleParams {sessionId}, procThreads} = do labelMyThread . B.unpack $ "client $" <> encode sessionId <> " commands" - let clntServiceId = (\THClientService {serviceId} -> serviceId) <$> (peerClientService =<< thAuth thParams') + let THandleParams {thVersion} = thParams' + clntServiceId = (\THClientService {serviceId} -> serviceId) <$> (peerClientService =<< thAuth thParams') process t acc@(rs, msgs) = (maybe acc (\(!r, !msg_) -> (r : rs, maybe msgs (: msgs) msg_))) - <$> processCommand clntServiceId t + <$> processCommand clntServiceId thVersion t forever $ atomically (readTBQueue rcvQ) >>= foldrM process ([], []) @@ -1438,8 +1439,8 @@ client mkIncProxyStats ps psOwn own sel = do incStat $ sel ps when own $ incStat $ sel psOwn - processCommand :: Maybe ServiceId -> VerifiedTransmission s -> M s (Maybe ResponseAndMessage) - processCommand clntServiceId (q_, (corrId, entId, cmd)) = case cmd of + processCommand :: Maybe ServiceId -> VersionSMP -> VerifiedTransmission s -> M s (Maybe ResponseAndMessage) + processCommand clntServiceId clntVersion (q_, (corrId, entId, cmd)) = case cmd of Cmd SProxiedClient command -> processProxiedCmd (corrId, entId, command) Cmd SSender command -> case command of SKEY k -> withQueue $ \q qr -> checkMode QMMessaging qr $ secureQueue_ q k @@ -1828,7 +1829,7 @@ client sendMessage :: MsgFlags -> MsgBody -> StoreQueue s -> QueueRec -> M s (Transmission BrokerMsg) sendMessage msgFlags msgBody q qr - | B.length msgBody > maxMessageLength = do + | B.length msgBody > maxMessageLength clntVersion = do stats <- asks serverStats incStat $ msgSentLarge stats pure $ err LARGE_MSG @@ -1981,7 +1982,7 @@ client -- rejectOrVerify filters allowed commands, no need to repeat it here. -- INTERNAL is used because processCommand never returns Nothing for sender commands (could be extracted for better types). -- `fst` removes empty message that is only returned for `SUB` command - Right t''@(_, (corrId', entId', _)) -> maybe (corrId', entId', ERR INTERNAL) fst <$> lift (processCommand Nothing t'') + Right t''@(_, (corrId', entId', _)) -> maybe (corrId', entId', ERR INTERNAL) fst <$> lift (processCommand Nothing fwdVersion t'') -- encode response r' <- case batchTransmissions clntTHParams [Right (Nothing, encodeTransmission clntTHParams r)] of [] -> throwE INTERNAL -- at least 1 item is guaranteed from NonEmpty/Right diff --git a/tests/CoreTests/BatchingTests.hs b/tests/CoreTests/BatchingTests.hs index 3fcc257f7..d013c0db4 100644 --- a/tests/CoreTests/BatchingTests.hs +++ b/tests/CoreTests/BatchingTests.hs @@ -389,7 +389,7 @@ randomMSG = do corrId <- atomically $ C.randomBytes 24 g rId <- atomically $ C.randomBytes 24 g msgId <- atomically $ C.randomBytes 24 g - msg <- atomically $ C.randomBytes maxMessageLength g + msg <- atomically $ C.randomBytes (maxMessageLength currentClientSMPRelayVersion) g pure (CorrId corrId, EntityId rId, MSG RcvMessage {msgId, msgBody = EncRcvMsgBody msg}) randomSENDv6 :: ByteString -> Int -> IO (Either TransportError (Maybe TAuthorizations, ByteString)) diff --git a/tests/SMPProxyTests.hs b/tests/SMPProxyTests.hs index 0e3db6424..5f1a59fd0 100644 --- a/tests/SMPProxyTests.hs +++ b/tests/SMPProxyTests.hs @@ -69,6 +69,7 @@ smpProxyTests = do let srv1 = SMPServer testHost testPort testKeyHash srv2 = SMPServer testHost2 testPort2 testKeyHash describe "client API" $ do + let maxLen = maxMessageLength encryptedBlockSMPVersion describe "one server" $ do it "deliver via proxy" . oneServer $ do deliverMessageViaProxy srv1 srv1 C.SEd448 "hello 1" "hello 2" @@ -77,7 +78,7 @@ smpProxyTests = do relayServ = srv2 (msg1, msg2) <- runIO $ do g <- C.newRandom - atomically $ (,) <$> C.randomBytes maxMessageLength g <*> C.randomBytes maxMessageLength g + atomically $ (,) <$> C.randomBytes maxLen g <*> C.randomBytes maxLen g it "deliver via proxy" . twoServersFirstProxy $ deliverMessageViaProxy proxyServ relayServ C.SEd448 "hello 1" "hello 2" it "max message size, Ed448 keys" . twoServersFirstProxy $ diff --git a/tests/ServerTests.hs b/tests/ServerTests.hs index 043f11e31..204365931 100644 --- a/tests/ServerTests.hs +++ b/tests/ServerTests.hs @@ -232,12 +232,12 @@ testCreateSecure = Resp "dabc" _ err5 <- sendRecv s ("", "dabc", sId, _SEND "hello") (err5, ERR AUTH) #== "rejects unsigned SEND" - let maxAllowedMessage = B.replicate maxMessageLength '-' + let maxAllowedMessage = B.replicate (maxMessageLength currentClientSMPRelayVersion) '-' Resp "bcda" _ OK <- signSendRecv s sKey ("bcda", sId, _SEND maxAllowedMessage) Resp "" _ (Msg mId3 msg3) <- tGet1 r (dec mId3 msg3, Right maxAllowedMessage) #== "delivers message of max size" - let biggerMessage = B.replicate (maxMessageLength + 1) '-' + let biggerMessage = B.replicate (maxMessageLength currentClientSMPRelayVersion + 1) '-' Resp "bcda" _ (ERR LARGE_MSG) <- signSendRecv s sKey ("bcda", sId, _SEND biggerMessage) pure () @@ -279,12 +279,12 @@ testCreateSndSecure = Resp "dabc" _ err5 <- sendRecv s ("", "dabc", sId, _SEND "hello") (err5, ERR AUTH) #== "rejects unsigned SEND" - let maxAllowedMessage = B.replicate maxMessageLength '-' + let maxAllowedMessage = B.replicate (maxMessageLength currentClientSMPRelayVersion) '-' Resp "bcda" _ OK <- signSendRecv s sKey ("bcda", sId, _SEND maxAllowedMessage) Resp "" _ (Msg mId3 msg3) <- tGet1 r (dec mId3 msg3, Right maxAllowedMessage) #== "delivers message of max size" - let biggerMessage = B.replicate (maxMessageLength + 1) '-' + let biggerMessage = B.replicate (maxMessageLength currentClientSMPRelayVersion + 1) '-' Resp "bcda" _ (ERR LARGE_MSG) <- signSendRecv s sKey ("bcda", sId, _SEND biggerMessage) pure ()