From 7af4f07a9761e1f386777f382bd01036f7ac4e79 Mon Sep 17 00:00:00 2001 From: asonix Date: Thu, 19 Mar 2020 21:36:10 -0500 Subject: [PATCH] Add logging in actix crate --- Cargo.toml | 2 +- http-signature-normalization-actix/Cargo.toml | 8 ++-- .../src/digest/middleware.rs | 27 +++++++---- http-signature-normalization-actix/src/lib.rs | 30 ++++++++++-- .../src/middleware.rs | 46 ++++++++++++++----- src/verify.rs | 7 +++ 6 files changed, 92 insertions(+), 28 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2fce513..4c52224 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "http-signature-normalization" description = "An HTTP Signatures library that leaves the signing to you" -version = "0.4.0" +version = "0.4.1" authors = ["asonix "] license-file = "LICENSE" readme = "README.md" diff --git a/http-signature-normalization-actix/Cargo.toml b/http-signature-normalization-actix/Cargo.toml index 36ad9a3..2f813ed 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.6" +version = "0.3.0-alpha.7" authors = ["asonix "] license-file = "LICENSE" readme = "README.md" @@ -12,7 +12,7 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] default = ["sha-2", "sha-3"] -digest = ["base64", "log"] +digest = ["base64"] sha-2 = ["digest", "sha2"] sha-3 = ["digest", "sha3"] @@ -31,8 +31,8 @@ base64 = { version = "0.11", optional = true } bytes = "0.5.4" chrono = "0.4.6" futures = "0.3" -http-signature-normalization = { version = "0.4.0", path = ".." } -log = { version = "0.4", optional = true } +http-signature-normalization = { version = "0.4.1", path = ".." } +log = "0.4" sha2 = { version = "0.8", optional = true } sha3 = { version = "0.8", optional = true } thiserror = "1.0" diff --git a/http-signature-normalization-actix/src/digest/middleware.rs b/http-signature-normalization-actix/src/digest/middleware.rs index 1695aed..be5d78d 100644 --- a/http-signature-normalization-actix/src/digest/middleware.rs +++ b/http-signature-normalization-actix/src/digest/middleware.rs @@ -1,5 +1,6 @@ //! Types for setting up Digest middleware verification +use super::{DigestPart, DigestVerify}; use actix_web::{ dev::{Body, Payload, Service, ServiceRequest, ServiceResponse, Transform}, error::PayloadError, @@ -12,6 +13,7 @@ use futures::{ stream::once, Stream, StreamExt, }; +use log::{debug, warn}; use std::{ cell::RefCell, future::Future, @@ -20,8 +22,6 @@ use std::{ task::{Context, Poll}, }; -use super::{DigestPart, DigestVerify}; - #[derive(Copy, Clone, Debug)] /// A type implementing FromRequest that can be used in route handler to guard for verified /// digests @@ -76,12 +76,17 @@ impl FromRequest for DigestVerified { type Config = (); fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - ready( - req.extensions() - .get::() - .map(|s| *s) - .ok_or(VerifyError), - ) + let res = req + .extensions() + .get::() + .map(|s| *s) + .ok_or(VerifyError); + + if res.is_err() { + debug!("Failed to fetch DigestVerified from request"); + } + + ready(res) } } @@ -134,7 +139,10 @@ where if let Some(digest) = req.headers().get("Digest") { let vec = match parse_digest(digest) { Some(vec) => vec, - None => return Box::pin(err(VerifyError.into())), + None => { + warn!("Digest header could not be parsed"); + return Box::pin(err(VerifyError.into())); + } }; let mut payload = req.take_payload(); let service = self.0.clone(); @@ -159,6 +167,7 @@ where service.borrow_mut().call(req).await } else { + warn!("Digest could not be verified"); Err(VerifyError.into()) } }) diff --git a/http-signature-normalization-actix/src/lib.rs b/http-signature-normalization-actix/src/lib.rs index 48ce1a8..28208b0 100644 --- a/http-signature-normalization-actix/src/lib.rs +++ b/http-signature-normalization-actix/src/lib.rs @@ -244,15 +244,39 @@ pub struct Config { #[derive(Debug, thiserror::Error)] /// An error when preparing to verify a request pub enum PrepareVerifyError { - #[error("Signature error, {0}")] - /// An error validating the request - Sig(#[from] http_signature_normalization::PrepareVerifyError), + #[error("Header is missing")] + /// Header is missing + Missing, + + #[error("Header is expired")] + /// Header is expired + Expired, + + #[error("Couldn't parse required field, {0}")] + /// Couldn't parse required field + ParseField(&'static str), #[error("Failed to read header, {0}")] /// An error converting the header to a string for validation Header(#[from] ToStrError), } +impl From for PrepareVerifyError { + fn from(e: http_signature_normalization::PrepareVerifyError) -> Self { + use http_signature_normalization as hsn; + + match e { + hsn::PrepareVerifyError::Parse(parse_error) => { + PrepareVerifyError::ParseField(parse_error.missing_field()) + } + hsn::PrepareVerifyError::Validate(validate_error) => match validate_error { + hsn::verify::ValidateError::Missing => PrepareVerifyError::Missing, + hsn::verify::ValidateError::Expired => PrepareVerifyError::Expired, + }, + } + } +} + impl Config { /// Create a new Config with a default expiration of 10 seconds pub fn new() -> Self { diff --git a/http-signature-normalization-actix/src/middleware.rs b/http-signature-normalization-actix/src/middleware.rs index 25d2a42..a81ee2b 100644 --- a/http-signature-normalization-actix/src/middleware.rs +++ b/http-signature-normalization-actix/src/middleware.rs @@ -1,11 +1,13 @@ //! Types for verifying requests with Actix Web +use crate::{Config, PrepareVerifyError, SignatureVerify}; use actix_web::{ dev::{Body, 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, @@ -14,8 +16,6 @@ use std::{ task::{Context, Poll}, }; -use crate::{Config, SignatureVerify}; - #[derive(Clone, Debug)] /// A marker type that can be used to guard routes when the signature middleware is set to /// 'optional' @@ -73,7 +73,7 @@ where /// By default, this middleware expects to verify Signature headers, and requires the presence /// of the header pub fn new(verify_signature: T, config: Config) -> Self { - VerifySignature(verify_signature, config, HeaderKind::Signature, true) + VerifySignature(verify_signature, config, HeaderKind::Signature, false) } /// Verify Authorization headers instead of Signature headers @@ -87,7 +87,7 @@ where /// is passed through. This can be used to set a global middleware, and then guard each route /// handler with the [`SignatureVerified`] type. pub fn optional(self) -> Self { - VerifySignature(self.0, self.1, self.2, false) + VerifySignature(self.0, self.1, self.2, true) } } @@ -109,7 +109,22 @@ where let unverified = match res { Ok(unverified) => unverified, - Err(_) => return Box::pin(err(VerifyError.into())), + Err(PrepareVerifyError::Expired) => { + warn!("Header is expired"); + return Box::pin(err(VerifyError.into())); + } + Err(PrepareVerifyError::Missing) => { + debug!("Header is missing"); + return Box::pin(err(VerifyError.into())); + } + Err(PrepareVerifyError::ParseField(field)) => { + debug!("Failed to parse field {}", field); + return Box::pin(err(VerifyError.into())); + } + Err(PrepareVerifyError::Header(e)) => { + debug!("Failed to parse header {}", e); + return Box::pin(err(VerifyError.into())); + } }; let algorithm = unverified.algorithm().map(|a| a.clone()); @@ -129,6 +144,7 @@ where req.extensions_mut().insert(SignatureVerified(key_id)); service.borrow_mut().call(req).await } else { + warn!("Signature is invalid"); Err(VerifyError.into()) } }) @@ -151,12 +167,17 @@ impl FromRequest for SignatureVerified { type Config = (); fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - ready( - req.extensions() - .get::() - .map(|s| s.clone()) - .ok_or(VerifyError), - ) + let res = req + .extensions() + .get::() + .map(|s| s.clone()) + .ok_or(VerifyError); + + if res.is_err() { + debug!("Failed to fetch SignatureVerified from request"); + } + + ready(res) } } @@ -220,10 +241,13 @@ where return self.handle(req); } + debug!("Authorization or Signature headers are missing"); 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)) } else { + debug!("Authorization or Signature headers are missing"); Box::pin(err(VerifyError.into())) } } diff --git a/src/verify.rs b/src/verify.rs index 3ad7e1b..69d7850 100644 --- a/src/verify.rs +++ b/src/verify.rs @@ -119,6 +119,13 @@ pub enum ValidateError { /// was invalid. pub struct ParseSignatureError(&'static str); +impl ParseSignatureError { + /// Get the name of the missing field + pub fn missing_field(&self) -> &'static str { + self.0 + } +} + impl Unverified { /// Get the Key ID from an Unverified type ///