Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Replace proposal #174

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a9395e3
Add Rust scaffolding
bifurcation May 31, 2024
17740df
Attempt to make a foreign-compatible trait
bifurcation May 31, 2024
6bb2247
Stub in test case for creation of a commit covering a Replace proposal
bifurcation Jul 1, 2024
66e5ed8
Add a struct for a Replace proposal
bifurcation Jul 1, 2024
19d9341
Allow inclusion of Replace in a Commit
bifurcation Jul 4, 2024
6079c58
Verify correct processing of Replace by committer and member
bifurcation Jul 5, 2024
c6f93e7
Apply validation rules to Replace proposals
bifurcation Jul 5, 2024
0130e04
Segregate pending update state by source
bifurcation Jul 5, 2024
cfc536e
Clean up some things that need to be feature-gated
bifurcation Jul 5, 2024
62ded9d
Test and fix abandonment logic
bifurcation Jul 5, 2024
494179f
Add leaf_node_epoch extension
bifurcation Jul 6, 2024
1269317
Partly revert update state tracking changes
bifurcation Jul 11, 2024
2e41ed7
Add tests for leaf node epoch logic
bifurcation Jul 11, 2024
78ac756
Fix test failures outside of 'replace'
bifurcation Jul 14, 2024
251f823
More test cleanup
bifurcation Jul 14, 2024
2096668
Move integration tests to group/mod.rs
bifurcation Jul 14, 2024
890945f
Clear errors building without default features
bifurcation Jul 14, 2024
21b682f
CI errors
bifurcation Jul 14, 2024
8fc1de2
Only derive serde traits when serde is enabled
bifurcation Jul 14, 2024
4d70372
Revert accidentally committed changes
bifurcation Jul 15, 2024
a0f8dcf
Fix no-std issues
bifurcation Jul 15, 2024
c864672
Build properly with only -F replace_proposal
bifurcation Jul 15, 2024
6c4ad02
Clippy errors
bifurcation Jul 15, 2024
9ae9682
Fix bad nolint directive
bifurcation Jul 15, 2024
eb8dd52
Address first comments from @mulmarta
bifurcation Jul 15, 2024
75f265d
if let
bifurcation Jul 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mls-rs-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ ffi = ["dep:safer-ffi", "dep:safer-ffi-gen"]
x509 = []
test_suite = ["serde", "dep:serde_json", "dep:itertools"]
serde = ["dep:serde", "zeroize/serde", "hex/serde", "dep:serde_bytes"]
replace_proposal = []

[dependencies]
mls-rs-codec = { version = "0.5.2", path = "../mls-rs-codec", default-features = false}
Expand Down
6 changes: 6 additions & 0 deletions mls-rs-core/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ impl ExtensionType {
pub const EXTERNAL_PUB: ExtensionType = ExtensionType(4);
pub const EXTERNAL_SENDERS: ExtensionType = ExtensionType(5);

// XXX(RLB): This value is chosen from the vendor range, since this is an experimental
// implementation that is only intended to work between `mls-rs` instances. Once a real value
// is assigned by IANA, this code will need to be updated.
#[cfg(feature = "replace_proposal")]
pub const LEAF_NODE_EPOCH: ExtensionType = ExtensionType(0xFF01);

/// Default extension types defined
/// in [RFC 9420](https://www.rfc-editor.org/rfc/rfc9420.html#name-leaf-node-contents)
pub const DEFAULT: &'static [ExtensionType] = &[
Expand Down
3 changes: 3 additions & 0 deletions mls-rs-core/src/group/proposal_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ impl ProposalType {
pub const EXTERNAL_INIT: ProposalType = ProposalType(6);
pub const GROUP_CONTEXT_EXTENSIONS: ProposalType = ProposalType(7);

#[cfg(feature = "replace_proposal")]
pub const REPLACE: ProposalType = ProposalType(0xff01);

/// Default proposal types defined
/// in [RFC 9420](https://www.rfc-editor.org/rfc/rfc9420.html#name-leaf-node-contents)
pub const DEFAULT: &'static [ProposalType] = &[
Expand Down
1 change: 1 addition & 0 deletions mls-rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ by_ref_proposal = []
psk = []
x509 = ["mls-rs-core/x509", "dep:mls-rs-identity-x509"]
rfc_compliant = ["state_update", "private_message", "custom_proposal", "out_of_order", "psk", "x509", "prior_epoch", "by_ref_proposal", "mls-rs-core/rfc_compliant"]
replace_proposal = ["mls-rs-core/replace_proposal", "by_ref_proposal"]

std = ["mls-rs-core/std", "mls-rs-codec/std", "mls-rs-identity-x509?/std", "hex/std", "futures/std", "itertools/use_std", "safer-ffi-gen?/std", "zeroize/std", "dep:debug_tree", "dep:thiserror", "serde?/std"]

Expand Down
15 changes: 13 additions & 2 deletions mls-rs/examples/basic_usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use mls_rs::{
basic::{BasicCredential, BasicIdentityProvider},
SigningIdentity,
},
CipherSuite, CipherSuiteProvider, Client, CryptoProvider, ExtensionList,
CipherSuite, CipherSuiteProvider, Client, CryptoProvider, ExtensionList, Group,
};

const CIPHERSUITE: CipherSuite = CipherSuite::CURVE25519_AES128;
Expand Down Expand Up @@ -61,8 +61,19 @@ fn main() -> Result<(), MlsError> {
alice_group.apply_pending_commit()?;

// Bob joins the group with the welcome message created as part of Alice's commit.
let (mut bob_group, _) = bob.join_group(None, &alice_commit.welcome_messages[0])?;
let (bob_group, _) = bob.join_group(None, &alice_commit.welcome_messages[0])?;

#[cfg(feature = "private_message")]
encrypt_decrypt(alice_group, bob_group)?;

Ok(())
}

#[cfg(feature = "private_message")]
fn encrypt_decrypt<C: MlsConfig>(
mut alice_group: Group<C>,
mut bob_group: Group<C>,
) -> Result<(), MlsError> {
// Alice encrypts an application message to Bob.
let msg = alice_group.encrypt_application_message(b"hello world", Default::default())?;

Expand Down
8 changes: 7 additions & 1 deletion mls-rs/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,10 +1045,16 @@ mod tests {

#[test]
fn builder_can_be_obtained_from_client_to_edit_properties_for_new_client() {
#[cfg(not(feature = "replace_proposal"))]
let expected_extensions = [33, 34].map(Into::into);

#[cfg(feature = "replace_proposal")]
let expected_extensions = [0xff01, 33, 34].map(Into::into);

let alice = TestClientBuilder::new_for_test()
.extension_type(33.into())
.build();
let bob = alice.to_builder().extension_type(34.into()).build();
assert_eq!(bob.config.supported_extensions(), [33, 34].map(Into::into));
assert_eq!(bob.config.supported_extensions(), expected_extensions);
}
}
11 changes: 10 additions & 1 deletion mls-rs/src/client_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ use crate::{
#[cfg(feature = "std")]
use crate::time::MlsTime;

#[cfg(feature = "replace_proposal")]
use alloc::vec;

use alloc::vec::Vec;

#[cfg(feature = "sqlite")]
Expand Down Expand Up @@ -879,8 +882,14 @@ pub(crate) struct Settings {

impl Default for Settings {
fn default() -> Self {
#[cfg(not(feature = "replace_proposal"))]
let extension_types = Default::default();

#[cfg(feature = "replace_proposal")]
let extension_types = vec![ExtensionType::LEAF_NODE_EPOCH];
Comment on lines +885 to +889
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if this should be default or not just because the feature is on? I think I view the feature more as "you can possibly do this" vs "you definitely want to do this"


Self {
extension_types: Default::default(),
extension_types,
protocol_versions: Default::default(),
key_package_extensions: Default::default(),
leaf_node_extensions: Default::default(),
Expand Down
43 changes: 43 additions & 0 deletions mls-rs/src/extension/built_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,35 @@ impl MlsCodecExtension for ExternalSendersExt {
}
}

/// Mark a LeafNode with the epoch in which it was created. Note that a LeafNode with
/// `leaf_node_source` set to `update` or `commit` is already bound to the `group_id` for the group
/// by way of the `LeafNodeTBS` signed structure.
#[cfg(feature = "replace_proposal")]
#[cfg_attr(
all(feature = "ffi", not(test)),
safer_ffi_gen::ffi_type(clone, opaque)
)]
#[derive(Clone, Debug, PartialEq, Eq, MlsSize, MlsEncode, MlsDecode)]
#[non_exhaustive]
pub struct LeafNodeEpochExt {
pub epoch: u64,
}

#[cfg(feature = "replace_proposal")]
#[cfg_attr(all(feature = "ffi", not(test)), safer_ffi_gen::safer_ffi_gen)]
impl LeafNodeEpochExt {
pub fn new(epoch: u64) -> Self {
Self { epoch }
}
}

#[cfg(feature = "replace_proposal")]
impl MlsCodecExtension for LeafNodeEpochExt {
fn extension_type() -> ExtensionType {
ExtensionType::LEAF_NODE_EPOCH
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -327,4 +356,18 @@ mod tests {
let restored = ExternalPubExt::from_extension(&as_extension).unwrap();
assert_eq!(ext, restored)
}

#[cfg(feature = "replace_proposal")]
#[test]
fn test_leaf_node_epoch() {
let ext = LeafNodeEpochExt {
epoch: 0x01234567890abcdef,
};

let as_extension = ext.clone().into_extension().unwrap();
assert_eq!(as_extension.extension_type, ExtensionType::LEAF_NODE_EPOCH);

let restored = LeafNodeEpochExt::from_extension(&as_extension).unwrap();
assert_eq!(ext, restored)
}
}
61 changes: 61 additions & 0 deletions mls-rs/src/group/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,19 @@ where
Ok(self)
}

/// Insert a [`ReplaceProposal`](crate::group::proposal::ReplaceProposal) into
/// the current commit that is being built.
#[cfg(feature = "replace_proposal")]
pub fn replace_member(
mut self,
to_replace: u32,
leaf_node: LeafNode,
) -> Result<Self, MlsError> {
Comment on lines +203 to +207
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a public interface to obtain the LeafNode? Maybe it would be easier to do replace_member(mut self, to_replace: u32, proposal: UpdateProposal) or even replace_member(mut self, proposal: MlsMessage) (message contains index) ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, to date we have made LeafNode private, maybe it should stay private?

let proposal = self.group.replace_proposal(to_replace, leaf_node)?;
self.proposals.push(proposal);
Ok(self)
}

/// Insert a
/// [`GroupContextExtensions`](crate::group::proposal::Proposal::GroupContextExtensions)
/// into the current commit that is being built.
Expand Down Expand Up @@ -1017,6 +1030,54 @@ mod tests {
assert_commit_builder_output(group, commit_output, vec![expected_remove], 0);
}

#[cfg(feature = "replace_proposal")]
#[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
async fn test_commit_builder_replace() -> Result<(), MlsError> {
let mut group = test_group_custom_config(TEST_PROTOCOL_VERSION, TEST_CIPHER_SUITE, |b| {
b.custom_proposal_type(ProposalType::REPLACE)
})
.await
.group;

let (alice, alice_kp) =
test_client_with_key_pkg(TEST_PROTOCOL_VERSION, TEST_CIPHER_SUITE, "alice").await;

// Add Alice to the group
let output = group
.commit_builder()
.add_member(alice_kp.clone())
.unwrap()
.build()
.await?;

group.apply_pending_commit().await?;

// Alice creates an Update proposal, including a fresh LeafNode
let (mut alice_group, _) = alice.join_group(None, &output.welcome_messages[0])?;
let proposal = alice_group.update_proposal(None, None)?;
let Proposal::Update(update) = proposal else {
panic!("non update proposal found")
};

// The committer replaces Alice's appearance in the group
let commit_output = group
.commit_builder()
.replace_member(1, update.leaf_node.clone())?
.build()
.await?;

let expected_replace = group.replace_proposal(1, update.leaf_node)?;

assert_commit_builder_output(
group.clone(),
commit_output.clone(),
vec![expected_replace],
0,
);

Ok(())
}

#[cfg(feature = "psk")]
#[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
async fn test_commit_builder_psk() {
Expand Down
Loading
Loading