Skip to content

Commit

Permalink
Revert "harden valid group info check"
Browse files Browse the repository at this point in the history 
This reverts commit 211e4fd.
  • Loading branch information
@erikolsson
erikolsson committed Jan 13, 2025
1 parent 211e4fd commit a5e924f
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 100 deletions.
42 changes: 4 additions & 38 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,18 +72,6 @@ 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 @@ -93,14 +81,14 @@ pub fn validate_initial_group_info_request(request: InitialGroupInfoRequest) ->
}
};

let mut external_group = match external_client.load_group(external_group_snapshot) {
let 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.clone(),
match validate_group_info_message(request.group_info_message,
0,
external_group.group_context().group_id()) {
ValidationResult::Valid => {},
Expand Down Expand Up @@ -136,14 +124,6 @@ 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 @@ -182,7 +162,7 @@ pub fn validate_external_join_request(request: ExternalJoinRequest) -> ExternalJ
};
}

match validate_group_info_message(request.proposed_external_join_info_message.clone(),
match validate_group_info_message(request.proposed_external_join_info_message,
external_group.group_context().epoch() + 1,
external_group.group_context().group_id()) {
ValidationResult::Valid => {},
Expand Down Expand Up @@ -231,13 +211,6 @@ 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 @@ -339,7 +312,7 @@ pub fn validate_welcome_message_request(request: WelcomeMessageRequest) -> Welco
};
}

match validate_group_info_message(request.group_info_message.clone(),
match validate_group_info_message(request.group_info_message,
external_group.group_context().epoch() + 1,
external_group.group_context().group_id()) {
ValidationResult::Valid => {},
Expand Down Expand Up @@ -403,13 +376,6 @@ 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: 0 additions & 62 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,22 +268,6 @@ 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 @@ -345,26 +329,6 @@ 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 @@ -580,32 +544,6 @@ 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 a5e924f

Please sign in to comment.