Skip to content

Commit

Permalink
Backport mixed protocol ciphersuite fix #4048 (#4049)
Browse files Browse the repository at this point in the history
Co-authored-by: Marko Dimjašević <[email protected]>
  • Loading branch information
pcapriotti and mdimjasevic authored May 15, 2024
1 parent 8284169 commit b61f37a
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 12 deletions.
1 change: 1 addition & 0 deletions changelog.d/3-bug-fixes/mixed-ciphersuite
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix hardcoded ciphersuite when switching to mixed
15 changes: 13 additions & 2 deletions integration/test/Test/MLS.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import qualified Data.Aeson.KeyMap as KM
import qualified Data.ByteString.Base64 as Base64
import qualified Data.ByteString.Char8 as B8
import qualified Data.Set as Set
import qualified Data.Text as T
import qualified Data.Text.Encoding as T
import qualified Data.Text.Read as T
import MLS.Util
import Notifications
import SetupHelpers
Expand Down Expand Up @@ -101,6 +103,7 @@ testMixedProtocolUpgrade secondDomain = do
bindResponse (getConversation alice qcnv) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json %. "protocol" `shouldMatch` "mixed"
resp.json %. "epoch" `shouldMatchInt` 0

bindResponse (putConversationProtocol alice qcnv "mixed") $ \resp -> do
resp.status `shouldMatchInt` 204
Expand All @@ -121,8 +124,9 @@ testMixedProtocolNonTeam secondDomain = do
bindResponse (putConversationProtocol bob qcnv "mixed") $ \resp -> do
resp.status `shouldMatchInt` 403

testMixedProtocolAddUsers :: HasCallStack => Domain -> App ()
testMixedProtocolAddUsers secondDomain = do
testMixedProtocolAddUsers :: HasCallStack => Domain -> Ciphersuite -> App ()
testMixedProtocolAddUsers secondDomain suite = do
setMLSCiphersuite suite
(alice, tid, _) <- createTeam OwnDomain 1
[bob, charlie] <- replicateM 2 (randomUser secondDomain def)
connectUsers [alice, bob, charlie]
Expand All @@ -139,6 +143,7 @@ testMixedProtocolAddUsers secondDomain = do

bindResponse (getConversation alice qcnv) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json %. "epoch" `shouldMatchInt` 0
createGroup alice1 resp.json

traverse_ uploadNewKeyPackage [bob1]
Expand All @@ -150,6 +155,12 @@ testMixedProtocolAddUsers secondDomain = do
n <- awaitMatch (\n -> nPayload n %. "type" `isEqual` "conversation.mls-welcome") ws
nPayload n %. "data" `shouldMatch` T.decodeUtf8 (Base64.encode welcome)

bindResponse (getConversation alice qcnv) $ \resp -> do
resp.status `shouldMatchInt` 200
resp.json %. "epoch" `shouldMatchInt` 1
(suiteCode, _) <- assertOne $ T.hexadecimal (T.pack suite.code)
resp.json %. "cipher_suite" `shouldMatchInt` suiteCode

testMixedProtocolUserLeaves :: HasCallStack => Domain -> App ()
testMixedProtocolUserLeaves secondDomain = do
(alice, tid, _) <- createTeam OwnDomain 1
Expand Down
3 changes: 1 addition & 2 deletions services/galley/src/Galley/API/Action.hs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ import Wire.API.Federation.API.Galley
import Wire.API.Federation.API.Galley qualified as F
import Wire.API.Federation.Error
import Wire.API.FederationStatus
import Wire.API.MLS.CipherSuite
import Wire.API.Routes.Internal.Brig.Connection
import Wire.API.Team.Feature
import Wire.API.Team.LegalHold
Expand Down Expand Up @@ -492,7 +491,7 @@ performAction tag origUser lconv action = do
SConversationUpdateProtocolTag -> do
case (protocolTag (convProtocol (tUnqualified lconv)), action, convTeam (tUnqualified lconv)) of
(ProtocolProteusTag, ProtocolMixedTag, Just _) -> do
E.updateToMixedProtocol lcnv (convType (tUnqualified lconv)) MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519
E.updateToMixedProtocol lcnv (convType (tUnqualified lconv))
pure (mempty, action)
(ProtocolMixedTag, ProtocolMLSTag, Just tid) -> do
mig <- getFeatureStatus @MlsMigrationConfig DontDoAuth tid
Expand Down
9 changes: 4 additions & 5 deletions services/galley/src/Galley/Cassandra/Conversation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -396,15 +396,14 @@ updateToMixedProtocol ::
r =>
Local ConvId ->
ConvType ->
CipherSuiteTag ->
Sem r ()
updateToMixedProtocol lcnv ct cs = do
updateToMixedProtocol lcnv ct = do
let gid = convToGroupId . groupIdParts ct $ Conv <$> tUntagged lcnv
epoch = Epoch 0
embedClient . retry x5 . batch $ do
setType BatchLogged
setConsistency LocalQuorum
addPrepQuery Cql.updateToMixedConv (tUnqualified lcnv, ProtocolMixedTag, gid, epoch, cs)
addPrepQuery Cql.updateToMixedConv (tUnqualified lcnv, ProtocolMixedTag, gid, epoch)
pure ()

updateToMLSProtocol ::
Expand Down Expand Up @@ -493,9 +492,9 @@ interpretConversationStoreToCassandra = interpret $ \case
ReleaseCommitLock gId epoch -> do
logEffect "ConversationStore.ReleaseCommitLock"
embedClient $ releaseCommitLock gId epoch
UpdateToMixedProtocol cid ct cs -> do
UpdateToMixedProtocol cid ct -> do
logEffect "ConversationStore.UpdateToMixedProtocol"
updateToMixedProtocol cid ct cs
updateToMixedProtocol cid ct
UpdateToMLSProtocol cid -> do
logEffect "ConversationStore.UpdateToMLSProtocol"
updateToMLSProtocol cid
4 changes: 2 additions & 2 deletions services/galley/src/Galley/Cassandra/Queries.hs
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ insertMLSSelfConv =
<> show (fromEnum ProtocolMLSTag)
<> ", ?)"

updateToMixedConv :: PrepQuery W (ConvId, ProtocolTag, GroupId, Epoch, CipherSuiteTag) ()
updateToMixedConv :: PrepQuery W (ConvId, ProtocolTag, GroupId, Epoch) ()
updateToMixedConv =
"insert into conversation (conv, protocol, group_id, epoch, cipher_suite) values (?, ?, ?, ?, ?)"
"insert into conversation (conv, protocol, group_id, epoch) values (?, ?, ?, ?)"

updateToMLSConv :: PrepQuery W (ConvId, ProtocolTag) ()
updateToMLSConv = "insert into conversation (conv, protocol) values (?, ?)"
Expand Down
2 changes: 1 addition & 1 deletion services/galley/src/Galley/Effects/ConversationStore.hs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ data ConversationStore m a where
SetGroupInfo :: ConvId -> GroupInfoData -> ConversationStore m ()
AcquireCommitLock :: GroupId -> Epoch -> NominalDiffTime -> ConversationStore m LockAcquired
ReleaseCommitLock :: GroupId -> Epoch -> ConversationStore m ()
UpdateToMixedProtocol :: Local ConvId -> ConvType -> CipherSuiteTag -> ConversationStore m ()
UpdateToMixedProtocol :: Local ConvId -> ConvType -> ConversationStore m ()
UpdateToMLSProtocol :: Local ConvId -> ConversationStore m ()

makeSem ''ConversationStore
Expand Down

0 comments on commit b61f37a

Please sign in to comment.