KeiSeiKit-1.0/_primitives/_rust/kei-auth-google/src/verify_helpers.rs
Parfii-bot 611b603469 fix(auth): Google OIDC account-takeover (CVE-2023-7028 class) — email_verified gate + sub as user_id + id_token cross-check
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>
2026-05-03 15:38:53 +08:00

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());
}
}