0e7ffcc node: Properly validate signed refs on fetch — heartwood

modified 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<G: cyphernet::Ecdh<Pk = NodeId> + Signer + Clone> Node<G> {

         }
         git::push(repo, "rad", refs.iter().map(|(a, b)| (a, b))).unwrap();
         self.storage
             .repository(id)
             .unwrap()
             .sign_refs(&self.signer)
             .unwrap();
         id
     }

modified radicle-node/src/test/simulator.rs

@@ -699,7 +699,7 @@ fn fetch<W: WriteRepository>(

     remote.fetch(&[refspec], Some(&mut opts), None)?;
     drop(opts);
     repo.verify()?;
     repo.validate()?;
     repo.set_head()?;
     Ok(updates)

modified 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]

modified 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::<Vec<_>>();
             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()

modified 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)]

modified radicle/src/node.rs

@@ -252,6 +252,7 @@ impl Seeds {

 #[serde(tag = "status", rename_all = "kebab-case")]
 pub enum FetchResult {
     Success { updated: Vec<RefUpdate> },
     // TODO: Create enum for reason.
     Failed { reason: String },
 }

modified radicle/src/storage.rs

@@ -296,10 +296,18 @@ pub trait ReadRepository {

     fn blob_at<'a>(&'a self, commit: Oid, path: &'a Path)
         -> Result<git2::Blob<'a>, 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<Verified>) -> Result<(), VerifyError>;
     /// Get the head of this repository.
     ///

modified 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<impl Iterator<Item = Result<(RemoteId, Qualified, Oid), refs::Error>>, 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::<RemoteId>(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<RemoteId, Refs> = self
             .remotes()?
             .map(|remote| {
                 let (id, remote) = remote?;
                 Ok((id, remote.refs.into()))
             })
             .collect::<Result<_, VerifyError>>()?;
         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
     fn validate_remote(&self, remote: &Remote<Verified>) -> Result<(), VerifyError> {
         // Contains a copy of the signed refs of this remote.
         let mut refs = BTreeMap::from((*remote.refs).clone());
         // 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::<Vec<_>>();
         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]

modified radicle/src/test/fixtures.rs

@@ -86,24 +86,26 @@ pub fn repository<P: AsRef<Path>>(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<git::Qualified> {
     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::<String>()
             .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.

modified 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<Verified>) -> Result<(), VerifyError> {
         Ok(())
     }