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

Remove test only distinction for group_context_ext_proposal #22

Merged
merged 9 commits into from
Apr 10, 2024
95 changes: 59 additions & 36 deletions .github/workflows/interop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,39 +26,39 @@ jobs:

# ---------------------------------------------------------------------------------------

- name: MLS++ | Checkout
run: |
git clone https://github.com/cisco/mlspp.git
cd mlspp
git checkout 623acd0839d1117e8665b6bd52eecad1ce05438d

- name: MLS++ | Install dependencies | 1/2
uses: lukka/run-vcpkg@v11
with:
vcpkgDirectory: "${{ github.workspace }}/vcpkg"
vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
vcpkgJsonGlob: "mlspp/vcpkg.json"
runVcpkgInstall: true

- name: MLS++ | Install dependencies | 2/2
uses: lukka/run-vcpkg@v11
with:
vcpkgDirectory: "${{ github.workspace }}/vcpkg"
vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
vcpkgJsonGlob: "mlspp/cmd/interop/vcpkg.json"
runVcpkgInstall: true

- name: MLS++ | Build | 1/2
working-directory: mlspp
run: |
cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
make

- name: MLS++ | Build | 2/2
working-directory: mlspp/cmd/interop
run: |
cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
make
#- name: MLS++ | Checkout
# run: |
# git clone https://github.com/cisco/mlspp.git
# cd mlspp
# git checkout 623acd0839d1117e8665b6bd52eecad1ce05438d

#- name: MLS++ | Install dependencies | 1/2
# uses: lukka/run-vcpkg@v11
# with:
# vcpkgDirectory: "${{ github.workspace }}/vcpkg"
# vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
# vcpkgJsonGlob: "mlspp/vcpkg.json"
# runVcpkgInstall: true

#- name: MLS++ | Install dependencies | 2/2
# uses: lukka/run-vcpkg@v11
# with:
# vcpkgDirectory: "${{ github.workspace }}/vcpkg"
# vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
# vcpkgJsonGlob: "mlspp/cmd/interop/vcpkg.json"
# runVcpkgInstall: true

#- name: MLS++ | Build | 1/2
# working-directory: mlspp
# run: |
# cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
# make

#- name: MLS++ | Build | 2/2
# working-directory: mlspp/cmd/interop
# run: |
# cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
# make

# ---------------------------------------------------------------------------------------

Expand Down Expand Up @@ -89,10 +89,32 @@ jobs:

# ---------------------------------------------------------------------------------------

- name: Test interoperability
# - name: Test interoperability
# run: |
# ./target/debug/interop_client&
# ./mlspp/cmd/interop/mlspp_client -live 12345&
#
# cd mls-implementations/interop
# # TODO(#1238):
# # * Add `commit.json` as soon as group context extensions proposals are supported.
# # Note: It's also possible to remove GCE proposals by hand from `commit.json`.
# # But let's not do this in CI for now and hope that it isn't needed anymore soon.
# for scenario in {welcome_join.json,external_join.json,application.json};
# do
# echo Running configs/$scenario
# errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -client localhost:12345 -config=configs/$scenario | grep error | wc -l)
# if [ "$errors" = "0" ];
# then
# echo "Success";
# else
# echo "Failed";
# exit 1;
# fi
# done

- name: Test interoperability (OpenMLS only)
run: |
./target/debug/interop_client&
./mlspp/cmd/interop/mlspp_client -live 12345&

cd mls-implementations/interop
# TODO(#1238):
Expand All @@ -102,7 +124,7 @@ jobs:
for scenario in {welcome_join.json,external_join.json,application.json};
do
echo Running configs/$scenario
errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -client localhost:12345 -config=configs/$scenario | grep error | wc -l)
errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -config=configs/$scenario | grep error | wc -l)
if [ "$errors" = "0" ];
then
echo "Success";
Expand All @@ -111,3 +133,4 @@ jobs:
exit 1;
fi
done

3 changes: 2 additions & 1 deletion openmls/src/framing/mls_content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ pub(crate) fn framed_content_tbs_serialized<'context, W: Write>(
// NewMemberCommit.
written += match serialized_context.into() {
Some(context) if matches!(sender, Sender::Member(_) | Sender::NewMemberCommit) => {
writer.write(context)?
writer.write_all(context)?;
context.len()
}
_ => 0,
};
Expand Down
13 changes: 7 additions & 6 deletions openmls/src/group/core_group/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1043,17 +1043,14 @@ impl CoreGroup {
group_info: group_info.filter(|_| self.use_ratchet_tree_extension),
})
}
}

// Test functions
#[cfg(test)]
impl CoreGroup {

Choose a reason for hiding this comment

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

Thanks. I need this for some work I have coming up too

pub(crate) fn create_group_context_ext_proposal(
/// Create a new group context extension proposal
pub(crate) fn create_group_context_ext_proposal<KeyStore: OpenMlsKeyStore>(
&self,
framing_parameters: FramingParameters,
extensions: Extensions,
signer: &impl Signer,
) -> Result<AuthenticatedContent, CreateGroupContextExtProposalError> {
) -> Result<AuthenticatedContent, CreateGroupContextExtProposalError<KeyStore::Error>> {
// Ensure that the group supports all the extensions that are wanted.
let required_extension = extensions
.iter()
Expand Down Expand Up @@ -1082,7 +1079,11 @@ impl CoreGroup {
)
.map_err(|e| e.into())
}
}

// Test functions
#[cfg(test)]
impl CoreGroup {
pub(crate) fn use_ratchet_tree_extension(&self) -> bool {
self.use_ratchet_tree_extension
}
Expand Down
8 changes: 4 additions & 4 deletions openmls/src/group/core_group/test_proposals.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use openmls_rust_crypto::OpenMlsRustCrypto;
use openmls_traits::{types::Ciphersuite, OpenMlsProvider};
use openmls_traits::{key_store::OpenMlsKeyStore, types::Ciphersuite, OpenMlsProvider};

use super::CoreGroup;
use crate::{
Expand Down Expand Up @@ -543,9 +543,9 @@ fn test_group_context_extension_proposal_fails(
}

#[apply(ciphersuites_and_providers)]
fn test_group_context_extension_proposal(
fn test_group_context_extension_proposal<KeyStore: OpenMlsKeyStore>(
ciphersuite: Ciphersuite,
provider: &impl OpenMlsProvider,
provider: &impl OpenMlsProvider<KeyStoreProvider = KeyStore>,
) {
// Basic group setup.
let group_aad = b"Alice's test group";
Expand Down Expand Up @@ -617,7 +617,7 @@ fn test_group_context_extension_proposal(
&[CredentialType::Basic],
));
let gce_proposal = alice_group
.create_group_context_ext_proposal(
.create_group_context_ext_proposal::<KeyStore>(
framing_parameters,
Extensions::single(required_application_id),
&alice_signer,
Expand Down
8 changes: 7 additions & 1 deletion openmls/src/group/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ pub(crate) enum CoreGroupParseMessageError {

/// Create group context ext proposal error
#[derive(Error, Debug, PartialEq, Clone)]
pub enum CreateGroupContextExtProposalError {
pub enum CreateGroupContextExtProposalError<KeyStoreError> {
/// See [`LibraryError`] for more details.
#[error(transparent)]
LibraryError(#[from] LibraryError),
Expand All @@ -507,6 +507,12 @@ pub enum CreateGroupContextExtProposalError {
/// See [`LeafNodeValidationError`] for more details.
#[error(transparent)]
LeafNodeValidation(#[from] LeafNodeValidationError),
/// See [`MlsGroupStateError`] for more details.
#[error(transparent)]
MlsGroupStateError(#[from] MlsGroupStateError),
/// See [`CreateCommitError`] for more details.
#[error(transparent)]
CreateCommitError(#[from] CreateCommitError<KeyStoreError>),
}

/// Error merging a commit.
Expand Down
2 changes: 1 addition & 1 deletion openmls/src/group/mls_group/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,5 +322,5 @@ pub enum ProposalError<KeyStoreError> {
ValidationError(#[from] ValidationError),
/// See [`CreateGroupContextExtProposalError`] for more details.
#[error(transparent)]
CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError),
CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError<KeyStoreError>),
}
64 changes: 57 additions & 7 deletions openmls/src/group/mls_group/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ use openmls_traits::{
};

use super::{
core_group::create_commit_params::CreateCommitParams,
errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError},
MlsGroup,
CreateGroupContextExtProposalError, GroupContextExtensionProposal, MlsGroup, MlsGroupState,
PendingCommitState, Proposal,
};
use crate::{
binary_tree::LeafNodeIndex,
Expand All @@ -14,7 +16,7 @@ use crate::{
framing::MlsMessageOut,
group::{errors::CreateAddProposalError, GroupId, QueuedProposal},
key_packages::KeyPackage,
messages::proposals::ProposalOrRefType,
messages::{group_info::GroupInfo, proposals::ProposalOrRefType},
prelude::LibraryError,
schedule::PreSharedKeyId,
treesync::LeafNode,
Expand Down Expand Up @@ -319,16 +321,19 @@ impl MlsGroup {
}
}

#[cfg(test)]
pub fn propose_group_context_extensions(
/// Creates a proposals with a new set of `extensions` for the group context.
///
/// Returns an error when the group does not support all the required capabilities
/// in the new `extensions`.
pub fn propose_group_context_extensions<KeyStore: OpenMlsKeyStore>(
&mut self,
provider: &impl OpenMlsProvider,
provider: &impl OpenMlsProvider<KeyStoreProvider = KeyStore>,
extensions: Extensions,
signer: &impl Signer,
) -> Result<(MlsMessageOut, ProposalRef), ProposalError<()>> {
) -> Result<(MlsMessageOut, ProposalRef), ProposalError<KeyStore::Error>> {
self.is_operational()?;

let proposal = self.group.create_group_context_ext_proposal(
let proposal = self.group.create_group_context_ext_proposal::<KeyStore>(
self.framing_parameters(),
extensions,
signer,
Expand All @@ -350,4 +355,49 @@ impl MlsGroup {

Ok((mls_message, proposal_ref))
}

/// Updates group context extensions
///
/// Returns an error when the group does not support all the required capabilities
/// in the new `extensions`.
#[allow(clippy::type_complexity)]
pub fn update_group_context_extensions<KeyStore: OpenMlsKeyStore>(
&mut self,
provider: &impl OpenMlsProvider<KeyStoreProvider = KeyStore>,
extensions: Extensions,
signer: &impl Signer,
) -> Result<
(MlsMessageOut, Option<MlsMessageOut>, Option<GroupInfo>),
CreateGroupContextExtProposalError<KeyStore::Error>,
> {
self.is_operational()?;

// Create group context extension proposals
let inline_proposals = vec![Proposal::GroupContextExtensions(
GroupContextExtensionProposal { extensions },
)];

let params = CreateCommitParams::builder()
.framing_parameters(self.framing_parameters())
.proposal_store(&self.proposal_store)
.inline_proposals(inline_proposals)
.build();
let create_commit_result = self.group.create_commit(params, provider, signer)?;

let mls_messages = self.content_to_mls_message(create_commit_result.commit, provider)?;
self.group_state = MlsGroupState::PendingCommit(Box::new(PendingCommitState::Member(
create_commit_result.staged_commit,
)));

// Since the state of the group might be changed, arm the state flag
self.flag_state_change();

Ok((
mls_messages,
create_commit_result
.welcome_option
.map(|w| MlsMessageOut::from_welcome(w, self.group.version())),
create_commit_result.group_info,
))
}
}
Loading
Loading