Don't use Rc<RefCell<>>

This commit is contained in:
asonix 2020-03-25 11:00:37 -05:00
parent 7af4f07a97
commit dbd852156b
5 changed files with 89 additions and 84 deletions

View file

@ -1,7 +1,7 @@
[package] [package]
name = "http-signature-normalization-actix" name = "http-signature-normalization-actix"
description = "An HTTP Signatures library that leaves the signing to you" 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 <asonix@asonix.dog>"] authors = ["asonix <asonix@asonix.dog>"]
license-file = "LICENSE" license-file = "LICENSE"
readme = "README.md" readme = "README.md"

View file

@ -16,7 +16,7 @@ This crate provides extensions the ClientRequest type from Actix Web, and provid
actix = "0.10.0-alpha.1" actix = "0.10.0-alpha.1"
actix-web = "3.0.0-alpha.1" actix-web = "3.0.0-alpha.1"
thiserror = "0.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" sha2 = "0.8"
``` ```

View file

@ -41,6 +41,7 @@ impl SignatureVerify for MyVerify {
async fn index( async fn index(
(_, sig_verified): (DigestVerified, SignatureVerified), (_, sig_verified): (DigestVerified, SignatureVerified),
req: HttpRequest, req: HttpRequest,
_body: web::Bytes,
) -> &'static str { ) -> &'static str {
info!("Verified request for {}", sig_verified.key_id()); info!("Verified request for {}", sig_verified.key_id());
info!("{:?}", req); info!("{:?}", req);

View file

@ -2,23 +2,21 @@
use super::{DigestPart, DigestVerify}; use super::{DigestPart, DigestVerify};
use actix_web::{ use actix_web::{
dev::{Body, Payload, Service, ServiceRequest, ServiceResponse, Transform}, dev::{MessageBody, Payload, Service, ServiceRequest, ServiceResponse, Transform},
error::PayloadError, error::PayloadError,
http::{header::HeaderValue, StatusCode}, http::{header::HeaderValue, StatusCode},
FromRequest, HttpMessage, HttpRequest, HttpResponse, ResponseError, FromRequest, HttpMessage, HttpRequest, HttpResponse, ResponseError,
}; };
use bytes::{Bytes, BytesMut}; use bytes::{Bytes, BytesMut};
use futures::{ use futures::{
channel::mpsc,
future::{err, ok, ready, Ready}, future::{err, ok, ready, Ready},
stream::once,
Stream, StreamExt, Stream, StreamExt,
}; };
use log::{debug, warn}; use log::{debug, warn};
use std::{ use std::{
cell::RefCell,
future::Future, future::Future,
pin::Pin, pin::Pin,
rc::Rc,
task::{Context, Poll}, task::{Context, Poll},
}; };
@ -45,7 +43,7 @@ pub struct DigestVerified;
pub struct VerifyDigest<T>(bool, T); pub struct VerifyDigest<T>(bool, T);
#[doc(hidden)] #[doc(hidden)]
pub struct VerifyMiddleware<T, S>(Rc<RefCell<S>>, bool, T); pub struct VerifyMiddleware<T, S>(S, bool, T);
#[derive(Debug, thiserror::Error)] #[derive(Debug, thiserror::Error)]
#[error("Error verifying digest")] #[error("Error verifying digest")]
@ -90,49 +88,41 @@ impl FromRequest for DigestVerified {
} }
} }
impl<T, S> Transform<S> for VerifyDigest<T> impl<T, S, B> Transform<S> for VerifyDigest<T>
where where
T: DigestVerify + Clone + 'static, T: DigestVerify + Clone + 'static,
S: Service< S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = actix_web::Error>
Request = ServiceRequest, + 'static,
Response = ServiceResponse<Body>,
Error = actix_web::Error,
> + 'static,
S::Error: 'static, S::Error: 'static,
B: MessageBody + 'static,
{ {
type Request = ServiceRequest; type Request = ServiceRequest;
type Response = ServiceResponse<Body>; type Response = ServiceResponse<B>;
type Error = actix_web::Error; type Error = actix_web::Error;
type Transform = VerifyMiddleware<T, S>; type Transform = VerifyMiddleware<T, S>;
type InitError = (); type InitError = ();
type Future = Ready<Result<Self::Transform, Self::InitError>>; type Future = Ready<Result<Self::Transform, Self::InitError>>;
fn new_transform(&self, service: S) -> Self::Future { fn new_transform(&self, service: S) -> Self::Future {
ok(VerifyMiddleware( ok(VerifyMiddleware(service, self.0, self.1.clone()))
Rc::new(RefCell::new(service)),
self.0,
self.1.clone(),
))
} }
} }
impl<T, S> Service for VerifyMiddleware<T, S> impl<T, S, B> Service for VerifyMiddleware<T, S>
where where
T: DigestVerify + Clone + 'static, T: DigestVerify + Clone + 'static,
S: Service< S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = actix_web::Error>
Request = ServiceRequest, + 'static,
Response = ServiceResponse<Body>,
Error = actix_web::Error,
> + 'static,
S::Error: 'static, S::Error: 'static,
B: MessageBody + 'static,
{ {
type Request = ServiceRequest; type Request = ServiceRequest;
type Response = ServiceResponse<Body>; type Response = ServiceResponse<B>;
type Error = actix_web::Error; type Error = actix_web::Error;
type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>>>>; type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>>>>;
fn poll_ready(&mut self, cx: &mut Context) -> Poll<Result<(), Self::Error>> { fn poll_ready(&mut self, cx: &mut Context) -> Poll<Result<(), Self::Error>> {
self.0.borrow_mut().poll_ready(cx) self.0.poll_ready(cx)
} }
fn call(&mut self, mut req: ServiceRequest) -> Self::Future { fn call(&mut self, mut req: ServiceRequest) -> Self::Future {
@ -144,41 +134,61 @@ where
return Box::pin(err(VerifyError.into())); return Box::pin(err(VerifyError.into()));
} }
}; };
let mut payload = req.take_payload(); let payload = req.take_payload();
let service = self.0.clone();
let mut verify_digest = self.2.clone(); let (tx, rx) = mpsc::channel(1);
let f1 = verify_payload(vec, self.2.clone(), payload, tx);
let payload: Pin<Box<dyn Stream<Item = Result<Bytes, PayloadError>> + '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 { Box::pin(async move {
let mut output_bytes = BytesMut::new(); f1.await?;
while let Some(res) = payload.next().await { f2.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<Box<dyn Stream<Item = Result<Bytes, PayloadError>> + 'static>>)
.into(),
);
req.extensions_mut().insert(DigestVerified);
service.borrow_mut().call(req).await
} else {
warn!("Digest could not be verified");
Err(VerifyError.into())
}
}) })
} else if self.1 { } else if self.1 {
Box::pin(err(VerifyError.into())) Box::pin(err(VerifyError.into()))
} else { } else {
Box::pin(self.0.borrow_mut().call(req)) Box::pin(self.0.call(req))
} }
} }
} }
async fn verify_payload<T>(
vec: Vec<DigestPart>,
mut verify_digest: T,
mut payload: Payload,
mut tx: mpsc::Sender<Bytes>,
) -> 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<Vec<DigestPart>> { fn parse_digest(h: &HeaderValue) -> Option<Vec<DigestPart>> {
let h = h.to_str().ok()?.split(";").next()?; let h = h.to_str().ok()?.split(";").next()?;
let v: Vec<_> = h let v: Vec<_> = h

View file

@ -2,17 +2,15 @@
use crate::{Config, PrepareVerifyError, SignatureVerify}; use crate::{Config, PrepareVerifyError, SignatureVerify};
use actix_web::{ use actix_web::{
dev::{Body, Payload, Service, ServiceRequest, ServiceResponse, Transform}, dev::{MessageBody, Payload, Service, ServiceRequest, ServiceResponse, Transform},
http::StatusCode, http::StatusCode,
Error, FromRequest, HttpMessage, HttpRequest, HttpResponse, ResponseError, Error, FromRequest, HttpMessage, HttpRequest, HttpResponse, ResponseError,
}; };
use futures::future::{err, ok, ready, Ready}; use futures::future::{err, ok, ready, Ready};
use log::{debug, warn}; use log::{debug, warn};
use std::{ use std::{
cell::RefCell,
future::Future, future::Future,
pin::Pin, pin::Pin,
rc::Rc,
task::{Context, Poll}, task::{Context, Poll},
}; };
@ -50,7 +48,7 @@ pub struct VerifySignature<T>(T, Config, HeaderKind, bool);
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[doc(hidden)] #[doc(hidden)]
pub struct VerifyMiddleware<T, S>(Rc<RefCell<S>>, Config, HeaderKind, bool, T); pub struct VerifyMiddleware<T, S>(S, Config, HeaderKind, bool, T);
#[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
enum HeaderKind { enum HeaderKind {
@ -91,16 +89,17 @@ where
} }
} }
impl<T, S> VerifyMiddleware<T, S> impl<T, S, B> VerifyMiddleware<T, S>
where where
T: SignatureVerify + 'static, T: SignatureVerify + 'static,
T::Future: 'static, T::Future: 'static,
S: Service<Request = ServiceRequest, Response = ServiceResponse<Body>, Error = Error> + 'static, S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = Error> + 'static,
B: MessageBody + 'static,
{ {
fn handle( fn handle(
&mut self, &mut self,
req: ServiceRequest, req: ServiceRequest,
) -> Pin<Box<dyn Future<Output = Result<ServiceResponse<Body>, Error>>>> { ) -> Pin<Box<dyn Future<Output = Result<ServiceResponse<B>, Error>>>> {
let res = self.1.begin_verify( let res = self.1.begin_verify(
req.method(), req.method(),
req.uri().path_and_query(), req.uri().path_and_query(),
@ -130,19 +129,18 @@ where
let algorithm = unverified.algorithm().map(|a| a.clone()); let algorithm = unverified.algorithm().map(|a| a.clone());
let key_id = unverified.key_id().to_owned(); let key_id = unverified.key_id().to_owned();
let service = self.0.clone(); let f1 = unverified.verify(|signature, signing_string| {
let fut = unverified.verify(|signature, signing_string| {
self.4 self.4
.signature_verify(algorithm, &key_id, signature, signing_string) .signature_verify(algorithm, &key_id, signature, signing_string)
}); });
Box::pin(async move { req.extensions_mut().insert(SignatureVerified(key_id));
let verified = fut.await?;
if verified { let f2 = self.0.call(req);
req.extensions_mut().insert(SignatureVerified(key_id));
service.borrow_mut().call(req).await Box::pin(async move {
if f1.await? {
f2.await
} else { } else {
warn!("Signature is invalid"); warn!("Signature is invalid");
Err(VerifyError.into()) Err(VerifyError.into())
@ -181,18 +179,16 @@ impl FromRequest for SignatureVerified {
} }
} }
impl<T, S> Transform<S> for VerifySignature<T> impl<T, S, B> Transform<S> for VerifySignature<T>
where where
T: SignatureVerify + Clone + 'static, T: SignatureVerify + Clone + 'static,
S: Service< S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = actix_web::Error>
Request = ServiceRequest, + 'static,
Response = ServiceResponse<Body>,
Error = actix_web::Error,
> + 'static,
S::Error: 'static, S::Error: 'static,
B: MessageBody + 'static,
{ {
type Request = ServiceRequest; type Request = ServiceRequest;
type Response = ServiceResponse<Body>; type Response = ServiceResponse<B>;
type Error = actix_web::Error; type Error = actix_web::Error;
type Transform = VerifyMiddleware<T, S>; type Transform = VerifyMiddleware<T, S>;
type InitError = (); type InitError = ();
@ -200,7 +196,7 @@ where
fn new_transform(&self, service: S) -> Self::Future { fn new_transform(&self, service: S) -> Self::Future {
ok(VerifyMiddleware( ok(VerifyMiddleware(
Rc::new(RefCell::new(service)), service,
self.1.clone(), self.1.clone(),
self.2, self.2,
self.3, self.3,
@ -209,23 +205,21 @@ where
} }
} }
impl<T, S> Service for VerifyMiddleware<T, S> impl<T, S, B> Service for VerifyMiddleware<T, S>
where where
T: SignatureVerify + Clone + 'static, T: SignatureVerify + Clone + 'static,
S: Service< S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = actix_web::Error>
Request = ServiceRequest, + 'static,
Response = ServiceResponse<Body>,
Error = actix_web::Error,
> + 'static,
S::Error: 'static, S::Error: 'static,
B: MessageBody + 'static,
{ {
type Request = ServiceRequest; type Request = ServiceRequest;
type Response = ServiceResponse<Body>; type Response = ServiceResponse<B>;
type Error = actix_web::Error; type Error = actix_web::Error;
type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>>>>; type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>>>>;
fn poll_ready(&mut self, cx: &mut Context) -> Poll<Result<(), Self::Error>> { fn poll_ready(&mut self, cx: &mut Context) -> Poll<Result<(), Self::Error>> {
self.0.borrow_mut().poll_ready(cx) self.0.poll_ready(cx)
} }
fn call(&mut self, req: ServiceRequest) -> Self::Future { fn call(&mut self, req: ServiceRequest) -> Self::Future {
@ -245,7 +239,7 @@ where
Box::pin(err(VerifyError.into())) Box::pin(err(VerifyError.into()))
} else if self.3 { } else if self.3 {
debug!("Headers are missing but Optional is true, continuing"); debug!("Headers are missing but Optional is true, continuing");
Box::pin(self.0.borrow_mut().call(req)) Box::pin(self.0.call(req))
} else { } else {
debug!("Authorization or Signature headers are missing"); debug!("Authorization or Signature headers are missing");
Box::pin(err(VerifyError.into())) Box::pin(err(VerifyError.into()))