diff --git a/.github/workflows/interop.yml b/.github/workflows/interop.yml deleted file mode 100644 index 33bba0f8d4..0000000000 --- a/.github/workflows/interop.yml +++ /dev/null @@ -1,113 +0,0 @@ -name: Test interop between OpenMLS & MLS++ - -on: - push: - branches: - - main - pull_request: - workflow_dispatch: - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - interop: - runs-on: ubuntu-latest - steps: - - name: OpenMLS | Checkout - uses: actions/checkout@v3 - - - name: OpenMLS | Install dependencies - run: sudo apt-get -y install protobuf-compiler - - - name: OpenMLS | Build - run: cargo build -p interop_client - - # --------------------------------------------------------------------------------------- - - - 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: test-runner | Checkout - run: | - git clone https://github.com/mlswg/mls-implementations.git - cd mls-implementations - git checkout f07090a844ebece12c064ce94ab853fd477db12f - - - 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 - - - name: test-runner | Build - run: | - # TODO(#1366) - cp interop_client/docker/test-runner/main.go.patch mls-implementations/interop/test-runner - cd mls-implementations/interop - go mod tidy -e - make run-go || echo "Build despite errors." - cd test-runner - # TODO(#1366) - patch main.go main.go.patch - go build - - # --------------------------------------------------------------------------------------- - - - 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 diff --git a/openmls/Cargo.toml b/openmls/Cargo.toml index 3c8cba95a0..2552441554 100644 --- a/openmls/Cargo.toml +++ b/openmls/Cargo.toml @@ -20,7 +20,7 @@ hex = "0.4" async-trait = { workspace = true } openmls_basic_credential = { version = "0.2.0", path = "../basic_credential", features = ["clonable", "test-utils"] } openmls_x509_credential = { version = "0.2.0", path = "../x509_credential" } -x509-cert = "0.2" +x509-cert = "0.3" subtle = "2.5" web-time = "1.1.0" indexmap = "2.0" diff --git a/openmls/src/binary_tree/array_representation/treemath.rs b/openmls/src/binary_tree/array_representation/treemath.rs index a706255c68..4f234f8d66 100644 --- a/openmls/src/binary_tree/array_representation/treemath.rs +++ b/openmls/src/binary_tree/array_representation/treemath.rs @@ -59,7 +59,7 @@ impl LeafNodeIndex { /// Warning: Only use when the node index represents a leaf node pub fn from_tree_index(node_index: u32) -> Self { - debug_assert!(node_index % 2 == 0); + debug_assert!(node_index.is_multiple_of(2)); LeafNodeIndex(node_index / 2) } } @@ -140,7 +140,7 @@ pub enum TreeNodeIndex { impl TreeNodeIndex { /// Create a new `TreeNodeIndex` from a `u32`. fn new(index: u32) -> Self { - if index % 2 == 0 { + if index.is_multiple_of(2) { TreeNodeIndex::Leaf(LeafNodeIndex::from_tree_index(index)) } else { TreeNodeIndex::Parent(ParentNodeIndex::from_tree_index(index)) diff --git a/openmls/src/ciphersuite/secret.rs b/openmls/src/ciphersuite/secret.rs index ce6a910166..0004137d52 100644 --- a/openmls/src/ciphersuite/secret.rs +++ b/openmls/src/ciphersuite/secret.rs @@ -184,7 +184,7 @@ impl Secret { let full_label = format!("{} {}", self.mls_version, label); log::trace!( "KDF expand with label \"{}\" and {:?} with context {:x?}", - &full_label, + full_label, self.ciphersuite, context ); diff --git a/openmls/src/framing/test_framing.rs b/openmls/src/framing/test_framing.rs index 4bad37dda9..3cc24c0a51 100644 --- a/openmls/src/framing/test_framing.rs +++ b/openmls/src/framing/test_framing.rs @@ -617,7 +617,7 @@ pub(crate) async fn setup_alice_bob_group( ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvider, ) -> ( - FramingParameters, + FramingParameters<'_>, CoreGroup, SignatureKeyPair, CoreGroup, diff --git a/openmls/src/group/core_group/create_commit_params.rs b/openmls/src/group/core_group/create_commit_params.rs index eb27cddd59..7669d3ff5f 100644 --- a/openmls/src/group/core_group/create_commit_params.rs +++ b/openmls/src/group/core_group/create_commit_params.rs @@ -93,7 +93,7 @@ impl CreateCommitParams<'_> { pub(crate) fn builder() -> TempBuilderCCPM0 { TempBuilderCCPM0 {} } - pub(crate) fn framing_parameters(&self) -> &FramingParameters { + pub(crate) fn framing_parameters(&self) -> &FramingParameters<'_> { &self.framing_parameters } pub(crate) fn proposal_store(&self) -> &ProposalStore { diff --git a/openmls/src/group/core_group/proposals.rs b/openmls/src/group/core_group/proposals.rs index f2b1f1f932..a089ccdd8f 100644 --- a/openmls/src/group/core_group/proposals.rs +++ b/openmls/src/group/core_group/proposals.rs @@ -330,7 +330,7 @@ impl ProposalQueue { /// Returns an iterator over all Add proposals in the queue /// in the order of the the Commit message - pub fn add_proposals(&self) -> impl Iterator { + pub fn add_proposals(&self) -> impl Iterator> { self.queued_proposals().filter_map(|queued_proposal| { if let Proposal::Add(add_proposal) = queued_proposal.proposal() { let sender = queued_proposal.sender(); @@ -346,7 +346,7 @@ impl ProposalQueue { /// Returns an iterator over all Remove proposals in the queue /// in the order of the the Commit message - pub fn remove_proposals(&self) -> impl Iterator { + pub fn remove_proposals(&self) -> impl Iterator> { self.queued_proposals().filter_map(|queued_proposal| { if let Proposal::Remove(remove_proposal) = queued_proposal.proposal() { let sender = queued_proposal.sender(); @@ -362,7 +362,7 @@ impl ProposalQueue { /// Returns an iterator over all Update in the queue /// in the order of the the Commit message - pub fn update_proposals(&self) -> impl Iterator { + pub fn update_proposals(&self) -> impl Iterator> { self.queued_proposals().filter_map(|queued_proposal| { if let Proposal::Update(update_proposal) = queued_proposal.proposal() { let sender = queued_proposal.sender(); @@ -378,7 +378,7 @@ impl ProposalQueue { /// Returns an iterator over all PresharedKey proposals in the queue /// in the order of the the Commit message - pub fn psk_proposals(&self) -> impl Iterator { + pub fn psk_proposals(&self) -> impl Iterator> { self.queued_proposals().filter_map(|queued_proposal| { if let Proposal::PreSharedKey(psk_proposal) = queued_proposal.proposal() { let sender = queued_proposal.sender(); @@ -540,7 +540,7 @@ impl ProposalQueue { } } // Check for presence of Removes and delete Updates - for (_, member) in members.iter_mut() { + for member in members.values_mut() { // Check if there are Removes if let Some(last_remove) = member.removes.last() { // Delete all Updates when a Remove is found diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/core_group/staged_commit.rs index df214dc00a..f7318a54d9 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/core_group/staged_commit.rs @@ -347,7 +347,7 @@ impl CoreGroup { let epoch_keypairs: EpochEncryptionKeyPair = old_epoch_keypairs .0 .into_iter() - .chain(state.new_keypairs.into_iter()) + .chain(state.new_keypairs) .filter(|keypair| new_owned_encryption_keys.contains(keypair.public_key())) .collect::>() .into(); @@ -433,22 +433,22 @@ impl StagedCommit { } /// Returns the Add proposals that are covered by the Commit message as in iterator over [QueuedAddProposal]. - pub fn add_proposals(&self) -> impl Iterator { + pub fn add_proposals(&self) -> impl Iterator> { self.staged_proposal_queue.add_proposals() } /// Returns the Remove proposals that are covered by the Commit message as in iterator over [QueuedRemoveProposal]. - pub fn remove_proposals(&self) -> impl Iterator { + pub fn remove_proposals(&self) -> impl Iterator> { self.staged_proposal_queue.remove_proposals() } /// Returns the Update proposals that are covered by the Commit message as in iterator over [QueuedUpdateProposal]. - pub fn update_proposals(&self) -> impl Iterator { + pub fn update_proposals(&self) -> impl Iterator> { self.staged_proposal_queue.update_proposals() } /// Returns the PresharedKey proposals that are covered by the Commit message as in iterator over [QueuedPskProposal]. - pub fn psk_proposals(&self) -> impl Iterator { + pub fn psk_proposals(&self) -> impl Iterator> { self.staged_proposal_queue.psk_proposals() } diff --git a/openmls/src/group/mls_group/mod.rs b/openmls/src/group/mls_group/mod.rs index 57e13b6d33..6d8351e6e8 100644 --- a/openmls/src/group/mls_group/mod.rs +++ b/openmls/src/group/mls_group/mod.rs @@ -397,7 +397,7 @@ impl MlsGroup { } /// Group framing parameters - pub(crate) fn framing_parameters(&self) -> FramingParameters { + pub(crate) fn framing_parameters(&self) -> FramingParameters<'_> { FramingParameters::new( &self.aad, self.mls_group_config.wire_format_policy().outgoing(), @@ -465,7 +465,7 @@ impl MlsGroup { } /// `Enum` that indicates whether the inner group state has been modified since the last time it was persisted. -/// `InnerState::Changed` indicates that the state has changed and that [`.save()`] should be called. +/// `InnerState::Changed` indicates that the state has changed and that `MlsGroup.save` should be called. /// `InnerState::Persisted` indicates that the state has not been modified and therefore doesn't need to be persisted. #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum InnerState { diff --git a/openmls/src/group/public_group/mod.rs b/openmls/src/group/public_group/mod.rs index 02160922dd..69ca466b43 100644 --- a/openmls/src/group/public_group/mod.rs +++ b/openmls/src/group/public_group/mod.rs @@ -212,7 +212,7 @@ impl PublicGroup { } /// Create an empty [`PublicGroupDiff`] based on this [`PublicGroup`]. - pub(crate) fn empty_diff(&self) -> PublicGroupDiff { + pub(crate) fn empty_diff(&self) -> PublicGroupDiff<'_> { PublicGroupDiff::new(self) } diff --git a/openmls/src/key_packages/mod.rs b/openmls/src/key_packages/mod.rs index e860b49638..c6d9ac4956 100644 --- a/openmls/src/key_packages/mod.rs +++ b/openmls/src/key_packages/mod.rs @@ -707,6 +707,7 @@ impl KeyPackageBuilder { /// A [`KeyPackageBundle`] contains a [`KeyPackage`] and the corresponding private /// key. +#[cfg(test)] #[derive(Debug, Clone, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] pub(crate) struct KeyPackageBundle { diff --git a/openmls/src/test_utils/test_framework/mod.rs b/openmls/src/test_utils/test_framework/mod.rs index fafe0bc304..255bb4e39e 100644 --- a/openmls/src/test_utils/test_framework/mod.rs +++ b/openmls/src/test_utils/test_framework/mod.rs @@ -43,7 +43,7 @@ use openmls_traits::{ OpenMlsCryptoProvider, }; use std::collections::HashMap; -use tls_codec::*; +use tls_codec::{Deserialize as _, Serialize as _}; pub mod client; pub mod errors; diff --git a/openmls/src/tree/tests_and_kats/kats/secret_tree.rs b/openmls/src/tree/tests_and_kats/kats/secret_tree.rs index 6b1eec26d8..d2d0863e14 100644 --- a/openmls/src/tree/tests_and_kats/kats/secret_tree.rs +++ b/openmls/src/tree/tests_and_kats/kats/secret_tree.rs @@ -70,6 +70,9 @@ pub struct Leaf { handshake_nonce: String, } +// clippy complains this type is never constructed, but that is not technically true since +// construction happens implicitly via deserialization, which clippy is unaware of +#[allow(unused)] #[derive(Serialize, Deserialize, Debug, Clone)] pub struct SecretTree { cipher_suite: u16, diff --git a/openmls/src/treesync/diff.rs b/openmls/src/treesync/diff.rs index e7ce6b7a48..d8883b6f67 100644 --- a/openmls/src/treesync/diff.rs +++ b/openmls/src/treesync/diff.rs @@ -459,7 +459,7 @@ impl TreeSyncDiff<'_> { &self, node_index: TreeNodeIndex, excluded_indices: &HashSet<&LeafNodeIndex>, - ) -> Vec<(TreeNodeIndex, NodeReference)> { + ) -> Vec<(TreeNodeIndex, NodeReference<'_>)> { match node_index { TreeNodeIndex::Leaf(leaf_index) => { // If the node is a leaf, check if it is in the exclusion list. @@ -527,7 +527,7 @@ impl TreeSyncDiff<'_> { pub(crate) fn copath_resolutions( &self, leaf_index: LeafNodeIndex, - ) -> Vec> { + ) -> Vec)>> { // If we're the only node in the tree, there's no copath. if self.diff.leaf_count() == MIN_TREE_SIZE { return vec![]; @@ -548,7 +548,7 @@ impl TreeSyncDiff<'_> { &self, leaf_index: LeafNodeIndex, exclusion_list: &HashSet<&LeafNodeIndex>, - ) -> Vec> { + ) -> Vec)>> { // If we're the only node in the tree, there's no copath. if self.diff.leaf_count() == 1 { return vec![]; diff --git a/openmls/src/treesync/mod.rs b/openmls/src/treesync/mod.rs index 18a983c5b3..3722d4b440 100644 --- a/openmls/src/treesync/mod.rs +++ b/openmls/src/treesync/mod.rs @@ -433,7 +433,7 @@ impl TreeSync { /// Create an empty diff based on this [`TreeSync`] instance all operations /// are created based on an initial, empty [`TreeSyncDiff`]. - pub(crate) fn empty_diff(&self) -> TreeSyncDiff { + pub(crate) fn empty_diff(&self) -> TreeSyncDiff<'_> { self.into() } diff --git a/x509_credential/Cargo.toml b/x509_credential/Cargo.toml index 5b1f758945..a5c8d52d47 100644 --- a/x509_credential/Cargo.toml +++ b/x509_credential/Cargo.toml @@ -19,5 +19,5 @@ percent-encoding = "2.3" # Rust Crypto secrecy = { version = "0.8", features = ["serde"] } -x509-cert = "0.2" -const-oid = "0.9" +x509-cert = "0.3" +const-oid = "0.10" diff --git a/x509_credential/src/lib.rs b/x509_credential/src/lib.rs index b87aecb584..cbfb6e78eb 100644 --- a/x509_credential/src/lib.rs +++ b/x509_credential/src/lib.rs @@ -117,24 +117,20 @@ impl X509Ext for Certificate { fn is_time_valid(&self) -> Result { // 'not_before' < now < 'not_after' - let x509_cert::time::Validity { - not_before, - not_after, - } = self.tbs_certificate.validity; - + let validity = self.tbs_certificate().validity(); let now = web_time::SystemTime::now(); let now = now .duration_since(web_time::UNIX_EPOCH) .map_err(|_| CryptoError::TimeError)?; - let is_nbf = now > not_before.to_unix_duration(); - let is_naf = now < not_after.to_unix_duration(); + let is_nbf = now > validity.not_before.to_unix_duration(); + let is_naf = now < validity.not_after.to_unix_duration(); Ok(is_nbf && is_naf) } fn public_key(&self) -> Result<&[u8], CryptoError> { - self.tbs_certificate - .subject_public_key_info + self.tbs_certificate() + .subject_public_key_info() .subject_public_key .as_bytes() .ok_or(CryptoError::IncompleteCertificate("spki")) @@ -145,14 +141,12 @@ impl X509Ext for Certificate { rfc5912::{ID_EC_PUBLIC_KEY, SECP_256_R_1, SECP_384_R_1, SECP_521_R_1}, rfc8410::{ID_ED_25519, ID_ED_448}, }; - let alg = self.tbs_certificate.subject_public_key_info.algorithm.oid; - let params = self - .tbs_certificate - .subject_public_key_info - .algorithm - .parameters - .as_ref() - .and_then(|param| x509_cert::spki::ObjectIdentifier::from_bytes(param.value()).ok()); + let spki = self.tbs_certificate().subject_public_key_info(); + let alg = spki.algorithm.oid; + let params = + spki.algorithm.parameters.as_ref().and_then(|param| { + x509_cert::spki::ObjectIdentifier::from_bytes(param.value()).ok() + }); let scheme = match (alg, params) { (ID_ED_25519, None) => SignatureScheme::ED25519, @@ -173,13 +167,13 @@ impl X509Ext for Certificate { ) -> Result<(), CryptoError> { let issuer_pk = issuer.public_key()?; let cert_signature = self - .signature + .signature() .as_bytes() .ok_or(CryptoError::InvalidCertificate)?; use x509_cert::der::Encode as _; let mut raw_tbs: Vec = vec![]; - self.tbs_certificate + self.tbs_certificate() .encode(&mut raw_tbs) .map_err(|_| CryptoError::CertificateEncodingError)?; let sc = issuer.signature_scheme()?; @@ -189,11 +183,8 @@ impl X509Ext for Certificate { } fn identity(&self) -> Result, CryptoError> { - let extensions = self - .tbs_certificate - .extensions - .as_ref() - .ok_or(CryptoError::InvalidCertificate)?; + let extensions = self.tbs_certificate().extensions(); + let extensions = extensions.as_ref().ok_or(CryptoError::InvalidCertificate)?; let san = extensions .iter() .find(|e| e.extn_id == const_oid::db::rfc5280::ID_CE_SUBJECT_ALT_NAME)