agent: decrypt messages out of order after disabling KEM

Co-authored-by: Evgeny @ SimpleX Chat <[email protected]>

protocol/diagrams/xrcp/session.mmd

@@ -17,11 +17,11 @@ note over CC, HC: TLS handshake
 par
     note over CC: validate client X509 credentials
     CC->>CI: session code from tlsUnique
-    CI-->>CC: user confirmation
+    CI-->>CC: user confirmation (optional if known)
 and
     note over HC: validate server X509 credentials
     HC->>HI: session code from tlsUnique
-    HI-->>HC: user confirmation
+    HI-->>HC: user confirmation (optional if known)
 end
 
 note over CI, HI: 3. Session verification and protocol negotiation

protocol/xrcp.md

@@ -121,9 +121,15 @@ During TLS handshake, parties validate certificate chains against previously kno
 
 ### Session verification and protocol negotiation
 
-Once TLS session is established, both the host and controller devices present a "session security code" to the user who must match them (e.g., visually or via QR code scan) and confirm on the host device. The session security code must be a digest of tlsunique channel binding. As it is computed as a digest of the TLS handshake for both the controller and the host, it will validate that the same TLS certificates are used on both sides, and that the same TLS session is established, mitigating the possibility of MITM attack in the connection.
+Once TLS session is established, both the host and controller devices present a "session security code" to the user. The session security code must be a digest of tlsunique channel binding. As it is computed as a digest of the TLS handshake for both the controller and the host, it will validate that the same TLS certificates are used on both sides, and that the same TLS session is established, mitigating the possibility of MITM attack in the connection.
 
-Once the session is confirmed by the user, the host sends HELLO block to the controller.
+To verify the session, the user matches the codes (e.g., visually or via QR code scan) and confirms the session on the host device.
+
+The user MUST verify the first session with a new controller. The host stores the long-term identity of the controller (CA fingerprint and Ed25519 public key) only after this session is verified. A controller with the stored long-term identity is a known controller.
+
+The user MAY verify the subsequent sessions with a known controller. In a session that the host confirms without user action, the controller is authenticated only by its long-term identity stored in the first session (see [Threat model](#threat-model)). SimpleX Chat mobile apps require user verification of sessions with known controllers only when the user enables the "Verify connections" option.
+
+Once the session is confirmed, the host sends HELLO block to the controller.
 
 XRCP blocks inside TLS are padded to 16384 bytes.
 
@@ -305,7 +311,7 @@ to receive: (rcvKey', sk, nonce) = HKDF(rcvKey, "SimpleXSbChain", 88)
 - prevent host and controller devices from establishing the session
 
 *cannot:*
-- same as passive adversary, provided that user visually verified session code out-of-band.
+- same as passive adversary, provided that user visually verified session code out-of-band in the first session with the controller.
 
 #### An active adversary with the access to the network:
 
@@ -324,6 +330,15 @@ to receive: (rcvKey', sk, nonce) = HKDF(rcvKey, "SimpleXSbChain", 88)
 *cannot:*
 - connect to the host or make host connect to itself.
 
+#### An active adversary with the access to the network who also obtained the long-term private keys of a known controller (e.g., from the controller device):
+
+*can:*
+- make host connect to itself instead of the controller.
+- access any data of the controlled host application, within the capabilities of the provided API, if the host confirms the session without user action.
+
+*cannot:*
+- access host application data, provided that user visually verified session code out-of-band.
+
 #### Compromised controller device:
 
 *can:*

src/Simplex/Messaging/Agent.hs

@@ -4388,8 +4388,9 @@ agentRatchetDecrypt' :: TVar ChaChaDRG -> DB.Connection -> ConnId -> CR.RatchetX
 agentRatchetDecrypt' g db connId rc encAgentMsg = do
   skipped <- liftIO $ getSkippedMsgKeys db connId
   (agentMsgBody_, rc', skippedDiff) <- withExceptT (SEAgentError . cryptoError) $ CR.rcDecrypt g rc skipped encAgentMsg
+  agentMsgBody <- liftEither $ first (SEAgentError . cryptoError) agentMsgBody_
   liftIO $ updateRatchet db connId rc' skippedDiff
-  liftEither $ bimap (SEAgentError . cryptoError) (,CR.rcRcvKEM rc') agentMsgBody_
+  pure (agentMsgBody, CR.rcRcvKEM rc')
 
 newSndQueue :: UserId -> ConnId -> Compatible SMPQueueInfo -> Maybe (C.APrivateAuthKey) -> AM' (NewSndQueue, C.PublicKeyX25519)
 newSndQueue userId connId (Compatible (SMPQueueInfo smpClientVersion SMPQueueAddress {smpServer, senderId, queueMode, dhPublicKey = rcvE2ePubDhKey})) sndKey_ = do

src/Simplex/Messaging/Crypto/Ratchet.hs

@@ -904,8 +904,7 @@ rcEncryptHeader rc@Ratchet {rcSnd = Just sr@SndRatchet {rcCKs, rcHKs}, rcDHRs, r
             rcNs = rcNs + 1,
             rcSupportKEM = rcSupportKEM',
             rcEnableKEM = rcEnableKEM',
-            rcVersion = rcVersion',
-            rcKEM = if pqEnc_ == Just PQEncOff then (\rck -> rck {rcKEMs = Nothing}) <$> rcKEM else rcKEM
+            rcVersion = rcVersion'
           }
   pure (msgEncryptKey, rc')
   where

src/Simplex/Messaging/Server.hs

@@ -2127,6 +2127,7 @@ client
           let proxyNonce = C.cbNonce $ bs corrId
           s' <- liftEitherWith (const CRYPTO) $ C.cbDecryptNoPad sessSecret proxyNonce s
           FwdTransmission {fwdCorrId, fwdVersion, fwdKey, fwdTransmission = EncTransmission et} <- liftEitherWith (const $ CMD SYNTAX) $ smpDecode s'
+          unless (fwdVersion `isCompatible` thServerVRange thParams') $ throwE $ transportErr TEVersion
           let clientSecret = C.dh' fwdKey serverPrivKey
               clientNonce = C.cbNonce $ bs fwdCorrId
           b <- liftEitherWith (const CRYPTO) $ C.cbDecrypt clientSecret clientNonce et

src/Simplex/Messaging/Transport.hs

@@ -865,7 +865,6 @@ forceCertChain :: CertChainPubKey -> CertChainPubKey
 forceCertChain cert@(CertChainPubKey (X.CertificateChain cc) signedKey) = length (show cc) `seq` show signedKey `seq` cert
 {-# INLINE forceCertChain #-}
 
--- This function is only used with v >= 8, so currently it's a simple record update.
 -- * Note: it requires updating version-based parameters, to be consistent with smpTHandle_.
 smpTHParamsSetVersion :: VersionSMP -> THandleParams SMPVersion p -> THandleParams SMPVersion p
 smpTHParamsSetVersion v params =

tests/AgentTests/DoubleRatchetTests.hs

@@ -65,6 +65,7 @@ doubleRatchetTests = do
       describe "message tests, KEM proposed again in reply" $ runMessageTests initRatchetsKEMProposedAgain True
       it "should disable and re-enable KEM" $ withRatchets_ @X25519 initRatchetsKEMAccepted testDisableEnableKEM
       it "should disable and re-enable KEM (always set PQEncryption)" $ withRatchets_ @X25519 initRatchetsKEMAccepted testDisableEnableKEMStrict
+      it "should decrypt messages out of order after disabling KEM" $ withRatchets_ @X25519 initRatchetsKEMAccepted testDisableKEMOutOfOrder
       it "should enable KEM when it was not enabled in handshake" $ withRatchets_ @X25519 initRatchets testEnableKEM
       it "should enable KEM when it was not enabled in handshake (always set PQEncryption)" $ withRatchets_ @X25519 initRatchets testEnableKEMStrict
 
@@ -291,6 +292,14 @@ testDisableEnableKEMStrict alice bob _ _ _ = do
   (alice, "now KEM is enabled") !#>! bob
   (bob, "KEM is enabled for both sides") !#>! alice
 
+testDisableKEMOutOfOrder :: forall a. (AlgorithmI a, DhAlgorithm a) => TestRatchets a
+testDisableKEMOutOfOrder alice bob encrypt decrypt _ = do
+  Right b1 <- withTVar (encrypt_ (Just PQEncOff)) hasSndKEM bob "disabling KEM"
+  Right b2 <- encrypt bob "KEM disabled"
+  Decrypted "KEM disabled" <- decrypt alice b2
+  Decrypted "disabling KEM" <- decrypt alice b1
+  pure ()
+
 testEnableKEM :: forall a. (AlgorithmI a, DhAlgorithm a) => TestRatchets a
 testEnableKEM alice bob _ _ _ = do
   (bob, "hello alice") \#> alice

tests/SMPClient.hs

@@ -316,6 +316,9 @@ prevRange vr = vr {maxVersion = max (minVersion vr) (prevVersion $ maxVersion vr
 prevVersion :: Version v -> Version v
 prevVersion (Version v) = Version (v - 1)
 
+nextVersion :: Version v -> Version v
+nextVersion (Version v) = Version (v + 1)
+
 proxyCfg :: AServerConfig
 proxyCfg = proxyCfgMS (ASType SQSMemory SMSJournal)
 

tests/SMPProxyTests.hs

@@ -191,6 +191,8 @@ deliverMessagesViaProxy proxyServ relayServ alg unsecuredMsgs securedMsgs = do
   forM_ unsecuredMsgs $ \msg -> do
     runExceptT' (proxySMPMessage pc NRMInteractive sess Nothing sndId noMsgFlags msg) `shouldReturn` Right ()
     runExceptT' (proxySMPMessage pc NRMInteractive sess {prSessionId = "bad session"} Nothing sndId noMsgFlags msg) `shouldReturn` Left (ProxyProtocolError $ SMP.PROXY SMP.NO_SESSION)
+    forM_ ([prevVersion minServerSMPRelayVersion, nextVersion currentServerSMPRelayVersion] :: [VersionSMP]) $ \v ->
+      runExceptT' (proxySMPMessage pc NRMInteractive sess {prVersion = v} Nothing sndId noMsgFlags msg) `shouldReturn` Left (ProxyProtocolError $ SMP.PROXY $ SMP.PROTOCOL $ SMP.PROXY $ SMP.BROKER $ SMP.TRANSPORT TEVersion)
     -- receive 1
     (_tSess, _, [(_entId, STEvent (Right (SMP.MSG RcvMessage {msgId, msgBody = EncRcvMsgBody encBody})))]) <- atomically $ readTBQueue msgQ
     dec msgId encBody `shouldBe` Right msg