Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat TRUSTED_PROXIES config var #486

Merged
merged 2 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 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 @@ -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"
Expand Down
9 changes: 2 additions & 7 deletions dev_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions rauthy.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/api/src/clients.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
}

Expand Down
54 changes: 23 additions & 31 deletions src/api/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
}

Expand All @@ -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?;

Expand Down
2 changes: 1 addition & 1 deletion src/api/src/fed_cm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
19 changes: 13 additions & 6 deletions src/api/src/generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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 ...?
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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(),
}
}
19 changes: 9 additions & 10 deletions src/api/src/oidc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions src/api/src/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
2 changes: 2 additions & 0 deletions src/common/src/constants.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::utils::build_trusted_proxies;
use crate::DbType;
use actix_web::http::Uri;
use lazy_static::lazy_static;
Expand Down Expand Up @@ -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"))
Expand Down
Loading