From 989edacd564fa658358f5ccfd08c243c5ebd8cda Mon Sep 17 00:00:00 2001 From: cloudhead Date: Fri, 27 Sep 2024 09:32:15 +0200 Subject: [PATCH] Include new `rad/root` in signed refs We ensure that a `rad/root` ref is included in the signed refs file under `rad/sigrefs` for all remotes. This prevents a certain kind of "grafting" attack where signed refs can be copied between repositories, by having the peer sign over the identity root together with the data refs. When verifying signed refs, we ensure that the ref is present and points to an identity branch root that matches the repository identity containing the signed refs. Alternatives: lots of alternatives were considered, but this one doesn't introduce any changes to the signing. The `rad/id/root` name was considered but is invalid due to `rad/id`. --- radicle-cli/examples/rad-clean.md | 6 +- radicle-cli/examples/rad-clone-all.md | 5 + radicle-cli/examples/rad-fork.md | 3 + radicle-cli/examples/rad-id-multi-delegate.md | 2 +- .../examples/rad-id-threshold-soft-fork.md | 2 + radicle-cli/examples/rad-id-threshold.md | 7 +- radicle-cli/examples/rad-inspect.md | 3 +- radicle-cli/examples/rad-merge-via-push.md | 2 + radicle-cli/examples/rad-sync.md | 12 +- radicle-node/src/test/peer.rs | 12 +- radicle-node/src/tests.rs | 13 ++- radicle-node/src/worker/fetch.rs | 5 + radicle/src/cob/identity.rs | 2 +- radicle/src/cob/patch.rs | 2 +- radicle/src/cob/store.rs | 20 +++- radicle/src/git.rs | 18 ++- radicle/src/rad.rs | 21 ++-- radicle/src/storage.rs | 21 +++- radicle/src/storage/git.rs | 73 +++++++----- radicle/src/storage/git/cob.rs | 4 +- radicle/src/storage/refs.rs | 106 ++++++++++-------- radicle/src/test/arbitrary.rs | 17 +-- radicle/src/test/storage.rs | 18 ++- 23 files changed, 238 insertions(+), 136 deletions(-) diff --git a/radicle-cli/examples/rad-clean.md b/radicle-cli/examples/rad-clean.md index 2d09bffe..66f93b16 100644 --- a/radicle-cli/examples/rad-clean.md +++ b/radicle-cli/examples/rad-clean.md @@ -18,8 +18,8 @@ Let's also inspect what remotes are in the repository: ``` ~alice $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi f209c9f68aa689af24220a20462e13ee9dfb2a95 -z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk 161b775a3509c8098de67f57f750972bba015b31 +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 99c549702e2bcfe02b0e68d4a2224fb7a1524529 +z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk e9f48ef90fe8592e1b1c95f96c21a59ca1495300 ``` Now let's clean the `heartwood` project: @@ -34,7 +34,7 @@ Inspecting the remotes again, we see that Bob is now gone: ``` ~alice $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi f209c9f68aa689af24220a20462e13ee9dfb2a95 +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 99c549702e2bcfe02b0e68d4a2224fb7a1524529 ``` Note that Bob will be fetched again if we do not untrack his diff --git a/radicle-cli/examples/rad-clone-all.md b/radicle-cli/examples/rad-clone-all.md index 8fa5c738..0c1378b5 100644 --- a/radicle-cli/examples/rad-clone-all.md +++ b/radicle-cli/examples/rad-clone-all.md @@ -36,12 +36,14 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │   └── master    └── rad       ├── id +       ├── root       └── sigrefs z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk └── refs    ├── heads    │   └── master    └── rad +       ├── root       └── sigrefs ``` @@ -75,17 +77,20 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │   └── master    └── rad       ├── id +       ├── root       └── sigrefs z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk └── refs    ├── heads    │   └── master    └── rad +       ├── root       └── sigrefs z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z └── refs    ├── heads    │   └── master    └── rad +       ├── root       └── sigrefs ``` diff --git a/radicle-cli/examples/rad-fork.md b/radicle-cli/examples/rad-fork.md index bfedf69f..6ef727ac 100644 --- a/radicle-cli/examples/rad-fork.md +++ b/radicle-cli/examples/rad-fork.md @@ -14,6 +14,7 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │   └── master    └── rad       ├── id +       ├── root       └── sigrefs ``` @@ -39,12 +40,14 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │   └── master    └── rad       ├── id +       ├── root       └── sigrefs z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk └── refs    ├── heads    │   └── master    └── rad +       ├── root       └── sigrefs ``` diff --git a/radicle-cli/examples/rad-id-multi-delegate.md b/radicle-cli/examples/rad-id-multi-delegate.md index 1b97ceb1..26d5cb06 100644 --- a/radicle-cli/examples/rad-id-multi-delegate.md +++ b/radicle-cli/examples/rad-id-multi-delegate.md @@ -4,7 +4,7 @@ $ rad id update --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --title "Add Bob" --des ``` ``` ~bob -$ rad watch --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --node z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z -r 'refs/rad/sigrefs' -t 95cd447c57de8d232c6154f5dba0451aa593520e -i 500 --timeout 5000 +$ rad watch --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --node z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z -r 'refs/rad/sigrefs' -t c9a828fc2fb01f893d6e6e9e17b9092dea2b3aba -i 500 --timeout 5000 $ rad sync --fetch rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji ✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. ✓ Fetched repository from 1 seed(s) diff --git a/radicle-cli/examples/rad-id-threshold-soft-fork.md b/radicle-cli/examples/rad-id-threshold-soft-fork.md index 7de2e9fb..eb9f18fc 100644 --- a/radicle-cli/examples/rad-id-threshold-soft-fork.md +++ b/radicle-cli/examples/rad-id-threshold-soft-fork.md @@ -29,6 +29,7 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │   └── master    └── rad       ├── id +       ├── root       └── sigrefs z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk └── refs @@ -36,6 +37,7 @@ z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk    │   └── xyz.radicle.issue    │      └── f12d512c51d30429f7916db038ae0360e2e938c2    └── rad +       ├── root       └── sigrefs ``` diff --git a/radicle-cli/examples/rad-id-threshold.md b/radicle-cli/examples/rad-id-threshold.md index 54baface..73d6ccc6 100644 --- a/radicle-cli/examples/rad-id-threshold.md +++ b/radicle-cli/examples/rad-id-threshold.md @@ -141,6 +141,7 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │      └── b09b2aa0ee055671c811e9ad4ba73eed211ebaa3    └── rad       ├── id +       ├── root       └── sigrefs ``` @@ -148,7 +149,7 @@ Similarly, she still does not have Bob's `rad/sigrefs`: ``` ~alice $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 7ffed605e4871bb0640ee1538181640e239b182c +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ae3c6b77dc1ed51c1c1e6a2772339c2779fa9ba8 ``` And she can still list the project, without any worries: @@ -195,6 +196,6 @@ $ rad sync -f ✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk.. ✓ Fetched repository from 2 seed(s) $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 7ffed605e4871bb0640ee1538181640e239b182c -z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk cc068d93ee77dc134518d7d0fbe55b39804baf53 +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ae3c6b77dc1ed51c1c1e6a2772339c2779fa9ba8 +z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk dace6fe948548168a2bb687718949d9b5d9076ee ``` diff --git a/radicle-cli/examples/rad-inspect.md b/radicle-cli/examples/rad-inspect.md index 548e5711..ee8061c6 100644 --- a/radicle-cli/examples/rad-inspect.md +++ b/radicle-cli/examples/rad-inspect.md @@ -26,6 +26,7 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │   └── master    └── rad       ├── id +       ├── root       └── sigrefs ``` @@ -33,7 +34,7 @@ And sigrefs: ``` $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi f209c9f68aa689af24220a20462e13ee9dfb2a95 +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 99c549702e2bcfe02b0e68d4a2224fb7a1524529 ``` Or display the repository identity's payload and delegates: diff --git a/radicle-cli/examples/rad-merge-via-push.md b/radicle-cli/examples/rad-merge-via-push.md index c7927229..aa6a397b 100644 --- a/radicle-cli/examples/rad-merge-via-push.md +++ b/radicle-cli/examples/rad-merge-via-push.md @@ -45,6 +45,7 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │      └── 696ec5508494692899337afe6713fe1796d0315c    └── rad       ├── id +       ├── root       └── sigrefs ``` @@ -115,5 +116,6 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi    │   └── master    └── rad       ├── id +       ├── root       └── sigrefs ``` diff --git a/radicle-cli/examples/rad-sync.md b/radicle-cli/examples/rad-sync.md index 1b958588..cd74fdb3 100644 --- a/radicle-cli/examples/rad-sync.md +++ b/radicle-cli/examples/rad-sync.md @@ -15,9 +15,9 @@ $ rad sync status --sort-by alias ╭──────────────────────────────────────────────────────────────────────────────────────────────╮ │ ● Node Address Status Tip Timestamp │ ├──────────────────────────────────────────────────────────────────────────────────────────────┤ -│ ● alice (you) alice.radicle.example:8776 unannounced a9ce0d1 [ ... ] │ -│ ● bob z6Mkt67…v4N1tRk bob.radicle.example:8776 out-of-sync f209c9f [ ... ] │ -│ ● eve z6Mkux1…nVhib7Z eve.radicle.example:8776 out-of-sync f209c9f [ ... ] │ +│ ● alice (you) alice.radicle.example:8776 unannounced 056b1db [ ... ] │ +│ ● bob z6Mkt67…v4N1tRk bob.radicle.example:8776 out-of-sync 99c5497 [ ... ] │ +│ ● eve z6Mkux1…nVhib7Z eve.radicle.example:8776 out-of-sync 99c5497 [ ... ] │ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ ``` @@ -37,9 +37,9 @@ $ rad sync status --sort-by alias ╭─────────────────────────────────────────────────────────────────────────────────────────╮ │ ● Node Address Status Tip Timestamp │ ├─────────────────────────────────────────────────────────────────────────────────────────┤ -│ ● alice (you) alice.radicle.example:8776 a9ce0d1 [ ... ] │ -│ ● bob z6Mkt67…v4N1tRk bob.radicle.example:8776 synced a9ce0d1 [ ... ] │ -│ ● eve z6Mkux1…nVhib7Z eve.radicle.example:8776 synced a9ce0d1 [ ... ] │ +│ ● alice (you) alice.radicle.example:8776 056b1db [ ... ] │ +│ ● bob z6Mkt67…v4N1tRk bob.radicle.example:8776 synced 056b1db [ ... ] │ +│ ● eve z6Mkux1…nVhib7Z eve.radicle.example:8776 synced 056b1db [ ... ] │ ╰─────────────────────────────────────────────────────────────────────────────────────────╯ ``` diff --git a/radicle-node/src/test/peer.rs b/radicle-node/src/test/peer.rs index 88f372b2..5110289f 100644 --- a/radicle-node/src/test/peer.rs +++ b/radicle-node/src/test/peer.rs @@ -13,7 +13,7 @@ use radicle::node::Database; use radicle::node::UserAgent; use radicle::node::{address, Alias, ConnectOptions}; use radicle::rad; -use radicle::storage::refs::{RefsAt, SignedRefsAt}; +use radicle::storage::refs::{RefsAt, SignedRefsAt, IDENTITY_ROOT}; use radicle::storage::{ReadRepository, RemoteRepository}; use radicle::Storage; @@ -353,9 +353,15 @@ where ann.into().signed(self.signer()).into() } - pub fn signed_refs_at(&self, refs: Refs, at: radicle::git::Oid) -> SignedRefsAt { + pub fn signed_refs_at( + &self, + mut refs: Refs, + at: radicle::git::Oid, + repo: &R, + ) -> SignedRefsAt { + refs.insert(IDENTITY_ROOT.to_ref_string(), repo.identity_root().unwrap()); SignedRefsAt { - sigrefs: refs.signed(self.signer()).unwrap(), + sigrefs: refs.signed(self.signer()).unwrap().verified(repo).unwrap(), at, } } diff --git a/radicle-node/src/tests.rs b/radicle-node/src/tests.rs index d6d77acb..3e4cb320 100644 --- a/radicle-node/src/tests.rs +++ b/radicle-node/src/tests.rs @@ -877,13 +877,15 @@ fn test_refs_announcement_followed() { let mut bob = Peer::with_storage("bob", [8, 8, 8, 8], storage_bob); let node_id = alice.id; - alice.storage_mut().repo_mut(&rid).remotes.insert( + let repo = alice.storage_mut().repo_mut(&rid); + + repo.remotes.insert( node_id, - bob.signed_refs_at(arbitrary::gen::(8), arbitrary::oid()), + bob.signed_refs_at(arbitrary::gen::(8), arbitrary::oid(), repo), ); // Generate some refs for Bob under their own node_id. - let sigrefs = bob.signed_refs_at(arbitrary::gen::(8), arbitrary::oid()); + let sigrefs = bob.signed_refs_at(arbitrary::gen::(8), arbitrary::oid(), repo); let node_id = bob.id; bob.init(); bob.storage_mut() @@ -1544,9 +1546,10 @@ fn test_queued_fetch_from_ann_same_rid() { // Finish the 1st fetch. // Ensure the ref is in the storage and cache. - alice.storage_mut().repo_mut(&rid).remotes.insert( + let repo = alice.storage_mut().repo_mut(&rid); + repo.remotes.insert( carol.id(), - carol.signed_refs_at(arbitrary::gen::(1), oid), + carol.signed_refs_at(arbitrary::gen::(1), oid, repo), ); alice .database_mut() diff --git a/radicle-node/src/worker/fetch.rs b/radicle-node/src/worker/fetch.rs index 0d4853eb..c71695a0 100644 --- a/radicle-node/src/worker/fetch.rs +++ b/radicle-node/src/worker/fetch.rs @@ -210,6 +210,11 @@ fn notify( // be a separate notification on the identity COB itself. continue; } + if r == *git::refs::storage::IDENTITY_ROOT { + // Don't notify about the peers's identity root pointer. This is only used + // for sigref verification. + continue; + } if let Some(rest) = r.strip_prefix(git::refname!("refs/heads/patches")) { if radicle::cob::ObjectId::from_str(rest.as_str()).is_ok() { // Don't notify about patch branches, since we already get diff --git a/radicle/src/cob/identity.rs b/radicle/src/cob/identity.rs index 0266ab44..9a166df0 100644 --- a/radicle/src/cob/identity.rs +++ b/radicle/src/cob/identity.rs @@ -86,7 +86,7 @@ pub enum ApplyError { /// This error indicates that the operations are not being applied /// in causal order, which is a requirement for this CRDT. /// - /// For example, this can occur if an operation references anothern operation + /// For example, this can occur if an operation references another operation /// that hasn't happened yet. #[error("causal dependency {0:?} missing")] Missing(EntryId), diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 4008ff73..e81fff55 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -341,7 +341,7 @@ impl<'a, R: WriteRepository> Merged<'a, R> { self, working: &git::raw::Repository, signer: &G, - ) -> Result<(), storage::Error> { + ) -> Result<(), storage::RepositoryError> { let nid = signer.public_key(); let stored_ref = git::refs::patch(&self.patch).with_namespace(nid.into()); let working_ref = git::refs::workdir::patch_upstream(&self.patch); diff --git a/radicle/src/cob/store.rs b/radicle/src/cob/store.rs index 0984ee73..058f92b8 100644 --- a/radicle/src/cob/store.rs +++ b/radicle/src/cob/store.rs @@ -91,7 +91,7 @@ pub enum Error { #[error("object `{1}` of type `{0}` was not found")] NotFound(TypeName, ObjectId), #[error("signed refs: {0}")] - SignRefs(#[from] storage::Error), + SignRefs(Box), #[error("invalid or unknown embed URI: {0}")] EmbedUri(Uri), #[error(transparent)] @@ -181,7 +181,9 @@ where changes, }, )?; - self.repo.sign_refs(signer).map_err(Error::SignRefs)?; + self.repo + .sign_refs(signer) + .map_err(|e| Error::SignRefs(Box::new(e)))?; Ok(updated) } @@ -220,8 +222,14 @@ where contents, }, )?; - self.repo.sign_refs(signer).map_err(Error::SignRefs)?; - + // Nb. We can't sign our refs before the identity refs exist, which are created after + // the identity COB is created. Therefore we manually sign refs when creating identity + // COBs. + if T::type_name() != &*crate::cob::identity::TYPENAME { + self.repo + .sign_refs(signer) + .map_err(|e| Error::SignRefs(Box::new(e)))?; + } Ok((*cob.id(), cob.object)) } @@ -234,7 +242,9 @@ where { Ok(_) => { cob::remove(self.repo, signer.public_key(), T::type_name(), id)?; - self.repo.sign_refs(signer).map_err(Error::SignRefs)?; + self.repo + .sign_refs(signer) + .map_err(|e| Error::SignRefs(Box::new(e)))?; Ok(()) } Err(err) if err.code() == git::raw::ErrorCode::NotFound => Ok(()), diff --git a/radicle/src/git.rs b/radicle/src/git.rs index beaafc74..2f1c56c9 100644 --- a/radicle/src/git.rs +++ b/radicle/src/git.rs @@ -184,7 +184,7 @@ pub mod refs { use super::*; - /// Where the project's identity document is stored. + /// Where the repo's identity document is stored. /// /// `refs/rad/id` /// @@ -192,6 +192,14 @@ pub mod refs { Qualified::from_components(name::component!("rad"), name::component!("id"), None) }); + /// Where the repo's identity root document is stored. + /// + /// `refs/rad/root` + /// + pub static IDENTITY_ROOT: Lazy = Lazy::new(|| { + Qualified::from_components(name::component!("rad"), name::component!("root"), None) + }); + /// Where the project's signed references are stored. /// /// `refs/rad/sigrefs` @@ -250,6 +258,14 @@ pub mod refs { IDENTITY_BRANCH.with_namespace(remote.into()) } + /// Get the root of the branch where the project's identity document is stored. + /// + /// `refs/namespaces//refs/rad/root` + /// + pub fn id_root(remote: &RemoteId) -> Namespaced { + IDENTITY_ROOT.with_namespace(remote.into()) + } + /// Get the branch where the `remote`'s signed references are /// stored. /// diff --git a/radicle/src/rad.rs b/radicle/src/rad.rs index 07917a74..7cea4a49 100644 --- a/radicle/src/rad.rs +++ b/radicle/src/rad.rs @@ -1,4 +1,5 @@ #![allow(clippy::let_unit_value)] +#![warn(clippy::unwrap_used)] use std::io; use std::path::Path; use std::str::FromStr; @@ -54,8 +55,7 @@ pub fn init( storage: S, ) -> Result<(RepoId, identity::Doc, SignedRefs), InitError> { // TODO: Better error when project id already exists in storage, but remote doesn't. - let pk = signer.public_key(); - let delegate = identity::Did::from(*pk); + let delegate: identity::Did = signer.public_key().into(); let proj = Project::new( name.to_owned(), description.to_owned(), @@ -70,10 +70,10 @@ pub fn init( ) })?; let doc = identity::Doc::initial(proj, delegate, visibility).verified()?; - let (project, _) = Repository::init(&doc, &storage, signer)?; + let (project, identity) = Repository::init(&doc, &storage, signer)?; let url = git::Url::from(project.id); - match init_configure(repo, &project, pk, &default_branch, &url, signer) { + match init_configure(repo, &project, &default_branch, &url, identity, signer) { Ok(signed) => Ok((project.id, doc, signed)), Err(err) => { if let Err(e) = project.remove() { @@ -91,15 +91,17 @@ pub fn init( fn init_configure( repo: &git2::Repository, - project: &Repository, - pk: &crypto::PublicKey, + stored: &Repository, default_branch: &BranchName, url: &git::Url, + identity: git::Oid, signer: &G, ) -> Result, InitError> where G: crypto::Signer, { + let pk = signer.public_key(); + git::configure_repository(repo)?; git::configure_remote(repo, &REMOTE_NAME, url, &url.clone().with_namespace(*pk))?; git::push( @@ -110,10 +112,11 @@ where &git::fmt::lit::refs_heads(default_branch).into(), )], )?; + stored.set_remote_identity_root_to(pk, identity)?; + stored.set_identity_head_to(identity)?; + stored.set_head()?; - let signed = project.sign_refs(signer)?; - let _head = project.set_identity_head()?; - let _head = project.set_head()?; + let signed = stored.sign_refs(signer)?; Ok(signed) } diff --git a/radicle/src/storage.rs b/radicle/src/storage.rs index f3e34f83..d1ac42f8 100644 --- a/radicle/src/storage.rs +++ b/radicle/src/storage.rs @@ -355,8 +355,8 @@ impl Remote { } impl Remote { - pub fn verified(self) -> Result, crypto::Error> { - let refs = self.refs.verified()?; + pub fn verified(self, repo: &R) -> Result, Error> { + let refs = self.refs.verified(repo)?; Ok(Remote { refs }) } @@ -628,11 +628,24 @@ pub trait WriteRepository: ReadRepository + SignRepository { fn set_head(&self) -> Result; /// Set the repository 'rad/id' to the canonical commit, agreed by quorum. fn set_identity_head(&self) -> Result { - let head = self.canonical_identity_head()?; + let head = self.canonical_identity_head().unwrap(); self.set_identity_head_to(head)?; Ok(head) } + /// Set the identity root reference to the canonical identity root commit. + fn set_remote_identity_root(&self, remote: &RemoteId) -> Result { + let root = self.identity_root()?; + self.set_remote_identity_root_to(remote, root)?; + + Ok(root) + } + /// Set the identity root reference to the given commit. + fn set_remote_identity_root_to( + &self, + remote: &RemoteId, + root: Oid, + ) -> Result<(), RepositoryError>; /// Set the repository 'rad/id' to the given commit. fn set_identity_head_to(&self, commit: Oid) -> Result<(), RepositoryError>; /// Set the user info of the Git repository. @@ -644,7 +657,7 @@ pub trait WriteRepository: ReadRepository + SignRepository { /// Allows signing refs. pub trait SignRepository { /// Sign the repository's refs under the `refs/rad/sigrefs` branch. - fn sign_refs(&self, signer: &G) -> Result, Error>; + fn sign_refs(&self, signer: &G) -> Result, RepositoryError>; } impl ReadStorage for T diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index d65f5932..41c27f0a 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -20,15 +20,15 @@ use crate::node::SyncedAt; use crate::storage::refs; use crate::storage::refs::{Refs, SignedRefs, SignedRefsAt}; use crate::storage::{ - ReadRepository, ReadStorage, Remote, Remotes, RepositoryError, RepositoryInfo, SetHead, - SignRepository, WriteRepository, WriteStorage, + ReadRepository, ReadStorage, Remote, Remotes, RepositoryInfo, SetHead, SignRepository, + WriteRepository, WriteStorage, }; use crate::{git, node}; pub use crate::git::{ ext, raw, refname, refspec, Oid, PatternStr, Qualified, RefError, RefString, UserInfo, }; -pub use crate::storage::Error; +pub use crate::storage::{Error, RepositoryError}; use super::refs::RefsAt; use super::{RemoteId, RemoteRepository, ValidateRepository}; @@ -773,9 +773,8 @@ impl ReadRepository for Repository { fn identity_root(&self) -> Result { let oid = self.backend.refname_to_id(CANONICAL_IDENTITY.as_str())?; - let walk = self.revwalk(oid.into())?.collect::>(); - let root = walk - .into_iter() + let root = self + .revwalk(oid.into())? .last() .ok_or(RepositoryError::Doc(DocError::Missing))??; @@ -783,14 +782,8 @@ impl ReadRepository for Repository { } fn identity_root_of(&self, remote: &RemoteId) -> Result { - let oid = self.identity_head_of(remote)?; - let walk = self.revwalk(oid)?.collect::>(); - let root = walk - .into_iter() - .last() - .ok_or(RepositoryError::Doc(DocError::Missing))??; - - Ok(root.into()) + self.reference_oid(remote, &git::refs::storage::IDENTITY_ROOT) + .map_err(RepositoryError::from) } fn canonical_identity_head(&self) -> Result { @@ -857,6 +850,19 @@ impl WriteRepository for Repository { Ok(()) } + fn set_remote_identity_root_to( + &self, + remote: &RemoteId, + root: Oid, + ) -> Result<(), RepositoryError> { + let refname = git::refs::storage::id_root(remote); + + self.raw() + .reference(refname.as_str(), *root, true, "set-id-root (radicle)")?; + + Ok(()) + } + fn set_user(&self, info: &UserInfo) -> Result<(), Error> { let mut config = self.backend.config()?; config.set_str("user.name", &info.name())?; @@ -870,20 +876,24 @@ impl WriteRepository for Repository { } impl SignRepository for Repository { - fn sign_refs(&self, signer: &G) -> Result, Error> { + fn sign_refs(&self, signer: &G) -> Result, RepositoryError> { let remote = signer.public_key(); + // Ensure the root reference is set, which is checked during sigref verification. + if self.identity_root_of(remote).is_err() { + self.set_remote_identity_root(remote)?; + } let mut refs = self.references_of(remote)?; // Don't sign the `rad/sigrefs` ref itself, and don't sign invalid OIDs. refs.retain(|name, oid| { name.as_refstr() != refs::SIGREFS_BRANCH.as_ref() && !oid.is_zero() }); - let signed = refs.signed(signer)?; - + let signed = refs.signed(signer)?.verified(self)?; signed.save(self)?; Ok(signed) } } + pub mod trailers { use std::str::FromStr; @@ -946,7 +956,6 @@ mod tests { use crate::git; use crate::storage::refs::SIGREFS_BRANCH; use crate::storage::{ReadRepository, ReadStorage}; - use crate::test::arbitrary; use crate::test::fixtures; #[test] @@ -999,7 +1008,13 @@ mod tests { assert_eq!( refs, - vec![&cob, "refs/heads/master", "refs/rad/id", "refs/rad/sigrefs"] + vec![ + &cob, + "refs/heads/master", + "refs/rad/id", + "refs/rad/root", + "refs/rad/sigrefs" + ] ); } @@ -1009,28 +1024,26 @@ mod tests { let mut rng = fastrand::Rng::new(); let signer = MockSigner::new(&mut rng); let storage = Storage::open(tmp.path(), fixtures::user()).unwrap(); - let proj_id = arbitrary::gen::(1); let alice = *signer.public_key(); - let project = storage.create(proj_id).unwrap(); - let backend = &project.backend; + let (rid, _, working, _) = + fixtures::project(tmp.path().join("project"), &storage, &signer).unwrap(); + let stored = storage.repository(rid).unwrap(); let sig = git2::Signature::now(&alice.to_string(), "anonymous@radicle.xyz").unwrap(); - let head = git::initial_commit(backend, &sig).unwrap(); - let tree = - git::write_tree(Path::new("README"), "Hello World!\n".as_bytes(), backend).unwrap(); + let head = working.head().unwrap().peel_to_commit().unwrap(); git::commit( - backend, + &working, &head, &git::RefString::try_from(format!("refs/remotes/{alice}/heads/master")).unwrap(), "Second commit", &sig, - &tree, + &head.tree().unwrap(), ) .unwrap(); - let signed = project.sign_refs(&signer).unwrap(); - let remote = project.remote(&alice).unwrap(); - let mut unsigned = project.references_of(&alice).unwrap(); + let signed = stored.sign_refs(&signer).unwrap(); + let remote = stored.remote(&alice).unwrap(); + let mut unsigned = stored.references_of(&alice).unwrap(); // The signed refs doesn't contain the signature ref itself. let sigref = (*SIGREFS_BRANCH).to_ref_string(); diff --git a/radicle/src/storage/git/cob.rs b/radicle/src/storage/git/cob.rs index a232e538..b3e1cc58 100644 --- a/radicle/src/storage/git/cob.rs +++ b/radicle/src/storage/git/cob.rs @@ -24,7 +24,7 @@ use crate::{ use super::{RemoteId, Repository}; -pub use crate::cob::{store, ObjectId}; +pub use crate::cob::{store, ObjectId, Store}; #[derive(Error, Debug)] pub enum ObjectsError { @@ -226,7 +226,7 @@ impl<'a, R: storage::ReadRepository> SignRepository for DraftStore<'a, R> { fn sign_refs( &self, signer: &G, - ) -> Result, Error> { + ) -> Result, RepositoryError> { // Since this is a draft store, we do not actually want to sign the refs. // Instead, we just return the existing signed refs. let remote = self.repo.remote(signer.public_key())?; diff --git a/radicle/src/storage/refs.rs b/radicle/src/storage/refs.rs index 060e8df7..9d72c231 100644 --- a/radicle/src/storage/refs.rs +++ b/radicle/src/storage/refs.rs @@ -16,7 +16,7 @@ use crate::git::ext as git_ext; use crate::git::Oid; use crate::profile::env; use crate::storage; -use crate::storage::{ReadRepository, RemoteId, WriteRepository}; +use crate::storage::{ReadRepository, RemoteId, RepoId, WriteRepository}; pub use crate::git::refs::storage::*; @@ -44,6 +44,12 @@ pub enum Error { Canonical(#[from] canonical::Error), #[error("invalid reference")] InvalidRef, + #[error("missing identity root reference '{0}'")] + MissingIdentityRoot(git::RefString), + #[error("missing identity object '{0}'")] + MissingIdentity(Oid), + #[error("mismatched identity: local {local}, remote {remote}")] + MismatchedIdentity { local: RepoId, remote: RepoId }, #[error("invalid reference: {0}")] Ref(#[from] git::RefError), #[error(transparent)] @@ -72,27 +78,17 @@ pub struct Refs(BTreeMap); impl Refs { /// Verify the given signature on these refs, and return [`SignedRefs`] on success. - pub fn verified( + pub fn verified( self, signer: PublicKey, signature: Signature, + repo: &R, ) -> Result, Error> { - let refs = self; - let msg = refs.canonical(); - - match signer.verify(msg, &signature) { - Ok(()) => Ok(SignedRefs { - refs, - signature, - id: signer, - _verified: PhantomData, - }), - Err(e) => Err(e.into()), - } + SignedRefs::new(self, signer, signature).verified(repo) } /// Sign these refs with the given signer and return [`SignedRefs`]. - pub fn signed(self, signer: &G) -> Result, Error> + pub fn signed(self, signer: &G) -> Result, Error> where G: Signer, { @@ -100,12 +96,7 @@ impl Refs { let msg = refs.canonical(); let signature = signer.try_sign(&msg)?; - Ok(SignedRefs { - refs, - signature, - id: *signer.public_key(), - _verified: PhantomData, - }) + Ok(SignedRefs::new(refs, *signer.public_key(), signature)) } /// Get a particular ref. @@ -216,17 +207,17 @@ pub struct SignedRefs { } impl SignedRefs { - pub fn new(refs: Refs, id: PublicKey, signature: Signature) -> Self { + pub fn new(refs: Refs, author: PublicKey, signature: Signature) -> Self { Self { refs, signature, - id, + id: author, _verified: PhantomData, } } - pub fn verified(self) -> Result, crypto::Error> { - match self.verify(&self.id) { + pub fn verified(self, repo: &R) -> Result, Error> { + match self.verify(repo) { Ok(()) => Ok(SignedRefs { refs: self.refs, signature: self.signature, @@ -237,13 +228,36 @@ impl SignedRefs { } } - pub fn verify(&self, signer: &PublicKey) -> Result<(), crypto::Error> { + pub fn verify(&self, repo: &R) -> Result<(), Error> { let canonical = self.refs.canonical(); + let local = repo.id(); - match signer.verify(canonical, &self.signature) { - Ok(()) => Ok(()), - Err(e) => Err(e), + // Verify signature. + if let Err(e) = self.id.verify(canonical, &self.signature) { + return Err(e.into()); } + // If the identity root was signed, verify it points to the right place. + if let Some(id_root) = self.refs.get(&IDENTITY_ROOT) { + // Get the identity at the given oid. + let Ok(doc) = repo.identity_doc_at(id_root) else { + return Err(Error::MissingIdentity(id_root)); + }; + let remote = RepoId::from(doc.blob); + + // Make sure the signed identity points to the local repo identity. + if remote != local { + return Err(Error::MismatchedIdentity { local, remote }); + } + } else { + // TODO(cloudhead): Make this into a hard error (`Error::MissingIdentityRoot`) for + // repos that have migrated to the new identity document schema. + log::debug!( + target: "storage", + "Signed ref verification for {} in {local}: {} is not provided", + self.id, *IDENTITY_ROOT + ); + } + Ok(()) } } @@ -264,20 +278,9 @@ impl SignedRefs { let refs = repo.blob_at(oid, Path::new(REFS_BLOB_PATH))?; let signature = repo.blob_at(oid, Path::new(SIGNATURE_BLOB_PATH))?; let signature: crypto::Signature = signature.content().try_into()?; + let refs = Refs::from_canonical(refs.content())?; - match remote.verify(refs.content(), &signature) { - Ok(()) => { - let refs = Refs::from_canonical(refs.content())?; - - Ok(Self { - refs, - signature, - id: remote, - _verified: PhantomData, - }) - } - Err(e) => Err(e.into()), - } + SignedRefs::new(refs, remote, signature).verified(repo) } /// Save the signed refs to disk. @@ -290,7 +293,7 @@ impl SignedRefs { // N.b. if the signatures match then there are no updates let parent = match SignedRefsAt::load(*remote, repo)? { Some(SignedRefsAt { sigrefs, at }) if sigrefs.signature == self.signature => { - return Ok(Updated::Unchanged { oid: at }) + return Ok(Updated::Unchanged { oid: at }); } Some(SignedRefsAt { at, .. }) => Some(raw.find_commit(*at)?), None => None, @@ -460,13 +463,14 @@ pub mod canonical { #[cfg(test)] mod tests { - use super::*; - use crate::assert_matches; - use crate::{cob::identity::Identity, rad, test::fixtures, Storage}; use crypto::test::signer::MockSigner; use qcheck_macros::quickcheck; use storage::{git::transport, RemoteRepository, SignRepository, WriteStorage}; + use super::*; + use crate::assert_matches; + use crate::{cob::identity::Identity, rad, test::fixtures, Storage}; + #[quickcheck] fn prop_canonical_roundtrip(refs: Refs) { let encoded = refs.canonical(); @@ -476,7 +480,6 @@ mod tests { } #[test] - #[should_panic] // Test that a user's signed refs are tied to a specific RID, and they can't simply be // used in a different repository. // @@ -634,6 +637,13 @@ mod tests { // Due to the verification, we get a validation error when trying to load Bob's remote. // The graft is not allowed. - london.remote(bob.public_key()).unwrap_err(); + assert_matches!( + london.remote(bob.public_key()), + Err(Error::MismatchedIdentity { + local, + remote, + }) + if local == london_rid && remote == paris_rid + ); } } diff --git a/radicle/src/test/arbitrary.rs b/radicle/src/test/arbitrary.rs index e90f3fa4..d049a329 100644 --- a/radicle/src/test/arbitrary.rs +++ b/radicle/src/test/arbitrary.rs @@ -71,12 +71,13 @@ pub fn vec(size: usize) -> Vec { pub fn nonempty_storage(size: usize) -> MockStorage { let mut storage = gen::(size); for _ in 0..size { - let id = gen::(1); + let doc = gen::(1); + let id = RepoId::from(doc.blob); storage.repos.insert( id, MockRepository { id, - doc: gen::(1), + doc, remotes: HashMap::new(), }, ); @@ -90,9 +91,9 @@ pub fn gen(size: usize) -> T { T::arbitrary(&mut gen) } -impl Arbitrary for storage::Remotes { +impl Arbitrary for storage::Remotes { fn arbitrary(g: &mut qcheck::Gen) -> Self { - let remotes: RandomMap> = + let remotes: RandomMap> = Arbitrary::arbitrary(g); storage::Remotes::new(remotes) @@ -181,10 +182,10 @@ impl Arbitrary for SignedRefs { fn arbitrary(g: &mut qcheck::Gen) -> Self { let bytes: [u8; 64] = Arbitrary::arbitrary(g); let signature = crypto::Signature::from(bytes); - let id = PublicKey::arbitrary(g); + let author = PublicKey::arbitrary(g); let refs = Refs::arbitrary(g); - Self::new(refs, id, signature) + Self::new(refs, author, signature) } } @@ -243,13 +244,13 @@ impl Arbitrary for MockRepository { } } -impl Arbitrary for storage::Remote { +impl Arbitrary for storage::Remote { fn arbitrary(g: &mut qcheck::Gen) -> Self { let refs = Refs::arbitrary(g); let signer = MockSigner::arbitrary(g); let signed = refs.signed(&signer).unwrap(); - storage::Remote::::new(signed) + storage::Remote::::new(signed) } } diff --git a/radicle/src/test/storage.rs b/radicle/src/test/storage.rs index 91d37e62..96464d15 100644 --- a/radicle/src/test/storage.rs +++ b/radicle/src/test/storage.rs @@ -300,19 +300,19 @@ impl ReadRepository for MockRepository { } fn identity_head_of(&self, _remote: &RemoteId) -> Result { - todo!() + Ok(self.doc.commit) } fn identity_root(&self) -> Result { - todo!() + Ok(self.doc.commit) } fn identity_root_of(&self, _remote: &RemoteId) -> Result { - todo!() + Ok(self.doc.commit) } fn canonical_identity_head(&self) -> Result { - Ok(Oid::from_str("cccccccccccccccccccccccccccccccccccccccc").unwrap()) + Ok(self.doc.commit) } fn merge_base(&self, _left: &Oid, _right: &Oid) -> Result { @@ -333,6 +333,14 @@ impl WriteRepository for MockRepository { todo!() } + fn set_remote_identity_root_to( + &self, + _remote: &RemoteId, + _root: Oid, + ) -> Result<(), RepositoryError> { + todo!() + } + fn set_user(&self, _info: &git::UserInfo) -> Result<(), Error> { todo!() } @@ -342,7 +350,7 @@ impl SignRepository for MockRepository { fn sign_refs( &self, _signer: &G, - ) -> Result, Error> { + ) -> Result, RepositoryError> { todo!() } }