From ec2ec56b9f422125b29df04fdc7c78f8a62cfdcc Mon Sep 17 00:00:00 2001 From: xDarksome Date: Thu, 19 Oct 2023 15:45:57 +0000 Subject: [PATCH] fix: CsrfToken::validate_format checking only JWT header (#52) --- Cargo.lock | 2 +- integration/integration.test.ts | 2 +- src/http_server/attestation.rs | 2 +- src/http_server/mod.rs | 38 ++++++++++++++++++++++++++++----- src/main.rs | 5 +---- 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 135a3b8..a501be8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -635,7 +635,7 @@ dependencies = [ [[package]] name = "bouncer" -version = "0.28.3" +version = "0.29.0" dependencies = [ "anyhow", "arrayvec", diff --git a/integration/integration.test.ts b/integration/integration.test.ts index 099c3be..2158298 100644 --- a/integration/integration.test.ts +++ b/integration/integration.test.ts @@ -160,7 +160,7 @@ describe('verify', () => { }) it('doesn\'t allow invalid `token` parameters', async () => { - let resp = await http.get(`${url}/index.js?token=`) + let resp = await http.get(`${url}/index.js?token=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.`) expect(resp.status).toBe(400) }) }) diff --git a/src/http_server/attestation.rs b/src/http_server/attestation.rs index 3e7773f..c75bdd6 100644 --- a/src/http_server/attestation.rs +++ b/src/http_server/attestation.rs @@ -47,7 +47,7 @@ pub(super) async fn post( headers: HeaderMap, body: Json, ) -> Result { - s.validate_csrf_token(&headers)?; + s.token_manager.validate_csrf_token(&headers)?; s.bouncer .set_attestation(&body.attestation_id, &body.origin) diff --git a/src/http_server/mod.rs b/src/http_server/mod.rs index 94b961b..3da95e8 100644 --- a/src/http_server/mod.rs +++ b/src/http_server/mod.rs @@ -33,10 +33,23 @@ mod metrics; struct Server { bouncer: B, + token_manager: TokenManager, +} + +struct TokenManager { encoding_key: jsonwebtoken::EncodingKey, decoding_key: jsonwebtoken::DecodingKey, } +impl TokenManager { + fn new(secret: &[u8]) -> Self { + Self { + encoding_key: jsonwebtoken::EncodingKey::from_secret(secret), + decoding_key: jsonwebtoken::DecodingKey::from_secret(secret), + } + } +} + type State = axum::extract::State>>; pub async fn run( @@ -66,8 +79,7 @@ pub async fn run( let state = Server { bouncer: app, - encoding_key: jsonwebtoken::EncodingKey::from_secret(secret), - decoding_key: jsonwebtoken::DecodingKey::from_secret(secret), + token_manager: TokenManager::new(secret), }; let server: Router = Router::new() @@ -125,7 +137,7 @@ async fn root( Ok(match s.bouncer.get_verify_status(project_id).await? { VerifyStatus::Disabled => String::new().into_response(), VerifyStatus::Enabled { verified_domains } => { - let token = s.generate_csrf_token()?; + let token = s.token_manager.generate_csrf_token()?; let html = index_html(&token); let csp = build_content_security_header(verified_domains); let headers = [ @@ -163,11 +175,12 @@ impl CsrfToken { /// Validates the format of the token without checking either signature or /// claims. fn validate_format(s: &str) -> bool { - jsonwebtoken::decode_header(s).is_ok() + s.chars() + .all(|c| c.is_ascii_alphanumeric() | matches!(c, '.' | '-' | '_')) } } -impl Server { +impl TokenManager { fn generate_csrf_token(&self) -> Result { use jsonwebtoken::{encode, get_current_timestamp, Header}; @@ -233,3 +246,18 @@ fn test_build_content_security_header() { http://*.localhost http://localhost", ); } + +#[test] +fn generated_csrf_tokens_are_valid() { + let tm = TokenManager::new(&[]); + let token = tm.generate_csrf_token().unwrap(); + assert!(CsrfToken::validate_format(&token)) +} + +#[test] +fn csrf_validation_checks_jwt_header_and_payload() { + let valid_header_invalid_payload = + "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9."; + + assert!(!CsrfToken::validate_format(&valid_header_invalid_payload)) +} diff --git a/src/main.rs b/src/main.rs index cabd417..a29c964 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,10 +18,7 @@ use { tap::TapFallible, tokio::signal::unix::{signal, SignalKind}, tracing::info, - wc::geoip::{ - block::{middleware::GeoBlockLayer, BlockingPolicy}, - MaxMindResolver, - }, + wc::geoip::MaxMindResolver, }; #[derive(Deserialize, Debug, Clone)]