Skip to content

Commit

Permalink
harden valid group info check
Browse files Browse the repository at this point in the history
  • Loading branch information
@erikolsson
erikolsson committed Jan 13, 2025
1 parent 653ee3a commit 211e4fd
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 4 deletions.
42 changes: 38 additions & 4 deletions core/mls/mls-tools/crates/mls/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ fn validate_group_info_message(group_info_message_bytes: Vec<u8>, expected_epoch
ValidationResult::Valid
}

fn validate_external_group_can_process_group_info_message(mut external_group: ExternalGroup<ExternalConfig>, group_info_message_bytes: Vec<u8>) -> Result<(), ValidationResult> {
let group_info_message = match MlsMessage::from_bytes(&group_info_message_bytes) {
Ok(group_info_message) => group_info_message,
Err(_) => return Err(ValidationResult::InvalidGroupInfo)
};

match external_group.process_incoming_message(group_info_message) {
Ok(_) => return Ok(()),
Err(_) => Err(ValidationResult::InvalidGroupInfo.into())
}
}

pub fn validate_initial_group_info_request(request: InitialGroupInfoRequest) -> InitialGroupInfoResponse {
let external_client = create_external_client();
let external_group_snapshot = match ExternalSnapshot::from_bytes(&request.external_group_snapshot) {
Expand All @@ -81,14 +93,14 @@ pub fn validate_initial_group_info_request(request: InitialGroupInfoRequest) ->
}
};

let external_group = match external_client.load_group(external_group_snapshot) {
let mut external_group = match external_client.load_group(external_group_snapshot) {
Ok(group) => group,
Err(_) => return InitialGroupInfoResponse {
result: ValidationResult::InvalidExternalGroup.into(),
}
};

match validate_group_info_message(request.group_info_message,
match validate_group_info_message(request.group_info_message.clone(),
0,
external_group.group_context().group_id()) {
ValidationResult::Valid => {},
Expand Down Expand Up @@ -124,6 +136,14 @@ pub fn validate_initial_group_info_request(request: InitialGroupInfoRequest) ->
result: ValidationResult::InvalidPublicSignatureKey.into(),
};
}

match validate_external_group_can_process_group_info_message(external_group, request.group_info_message) {
Ok(_) => {},
Err(result) => return InitialGroupInfoResponse {
result: result.into(),
}
}

return InitialGroupInfoResponse {
result: ValidationResult::Valid.into(),
};
Expand Down Expand Up @@ -162,7 +182,7 @@ pub fn validate_external_join_request(request: ExternalJoinRequest) -> ExternalJ
};
}

match validate_group_info_message(request.proposed_external_join_info_message,
match validate_group_info_message(request.proposed_external_join_info_message.clone(),
external_group.group_context().epoch() + 1,
external_group.group_context().group_id()) {
ValidationResult::Valid => {},
Expand Down Expand Up @@ -211,6 +231,13 @@ pub fn validate_external_join_request(request: ExternalJoinRequest) -> ExternalJ
true => {}
}

match validate_external_group_can_process_group_info_message(external_group, request.proposed_external_join_info_message) {
Ok(_) => {},
Err(result) => return ExternalJoinResponse {
result: result.into(),
}
}

return ExternalJoinResponse {
result: ValidationResult::Valid.into(),
};
Expand Down Expand Up @@ -312,7 +339,7 @@ pub fn validate_welcome_message_request(request: WelcomeMessageRequest) -> Welco
};
}

match validate_group_info_message(request.group_info_message,
match validate_group_info_message(request.group_info_message.clone(),
external_group.group_context().epoch() + 1,
external_group.group_context().group_id()) {
ValidationResult::Valid => {},
Expand Down Expand Up @@ -376,6 +403,13 @@ pub fn validate_welcome_message_request(request: WelcomeMessageRequest) -> Welco
};
}

match validate_external_group_can_process_group_info_message(external_group, request.group_info_message) {
Ok(_) => {},
Err(result) => return WelcomeMessageResponse {
result: result.into(),
}
}

return WelcomeMessageResponse {
result: ValidationResult::Valid.into(),
};
Expand Down
62 changes: 62 additions & 0 deletions packages/sdk/src/tests/multi_ne/mls.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,22 @@ describe('mlsTests', () => {
)
})

test('invalid group info for initialize group is rejected', async () => {
const { groupInfoMessage, externalGroupSnapshot } =
await createGroupInfoAndExternalSnapshot(bobMlsGroup)
// tamper with the message a little bit
const invalidGroupInfoMessage = groupInfoMessage
invalidGroupInfoMessage[invalidGroupInfoMessage.length - 2] += 1 // make it invalid
const payload = makeMlsPayloadInitializeGroup(
bobMlsClient.signaturePublicKey(),
externalGroupSnapshot,
groupInfoMessage,
)
await expect(bobClient._debugSendMls(streamId, payload)).rejects.toThrow(
'INVALID_GROUP_INFO',
)
})

test('clients can create MLS Groups in channels', async () => {
const { groupInfoMessage, externalGroupSnapshot } =
await createGroupInfoAndExternalSnapshot(bobMlsGroup)
Expand Down Expand Up @@ -329,6 +345,26 @@ describe('mlsTests', () => {
)
})

test('Invalid group info for external commits is rejected', async () => {
const { commit, groupInfoMessage } = await commitExternal(
aliceMlsClient,
latestGroupInfoMessage,
latestExternalGroupSnapshot,
)
// tamper with the message a little bit
const invalidGroupInfoMessage = groupInfoMessage
invalidGroupInfoMessage[invalidGroupInfoMessage.length - 2] += 1 // make it invalid

const aliceMlsPayload = makeMlsPayloadExternalJoin(
aliceMlsClient.signaturePublicKey(),
commit,
invalidGroupInfoMessage,
)
await expect(aliceClient._debugSendMls(streamId, aliceMlsPayload)).rejects.toThrow(
'INVALID_GROUP_INFO',
)
})

test('Valid external commits are accepted', async () => {
const { commit: aliceCommit, groupInfoMessage: aliceGroupInfoMessage } =
await commitExternal(
Expand Down Expand Up @@ -544,6 +580,32 @@ describe('mlsTests', () => {
)
})

test('invalid group info for welcome messages is rejected', async () => {
const mls = bobClient.streams.get(streamId)!.view.membershipContent.mls
const keyPackage = Object.values(mls.pendingKeyPackages)[0]
const kp = MlsMessage.fromBytes(keyPackage.keyPackage)
const commitOutput = await bobMlsGroup.addMember(kp)
await bobMlsGroup.clearPendingCommit()
const groupInfoMessage = commitOutput.externalCommitGroupInfo!.toBytes()

// tamper with the message a little bit
const invalidGroupInfoMessage = groupInfoMessage
invalidGroupInfoMessage[invalidGroupInfoMessage.length - 2] += 1 // make it invalid

const commit = commitOutput.commitMessage.toBytes()
const welcomeMessages = commitOutput.welcomeMessages.map((wm) => wm.toBytes())

const payload = makeMlsPayloadWelcomeMessage(
commit,
[keyPackage.signaturePublicKey],
invalidGroupInfoMessage,
welcomeMessages,
)
await expect(aliceClient._debugSendMls(streamId, payload)).rejects.toThrow(
'INVALID_GROUP_INFO',
)
})

test('clients can add other members from key packages', async () => {
const mls = bobClient.streams.get(streamId)!.view.membershipContent.mls
const keyPackage = Object.values(mls.pendingKeyPackages)[0]
Expand Down

0 comments on commit 211e4fd

Please sign in to comment.