From 38dc571f2e108e8775e5e2490baca6f03aaafccb Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Wed, 14 Sep 2022 16:01:32 +0200 Subject: [PATCH] node: Add verification on fetch --- node/Cargo.toml | 2 +- node/src/git.rs | 28 +++++- node/src/protocol.rs | 3 + node/src/rad.rs | 6 +- node/src/storage.rs | 14 ++- node/src/storage/git.rs | 173 +++++++++++++++++++++++++++++++++++--- node/src/test/fixtures.rs | 74 +++++++++++----- node/src/test/storage.rs | 5 +- node/src/test/tests.rs | 2 +- 9 files changed, 260 insertions(+), 47 deletions(-) diff --git a/node/Cargo.toml b/node/Cargo.toml index ec1f1cb2..96564575 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -30,9 +30,9 @@ radicle-git-ext = { version = "0", features = ["serde"] } nonempty = { version = "0.8.0", features = ["serialize"] } nakamoto-net = { version = "0.3.0" } nakamoto-net-poll = { version = "0.3.0" } +tempfile = { version = "3.3.0" } thiserror = { version = "1" } [dev-dependencies] quickcheck = { version = "1", default-features = false } quickcheck_macros = { version = "1", default-features = false } -tempfile = { version = "3.3.0" } diff --git a/node/src/git.rs b/node/src/git.rs index 786898b4..65298f4f 100644 --- a/node/src/git.rs +++ b/node/src/git.rs @@ -83,23 +83,45 @@ pub fn initial_commit<'a>( Ok(commit) } -/// Create a commit. +/// Create a commit and update the given ref to it. pub fn commit<'a>( repo: &'a git2::Repository, parent: &'a git2::Commit, + target: &RefStr, message: &str, user: &str, ) -> Result, git2::Error> { let sig = git2::Signature::now(user, "anonymous@radicle.xyz")?; let tree_id = repo.index()?.write_tree()?; let tree = repo.find_tree(tree_id)?; - // TODO: Take the ref as parameter. - let oid = repo.commit(None, &sig, &sig, message, &tree, &[parent])?; + let oid = repo.commit(Some(target.as_str()), &sig, &sig, message, &tree, &[parent])?; let commit = repo.find_commit(oid).unwrap(); Ok(commit) } +/// Push the refs to the radicle remote. +pub fn push(repo: &git2::Repository) -> Result<(), git2::Error> { + let mut remote = repo.find_remote("rad")?; + let refspecs = remote.push_refspecs().unwrap(); + let refspec = refspecs.into_iter().next().unwrap().unwrap(); + + // The `git2` crate doesn't seem to support push refspecs with '*' in them, + // so we manually replace it with the current branch. + let head = repo.head().unwrap(); + let branch = head.shorthand().unwrap(); + let refspec = refspec.replace('*', branch); + + remote.push::<&str>(&[&refspec], None) +} + +/// Get the repository head. +pub fn head(repo: &git2::Repository) -> Result { + let head = repo.head()?.peel_to_commit()?; + + Ok(head) +} + /// Configure a repository's radicle remote. /// /// Takes the repository in which to configure the remote, the name of the remote, the public diff --git a/node/src/protocol.rs b/node/src/protocol.rs index f2f21501..469f6822 100644 --- a/node/src/protocol.rs +++ b/node/src/protocol.rs @@ -69,6 +69,8 @@ pub enum FetchError { Git(#[from] git2::Error), #[error(transparent)] Storage(#[from] storage::Error), + #[error(transparent)] + Fetch(#[from] storage::FetchError), } /// Result of looking up seeds in our routing table. @@ -89,6 +91,7 @@ pub enum FetchLookup { /// Result of a fetch request from a specific seed. #[derive(Debug)] +#[allow(clippy::large_enum_variant)] pub enum FetchResult { /// Successful fetch from a seed. Fetched { diff --git a/node/src/rad.rs b/node/src/rad.rs index 8d74e85c..82ea4a41 100644 --- a/node/src/rad.rs +++ b/node/src/rad.rs @@ -243,7 +243,7 @@ mod tests { let signer = crypto::MockSigner::default(); let public_key = *signer.public_key(); let mut storage = Storage::open(tempdir.path().join("storage")).unwrap(); - let repo = fixtures::repository(tempdir.path().join("working")); + let (repo, _) = fixtures::repository(tempdir.path().join("working")); let (proj, refs) = init( &repo, @@ -280,7 +280,7 @@ mod tests { let bob = crypto::MockSigner::new(&mut rng); let bob_id = bob.public_key(); let mut storage = Storage::open(tempdir.path().join("storage")).unwrap(); - let original = fixtures::repository(tempdir.path().join("original")); + let (original, _) = fixtures::repository(tempdir.path().join("original")); // Alice creates a project. let (id, alice_refs) = init( @@ -311,7 +311,7 @@ mod tests { let signer = crypto::MockSigner::default(); let remote_id = signer.public_key(); let mut storage = Storage::open(tempdir.path().join("storage")).unwrap(); - let original = fixtures::repository(tempdir.path().join("original")); + let (original, _) = fixtures::repository(tempdir.path().join("original")); let (id, _) = init( &original, diff --git a/node/src/storage.rs b/node/src/storage.rs index 4d02ccba..edd13772 100644 --- a/node/src/storage.rs +++ b/node/src/storage.rs @@ -47,6 +47,18 @@ pub enum Error { InvalidHead, } +/// Fetch error. +#[derive(Error, Debug)] +#[allow(clippy::large_enum_variant)] +pub enum FetchError { + #[error("git: {0}")] + Git(#[from] git2::Error), + #[error("i/o: {0}")] + Io(#[from] io::Error), + #[error("verify: {0}")] + Verify(#[from] git::VerifyError), +} + pub type RemoteId = PublicKey; /// An update to a reference. @@ -241,7 +253,7 @@ pub trait ReadRepository<'r> { } pub trait WriteRepository<'r>: ReadRepository<'r> { - fn fetch(&mut self, url: &Url) -> Result, git2::Error>; + fn fetch(&mut self, url: &Url) -> Result, FetchError>; fn raw(&self) -> &git2::Repository; } diff --git a/node/src/storage/git.rs b/node/src/storage/git.rs index e2abe705..0be306c5 100644 --- a/node/src/storage/git.rs +++ b/node/src/storage/git.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::path::{Path, PathBuf}; use std::{fmt, fs, io}; @@ -14,7 +14,8 @@ use crate::identity::{Id, Project}; use crate::storage::refs; use crate::storage::refs::{Refs, SignedRefs}; use crate::storage::{ - Error, Inventory, ReadRepository, ReadStorage, Remote, WriteRepository, WriteStorage, + Error, FetchError, Inventory, ReadRepository, ReadStorage, Remote, WriteRepository, + WriteStorage, }; use super::{RefUpdate, RemoteId}; @@ -148,6 +149,24 @@ pub struct Repository { // git config for later. } +#[derive(Debug, Error)] +pub enum VerifyError { + #[error("invalid remote `{0}`")] + InvalidRemote(RemoteId), + #[error("invalid target `{2}` for reference `{1}` of remote `{0}`")] + InvalidRefTarget(RemoteId, git::RefString, git2::Oid), + #[error("invalid reference")] + InvalidRef, + #[error("ref error: {0}")] + Ref(#[from] git::RefError), + #[error("refs error: {0}")] + Refs(#[from] refs::Error), + #[error("unknown reference `{1}` in remote `{0}`")] + UnknownRef(RemoteId, git::RefString), + #[error("git: {0}")] + Git(#[from] git2::Error), +} + impl Repository { pub fn open>(path: P) -> Result { let backend = match git2::Repository::open_bare(path.as_ref()) { @@ -174,6 +193,39 @@ impl Repository { Ok(Self { backend }) } + pub fn head(&self) -> Result { + // TODO: Find longest history, get document and get head. + // Perhaps we should even set a local `HEAD` or at least `refs/heads/master` + todo!(); + } + + pub fn verify(&self) -> Result<(), VerifyError> { + let remotes = self.remotes()?.collect::, _>>()?; + + for r in self.backend.references()? { + let r = r?; + let name = r.name().ok_or(VerifyError::InvalidRef)?; + let oid = r.target().ok_or(VerifyError::InvalidRef)?; + let (remote, refname) = git::parse_ref::(name)?; + + if refname == *refs::SIGNATURE_REF { + continue; + } + let remote = remotes + .get(&remote) + .ok_or(VerifyError::InvalidRemote(remote))?; + let signed_oid = remote + .refs + .get(&refname) + .ok_or_else(|| VerifyError::UnknownRef(remote.id, refname.clone()))?; + + if git::Oid::from(oid) != *signed_oid { + return Err(VerifyError::InvalidRefTarget(remote.id, refname, oid)); + } + } + Ok(()) + } + pub fn inspect(&self) -> Result<(), Error> { for r in self.backend.references()? { let r = r?; @@ -228,6 +280,7 @@ impl<'r> ReadRepository<'r> for Repository { remote: &RemoteId, name: &git::RefStr, ) -> Result, git2::Error> { + let name = name.strip_prefix(git::refname!("refs")).unwrap_or(name); let name = format!("refs/remotes/{remote}/{name}"); self.backend.find_reference(&name).map(Some).or_else(|e| { if git::ext::is_not_found_err(&e) { @@ -288,7 +341,7 @@ impl<'r> ReadRepository<'r> for Repository { impl<'r> WriteRepository<'r> for Repository { /// Fetch all remotes of a project from the given URL. - fn fetch(&mut self, url: &git::Url) -> Result, git2::Error> { + fn fetch(&mut self, url: &git::Url) -> Result, FetchError> { // TODO: Have function to fetch specific remotes. // // Repository layout should look like this: @@ -303,6 +356,33 @@ impl<'r> WriteRepository<'r> for Repository { let refs: &[&str] = &["refs/remotes/*:refs/remotes/*"]; let mut updates = Vec::new(); let mut callbacks = git2::RemoteCallbacks::new(); + let tempdir = tempfile::tempdir()?; + // TODO: Comment + let staging = { + let mut builder = git2::build::RepoBuilder::new(); + let path = tempdir.path().join("git"); + let staging_repo = builder + .bare(true) + // TODO: Comment + .clone_local(git2::build::CloneLocal::Local) + .clone( + &git::Url { + scheme: git::url::Scheme::File, + path: self.backend.path().to_string_lossy().to_string().into(), + ..git::Url::default() + } + .to_string(), + &path, + )?; + + staging_repo + .remote_anonymous(&url)? + .fetch(refs, None, None)?; + // TODO: Comment + Repository::from(staging_repo).verify()?; + + path + }; callbacks.update_tips(|name, old, new| { if let Ok(name) = git::RefString::try_from(name) { @@ -316,7 +396,14 @@ impl<'r> WriteRepository<'r> for Repository { }); { - let mut remote = self.backend.remote_anonymous(&url)?; + let mut remote = self.backend.remote_anonymous( + &git::Url { + scheme: git::url::Scheme::File, + path: staging.to_string_lossy().to_string().into(), + ..git::Url::default() + } + .to_string(), + )?; let mut opts = git2::FetchOptions::default(); opts.remote_callbacks(callbacks); @@ -343,9 +430,10 @@ impl From for Repository { #[cfg(test)] mod tests { use super::*; + use crate::assert_matches; use crate::git; use crate::storage::refs::SIGNATURE_REF; - use crate::storage::{ReadStorage, WriteRepository}; + use crate::storage::{ReadStorage, RefUpdate, WriteRepository}; use crate::test::arbitrary; use crate::test::crypto::MockSigner; use crate::test::fixtures; @@ -410,6 +498,13 @@ mod tests { // Four refs are created for each remote. assert_eq!(updates.len(), remotes.len() * 4); + for update in updates { + assert_matches!( + update, + RefUpdate::Created { name, .. } if name.starts_with("refs/remotes") + ); + } + for remote in remotes { let (id, _) = remote.unwrap(); let alice_repo = alice.repository(proj).unwrap(); @@ -422,6 +517,57 @@ mod tests { } } + #[test] + fn test_fetch_update() { + let tmp = tempfile::tempdir().unwrap(); + let alice = Storage::open(tmp.path().join("alice/storage")).unwrap(); + let bob = Storage::open(tmp.path().join("bob/storage")).unwrap(); + + let alice_signer = MockSigner::new(&mut fastrand::Rng::new()); + let alice_id = alice_signer.public_key(); + let (proj_id, _, proj_repo, alice_head) = + fixtures::project(tmp.path().join("alice/project"), &alice, &alice_signer).unwrap(); + + let refname = git::refname!("refs/heads/master"); + let alice_url = git::Url { + scheme: git_url::Scheme::File, + path: alice + .path() + .join(proj_id.to_string()) + .to_string_lossy() + .into_owned() + .into(), + ..git::Url::default() + }; + + // Have Bob fetch Alice's refs. + let updates = bob.repository(&proj_id).unwrap().fetch(&alice_url).unwrap(); + // Three refs are created: the branch, the signature and the id. + assert_eq!(updates.len(), 3); + + let alice_proj_storage = alice.repository(&proj_id).unwrap(); + let alice_head = proj_repo.find_commit(alice_head).unwrap(); + let alice_head = git::commit(&proj_repo, &alice_head, &refname, "Making changes", "Alice") + .unwrap() + .id(); + git::push(&proj_repo).unwrap(); + alice.sign_refs(&alice_proj_storage, &alice_signer).unwrap(); + + // Have Bob fetch Alice's new commit. + let updates = bob.repository(&proj_id).unwrap().fetch(&alice_url).unwrap(); + // The branch and signature refs are updated. + assert_matches!( + updates.as_slice(), + &[RefUpdate::Updated { .. }, RefUpdate::Updated { .. }] + ); + + // Bob's storage is updated. + let bob_repo = bob.repository(&proj_id).unwrap(); + let bob_master = bob_repo.reference(alice_id, &refname).unwrap().unwrap(); + + assert_eq!(bob_master.target().unwrap(), alice_head); + } + #[test] fn test_sign_refs() { let tmp = tempfile::tempdir().unwrap(); @@ -435,15 +581,14 @@ mod tests { let sig = git2::Signature::now(&alice.to_string(), "anonymous@radicle.xyz").unwrap(); let head = git::initial_commit(backend, &sig).unwrap(); - let head = git::commit(backend, &head, "Second commit", &alice.to_string()).unwrap(); - backend - .reference( - &format!("refs/remotes/{alice}/heads/master"), - head.id(), - false, - "test", - ) - .unwrap(); + git::commit( + backend, + &head, + &git::RefString::try_from(format!("refs/remotes/{alice}/heads/master")).unwrap(), + "Second commit", + &alice.to_string(), + ) + .unwrap(); let signed = storage.sign_refs(&project, &signer).unwrap(); let remote = project.remote(&alice).unwrap(); diff --git a/node/src/test/fixtures.rs b/node/src/test/fixtures.rs index 44ecef2a..da7bd1f8 100644 --- a/node/src/test/fixtures.rs +++ b/node/src/test/fixtures.rs @@ -1,10 +1,12 @@ use std::path::Path; -use crate::crypto::Signer as _; +use crate::crypto::{Signer, Verified}; use crate::git; use crate::identity::Id; +use crate::rad; use crate::storage::git::Storage; -use crate::storage::WriteStorage; +use crate::storage::refs::SignedRefs; +use crate::storage::{BranchName, WriteStorage}; use crate::test::arbitrary; use crate::test::crypto::MockSigner; @@ -37,21 +39,21 @@ pub fn storage>(path: P) -> Storage { ) .unwrap(); - let head = git::commit(raw, &head, "Second commit", &remote.to_string()).unwrap(); - raw.reference( - &format!("refs/remotes/{remote}/heads/master"), - head.id(), - false, - "test", + let head = git::commit( + raw, + &head, + &git::RefString::try_from(format!("refs/remotes/{remote}/heads/master")).unwrap(), + "Second commit", + &remote.to_string(), ) .unwrap(); - let head = git::commit(raw, &head, "Third commit", &remote.to_string()).unwrap(); - raw.reference( - &format!("refs/remotes/{remote}/heads/patch/3"), - head.id(), - false, - "test", + git::commit( + raw, + &head, + &git::RefString::try_from(format!("refs/remotes/{remote}/heads/patch/3")).unwrap(), + "Third commit", + &remote.to_string(), ) .unwrap(); @@ -61,16 +63,44 @@ pub fn storage>(path: P) -> Storage { storage } +/// Create a new repository at the given path, and initialize it into a project. +pub fn project<'r, P: AsRef, S: WriteStorage<'r>, G: Signer>( + path: P, + storage: S, + signer: G, +) -> Result<(Id, SignedRefs, git2::Repository, git2::Oid), rad::InitError> { + let (repo, head) = repository(path); + let (id, refs) = rad::init( + &repo, + "acme", + "Acme's repository", + BranchName::from("master"), + signer, + storage, + )?; + + Ok((id, refs, repo, head)) +} + /// Creates a regular repository at the given path with a couple of commits. -pub fn repository>(path: P) -> git2::Repository { +pub fn repository>(path: P) -> (git2::Repository, git2::Oid) { let repo = git2::Repository::init(path).unwrap(); - { - let sig = git2::Signature::now("anonymous", "anonymous@radicle.xyz").unwrap(); - let head = git::initial_commit(&repo, &sig).unwrap(); - let head = git::commit(&repo, &head, "Second commit", "anonymous").unwrap(); - let _branch = repo.branch("master", &head, false).unwrap(); - } - repo + let sig = git2::Signature::now("anonymous", "anonymous@radicle.xyz").unwrap(); + let head = git::initial_commit(&repo, &sig).unwrap(); + let oid = git::commit( + &repo, + &head, + git::refname!("refs/heads/master").as_refstr(), + "Second commit", + "anonymous", + ) + .unwrap() + .id(); + + // Look, I don't really understand why we have to do this, but we do. + drop(head); + + (repo, oid) } #[cfg(test)] diff --git a/node/src/test/storage.rs b/node/src/test/storage.rs index 16b78e46..42244fad 100644 --- a/node/src/test/storage.rs +++ b/node/src/test/storage.rs @@ -5,7 +5,8 @@ use crate::git; use crate::identity::{Id, Project}; use crate::storage::{refs, RefUpdate}; use crate::storage::{ - Error, Inventory, ReadRepository, ReadStorage, Remote, RemoteId, WriteRepository, WriteStorage, + Error, FetchError, Inventory, ReadRepository, ReadStorage, Remote, RemoteId, WriteRepository, + WriteStorage, }; #[derive(Clone, Debug)] @@ -119,7 +120,7 @@ impl ReadRepository<'_> for MockRepository { } impl WriteRepository<'_> for MockRepository { - fn fetch(&mut self, _url: &Url) -> Result, git2::Error> { + fn fetch(&mut self, _url: &Url) -> Result, FetchError> { Ok(vec![]) } diff --git a/node/src/test/tests.rs b/node/src/test/tests.rs index 0e1ee7e9..2c00ca09 100644 --- a/node/src/test/tests.rs +++ b/node/src/test/tests.rs @@ -386,7 +386,7 @@ fn test_push_and_pull() { let tempdir = tempfile::tempdir().unwrap(); let storage_alice = Storage::open(tempdir.path().join("alice").join("storage")).unwrap(); - let repo = fixtures::repository(tempdir.path().join("working")); + let (repo, _) = fixtures::repository(tempdir.path().join("working")); let mut alice = Peer::new("alice", [7, 7, 7, 7], storage_alice); let storage_bob = Storage::open(tempdir.path().join("bob").join("storage")).unwrap();