Skip to content

Commit

Permalink
Merge pull request #63 from sebadob/impl-session-ip-validator
Browse files Browse the repository at this point in the history
Impl session ip validator
  • Loading branch information
sebadob committed Sep 29, 2023
2 parents 828bcd2 + 9de4452 commit 26924ff
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 16 deletions.
2 changes: 0 additions & 2 deletions dev_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ in another terminal:

### Stage 2 - features - do before v1.0.0

- add session remote UI in UI
- add env var to additionally check session remote IP each time? makes sense?
- add a new table that keeps track about when password expiry / reset emails were sent out to avoid duplicates
- when a user changes his email address, set email to not verified again and send a validation email
- add tracing-actix-web + opentelemetry
Expand Down
5 changes: 5 additions & 0 deletions rauthy-common/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ lazy_static! {
.parse::<u32>()
.expect("SESSION_TIMEOUT cannot be parsed to u32 - bad format");

pub static ref SESSION_VALIDATE_IP: bool = env::var("SESSION_VALIDATE_IP")
.unwrap_or_else(|_| String::from("true"))
.parse::<bool>()
.expect("SESSION_VALIDATE_IP cannot be parsed to bool - bad format");

pub static ref SMTP_USERNAME: String = env::var("SMTP_USERNAME")
.expect("SMTP_USERNAME is not set")
.trim()
Expand Down
12 changes: 12 additions & 0 deletions rauthy-handlers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use actix_web::dev::ServiceRequest;
use actix_web::{web, HttpRequest, HttpResponse};
use rauthy_common::constants::{COOKIE_MFA, PROXY_MODE};
use rauthy_common::error_response::{ErrorResponse, ErrorResponseType};
Expand Down Expand Up @@ -136,3 +137,14 @@ pub fn real_ip_from_req(req: &HttpRequest) -> Option<String> {
req.connection_info().peer_addr().map(|ip| ip.to_string())
}
}

pub fn real_ip_from_svc_req(req: &ServiceRequest) -> Option<String> {
if *PROXY_MODE {
// TODO maybe make this configurable and extract headers in user-configured order?
req.connection_info()
.realip_remote_addr()
.map(|ip| ip.to_string())
} else {
req.connection_info().peer_addr().map(|ip| ip.to_string())
}
}
12 changes: 2 additions & 10 deletions rauthy-handlers/src/middleware/logging.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::real_ip_from_svc_req;
use actix_web::http::header::HeaderValue;
use actix_web::http::{Method, Uri};
use actix_web::{
Expand All @@ -6,7 +7,6 @@ use actix_web::{
};
use futures::future::LocalBoxFuture;
use lazy_static::lazy_static;
use rauthy_common::constants::PROXY_MODE;
use rauthy_common::error_response::{ErrorResponse, ErrorResponseType};
use std::env;
use std::future::{ready, Ready};
Expand Down Expand Up @@ -111,15 +111,7 @@ async fn handle_req(req: &ServiceRequest) -> Result<(), ErrorResponse> {

async fn log_access(uri: &Uri, req: &ServiceRequest) -> Result<(), ErrorResponse> {
let path = uri.path();
let ip = {
let conn_info = req.connection_info();
let ip = if *PROXY_MODE {
conn_info.realip_remote_addr()
} else {
conn_info.peer_addr()
};
ip.unwrap_or("<UNKNOWN>").to_string()
};
let ip = real_ip_from_svc_req(req).unwrap_or_else(|| "<UNKNOWN>".to_string());

match *LOG_LEVEL_ACCESS {
LogLevelAccess::Debug => {
Expand Down
10 changes: 8 additions & 2 deletions rauthy-handlers/src/middleware/session.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::real_ip_from_svc_req;
use actix_web::{
dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform},
web, Error, HttpMessage,
};
use futures::future::LocalBoxFuture;
use rauthy_common::constants::COOKIE_SESSION;
use rauthy_common::constants::{COOKIE_SESSION, SESSION_VALIDATE_IP};
use rauthy_common::error_response::ErrorResponse;
use rauthy_models::app_state::AppState;
use rauthy_models::entity::sessions::Session;
Expand Down Expand Up @@ -82,7 +83,12 @@ async fn get_session_from_cookie(req: &ServiceRequest) -> Result<Option<Session>

match Session::find(data, session_id.to_string()).await {
Ok(mut session) => {
if session.is_valid(data.session_timeout) {
let remote_ip = if *SESSION_VALIDATE_IP {
real_ip_from_svc_req(req)
} else {
None
};
if session.is_valid(data.session_timeout, remote_ip) {
let now = OffsetDateTime::now_utc().unix_timestamp();
// only update the last_seen, if it is older than 10 seconds
if session.last_seen < now - 10 {
Expand Down
17 changes: 15 additions & 2 deletions rauthy-models/src/entity/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use sqlx::{FromRow, Row};
use std::ops::Add;
use std::str::FromStr;
use time::OffsetDateTime;
use tracing::error;
use tracing::{error, warn};
use utoipa::ToSchema;

#[derive(Debug, Clone, FromRow, Serialize, Deserialize)]
Expand Down Expand Up @@ -416,7 +416,7 @@ impl Session {
}

/// Checks if the current session is valid: has not expired and has not timed out (last_seen)
pub fn is_valid(&self, session_timeout: u32) -> bool {
pub fn is_valid(&self, session_timeout: u32, remote_ip: Option<String>) -> bool {
let now = OffsetDateTime::now_utc().unix_timestamp();
if self.exp < now {
return false;
Expand All @@ -427,6 +427,19 @@ impl Session {
if self.state == SessionState::Unknown {
return false;
}
if let Some(ip) = remote_ip {
if (self.state == SessionState::Open || self.state == SessionState::Auth)
&& self.remote_ip.as_ref() != Some(&ip)
{
warn!(
"Invalid access for session {} / {} with different IP: {}",
self.id,
self.remote_ip.as_ref().unwrap(),
ip,
);
return false;
}
}
true
}

Expand Down
18 changes: 18 additions & 0 deletions rauthy.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,24 @@ OPEN_USER_REG=true
# default: ''
#USER_REG_DOMAIN_RESTRICTION=@some-mail-domain.com

# If set to 'true', this will validate the remote peer IP address with each request and compare it with the
# IP which was used during the initial session creation / login.
# If the IP is different, the session will be rejected.
# This is a security hardening and prevents stolen access credentials, for instance if an attacker might have
# copied the encrypted session cookie and the XSRF token from the local storage from a user. However, this event
# is really unlikely, since it may only happen if an attacker has direct access to the machine itself.
#
# If your users are using mobile networks and get new IP addresses all the time, this means they have to do a
# new login each time. This is no big deal at all with Webauthn / FIDO keys anyway and should not be a reason
# to deactivate this feature.
#
# Caution: If you are running behind a reverse proxy which does not provide the X-FORWARDED-FOR header correctly,
# or you have the PROXY_MODE in this config disabled, this feature will not work. You can validate the IPs for
# each session in the Admin UI. If these are correct, your setup is okay.
#
# (default: true)
SESSION_VALIDATE_IP=true

#####################################
############# BACKUPS ###############
#####################################
Expand Down

0 comments on commit 26924ff

Please sign in to comment.