feat(session): Validate MLS proposal sender and type#23
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
I have implemented a critical validation step for incoming MLS proposals within the session processing logic. This enhancement ensures that only proposals conforming to the DAVE protocol's security requirements are accepted, specifically by validating the sender type and the proposal's content type.
Key Changes
Proposal Validation Function: I introduced a new private method,
proposalAllowedByDAVELocked, insession/mls.go. This function is responsible for inspecting anMLSMessageto determine if it originates from anExternalsender and if its content is either anAddorRemoveproposal.Integration into Batch Processing: The
processProposalBatchLockedfunction now callsproposalAllowedByDAVELockedfor each incoming proposal. If a proposal fails this validation, it is logged and rejected, preventing further processing.Enhanced Test Coverage: I updated
session/proposal_validation_test.goto include new test cases that specifically verify the rejection of proposals with disallowed sender types (e.g.,SenderTypeNewMemberProposal) and disallowed proposal types (e.g.,PreSharedKeyProposal). I also addedbuildExternalSenderPackageWithKeyto facilitate accurate testing of proposals originating from the external sender, mirroring real-world DAVE behavior.Technical Impact
This change significantly enhances the security and robustness of our MLS session management. By strictly validating proposal senders and types at an early stage, I am enforcing DAVE's protocol specification, which dictates that only
AddandRemoveproposals from the voice gateway's external sender are permitted. This prevents potential vulnerabilities from malformed or unauthorized proposals, ensuring the integrity and security of the group state. The improved test suite now provides stronger assurances that our validation logic correctly handles both valid and invalid proposal scenarios.🧪 Test Plan & Evidence
Suggested Manual Verification
go test ./...and ensure all tests pass