Skip to content

Commit

Permalink
[Bugfix] ExternalGroup was not able to issue local proposals (#210)
Browse files Browse the repository at this point in the history
* [Bugfix] ExternalGroup was not able to issue local proposals

* Bump version

---------

Co-authored-by: Marta Mularczyk <mulmarta@amazon.com>
  • Loading branch information
mulmarta and Marta Mularczyk authored Nov 11, 2024
1 parent 0532380 commit b6d257e
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 23 deletions.
2 changes: 1 addition & 1 deletion mls-rs/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mls-rs"
version = "0.42.1"
version = "0.42.2"
edition = "2021"
description = "An implementation of Messaging Layer Security (RFC 9420)"
homepage = "https://github.com/awslabs/mls-rs"
Expand Down
18 changes: 14 additions & 4 deletions mls-rs/src/group/proposal_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4173,17 +4173,27 @@ mod tests {
#[cfg(feature = "by_ref_proposal")]
let receiver = receiver.with_extensions(extensions);

let (receiver, proposals, proposer) = if by_ref {
let (receiver, proposals, proposer, source) = if by_ref {
let proposal_ref = make_proposal_ref(proposal, proposer).await;
let receiver = receiver.cache(proposal_ref.clone(), proposal.clone(), proposer);
(receiver, vec![ProposalOrRef::from(proposal_ref)], proposer)
(
receiver,
vec![ProposalOrRef::from(proposal_ref.clone())],
proposer,
ProposalSource::ByReference(proposal_ref),
)
} else {
(receiver, vec![proposal.clone().into()], committer)
(
receiver,
vec![proposal.clone().into()],
committer,
ProposalSource::Local,
)
};

let res = receiver.receive(proposals).await;

if proposer_can_propose(proposer, proposal.proposal_type(), by_ref).is_err() {
if proposer_can_propose(proposer, proposal.proposal_type(), &source).is_err() {
assert_matches!(res, Err(MlsError::InvalidProposalTypeForSender));
} else {
let is_self_update = proposal.proposal_type() == ProposalType::UPDATE
Expand Down
41 changes: 23 additions & 18 deletions mls-rs/src/group/proposal_filter/filtering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ use crate::{
CipherSuiteProvider, ExtensionList,
};

use super::filtering_common::{filter_out_invalid_psks, ApplyProposalsOutput, ProposalApplier};
use super::{
filtering_common::{filter_out_invalid_psks, ApplyProposalsOutput, ProposalApplier},
ProposalSource,
};

#[cfg(feature = "by_ref_proposal")]
use crate::extension::ExternalSendersExt;
Expand Down Expand Up @@ -439,18 +442,18 @@ fn filter_out_external_init(
pub(crate) fn proposer_can_propose(
proposer: Sender,
proposal_type: ProposalType,
by_ref: bool,
source: &ProposalSource,
) -> Result<(), MlsError> {
let can_propose = match (proposer, by_ref) {
(Sender::Member(_), false) => matches!(
let can_propose = match (proposer, source) {
(Sender::Member(_), ProposalSource::ByValue | ProposalSource::Local) => matches!(
proposal_type,
ProposalType::ADD
| ProposalType::REMOVE
| ProposalType::PSK
| ProposalType::RE_INIT
| ProposalType::GROUP_CONTEXT_EXTENSIONS
),
(Sender::Member(_), true) => matches!(
(Sender::Member(_), ProposalSource::ByReference(_)) => matches!(
proposal_type,
ProposalType::ADD
| ProposalType::UPDATE
Expand All @@ -460,23 +463,25 @@ pub(crate) fn proposer_can_propose(
| ProposalType::GROUP_CONTEXT_EXTENSIONS
),
#[cfg(feature = "by_ref_proposal")]
(Sender::External(_), false) => false,
(Sender::External(_), ProposalSource::ByValue) => false,
#[cfg(feature = "by_ref_proposal")]
(Sender::External(_), true) => matches!(
(Sender::External(_), _) => matches!(
proposal_type,
ProposalType::ADD
| ProposalType::REMOVE
| ProposalType::RE_INIT
| ProposalType::PSK
| ProposalType::GROUP_CONTEXT_EXTENSIONS
),
(Sender::NewMemberCommit, false) => matches!(
(Sender::NewMemberCommit, ProposalSource::ByValue | ProposalSource::Local) => matches!(
proposal_type,
ProposalType::REMOVE | ProposalType::PSK | ProposalType::EXTERNAL_INIT
),
(Sender::NewMemberCommit, true) => false,
(Sender::NewMemberProposal, false) => false,
(Sender::NewMemberProposal, true) => matches!(proposal_type, ProposalType::ADD),
(Sender::NewMemberCommit, ProposalSource::ByReference(_)) => false,
(Sender::NewMemberProposal, ProposalSource::ByValue | ProposalSource::Local) => false,
(Sender::NewMemberProposal, ProposalSource::ByReference(_)) => {
matches!(proposal_type, ProposalType::ADD)
}
};

can_propose
Expand All @@ -490,7 +495,7 @@ pub(crate) fn filter_out_invalid_proposers(
) -> Result<ProposalBundle, MlsError> {
for i in (0..proposals.add_proposals().len()).rev() {
let p = &proposals.add_proposals()[i];
let res = proposer_can_propose(p.sender, ProposalType::ADD, p.is_by_reference());
let res = proposer_can_propose(p.sender, ProposalType::ADD, &p.source);

if !apply_strategy(strategy, p.is_by_reference(), res)? {
proposals.remove::<AddProposal>(i);
Expand All @@ -499,7 +504,7 @@ pub(crate) fn filter_out_invalid_proposers(

for i in (0..proposals.update_proposals().len()).rev() {
let p = &proposals.update_proposals()[i];
let res = proposer_can_propose(p.sender, ProposalType::UPDATE, p.is_by_reference());
let res = proposer_can_propose(p.sender, ProposalType::UPDATE, &p.source);

if !apply_strategy(strategy, p.is_by_reference(), res)? {
proposals.remove::<UpdateProposal>(i);
Expand All @@ -509,7 +514,7 @@ pub(crate) fn filter_out_invalid_proposers(

for i in (0..proposals.remove_proposals().len()).rev() {
let p = &proposals.remove_proposals()[i];
let res = proposer_can_propose(p.sender, ProposalType::REMOVE, p.is_by_reference());
let res = proposer_can_propose(p.sender, ProposalType::REMOVE, &p.source);

if !apply_strategy(strategy, p.is_by_reference(), res)? {
proposals.remove::<RemoveProposal>(i);
Expand All @@ -519,7 +524,7 @@ pub(crate) fn filter_out_invalid_proposers(
#[cfg(feature = "psk")]
for i in (0..proposals.psk_proposals().len()).rev() {
let p = &proposals.psk_proposals()[i];
let res = proposer_can_propose(p.sender, ProposalType::PSK, p.is_by_reference());
let res = proposer_can_propose(p.sender, ProposalType::PSK, &p.source);

if !apply_strategy(strategy, p.is_by_reference(), res)? {
proposals.remove::<PreSharedKeyProposal>(i);
Expand All @@ -528,7 +533,7 @@ pub(crate) fn filter_out_invalid_proposers(

for i in (0..proposals.reinit_proposals().len()).rev() {
let p = &proposals.reinit_proposals()[i];
let res = proposer_can_propose(p.sender, ProposalType::RE_INIT, p.is_by_reference());
let res = proposer_can_propose(p.sender, ProposalType::RE_INIT, &p.source);

if !apply_strategy(strategy, p.is_by_reference(), res)? {
proposals.remove::<ReInitProposal>(i);
Expand All @@ -537,7 +542,7 @@ pub(crate) fn filter_out_invalid_proposers(

for i in (0..proposals.external_init_proposals().len()).rev() {
let p = &proposals.external_init_proposals()[i];
let res = proposer_can_propose(p.sender, ProposalType::EXTERNAL_INIT, p.is_by_reference());
let res = proposer_can_propose(p.sender, ProposalType::EXTERNAL_INIT, &p.source);

if !apply_strategy(strategy, p.is_by_reference(), res)? {
proposals.remove::<ExternalInit>(i);
Expand All @@ -547,7 +552,7 @@ pub(crate) fn filter_out_invalid_proposers(
for i in (0..proposals.group_context_ext_proposals().len()).rev() {
let p = &proposals.group_context_ext_proposals()[i];
let gce_type = ProposalType::GROUP_CONTEXT_EXTENSIONS;
let res = proposer_can_propose(p.sender, gce_type, p.is_by_reference());
let res = proposer_can_propose(p.sender, gce_type, &p.source);

if !apply_strategy(strategy, p.is_by_reference(), res)? {
proposals.remove::<ExtensionList>(i);
Expand Down

0 comments on commit b6d257e

Please sign in to comment.