Merge pull request #486 from sebadob/feat-trusted-proxies

Feat `TRUSTED_PROXIES` config var

Cargo.toml

@@ -38,6 +38,7 @@ bincode = "1"
 cached = "0.51.0"
 chacha20poly1305 = { version = "0.10", features = ["std"] }
 chrono = { version = "0.4", default-features = false, features = ["clock", "serde", "std"] }
+cidr = "0.2.2"
 cron = "0.12"
 cryptr = { version = "0.4", features = ["s3", "streaming"] }
 css-color = "0.2"

dev_notes.md

@@ -8,19 +8,14 @@
 
 ## Stage 2 - features - do before v1.0.0
 
-- `code_challenge` missing on `/authorize` -> show error earlier than after login
-
+- add "request time taken" to access logging middleware
+- possibility to check for existing valid email?
 - prettify the UI
 - check out the possibility to include SCIM
 - update the book with all the new features
 - benchmarks and performance tuning
 - maybe get a nicer logo
 
-### Notes for performance optimizations
-
-- all the `get_`s on the `Client` will probably be good with returning slices instead of real Strings
-  -> less memory allocations
-
 ### `rauthy-client` TODO's
 
 - when implementing userinfo lookup, add an fn to validate the `at_hash` as well

rauthy.cfg

@@ -944,6 +944,15 @@ HTTP_WORKERS=1
 # When rauthy is running behind a reverse proxy, set to true (default: false)
 PROXY_MODE=false
 
+# A `\n` separated list of trusted proxy CIDRs.
+# When `PROXY_MODE=true` or `PEER_IP_HEADER_NAME` is set,
+# these are mandatory to be able to extract the real client
+# IP properly and safely to prevent IP header spoofing.
+# All requests with a different source will be blocked.
+TRUSTED_PROXIES="
+192.168.14.0/24
+"
+
 # To enable or disable the additional HTTP server to expose the /metrics endpoint
 # default: true
 #METRICS_ENABLE=true

src/api/src/clients.rs

@@ -180,7 +180,7 @@ pub async fn post_clients_dyn(
                 .finish());
         }
     } else {
-        let ip = real_ip_from_req(&req).unwrap_or_default();
+        let ip = real_ip_from_req(&req)?;
         ClientDyn::rate_limit_ip(&data, ip).await?;
     }
 

src/api/src/events.rs

@@ -75,37 +75,29 @@ pub async fn sse_events(
 
     params.validate()?;
 
-    match real_ip_from_req(&req) {
-        None => Err(ErrorResponse::new(
-            ErrorResponseType::NotFound,
-            "Cannot extract client IP from HttpRequest. This is an internal network error."
-                .to_string(),
-        )),
-        Some(ip) => {
-            let params = params.into_inner();
-            let (tx, rx) = mpsc::channel(10);
+    let ip = real_ip_from_req(&req)?.to_string();
+    let params = params.into_inner();
+    let (tx, rx) = mpsc::channel(10);
 
-            let level = params.level.map(|l| l.into()).unwrap_or_default();
-            if let Err(err) = data
-                .tx_events_router
-                .send_async(EventRouterMsg::ClientReg {
-                    ip,
-                    tx,
-                    latest: params.latest,
-                    level,
-                })
-                .await
-            {
-                Err(ErrorResponse::new(
-                    ErrorResponseType::Internal,
-                    format!("Cannot register SSE client: {:?}", err),
-                ))
-            } else {
-                Ok(sse::Sse::from_infallible_receiver(rx)
-                    .with_keep_alive(Duration::from_secs(*SSE_KEEP_ALIVE as u64))
-                    .with_retry_duration(Duration::from_secs(10)))
-            }
-        }
+    let level = params.level.map(|l| l.into()).unwrap_or_default();
+    if let Err(err) = data
+        .tx_events_router
+        .send_async(EventRouterMsg::ClientReg {
+            ip,
+            tx,
+            latest: params.latest,
+            level,
+        })
+        .await
+    {
+        Err(ErrorResponse::new(
+            ErrorResponseType::Internal,
+            format!("Cannot register SSE client: {:?}", err),
+        ))
+    } else {
+        Ok(sse::Sse::from_infallible_receiver(rx)
+            .with_keep_alive(Duration::from_secs(*SSE_KEEP_ALIVE as u64))
+            .with_retry_duration(Duration::from_secs(10)))
     }
 }
 
@@ -128,7 +120,7 @@ pub async fn post_event_test(
 ) -> Result<HttpResponse, ErrorResponse> {
     principal.validate_api_key_or_admin_session(AccessGroup::Events, AccessRights::Create)?;
 
-    Event::test(real_ip_from_req(&req))
+    Event::test(real_ip_from_req(&req)?)
         .send(&data.tx_events)
         .await?;
 

src/api/src/fed_cm.rs

@@ -449,7 +449,7 @@ async fn login_status_from_req(
                 }
             };
 
-            if session.is_valid(*SESSION_TIMEOUT_FED_CM, real_ip_from_req(req)) {
+            if session.is_valid(*SESSION_TIMEOUT_FED_CM, real_ip_from_req(req).ok()) {
                 (
                     FedCMLoginStatus::LoggedIn,
                     session.user_id.unwrap_or_default(),

src/api/src/generic.rs

@@ -375,11 +375,12 @@ pub async fn post_migrate_enc_key(
     req_data: actix_web_validator::Json<EncKeyMigrateRequest>,
 ) -> Result<HttpResponse, ErrorResponse> {
     principal.validate_api_key_or_admin_session(AccessGroup::Secrets, AccessRights::Update)?;
+    let ip = real_ip_from_req(&req)?;
 
     encryption::migrate_encryption_alg(&data, &req_data.key_id).await?;
 
     data.tx_events
-        .send_async(Event::secrets_migrated(real_ip_from_req(&req)))
+        .send_async(Event::secrets_migrated(ip))
         .await
         .unwrap();
 
@@ -682,9 +683,12 @@ pub async fn get_ready(data: web::Data<AppState>) -> impl Responder {
 /// If `BLACKLIST_SUSPICIOUS_REQUESTS` is set, it will also compare the
 /// request path against common bot / hacker scan targets and blacklist preemptively.
 #[get("/")]
-pub async fn catch_all(data: web::Data<AppState>, req: HttpRequest) -> impl Responder {
+pub async fn catch_all(
+    data: web::Data<AppState>,
+    req: HttpRequest,
+) -> Result<HttpResponse, ErrorResponse> {
     let path = req.path();
-    let ip = real_ip_from_req(&req).unwrap_or_default();
+    let ip = real_ip_from_req(&req)?.to_string();
 
     if *SUSPICIOUS_REQUESTS_LOG && path.len() > 1 {
         // TODO create a new event type for these? maybe too many events ...?
@@ -714,12 +718,12 @@ pub async fn catch_all(data: web::Data<AppState>, req: HttpRequest) -> impl Resp
         }
     }
 
-    HttpResponse::MovedPermanently()
+    Ok(HttpResponse::MovedPermanently()
         .insert_header((
             header::LOCATION,
             HeaderValue::from_str("/auth/v1/").unwrap(),
         ))
-        .finish()
+        .finish())
 }
 
 #[get("/v1")]
@@ -779,5 +783,8 @@ pub async fn get_version(data: web::Data<AppState>) -> Result<HttpResponse, Erro
 )]
 #[get("/whoami")]
 pub async fn get_whoami(req: HttpRequest) -> String {
-    real_ip_from_req(&req).unwrap_or_default()
+    match real_ip_from_req(&req) {
+        Ok(ip) => ip.to_string(),
+        Err(_) => "UNKNOWN".to_string(),
+    }
 }

src/api/src/oidc.rs

@@ -173,10 +173,10 @@ pub async fn get_authorize(
     let session = if let Some(session) = &principal.session {
         match principal.validate_session_auth_or_init() {
             Ok(_) => session.clone(),
-            Err(_) => Session::new(*SESSION_LIFETIME, real_ip_from_req(&req)),
+            Err(_) => Session::new(*SESSION_LIFETIME, Some(real_ip_from_req(&req)?)),
         }
     } else {
-        Session::new(*SESSION_LIFETIME, real_ip_from_req(&req))
+        Session::new(*SESSION_LIFETIME, Some(real_ip_from_req(&req)?))
     };
 
     if let Err(err) = session.save(&data).await {
@@ -294,7 +294,7 @@ pub async fn post_authorize(
         }
     };
 
-    let ip = real_ip_from_req(&req);
+    let ip = real_ip_from_req(&req)?;
     auth::handle_login_delay(
         &data,
         ip,
@@ -419,8 +419,7 @@ pub async fn post_device_auth(
     // handle ip rate-limiting
     if DEVICE_GRANT_RATE_LIMIT.is_some() {
         match real_ip_from_req(&req) {
-            None => {
-                let err = "Cannot extract client IP for rate-limiting - denying request";
+            Err(err) => {
                 error!("{err}");
                 return HttpResponse::InternalServerError().json(OAuth2ErrorResponse {
                     error: OAuth2ErrorTypeResponse::InvalidRequest,
@@ -429,8 +428,8 @@ pub async fn post_device_auth(
                     )),
                 });
             }
-            Some(ip) => {
-                if let Some(dt) = DeviceIpRateLimit::is_limited(&data, ip.clone()).await {
+            Ok(ip) => {
+                if let Some(dt) = DeviceIpRateLimit::is_limited(&data, ip.to_string()).await {
                     return HttpResponse::TooManyRequests()
                         .insert_header((HEADER_RETRY_NOT_BEFORE, dt.timestamp()))
                         .json(OAuth2ErrorResponse {
@@ -442,7 +441,7 @@ pub async fn post_device_auth(
                         });
                 }
 
-                if let Err(err) = DeviceIpRateLimit::insert(&data, ip).await {
+                if let Err(err) = DeviceIpRateLimit::insert(&data, ip.to_string()).await {
                     error!(
                         "Error inserting IP into the cache for rate-limiting: {:?}",
                         err
@@ -735,7 +734,7 @@ pub async fn post_session(
     data: web::Data<AppState>,
     req: HttpRequest,
 ) -> Result<HttpResponse, ErrorResponse> {
-    let session = Session::new(*SESSION_LIFETIME, real_ip_from_req(&req));
+    let session = Session::new(*SESSION_LIFETIME, real_ip_from_req(&req).ok());
     session.save(&data).await?;
     let cookie = session.client_cookie();
 
@@ -880,7 +879,7 @@ pub async fn post_token(
     data: web::Data<AppState>,
     payload: actix_web_validator::Form<TokenRequest>,
 ) -> Result<HttpResponse, ErrorResponse> {
-    let ip = real_ip_from_req(&req);
+    let ip = real_ip_from_req(&req)?;
 
     if payload.grant_type == GRANT_TYPE_DEVICE_CODE {
         // the `urn:ietf:params:oauth:grant-type:device_code` needs

src/api/src/users.rs

@@ -136,14 +136,17 @@ pub async fn post_users(
     let user = User::create_from_new(&data, user.into_inner()).await?;
 
     data.tx_events
-        .send_async(Event::new_user(user.email.clone(), real_ip_from_req(&req)))
+        .send_async(Event::new_user(
+            user.email.clone(),
+            real_ip_from_req(&req)?.to_string(),
+        ))
         .await
         .unwrap();
     if user.is_admin() {
         data.tx_events
             .send_async(Event::new_rauthy_admin(
                 user.email.clone(),
-                real_ip_from_req(&req),
+                real_ip_from_req(&req)?.to_string(),
             ))
             .await
             .unwrap();
@@ -334,7 +337,10 @@ pub async fn post_users_register(
     let user = User::create_from_reg(&data, req_data.into_inner(), lang).await?;
 
     data.tx_events
-        .send_async(Event::new_user(user.email, real_ip_from_req(&req)))
+        .send_async(Event::new_user(
+            user.email,
+            real_ip_from_req(&req)?.to_string(),
+        ))
         .await
         .unwrap();
 
@@ -1212,7 +1218,7 @@ pub async fn put_user_by_id(
         data.tx_events
             .send_async(Event::new_rauthy_admin(
                 user.email.clone(),
-                real_ip_from_req(&req),
+                real_ip_from_req(&req)?.to_string(),
             ))
             .await
             .unwrap();

src/common/Cargo.toml

@@ -13,6 +13,7 @@ argon2 = { workspace = true }
 base64 = { workspace = true }
 chacha20poly1305 = { workspace = true }
 chrono = { workspace = true }
+cidr = { workspace = true }
 flume = { workspace = true }
 gethostname = { workspace = true }
 lazy_static = { workspace = true }

src/common/src/constants.rs

@@ -1,3 +1,4 @@
+use crate::utils::build_trusted_proxies;
 use crate::DbType;
 use actix_web::http::Uri;
 use lazy_static::lazy_static;
@@ -335,6 +336,7 @@ lazy_static! {
         .unwrap_or_else(|_| String::from("false"))
         .parse::<bool>()
         .unwrap_or(true);
+    pub static ref TRUSTED_PROXIES: Vec<cidr::IpCidr> = build_trusted_proxies();
 
     pub static ref OPEN_USER_REG: bool = env::var("OPEN_USER_REG")
         .unwrap_or_else(|_| String::from("false"))