fix(core): 回调先校验 state 再处理 error,堵掉伪造取消失效登录
reviewer P2(crates/core/src/auth/microsoft/callback.rs):handle() 先处理 error 参数、 之后才校验 state,于是任意一次 `GET http://localhost:{port}/auth?error=access_denied` (state 随便给甚至不给)都会让 wait_for_code 直接返回 Canceled,登录被静默中止、账户不入库 —— 不需要知道随机 state、不需要 code、不需要任何凭据。违反 RFC 6749 §4.1.2.1 / RFC 9207 (错误响应同样要校验 state),也与本模块「与本次登录无关的请求一律继续等待」的声明矛盾。 - state 校验提到 error 之前:state 不匹配的请求(无论带不带 error)一律 400 后继续等; 只有带**本机 state** 的 access_denied 才判 Canceled。模块文档第 4 条同步改写。 - 顺带采纳 reviewer 的两条非计数观察: * 读头预算改为 accept 之后按 deadline 重算(原来复用 accept 前的 remaining, 总时长可超发 ≤5s);预算归零直接超时。 * state 由 64 位升到 128 位(两个独立随机种子的 SipHash 输出拼接),对齐 RFC 6749 §10.10 的熵建议;新增单测断言 32 位 hex 且每次不同。 - 新增回归用例(crates/core/tests/auth/microsoft_authcode.rs): `foreign_state_cannot_cancel_the_login` —— 先发 `?error=access_denied`(无 state) 再发 `?error=access_denied&state=not-our-state`,两次都必须 400 且服务器继续等待, 随后真回调拿到 code;`access_denied_with_our_state_is_still_canceled` 钉住另一半语义。 既有的「陈旧 state 回 400 后继续等」用例保持通过。 门禁:cargo fmt --all --check OK;cargo clippy --workspace --all-targets -- -D warnings exit 0 无告警;cargo test -p unlucky-core -p unlucky-app 18 个 suite 全绿(auth 59 例)。 AskaEth <i@yeij.top>
This commit is contained in:
1 parent
8194e38bcf
commit
07fd6113a7
3 files changed
+93
-10
No files matched your search
@@ -195,18 +195,26 @@ where
|
||||
}
|
||||
|
||||
/// 每次登录一个 `state`(不用于加密,只用来把迟到的旧回调挡掉)。
|
||||
///
|
||||
/// 128 位(两个独立随机种子的 SipHash 输出拼接):RFC 6749 §10.10 建议 `state` 至少
|
||||
/// 128 位熵,挡住「猜到 state 就能伪造回调」这类攻击面。
|
||||
fn nonce() -> String {
|
||||
use std::collections::hash_map::RandomState;
|
||||
use std::hash::{BuildHasher, Hasher};
|
||||
|
||||
let mut hasher = RandomState::new().build_hasher();
|
||||
let nanos = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.map(|since| since.as_nanos())
|
||||
.unwrap_or(0);
|
||||
hasher.write_u128(nanos);
|
||||
hasher.write_u32(std::process::id());
|
||||
format!("{:016x}", hasher.finish())
|
||||
let mut halves = [String::new(), String::new()];
|
||||
for half in &mut halves {
|
||||
// 每次 `RandomState::new()` 都是独立随机种子 → 两段各 64 位、彼此不可推。
|
||||
let mut hasher = RandomState::new().build_hasher();
|
||||
hasher.write_u128(nanos);
|
||||
hasher.write_u32(std::process::id());
|
||||
*half = format!("{:016x}", hasher.finish());
|
||||
}
|
||||
format!("{}{}", halves[0], halves[1])
|
||||
}
|
||||
|
||||
/// 百分号编码(RFC 3986 unreserved 之外一律转义;空格用 `%20` 而不是 `+`)。
|
||||
@@ -264,4 +272,13 @@ mod tests {
|
||||
fn grant_type_is_the_standard_one() {
|
||||
assert_eq!(AUTHORIZATION_CODE_GRANT, "authorization_code");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generated_state_has_128_bits_of_entropy() {
|
||||
let first = super::nonce();
|
||||
let second = super::nonce();
|
||||
assert_eq!(first.len(), 32, "128 位 = 32 个 hex 字符:{first}");
|
||||
assert!(first.chars().all(|c| c.is_ascii_hexdigit()), "{first}");
|
||||
assert_ne!(first, second, "每次登录的 state 必须不同");
|
||||
}
|
||||
}
|
||||
@@ -12,7 +12,9 @@
|
||||
//! `redirect_uri` 必须用**最终绑到的端口**回填,换 token 时再原样带上(URI 严格一致)。
|
||||
//! 3. **非 `/auth` 的请求(`/favicon.ico`、浏览器预连)回 404 后继续等**:让一个 favicon
|
||||
//! 把整次登录判成失败是最容易犯的错。
|
||||
//! 4. **`state` 不匹配的迟到回调回 400 后继续等**:旧标签页的回调不该打断新的登录;
|
||||
//! 4. **`state` 不匹配的请求一律回 400 后继续等**——包括带 `error=` 的那些:旧标签页的
|
||||
//! 迟到回调不该打断新的登录,更要紧的是 RFC 6749 §4.1.2.1 / RFC 9207 要求**错误响应同样
|
||||
//! 要校验 `state`**;否则任何人 `GET /auth?error=access_denied` 就能静默中止别人的登录。
|
||||
//! 真正的超时由调用方的时间预算兜底。
|
||||
|
||||
use std::time::Duration;
|
||||
@@ -131,7 +133,12 @@ impl CallbackServer {
|
||||
Err(_) => return Err(wait_timeout(timeout)),
|
||||
};
|
||||
// 一条连接最多读这么久请求头:慢客户端/半开连接不能把整个登录拖过时间预算。
|
||||
let budget = REQUEST_READ_TIMEOUT.min(remaining);
|
||||
// 预算按 **accept 之后** 的剩余时间重算(用 accept 前那份会让总时长超发最多 5s)。
|
||||
let budget = REQUEST_READ_TIMEOUT
|
||||
.min(deadline.saturating_duration_since(tokio::time::Instant::now()));
|
||||
if budget.is_zero() {
|
||||
return Err(wait_timeout(timeout));
|
||||
}
|
||||
match tokio::time::timeout(budget, self.handle(&mut stream)).await {
|
||||
Ok(result) => {
|
||||
if let Some(code) = result? {
|
||||
@@ -157,14 +164,17 @@ impl CallbackServer {
|
||||
return Ok(None);
|
||||
}
|
||||
let params = Query::parse(query);
|
||||
if let Some(error) = params.get("error") {
|
||||
respond(stream, 200, DENIED_PAGE).await;
|
||||
return Err(oauth_error(error, params.get("error_description")));
|
||||
}
|
||||
// state 必须先校验,且**先于** error:错误响应同样要认 state(RFC 6749 §4.1.2.1 /
|
||||
// RFC 9207)。否则任意一次 `GET /auth?error=access_denied`(state 随便给或不给)就能
|
||||
// 把这台机器上正在进行的登录静默中止 —— 不需要知道随机 state、不需要 code。
|
||||
if params.get("state") != Some(self.state.as_str()) {
|
||||
respond(stream, 400, BAD_REQUEST_PAGE).await;
|
||||
return Ok(None);
|
||||
}
|
||||
if let Some(error) = params.get("error") {
|
||||
respond(stream, 200, DENIED_PAGE).await;
|
||||
return Err(oauth_error(error, params.get("error_description")));
|
||||
}
|
||||
let Some(code) = params.get("code").filter(|code| !code.is_empty()) else {
|
||||
respond(stream, 400, BAD_REQUEST_PAGE).await;
|
||||
return Ok(None);
|
||||
|
||||
Reference in new issue
Block a user