Opus Cross-cutting audit found a classic OIDC account-takeover hole in
kei-auth-google::verify(). Same class as the public Booking.com / Slack /
GitLab pattern.
Root cause: verify() accepted info.email from userinfo response as user_id
WITHOUT checking info.email_verified. A Google Workspace admin can mint
accounts with arbitrary unverified email aliases. Attacker then OAuth-flows
into the relying party using a victim's email as their alias and gets a
session bound to that user_id. No email verification = no auth.
Fix in 3 layers (defense in depth):
1. email_verified GATE
- client.rs: UserInfo gains email_verified: bool with #[serde(default)] —
absent field defaults to false (fail-closed).
- error.rs: new Error::EmailNotVerified variant.
- provider.rs::verify(): rejects with EmailNotVerified before any session
is built when email_verified != true.
2. sub AS PRIMARY user_id
- provider.rs::verify(): user_id = info.sub (Google's stable account id),
NOT info.email. Email is now mutable metadata only. Email reassignment
in Google Workspace cannot redirect an existing user_id binding.
3. id_token.sub CROSS-CHECK
- id_token.rs (new, 104 LOC): JWT-claims-only extract_sub() — parses
base64-payload without signature verification (signature verification
against Google JWKS is a documented follow-up atomar).
- provider.rs::verify(): when TokenResponse.id_token is present, decode
claims and require id_token.sub == userinfo.sub. New
Error::IdSubMismatch + IdTokenMalformed variants.
- This adds defense against a forged userinfo response even though
signature is not yet verified.
Constructor Pattern compliance: provider.rs split into provider.rs (181 LOC)
+ verify_helpers.rs (114 LOC, with unpack_challenge / check_state /
enforce_email_verified / cross_check_id_token_sub helpers). All files <200
LOC, all functions <30 LOC.
Tests added: tests/google_security_regression.rs (164 LOC, 5 dedicated
CVE-2023-7028 regression tests). All 26 tests pass:
- verify_rejects_unverified_email
- verify_rejects_missing_email_verified_field
- verify_uses_sub_not_email_as_user_id
- verify_rejects_id_token_sub_mismatch
- verify_accepts_matching_id_token_sub
cargo check --workspace clean. cargo test -p kei-auth-google: 26/26 pass.
Follow-up: JWT signature verification against Google's JWKS endpoint with
kid-based key cache + RS256/ES256 — separate atomar (~150 LOC).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
114 lines
3.8 KiB
Rust
114 lines
3.8 KiB
Rust
// SPDX-License-Identifier: Apache-2.0
|
|
// Copyright 2026 <author org>
|
|
//!
|
|
//! Pure helpers extracted from [`crate::provider`]. Each one is a
|
|
//! single-responsibility check used inside `verify()` — split out so
|
|
//! the provider file stays under the 200-LOC Constructor Pattern bound
|
|
//! and so the security-critical predicates are unit-testable in
|
|
//! isolation (no HTTP, no async).
|
|
|
|
use crate::client::{TokenResponse, UserInfo};
|
|
use crate::error::Error;
|
|
use crate::id_token::extract_sub as extract_id_token_sub;
|
|
use kei_runtime_core::traits::auth::AuthChallenge;
|
|
use subtle::ConstantTimeEq;
|
|
|
|
/// Pull `(code, state, expected_state, code_verifier)` out of an
|
|
/// [`AuthChallenge::OAuthCode`] for `provider == "google"`.
|
|
pub(crate) fn unpack_challenge<'a>(
|
|
c: &'a AuthChallenge,
|
|
) -> kei_runtime_core::Result<(&'a str, &'a str, &'a str, Option<&'a str>)> {
|
|
match c {
|
|
AuthChallenge::OAuthCode {
|
|
provider, code, state, expected_state, code_verifier,
|
|
} if provider == "google" => Ok((
|
|
code.as_str(),
|
|
state.as_str(),
|
|
expected_state.as_str(),
|
|
code_verifier.as_deref(),
|
|
)),
|
|
AuthChallenge::OAuthCode { provider, .. } => Err(kei_runtime_core::Error::Auth(
|
|
format!("wrong provider for google: {provider}"),
|
|
)),
|
|
_ => Err(kei_runtime_core::Error::from(Error::MissingState)),
|
|
}
|
|
}
|
|
|
|
/// Constant-time CSRF-state compare. Returns
|
|
/// [`kei_runtime_core::Error::CsrfStateMismatch`] on disagreement.
|
|
pub(crate) fn check_state(got: &str, expected: &str) -> kei_runtime_core::Result<()> {
|
|
let ok: bool = got.as_bytes().ct_eq(expected.as_bytes()).into();
|
|
if !ok {
|
|
Err(kei_runtime_core::Error::CsrfStateMismatch)
|
|
} else {
|
|
Ok(())
|
|
}
|
|
}
|
|
|
|
/// Reject userinfo where `email_verified` is absent / false.
|
|
///
|
|
/// CVE-2023-7028 class fix: Google Workspace admins can mint accounts
|
|
/// with arbitrary unverified email aliases. Trusting `email` without
|
|
/// the verified flag is account-takeover-equivalent.
|
|
pub(crate) fn enforce_email_verified(info: &UserInfo) -> kei_runtime_core::Result<()> {
|
|
if !info.email_verified {
|
|
return Err(kei_runtime_core::Error::from(Error::EmailNotVerified));
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
/// If `token.id_token` is `Some`, decode its claims and require
|
|
/// `id_token.sub == info.sub`. Skipped (Ok) when absent. Signature
|
|
/// verification is a follow-up; this is defence-in-depth against a
|
|
/// forged userinfo response.
|
|
pub(crate) fn cross_check_id_token_sub(
|
|
token: &TokenResponse,
|
|
info: &UserInfo,
|
|
) -> kei_runtime_core::Result<()> {
|
|
let Some(id_token) = token.id_token.as_deref() else {
|
|
return Ok(());
|
|
};
|
|
let id_sub = extract_id_token_sub(id_token)
|
|
.map_err(kei_runtime_core::Error::from)?;
|
|
if id_sub != info.sub {
|
|
return Err(kei_runtime_core::Error::from(Error::IdSubMismatch));
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
fn ui(email_verified: bool, sub: &str) -> UserInfo {
|
|
UserInfo {
|
|
sub: sub.into(),
|
|
email: "x@y.z".into(),
|
|
email_verified,
|
|
name: "X".into(),
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn enforce_email_verified_passes_when_true() {
|
|
assert!(enforce_email_verified(&ui(true, "abc")).is_ok());
|
|
}
|
|
|
|
#[test]
|
|
fn enforce_email_verified_rejects_false() {
|
|
let err = enforce_email_verified(&ui(false, "abc")).unwrap_err();
|
|
assert!(format!("{err}").contains("not verified"));
|
|
}
|
|
|
|
#[test]
|
|
fn cross_check_no_id_token_is_ok() {
|
|
let tok = TokenResponse { access_token: "t".into(), expires_in: 0, id_token: None };
|
|
assert!(cross_check_id_token_sub(&tok, &ui(true, "abc")).is_ok());
|
|
}
|
|
|
|
#[test]
|
|
fn check_state_constant_time_ok() {
|
|
assert!(check_state("abc", "abc").is_ok());
|
|
assert!(check_state("abc", "abd").is_err());
|
|
}
|
|
}
|