Skip to content

Commit

Permalink
Verify the HMAC Tag
Browse files Browse the repository at this point in the history
  • Loading branch information
Taha Afzal committed Aug 29, 2024
1 parent a1c779a commit 1d79268
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 8 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ urlencoding = "2"
htmlescape = "0.3"
hmac = { version = "0.12", features = ["std"] }
sha2 = "0.10"
hex = "0.4"

# We need the optional `derive` feature to use `serde`'s procedural macros:
# `#[derive(Serialize)]` and `#[derive(Deserialize)]`.
Expand Down
8 changes: 8 additions & 0 deletions Notes/Ch6-10.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@
- [Cross-Site Scripting (XSS)](#cross-site-scripting-xss)
- [Message Authentication Codes (MACs)](#message-authentication-codes-macs)
- [Add An HMAC Tag To Protect Query Parameters](#add-an-hmac-tag-to-protect-query-parameters)
- [Verifying the HMAC Tag](#verifying-the-hmac-tag)

# Ch 6 - Reject Invalid Subscribers #1
* Our input validation for `/POST` is limited: we just ensure that both the `name` and the `email` fields are provided, even if they are empty.
Expand Down Expand Up @@ -1242,3 +1243,10 @@ TEST_LOG=true cargo test --quiet --release newsletters_are_delivered | grep "VER
* To get around it, we can move the `hmac_tag` code to the `match_credentials` but we would not be propogating the error context upstream to the middleware chain.
* We can ger around it by using `actix_web::errorr::InternalError`.
* We can now inject the secret used by our HMACs into the application state but `Secret<String>` as the type injected into the application state has risk of conflict -- another middleware or service registring another `Secret<String>` against the application state, overriding our HMAC secret so we can create a wrapper to get around the issue.

#### Verifying the HMAC Tag
* We should verify the HMAC tag in `GET /login` after we extract the tag query parameter.
* We have 2 scenarios:
1. There is no error, so we don't expect any query parameters
2. There is an error to be reported, so we will have both the `error` and the `tag`
* If the verification fails, we can log the verification failure as a warning and skip the error message when rendering the HTML. So, a user being redirected with some dodgy query parameters will see our login page.
47 changes: 39 additions & 8 deletions src/routes/login_form/get.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,52 @@
use actix_web::{http::header::ContentType, web, HttpResponse};
use hmac::{Hmac, Mac};
use secrecy::ExposeSecret;

use super::post::HmacSecret;

#[derive(serde::Deserialize)]
pub struct QueryParams {
error: Option<String>,
error: String,
tag: String,
}

impl QueryParams {
fn verify(self, secret: &HmacSecret) -> Result<String, anyhow::Error> {
let tag = hex::decode(self.tag)?;

let query_string = format!("error={}", urlencoding::Encoded::new(&self.error));
let mut mac =
Hmac::<sha2::Sha256>::new_from_slice(secret.0.expose_secret().as_bytes()).unwrap();
mac.update(query_string.as_bytes());
mac.verify_slice(&tag)?;

Ok(self.error)
}
}

pub async fn login_form(query: web::Query<QueryParams>) -> HttpResponse {
let error_html = match query.0.error {
pub async fn login_form(
query: Option<web::Query<QueryParams>>,
hmac_secret: web::Data<HmacSecret>,
) -> HttpResponse {
let error_html = match query {
None => "".into(),
// Encode the error message to prevent XSS attacks
// since the error message is being injected into the HTML
Some(error_message) => format!(
"<p><i>{}</i></p>",
htmlescape::encode_minimal(&error_message)
),
Some(query) => match query.0.verify(&hmac_secret) {
Ok(error) => {
format!("<p><i>{}</i></p>", htmlescape::encode_minimal(&error))
}
Err(error) => {
tracing::warn!(
error.message = %error,
error.cause_chain = ?error,
"Failed to verify query parameters using the HMAC tag"
);
"".into()
}
},
};

HttpResponse::Ok()
.content_type(ContentType::html())
.body(format!(
Expand Down

0 comments on commit 1d79268

Please sign in to comment.