Browse Source

Ensure SSO token is only usable on the same client

This commit adds an extra check via cookies to ensure the same browser/client is used to request and provide the SSO token.
Previously it would be able to provide a custom link which attackers could use to steal data.
While an attacker would still need the Master Password to be able to decrypt or execute specific actions, they were able to fetch encrypted data.

Solved with some help of Claude Code.

Signed-off-by: BlackDex <black.dex@gmail.com>
pull/7163/head
BlackDex 4 weeks ago
parent
commit
9296b24c43
No known key found for this signature in database GPG Key ID: 58C80A2AA6C765E1
  1. 1
      migrations/mysql/2026-04-25-120000_sso_auth_binding/down.sql
  2. 1
      migrations/mysql/2026-04-25-120000_sso_auth_binding/up.sql
  3. 1
      migrations/postgresql/2026-04-25-120000_sso_auth_binding/down.sql
  4. 1
      migrations/postgresql/2026-04-25-120000_sso_auth_binding/up.sql
  5. 1
      migrations/sqlite/2026-04-25-120000_sso_auth_binding/down.sql
  6. 1
      migrations/sqlite/2026-04-25-120000_sso_auth_binding/up.sql
  7. 43
      src/api/identity.rs
  8. 7
      src/crypto.rs
  9. 10
      src/db/models/sso_auth.rs
  10. 1
      src/db/schema.rs
  11. 3
      src/sso.rs
  12. 3
      src/sso_client.rs

1
migrations/mysql/2026-04-25-120000_sso_auth_binding/down.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth DROP COLUMN binding_hash;

1
migrations/mysql/2026-04-25-120000_sso_auth_binding/up.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT;

1
migrations/postgresql/2026-04-25-120000_sso_auth_binding/down.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth DROP COLUMN binding_hash;

1
migrations/postgresql/2026-04-25-120000_sso_auth_binding/up.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT;

1
migrations/sqlite/2026-04-25-120000_sso_auth_binding/down.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth DROP COLUMN binding_hash;

1
migrations/sqlite/2026-04-25-120000_sso_auth_binding/up.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT;

43
src/api/identity.rs

@ -2,6 +2,7 @@ use chrono::Utc;
use num_traits::FromPrimitive; use num_traits::FromPrimitive;
use rocket::{ use rocket::{
form::{Form, FromForm}, form::{Form, FromForm},
http::{Cookie, CookieJar, SameSite},
response::Redirect, response::Redirect,
serde::json::Json, serde::json::Json,
Route, Route,
@ -23,7 +24,8 @@ use crate::{
ApiResult, EmptyResult, JsonResult, ApiResult, EmptyResult, JsonResult,
}, },
auth, auth,
auth::{generate_organization_api_key_login_claims, AuthMethod, ClientHeaders, ClientIp, ClientVersion}, auth::{generate_organization_api_key_login_claims, AuthMethod, ClientHeaders, ClientIp, ClientVersion, Secure},
crypto,
db::{ db::{
models::{ models::{
AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeWrapper, OrganizationApiKey, AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeWrapper, OrganizationApiKey,
@ -1133,13 +1135,16 @@ fn prevalidate() -> JsonResult {
} }
} }
const SSO_BINDING_COOKIE: &str = "VW_SSO_BINDING";
#[get("/connect/oidc-signin?<code>&<state>", rank = 1)] #[get("/connect/oidc-signin?<code>&<state>", rank = 1)]
async fn oidcsignin(code: OIDCCode, state: String, mut conn: DbConn) -> ApiResult<Redirect> { async fn oidcsignin(code: OIDCCode, state: String, cookies: &CookieJar<'_>, mut conn: DbConn) -> ApiResult<Redirect> {
_oidcsignin_redirect( _oidcsignin_redirect(
state, state,
OIDCCodeWrapper::Ok { OIDCCodeWrapper::Ok {
code, code,
}, },
cookies,
&mut conn, &mut conn,
) )
.await .await
@ -1152,6 +1157,7 @@ async fn oidcsignin_error(
state: String, state: String,
error: String, error: String,
error_description: Option<String>, error_description: Option<String>,
cookies: &CookieJar<'_>,
mut conn: DbConn, mut conn: DbConn,
) -> ApiResult<Redirect> { ) -> ApiResult<Redirect> {
_oidcsignin_redirect( _oidcsignin_redirect(
@ -1160,6 +1166,7 @@ async fn oidcsignin_error(
error, error,
error_description, error_description,
}, },
cookies,
&mut conn, &mut conn,
) )
.await .await
@ -1171,6 +1178,7 @@ async fn oidcsignin_error(
async fn _oidcsignin_redirect( async fn _oidcsignin_redirect(
base64_state: String, base64_state: String,
code_response: OIDCCodeWrapper, code_response: OIDCCodeWrapper,
cookies: &CookieJar<'_>,
conn: &mut DbConn, conn: &mut DbConn,
) -> ApiResult<Redirect> { ) -> ApiResult<Redirect> {
let state = sso::decode_state(&base64_state)?; let state = sso::decode_state(&base64_state)?;
@ -1179,6 +1187,17 @@ async fn _oidcsignin_redirect(
None => err!(format!("Cannot retrieve sso_auth for {state}")), None => err!(format!("Cannot retrieve sso_auth for {state}")),
Some(sso_auth) => sso_auth, Some(sso_auth) => sso_auth,
}; };
// Browser-binding check
// The cookie was set on /connect/authorize and must come from the same browser that initiated the flow.
let cookie_value = cookies.get(SSO_BINDING_COOKIE).map(|c| c.value().to_string());
let provided_hash = cookie_value.as_deref().map(|v| crypto::sha256_hex(v.as_bytes()));
match (sso_auth.binding_hash.as_deref(), provided_hash.as_deref()) {
(Some(expected), Some(actual)) if crypto::ct_eq(expected, actual) => {}
_ => err!(format!("SSO session binding mismatch for {state}")),
}
cookies.remove(Cookie::build(SSO_BINDING_COOKIE).path("/identity/connect/").build());
sso_auth.code_response = Some(code_response); sso_auth.code_response = Some(code_response);
sso_auth.updated_at = Utc::now().naive_utc(); sso_auth.updated_at = Utc::now().naive_utc();
sso_auth.save(conn).await?; sso_auth.save(conn).await?;
@ -1225,7 +1244,7 @@ struct AuthorizeData {
// The `redirect_uri` will change depending of the client (web, android, ios ..) // The `redirect_uri` will change depending of the client (web, android, ios ..)
#[get("/connect/authorize?<data..>")] #[get("/connect/authorize?<data..>")]
async fn authorize(data: AuthorizeData, conn: DbConn) -> ApiResult<Redirect> { async fn authorize(data: AuthorizeData, cookies: &CookieJar<'_>, secure: Secure, conn: DbConn) -> ApiResult<Redirect> {
let AuthorizeData { let AuthorizeData {
client_id, client_id,
redirect_uri, redirect_uri,
@ -1239,7 +1258,23 @@ async fn authorize(data: AuthorizeData, conn: DbConn) -> ApiResult<Redirect> {
err!("Unsupported code challenge method"); err!("Unsupported code challenge method");
} }
let auth_url = sso::authorize_url(state, code_challenge, &client_id, &redirect_uri, conn).await?; // Generate browser-binding token. Stored hashed in DB; raw value handed to the browser as a cookie.
// Validated on /connect/oidc-signin
let binding_token = data_encoding::BASE64URL_NOPAD.encode(&crypto::get_random_bytes::<32>());
let binding_hash = crypto::sha256_hex(binding_token.as_bytes());
let auth_url =
sso::authorize_url(state, code_challenge, &client_id, &redirect_uri, Some(binding_hash), conn).await?;
cookies.add(
Cookie::build((SSO_BINDING_COOKIE, binding_token))
.path("/identity/connect/")
.max_age(time::Duration::seconds(sso::SSO_AUTH_EXPIRATION.num_seconds()))
.same_site(SameSite::Lax) // Lax is needed because the IdP runs on a different FQDN
.http_only(true)
.secure(secure.https)
.build(),
);
Ok(Redirect::temporary(String::from(auth_url))) Ok(Redirect::temporary(String::from(auth_url)))
} }

7
src/crypto.rs

@ -113,3 +113,10 @@ pub fn ct_eq<T: AsRef<[u8]>, U: AsRef<[u8]>>(a: T, b: U) -> bool {
use subtle::ConstantTimeEq; use subtle::ConstantTimeEq;
a.as_ref().ct_eq(b.as_ref()).into() a.as_ref().ct_eq(b.as_ref()).into()
} }
//
// SHA256
//
pub fn sha256_hex(data: &[u8]) -> String {
HEXLOWER.encode(digest::digest(&digest::SHA256, data).as_ref())
}

10
src/db/models/sso_auth.rs

@ -54,11 +54,18 @@ pub struct SsoAuth {
pub auth_response: Option<OIDCAuthenticatedUser>, pub auth_response: Option<OIDCAuthenticatedUser>,
pub created_at: NaiveDateTime, pub created_at: NaiveDateTime,
pub updated_at: NaiveDateTime, pub updated_at: NaiveDateTime,
pub binding_hash: Option<String>,
} }
/// Local methods /// Local methods
impl SsoAuth { impl SsoAuth {
pub fn new(state: OIDCState, client_challenge: OIDCCodeChallenge, nonce: String, redirect_uri: String) -> Self { pub fn new(
state: OIDCState,
client_challenge: OIDCCodeChallenge,
nonce: String,
redirect_uri: String,
binding_hash: Option<String>,
) -> Self {
let now = Utc::now().naive_utc(); let now = Utc::now().naive_utc();
SsoAuth { SsoAuth {
@ -70,6 +77,7 @@ impl SsoAuth {
updated_at: now, updated_at: now,
code_response: None, code_response: None,
auth_response: None, auth_response: None,
binding_hash,
} }
} }
} }

1
src/db/schema.rs

@ -265,6 +265,7 @@ table! {
auth_response -> Nullable<Text>, auth_response -> Nullable<Text>,
created_at -> Timestamp, created_at -> Timestamp,
updated_at -> Timestamp, updated_at -> Timestamp,
binding_hash -> Nullable<Text>,
} }
} }

3
src/sso.rs

@ -188,6 +188,7 @@ pub async fn authorize_url(
client_challenge: OIDCCodeChallenge, client_challenge: OIDCCodeChallenge,
client_id: &str, client_id: &str,
raw_redirect_uri: &str, raw_redirect_uri: &str,
binding_hash: Option<String>,
conn: DbConn, conn: DbConn,
) -> ApiResult<Url> { ) -> ApiResult<Url> {
let redirect_uri = match client_id { let redirect_uri = match client_id {
@ -203,7 +204,7 @@ pub async fn authorize_url(
_ => err!(format!("Unsupported client {client_id}")), _ => err!(format!("Unsupported client {client_id}")),
}; };
let (auth_url, sso_auth) = Client::authorize_url(state, client_challenge, redirect_uri).await?; let (auth_url, sso_auth) = Client::authorize_url(state, client_challenge, redirect_uri, binding_hash).await?;
sso_auth.save(&conn).await?; sso_auth.save(&conn).await?;
Ok(auth_url) Ok(auth_url)
} }

3
src/sso_client.rs

@ -117,6 +117,7 @@ impl Client {
state: OIDCState, state: OIDCState,
client_challenge: OIDCCodeChallenge, client_challenge: OIDCCodeChallenge,
redirect_uri: String, redirect_uri: String,
binding_hash: Option<String>,
) -> ApiResult<(Url, SsoAuth)> { ) -> ApiResult<(Url, SsoAuth)> {
let scopes = CONFIG.sso_scopes_vec().into_iter().map(Scope::new); let scopes = CONFIG.sso_scopes_vec().into_iter().map(Scope::new);
let base64_state = data_encoding::BASE64.encode(state.to_string().as_bytes()); let base64_state = data_encoding::BASE64.encode(state.to_string().as_bytes());
@ -139,7 +140,7 @@ impl Client {
} }
let (auth_url, _, nonce) = auth_req.url(); let (auth_url, _, nonce) = auth_req.url();
Ok((auth_url, SsoAuth::new(state, client_challenge, nonce.secret().clone(), redirect_uri))) Ok((auth_url, SsoAuth::new(state, client_challenge, nonce.secret().clone(), redirect_uri, binding_hash)))
} }
pub async fn exchange_code( pub async fn exchange_code(

Loading…
Cancel
Save