From 6b3a4f8942dff378a4c7fa4a9d7af24fdbc1f310 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Thu, 9 Mar 2023 22:09:44 +0100 Subject: [PATCH] Add verify methods back in, some more fixes (#28) --- Cargo.lock | 2 +- Cargo.toml | 2 +- docs/08_receiving_activities.md | 4 + docs/10_fetching_objects_with_unknown_type.md | 4 + .../live_federation/activities/create_post.rs | 5 ++ examples/live_federation/objects/person.rs | 17 +++- examples/live_federation/objects/post.rs | 11 ++- .../local_federation/activities/accept.rs | 4 + .../activities/create_post.rs | 5 ++ .../local_federation/activities/follow.rs | 4 + examples/local_federation/objects/person.rs | 17 +++- examples/local_federation/objects/post.rs | 11 ++- src/actix_web/inbox.rs | 3 +- src/axum/inbox.rs | 4 +- src/fetch/object_id.rs | 1 + src/protocol/context.rs | 6 +- src/traits.rs | 82 +++++++++++++++++-- 17 files changed, 164 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9821225..c373b81 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 3 [[package]] name = "activitypub_federation" -version = "0.3.4" +version = "0.4.0-rc1" dependencies = [ "activitystreams-kinds", "actix-rt", diff --git a/Cargo.toml b/Cargo.toml index dfb13a6..fb02fd4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "activitypub_federation" -version = "0.4.0-rc1" +version = "0.4.0-rc2" edition = "2021" description = "High-level Activitypub framework" keywords = ["activitypub", "activitystreams", "federation", "fediverse"] diff --git a/docs/08_receiving_activities.md b/docs/08_receiving_activities.md index 81d698e..47e5482 100644 --- a/docs/08_receiving_activities.md +++ b/docs/08_receiving_activities.md @@ -36,6 +36,10 @@ impl ActivityHandler for Follow { fn actor(&self) -> &Url { self.actor.inner() } + + async fn verify(&self, _data: &RequestData) -> Result<(), Self::Error> { + Ok(()) + } async fn receive(self, data: &RequestData) -> Result<(), Self::Error> { let actor = self.actor.dereference(data).await?; diff --git a/docs/10_fetching_objects_with_unknown_type.md b/docs/10_fetching_objects_with_unknown_type.md index 65ee7ad..e5bf4d0 100644 --- a/docs/10_fetching_objects_with_unknown_type.md +++ b/docs/10_fetching_objects_with_unknown_type.md @@ -44,6 +44,10 @@ impl ApubObject for SearchableDbObjects { ) -> Result { unimplemented!(); } + + async fn verify(apub: &Self::ApubType, expected_domain: &Url, _data: &RequestData) -> Result<(), Self::Error> { + Ok(()) + } async fn from_apub( apub: Self::ApubType, diff --git a/examples/live_federation/activities/create_post.rs b/examples/live_federation/activities/create_post.rs index e87f7f7..7bac286 100644 --- a/examples/live_federation/activities/create_post.rs +++ b/examples/live_federation/activities/create_post.rs @@ -65,6 +65,11 @@ impl ActivityHandler for CreatePost { self.actor.inner() } + async fn verify(&self, data: &RequestData) -> Result<(), Self::Error> { + DbPost::verify(&self.object, &self.id, data).await?; + Ok(()) + } + async fn receive(self, data: &RequestData) -> Result<(), Self::Error> { DbPost::from_apub(self.object, data).await?; Ok(()) diff --git a/examples/live_federation/objects/person.rs b/examples/live_federation/objects/person.rs index 545a504..786ce5d 100644 --- a/examples/live_federation/objects/person.rs +++ b/examples/live_federation/objects/person.rs @@ -4,7 +4,7 @@ use activitypub_federation::{ fetch::object_id::ObjectId, http_signatures::generate_actor_keypair, kinds::actor::PersonType, - protocol::public_key::PublicKey, + protocol::{public_key::PublicKey, verification::verify_domains_match}, traits::{ActivityHandler, Actor, ApubObject}, }; use chrono::{Local, NaiveDateTime}; @@ -99,6 +99,15 @@ impl ApubObject for DbUser { }) } + async fn verify( + apub: &Self::ApubType, + expected_domain: &Url, + _data: &RequestData, + ) -> Result<(), Self::Error> { + verify_domains_match(apub.id.inner(), expected_domain)?; + Ok(()) + } + async fn from_apub( apub: Self::ApubType, _data: &RequestData, @@ -117,11 +126,15 @@ impl ApubObject for DbUser { } impl Actor for DbUser { - fn public_key(&self) -> &str { + fn public_key_pem(&self) -> &str { &self.public_key } fn inbox(&self) -> Url { self.inbox.clone() } + + fn id(&self) -> &Url { + self.ap_id.inner() + } } diff --git a/examples/live_federation/objects/post.rs b/examples/live_federation/objects/post.rs index 783b725..825207d 100644 --- a/examples/live_federation/objects/post.rs +++ b/examples/live_federation/objects/post.rs @@ -9,7 +9,7 @@ use activitypub_federation::{ config::RequestData, fetch::object_id::ObjectId, kinds::{object::NoteType, public}, - protocol::helpers::deserialize_one_or_many, + protocol::{helpers::deserialize_one_or_many, verification::verify_domains_match}, traits::{Actor, ApubObject}, }; use activitystreams_kinds::link::MentionType; @@ -65,6 +65,15 @@ impl ApubObject for DbPost { unimplemented!() } + async fn verify( + apub: &Self::ApubType, + expected_domain: &Url, + _data: &RequestData, + ) -> Result<(), Self::Error> { + verify_domains_match(apub.id.inner(), expected_domain)?; + Ok(()) + } + async fn from_apub( apub: Self::ApubType, data: &RequestData, diff --git a/examples/local_federation/activities/accept.rs b/examples/local_federation/activities/accept.rs index 25acf15..3e2add0 100644 --- a/examples/local_federation/activities/accept.rs +++ b/examples/local_federation/activities/accept.rs @@ -42,6 +42,10 @@ impl ActivityHandler for Accept { self.actor.inner() } + async fn verify(&self, _data: &RequestData) -> Result<(), Self::Error> { + Ok(()) + } + async fn receive(self, _data: &RequestData) -> Result<(), Self::Error> { Ok(()) } diff --git a/examples/local_federation/activities/create_post.rs b/examples/local_federation/activities/create_post.rs index 547c723..5ca0a7f 100644 --- a/examples/local_federation/activities/create_post.rs +++ b/examples/local_federation/activities/create_post.rs @@ -50,6 +50,11 @@ impl ActivityHandler for CreatePost { self.actor.inner() } + async fn verify(&self, data: &RequestData) -> Result<(), Self::Error> { + DbPost::verify(&self.object, &self.id, data).await?; + Ok(()) + } + async fn receive(self, data: &RequestData) -> Result<(), Self::Error> { DbPost::from_apub(self.object, data).await?; Ok(()) diff --git a/examples/local_federation/activities/follow.rs b/examples/local_federation/activities/follow.rs index aca8760..b5cd435 100644 --- a/examples/local_federation/activities/follow.rs +++ b/examples/local_federation/activities/follow.rs @@ -47,6 +47,10 @@ impl ActivityHandler for Follow { self.actor.inner() } + async fn verify(&self, _data: &RequestData) -> Result<(), Self::Error> { + Ok(()) + } + // Ignore clippy false positive: https://github.com/rust-lang/rust-clippy/issues/6446 #[allow(clippy::await_holding_lock)] async fn receive(self, data: &RequestData) -> Result<(), Self::Error> { diff --git a/examples/local_federation/objects/person.rs b/examples/local_federation/objects/person.rs index 8e1c7a1..2b13786 100644 --- a/examples/local_federation/objects/person.rs +++ b/examples/local_federation/objects/person.rs @@ -11,7 +11,7 @@ use activitypub_federation::{ fetch::{object_id::ObjectId, webfinger::webfinger_resolve_actor}, http_signatures::generate_actor_keypair, kinds::actor::PersonType, - protocol::{context::WithContext, public_key::PublicKey}, + protocol::{context::WithContext, public_key::PublicKey, verification::verify_domains_match}, traits::{ActivityHandler, Actor, ApubObject}, }; use chrono::{Local, NaiveDateTime}; @@ -171,6 +171,15 @@ impl ApubObject for DbUser { }) } + async fn verify( + apub: &Self::ApubType, + expected_domain: &Url, + _data: &RequestData, + ) -> Result<(), Self::Error> { + verify_domains_match(apub.id.inner(), expected_domain)?; + Ok(()) + } + async fn from_apub( apub: Self::ApubType, data: &RequestData, @@ -192,7 +201,11 @@ impl ApubObject for DbUser { } impl Actor for DbUser { - fn public_key(&self) -> &str { + fn id(&self) -> &Url { + self.ap_id.inner() + } + + fn public_key_pem(&self) -> &str { &self.public_key } diff --git a/examples/local_federation/objects/post.rs b/examples/local_federation/objects/post.rs index 601fe8b..07d7555 100644 --- a/examples/local_federation/objects/post.rs +++ b/examples/local_federation/objects/post.rs @@ -3,7 +3,7 @@ use activitypub_federation::{ config::RequestData, fetch::object_id::ObjectId, kinds::{object::NoteType, public}, - protocol::helpers::deserialize_one_or_many, + protocol::{helpers::deserialize_one_or_many, verification::verify_domains_match}, traits::ApubObject, }; use serde::{Deserialize, Serialize}; @@ -73,6 +73,15 @@ impl ApubObject for DbPost { }) } + async fn verify( + apub: &Self::ApubType, + expected_domain: &Url, + _data: &RequestData, + ) -> Result<(), Self::Error> { + verify_domains_match(apub.id.inner(), expected_domain)?; + Ok(()) + } + async fn from_apub( apub: Self::ApubType, data: &RequestData, diff --git a/src/actix_web/inbox.rs b/src/actix_web/inbox.rs index 804cb1d..fdcce81 100644 --- a/src/actix_web/inbox.rs +++ b/src/actix_web/inbox.rs @@ -42,10 +42,11 @@ where request.headers(), request.method(), request.uri(), - actor.public_key(), + actor.public_key_pem(), )?; debug!("Receiving activity {}", activity.id().to_string()); + activity.verify(data).await?; activity.receive(data).await?; Ok(HttpResponse::Ok().finish()) } diff --git a/src/axum/inbox.rs b/src/axum/inbox.rs index 92493c0..03362c0 100644 --- a/src/axum/inbox.rs +++ b/src/axum/inbox.rs @@ -44,15 +44,15 @@ where .dereference(data) .await?; - // TODO: why do errors here not get returned over http? verify_signature( &activity_data.headers, &activity_data.method, &activity_data.uri, - actor.public_key(), + actor.public_key_pem(), )?; debug!("Receiving activity {}", activity.id().to_string()); + activity.verify(data).await?; activity.receive(data).await?; Ok(()) } diff --git a/src/fetch/object_id.rs b/src/fetch/object_id.rs index a50c8db..af9b4fa 100644 --- a/src/fetch/object_id.rs +++ b/src/fetch/object_id.rs @@ -148,6 +148,7 @@ where let res2 = res?; + Kind::verify(&res2, self.inner(), data).await?; Kind::from_apub(res2, data).await } } diff --git a/src/protocol/context.rs b/src/protocol/context.rs index d71cb7b..692e7d0 100644 --- a/src/protocol/context.rs +++ b/src/protocol/context.rs @@ -62,7 +62,7 @@ impl WithContext { #[async_trait::async_trait] impl ActivityHandler for WithContext where - T: ActivityHandler + Send, + T: ActivityHandler + Send + Sync, { type DataType = ::DataType; type Error = ::Error; @@ -75,6 +75,10 @@ where self.inner.actor() } + async fn verify(&self, data: &RequestData) -> Result<(), Self::Error> { + self.inner.verify(data).await + } + async fn receive(self, data: &RequestData) -> Result<(), Self::Error> { self.inner.receive(data).await } diff --git a/src/traits.rs b/src/traits.rs index 4473925..4744632 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -1,6 +1,6 @@ //! Traits which need to be implemented for federated data types -use crate::config::RequestData; +use crate::{config::RequestData, protocol::public_key::PublicKey}; use async_trait::async_trait; use chrono::NaiveDateTime; use std::ops::Deref; @@ -13,6 +13,7 @@ use url::Url; /// # use url::Url; /// # use activitypub_federation::protocol::public_key::PublicKey; /// # use activitypub_federation::config::RequestData; +/// use activitypub_federation::protocol::verification::verify_domains_match; /// # use activitypub_federation::traits::ApubObject; /// # use activitypub_federation::traits::tests::{DbConnection, Person}; /// # pub struct DbUser { @@ -53,7 +54,13 @@ use url::Url; /// }) /// } /// -/// async fn from_apub(apub: Self::ApubType, data: &RequestData) -> Result { +/// async fn verify(apub: &Self::ApubType, expected_domain: &Url, data: &RequestData,) -> Result<(), Self::Error> { +/// verify_domains_match(apub.id.inner(), expected_domain)?; +/// // additional application specific checks +/// Ok(()) +/// } +/// +/// async fn from_apub(apub: Self::ApubType, data: &RequestData) -> Result { /// // Called when a remote object gets received over Activitypub. Validate and insert it /// // into the database. /// @@ -124,6 +131,19 @@ pub trait ApubObject: Sized { data: &RequestData, ) -> Result; + /// Verifies that the received object is valid. + /// + /// You should check here that the domain of id matches `expected_domain`. Additionally you + /// should perform any application specific checks. + /// + /// It is necessary to use a separate method for this, because it might be used for activities + /// like `Delete/Note`, which shouldn't perform any database write for the inner `Note`. + async fn verify( + apub: &Self::ApubType, + expected_domain: &Url, + data: &RequestData, + ) -> Result<(), Self::Error>; + /// Convert object from ActivityPub type to database type. /// /// Called when an object is received from HTTP fetch or as part of an activity. This method @@ -166,6 +186,10 @@ pub trait ApubObject: Sized { /// self.actor.inner() /// } /// +/// async fn verify(&self, data: &RequestData) -> Result<(), Self::Error> { +/// Ok(()) +/// } +/// /// async fn receive(self, data: &RequestData) -> Result<(), Self::Error> { /// let local_user = self.object.dereference(data).await?; /// let follower = self.actor.dereference(data).await?; @@ -189,6 +213,12 @@ pub trait ActivityHandler { /// `actor` field of activity fn actor(&self) -> &Url; + /// Verifies that the received activity is valid. + /// + /// This needs to be a separate method, because it might be used for activities + /// like `Undo/Follow`, which shouldn't perform any database write for the inner `Follow`. + async fn verify(&self, data: &RequestData) -> Result<(), Self::Error>; + /// Called when an activity is received. /// /// Should perform validation and possibly write action to the database. In case the activity @@ -198,12 +228,23 @@ pub trait ActivityHandler { /// Trait to allow retrieving common Actor data. pub trait Actor: ApubObject { + /// `id` field of the actor + fn id(&self) -> &Url; + /// The actor's public key for verifying signatures of incoming activities. - fn public_key(&self) -> &str; + /// + /// Use [generate_actor_keypair](crate::http_signatures::generate_actor_keypair) to create the + /// actor keypair. + fn public_key_pem(&self) -> &str; /// The inbox where activities for this user should be sent to fn inbox(&self) -> Url; + /// Generates a public key struct for use in the actor json representation + fn public_key(&self) -> PublicKey { + PublicKey::new(self.id().clone(), self.public_key_pem().to_string()) + } + /// The actor's shared inbox, if any fn shared_inbox(&self) -> Option { None @@ -219,7 +260,7 @@ pub trait Actor: ApubObject { #[async_trait] impl ActivityHandler for Box where - T: ActivityHandler + Send, + T: ActivityHandler + Send + Sync, { type DataType = T::DataType; type Error = T::Error; @@ -232,6 +273,10 @@ where self.deref().actor() } + async fn verify(&self, data: &RequestData) -> Result<(), Self::Error> { + (*self).verify(data).await + } + async fn receive(self, data: &RequestData) -> Result<(), Self::Error> { (*self).receive(data).await } @@ -247,7 +292,7 @@ pub mod tests { use crate::{ fetch::object_id::ObjectId, http_signatures::{generate_actor_keypair, Keypair}, - protocol::public_key::PublicKey, + protocol::{public_key::PublicKey, verification::verify_domains_match}, }; use activitystreams_kinds::{activity::FollowType, actor::PersonType}; use anyhow::Error; @@ -333,6 +378,15 @@ pub mod tests { }) } + async fn verify( + apub: &Self::ApubType, + expected_domain: &Url, + _data: &RequestData, + ) -> Result<(), Self::Error> { + verify_domains_match(apub.id.inner(), expected_domain)?; + Ok(()) + } + async fn from_apub( apub: Self::ApubType, _data: &RequestData, @@ -350,7 +404,11 @@ pub mod tests { } impl Actor for DbUser { - fn public_key(&self) -> &str { + fn id(&self) -> &Url { + &self.apub_id + } + + fn public_key_pem(&self) -> &str { &self.public_key } @@ -382,6 +440,10 @@ pub mod tests { self.actor.inner() } + async fn verify(&self, _: &RequestData) -> Result<(), Self::Error> { + Ok(()) + } + async fn receive(self, _data: &RequestData) -> Result<(), Self::Error> { Ok(()) } @@ -413,6 +475,14 @@ pub mod tests { todo!() } + async fn verify( + _: &Self::ApubType, + _: &Url, + _: &RequestData, + ) -> Result<(), Self::Error> { + todo!() + } + async fn from_apub( _: Self::ApubType, _: &RequestData,