From c4a4ea5b6ce1f3305ad1aa2500e751af297009a6 Mon Sep 17 00:00:00 2001 From: fdb-hiroshima <35889323+fdb-hiroshima@users.noreply.github.com> Date: Sat, 5 Jan 2019 22:30:28 +0100 Subject: [PATCH] Support blind key rotation (#399) * Allow receiving objects with new unknown key * Rotate key after sending Delete activity * Do the right check --- plume-models/src/users.rs | 28 ++++++++++++++++++++++++++++ src/routes/comments.rs | 6 +++++- src/routes/instance.rs | 17 ++++++++++++++--- src/routes/posts.rs | 9 +++++++-- src/routes/user.rs | 20 ++++++++++++++------ 5 files changed, 68 insertions(+), 12 deletions(-) diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 37e8b7ea..72b8b58b 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -364,6 +364,10 @@ impl User { .followers_string()?), users::avatar_id.eq(avatar.map(|a| a.id)), users::last_fetched_date.eq(Utc::now().naive_utc()), + users::public_key.eq(json + .custom_props + .public_key_publickey()? + .public_key_pem_string()?), )) .execute(conn) .map(|_| ()) @@ -638,6 +642,30 @@ impl User { ).map_err(Error::from) } + pub fn rotate_keypair(&self, conn: &Connection) -> Result> { + if self.private_key.is_none() { + return Err(Error::InvalidValue) + } + if (Utc::now().naive_utc() - self.last_fetched_date).num_minutes() < 10 { + //rotated recently + self.get_keypair() + } else { + let (public_key, private_key) = gen_keypair(); + let public_key = String::from_utf8(public_key).expect("NewUser::new_local: public key error"); + let private_key = String::from_utf8(private_key).expect("NewUser::new_local: private key error"); + let res = PKey::from_rsa( + Rsa::private_key_from_pem(private_key.as_ref())? + )?; + diesel::update(self) + .set((users::public_key.eq(public_key), + users::private_key.eq(Some(private_key)), + users::last_fetched_date.eq(Utc::now().naive_utc()))) + .execute(conn) + .map_err(Error::from) + .map(|_| res) + } + } + pub fn to_activity(&self, conn: &Connection) -> Result { let mut actor = Person::default(); actor diff --git a/src/routes/comments.rs b/src/routes/comments.rs index 34bccd31..50b8fe59 100644 --- a/src/routes/comments.rs +++ b/src/routes/comments.rs @@ -7,6 +7,8 @@ use rocket_i18n::I18n; use validator::Validate; use template_utils::Ructe; +use std::time::Duration; + use plume_common::{utils, activity_pub::{broadcast, ApRequest, ActivityStream, inbox::Deletable}}; use plume_models::{ @@ -104,7 +106,9 @@ pub fn delete(blog: String, slug: String, id: i32, user: User, conn: DbConn, wor if comment.author_id == user.id { let dest = User::one_by_instance(&*conn)?; let delete_activity = comment.delete(&*conn)?; - worker.execute(move || broadcast(&user, delete_activity, dest)); + let user_c = user.clone(); + worker.execute(move || broadcast(&user_c, delete_activity, dest)); + worker.execute_after(Duration::from_secs(10*60), move || {user.rotate_keypair(&conn).expect("Failed to rotate keypair");}); } } Ok(Redirect::to(uri!(super::posts::details: blog = blog, slug = slug, responding_to = _))) diff --git a/src/routes/instance.rs b/src/routes/instance.rs index 42554cb6..da77739b 100644 --- a/src/routes/instance.rs +++ b/src/routes/instance.rs @@ -10,6 +10,7 @@ use plume_models::{ admin::Admin, comments::Comment, db_conn::DbConn, + Error, headers::Headers, posts::Post, users::User, @@ -180,16 +181,26 @@ pub fn ban(_admin: Admin, conn: DbConn, id: i32, searcher: Searcher) -> Result, headers: Headers, searcher: Searcher) -> Result> { let act = data.1.into_inner(); + let sig = data.0; let activity = act.clone(); let actor_id = activity["actor"].as_str() .or_else(|| activity["actor"]["id"].as_str()).ok_or(status::BadRequest(Some("Missing actor id for activity")))?; let actor = User::from_url(&conn, actor_id).expect("instance::shared_inbox: user error"); - if !verify_http_headers(&actor, &headers.0, &data.0).is_secure() && + if !verify_http_headers(&actor, &headers.0, &sig).is_secure() && !act.clone().verify(&actor) { - println!("Rejected invalid activity supposedly from {}, with headers {:?}", actor.username, headers.0); - return Err(status::BadRequest(Some("Invalid signature"))); + // maybe we just know an old key? + actor.refetch(&conn).and_then(|_| User::get(&conn, actor.id)) + .and_then(|u| if verify_http_headers(&u, &headers.0, &sig).is_secure() || + act.clone().verify(&u) { + Ok(()) + } else { + Err(Error::Signature) + }) + .map_err(|_| { + println!("Rejected invalid activity supposedly from {}, with headers {:?}", actor.username, headers.0); + status::BadRequest(Some("Invalid signature"))})?; } if Instance::is_blocked(&*conn, actor_id).map_err(|_| status::BadRequest(Some("Can't tell if instance is blocked")))? { diff --git a/src/routes/posts.rs b/src/routes/posts.rs index f8538830..42a59716 100644 --- a/src/routes/posts.rs +++ b/src/routes/posts.rs @@ -3,7 +3,10 @@ use heck::{CamelCase, KebabCase}; use rocket::request::LenientForm; use rocket::response::{Redirect, Flash}; use rocket_i18n::I18n; -use std::{collections::{HashMap, HashSet}, borrow::Cow}; +use std::{ + collections::{HashMap, HashSet}, + borrow::Cow, time::Duration, +}; use validator::{Validate, ValidationError, ValidationErrors}; use plume_common::activity_pub::{broadcast, ActivityStream, ApRequest, inbox::Deletable}; @@ -397,7 +400,9 @@ pub fn delete(blog_name: String, slug: String, conn: DbConn, user: User, worker: } else { let dest = User::one_by_instance(&*conn)?; let delete_activity = post.delete(&(&conn, &searcher))?; - worker.execute(move || broadcast(&user, delete_activity, dest)); + let user_c = user.clone(); + worker.execute(move || broadcast(&user_c, delete_activity, dest)); + worker.execute_after(Duration::from_secs(10*60), move || {user.rotate_keypair(&conn).expect("Failed to rotate keypair");}); Ok(Redirect::to(uri!(super::blogs::details: name = blog_name, page = _))) } diff --git a/src/routes/user.rs b/src/routes/user.rs index a6a91de0..387da6f2 100644 --- a/src/routes/user.rs +++ b/src/routes/user.rs @@ -370,6 +370,7 @@ pub fn inbox( ) -> Result>> { let user = User::find_local(&*conn, &name).map_err(|_| None)?; let act = data.1.into_inner(); + let sig = data.0; let activity = act.clone(); let actor_id = activity["actor"] @@ -380,14 +381,21 @@ pub fn inbox( ))))?; let actor = User::from_url(&conn, actor_id).expect("user::inbox: user error"); - if !verify_http_headers(&actor, &headers.0, &data.0).is_secure() + if !verify_http_headers(&actor, &headers.0, &sig).is_secure() && !act.clone().verify(&actor) { - println!( - "Rejected invalid activity supposedly from {}, with headers {:?}", - actor.username, headers.0 - ); - return Err(Some(status::BadRequest(Some("Invalid signature")))); + // maybe we just know an old key? + actor.refetch(&conn).and_then(|_| User::get(&conn, actor.id)) + .and_then(|actor| if verify_http_headers(&actor, &headers.0, &sig).is_secure() + || act.clone().verify(&actor) + { + Ok(()) + } else { + Err(Error::Signature) + }) + .map_err(|_| { + println!("Rejected invalid activity supposedly from {}, with headers {:?}", actor.username, headers.0); + status::BadRequest(Some("Invalid signature"))})?; } if Instance::is_blocked(&*conn, actor_id).map_err(|_| None)? {