Merge pull request #143 from sebadob/impl-dpop-nonces

Impl dpop nonces
sebadob · Nov 3, 2023 · 29dce66 · 29dce66
2 parents 4329303 + 6b2ac39
commit 29dce66
Show file tree

Hide file tree

Showing 9 changed files with 417 additions and 155 deletions.
diff --git a/rauthy-common/src/constants.rs b/rauthy-common/src/constants.rs
@@ -7,6 +7,8 @@ use std::str::FromStr;
 use std::string::ToString;
 
 pub const RAUTHY_VERSION: &str = env!("CARGO_PKG_VERSION");
+
+pub const HEADER_DPOP_NONCE: &str = "DPoP-Nonce";
 pub const HEADER_HTML: (&str, &str) = ("content-type", "text/html;charset=utf-8");
 pub const HEADER_RETRY_NOT_BEFORE: &str = "x-retry-not-before";
 pub const APPLICATION_JSON: &str = "application/json";
@@ -30,6 +32,7 @@ pub const EVENTS_LATEST_LIMIT: u16 = 100;
 
 pub const CACHE_NAME_12HR: &str = "12hr";
 pub const CACHE_NAME_AUTH_CODES: &str = "auth-codes";
+pub const CACHE_NAME_DPOP_NONCES: &str = "dpop-nonces";
 pub const CACHE_NAME_LOGIN_DELAY: &str = "login-dly";
 pub const CACHE_NAME_SESSIONS: &str = "sessions";
 pub const CACHE_NAME_POW: &str = "pow";
@@ -99,6 +102,10 @@ lazy_static! {
         let uri = format!("{}://{}/auth/v1/oidc/token", scheme, pub_url);
         Uri::from_str(&uri).unwrap()
     };
+    pub static ref DPOP_FORCE_NONCE: bool = env::var("DPOP_NONCE_FORCE")
+        .unwrap_or_else(|_| String::from("true"))
+        .parse::<bool>()
+        .unwrap_or(true);
 
     pub static ref PROXY_MODE: bool = env::var("PROXY_MODE")
         .unwrap_or_else(|_| String::from("false"))
@@ -144,6 +151,12 @@ lazy_static! {
         .parse::<bool>()
         .expect("ADMIN_FORCE_MFA cannot be parsed to bool - bad format");
 
+    pub static ref DPOP_NONCE_EXP: u32 = env::var("DPOP_NONCE_EXP")
+        .unwrap_or_else(|_| String::from("900"))
+        // parsing to u32 to be able to typecast to i64 for chrono safely
+        .parse::<u32>()
+        .expect("DPOP_NONCE_EXP cannot be parsed to u32 - bad format");
+
     pub static ref SESSION_LIFETIME: u32 = env::var("SESSION_LIFETIME")
         .unwrap_or_else(|_| String::from("14400"))
         .parse::<u32>()

diff --git a/rauthy-common/src/error_response.rs b/rauthy-common/src/error_response.rs
@@ -1,29 +1,35 @@
-use crate::constants::{APPLICATION_JSON, HEADER_HTML, HEADER_RETRY_NOT_BEFORE};
+use crate::constants::{APPLICATION_JSON, HEADER_DPOP_NONCE, HEADER_HTML, HEADER_RETRY_NOT_BEFORE};
 use crate::utils::build_csp_header;
 use actix_multipart::MultipartError;
 use actix_web::error::BlockingError;
-use actix_web::http::header::{ToStrError, WWW_AUTHENTICATE};
+use actix_web::http::header::{
+    ToStrError, ACCESS_CONTROL_ALLOW_ORIGIN, ACCESS_CONTROL_EXPOSE_HEADERS, WWW_AUTHENTICATE,
+};
 use actix_web::http::StatusCode;
 use actix_web::{HttpResponse, HttpResponseBuilder, ResponseError};
 use css_color::ParseColorError;
 use derive_more::Display;
 use redhac::CacheError;
 use serde::{Deserialize, Serialize};
 use serde_json::Error;
+use std::fmt::{Display, Formatter};
 use std::string::FromUtf8Error;
 use time::OffsetDateTime;
 use tracing::{debug, error};
 use utoipa::ToSchema;
 
-#[derive(Debug, Clone, Display, PartialEq, Eq, Serialize, Deserialize, ToSchema)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, ToSchema)]
 pub enum ErrorResponseType {
     BadRequest,
     Connection,
     CSRFTokenError,
     Database,
     DatabaseIo,
     Disabled,
-    DPoP,
+    // These String could be optimized in the future with borrowing
+    // -> just not going down that rabbit hole for now
+    DPoP(Option<String>),
+    UseDpopNonce((Option<String>, String)),
     Forbidden,
     Internal,
     JoseError,
@@ -38,6 +44,12 @@ pub enum ErrorResponseType {
     Unauthorized,
 }
 
+impl Display for ErrorResponseType {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{:?}", self)
+    }
+}
+
 // This is the default `ErrorResponse` that could be the answer on almost every API endpoint in
 // case something is wrong.<br>
 // Except for input validations, every error will have this format and every possible error in the
@@ -70,13 +82,15 @@ impl ErrorResponse {
 impl ResponseError for ErrorResponse {
     fn status_code(&self) -> StatusCode {
         match self.error {
-            ErrorResponseType::BadRequest => StatusCode::BAD_REQUEST,
+            ErrorResponseType::BadRequest | ErrorResponseType::UseDpopNonce(_) => {
+                StatusCode::BAD_REQUEST
+            }
             ErrorResponseType::Forbidden => StatusCode::FORBIDDEN,
             ErrorResponseType::MfaRequired => StatusCode::NOT_ACCEPTABLE,
             ErrorResponseType::NotFound => StatusCode::NOT_FOUND,
             ErrorResponseType::Disabled
             | ErrorResponseType::CSRFTokenError
-            | ErrorResponseType::DPoP
+            | ErrorResponseType::DPoP(_)
             | ErrorResponseType::PasswordExpired
             | ErrorResponseType::SessionExpired
             | ErrorResponseType::SessionTimeout
@@ -90,32 +104,60 @@ impl ResponseError for ErrorResponse {
 
     fn error_response(&self) -> HttpResponse {
         let status = self.status_code();
-
-        match self.error {
+        match &self.error {
             ErrorResponseType::TooManyRequests(not_before_timestamp) => {
-                HttpResponseBuilder::new(self.status_code())
-                    .append_header((HEADER_RETRY_NOT_BEFORE, not_before_timestamp))
-                    .append_header(HEADER_HTML)
+                HttpResponseBuilder::new(status)
+                    .insert_header((HEADER_RETRY_NOT_BEFORE, *not_before_timestamp))
+                    .insert_header(HEADER_HTML)
                     // TODO we could possibly do a small `unsafe` call here to just take
                     // the content without cloning it -> more efficient, especially for blocked IPs
                     .body(self.message.clone())
             }
 
-            ErrorResponseType::DPoP => HttpResponseBuilder::new(self.status_code())
-                .append_header((WWW_AUTHENTICATE, "DPoP error=invalid_dpop_proof"))
-                .content_type(APPLICATION_JSON)
-                .body(serde_json::to_string(&self).unwrap()),
+            ErrorResponseType::DPoP(header_origen) => {
+                if let Some(origen) = header_origen {
+                    HttpResponseBuilder::new(status)
+                        .insert_header((WWW_AUTHENTICATE, "DPoP error=invalid_dpop_proof"))
+                        .content_type(APPLICATION_JSON)
+                        .insert_header((ACCESS_CONTROL_ALLOW_ORIGIN, origen.as_str()))
+                        .body(serde_json::to_string(&self).unwrap())
+                } else {
+                    HttpResponseBuilder::new(status)
+                        .insert_header((WWW_AUTHENTICATE, "DPoP error=invalid_dpop_proof"))
+                        .content_type(APPLICATION_JSON)
+                        .body(serde_json::to_string(&self).unwrap())
+                }
+            }
+
+            ErrorResponseType::UseDpopNonce((header_origen, value)) => {
+                if let Some(origen) = header_origen {
+                    HttpResponseBuilder::new(status)
+                        .insert_header((WWW_AUTHENTICATE, "DPoP error=use_dpop_nonce"))
+                        .insert_header((HEADER_DPOP_NONCE, value.as_str()))
+                        .content_type(APPLICATION_JSON)
+                        .insert_header((ACCESS_CONTROL_ALLOW_ORIGIN, origen.as_str()))
+                        .insert_header((ACCESS_CONTROL_EXPOSE_HEADERS, HEADER_DPOP_NONCE))
+                        .content_type(APPLICATION_JSON)
+                        .body(serde_json::to_string(&self).unwrap())
+                } else {
+                    HttpResponseBuilder::new(status)
+                        .insert_header((WWW_AUTHENTICATE, "DPoP error=use_dpop_nonce"))
+                        .insert_header((HEADER_DPOP_NONCE, value.as_str()))
+                        .content_type(APPLICATION_JSON)
+                        .body(serde_json::to_string(&self).unwrap())
+                }
+            }
 
             _ => {
                 if status == StatusCode::UNAUTHORIZED {
-                    HttpResponseBuilder::new(self.status_code())
+                    HttpResponseBuilder::new(status)
                         .append_header((WWW_AUTHENTICATE, "OAuth"))
                         .content_type(APPLICATION_JSON)
-                        .body(serde_json::to_string(&self).unwrap())
+                        .body(serde_json::to_string(self).unwrap())
                 } else {
-                    HttpResponseBuilder::new(self.status_code())
+                    HttpResponseBuilder::new(status)
                         .content_type(APPLICATION_JSON)
-                        .body(serde_json::to_string(&self).unwrap())
+                        .body(serde_json::to_string(self).unwrap())
                 }
             }
         }

diff --git a/rauthy-handlers/src/oidc.rs b/rauthy-handlers/src/oidc.rs
@@ -4,7 +4,7 @@ use std::time::{SystemTime, UNIX_EPOCH};
 use actix_web::cookie::time::OffsetDateTime;
 use actix_web::http::header::HeaderValue;
 use actix_web::http::{header, StatusCode};
-use actix_web::{get, post, web, HttpRequest, HttpResponse, ResponseError};
+use actix_web::{get, post, web, HttpRequest, HttpResponse, HttpResponseBuilder, ResponseError};
 use tracing::debug;
 
 use rauthy_common::constants::{COOKIE_MFA, HEADER_HTML, SESSION_LIFETIME};
@@ -529,18 +529,13 @@ pub async fn post_token(
     let ip = real_ip_from_req(&req);
 
     let res = match auth::get_token_set(req_data.into_inner(), &data, req).await {
-        Ok((token_set, header_origen)) => {
-            let http_resp = if let Some(o) = header_origen {
-                HttpResponse::Ok()
-                    .insert_header(o)
-                    // .insert_header(csrf_header)
-                    .json(token_set)
-            } else {
-                HttpResponse::Ok()
-                    // .insert_header(csrf_header)
-                    .json(token_set)
-            };
-            Ok((http_resp, add_login_delay))
+        Ok((token_set, headers)) => {
+            let mut builder = HttpResponseBuilder::new(StatusCode::OK);
+            for h in headers {
+                builder.insert_header(h);
+            }
+            let resp = builder.json(token_set);
+            Ok((resp, add_login_delay))
         }
         Err(err) => Err((err, add_login_delay)),
     };

diff --git a/rauthy-main/src/main.rs b/rauthy-main/src/main.rs
@@ -19,9 +19,10 @@ use actix_web::{middleware, web, App, HttpServer};
 use actix_web_prom::PrometheusMetricsBuilder;
 use prometheus::Registry;
 use rauthy_common::constants::{
-    CACHE_NAME_12HR, CACHE_NAME_AUTH_CODES, CACHE_NAME_LOGIN_DELAY, CACHE_NAME_POW,
-    CACHE_NAME_SESSIONS, CACHE_NAME_WEBAUTHN, CACHE_NAME_WEBAUTHN_DATA, POW_EXP, RAUTHY_VERSION,
-    SWAGGER_UI_EXTERNAL, SWAGGER_UI_INTERNAL, WEBAUTHN_DATA_EXP, WEBAUTHN_REQ_EXP,
+    CACHE_NAME_12HR, CACHE_NAME_AUTH_CODES, CACHE_NAME_DPOP_NONCES, CACHE_NAME_LOGIN_DELAY,
+    CACHE_NAME_POW, CACHE_NAME_SESSIONS, CACHE_NAME_WEBAUTHN, CACHE_NAME_WEBAUTHN_DATA,
+    DPOP_NONCE_EXP, POW_EXP, RAUTHY_VERSION, SWAGGER_UI_EXTERNAL, SWAGGER_UI_INTERNAL,
+    WEBAUTHN_DATA_EXP, WEBAUTHN_REQ_EXP,
 };
 use rauthy_common::password_hasher;
 use rauthy_handlers::middleware::ip_blacklist::RauthyIpBlacklistMiddleware;
@@ -112,6 +113,13 @@ async fn main() -> Result<(), Box<dyn Error>> {
         Some(64),
     );
 
+    // DPoP nonces
+    cache_config.spawn_cache(
+        CACHE_NAME_DPOP_NONCES.to_string(),
+        redhac::TimedCache::with_lifespan(*DPOP_NONCE_EXP as u64),
+        None,
+    );
+
     // sessions
     let session_lifetime = env::var("SESSION_LIFETIME")
         .unwrap_or_else(|_| String::from("14400"))

diff --git a/rauthy-main/tests/handler_auth.rs b/rauthy-main/tests/handler_auth.rs
@@ -7,7 +7,7 @@ use chrono::Utc;
 use ed25519_compact::Noise;
 use josekit::jwk;
 use pretty_assertions::assert_eq;
-use rauthy_common::constants::{DPOP_TOKEN_ENDPOINT, TOKEN_DPOP};
+use rauthy_common::constants::{DPOP_TOKEN_ENDPOINT, HEADER_DPOP_NONCE, TOKEN_DPOP};
 use rauthy_common::error_response::{ErrorResponse, ErrorResponseType};
 use rauthy_common::utils::{base64_url_encode, base64_url_no_pad_encode, get_rand};
 use rauthy_models::entity::dpop_proof::{DPoPClaims, DPoPHeader};
@@ -586,13 +586,13 @@ async fn test_dpop() -> Result<(), Box<dyn Error>> {
         },
         kid: None,
     };
-    let claims = DPoPClaims {
+    let mut claims = DPoPClaims {
         jti: "-BwC3ESc6acc2lTc".to_string(),
         htm: http::Method::POST,
         htu: DPOP_TOKEN_ENDPOINT.clone(),
         iat: Utc::now().timestamp(),
         // ath: None,
-        // nonce: None,
+        nonce: None,
     };
 
     let fingerprint = header.jwk.fingerprint().unwrap();
@@ -606,15 +606,52 @@ async fn test_dpop() -> Result<(), Box<dyn Error>> {
     let sig = kp.sk.sign(&dpop_token, Some(Noise::generate()));
     let sig_b64 = base64_url_no_pad_encode(sig.as_ref());
     write!(dpop_token, ".{}", sig_b64).unwrap();
-    println!("test signed token:\n{}", dpop_token);
+    println!("test signed token without nonce:\n{}", dpop_token);
+
+    let res = client
+        .post(&url)
+        .header(TOKEN_DPOP, &dpop_token)
+        .form(&body)
+        .send()
+        .await?;
+    // this should fail, because nonce is enforced, which we did not provide
+    assert_eq!(res.status(), 400);
+    let nonce = res
+        .headers()
+        .get(HEADER_DPOP_NONCE)
+        .unwrap()
+        .to_str()
+        .unwrap();
+    println!("nonce we should use: {}", nonce);
+
+    // insert the nonce and rebuild
+    claims.nonce = Some(nonce.to_string());
+
+    let claims_json = serde_json::to_string(&claims).unwrap();
+    let claims_b64 = base64_url_no_pad_encode(claims_json.as_bytes());
+    let mut dpop_token = format!("{}.{}", header_b64, claims_b64);
+
+    let sig = kp.sk.sign(&dpop_token, Some(Noise::generate()));
+    let sig_b64 = base64_url_no_pad_encode(sig.as_ref());
+    write!(dpop_token, ".{}", sig_b64).unwrap();
+    println!("test signed token with nonce:\n{}", dpop_token);
 
     let mut res = client
         .post(&url)
         .header(TOKEN_DPOP, &dpop_token)
         .form(&body)
         .send()
         .await?;
+    // not it should be good
     res = check_status(res, 200).await?;
+    // make sure nonce header is still there
+    let nonce_new = res
+        .headers()
+        .get(HEADER_DPOP_NONCE)
+        .unwrap()
+        .to_str()
+        .unwrap();
+    assert_eq!(nonce, nonce_new);
 
     let ts = res.json::<TokenSet>().await?;
     assert_eq!(ts.token_type, JwtTokenType::DPoP);