From 8910550663c980fbe0a3566307a61eefb41e97d2 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 3 Jan 2025 18:41:20 +0000 Subject: [PATCH 1/9] Upgrade deps (#133) * Upgrade deps * fmt * fix --- Cargo.toml | 62 +++++++++++++------------- examples/local_federation/axum/http.rs | 4 +- src/actix_web/inbox.rs | 8 ++-- src/axum/inbox.rs | 2 - src/axum/middleware.rs | 3 +- src/error.rs | 9 +++- src/lib.rs | 4 +- 7 files changed, 47 insertions(+), 45 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5ac2d79..e08c794 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,70 +32,70 @@ redundant_closure_for_method_calls = "deny" unwrap_used = "deny" [dependencies] -chrono = { version = "0.4.38", features = ["clock"], default-features = false } -serde = { version = "1.0.204", features = ["derive"] } -async-trait = "0.1.81" -url = { version = "2.5.2", features = ["serde"] } -serde_json = { version = "1.0.120", features = ["preserve_order"] } -reqwest = { version = "0.12.5", default-features = false, features = [ +chrono = { version = "0.4.39", features = ["clock"], default-features = false } +serde = { version = "1.0.217", features = ["derive"] } +async-trait = "0.1.84" +url = { version = "2.5.4", features = ["serde"] } +serde_json = { version = "1.0.134", features = ["preserve_order"] } +reqwest = { version = "0.12.12", default-features = false, features = [ "json", "stream", "rustls-tls", ] } -reqwest-middleware = "0.3.2" -tracing = "0.1.40" +reqwest-middleware = "0.4.0" +tracing = "0.1.41" base64 = "0.22.1" rand = "0.8.5" -rsa = "0.9.6" -once_cell = "1.19.0" -http = "1.1.0" +rsa = "0.9.7" +once_cell = "1.20.2" +http = "1.2.0" sha2 = { version = "0.10.8", features = ["oid"] } -thiserror = "1.0.62" -derive_builder = "0.20.0" -itertools = "0.13.0" +thiserror = "2.0.9" +derive_builder = "0.20.2" +itertools = "0.14.0" dyn-clone = "1.0.17" enum_delegate = "0.2.0" httpdate = "1.0.3" -http-signature-normalization-reqwest = { version = "0.12.0", default-features = false, features = [ +http-signature-normalization-reqwest = { version = "0.13.0", default-features = false, features = [ "sha-2", "middleware", "default-spawner", ] } http-signature-normalization = "0.7.0" -bytes = "1.6.1" -futures-core = { version = "0.3.30", default-features = false } -pin-project-lite = "0.2.14" +bytes = "1.9.0" +futures-core = { version = "0.3.31", default-features = false } +pin-project-lite = "0.2.15" activitystreams-kinds = "0.3.0" -regex = { version = "1.10.5", default-features = false, features = [ +regex = { version = "1.11.1", default-features = false, features = [ "std", "unicode", ] } -tokio = { version = "1.38.0", features = [ +tokio = { version = "1.42.0", features = [ "sync", "rt", "rt-multi-thread", "time", ] } -diesel = { version = "2.2.1", features = [ +diesel = { version = "2.2.6", features = [ "postgres", ], default-features = false, optional = true } -futures = "0.3.30" -moka = { version = "0.12.8", features = ["future"] } +futures = "0.3.31" +moka = { version = "0.12.9", features = ["future"] } # Actix-web -actix-web = { version = "4.8.0", default-features = false, optional = true } +actix-web = { version = "4.9.0", default-features = false, optional = true } http02 = { package = "http", version = "0.2.12", optional = true } # Axum -axum = { version = "0.7.5", features = ["json"], default-features = false, optional = true } -tower = { version = "0.4.13", optional = true } +axum = { version = "0.8.1", features = ["json"], default-features = false, optional = true } +tower = { version = "0.5.2", 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"] } +anyhow = "1.0.95" +axum = { version = "0.8.1", features = ["macros"] } +axum-extra = { version = "0.10.0", features = ["typed-header"] } +env_logger = "0.11.6" +tokio = { version = "1.42.0", features = ["full"] } [profile.dev] strip = "symbols" diff --git a/examples/local_federation/axum/http.rs b/examples/local_federation/axum/http.rs index dd9d002..e010a73 100644 --- a/examples/local_federation/axum/http.rs +++ b/examples/local_federation/axum/http.rs @@ -30,8 +30,8 @@ pub fn listen(config: &FederationConfig) -> Result<(), Error> { info!("Listening with axum on {hostname}"); let config = config.clone(); let app = Router::new() - .route("/:user/inbox", post(http_post_user_inbox)) - .route("/:user", get(http_get_user)) + .route("/{user}/inbox", post(http_post_user_inbox)) + .route("/{user}", get(http_get_user)) .route("/.well-known/webfinger", get(webfinger)) .layer(FederationMiddleware::new(config)); diff --git a/src/actix_web/inbox.rs b/src/actix_web/inbox.rs index 9bce475..0912e72 100644 --- a/src/actix_web/inbox.rs +++ b/src/actix_web/inbox.rs @@ -122,14 +122,14 @@ mod test { let (_, _, config) = setup_receive_test().await; let actor = Url::parse("http://ds9.lemmy.ml/u/lemmy_alpha").unwrap(); - let id = "http://localhost:123/1"; + let activity_id = "http://localhost:123/1"; let activity = json!({ "actor": actor.as_str(), "to": ["https://www.w3.org/ns/activitystreams#Public"], "object": "http://ds9.lemmy.ml/post/1", "cc": ["http://enterprise.lemmy.ml/c/main"], "type": "Delete", - "id": id + "id": activity_id } ); let body: Bytes = serde_json::to_vec(&activity).unwrap().into(); @@ -144,8 +144,8 @@ mod test { .await; match res { - Err(Error::ParseReceivedActivity(_, url)) => { - assert_eq!(id, url.expect("has url").as_str()); + Err(Error::ParseReceivedActivity { err: _, id }) => { + assert_eq!(activity_id, id.expect("has url").as_str()); } _ => unreachable!(), } diff --git a/src/axum/inbox.rs b/src/axum/inbox.rs index 1767c10..84567e0 100644 --- a/src/axum/inbox.rs +++ b/src/axum/inbox.rs @@ -10,7 +10,6 @@ use crate::{ traits::{ActivityHandler, Actor, Object}, }; use axum::{ - async_trait, body::Body, extract::FromRequest, http::{Request, StatusCode}, @@ -58,7 +57,6 @@ pub struct ActivityData { body: Vec, } -#[async_trait] impl FromRequest for ActivityData where S: Send + Sync, diff --git a/src/axum/middleware.rs b/src/axum/middleware.rs index 290dc94..2f8b0e9 100644 --- a/src/axum/middleware.rs +++ b/src/axum/middleware.rs @@ -1,5 +1,5 @@ use crate::config::{Data, FederationConfig, FederationMiddleware}; -use axum::{async_trait, body::Body, extract::FromRequestParts, http::Request, response::Response}; +use axum::{body::Body, extract::FromRequestParts, http::Request, response::Response}; use http::{request::Parts, StatusCode}; use std::task::{Context, Poll}; use tower::{Layer, Service}; @@ -43,7 +43,6 @@ where } } -#[async_trait] impl FromRequestParts for Data where S: Send + Sync, diff --git a/src/error.rs b/src/error.rs index 1866e48..06c8085 100644 --- a/src/error.rs +++ b/src/error.rs @@ -44,11 +44,16 @@ pub enum Error { #[error("Failed to parse object {1} with content {2}: {0}")] ParseFetchedObject(serde_json::Error, Url, String), /// Failed to parse an activity received from another instance - #[error("Failed to parse incoming activity {}: {0}", match .1 { + #[error("Failed to parse incoming activity {}: {0}", match .id { Some(t) => format!("with id {t}"), None => String::new(), })] - ParseReceivedActivity(serde_json::Error, Option), + ParseReceivedActivity { + /// The parse error + err: serde_json::Error, + /// ID of the Activitypub object which caused this error + id: Option, + }, /// Reqwest Middleware Error #[error(transparent)] ReqwestMiddleware(#[from] reqwest_middleware::Error), diff --git a/src/lib.rs b/src/lib.rs index 0a44fc9..b7fe013 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -52,10 +52,10 @@ where ::Error: From, Datatype: Clone, { - let activity: Activity = serde_json::from_slice(body).map_err(|e| { + let activity: Activity = serde_json::from_slice(body).map_err(|err| { // Attempt to include activity id in error message let id = extract_id(body).ok(); - Error::ParseReceivedActivity(e, id) + Error::ParseReceivedActivity { err, id } })?; data.config.verify_url_and_domain(&activity).await?; let actor = ObjectId::::from(activity.actor().clone()) From 43b51d79ce3d194fd9d435d15687a9788a512ae2 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 9 Jan 2025 11:44:59 +0100 Subject: [PATCH 2/9] Revert "Upgrade deps (#133)" This reverts commit 8910550663c980fbe0a3566307a61eefb41e97d2. --- Cargo.toml | 62 +++++++++++++------------- examples/local_federation/axum/http.rs | 4 +- src/actix_web/inbox.rs | 8 ++-- src/axum/inbox.rs | 2 + src/axum/middleware.rs | 3 +- src/error.rs | 9 +--- src/lib.rs | 4 +- 7 files changed, 45 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e08c794..5ac2d79 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,70 +32,70 @@ redundant_closure_for_method_calls = "deny" unwrap_used = "deny" [dependencies] -chrono = { version = "0.4.39", features = ["clock"], default-features = false } -serde = { version = "1.0.217", features = ["derive"] } -async-trait = "0.1.84" -url = { version = "2.5.4", features = ["serde"] } -serde_json = { version = "1.0.134", features = ["preserve_order"] } -reqwest = { version = "0.12.12", default-features = false, features = [ +chrono = { version = "0.4.38", features = ["clock"], default-features = false } +serde = { version = "1.0.204", features = ["derive"] } +async-trait = "0.1.81" +url = { version = "2.5.2", features = ["serde"] } +serde_json = { version = "1.0.120", features = ["preserve_order"] } +reqwest = { version = "0.12.5", default-features = false, features = [ "json", "stream", "rustls-tls", ] } -reqwest-middleware = "0.4.0" -tracing = "0.1.41" +reqwest-middleware = "0.3.2" +tracing = "0.1.40" base64 = "0.22.1" rand = "0.8.5" -rsa = "0.9.7" -once_cell = "1.20.2" -http = "1.2.0" +rsa = "0.9.6" +once_cell = "1.19.0" +http = "1.1.0" sha2 = { version = "0.10.8", features = ["oid"] } -thiserror = "2.0.9" -derive_builder = "0.20.2" -itertools = "0.14.0" +thiserror = "1.0.62" +derive_builder = "0.20.0" +itertools = "0.13.0" dyn-clone = "1.0.17" enum_delegate = "0.2.0" httpdate = "1.0.3" -http-signature-normalization-reqwest = { version = "0.13.0", default-features = false, features = [ +http-signature-normalization-reqwest = { version = "0.12.0", default-features = false, features = [ "sha-2", "middleware", "default-spawner", ] } http-signature-normalization = "0.7.0" -bytes = "1.9.0" -futures-core = { version = "0.3.31", default-features = false } -pin-project-lite = "0.2.15" +bytes = "1.6.1" +futures-core = { version = "0.3.30", default-features = false } +pin-project-lite = "0.2.14" activitystreams-kinds = "0.3.0" -regex = { version = "1.11.1", default-features = false, features = [ +regex = { version = "1.10.5", default-features = false, features = [ "std", "unicode", ] } -tokio = { version = "1.42.0", features = [ +tokio = { version = "1.38.0", features = [ "sync", "rt", "rt-multi-thread", "time", ] } -diesel = { version = "2.2.6", features = [ +diesel = { version = "2.2.1", features = [ "postgres", ], default-features = false, optional = true } -futures = "0.3.31" -moka = { version = "0.12.9", features = ["future"] } +futures = "0.3.30" +moka = { version = "0.12.8", features = ["future"] } # Actix-web -actix-web = { version = "4.9.0", default-features = false, optional = true } +actix-web = { version = "4.8.0", default-features = false, optional = true } http02 = { package = "http", version = "0.2.12", optional = true } # Axum -axum = { version = "0.8.1", features = ["json"], default-features = false, optional = true } -tower = { version = "0.5.2", optional = true } +axum = { version = "0.7.5", features = ["json"], default-features = false, optional = true } +tower = { version = "0.4.13", optional = true } [dev-dependencies] -anyhow = "1.0.95" -axum = { version = "0.8.1", features = ["macros"] } -axum-extra = { version = "0.10.0", features = ["typed-header"] } -env_logger = "0.11.6" -tokio = { version = "1.42.0", features = ["full"] } +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"] } [profile.dev] strip = "symbols" diff --git a/examples/local_federation/axum/http.rs b/examples/local_federation/axum/http.rs index e010a73..dd9d002 100644 --- a/examples/local_federation/axum/http.rs +++ b/examples/local_federation/axum/http.rs @@ -30,8 +30,8 @@ pub fn listen(config: &FederationConfig) -> Result<(), Error> { info!("Listening with axum on {hostname}"); let config = config.clone(); let app = Router::new() - .route("/{user}/inbox", post(http_post_user_inbox)) - .route("/{user}", get(http_get_user)) + .route("/:user/inbox", post(http_post_user_inbox)) + .route("/:user", get(http_get_user)) .route("/.well-known/webfinger", get(webfinger)) .layer(FederationMiddleware::new(config)); diff --git a/src/actix_web/inbox.rs b/src/actix_web/inbox.rs index 0912e72..9bce475 100644 --- a/src/actix_web/inbox.rs +++ b/src/actix_web/inbox.rs @@ -122,14 +122,14 @@ mod test { let (_, _, config) = setup_receive_test().await; let actor = Url::parse("http://ds9.lemmy.ml/u/lemmy_alpha").unwrap(); - let activity_id = "http://localhost:123/1"; + let id = "http://localhost:123/1"; let activity = json!({ "actor": actor.as_str(), "to": ["https://www.w3.org/ns/activitystreams#Public"], "object": "http://ds9.lemmy.ml/post/1", "cc": ["http://enterprise.lemmy.ml/c/main"], "type": "Delete", - "id": activity_id + "id": id } ); let body: Bytes = serde_json::to_vec(&activity).unwrap().into(); @@ -144,8 +144,8 @@ mod test { .await; match res { - Err(Error::ParseReceivedActivity { err: _, id }) => { - assert_eq!(activity_id, id.expect("has url").as_str()); + Err(Error::ParseReceivedActivity(_, url)) => { + assert_eq!(id, url.expect("has url").as_str()); } _ => unreachable!(), } diff --git a/src/axum/inbox.rs b/src/axum/inbox.rs index 84567e0..1767c10 100644 --- a/src/axum/inbox.rs +++ b/src/axum/inbox.rs @@ -10,6 +10,7 @@ use crate::{ traits::{ActivityHandler, Actor, Object}, }; use axum::{ + async_trait, body::Body, extract::FromRequest, http::{Request, StatusCode}, @@ -57,6 +58,7 @@ pub struct ActivityData { body: Vec, } +#[async_trait] impl FromRequest for ActivityData where S: Send + Sync, diff --git a/src/axum/middleware.rs b/src/axum/middleware.rs index 2f8b0e9..290dc94 100644 --- a/src/axum/middleware.rs +++ b/src/axum/middleware.rs @@ -1,5 +1,5 @@ use crate::config::{Data, FederationConfig, FederationMiddleware}; -use axum::{body::Body, extract::FromRequestParts, http::Request, response::Response}; +use axum::{async_trait, body::Body, extract::FromRequestParts, http::Request, response::Response}; use http::{request::Parts, StatusCode}; use std::task::{Context, Poll}; use tower::{Layer, Service}; @@ -43,6 +43,7 @@ where } } +#[async_trait] impl FromRequestParts for Data where S: Send + Sync, diff --git a/src/error.rs b/src/error.rs index 06c8085..1866e48 100644 --- a/src/error.rs +++ b/src/error.rs @@ -44,16 +44,11 @@ pub enum Error { #[error("Failed to parse object {1} with content {2}: {0}")] ParseFetchedObject(serde_json::Error, Url, String), /// Failed to parse an activity received from another instance - #[error("Failed to parse incoming activity {}: {0}", match .id { + #[error("Failed to parse incoming activity {}: {0}", match .1 { Some(t) => format!("with id {t}"), None => String::new(), })] - ParseReceivedActivity { - /// The parse error - err: serde_json::Error, - /// ID of the Activitypub object which caused this error - id: Option, - }, + ParseReceivedActivity(serde_json::Error, Option), /// Reqwest Middleware Error #[error(transparent)] ReqwestMiddleware(#[from] reqwest_middleware::Error), diff --git a/src/lib.rs b/src/lib.rs index b7fe013..0a44fc9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -52,10 +52,10 @@ where ::Error: From, Datatype: Clone, { - let activity: Activity = serde_json::from_slice(body).map_err(|err| { + let activity: Activity = serde_json::from_slice(body).map_err(|e| { // Attempt to include activity id in error message let id = extract_id(body).ok(); - Error::ParseReceivedActivity { err, id } + Error::ParseReceivedActivity(e, id) })?; data.config.verify_url_and_domain(&activity).await?; let actor = ObjectId::::from(activity.actor().clone()) From 2ad0eff31ce37117b50f78b58292a87eda8c8bbf Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 21 Jan 2025 12:58:13 +0100 Subject: [PATCH 3/9] 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 4/9] 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(()) + } +} From 9e21083e681bbfcc14ca740994ce39f6718e1268 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 21 Jan 2025 13:06:57 +0100 Subject: [PATCH 5/9] Fix shear check --- .woodpecker.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.woodpecker.yml b/.woodpecker.yml index 7781a05..56cfed9 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -1,5 +1,6 @@ variables: - &rust_image "rust:1.78-bullseye" + - &install_binstall "wget -O- https://github.com/cargo-bins/cargo-binstall/releases/latest/download/cargo-binstall-x86_64-unknown-linux-musl.tgz | tar -xvz -C /usr/local/cargo/bin" steps: cargo_fmt: @@ -10,7 +11,7 @@ steps: - event: pull_request cargo_shear: - image: *rust_nightly_image + image: *rust_image commands: - *install_binstall - cargo binstall -y cargo-shear From ce8376718006ebcc93f37aec19a8a90d059aefcc Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Tue, 21 Jan 2025 13:13:20 +0100 Subject: [PATCH 6/9] No cargo shear --- .woodpecker.yml | 10 ---------- Cargo.toml | 1 + 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/.woodpecker.yml b/.woodpecker.yml index 56cfed9..b82dd94 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -1,6 +1,5 @@ variables: - &rust_image "rust:1.78-bullseye" - - &install_binstall "wget -O- https://github.com/cargo-bins/cargo-binstall/releases/latest/download/cargo-binstall-x86_64-unknown-linux-musl.tgz | tar -xvz -C /usr/local/cargo/bin" steps: cargo_fmt: @@ -10,15 +9,6 @@ steps: when: - event: pull_request - cargo_shear: - image: *rust_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 53698d8..5ac2d79 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,6 +93,7 @@ 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"] } From 4ad668cc1074a691e77dd9dc8fae3835262c9cbd Mon Sep 17 00:00:00 2001 From: Nutomic Date: Thu, 23 Jan 2025 10:11:49 +0000 Subject: [PATCH 7/9] Add more url validation (#134) * Add more url validation * fix * more fix * Verify url after redirect * Dont allow redirect for webfinger * clippy * more domain validation * clippy * fix lemmy test * Remove trailing . from domain * clippy * fix * manual redirect handling * clippy * prevent infinite recursion * add timeout, comment --- examples/local_federation/main.rs | 7 ++- src/config.rs | 82 +++++++++++++++++++++++++++---- src/error.rs | 3 ++ src/fetch/mod.rs | 19 ++++++- src/fetch/webfinger.rs | 20 ++++++-- 5 files changed, 112 insertions(+), 19 deletions(-) diff --git a/examples/local_federation/main.rs b/examples/local_federation/main.rs index d23a594..f0ea3c4 100644 --- a/examples/local_federation/main.rs +++ b/examples/local_federation/main.rs @@ -7,6 +7,7 @@ use crate::{ }; use error::Error; use std::{env::args, str::FromStr}; +use tokio::try_join; use tracing::log::{info, LevelFilter}; mod activities; @@ -34,8 +35,10 @@ async fn main() -> Result<(), Error> { .map(|arg| Webserver::from_str(&arg).unwrap()) .unwrap_or(Webserver::Axum); - let alpha = new_instance("localhost:8001", "alpha".to_string()).await?; - let beta = new_instance("localhost:8002", "beta".to_string()).await?; + let (alpha, beta) = try_join!( + new_instance("localhost:8001", "alpha".to_string()), + new_instance("localhost:8002", "beta".to_string()) + )?; listen(&alpha, &webserver)?; listen(&beta, &webserver)?; info!("Local instances started"); diff --git a/src/config.rs b/src/config.rs index 63af704..608539a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -26,11 +26,14 @@ use bytes::Bytes; use derive_builder::Builder; use dyn_clone::{clone_trait_object, DynClone}; use moka::future::Cache; -use reqwest::Request; +use once_cell::sync::Lazy; +use regex::Regex; +use reqwest::{redirect::Policy, Client, Request}; use reqwest_middleware::{ClientWithMiddleware, RequestBuilder}; use rsa::{pkcs8::DecodePrivateKey, RsaPrivateKey}; use serde::de::DeserializeOwned; use std::{ + net::IpAddr, ops::Deref, sync::{ atomic::{AtomicU32, Ordering}, @@ -38,6 +41,7 @@ use std::{ }, time::Duration, }; +use tokio::net::lookup_host; use url::Url; /// Configuration for this library, with various federation related settings @@ -54,9 +58,14 @@ pub struct FederationConfig { /// [crate::fetch::object_id::ObjectId] for more details. #[builder(default = "20")] pub(crate) http_fetch_limit: u32, - #[builder(default = "reqwest::Client::default().into()")] - /// HTTP client used for all outgoing requests. Middleware can be used to add functionality - /// like log tracing or retry of failed requests. + #[builder(default = "default_client()")] + /// HTTP client used for all outgoing requests. When passing a custom client here you should + /// also disable redirects and set timeouts. + /// + /// Middleware can be used to add functionality like log tracing or retry of failed requests. + /// Redirects are disabled by default, because automatic redirect URLs can't be validated. + /// Instead a single redirect is handled manually. The default client sets a timeout of 10s + /// to avoid excessive resource usage when connecting to dead servers. pub(crate) client: ClientWithMiddleware, /// Run library in debug mode. This allows usage of http and localhost urls. It also sends /// outgoing activities synchronously, not in background thread. This helps to make tests @@ -105,6 +114,9 @@ pub struct FederationConfig { pub(crate) queue_retry_count: usize, } +pub(crate) static DOMAIN_REGEX: Lazy = + Lazy::new(|| Regex::new(r"^[a-zA-Z0-9.-]*$").expect("compile regex")); + impl FederationConfig { /// Returns a new config builder with default values. pub fn builder() -> FederationConfigBuilder { @@ -159,17 +171,56 @@ impl FederationConfig { return Ok(()); } - if url.domain().is_none() { + let Some(domain) = url.domain() else { return Err(Error::UrlVerificationError("Url must have a domain")); + }; + if !DOMAIN_REGEX.is_match(domain) { + return Err(Error::UrlVerificationError("Invalid characters in domain")); } - if url.domain() == Some("localhost") && !self.debug { - return Err(Error::UrlVerificationError( - "Localhost is only allowed in debug mode", - )); + // Extra checks only for production mode + if !self.debug { + if url.port().is_some() { + return Err(Error::UrlVerificationError("Explicit port is not allowed")); + } + + // Resolve domain and see if it points to private IP + // TODO: Use is_global() once stabilized + // https://doc.rust-lang.org/std/net/enum.IpAddr.html#method.is_global + let invalid_ip = + lookup_host((domain.to_owned(), 80)) + .await? + .any(|addr| match addr.ip() { + IpAddr::V4(addr) => { + addr.is_private() + || addr.is_link_local() + || addr.is_loopback() + || addr.is_multicast() + } + IpAddr::V6(addr) => { + addr.is_loopback() + || addr.is_multicast() + || ((addr.segments()[0] & 0xfe00) == 0xfc00) // is_unique_local + || ((addr.segments()[0] & 0xffc0) == 0xfe80) // is_unicast_link_local + } + }); + if invalid_ip { + return Err(Error::UrlVerificationError( + "Localhost is only allowed in debug mode", + )); + } } - self.url_verifier.verify(url).await?; + // It is valid but uncommon for domains to end with `.` char. Drop this so it cant be used + // to bypass domain blocklist. Avoid cloning url in common case. + if domain.ends_with('.') { + let mut url = url.clone(); + let domain = &domain[0..domain.len() - 1]; + url.set_host(Some(domain))?; + self.url_verifier.verify(&url).await?; + } else { + self.url_verifier.verify(url).await?; + } Ok(()) } @@ -370,6 +421,17 @@ impl FederationMiddleware { } } +fn default_client() -> ClientWithMiddleware { + let timeout = Duration::from_secs(10); + Client::builder() + .redirect(Policy::none()) + .timeout(timeout) + .connect_timeout(timeout) + .build() + .unwrap_or_else(|_| Client::default()) + .into() +} + #[cfg(test)] #[allow(clippy::unwrap_used)] mod test { diff --git a/src/error.rs b/src/error.rs index 1866e48..4a53fd8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -78,6 +78,9 @@ pub enum Error { /// Attempted to fetch object but the response's id field doesn't match #[error("Attempted to fetch object from {0} but the response's id field doesn't match")] FetchWrongId(Url), + /// I/O error from OS + #[error(transparent)] + IoError(#[from] std::io::Error), /// Other generic errors #[error("{0}")] Other(String), diff --git a/src/fetch/mod.rs b/src/fetch/mod.rs index b8e9c84..424a115 100644 --- a/src/fetch/mod.rs +++ b/src/fetch/mod.rs @@ -11,7 +11,7 @@ use crate::{ FEDERATION_CONTENT_TYPE, }; use bytes::Bytes; -use http::{HeaderValue, StatusCode}; +use http::{header::LOCATION, HeaderValue, StatusCode}; use serde::de::DeserializeOwned; use std::sync::atomic::Ordering; use tracing::info; @@ -59,7 +59,7 @@ pub async fn fetch_object_http( r#"application/ld+json; profile="https://www.w3.org/ns/activitystreams""#, // activitypub standard r#"application/activity+json; charset=utf-8"#, // mastodon ]; - let res = fetch_object_http_with_accept(url, data, &FETCH_CONTENT_TYPE).await?; + let res = fetch_object_http_with_accept(url, data, &FETCH_CONTENT_TYPE, false).await?; // Ensure correct content-type to prevent vulnerabilities, with case insensitive comparison. let content_type = res @@ -74,6 +74,7 @@ pub async fn fetch_object_http( // Ensure id field matches final url after redirect if res.object_id.as_ref() != Some(&res.url) { if let Some(res_object_id) = res.object_id { + data.config.verify_url_valid(&res_object_id).await?; // If id is different but still on the same domain, attempt to request object // again from url in id field. if res_object_id.domain() == res.url.domain() { @@ -99,6 +100,7 @@ async fn fetch_object_http_with_accept( url: &Url, data: &Data, content_type: &HeaderValue, + recursive: bool, ) -> Result, Error> { let config = &data.config; config.verify_url_valid(url).await?; @@ -131,6 +133,19 @@ async fn fetch_object_http_with_accept( req.send().await? }; + // Allow a single redirect using recursion. Further redirects are ignored. + let location = res.headers().get(LOCATION).and_then(|l| l.to_str().ok()); + if let (Some(location), false) = (location, recursive) { + let location = location.parse()?; + return Box::pin(fetch_object_http_with_accept( + &location, + data, + content_type, + true, + )) + .await; + } + if res.status() == StatusCode::GONE { return Err(Error::ObjectDeleted(url.clone())); } diff --git a/src/fetch/webfinger.rs b/src/fetch/webfinger.rs index 8460245..8d53078 100644 --- a/src/fetch/webfinger.rs +++ b/src/fetch/webfinger.rs @@ -1,5 +1,5 @@ use crate::{ - config::Data, + config::{Data, DOMAIN_REGEX}, error::Error, fetch::{fetch_object_http_with_accept, object_id::ObjectId}, traits::{Actor, Object}, @@ -54,21 +54,31 @@ where .splitn(2, '@') .collect_tuple() .ok_or(WebFingerError::WrongFormat.into_crate_error())?; + + // For production mode make sure that domain doesnt contain any port or path. + if !data.config.debug && !DOMAIN_REGEX.is_match(domain) { + return Err(Error::UrlVerificationError("Invalid characters in domain").into()); + } + let protocol = if data.config.debug { "http" } else { "https" }; let fetch_url = format!("{protocol}://{domain}/.well-known/webfinger?resource=acct:{identifier}"); debug!("Fetching webfinger url: {}", &fetch_url); - let res: Webfinger = fetch_object_http_with_accept( + let res = fetch_object_http_with_accept::<_, Webfinger>( &Url::parse(&fetch_url).map_err(Error::UrlParse)?, data, &WEBFINGER_CONTENT_TYPE, + false, ) - .await? - .object; + .await?; + if res.url.as_str() != fetch_url { + data.config.verify_url_valid(&res.url).await?; + } - debug_assert_eq!(res.subject, format!("acct:{identifier}")); + debug_assert_eq!(res.object.subject, format!("acct:{identifier}")); let links: Vec = res + .object .links .iter() .filter(|link| { From f8f0d9c47e0fdd862f6c659f12a6878a488479e2 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 23 Jan 2025 11:13:50 +0100 Subject: [PATCH 8/9] Version 0.6.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5ac2d79..5c40f42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "activitypub_federation" -version = "0.6.1" +version = "0.6.2" edition = "2021" description = "High-level Activitypub framework" keywords = ["activitypub", "activitystreams", "federation", "fediverse"] From 64b990b5fc521b48cc4dac8aaeb6aff567dba1be Mon Sep 17 00:00:00 2001 From: naught101 Date: Wed, 29 Jan 2025 20:10:53 +1100 Subject: [PATCH 9/9] Update README.md for widespread use (#138) Remove "while not in widespread use" - There are a number of semi-popular use-cases now. Certainly enough to justify the broader use. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index debe40f..8851036 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ A high-level framework for [ActivityPub](https://www.w3.org/TR/activitypub/) fed The ActivityPub protocol is a decentralized social networking protocol. It allows web servers to exchange data using JSON over HTTP. Data can be fetched on demand, and also delivered directly to inboxes for live updates. -While Activitypub is not in widespread use yet, is has the potential to form the basis of the next generation of social media. This is because it has a number of major advantages compared to existing platforms and alternative technologies: +Activitypub has the potential to form the basis of the next generation of social media. This is because it has a number of major advantages compared to existing platforms and alternative technologies: - **Interoperability**: Imagine being able to comment under a Youtube video directly from twitter.com, and having the comment shown under the video on youtube.com. Or following a Subreddit from Facebook. Such functionality is already available on the equivalent Fediverse platforms, thanks to common usage of Activitypub. - **Ease of use**: From a user perspective, decentralized social media works almost identically to existing websites: a website with email and password based login. Unlike pure peer-to-peer networks, it is not necessary to handle private keys or install any local software.