mirror of
https://github.com/dani-garcia/vaultwarden.git
synced 2026-05-06 12:28:51 +03:00
Several SSO Fixes (#7163)
* 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> * Check email-verified on SSO login/create This commit prevents possible account takeover via SSO which doesn't check/validate or provide validated status of the email. It was checked at other locations, but was skipped here. Signed-off-by: BlackDex <black.dex@gmail.com> * Prevent data disclosure via SSO endpoints This commit prevents some data disclosure and user enumeration by only returning the fake SSO identifier. Since we do not check the identifier anywhere useful, returning the fake one is just fine. During an invite to an org, that link contains the correct UUID and will be used for the master password requirements. For anything else, server admins should set the `SSO_MASTER_PASSWORD_POLICY` env variable. Signed-off-by: BlackDex <black.dex@gmail.com> * Adjust admin layout to fix issues when SSO is enabled Signed-off-by: BlackDex <black.dex@gmail.com> --------- Signed-off-by: BlackDex <black.dex@gmail.com>
This commit is contained in:
committed by
GitHub
parent
a354e57659
commit
d297e274a3
@@ -0,0 +1 @@
|
||||
ALTER TABLE sso_auth DROP COLUMN binding_hash;
|
||||
@@ -0,0 +1 @@
|
||||
ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT;
|
||||
@@ -0,0 +1 @@
|
||||
ALTER TABLE sso_auth DROP COLUMN binding_hash;
|
||||
@@ -0,0 +1 @@
|
||||
ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT;
|
||||
@@ -0,0 +1 @@
|
||||
ALTER TABLE sso_auth DROP COLUMN binding_hash;
|
||||
@@ -0,0 +1 @@
|
||||
ALTER TABLE sso_auth ADD COLUMN binding_hash TEXT;
|
||||
@@ -907,36 +907,21 @@ async fn _get_org_details(
|
||||
Ok(json!(ciphers_json))
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct OrgDomainDetails {
|
||||
email: String,
|
||||
}
|
||||
|
||||
// Returning a Domain/Organization here allow to prefill it and prevent prompting the user
|
||||
// So we either return an Org name associated to the user or a dummy value.
|
||||
// So we return a dummy value, since we only support a single SSO integration, and do not use the response anywhere
|
||||
// In use since `v2025.6.0`, appears to use only the first `organizationIdentifier`
|
||||
#[post("/organizations/domain/sso/verified", data = "<data>")]
|
||||
async fn get_org_domain_sso_verified(data: Json<OrgDomainDetails>, conn: DbConn) -> JsonResult {
|
||||
let data: OrgDomainDetails = data.into_inner();
|
||||
|
||||
let identifiers = match Organization::find_org_user_email(&data.email, &conn)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|o| (o.name, o.uuid.to_string()))
|
||||
.collect::<Vec<(String, String)>>()
|
||||
{
|
||||
v if !v.is_empty() => v,
|
||||
_ => vec![(FAKE_SSO_IDENTIFIER.to_string(), FAKE_SSO_IDENTIFIER.to_string())],
|
||||
};
|
||||
|
||||
#[post("/organizations/domain/sso/verified")]
|
||||
fn get_org_domain_sso_verified() -> JsonResult {
|
||||
// Always return a dummy value, no matter if SSO is enabled or not
|
||||
Ok(Json(json!({
|
||||
"object": "list",
|
||||
"data": identifiers.into_iter().map(|(name, identifier)| json!({
|
||||
"organizationName": name, // appear unused
|
||||
"organizationIdentifier": identifier,
|
||||
"domainName": CONFIG.domain(), // appear unused
|
||||
})).collect::<Vec<Value>>()
|
||||
"data": [{
|
||||
"organizationIdentifier": FAKE_SSO_IDENTIFIER,
|
||||
// These appear to be unused
|
||||
"organizationName": FAKE_SSO_IDENTIFIER,
|
||||
"domainName": CONFIG.domain()
|
||||
}],
|
||||
"continuationToken": null
|
||||
})))
|
||||
}
|
||||
|
||||
@@ -3049,10 +3034,7 @@ async fn put_reset_password_enrollment(
|
||||
err!("User to enroll isn't member of required organization", "The user_id and acting user do not match");
|
||||
}
|
||||
|
||||
let Some(mut membership) = Membership::find_confirmed_by_user_and_org(&headers.user.uuid, &org_id, &conn).await
|
||||
else {
|
||||
err!("User to enroll isn't member of required organization")
|
||||
};
|
||||
let mut membership = headers.membership;
|
||||
|
||||
check_reset_password_applicable(&org_id, &conn).await?;
|
||||
|
||||
|
||||
+66
-5
@@ -2,6 +2,7 @@ use chrono::Utc;
|
||||
use num_traits::FromPrimitive;
|
||||
use rocket::{
|
||||
form::{Form, FromForm},
|
||||
http::{Cookie, CookieJar, SameSite},
|
||||
response::Redirect,
|
||||
serde::json::Json,
|
||||
Route,
|
||||
@@ -23,7 +24,8 @@ use crate::{
|
||||
ApiResult, EmptyResult, JsonResult,
|
||||
},
|
||||
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::{
|
||||
models::{
|
||||
AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeWrapper, OrganizationApiKey,
|
||||
@@ -228,7 +230,33 @@ async fn _sso_login(
|
||||
}
|
||||
)
|
||||
}
|
||||
Some((user, None)) => Some((user, None)),
|
||||
Some((user, None)) => match user_infos.email_verified {
|
||||
None if !CONFIG.sso_allow_unknown_email_verification() => {
|
||||
error!(
|
||||
"Login failure ({}), existing non SSO user ({}) with same email ({}) and email verification status is unknown",
|
||||
user_infos.identifier, user.uuid, user.email
|
||||
);
|
||||
err_silent!(
|
||||
"Email verification status is unknown",
|
||||
ErrorEvent {
|
||||
event: EventType::UserFailedLogIn
|
||||
}
|
||||
)
|
||||
}
|
||||
Some(false) => {
|
||||
error!(
|
||||
"Login failure ({}), existing non SSO user ({}) with same email ({}) and email is not verified",
|
||||
user_infos.identifier, user.uuid, user.email
|
||||
);
|
||||
err_silent!(
|
||||
"Email is not verified by the SSO provider",
|
||||
ErrorEvent {
|
||||
event: EventType::UserFailedLogIn
|
||||
}
|
||||
)
|
||||
}
|
||||
_ => Some((user, None)),
|
||||
},
|
||||
},
|
||||
Some((user, sso_user)) => Some((user, Some(sso_user))),
|
||||
};
|
||||
@@ -1133,13 +1161,16 @@ fn prevalidate() -> JsonResult {
|
||||
}
|
||||
}
|
||||
|
||||
const SSO_BINDING_COOKIE: &str = "VW_SSO_BINDING";
|
||||
|
||||
#[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(
|
||||
state,
|
||||
OIDCCodeWrapper::Ok {
|
||||
code,
|
||||
},
|
||||
cookies,
|
||||
&mut conn,
|
||||
)
|
||||
.await
|
||||
@@ -1152,6 +1183,7 @@ async fn oidcsignin_error(
|
||||
state: String,
|
||||
error: String,
|
||||
error_description: Option<String>,
|
||||
cookies: &CookieJar<'_>,
|
||||
mut conn: DbConn,
|
||||
) -> ApiResult<Redirect> {
|
||||
_oidcsignin_redirect(
|
||||
@@ -1160,6 +1192,7 @@ async fn oidcsignin_error(
|
||||
error,
|
||||
error_description,
|
||||
},
|
||||
cookies,
|
||||
&mut conn,
|
||||
)
|
||||
.await
|
||||
@@ -1171,6 +1204,7 @@ async fn oidcsignin_error(
|
||||
async fn _oidcsignin_redirect(
|
||||
base64_state: String,
|
||||
code_response: OIDCCodeWrapper,
|
||||
cookies: &CookieJar<'_>,
|
||||
conn: &mut DbConn,
|
||||
) -> ApiResult<Redirect> {
|
||||
let state = sso::decode_state(&base64_state)?;
|
||||
@@ -1179,6 +1213,17 @@ async fn _oidcsignin_redirect(
|
||||
None => err!(format!("Cannot retrieve sso_auth for {state}")),
|
||||
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.updated_at = Utc::now().naive_utc();
|
||||
sso_auth.save(conn).await?;
|
||||
@@ -1225,7 +1270,7 @@ struct AuthorizeData {
|
||||
|
||||
// The `redirect_uri` will change depending of the client (web, android, ios ..)
|
||||
#[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 {
|
||||
client_id,
|
||||
redirect_uri,
|
||||
@@ -1239,7 +1284,23 @@ async fn authorize(data: AuthorizeData, conn: DbConn) -> ApiResult<Redirect> {
|
||||
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)))
|
||||
}
|
||||
|
||||
@@ -113,3 +113,10 @@ pub fn ct_eq<T: AsRef<[u8]>, U: AsRef<[u8]>>(a: T, b: U) -> bool {
|
||||
use subtle::ConstantTimeEq;
|
||||
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())
|
||||
}
|
||||
|
||||
@@ -54,11 +54,18 @@ pub struct SsoAuth {
|
||||
pub auth_response: Option<OIDCAuthenticatedUser>,
|
||||
pub created_at: NaiveDateTime,
|
||||
pub updated_at: NaiveDateTime,
|
||||
pub binding_hash: Option<String>,
|
||||
}
|
||||
|
||||
/// Local methods
|
||||
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();
|
||||
|
||||
SsoAuth {
|
||||
@@ -70,6 +77,7 @@ impl SsoAuth {
|
||||
updated_at: now,
|
||||
code_response: None,
|
||||
auth_response: None,
|
||||
binding_hash,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -265,6 +265,7 @@ table! {
|
||||
auth_response -> Nullable<Text>,
|
||||
created_at -> Timestamp,
|
||||
updated_at -> Timestamp,
|
||||
binding_hash -> Nullable<Text>,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+2
-1
@@ -188,6 +188,7 @@ pub async fn authorize_url(
|
||||
client_challenge: OIDCCodeChallenge,
|
||||
client_id: &str,
|
||||
raw_redirect_uri: &str,
|
||||
binding_hash: Option<String>,
|
||||
conn: DbConn,
|
||||
) -> ApiResult<Url> {
|
||||
let redirect_uri = match client_id {
|
||||
@@ -203,7 +204,7 @@ pub async fn authorize_url(
|
||||
_ => 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?;
|
||||
Ok(auth_url)
|
||||
}
|
||||
|
||||
+2
-1
@@ -117,6 +117,7 @@ impl Client {
|
||||
state: OIDCState,
|
||||
client_challenge: OIDCCodeChallenge,
|
||||
redirect_uri: String,
|
||||
binding_hash: Option<String>,
|
||||
) -> ApiResult<(Url, SsoAuth)> {
|
||||
let scopes = CONFIG.sso_scopes_vec().into_iter().map(Scope::new);
|
||||
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();
|
||||
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(
|
||||
|
||||
Vendored
+13
-2
@@ -1,6 +1,17 @@
|
||||
body {
|
||||
padding-top: 75px;
|
||||
}
|
||||
/* Some extra width's for the main layout */
|
||||
@media (min-width: 1600px) {
|
||||
.container-xxl {
|
||||
max-width: 1520px;
|
||||
}
|
||||
}
|
||||
@media (min-width: 1800px) {
|
||||
.container-xxl {
|
||||
max-width: 1720px;
|
||||
}
|
||||
}
|
||||
img {
|
||||
width: 48px;
|
||||
height: 48px;
|
||||
@@ -38,8 +49,8 @@ img {
|
||||
max-width: 130px;
|
||||
}
|
||||
#users-table .vw-actions, #orgs-table .vw-actions {
|
||||
min-width: 155px;
|
||||
max-width: 160px;
|
||||
min-width: 170px;
|
||||
max-width: 180px;
|
||||
}
|
||||
#users-table .vw-org-cell {
|
||||
max-height: 120px;
|
||||
|
||||
@@ -27,7 +27,7 @@
|
||||
</symbol>
|
||||
</svg>
|
||||
<nav class="navbar navbar-expand-md navbar-dark bg-dark mb-4 shadow fixed-top">
|
||||
<div class="container-xl">
|
||||
<div class="container-xxl">
|
||||
<a class="navbar-brand" href="{{urlpath}}/admin"><img class="vaultwarden-icon" src="{{urlpath}}/vw_static/vaultwarden-icon.png" alt="V">aultwarden Admin</a>
|
||||
<button class="navbar-toggler" type="button" data-bs-toggle="collapse" data-bs-target="#navbarCollapse"
|
||||
aria-controls="navbarCollapse" aria-expanded="false" aria-label="Toggle navigation">
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<main class="container-xl">
|
||||
<main class="container-xxl">
|
||||
<div id="diagnostics-block" class="my-3 p-3 rounded shadow">
|
||||
<h6 class="border-bottom pb-2 mb-2">Diagnostics</h6>
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<main class="container-xl">
|
||||
<main class="container-xxl">
|
||||
{{#if error}}
|
||||
<div class="align-items-center p-3 mb-3 text-opacity-50 text-dark bg-warning rounded shadow">
|
||||
<div>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<main class="container-xl">
|
||||
<main class="container-xxl">
|
||||
<div id="organizations-block" class="my-3 p-3 rounded shadow">
|
||||
<h6 class="border-bottom pb-2 mb-3">Organizations</h6>
|
||||
<div class="table-responsive-xl small">
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<main class="container-xl">
|
||||
<main class="container-xxl">
|
||||
<div id="admin_token_warning" class="alert alert-warning alert-dismissible fade show d-none">
|
||||
<button type="button" class="btn-close" data-bs-target="admin_token_warning" data-bs-dismiss="alert" aria-label="Close"></button>
|
||||
You are using a plain text `ADMIN_TOKEN` which is insecure.<br>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
<main class="container-xl">
|
||||
<main class="container-xxl">
|
||||
<div id="users-block" class="my-3 p-3 rounded shadow">
|
||||
<h6 class="border-bottom pb-2 mb-3">Registered Users</h6>
|
||||
<div class="table-responsive-xl small">
|
||||
@@ -43,7 +43,7 @@
|
||||
</td>
|
||||
{{#if ../sso_enabled}}
|
||||
<td>
|
||||
<span class="d-block">{{sso_identifier}}</span>
|
||||
<span class="d-block text-break text-wrap">{{sso_identifier}}</span>
|
||||
</td>
|
||||
{{/if}}
|
||||
<td>
|
||||
|
||||
Reference in New Issue
Block a user