From dbd852156b29b4b9d91bd6b916f0cf6949fd501f Mon Sep 17 00:00:00 2001 From: asonix Date: Wed, 25 Mar 2020 11:00:37 -0500 Subject: [PATCH] Don't use Rc> --- http-signature-normalization-actix/Cargo.toml | 2 +- http-signature-normalization-actix/README.md | 2 +- .../examples/server.rs | 1 + .../src/digest/middleware.rs | 110 ++++++++++-------- .../src/middleware.rs | 58 +++++---- 5 files changed, 89 insertions(+), 84 deletions(-) diff --git a/http-signature-normalization-actix/Cargo.toml b/http-signature-normalization-actix/Cargo.toml index 2f813ed..60649c8 100644 --- a/http-signature-normalization-actix/Cargo.toml +++ b/http-signature-normalization-actix/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "http-signature-normalization-actix" description = "An HTTP Signatures library that leaves the signing to you" -version = "0.3.0-alpha.7" +version = "0.3.0-alpha.8" authors = ["asonix "] license-file = "LICENSE" readme = "README.md" diff --git a/http-signature-normalization-actix/README.md b/http-signature-normalization-actix/README.md index 784aaf2..e127e05 100644 --- a/http-signature-normalization-actix/README.md +++ b/http-signature-normalization-actix/README.md @@ -16,7 +16,7 @@ This crate provides extensions the ClientRequest type from Actix Web, and provid actix = "0.10.0-alpha.1" actix-web = "3.0.0-alpha.1" thiserror = "0.1" -http-signature-normalization-actix = { version = "0.3.0-alpha.6", default-features = false, features = ["sha-2"] } +http-signature-normalization-actix = { version = "0.3.0-alpha.8", default-features = false, features = ["sha-2"] } sha2 = "0.8" ``` diff --git a/http-signature-normalization-actix/examples/server.rs b/http-signature-normalization-actix/examples/server.rs index 961bd5f..75204ae 100644 --- a/http-signature-normalization-actix/examples/server.rs +++ b/http-signature-normalization-actix/examples/server.rs @@ -41,6 +41,7 @@ impl SignatureVerify for MyVerify { async fn index( (_, sig_verified): (DigestVerified, SignatureVerified), req: HttpRequest, + _body: web::Bytes, ) -> &'static str { info!("Verified request for {}", sig_verified.key_id()); info!("{:?}", req); diff --git a/http-signature-normalization-actix/src/digest/middleware.rs b/http-signature-normalization-actix/src/digest/middleware.rs index be5d78d..b081445 100644 --- a/http-signature-normalization-actix/src/digest/middleware.rs +++ b/http-signature-normalization-actix/src/digest/middleware.rs @@ -2,23 +2,21 @@ use super::{DigestPart, DigestVerify}; use actix_web::{ - dev::{Body, Payload, Service, ServiceRequest, ServiceResponse, Transform}, + dev::{MessageBody, Payload, Service, ServiceRequest, ServiceResponse, Transform}, error::PayloadError, http::{header::HeaderValue, StatusCode}, FromRequest, HttpMessage, HttpRequest, HttpResponse, ResponseError, }; use bytes::{Bytes, BytesMut}; use futures::{ + channel::mpsc, future::{err, ok, ready, Ready}, - stream::once, Stream, StreamExt, }; use log::{debug, warn}; use std::{ - cell::RefCell, future::Future, pin::Pin, - rc::Rc, task::{Context, Poll}, }; @@ -45,7 +43,7 @@ pub struct DigestVerified; pub struct VerifyDigest(bool, T); #[doc(hidden)] -pub struct VerifyMiddleware(Rc>, bool, T); +pub struct VerifyMiddleware(S, bool, T); #[derive(Debug, thiserror::Error)] #[error("Error verifying digest")] @@ -90,49 +88,41 @@ impl FromRequest for DigestVerified { } } -impl Transform for VerifyDigest +impl Transform for VerifyDigest where T: DigestVerify + Clone + 'static, - S: Service< - Request = ServiceRequest, - Response = ServiceResponse, - Error = actix_web::Error, - > + 'static, + S: Service, Error = actix_web::Error> + + 'static, S::Error: 'static, + B: MessageBody + 'static, { type Request = ServiceRequest; - type Response = ServiceResponse; + type Response = ServiceResponse; type Error = actix_web::Error; type Transform = VerifyMiddleware; type InitError = (); type Future = Ready>; fn new_transform(&self, service: S) -> Self::Future { - ok(VerifyMiddleware( - Rc::new(RefCell::new(service)), - self.0, - self.1.clone(), - )) + ok(VerifyMiddleware(service, self.0, self.1.clone())) } } -impl Service for VerifyMiddleware +impl Service for VerifyMiddleware where T: DigestVerify + Clone + 'static, - S: Service< - Request = ServiceRequest, - Response = ServiceResponse, - Error = actix_web::Error, - > + 'static, + S: Service, Error = actix_web::Error> + + 'static, S::Error: 'static, + B: MessageBody + 'static, { type Request = ServiceRequest; - type Response = ServiceResponse; + type Response = ServiceResponse; type Error = actix_web::Error; type Future = Pin>>>; fn poll_ready(&mut self, cx: &mut Context) -> Poll> { - self.0.borrow_mut().poll_ready(cx) + self.0.poll_ready(cx) } fn call(&mut self, mut req: ServiceRequest) -> Self::Future { @@ -144,41 +134,61 @@ where return Box::pin(err(VerifyError.into())); } }; - let mut payload = req.take_payload(); - let service = self.0.clone(); - let mut verify_digest = self.2.clone(); + let payload = req.take_payload(); + + let (tx, rx) = mpsc::channel(1); + let f1 = verify_payload(vec, self.2.clone(), payload, tx); + + let payload: Pin> + 'static>> = + Box::pin(rx.map(|bytes| Ok(bytes))); + req.set_payload(payload.into()); + req.extensions_mut().insert(DigestVerified); + + let f2 = self.0.call(req); Box::pin(async move { - let mut output_bytes = BytesMut::new(); - while let Some(res) = payload.next().await { - let bytes = res?; - output_bytes.extend(bytes); - } - let bytes = output_bytes.freeze(); - - if verify_digest.verify(&vec, &bytes.as_ref()) { - req.set_payload( - (Box::pin(once(ok(bytes))) - as Pin> + 'static>>) - .into(), - ); - - req.extensions_mut().insert(DigestVerified); - - service.borrow_mut().call(req).await - } else { - warn!("Digest could not be verified"); - Err(VerifyError.into()) - } + f1.await?; + f2.await }) } else if self.1 { Box::pin(err(VerifyError.into())) } else { - Box::pin(self.0.borrow_mut().call(req)) + Box::pin(self.0.call(req)) } } } +async fn verify_payload( + vec: Vec, + mut verify_digest: T, + mut payload: Payload, + mut tx: mpsc::Sender, +) -> Result<(), actix_web::Error> +where + T: DigestVerify + Clone + 'static, +{ + let mut output_bytes = BytesMut::new(); + + while let Some(res) = payload.next().await { + let bytes = res?; + output_bytes.extend(bytes); + + if tx.is_closed() { + warn!("Payload dropped. If this was unexpected, it could be that the payload isn't required in the route this middleware is guarding"); + return Err(VerifyError.into()); + } + } + + let bytes = output_bytes.freeze(); + + if verify_digest.verify(&vec, &bytes.as_ref()) { + tx.try_send(bytes).map_err(|_| VerifyError.into()) + } else { + warn!("Digest could not be verified"); + Err(VerifyError.into()) + } +} + fn parse_digest(h: &HeaderValue) -> Option> { let h = h.to_str().ok()?.split(";").next()?; let v: Vec<_> = h diff --git a/http-signature-normalization-actix/src/middleware.rs b/http-signature-normalization-actix/src/middleware.rs index a81ee2b..a4ce747 100644 --- a/http-signature-normalization-actix/src/middleware.rs +++ b/http-signature-normalization-actix/src/middleware.rs @@ -2,17 +2,15 @@ use crate::{Config, PrepareVerifyError, SignatureVerify}; use actix_web::{ - dev::{Body, Payload, Service, ServiceRequest, ServiceResponse, Transform}, + dev::{MessageBody, Payload, Service, ServiceRequest, ServiceResponse, Transform}, http::StatusCode, Error, FromRequest, HttpMessage, HttpRequest, HttpResponse, ResponseError, }; use futures::future::{err, ok, ready, Ready}; use log::{debug, warn}; use std::{ - cell::RefCell, future::Future, pin::Pin, - rc::Rc, task::{Context, Poll}, }; @@ -50,7 +48,7 @@ pub struct VerifySignature(T, Config, HeaderKind, bool); #[derive(Clone, Debug)] #[doc(hidden)] -pub struct VerifyMiddleware(Rc>, Config, HeaderKind, bool, T); +pub struct VerifyMiddleware(S, Config, HeaderKind, bool, T); #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] enum HeaderKind { @@ -91,16 +89,17 @@ where } } -impl VerifyMiddleware +impl VerifyMiddleware where T: SignatureVerify + 'static, T::Future: 'static, - S: Service, Error = Error> + 'static, + S: Service, Error = Error> + 'static, + B: MessageBody + 'static, { fn handle( &mut self, req: ServiceRequest, - ) -> Pin, Error>>>> { + ) -> Pin, Error>>>> { let res = self.1.begin_verify( req.method(), req.uri().path_and_query(), @@ -130,19 +129,18 @@ where let algorithm = unverified.algorithm().map(|a| a.clone()); let key_id = unverified.key_id().to_owned(); - let service = self.0.clone(); - - let fut = unverified.verify(|signature, signing_string| { + let f1 = unverified.verify(|signature, signing_string| { self.4 .signature_verify(algorithm, &key_id, signature, signing_string) }); - Box::pin(async move { - let verified = fut.await?; + req.extensions_mut().insert(SignatureVerified(key_id)); - if verified { - req.extensions_mut().insert(SignatureVerified(key_id)); - service.borrow_mut().call(req).await + let f2 = self.0.call(req); + + Box::pin(async move { + if f1.await? { + f2.await } else { warn!("Signature is invalid"); Err(VerifyError.into()) @@ -181,18 +179,16 @@ impl FromRequest for SignatureVerified { } } -impl Transform for VerifySignature +impl Transform for VerifySignature where T: SignatureVerify + Clone + 'static, - S: Service< - Request = ServiceRequest, - Response = ServiceResponse, - Error = actix_web::Error, - > + 'static, + S: Service, Error = actix_web::Error> + + 'static, S::Error: 'static, + B: MessageBody + 'static, { type Request = ServiceRequest; - type Response = ServiceResponse; + type Response = ServiceResponse; type Error = actix_web::Error; type Transform = VerifyMiddleware; type InitError = (); @@ -200,7 +196,7 @@ where fn new_transform(&self, service: S) -> Self::Future { ok(VerifyMiddleware( - Rc::new(RefCell::new(service)), + service, self.1.clone(), self.2, self.3, @@ -209,23 +205,21 @@ where } } -impl Service for VerifyMiddleware +impl Service for VerifyMiddleware where T: SignatureVerify + Clone + 'static, - S: Service< - Request = ServiceRequest, - Response = ServiceResponse, - Error = actix_web::Error, - > + 'static, + S: Service, Error = actix_web::Error> + + 'static, S::Error: 'static, + B: MessageBody + 'static, { type Request = ServiceRequest; - type Response = ServiceResponse; + type Response = ServiceResponse; type Error = actix_web::Error; type Future = Pin>>>; fn poll_ready(&mut self, cx: &mut Context) -> Poll> { - self.0.borrow_mut().poll_ready(cx) + self.0.poll_ready(cx) } fn call(&mut self, req: ServiceRequest) -> Self::Future { @@ -245,7 +239,7 @@ where Box::pin(err(VerifyError.into())) } else if self.3 { debug!("Headers are missing but Optional is true, continuing"); - Box::pin(self.0.borrow_mut().call(req)) + Box::pin(self.0.call(req)) } else { debug!("Authorization or Signature headers are missing"); Box::pin(err(VerifyError.into()))