diff --git a/radicle-node/src/test/environment.rs b/radicle-node/src/test/environment.rs index 93814f8b..8fdeae21 100644 --- a/radicle-node/src/test/environment.rs +++ b/radicle-node/src/test/environment.rs @@ -22,7 +22,7 @@ use radicle::node::Handle as _; use radicle::profile::Home; use radicle::profile::Profile; use radicle::rad; -use radicle::storage::ReadStorage as _; +use radicle::storage::{ReadStorage as _, WriteRepository}; use radicle::test::fixtures; use radicle::Storage; @@ -366,6 +366,12 @@ impl + Signer + Clone> Node { } git::push(repo, "rad", refs.iter().map(|(a, b)| (a, b))).unwrap(); + self.storage + .repository(id) + .unwrap() + .sign_refs(&self.signer) + .unwrap(); + id } diff --git a/radicle-node/src/test/simulator.rs b/radicle-node/src/test/simulator.rs index 60b4166a..3b283ad6 100644 --- a/radicle-node/src/test/simulator.rs +++ b/radicle-node/src/test/simulator.rs @@ -699,7 +699,7 @@ fn fetch( remote.fetch(&[refspec], Some(&mut opts), None)?; drop(opts); - repo.verify()?; + repo.validate()?; repo.set_head()?; Ok(updates) diff --git a/radicle-node/src/tests/e2e.rs b/radicle-node/src/tests/e2e.rs index 2d142904..a1a2b965 100644 --- a/radicle-node/src/tests/e2e.rs +++ b/radicle-node/src/tests/e2e.rs @@ -1,8 +1,9 @@ use std::{collections::HashSet, thread, time}; use radicle::crypto::{test::signer::MockSigner, Signer}; +use radicle::git; use radicle::node::{FetchResult, Handle as _}; -use radicle::storage::{ReadRepository, ReadStorage}; +use radicle::storage::{ReadRepository, ReadStorage, WriteRepository, WriteStorage}; use radicle::test::fixtures; use radicle::{assert_matches, rad}; @@ -194,7 +195,90 @@ fn test_replication() { assert_eq!(inventory.first(), Some(&acme)); assert_eq!(alice_refs, bob_refs); - assert_matches!(alice.storage.repository(acme).unwrap().verify(), Ok(())); + assert_matches!(alice.storage.repository(acme).unwrap().validate(), Ok(())); +} + +#[test] +fn test_replication_no_delegates() { + logger::init(log::Level::Debug); + + let tmp = tempfile::tempdir().unwrap(); + let alice = Node::init(tmp.path()); + let mut bob = Node::init(tmp.path()); + let (repo, _) = fixtures::repository(tmp.path().join("acme")); + + let acme = bob.project_from("acme", "", &repo); + + // Populate repo, but don't sign the refs. + let branches = fixtures::populate(&repo, 1); + git::push(&repo, "rad", branches.iter().map(|b| (b, b))).unwrap(); + + let mut alice = alice.spawn(service::Config::default()); + let bob = bob.spawn(service::Config::default()); + + alice.connect(&bob); + converge([&alice, &bob]); + + alice.handle.track_repo(acme, Scope::All).unwrap(); + let result = alice.handle.fetch(acme, bob.id).unwrap(); + + assert_matches!( + result, + FetchResult::Failed { + reason + } if reason == "no delegates in transfer" + ); +} + +#[test] +fn test_replication_invalid() { + let tmp = tempfile::tempdir().unwrap(); + let alice = Node::init(tmp.path()); + let mut bob = Node::init(tmp.path()); + let carol = MockSigner::default(); + let acme = bob.project("acme", ""); + let repo = bob.storage.repository_mut(acme).unwrap(); + let (_, head) = repo.head().unwrap(); + let id = repo.identity_head().unwrap(); + + // Create some unsigned refs for Carol in Bob's storage. + repo.raw() + .reference( + &git::qualified!("refs/heads/carol").with_namespace(carol.public_key().into()), + *head, + true, + &String::default(), + ) + .unwrap(); + repo.raw() + .reference( + &git::refs::storage::id(carol.public_key()), + id.into(), + true, + &String::default(), + ) + .unwrap(); + + let mut alice = alice.spawn(service::Config::default()); + let bob = bob.spawn(service::Config::default()); + + alice.connect(&bob); + converge([&alice, &bob]); + + alice.handle.track_node(*carol.public_key(), None).unwrap(); + alice.handle.track_repo(acme, Scope::Trusted).unwrap(); + let result = alice.handle.fetch(acme, bob.id).unwrap(); + + // Fetch is successful despite not fetching Carol's refs, since she isn't a delegate. + assert!(result.is_success()); + + let repo = alice.storage.repository(acme).unwrap(); + let mut remotes = repo.remote_ids().unwrap(); + + assert_eq!(remotes.next().unwrap().unwrap(), bob.id); + assert!(remotes.next().is_none()); + + repo.validate().unwrap(); } #[test] @@ -244,7 +328,7 @@ fn test_migrated_clone() { .unwrap(); assert_eq!(alice_refs, bob_refs); - assert_matches!(alice.storage.repository(acme).unwrap().verify(), Ok(())); + assert_matches!(alice.storage.repository(acme).unwrap().validate(), Ok(())); } #[test] diff --git a/radicle-node/src/worker/fetch.rs b/radicle-node/src/worker/fetch.rs index 613eb620..132c8548 100644 --- a/radicle-node/src/worker/fetch.rs +++ b/radicle-node/src/worker/fetch.rs @@ -258,6 +258,8 @@ impl<'a> StagingPhaseFinal<'a> { .into_iter() .flat_map(|(remote, verified)| match verified { VerifiedRemote::Failed { reason } => { + // TODO: We should include the skipped remotes in the fetch result, + // with the reason why they're skipped. log::warn!( target: "worker", "{remote} failed to verify, will not fetch any further refs: {reason}", @@ -276,21 +278,23 @@ impl<'a> StagingPhaseFinal<'a> { for refname in [heads, cobs, tags, notes] { let pattern = refname.with_pattern(git::refspec::STAR); - refspecs.push( + refspecs.push(( + remote.id, Refspec { src: pattern.clone(), dst: pattern, force: true, } .to_string(), - ); + )); } // Then add the special refs. let id = ns.join(&*radicle::git::refs::storage::IDENTITY_BRANCH); let sigrefs = ns.join(&*radicle::git::refs::storage::SIGREFS_BRANCH); - refspecs.push( + refspecs.push(( + remote.id, Refspec { src: id.clone(), dst: id, @@ -298,8 +302,9 @@ impl<'a> StagingPhaseFinal<'a> { force: true, } .to_string(), - ); - refspecs.push( + )); + refspecs.push(( + remote.id, Refspec { src: sigrefs.clone(), dst: sigrefs, @@ -307,11 +312,22 @@ impl<'a> StagingPhaseFinal<'a> { force: false, } .to_string(), - ); + )); refspecs } }) .collect::>(); + + let (fetching, specs): (HashSet<_>, Vec<_>) = specs.into_iter().unzip(); + + if !self + .repo + .delegates()? + .iter() + .all(|d| fetching.contains(d.as_key())) + { + return Err(error::Transfer::NoDelegates); + } log::debug!(target: "worker", "Transferring staging to production {url}"); let mut opts = git::raw::FetchOptions::default(); @@ -349,21 +365,21 @@ impl<'a> StagingPhaseFinal<'a> { self.trusted .iter() .map(|remote| { - let verification = match ( - self.repo.identity_doc_of(remote), - SignedRefs::load(remote, self.repo.deref()), - ) { - (Ok(doc), Ok(refs)) => VerifiedRemote::Success { - _doc: doc, - remote: Remote::new(*remote, refs), - }, - (Err(e), _) => VerifiedRemote::Failed { - reason: e.to_string(), - }, - (_, Err(e)) => VerifiedRemote::Failed { - reason: e.to_string(), - }, - }; + let verification = + match (self.repo.identity_doc_of(remote), self.repo.remote(remote)) { + (Ok(doc), Ok(remote)) => match self.repo.validate_remote(&remote) { + Ok(()) => VerifiedRemote::Success { _doc: doc, remote }, + Err(e) => VerifiedRemote::Failed { + reason: e.to_string(), + }, + }, + (Err(e), _) => VerifiedRemote::Failed { + reason: e.to_string(), + }, + (_, Err(e)) => VerifiedRemote::Failed { + reason: e.to_string(), + }, + }; (*remote, verification) }) .collect() diff --git a/radicle-node/src/worker/fetch/error.rs b/radicle-node/src/worker/fetch/error.rs index 066063b8..abf8eff9 100644 --- a/radicle-node/src/worker/fetch/error.rs +++ b/radicle-node/src/worker/fetch/error.rs @@ -30,6 +30,8 @@ pub enum Transfer { Identity(#[from] identity::IdentityError), #[error(transparent)] Storage(#[from] storage::Error), + #[error("no delegates in transfer")] + NoDelegates, } #[derive(Debug, Error)] diff --git a/radicle/src/node.rs b/radicle/src/node.rs index 39c78e3d..dc06eed1 100644 --- a/radicle/src/node.rs +++ b/radicle/src/node.rs @@ -252,6 +252,7 @@ impl Seeds { #[serde(tag = "status", rename_all = "kebab-case")] pub enum FetchResult { Success { updated: Vec }, + // TODO: Create enum for reason. Failed { reason: String }, } diff --git a/radicle/src/storage.rs b/radicle/src/storage.rs index 12223eff..91dacc94 100644 --- a/radicle/src/storage.rs +++ b/radicle/src/storage.rs @@ -296,10 +296,18 @@ pub trait ReadRepository { fn blob_at<'a>(&'a self, commit: Oid, path: &'a Path) -> Result, git_ext::Error>; - /// Verify all references in the repository, checking that they are signed - /// as part of 'sigrefs'. Also verify that no signed reference is missing - /// from the repository. - fn verify(&self) -> Result<(), VerifyError>; + /// Validate all remotes with [`ReadStorage::validate`]. + fn validate(&self) -> Result<(), VerifyError> { + for (_, remote) in self.remotes()? { + self.validate_remote(&remote)?; + } + Ok(()) + } + + /// Validate all the remote's references in the repository, checking that they are signed as + /// part of the remote's signed refs. Also verify that no signed reference is missing from the + /// repository. + fn validate_remote(&self, remote: &Remote) -> Result<(), VerifyError>; /// Get the head of this repository. /// diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index 77902d33..2ffd02e2 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -208,6 +208,8 @@ pub enum VerifyError { MissingRef(RemoteId, git::RefString), #[error("git: {0}")] Git(#[from] git2::Error), + #[error(transparent)] + Storage(#[from] Error), } impl Repository { @@ -346,33 +348,6 @@ impl Repository { }); Ok(remotes) } - - /// Return all references that are namespaced, ie. that are signed by a node and verified. - fn namespaced_references( - &self, - ) -> Result>, git2::Error> - { - let refs = self.backend.references_glob("refs/namespaces/*")?; - let refs = refs - .map(|reference| { - let r = reference?; - let name = r.name().ok_or(refs::Error::InvalidRef)?; - let (namespace, refname) = git::parse_ref_namespaced::(name)?; - let Some(oid) = r.target() else { - // Ignore symbolic refs, eg. `HEAD`. - return Ok(None); - }; - - if refname == *refs::SIGREFS_BRANCH { - // Ignore the signed-refs reference, as this is what we're verifying. - return Ok(None); - } - Ok(Some((namespace, refname.to_owned(), oid.into()))) - }) - .filter_map(Result::transpose); - - Ok(refs) - } } impl ReadRepository for Repository { @@ -396,39 +371,32 @@ impl ReadRepository for Repository { .get(&self.backend) } - fn verify(&self) -> Result<(), VerifyError> { - let mut remotes: HashMap = self - .remotes()? - .map(|remote| { - let (id, remote) = remote?; - Ok((id, remote.refs.into())) - }) - .collect::>()?; + fn validate_remote(&self, remote: &Remote) -> Result<(), VerifyError> { + // Contains a copy of the signed refs of this remote. + let mut refs = BTreeMap::from((*remote.refs).clone()); - for entry in self.namespaced_references()? { - let (remote_id, refname, oid) = entry?; - let remote = remotes - .get_mut(&remote_id) - .ok_or(VerifyError::InvalidRemote(remote_id))?; - let refname = RefString::from(refname); - let signed_oid = remote + // Check all repository references, making sure they are present in the signed refs map. + for (refname, oid) in self.references_of(&remote.id)? { + // Skip validation of the signed refs branch, as it is not part of `Remote`. + if refname == refs::SIGREFS_BRANCH.to_ref_string() { + continue; + } + let signed_oid = refs .remove(&refname) - .ok_or_else(|| VerifyError::UnknownRef(remote_id, refname.clone()))?; + .ok_or_else(|| VerifyError::UnknownRef(remote.id, refname.clone()))?; if oid != signed_oid { - return Err(VerifyError::InvalidRefTarget(remote_id, refname, *oid)); + return Err(VerifyError::InvalidRefTarget(remote.id, refname, *oid)); } } - for (remote, refs) in remotes.into_iter() { - // The refs that are left in the map, are ones that were signed, but are not - // in the repository. - if let Some((name, _)) = refs.into_iter().next() { - return Err(VerifyError::MissingRef(remote, name)); - } - // Verify identity history of remote. - self.identity_of(&remote)?.verified(self.id)?; + // The refs that are left in the map, are ones that were signed, but are not + // in the repository. If any are left, bail. + if let Some((name, _)) = refs.into_iter().next() { + return Err(VerifyError::MissingRef(remote.id, name)); } + // Finally, verify the identity history of remote. + self.identity_of(&remote.id)?.verified(self.id)?; Ok(()) } @@ -727,7 +695,7 @@ mod tests { } #[test] - fn test_namespaced_references() { + fn test_references_of() { let tmp = tempfile::tempdir().unwrap(); let signer = MockSigner::default(); let storage = Storage::open(tmp.path().join("storage")).unwrap(); @@ -739,14 +707,17 @@ mod tests { let proj = storage.repository(id).unwrap(); let mut refs = proj - .namespaced_references() + .references_of(signer.public_key()) .unwrap() - .map(|r| r.unwrap()) - .map(|(_, r, _)| r.to_string()) + .iter() + .map(|(r, _)| r.to_string()) .collect::>(); refs.sort(); - assert_eq!(refs, vec!["refs/heads/master", "refs/rad/id"]); + assert_eq!( + refs, + vec!["refs/heads/master", "refs/rad/id", "refs/rad/sigrefs"] + ); } #[test] diff --git a/radicle/src/test/fixtures.rs b/radicle/src/test/fixtures.rs index c20cc4b2..ef411c5a 100644 --- a/radicle/src/test/fixtures.rs +++ b/radicle/src/test/fixtures.rs @@ -86,24 +86,26 @@ pub fn repository>(path: P) -> (git2::Repository, git2::Oid) { } /// Populate a repository with commits, branches and blobs. -pub fn populate(repo: &git2::Repository, scale: usize) { +pub fn populate(repo: &git2::Repository, scale: usize) -> Vec { assert!( scale <= 8, "Scale parameter must be less than or equal to 8" ); if scale == 0 { - return; + return vec![]; } let head = repo.head().unwrap().peel_to_commit().unwrap(); let rng = fastrand::Rng::with_seed(42); let mut buffer = vec![0; 1024 * 1024 * scale]; + let mut refs = Vec::new(); for _ in 0..scale { let random = std::iter::repeat_with(|| rng.alphanumeric()) .take(7) .collect::() .to_lowercase(); - let name = format!("feature/{random}"); + let name = + git::refname!("feature").join(git::RefString::try_from(random.as_str()).unwrap()); let signature = git2::Signature::now("Radicle", "radicle@radicle.xyz").unwrap(); rng.fill(&mut buffer); @@ -113,17 +115,21 @@ pub fn populate(repo: &git2::Repository, scale: usize) { builder.insert("random.txt", blob, 0o100_644).unwrap(); let tree_oid = builder.write().unwrap(); let tree = repo.find_tree(tree_oid).unwrap(); + let refstr = git::refs::workdir::branch(&name); repo.commit( - Some(&format!("refs/heads/{name}")), + Some(&refstr), &signature, &signature, - &format!("Initialize new branch feature/{random}"), + &format!("Initialize new branch {name}"), &tree, &[&head], ) .unwrap(); + + refs.push(git::Qualified::from_refstr(refstr).unwrap()); } + refs } /// Generate random fixtures. diff --git a/radicle/src/test/storage.rs b/radicle/src/test/storage.rs index b1469845..5f54d2ef 100644 --- a/radicle/src/test/storage.rs +++ b/radicle/src/test/storage.rs @@ -140,7 +140,7 @@ impl ReadRepository for MockRepository { todo!() } - fn verify(&self) -> Result<(), VerifyError> { + fn validate_remote(&self, _remote: &Remote) -> Result<(), VerifyError> { Ok(()) }