radicle: Reject COB changes with forged identity

Each COB change names an identity commit in its `Rad-Resource`
trailer. Patches and issues check the rights of the author against the
document in that commit. Nothing checked that the commit is a revision
of the repository identity. Any user could write a commit with a
document that names them as a delegate, cite it, and act as a delegate.

- Add `Identity::is_accepted`, which follows each revision's parent
  back from `rad/id`, so a change that cites the head costs one blob
  read
- Add `ReadRepository::identity_revision_doc`, which returns the
  document of an accepted identity revision only
- Use it in `Op::identity_doc`, so a change with any other resource
  gets no identity document and is rejected
- Document that `identity_doc_at` does not check the commit
- Test forged resources on patches and issues, and the accepted chain

Accepted revisions form a chain, because a revision is only adopted if
its parent is the current revision. Callers synchronize `rad/id` with
the current revision after identity writes, so the chain holds every
accepted revision.
This commit is contained in:
Daniel Norman
2026-11-02 21:27:45 +01:00
committed by Lorenz Leutgeb
parent 783b06fb04
commit eee5cb8d8c
16 changed files with 277 additions and 48 deletions
+2 -2
View File
@@ -17,7 +17,7 @@ use radicle::prelude::*;
use radicle::rad;
use radicle::storage;
use radicle::storage::RemoteId;
use radicle::storage::{HasRepoId, RepositoryError};
use radicle::storage::{HasRepoId, IdentityDocError, RepositoryError};
use crate::commands::checkout;
use crate::commands::sync;
@@ -330,7 +330,7 @@ impl CloneResult {
#[derive(Debug)]
pub enum CheckoutFailure {
Identity { rid: RepoId, err: RepositoryError },
Identity { rid: RepoId, err: IdentityDocError },
Payload { rid: RepoId, err: doc::PayloadError },
Exists { rid: RepoId, path: PathBuf },
}
+5 -4
View File
@@ -92,10 +92,11 @@ pub fn seeding(profile: &Profile) -> anyhow::Result<()> {
Ok(policy::SeedPolicy { rid, policy }) => {
let id = rid.to_string();
let name = match storage
.repository(rid)
.and_then(|repo| repo.identity_doc().map(|identity| identity.project()))
{
let name = match storage.repository(rid).and_then(|repo| {
repo.identity_doc()
.map(|identity| identity.project())
.map_err(radicle::storage::RepositoryError::from)
}) {
Ok(Some(Ok(project))) => project.name().to_string().into(),
Ok(None) => term::format::dim("No name provided.".to_string()),
Err(_) | Ok(Some(Err(_))) => {
+3 -3
View File
@@ -4,7 +4,7 @@ use std::io::Write;
use radicle::node::sync;
use radicle::node::{Handle as _, NodeId};
use radicle::storage::{ReadRepository, RepositoryError, refs};
use radicle::storage::{IdentityDocError, ReadRepository, refs};
use radicle::{Node, Profile};
use crate::terminal as term;
@@ -87,7 +87,7 @@ impl Default for SyncSettings {
#[derive(thiserror::Error, Debug)]
pub enum SyncError {
#[error(transparent)]
Repository(#[from] RepositoryError),
IdentityDoc(#[from] IdentityDocError),
#[error(transparent)]
Node(#[from] radicle::node::Error),
#[error("all seeds timed out")]
@@ -100,7 +100,7 @@ impl SyncError {
fn is_connection_err(&self) -> bool {
match self {
Self::Node(e) => e.is_connection_err(),
Self::Repository(_) | Self::AllSeedsTimedOut | Self::Target(_) => false,
Self::IdentityDoc(_) | Self::AllSeedsTimedOut | Self::Target(_) => false,
}
}
}
+4 -2
View File
@@ -5,7 +5,6 @@ use bstr::BString;
use radicle::crypto::PublicKey;
use radicle::git::Oid;
use radicle::identity::{Doc, DocError};
use radicle::storage::ReadRepository;
use radicle::storage::git::Repository;
use crate::policy::{Allowed, BlockList};
@@ -89,7 +88,10 @@ where
}
pub fn verified(&self, head: Oid) -> Result<Doc, DocError> {
Ok(self.repository().identity_doc_at(head)?.doc)
// During fetch, the candidate may be ahead of local `rad/id`, or the
// local identity history may not exist yet. Validate the document here;
// repository identity validation happens after the refs are fetched.
Ok(Doc::load_at(head, self.repository())?.doc)
}
}
+11 -9
View File
@@ -38,6 +38,7 @@ use gix_protocol::handshake::Ref;
use nonempty::NonEmpty;
use radicle::crypto::PublicKey;
use radicle::git::fmt::{Component, Namespaced, Qualified, refname};
use radicle::identity::Doc;
use radicle::storage::ReadRepository;
use radicle::storage::git::Repository;
use radicle::storage::refs::{RefsAt, Special};
@@ -237,15 +238,16 @@ impl ProtocolStage for CanonicalId {
) -> Result<Updates<'a>, error::Prepare> {
// SAFETY: checked by `pre_validate` that the `refs/rad/id`
// was received
let verified = repo
.identity_doc_at(
*s.canonical_rad_id()
.expect("ensure we got canonical 'rad/id' ref"),
)
.map_err(|err| error::Prepare::Verification {
remote: self.remote,
err: Box::new(err),
})?;
// Bootstrap delegate discovery before the local accepted history exists.
let verified = Doc::load_at(
*s.canonical_rad_id()
.expect("ensure we got canonical 'rad/id' ref"),
repo,
)
.map_err(|err| error::Prepare::Verification {
remote: self.remote,
err: Box::new(err),
})?;
if verified.is_delegate(&self.remote.into()) {
let is_delegate = |remote: &PublicKey| verified.is_delegate(&remote.into());
Ok(Updates::build(
+2
View File
@@ -64,6 +64,8 @@ pub enum AuthorizationError {
PolicyStore(#[from] radicle::node::policy::store::Error),
#[error(transparent)]
Repository(#[from] radicle::storage::RepositoryError),
#[error(transparent)]
IdentityDoc(#[from] radicle::storage::IdentityDocError),
}
/// Fetch job sent to worker thread.
@@ -18,6 +18,8 @@ pub enum Fetch {
#[error(transparent)]
Repository(#[from] radicle::storage::RepositoryError),
#[error(transparent)]
IdentityDoc(#[from] storage::IdentityDocError),
#[error(transparent)]
RefsDb(Box<radicle::node::refs::Error>),
#[error(
"validation of the storage repository failed: the delegates {delegates:?} failed to validate to meet a threshold of {threshold}"
+72
View File
@@ -1477,6 +1477,77 @@ mod test {
);
}
#[test]
fn identity_doc_at_checks_accepted_history() {
let NodeWithRepo { node, repo } = NodeWithRepo::default();
let bob = SigningKey::mock(103);
let mut identity = Identity::load_mut(&*repo, &node.signer).unwrap();
let mut doc = identity.doc().clone().edit();
let title = Title::new("Identity update").unwrap();
let r0 = identity.current;
doc.delegate(bob.public_key().into());
let r1 = identity
.update(title.clone(), "", &doc.clone().verified().unwrap())
.unwrap();
repo.set_identity_head_to(identity.current).unwrap();
// With two delegates, this revision stays pending until Bob accepts.
doc.visibility = Visibility::private([]);
let r2 = identity
.update(title, "", &doc.clone().verified().unwrap())
.unwrap();
let forged = cob::test::forge_identity(repo.raw(), &doc.verified().unwrap());
assert_eq!(repo.identity_doc_at(r0).unwrap().commit, r0);
assert_eq!(repo.identity_doc_at(r1).unwrap().commit, r1);
for commit in [r2, forged] {
assert_eq!(Doc::load_at(commit, &*repo).unwrap().commit, commit);
assert!(matches!(
repo.identity_doc_at(commit),
Err(crate::storage::IdentityDocError::UnacceptedRevision(id)) if id == commit
));
}
// The current-document lookup also rejects an unaccepted cached head.
repo.set_identity_head_to(r2).unwrap();
assert!(matches!(
repo.identity_doc(),
Err(crate::storage::IdentityDocError::UnacceptedRevision(id)) if id == r2
));
repo.set_identity_head_to(r1).unwrap();
let mut identity = Identity::load_mut(&*repo, &bob).unwrap();
identity.accept(&r2).unwrap();
// Accepted revisions are recognized even ahead of cached `rad/id`.
assert_eq!(repo.identity_head().unwrap(), r1);
assert_eq!(repo.identity_doc_at(r2).unwrap().commit, r2);
assert_eq!(repo.canonical_identity_doc().unwrap().commit, r2);
repo.set_identity_head_to(identity.current).unwrap();
assert_eq!(repo.identity_doc_at(r0).unwrap().commit, r0);
assert_eq!(repo.identity_doc_at(r1).unwrap().commit, r1);
assert_eq!(repo.identity_doc_at(r2).unwrap().commit, r2);
}
#[test]
fn identity_document_lookup_preserves_loading_errors() {
let fixture = NodeWithRepo::default();
let repo = &fixture.repo;
let head = repo.identity_head().unwrap();
repo.raw()
.find_reference("refs/rad/id")
.unwrap()
.delete()
.unwrap();
for result in [repo.identity_doc(), repo.identity_doc_at(head)] {
let Err(crate::storage::IdentityDocError::Repository(err)) = result else {
panic!("expected a repository loading error, got {result:?}");
};
assert!(err.is_not_found());
}
}
#[test]
fn identity_update_rejected() {
let NodeWithRepo { node, repo } = NodeWithRepo::default();
@@ -2663,6 +2734,7 @@ mod test {
let mut eve_identity = Identity::load_mut(&repo, &eve).unwrap();
eve_identity.accept(&revision).unwrap();
repo.set_identity_head_to(eve_identity.current).unwrap();
let identity: Identity = Identity::load(&repo).unwrap();
let root = repo.identity_root().unwrap();
+39 -3
View File
@@ -16,11 +16,10 @@ use crate::cob::store::{Cob, CobAction};
use crate::cob::thread::{Comment, CommentId, Thread};
use crate::cob::{ActorId, Embed, EntryId, ObjectId, TypeName, op, store};
use crate::cob::{TitleError, thread};
use crate::identity::doc::DocError;
use crate::node::NodeId;
use crate::prelude::{Did, Doc, ReadRepository, RepoId};
use crate::storage;
use crate::storage::{HasRepoId, RepositoryError, WriteRepository};
use crate::storage::{HasRepoId, IdentityDocError, RepositoryError, WriteRepository};
pub use cache::Cache;
@@ -48,7 +47,7 @@ impl<'a> IssueStream<'a> {
pub enum Error {
/// Error loading the identity document.
#[error("identity doc failed to load: {0}")]
Doc(#[from] DocError),
Doc(#[from] IdentityDocError),
#[error("thread apply failed: {0}")]
Thread(#[from] thread::Error),
#[error("store: {0}")]
@@ -1845,4 +1844,41 @@ mod test {
assert_eq!(cob.history.len(), 3);
assert_eq!(cob.object.len(), 3);
}
#[test]
fn test_issue_forged_identity_resource() {
use crate::cob::test::SignerOpExt as _;
let test::setup::NodeWithRepo { node, repo, .. } = test::setup::NodeWithRepo::default();
let eve = SigningKey::mock(31);
let mut issues = Cache::no_cache(&*repo, &node.signer).unwrap();
let id = issues
.create(
cob::Title::new("My first issue").unwrap(),
"Blah blah blah.",
&[],
&[],
[],
)
.unwrap()
.id;
let label = Action::Label {
labels: BTreeSet::from([Label::new("bug").unwrap()]),
};
// Eve is not a delegate, so the real identity does not allow the action.
let identity = repo.identity_head().unwrap();
cob::test::store_change::<Issue>(&repo, &eve, identity, &id, &label);
let issue = issues.get(&id).unwrap().unwrap();
assert_eq!(issue.labels().count(), 0);
// A forged identity document that names Eve as a delegate must not
// allow the action either.
let mut doc = repo.identity_doc().unwrap().doc.edit();
doc.delegate(eve.did());
let forged = cob::test::forge_identity(repo.raw(), &doc.verified().unwrap());
cob::test::store_change::<Issue>(&repo, &eve, forged, &id, &label);
let issue = issues.get(&id).unwrap().unwrap();
assert_eq!(issue.labels().count(), 0);
}
}
+2 -3
View File
@@ -9,9 +9,8 @@ use radicle_crypto::PublicKey;
use crate::cob;
use crate::cob::Timestamp;
use crate::git;
use crate::identity;
use crate::identity::DocAt;
use crate::storage::ReadRepository;
use crate::storage::{IdentityDocError, ReadRepository};
/// The author of an [`Op`].
pub type ActorId = PublicKey;
@@ -112,7 +111,7 @@ impl<A> Op<A> {
pub fn identity_doc<R: ReadRepository>(
&self,
repo: &R,
) -> Result<Option<DocAt>, identity::DocError> {
) -> Result<Option<DocAt>, IdentityDocError> {
match self.identity {
None => Ok(None),
Some(head) => repo.identity_doc_at(head).map(Some),
+42 -3
View File
@@ -14,7 +14,7 @@ use std::sync::LazyLock;
use nonempty::NonEmpty;
use serde::{Deserialize, Serialize};
use storage::{HasRepoId, RepositoryError};
use storage::{HasRepoId, IdentityDocError, RepositoryError};
use thiserror::Error;
use crate::cob;
@@ -29,7 +29,7 @@ use crate::cob::{ActorId, Embed, EntryId, ObjectId, TypeName, Uri, op, store};
use crate::crypto::PublicKey;
use crate::git;
use crate::identity::PayloadError;
use crate::identity::doc::{DefaultBranchError, DocAt, DocError};
use crate::identity::doc::{DefaultBranchError, DocAt};
use crate::prelude::*;
use crate::storage;
use crate::storage::git::Repository;
@@ -159,7 +159,7 @@ pub enum Error {
Thread(#[from] thread::Error),
/// Error loading the identity document committed to by an operation.
#[error("identity doc failed to load: {0}")]
Doc(#[from] DocError),
Doc(#[from] IdentityDocError),
/// Identity document is missing.
#[error("missing identity document")]
MissingIdentity,
@@ -4392,4 +4392,43 @@ mod test {
})
);
}
#[test]
fn test_patch_forged_identity_resource() {
let alice = test::setup::NodeWithRepo::default();
let eve = SigningKey::mock(31);
let assignee = SigningKey::mock(42).did();
let checkout = alice.repo.checkout();
let branch = checkout.branch_with([("README", b"Hello World!")]);
let mut patches = Cache::no_cache(&*alice.repo, &alice.signer).unwrap();
let id = patches
.create(
cob::Title::new("My first patch").unwrap(),
"Blah blah blah.",
MergeTarget::Delegates,
branch.base,
branch.oid,
&[],
)
.unwrap()
.id;
let assign = Action::Assign {
assignees: BTreeSet::from([assignee]),
};
// Eve is not a delegate, so the real identity does not allow the action.
let identity = alice.repo.identity_head().unwrap();
cob::test::store_change::<Patch>(&alice.repo, &eve, identity, &id, &assign);
let patch = patches.get(&id).unwrap().unwrap();
assert_eq!(patch.assignees().count(), 0);
// A forged identity document that names Eve as a delegate must not
// allow the action either.
let mut doc = alice.repo.identity_doc().unwrap().doc.edit();
doc.delegate(eve.did());
let forged = cob::test::forge_identity(alice.repo.raw(), &doc.verified().unwrap());
cob::test::store_change::<Patch>(&alice.repo, &eve, forged, &id, &assign);
let patch = patches.get(&id).unwrap().unwrap();
assert_eq!(patch.assignees().count(), 0);
}
}
+50
View File
@@ -239,3 +239,53 @@ fn encoded<T: Cob>(
(data, hash.into())
}
/// Write a commit that holds `doc` as `embeds/radicle.json`.
/// The commit is not a revision of the repository identity.
pub fn forge_identity(repo: &git::raw::Repository, doc: &crate::identity::Doc) -> Oid {
let (_, bytes) = doc.encode().unwrap();
let blob = repo.blob(&bytes).unwrap();
let mut embeds = repo.treebuilder(None).unwrap();
embeds
.insert(*crate::identity::doc::PATH, blob, 0o100644)
.unwrap();
let embeds = embeds.write().unwrap();
let mut tree = repo.treebuilder(None).unwrap();
tree.insert("embeds", embeds, 0o040000).unwrap();
let tree = repo.find_tree(tree.write().unwrap()).unwrap();
let sig =
git::raw::Signature::new("forger", "forger@localhost", &git::raw::Time::new(0, 0)).unwrap();
repo.commit(None, &sig, &sig, "Forged identity", &tree, &[])
.unwrap()
.into()
}
/// Store a change by `signer` on the object `id`, with `resource` as its
/// identity commit. Then point the ref of `signer` for the object at it.
pub fn store_change<T: CobWithType>(
repo: &crate::storage::git::Repository,
signer: &impl crypto::Signer,
resource: Oid,
id: &crate::cob::ObjectId,
action: &impl Serialize,
) {
let contents = NonEmpty::new(encoding::encode(action).unwrap());
let entry = crate::cob::change::Storage::store(
repo,
Some(resource),
vec![],
signer,
crate::cob::change::Template {
type_name: T::type_name().clone(),
tips: vec![**id],
message: "Store change".to_owned(),
embeds: vec![],
contents,
},
)
.unwrap();
crate::cob::object::Storage::update(repo, signer.public_key(), T::type_name(), id, &entry.id)
.unwrap();
}
+27 -3
View File
@@ -155,6 +155,25 @@ pub enum RepositoryError {
CanonicalRefs(#[from] doc::CanonicalRefsError),
#[error(transparent)]
FindObjects(#[from] canonical::effects::FindObjectsError),
#[error(transparent)]
IdentityDoc(#[from] IdentityDocError),
}
/// Error looking up an accepted repository identity document.
#[derive(Error, Debug)]
pub enum IdentityDocError {
#[error(transparent)]
Repository(Box<RepositoryError>),
#[error(transparent)]
Doc(#[from] DocError),
#[error("commit {0} is not an accepted revision of the repository identity")]
UnacceptedRevision(Oid),
}
impl From<RepositoryError> for IdentityDocError {
fn from(err: RepositoryError) -> Self {
Self::Repository(Box::new(err))
}
}
impl From<Error> for RepositoryError {
@@ -562,7 +581,8 @@ pub trait ReadRepository: Sized + ValidateRepository {
/// Compute the canonical identity document.
fn canonical_identity_doc(&self) -> Result<DocAt, RepositoryError> {
let head = self.canonical_identity_head()?;
let doc = self.identity_doc_at(head)?;
// The computed head is authoritative and may be ahead of cached `rad/id`.
let doc = Doc::load_at(head, self)?;
Ok(doc)
}
@@ -618,7 +638,7 @@ pub trait ReadRepository: Sized + ValidateRepository {
}
/// Get the repository's identity document.
fn identity_doc(&self) -> Result<DocAt, RepositoryError> {
fn identity_doc(&self) -> Result<DocAt, IdentityDocError> {
let head = self.identity_head()?;
let doc = self.identity_doc_at(head)?;
@@ -626,7 +646,11 @@ pub trait ReadRepository: Sized + ValidateRepository {
}
/// Get the repository's identity document at a specific commit.
fn identity_doc_at(&self, head: Oid) -> Result<DocAt, DocError>;
///
/// The commit must be an accepted revision of the evaluated [`Identity`] COB.
/// Returns [`IdentityDocError::UnacceptedRevision`] otherwise.
/// For document loading without this check, use [`Doc::load_at`].
fn identity_doc_at(&self, head: Oid) -> Result<DocAt, IdentityDocError>;
/// Get the merge base of two commits.
fn merge_base(&self, left: &Oid, right: &Oid) -> Result<Oid, crate::git::raw::Error>;
+10 -10
View File
@@ -18,8 +18,8 @@ use crate::identity::{Doc, DocAt, RepoId};
use crate::identity::{Identity, Project};
use crate::storage::refs::{FeatureLevel, Refs, SignedRefs};
use crate::storage::{
ReadRepository, ReadStorage, Remote, Remotes, RepositoryInfo, SetHead, SignRepository,
WriteRepository, WriteStorage,
IdentityDocError, ReadRepository, ReadStorage, Remote, Remotes, RepositoryInfo, SetHead,
SignRepository, WriteRepository, WriteStorage,
};
use crate::storage::{SignedRefsInfo, refs};
use crate::{git, git::Oid, node};
@@ -572,12 +572,8 @@ impl Repository {
/// Get the canonical project information.
#[deprecated(note = "Use `Doc::project()` on `Self::identity_doc()`.")]
pub fn project(&self) -> Result<Option<Project>, RepositoryError> {
self.identity_doc().and_then(|identity| {
identity
.project()
.transpose()
.map_err(RepositoryError::Payload)
})
let identity = self.identity_doc()?;
Ok(identity.project().transpose()?)
}
pub fn identity_doc_of(&self, remote: &RemoteId) -> Result<Doc, DocError> {
@@ -836,8 +832,12 @@ impl ReadRepository for Repository {
Ok(refs)
}
fn identity_doc_at(&self, head: Oid) -> Result<DocAt, DocError> {
Doc::load_at(head, self)
fn identity_doc_at(&self, head: Oid) -> Result<DocAt, IdentityDocError> {
let identity = Identity::load(self)?;
if !identity.revision(&head).is_some_and(|r| r.is_accepted()) {
return Err(IdentityDocError::UnacceptedRevision(head));
}
Ok(Doc::load_at(head, self)?)
}
fn head(&self) -> Result<(Qualified<'_>, Oid), RepositoryError> {
+3 -3
View File
@@ -14,10 +14,10 @@ use crate::git;
use crate::git::fmt::*;
use crate::git::*;
use crate::identity;
use crate::identity::doc::DocError;
use crate::node::NodeId;
use crate::storage;
use crate::storage::Error;
use crate::storage::IdentityDocError;
use crate::storage::{
ReadRepository,
git::{Remote, Remotes, Validations},
@@ -364,11 +364,11 @@ impl<R: storage::ReadRepository> ReadRepository for DraftStore<'_, R> {
self.repo.references_glob(pattern)
}
fn identity_doc(&self) -> Result<crate::identity::DocAt, RepositoryError> {
fn identity_doc(&self) -> Result<crate::identity::DocAt, IdentityDocError> {
self.repo.identity_doc()
}
fn identity_doc_at(&self, head: Oid) -> Result<crate::identity::DocAt, DocError> {
fn identity_doc_at(&self, head: Oid) -> Result<crate::identity::DocAt, IdentityDocError> {
self.repo.identity_doc_at(head)
}
+3 -3
View File
@@ -10,7 +10,7 @@ use crate::crypto::ExtendedSignature;
pub use crate::git;
use crate::git::fmt;
use crate::identity::doc::{Doc, DocAt, DocError, RawDoc, RepoId};
use crate::identity::doc::{Doc, DocAt, RawDoc, RepoId};
use crate::node::NodeId;
pub use crate::storage::*;
@@ -307,11 +307,11 @@ impl ReadRepository for MockRepository {
todo!()
}
fn identity_doc(&self) -> Result<crate::identity::DocAt, RepositoryError> {
fn identity_doc(&self) -> Result<crate::identity::DocAt, IdentityDocError> {
Ok(self.doc.clone())
}
fn identity_doc_at(&self, _head: Oid) -> Result<crate::identity::DocAt, DocError> {
fn identity_doc_at(&self, _head: Oid) -> Result<crate::identity::DocAt, IdentityDocError> {
Ok(self.doc.clone())
}