From 2ad0eff31ce37117b50f78b58292a87eda8c8bbf Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 21 Jan 2025 12:58:13 +0100 Subject: [PATCH 1/2] Clippy fixes, add cargo shear --- .woodpecker.yml | 9 +++++++++ Cargo.toml | 1 - src/activity_sending.rs | 4 ++-- src/fetch/object_id.rs | 1 + src/http_signatures.rs | 2 ++ 5 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.woodpecker.yml b/.woodpecker.yml index b82dd94..7781a05 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -9,6 +9,15 @@ steps: when: - event: pull_request + cargo_shear: + image: *rust_nightly_image + commands: + - *install_binstall + - cargo binstall -y cargo-shear + - cargo shear + when: + - event: pull_request + cargo_clippy: image: *rust_image environment: diff --git a/Cargo.toml b/Cargo.toml index 5ac2d79..53698d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,7 +93,6 @@ tower = { version = "0.4.13", optional = true } [dev-dependencies] anyhow = "1.0.86" axum = { version = "0.7.5", features = ["macros"] } -axum-extra = { version = "0.9.3", features = ["typed-header"] } env_logger = "0.11.3" tokio = { version = "1.38.0", features = ["full"] } diff --git a/src/activity_sending.rs b/src/activity_sending.rs index 1c84757..4971485 100644 --- a/src/activity_sending.rs +++ b/src/activity_sending.rs @@ -136,8 +136,8 @@ impl SendActivityTask { } } -pub(crate) async fn build_tasks<'a, Activity, Datatype, ActorType>( - activity: &'a Activity, +pub(crate) async fn build_tasks( + activity: &Activity, actor: &ActorType, inboxes: Vec, data: &Data, diff --git a/src/fetch/object_id.rs b/src/fetch/object_id.rs index 108dc1a..b6aafc6 100644 --- a/src/fetch/object_id.rs +++ b/src/fetch/object_id.rs @@ -360,6 +360,7 @@ const _IMPL_DIESEL_NEW_TYPE_FOR_OBJECT_ID: () = { } }; +/// Internal only #[cfg(test)] #[allow(clippy::unwrap_used)] pub mod tests { diff --git a/src/http_signatures.rs b/src/http_signatures.rs index aa526f9..f30e75c 100644 --- a/src/http_signatures.rs +++ b/src/http_signatures.rs @@ -277,6 +277,7 @@ pub(crate) fn verify_body_hash( Ok(()) } +/// Internal only #[cfg(test)] #[allow(clippy::unwrap_used)] pub mod test { @@ -378,6 +379,7 @@ pub mod test { assert_eq!(invalid, Err(Error::ActivityBodyDigestInvalid)); } + /// Internal only, return hardcoded keypair for testing pub fn test_keypair() -> Keypair { let rsa = RsaPrivateKey::from_pkcs1_pem(PRIVATE_KEY).unwrap(); let pkey = RsaPublicKey::from(&rsa); From 2d90dad9f76ee87e546d4ff0a5a1fda1e22b93e2 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Tue, 21 Jan 2025 11:59:35 +0000 Subject: [PATCH 2/2] Add verify_is_remote_object (#136) * Add verify_is_remote_object * doctest --- src/protocol/verification.rs | 38 +++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/protocol/verification.rs b/src/protocol/verification.rs index 18595b9..c6c342a 100644 --- a/src/protocol/verification.rs +++ b/src/protocol/verification.rs @@ -1,6 +1,7 @@ //! Verify that received data is valid -use crate::error::Error; +use crate::{config::Data, error::Error, fetch::object_id::ObjectId, traits::Object}; +use serde::Deserialize; use url::Url; /// Check that both urls have the same domain. If not, return UrlVerificationError. @@ -36,3 +37,38 @@ pub fn verify_urls_match(a: &Url, b: &Url) -> Result<(), Error> { } Ok(()) } + +/// Check that the given ID doesn't match the local domain. +/// +/// It is important to verify this to avoid local objects from being overwritten. In general +/// locally created objects should be considered authorative, while incoming federated data +/// is untrusted. Lack of such a check could allow an attacker to rewrite local posts. It could +/// also result in an `object.local` field being overwritten with `false` for local objects, resulting in invalid data. +/// +/// ``` +/// # use activitypub_federation::fetch::object_id::ObjectId; +/// # use activitypub_federation::config::FederationConfig; +/// # use activitypub_federation::protocol::verification::verify_is_remote_object; +/// # use activitypub_federation::traits::tests::{DbConnection, DbUser}; +/// # tokio::runtime::Runtime::new().unwrap().block_on(async { +/// # let config = FederationConfig::builder().domain("example.com").app_data(DbConnection).build().await?; +/// # let data = config.to_request_data(); +/// let id = ObjectId::::parse("https://remote.com/u/name")?; +/// assert!(verify_is_remote_object(&id, &data).is_ok()); +/// # Ok::<(), anyhow::Error>(()) +/// # }).unwrap(); +/// ``` +pub fn verify_is_remote_object( + id: &ObjectId, + data: &Data<::DataType>, +) -> Result<(), Error> +where + Kind: Object + Send + 'static, + for<'de2> ::Kind: Deserialize<'de2>, +{ + if id.is_local(data) { + Err(Error::UrlVerificationError("Object is not remote")) + } else { + Ok(()) + } +}