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

Pull upstream for stable storage api #32

Merged
merged 13 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions .github/workflows/interop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,16 @@ jobs:
run: |
git clone https://github.com/mlswg/mls-implementations.git
cd mls-implementations
git checkout f07090a844ebece12c064ce94ab853fd477db12f
git checkout 8a6ee96bc732abca77d872babf1830ccfec7fa49
- name: test-runner | Install dependencies
run: |
sudo apt-get -y install protoc-gen-go
echo $(go env GOPATH)/bin >> $GITHUB_PATH
go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
cd mls-implementations
go get -u google.golang.org/grpc
- name: test-runner | Build
run: |
Expand All @@ -84,7 +86,6 @@ jobs:
make run-go || echo "Build despite errors."
cd test-runner
# TODO(#1366)
go get -u google.golang.org/grpc
go mod tidy -e
patch main.go main.go.patch
go build
Expand Down
8 changes: 0 additions & 8 deletions memory_storage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,14 +867,6 @@ impl StorageProvider<CURRENT_VERSION> for MemoryStorage {
self.append::<CURRENT_VERSION>(OWN_LEAF_NODES_LABEL, &key, value)
}

fn clear_own_leaf_nodes<GroupId: traits::GroupId<CURRENT_VERSION>>(
&self,
group_id: &GroupId,
) -> Result<(), Self::Error> {
let key = serde_json::to_vec(group_id)?;
self.delete::<CURRENT_VERSION>(OWN_LEAF_NODES_LABEL, &key)
}

fn aad<GroupId: traits::GroupId<CURRENT_VERSION>>(
&self,
group_id: &GroupId,
Expand Down
7 changes: 0 additions & 7 deletions memory_storage/src/test_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,6 @@ impl StorageProvider<V_TEST> for MemoryStorage {
todo!()
}

fn clear_own_leaf_nodes<GroupId: traits::GroupId<V_TEST>>(
&self,
_group_id: &GroupId,
) -> Result<(), Self::Error> {
todo!()
}

fn aad<GroupId: traits::GroupId<V_TEST>>(
&self,
_group_id: &GroupId,
Expand Down
2 changes: 2 additions & 0 deletions openmls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ openmls_memory_storage = { path = "../memory_storage", features = [
], optional = true }
openmls_test = { path = "../openmls_test", optional = true }
openmls_libcrux_crypto = { path = "../libcrux_crypto", optional = true }
once_cell = { version = "1.19.0", optional = true }

[features]
default = ["backtrace"]
Expand All @@ -49,6 +50,7 @@ test-utils = [
"dep:openmls_basic_credential",
"dep:openmls_memory_storage",
"dep:openmls_test",
"dep:once_cell",
]
libcrux-provider = [
"dep:openmls_libcrux_crypto",
Expand Down
2 changes: 1 addition & 1 deletion openmls/src/framing/mls_auth_content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) struct FramedContentAuthData {
}

impl FramedContentAuthData {
pub(super) fn deserialize<R: Read>(
pub(crate) fn deserialize<R: Read>(
bytes: &mut R,
content_type: ContentType,
) -> Result<Self, tls_codec::Error> {
Expand Down
9 changes: 7 additions & 2 deletions openmls/src/group/core_group/new_from_welcome.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,12 @@ pub(in crate::group) fn build_staged_welcome<Provider: OpenMlsProvider>(
log_crypto!(trace, " Got: {:x?}", confirmation_tag);
log_crypto!(trace, " Expected: {:x?}", public_group.confirmation_tag());
debug_assert!(false, "Confirmation tag mismatch");
return Err(WelcomeError::ConfirmationTagMismatch);

// in some tests we need to be able to proceed despite the tag being wrong,
// e.g. to test whether a later validation check is performed correctly.
if !crate::skip_validation::is_disabled::confirmation_tag() {
return Err(WelcomeError::ConfirmationTagMismatch);
}
}

let message_secrets_store = MessageSecretsStore::new_with_secret(0, message_secrets);
Expand Down Expand Up @@ -228,7 +233,7 @@ pub(in crate::group) fn build_staged_welcome<Provider: OpenMlsProvider>(
Ok(group)
}

/// Process a Welcome message up to the point where the ratchet tree is is required.
/// Process a Welcome message up to the point where the ratchet tree is required.
pub(in crate::group) fn process_welcome<Provider: OpenMlsProvider>(
welcome: Welcome,
key_package_bundle: &KeyPackageBundle,
Expand Down
7 changes: 6 additions & 1 deletion openmls/src/group/core_group/staged_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,12 @@ impl CoreGroup {
// TODO: We have tests expecting this error.
// They need to be rewritten.
// debug_assert!(false, "Confirmation tag mismatch");
return Err(StageCommitError::ConfirmationTagMismatch);

// in some tests we need to be able to proceed despite the tag being wrong,
// e.g. to test whether a later validation check is performed correctly.
if !crate::skip_validation::is_disabled::confirmation_tag() {
return Err(StageCommitError::ConfirmationTagMismatch);
}
}

diff.update_interim_transcript_hash(ciphersuite, provider.crypto(), own_confirmation_tag)?;
Expand Down
4 changes: 2 additions & 2 deletions openmls/src/group/mls_group/creation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ fn transpose_err_opt<T, E>(v: Result<Option<T>, E>) -> Option<Result<T, E>> {
}

impl ProcessedWelcome {
/// Creates a new processed [`Welcome`] message that can be used to parse
/// it before creating a [`StagedWelcome`].
/// Creates a new processed [`Welcome`] message , which can be
/// inspected before creating a [`StagedWelcome`].
///
/// This does not require a ratchet tree yet.
///
Expand Down
6 changes: 3 additions & 3 deletions openmls/src/group/mls_group/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,12 +495,12 @@ pub struct StagedWelcome {
group: StagedCoreWelcome,
}

/// A parsed, but not fully processed `Welcome` message.
/// A `Welcome` message that has been processed but not staged yet.
///
/// This may be used in order to retrieve information from the `Welcome` about
/// the ratchet tree.
/// the ratchet tree and PSKs.
///
/// Use `into_staged_welcome` to get the [`StagedWelcome`] on this.
/// Use `into_staged_welcome` to stage it into a [`StagedWelcome`].
pub struct ProcessedWelcome {
// The group configuration. See [`MlsGroupJoinConfig`] for more information.
mls_group_config: MlsGroupJoinConfig,
Expand Down
2 changes: 1 addition & 1 deletion openmls/src/group/mls_group/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl MlsGroup {
self.own_leaf_nodes.clear();
provider
.storage()
.clear_own_leaf_nodes(self.group_id())
.delete_own_leaf_nodes(self.group_id())
.map_err(MergeCommitError::StorageError)?;

// Delete a potential pending commit
Expand Down
161 changes: 143 additions & 18 deletions openmls/src/group/mls_group/test_mls_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ use crate::{
key_packages::*,
messages::proposals::*,
prelude::Capabilities,
test_utils::test_framework::{
errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse,
MlsGroupTestSetup,
test_utils::{
frankenstein::{self, FrankenMlsMessage},
test_framework::{
errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse,
MlsGroupTestSetup,
},
},
tree::sender_ratchet::SenderRatchetConfiguration,
};
Expand Down Expand Up @@ -1143,15 +1146,57 @@ fn remove_prosposal_by_ref(
// Test that the builder pattern accurately configures the new group.
#[openmls_test]
fn group_context_extensions_proposal() {
let alice_provider = &mut Provider::default();
let bob_provider = &mut Provider::default();
let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) =
setup_client("Alice", ciphersuite, provider);
setup_client("Alice", ciphersuite, alice_provider);
let (bob_credential_with_key, _bob_kpb, bob_signer, _bob_pk) =
setup_client("bob", ciphersuite, bob_provider);

// === Alice creates a group ===
let mut alice_group = MlsGroup::builder()
.ciphersuite(ciphersuite)
.build(provider, &alice_signer, alice_credential_with_key)
.with_wire_format_policy(WireFormatPolicy::new(
OutgoingWireFormatPolicy::AlwaysPlaintext,
IncomingWireFormatPolicy::Mixed,
))
.build(alice_provider, &alice_signer, alice_credential_with_key)
.expect("error creating group using builder");

// === Alice adds Bob ===
let bob_key_package = KeyPackage::builder()
.build(
ciphersuite,
bob_provider,
&bob_signer,
bob_credential_with_key,
)
.expect("error building key package");

let (_, welcome, _) = alice_group
.add_members(
alice_provider,
&alice_signer,
&[bob_key_package.key_package().clone()],
)
.unwrap();
alice_group.merge_pending_commit(alice_provider).unwrap();

let welcome: MlsMessageIn = welcome.into();
let welcome = welcome
.into_welcome()
.expect("expected message to be a welcome");

let mut bob_group = StagedWelcome::new_from_welcome(
bob_provider,
alice_group.configuration(),
welcome,
Some(alice_group.export_ratchet_tree().into()),
)
.expect("Error creating staged join from Welcome")
.into_group(bob_provider)
.expect("Error creating group from staged join");

// No required capabilities, so no specifically required extensions.
assert!(alice_group
.group()
Expand All @@ -1168,20 +1213,104 @@ fn group_context_extensions_proposal() {
RequiredCapabilitiesExtension::new(&[ExtensionType::RatchetTree], &[], &[]),
));

alice_group
.propose_group_context_extensions(provider, new_extensions.clone(), &alice_signer)
let (proposal, _) = alice_group
.propose_group_context_extensions(alice_provider, new_extensions.clone(), &alice_signer)
.expect("failed to build group context extensions proposal");

let proc_msg = bob_group
.process_message(bob_provider, proposal.into_protocol_message().unwrap())
.unwrap();
match proc_msg.into_content() {
ProcessedMessageContent::ProposalMessage(proposal) => bob_group
.store_pending_proposal(bob_provider.storage(), *proposal)
.unwrap(),
_ => unreachable!(),
};

assert_eq!(alice_group.pending_proposals().count(), 1);

alice_group
.commit_to_pending_proposals(provider, &alice_signer)
let (commit, _, _) = alice_group
.commit_to_pending_proposals(alice_provider, &alice_signer)
.expect("failed to commit to pending proposals");

// we'll change the commit we feed to bob to include two GCE proposals
let mut franken_commit = FrankenMlsMessage::tls_deserialize(
&mut commit.tls_serialize_detached().unwrap().as_slice(),
)
.unwrap();

// Craft a commit that has two GroupContextExtension proposals. This is forbidden by the RFC.
// Change the commit before alice commits, so alice's state is still in the old epoch and we can
// use her state to forge the macs and signatures
match &mut franken_commit.body {
frankenstein::FrankenMlsMessageBody::PublicMessage(msg) => {
match &mut msg.content.body {
frankenstein::FrankenFramedContentBody::Commit(commit) => {
let second_gces = frankenstein::FrankenProposalOrRef::Proposal(
frankenstein::FrankenProposal::GroupContextExtensions(vec![
frankenstein::FrankenExtension::LastResort,
]),
);

commit.proposals.push(second_gces);
}
_ => unreachable!(),
}

let group_context = alice_group.export_group_context().clone();

let bob_group_context = bob_group.export_group_context();
assert_eq!(
bob_group_context.confirmed_transcript_hash(),
group_context.confirmed_transcript_hash()
);

let secrets = alice_group.group.message_secrets();
let membership_key = secrets.membership_key().as_slice();

*msg = frankenstein::FrankenPublicMessage::auth(
alice_provider,
group_context.ciphersuite(),
&alice_signer,
msg.content.clone(),
Some(&group_context.into()),
Some(membership_key),
// this is a dummy confirmation_tag:
Some(vec![0u8; 32].into()),
);
}
_ => unreachable!(),
}

// alice merges the unmodified commit
alice_group
.merge_pending_commit(provider)
.merge_pending_commit(alice_provider)
.expect("error merging pending commit");

let fake_commit = MlsMessageIn::tls_deserialize(
&mut franken_commit.tls_serialize_detached().unwrap().as_slice(),
)
.unwrap();

let fake_commit_protocol_msg = fake_commit.into_protocol_message().unwrap();

let err = {
let validation_skip_handle = crate::skip_validation::checks::confirmation_tag::handle();
validation_skip_handle.with_disabled(|| {
bob_group.process_message(bob_provider, fake_commit_protocol_msg.clone())
})
}
.expect_err("expected an error");

assert!(matches!(
err,
ProcessMessageError::InvalidCommit(
StageCommitError::GroupContextExtensionsProposalValidationError(
GroupContextExtensionsProposalValidationError::TooManyGCEProposals
)
)
));

let required_capabilities = alice_group
.group()
.context()
Expand All @@ -1195,18 +1324,18 @@ fn group_context_extensions_proposal() {
// === committing to two group context extensions should fail

alice_group
.propose_group_context_extensions(provider, new_extensions, &alice_signer)
.propose_group_context_extensions(alice_provider, new_extensions, &alice_signer)
.expect("failed to build group context extensions proposal");

// the proposals need to be different or they will be deduplicated
alice_group
.propose_group_context_extensions(provider, new_extensions_2, &alice_signer)
.propose_group_context_extensions(alice_provider, new_extensions_2, &alice_signer)
.expect("failed to build group context extensions proposal");

assert_eq!(alice_group.pending_proposals().count(), 2);

alice_group
.commit_to_pending_proposals(provider, &alice_signer)
.commit_to_pending_proposals(alice_provider, &alice_signer)
.expect_err(
"expected error when committing to multiple group context extensions proposals",
);
Expand All @@ -1220,12 +1349,8 @@ fn group_context_extensions_proposal() {
));

alice_group
.propose_group_context_extensions(provider, new_extensions, &alice_signer)
.propose_group_context_extensions(alice_provider, new_extensions, &alice_signer)
.expect_err("expected an error building GCE proposal with bad required_capabilities");

// TODO: we need to test that processing a commit with multiple group context extensions
// proposal also fails. however, we can't generate this commit, because our functions for
// constructing commits does not permit it. See #1476
}

// Test that the builder pattern accurately configures the new group.
Expand Down
1 change: 1 addition & 0 deletions openmls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub mod storage;

// Private
mod binary_tree;
mod skip_validation;
mod tree;

/// Single place, re-exporting the most used public functions.
Expand Down
Loading
Loading