From a528046a688e1dd164f275bd4a81ed056556ef4c Mon Sep 17 00:00:00 2001
From: asonix <asonix@asonix.dog>
Date: Wed, 22 Apr 2020 17:41:01 -0500
Subject: [PATCH] Update sig lib, forward more error context, better blind key
 rotation support

---
 Cargo.lock                 | 28 +++++++--------
 Cargo.toml                 |  2 +-
 src/data/actor.rs          | 61 ++++++++++++++++++++++++-------
 src/error.rs               |  4 +--
 src/middleware/verifier.rs | 73 +++++++++++++++++++++++++-------------
 src/routes/inbox.rs        |  2 +-
 6 files changed, 114 insertions(+), 56 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 4190d74..db689fc 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -653,9 +653,9 @@ dependencies = [
 
 [[package]]
 name = "cc"
-version = "1.0.50"
+version = "1.0.52"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "95e28fa049fda1c330bcf9d723be7663a899c4679724b34c81e9f5a326aab8cd"
+checksum = "c3d87b23d6a92cd03af510a5ade527033f6aa6fa92161e2d5863a907d4c5e31d"
 
 [[package]]
 name = "cfg-if"
@@ -1192,9 +1192,9 @@ dependencies = [
 
 [[package]]
 name = "http-signature-normalization"
-version = "0.4.1"
+version = "0.4.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "257835255b5d40c6de712d90e56dc874ca5da2816121e7b9f3cfc7b3a55a5714"
+checksum = "fde6b0321d465ea2cdc18b5d5ec73eee2ff20177ecee126cd8dd997f6c49e088"
 dependencies = [
  "chrono",
  "thiserror",
@@ -1202,9 +1202,9 @@ dependencies = [
 
 [[package]]
 name = "http-signature-normalization-actix"
-version = "0.3.0-alpha.9"
+version = "0.3.0-alpha.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "195de65fba66cf11426765b07fde96553f02c9a982f760d9e736330cc2e7f07f"
+checksum = "31cbaa36c8b370b423bc69bdbe146bfc52d32ad3e38a91281f1a4ac0b1762274"
 dependencies = [
  "actix-http",
  "actix-web",
@@ -1765,9 +1765,9 @@ checksum = "237844750cfbb86f67afe27eee600dfbbcb6188d734139b534cbfbf4f96792ae"
 
 [[package]]
 name = "pin-utils"
-version = "0.1.0-alpha.4"
+version = "0.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5894c618ce612a3fa23881b152b608bafb8c56cfc22f434a3ba3120b40f7b587"
+checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184"
 
 [[package]]
 name = "postgres-protocol"
@@ -2092,9 +2092,9 @@ dependencies = [
 
 [[package]]
 name = "ructe"
-version = "0.11.0"
+version = "0.11.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "90bfdbe059afc8a41171d0c9f362c2bd67e03131f2d71939feef0a26b0a765b0"
+checksum = "15be061bce86758995098b361a5d3ed2b4203f658dffd522a1ee680b457418b2"
 dependencies = [
  "base64 0.12.0",
  "bytecount",
@@ -2438,9 +2438,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a"
 
 [[package]]
 name = "structopt"
-version = "0.3.13"
+version = "0.3.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ff6da2e8d107dfd7b74df5ef4d205c6aebee0706c647f6bc6a2d5789905c00fb"
+checksum = "863246aaf5ddd0d6928dfeb1a9ca65f505599e4e1b399935ef7e75107516b4ef"
 dependencies = [
  "clap",
  "lazy_static",
@@ -2449,9 +2449,9 @@ dependencies = [
 
 [[package]]
 name = "structopt-derive"
-version = "0.4.6"
+version = "0.4.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a489c87c08fbaf12e386665109dd13470dcc9c4583ea3e10dd2b4523e5ebd9ac"
+checksum = "d239ca4b13aee7a2142e6795cbd69e457665ff8037aed33b3effdc430d2f927a"
 dependencies = [
  "heck",
  "proc-macro-error",
diff --git a/Cargo.toml b/Cargo.toml
index f8b944d..c7f83a7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,7 +30,7 @@ deadpool-postgres = "0.5.5"
 dotenv = "0.15.0"
 env_logger = "0.7.1"
 futures = "0.3.4"
-http-signature-normalization-actix = { version = "0.3.0-alpha.9", default-features = false, features = ["sha-2"] }
+http-signature-normalization-actix = { version = "0.3.0-alpha.10", default-features = false, features = ["sha-2"] }
 log = "0.4"
 lru = "0.4.3"
 mime = "0.3.16"
diff --git a/src/data/actor.rs b/src/data/actor.rs
index 23f6802..ef4ba5d 100644
--- a/src/data/actor.rs
+++ b/src/data/actor.rs
@@ -8,6 +8,27 @@ use uuid::Uuid;
 
 const REFETCH_DURATION: Duration = Duration::from_secs(60 * 30);
 
+#[derive(Debug)]
+pub enum MaybeCached<T> {
+    Cached(T),
+    Fetched(T),
+}
+
+impl<T> MaybeCached<T> {
+    pub fn is_cached(&self) -> bool {
+        match self {
+            MaybeCached::Cached(_) => true,
+            _ => false,
+        }
+    }
+
+    pub fn into_inner(self) -> T {
+        match self {
+            MaybeCached::Cached(t) | MaybeCached::Fetched(t) => t,
+        }
+    }
+}
+
 #[derive(Clone)]
 pub struct ActorCache {
     db: Db,
@@ -32,19 +53,11 @@ impl ActorCache {
         self.following.read().await.contains(id)
     }
 
-    pub async fn get(&self, id: &XsdAnyUri, requests: &Requests) -> Result<Actor, MyError> {
-        if let Some(actor) = self.cache.read().await.get(id) {
-            return Ok(actor.clone());
-        }
-
-        if let Some(actor) = self.lookup(id).await? {
-            self.cache
-                .write()
-                .await
-                .insert(id.clone(), actor.clone(), REFETCH_DURATION);
-            return Ok(actor);
-        }
-
+    pub async fn get_no_cache(
+        &self,
+        id: &XsdAnyUri,
+        requests: &Requests,
+    ) -> Result<Actor, MyError> {
         let accepted_actor = requests.fetch::<AcceptedActors>(id.as_str()).await?;
 
         let input_host = id.as_url().host();
@@ -85,6 +98,28 @@ impl ActorCache {
         Ok(actor)
     }
 
+    pub async fn get(
+        &self,
+        id: &XsdAnyUri,
+        requests: &Requests,
+    ) -> Result<MaybeCached<Actor>, MyError> {
+        if let Some(actor) = self.cache.read().await.get(id) {
+            return Ok(MaybeCached::Cached(actor.clone()));
+        }
+
+        if let Some(actor) = self.lookup(id).await? {
+            self.cache
+                .write()
+                .await
+                .insert(id.clone(), actor.clone(), REFETCH_DURATION);
+            return Ok(MaybeCached::Cached(actor));
+        }
+
+        self.get_no_cache(id, requests)
+            .await
+            .map(MaybeCached::Fetched)
+    }
+
     pub async fn follower(&self, actor: &Actor) -> Result<(), MyError> {
         self.save(actor.clone()).await
     }
diff --git a/src/error.rs b/src/error.rs
index 7ab3d86..bc3a1af 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -80,8 +80,8 @@ pub enum MyError {
     #[error("Timed out while waiting on db pool, {0:?}")]
     DbTimeout(TimeoutType),
 
-    #[error("Invalid algorithm provided to verifier")]
-    Algorithm,
+    #[error("Invalid algorithm provided to verifier, {0}")]
+    Algorithm(String),
 
     #[error("Object has already been relayed")]
     Duplicate,
diff --git a/src/middleware/verifier.rs b/src/middleware/verifier.rs
index f5fe2ff..9b7c9fe 100644
--- a/src/middleware/verifier.rs
+++ b/src/middleware/verifier.rs
@@ -2,7 +2,7 @@ use crate::{data::ActorCache, error::MyError, requests::Requests};
 use activitystreams::primitives::XsdAnyUri;
 use actix_web::web;
 use http_signature_normalization_actix::{prelude::*, verify::DeprecatedAlgorithm};
-use log::{error, warn};
+use log::error;
 use rsa::{hash::Hashes, padding::PaddingScheme, PublicKey, RSAPublicKey};
 use rsa_pem::KeyExt;
 use sha2::{Digest, Sha256};
@@ -22,37 +22,64 @@ impl MyVerify {
         let mut uri: XsdAnyUri = key_id.parse()?;
         uri.as_url_mut().set_fragment(None);
         let actor = self.1.get(&uri, &self.0).await?;
-
-        let public_key = RSAPublicKey::from_pem_pkcs8(&actor.public_key)?;
+        let was_cached = actor.is_cached();
+        let actor = actor.into_inner();
 
         match algorithm {
             Some(Algorithm::Hs2019) => (),
             Some(Algorithm::Deprecated(DeprecatedAlgorithm::RsaSha256)) => (),
-            other => {
-                warn!("Invalid algorithm supplied for signature, {:?}", other);
-                return Err(MyError::Algorithm);
+            Some(other) => {
+                return Err(MyError::Algorithm(other.to_string()));
             }
+            None => (),
         };
 
-        web::block(move || {
-            let decoded = base64::decode(signature)?;
-            let hashed = Sha256::digest(signing_string.as_bytes());
+        let res = do_verify(&actor.public_key, signature.clone(), signing_string.clone()).await;
 
-            public_key.verify(
-                PaddingScheme::PKCS1v15,
-                Some(&Hashes::SHA2_256),
-                &hashed,
-                &decoded,
-            )?;
+        if let Err(e) = res {
+            if !was_cached {
+                return Err(e);
+            }
+        } else {
+            return Ok(true);
+        }
 
-            Ok(()) as Result<(), MyError>
-        })
-        .await?;
+        // Previously we verified the sig from an actor's local cache
+        //
+        // Now we make sure we fetch an updated actor
+        let actor = self.1.get_no_cache(&uri, &self.0).await?;
+
+        do_verify(&actor.public_key, signature, signing_string).await?;
 
         Ok(true)
     }
 }
 
+async fn do_verify(
+    public_key: &str,
+    signature: String,
+    signing_string: String,
+) -> Result<(), MyError> {
+    let public_key = RSAPublicKey::from_pem_pkcs8(public_key)?;
+
+    web::block(move || {
+        let decoded = base64::decode(signature)?;
+        let hashed = Sha256::digest(signing_string.as_bytes());
+
+        public_key.verify(
+            PaddingScheme::PKCS1v15,
+            Some(&Hashes::SHA2_256),
+            &hashed,
+            &decoded,
+        )?;
+
+        Ok(()) as Result<(), MyError>
+    })
+    .await?;
+
+    Ok(())
+}
+
 impl SignatureVerify for MyVerify {
     type Error = MyError;
     type Future = Pin<Box<dyn Future<Output = Result<bool, Self::Error>>>>;
@@ -60,14 +87,10 @@ impl SignatureVerify for MyVerify {
     fn signature_verify(
         &mut self,
         algorithm: Option<Algorithm>,
-        key_id: &str,
-        signature: &str,
-        signing_string: &str,
+        key_id: String,
+        signature: String,
+        signing_string: String,
     ) -> Self::Future {
-        let key_id = key_id.to_owned();
-        let signature = signature.to_owned();
-        let signing_string = signing_string.to_owned();
-
         let this = self.clone();
 
         Box::pin(async move {
diff --git a/src/routes/inbox.rs b/src/routes/inbox.rs
index e3e9156..33796bd 100644
--- a/src/routes/inbox.rs
+++ b/src/routes/inbox.rs
@@ -26,7 +26,7 @@ pub async fn route(
 ) -> Result<HttpResponse, MyError> {
     let input = input.into_inner();
 
-    let actor = actors.get(&input.actor, &client).await?;
+    let actor = actors.get(&input.actor, &client).await?.into_inner();
 
     let (is_blocked, is_whitelisted, is_listener) = join!(
         state.is_blocked(&actor.id),