Skip to content

Commit

Permalink
fix: Improve error handling (#309)
Browse files Browse the repository at this point in the history
  • Loading branch information
@abdolence
abdolence authored Feb 2, 2025
1 parent 9d67410 commit e149ae9
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 82 deletions.
6 changes: 1 addition & 5 deletions src/hyper_tokio/hyper_ext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,7 @@ impl HyperExtensions {
Self::http_body_to_string(req_body)
.and_then(|body| async {
signature_verifier
.verify(
received_hash.to_str().unwrap(),
&body,
received_ts.to_str().unwrap(),
)
.verify(received_hash.to_str()?, &body, received_ts.to_str()?)
.map(|_| (body, parts))
.map_err(|e| e.into())
})
Expand Down
242 changes: 165 additions & 77 deletions src/signature_verifier.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::models::SlackSigningSecret;
use futures_util::TryFutureExt;
use hmac::digest::Key;
use hmac::{Hmac, Mac};
use rsb_derive::Builder;
use rvstruct::*;
Expand All @@ -9,63 +11,118 @@ use subtle::ConstantTimeEq;

#[derive(Clone)]
pub struct SlackEventSignatureVerifier {
secret_bytes: Vec<u8>,
secret: SlackSigningSecret,
}

impl SlackEventSignatureVerifier {
pub const SLACK_SIGNED_HASH_HEADER: &'static str = "x-slack-signature";
pub const SLACK_SIGNED_TIMESTAMP: &'static str = "x-slack-request-timestamp";

// 5 minutes is the maximum allowed timestamp age for Slack events
pub const MAX_TIMESTAMP_AGE_SECONDS: i64 = 60 * 5;

pub fn new(secret: &SlackSigningSecret) -> Self {
let secret_bytes = secret.value().as_bytes().to_vec();
SlackEventSignatureVerifier { secret_bytes }
SlackEventSignatureVerifier {
secret: secret.clone(),
}
}

fn sign<'a, 'b>(&'a self, body: &'b str, ts: &'b str) -> String {
let mut mac = Hmac::<Sha256>::new_from_slice(&self.secret_bytes)
.expect("HMAC can take key of any size");
mac.update(b"v0:");
mac.update(ts.as_bytes());
mac.update(b":");
mac.update(body.as_bytes());
let result = mac.finalize().into_bytes();
format!("v0={}", hex::encode(result))
pub fn verify<'b>(
&self,
hash: &'b str,
body: &'b str,
ts: &'b str,
) -> Result<(), SlackEventSignatureVerifierError> {
self.verify_at_time(hash, body, ts, chrono::Utc::now().timestamp())
}

pub fn verify<'b>(
fn verify_at_time<'b>(
&self,
hash: &'b str,
body: &'b str,
ts: &'b str,
time: i64,
) -> Result<(), SlackEventSignatureVerifierError> {
if self.secret_bytes.is_empty() {
let hash_to_check = self.sign(body, ts)?;
if hash_to_check.as_bytes().ct_eq(hash.as_bytes()).unwrap_u8() == 1 {
Self::validate_ts(ts, time)?;
Ok(())
} else {
Err(SlackEventSignatureVerifierError::WrongSignatureError(
SlackEventWrongSignatureErrorInit {
body_len: body.len(),
ts: ts.into(),
received_hash: hash.into(),
generated_hash: hash_to_check,
}
.into(),
))
}
}

fn sign<'a, 'b>(
&'a self,
body: &'b str,
ts: &'b str,
) -> Result<String, SlackEventSignatureVerifierError> {
if self.secret.value().is_empty() {
Err(SlackEventSignatureVerifierError::CryptoInitError(
SlackEventSignatureCryptoInitError::new("secret key is empty".into()),
SlackEventSignatureCryptoInitError::new("Secret key is empty.".into()),
))
} else {
let hash_to_check = self.sign(body, ts);
if hash_to_check.as_bytes().ct_eq(hash.as_bytes()).unwrap_u8() == 1 {
Ok(())
} else {
Err(SlackEventSignatureVerifierError::WrongSignatureError(
SlackEventWrongSignatureErrorInit {
body_len: body.len(),
let mut mac =
Hmac::<Sha256>::new_from_slice(self.secret.value().as_bytes()).map_err(|e| {
SlackEventSignatureVerifierError::CryptoInitError(
SlackEventSignatureCryptoInitErrorInit {
message: format!("HMAC init error: {}.", e),
}
.into(),
)
})?;
mac.update(b"v0:");
mac.update(ts.as_bytes());
mac.update(b":");
mac.update(body.as_bytes());
let result = mac.finalize().into_bytes();
Ok(format!("v0={}", hex::encode(result)))
}
}

fn validate_ts(ts: &str, time: i64) -> Result<(), SlackEventSignatureVerifierError> {
let ts_int = ts.parse::<i64>().map_err(|e| {
SlackEventSignatureVerifierError::IncorrectOrExpiredTimestampError(
SlackEventTimestampErrorInit {
ts: ts.into(),
message: format!("Timestamp is not a valid integer: {}", e),
}
.into(),
)
})?;
if (time - ts_int).abs() > Self::MAX_TIMESTAMP_AGE_SECONDS {
Err(
SlackEventSignatureVerifierError::IncorrectOrExpiredTimestampError(
SlackEventTimestampErrorInit {
ts: ts.into(),
received_hash: hash.into(),
generated_hash: hash_to_check,
message: format!(
"Timestamp is either incorrect or expired: now: {}, ts: {}. It should be within {} seconds.",
time, ts_int, Self::MAX_TIMESTAMP_AGE_SECONDS
),
}
.into(),
))
}
),
)
} else {
Ok(())
}
}
}

#[derive(Debug)]
#[derive(Debug, Clone)]
pub enum SlackEventSignatureVerifierError {
CryptoInitError(SlackEventSignatureCryptoInitError),
AbsentSignatureError(SlackEventAbsentSignatureError),
WrongSignatureError(SlackEventWrongSignatureError),
IncorrectOrExpiredTimestampError(SlackEventTimestampError),
}

impl Display for SlackEventSignatureVerifierError {
Expand All @@ -74,6 +131,9 @@ impl Display for SlackEventSignatureVerifierError {
SlackEventSignatureVerifierError::CryptoInitError(ref err) => err.fmt(f),
SlackEventSignatureVerifierError::AbsentSignatureError(ref err) => err.fmt(f),
SlackEventSignatureVerifierError::WrongSignatureError(ref err) => err.fmt(f),
SlackEventSignatureVerifierError::IncorrectOrExpiredTimestampError(ref err) => {
err.fmt(f)
}
}
}
}
Expand All @@ -84,6 +144,9 @@ impl Error for SlackEventSignatureVerifierError {
SlackEventSignatureVerifierError::CryptoInitError(ref err) => Some(err),
SlackEventSignatureVerifierError::AbsentSignatureError(ref err) => Some(err),
SlackEventSignatureVerifierError::WrongSignatureError(ref err) => Some(err),
SlackEventSignatureVerifierError::IncorrectOrExpiredTimestampError(ref err) => {
Some(err)
}
}
}
}
Expand Down Expand Up @@ -139,70 +202,95 @@ impl Display for SlackEventWrongSignatureError {

impl Error for SlackEventWrongSignatureError {}

#[test]
fn check_signature_success() {
use sha2::Digest;
#[derive(Debug, PartialEq, Eq, Clone, Builder)]
pub struct SlackEventTimestampError {
pub ts: String,
pub message: String,
}

impl Display for SlackEventTimestampError {
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
write!(
f,
"Slack API timestamp validation error for ts: '{}'. Reason: {}",
self.ts, self.message
)
}
}

impl Error for SlackEventTimestampError {}

let key_str: String = hex::encode(Sha256::digest("test-key"));
#[cfg(test)]
mod test {
use super::*;

let verifier = SlackEventSignatureVerifier::new(&key_str.into());
#[test]
fn check_signature_success() {
use sha2::Digest;

const TEST_BODY: &str = "test-body";
const TEST_TS: &str = "test-ts";
let key_str: String = hex::encode(Sha256::digest("test-key"));

let hash = verifier.sign(TEST_BODY, TEST_TS);
verifier
.verify(&hash, TEST_BODY, TEST_TS)
.expect("signature verification failed");
}
let verifier = SlackEventSignatureVerifier::new(&key_str.into());

#[test]
fn test_precoded_data() {
const TEST_SECRET: &str = "d058b0b8f3f91e4446ad981890c9b6c16b2acc85367e30a2d76b8a95e525c02a";
const TEST_HASH: &str = "v0=37ca0519af8b621f18b13586fc72488ebb159fc730a5d1718dd823dec69dea95";
const TEST_BODY: &str = "test-body";
const TEST_TS: &str = "test-ts";
const TEST_BODY: &str = "test-body";
let test_ts = chrono::Utc::now().timestamp().to_string();

let verifier = SlackEventSignatureVerifier::new(&TEST_SECRET.to_string().into());
let hash = verifier.sign(TEST_BODY, &test_ts).unwrap();
verifier
.verify(&hash, TEST_BODY, &test_ts)
.expect("signature verification failed");
}

verifier
.verify(TEST_HASH, TEST_BODY, TEST_TS)
.expect("signature verification failed");
}
#[test]
fn test_precoded_data() {
const TEST_SECRET: &str = "fa2c05deeef5a4077494e89f54394de0";
const TEST_HASH: &str =
"v0=baa91ef62d346e56607bbdd278a8acd570b91226854f15a382f43d33be42f666";
const TEST_BODY: &str = "test-body";
const TEST_TS: &str = "1531420618";

let verifier = SlackEventSignatureVerifier::new(&TEST_SECRET.to_string().into());

#[test]
fn check_empty_secret_error_test() {
match SlackEventSignatureVerifier::new(&"".to_string().into()).verify(
"test-hash",
"test-body",
"test-ts",
) {
Err(SlackEventSignatureVerifierError::CryptoInitError(ref err)) => {
assert!(!err.message.is_empty())
verifier
.verify_at_time(TEST_HASH, TEST_BODY, TEST_TS, 1531420620)
.expect("signature verification failed");
}

#[test]
fn check_empty_secret_error_test() {
match SlackEventSignatureVerifier::new(&"".to_string().into()).verify(
"test-hash",
"test-body",
"1531420618",
) {
Err(SlackEventSignatureVerifierError::CryptoInitError(ref err)) => {
assert!(!err.message.is_empty())
}
Err(e) => panic!("{}", e),
_ => unreachable!(),
}
_ => unreachable!(),
}
}

#[test]
fn check_signature_error() {
use sha2::Digest;
#[test]
fn check_signature_error() {
use sha2::Digest;

let key_str_correct: String = hex::encode(Sha256::digest("correct-key"));
let key_str_malicious: String = hex::encode(Sha256::digest("malicious-key"));
let key_str_correct: String = hex::encode(Sha256::digest("correct-key"));
let key_str_malicious: String = hex::encode(Sha256::digest("malicious-key"));

let verifier_correct = SlackEventSignatureVerifier::new(&key_str_correct.into());
let verifier_malicious = SlackEventSignatureVerifier::new(&key_str_malicious.into());
let verifier_correct = SlackEventSignatureVerifier::new(&key_str_correct.into());
let verifier_malicious = SlackEventSignatureVerifier::new(&key_str_malicious.into());

const TEST_BODY: &str = "test-body";
const TEST_TS: &str = "test-ts";
const TEST_BODY: &str = "test-body";
let test_ts = chrono::Utc::now().timestamp().to_string();

let hash = verifier_malicious.sign(TEST_BODY, TEST_TS);
let err = verifier_correct
.verify(&hash, TEST_BODY, TEST_TS)
.unwrap_err();
match err {
SlackEventSignatureVerifierError::WrongSignatureError(_) => {}
_ => panic!("unexpected error, {}", err),
let hash = verifier_malicious.sign(TEST_BODY, &test_ts).unwrap();
let err = verifier_correct
.verify(&hash, TEST_BODY, &test_ts)
.unwrap_err();
match err {
SlackEventSignatureVerifierError::WrongSignatureError(_) => {}
_ => panic!("unexpected error, {}", err),
}
}
}

0 comments on commit e149ae9

Please sign in to comment.