From 357f83c96a85f771ea70fcd301f568d74f2fda7d Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 11 Sep 2026 12:43:18 +0100 Subject: [PATCH] Rate limit device code link attempts Add a dedicated `device_code_link` rate limiter for device authorization user code verification. This is to help mitigate the User Code Brute Forcing attack as described by [RFC 8628 section 5.1](https://datatracker.ietf.org/doc/html/rfc8628#section-5.1) Proposed default is 1 per minute and burst of 10. This can be overridden by the server admin. --- crates/config/src/sections/rate_limiting.rs | 19 ++ crates/handlers/src/oauth2/device/link.rs | 193 +++++++++++++++++++- crates/handlers/src/rate_limit.rs | 44 +++++ docs/config.schema.json | 12 ++ docs/reference/configuration.md | 8 + templates/pages/device_link.html | 8 + translations/en.json | 6 +- 7 files changed, 285 insertions(+), 5 deletions(-) diff --git a/crates/config/src/sections/rate_limiting.rs b/crates/config/src/sections/rate_limiting.rs index 0b7c95dbc..bcefa4093 100644 --- a/crates/config/src/sections/rate_limiting.rs +++ b/crates/config/src/sections/rate_limiting.rs @@ -31,6 +31,13 @@ pub struct RateLimitingConfig { /// Email authentication-specific rate limits #[serde(default)] pub email_authentication: EmailauthenticationRateLimitingConfig, + + /// Controls how many user code verification attempts are permitted + /// based on source IP address, when linking a device through the + /// Device Authorization Grant. + /// This can protect against brute-forcing the user code. + #[serde(default = "default_device_code_link")] + pub device_code_link: RateLimiterConfiguration, } #[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)] @@ -174,6 +181,10 @@ impl ConfigurationSection for RateLimitingConfig { return Err(error_on_nested_field(error, "login", "per_account").into()); } + if let Some(error) = error_on_limiter(&self.device_code_link) { + return Err(error_on_field(error, "device_code_link").into()); + } + Ok(()) } } @@ -257,6 +268,13 @@ fn default_email_authentication_attempt_per_session() -> RateLimiterConfiguratio } } +fn default_device_code_link() -> RateLimiterConfiguration { + RateLimiterConfiguration { + burst: NonZeroU32::new(10).unwrap(), + per_second: 1.0 / 60.0, + } +} + impl Default for RateLimitingConfig { fn default() -> Self { RateLimitingConfig { @@ -264,6 +282,7 @@ impl Default for RateLimitingConfig { registration: default_registration(), account_recovery: AccountRecoveryRateLimitingConfig::default(), email_authentication: EmailauthenticationRateLimitingConfig::default(), + device_code_link: default_device_code_link(), } } } diff --git a/crates/handlers/src/oauth2/device/link.rs b/crates/handlers/src/oauth2/device/link.rs index 38f55ac12..b568f0a49 100644 --- a/crates/handlers/src/oauth2/device/link.rs +++ b/crates/handlers/src/oauth2/device/link.rs @@ -5,6 +5,8 @@ // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial // Please see LICENSE files in the repository root for full details. +use std::sync::LazyLock; + use axum::{ Form, extract::State, @@ -21,11 +23,22 @@ use mas_i18n::DataLocale; use mas_router::UrlBuilder; use mas_storage::BoxRepository; use mas_templates::{ - DeviceLinkContext, DeviceLinkFormField, FieldError, FormState, TemplateContext, Templates, + DeviceLinkContext, DeviceLinkFormField, FieldError, FormError, FormState, TemplateContext, + Templates, }; +use opentelemetry::{Key, KeyValue, metrics::Counter}; use serde::{Deserialize, Serialize}; -use crate::{PreferredLanguage, SiteConfig}; +use crate::{Limiter, METER, PreferredLanguage, RequesterFingerprint, SiteConfig}; + +static USER_CODE_ATTEMPT_COUNTER: LazyLock> = LazyLock::new(|| { + METER + .u64_counter("mas.oauth2.device_code_link_attempt") + .with_description("Number of user codes submitted on the device link page") + .with_unit("{attempt}") + .build() +}); +const RESULT: Key = Key::from_static_str("result"); #[derive(Serialize, Deserialize)] pub struct Params { @@ -42,6 +55,8 @@ pub(crate) async fn get( State(templates): State, State(url_builder): State, State(site_config): State, + State(limiter): State, + requester: RequesterFingerprint, cookie_jar: CookieJar, Query(mut query): Query, ) -> Result { @@ -63,6 +78,8 @@ pub(crate) async fn get( &locale, &templates, &url_builder, + &limiter, + requester, cookie_jar, query, ) @@ -78,6 +95,8 @@ pub(crate) async fn post( State(templates): State, State(url_builder): State, State(site_config): State, + State(limiter): State, + requester: RequesterFingerprint, cookie_jar: CookieJar, Form(form): Form>, ) -> Result { @@ -96,12 +115,15 @@ pub(crate) async fn post( &locale, &templates, &url_builder, + &limiter, + requester, cookie_jar, form, ) .await } +#[expect(clippy::too_many_arguments)] async fn handle_code( rng: &mut BoxRng, clock: &BoxClock, @@ -109,6 +131,8 @@ async fn handle_code( locale: &DataLocale, templates: &Templates, url_builder: &UrlBuilder, + limiter: &Limiter, + requester: RequesterFingerprint, cookie_jar: CookieJar, params: Params, ) -> Result { @@ -116,6 +140,25 @@ async fn handle_code( // If we have a code, find it in the database if let Some(code) = ¶ms.code { + // Rate-limit how many user codes a single requester can try, to make + // brute-forcing the user code impractical. RFC 8628 section 5.1. + // This is checked before looking the code up, so that guesses beyond the + // allowance are never evaluated. + if let Err(e) = limiter.check_device_code_link(requester) { + tracing::warn!(error = &e as &dyn std::error::Error, "ratelimit exceeded"); + USER_CODE_ATTEMPT_COUNTER.add(1, &[KeyValue::new(RESULT, "rate_limited")]); + + let (csrf_token, cookie_jar) = cookie_jar.csrf_token(clock, rng); + let ctx = DeviceLinkContext::new() + .with_form_state(form_state.with_error_on_form(FormError::RateLimitExceeded)) + .with_csrf(csrf_token.form_value()) + .with_language(*locale); + + let content = templates.render_device_link(&ctx)?; + + return Ok((cookie_jar, Html(content)).into_response()); + } + let code = code.to_uppercase(); let grant = repo .oauth2_device_code_grant() @@ -128,12 +171,14 @@ async fn handle_code( if let Some(grant) = grant { // This is a valid code, redirect to the consent page // This will in turn redirect to the login page if the user is not logged in + USER_CODE_ATTEMPT_COUNTER.add(1, &[KeyValue::new(RESULT, "success")]); let destination = url_builder.redirect(&mas_router::DeviceCodeConsent::new(grant.id)); return Ok((cookie_jar, destination).into_response()); } // The code isn't valid, set an error on the form + USER_CODE_ATTEMPT_COUNTER.add(1, &[KeyValue::new(RESULT, "invalid")]); form_state = form_state.with_error_on_field(DeviceLinkFormField::Code, FieldError::Invalid); } @@ -149,3 +194,147 @@ async fn handle_code( Ok((cookie_jar, Html(content)).into_response()) } + +#[cfg(test)] +mod tests { + use std::net::{IpAddr, Ipv4Addr}; + + use hyper::{Request, StatusCode, header::CONTENT_TYPE}; + use mas_router::{Route, SimpleRoute}; + use oauth2_types::{ + registration::ClientRegistrationResponse, requests::DeviceAuthorizationResponse, + }; + use sqlx::PgPool; + + use crate::test_utils::{CookieHelper, RequestBuilderExt, ResponseExt, TestState, setup}; + + const ALICE: IpAddr = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)); + const BOB: IpAddr = IpAddr::V4(Ipv4Addr::new(4, 3, 2, 1)); + + /// Start a device authorization grant and return its user code + async fn get_user_code(state: &TestState) -> String { + let request = + Request::post(mas_router::OAuth2RegistrationEndpoint::PATH).json(serde_json::json!({ + "client_uri": "https://example.com/", + "token_endpoint_auth_method": "none", + "grant_types": ["urn:ietf:params:oauth:grant-type:device_code"], + "response_types": [], + })); + + let response = state.request(request).await; + response.assert_status(StatusCode::CREATED); + let response: ClientRegistrationResponse = response.json(); + + let request = Request::post(mas_router::OAuth2DeviceAuthorizationEndpoint::PATH).form( + serde_json::json!({ + "client_id": response.client_id, + "scope": "openid", + }), + ); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + let response: DeviceAuthorizationResponse = response.json(); + + response.user_code + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_link_rate_limit(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + let cookies = CookieHelper::new(); + + // Render the link page to get a CSRF token. This shouldn't consume any of the + // rate limit allowance, as no code is submitted + let request = Request::get( + mas_router::DeviceCodeLink::default() + .path_and_query() + .as_ref(), + ) + .client_ip(ALICE) + .empty(); + let request = cookies.with_cookies(request); + let response = state.request(request).await; + cookies.save_cookies(&response); + response.assert_status(StatusCode::OK); + response.assert_header_value(CONTENT_TYPE, "text/html; charset=utf-8"); + let csrf_token = response + .body() + .split("name=\"csrf\" value=\"") + .nth(1) + .unwrap() + .split('\"') + .next() + .unwrap() + .to_owned(); + + let request = Request::post(mas_router::DeviceCodeLink::route()) + .client_ip(ALICE) + .form(serde_json::json!({ + "csrf": csrf_token, + "code": "AAAAAA", + })); + let request = cookies.with_cookies(request); + + // The default burst allowance is 10 attempts, which should all be told that the + // code is invalid + for _ in 0..10 { + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + let body = response.body(); + assert!(body.contains(r#"data-error-kind="invalid""#)); + assert!(!body.contains("too many requests")); + } + + // The next attempt should be rate-limited + let response = state.request(request.clone()).await; + response.assert_status(StatusCode::OK); + let body = response.body(); + assert!(!body.contains(r#"data-error-kind="invalid""#)); + assert!(body.contains("too many requests")); + + // A valid code is refused too: the allowance is checked before the code is + // looked up, so that guesses beyond it are never evaluated + let user_code = get_user_code(&state).await; + let request = Request::get( + mas_router::DeviceCodeLink::with_code(user_code.clone()) + .path_and_query() + .as_ref(), + ) + .client_ip(ALICE) + .empty(); + let request = cookies.with_cookies(request); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + assert!(response.body().contains("too many requests")); + + // Another requester is unaffected and can still use that code + let request = Request::get( + mas_router::DeviceCodeLink::with_code(user_code) + .path_and_query() + .as_ref(), + ) + .client_ip(BOB) + .empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + } + + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_link_valid_code(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool).await.unwrap(); + let user_code = get_user_code(&state).await; + + // A valid code redirects to the consent page + let request = Request::get( + mas_router::DeviceCodeLink::with_code(user_code) + .path_and_query() + .as_ref(), + ) + .client_ip(ALICE) + .empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + } +} diff --git a/crates/handlers/src/rate_limit.rs b/crates/handlers/src/rate_limit.rs index 07acc1bc3..246e2d5d4 100644 --- a/crates/handlers/src/rate_limit.rs +++ b/crates/handlers/src/rate_limit.rs @@ -51,6 +51,12 @@ pub enum EmailAuthenticationLimitedError { Email(String), } +#[derive(Debug, Clone, thiserror::Error)] +pub enum DeviceCodeLinkLimitedError { + #[error("Too many device code link attempts for requester {0}")] + Requester(RequesterFingerprint), +} + /// Key used to rate limit requests per requester #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct RequesterFingerprint { @@ -122,6 +128,7 @@ struct LimiterInner { email_authentication_per_email: KeyedRateLimiter, email_authentication_emails_per_session: KeyedRateLimiter, email_authentication_attempt_per_session: KeyedRateLimiter, + device_code_link_per_requester: KeyedRateLimiter, } impl LimiterInner { @@ -148,6 +155,7 @@ impl LimiterInner { email_authentication_attempt_per_session: RateLimiter::keyed( config.email_authentication.attempt_per_session.to_quota()?, ), + device_code_link_per_requester: RateLimiter::keyed(config.device_code_link.to_quota()?), }) } } @@ -193,6 +201,7 @@ impl Limiter { this.inner .email_authentication_attempt_per_session .retain_recent(); + this.inner.device_code_link_per_requester.retain_recent(); interval.tick().await; } @@ -325,6 +334,24 @@ impl Limiter { .check_key(&authentication.id) .map_err(|_| EmailAuthenticationLimitedError::Authentication(authentication.id)) } + + /// Check if a user code can be submitted on the device link page + /// + /// This protects against brute-forcing the user code of a device code + /// grant, as described in RFC 8628 section 5.1. + /// + /// # Errors + /// + /// Returns an error if the operation is rate limited. + pub fn check_device_code_link( + &self, + requester: RequesterFingerprint, + ) -> Result<(), DeviceCodeLinkLimitedError> { + self.inner + .device_code_link_per_requester + .check_key(&requester) + .map_err(|_| DeviceCodeLinkLimitedError::Requester(requester)) + } } #[cfg(test)] @@ -401,4 +428,21 @@ mod tests { // The other account isn't rate-limited assert!(limiter.check_password(requesters[603], &bob).is_ok()); } + + #[test] + fn test_device_code_link_limiter() { + let limiter = Limiter::new(&RateLimitingConfig::default()).unwrap(); + + let alice = RequesterFingerprint::new([1, 2, 3, 4].into()); + let bob = RequesterFingerprint::new([4, 3, 2, 1].into()); + + // The default burst allowance is 10 attempts per requester + for _ in 0..10 { + assert!(limiter.check_device_code_link(alice).is_ok()); + } + assert!(limiter.check_device_code_link(alice).is_err()); + + // Another requester is unaffected + assert!(limiter.check_device_code_link(bob).is_ok()); + } } diff --git a/docs/config.schema.json b/docs/config.schema.json index a6b6560bf..cbe8f67c8 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -2021,6 +2021,18 @@ "$ref": "#/definitions/EmailauthenticationRateLimitingConfig" } ] + }, + "device_code_link": { + "description": "Controls how many user code verification attempts are permitted\nbased on source IP address, when linking a device through the\nDevice Authorization Grant.\nThis can protect against brute-forcing the user code.", + "default": { + "burst": 10, + "per_second": 0.016666666666666666 + }, + "allOf": [ + { + "$ref": "#/definitions/RateLimiterConfiguration" + } + ] } } }, diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 3600ff226..e9f07ae34 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -640,6 +640,14 @@ rate_limiting: attempt_per_session: burst: 10 per_second: 0.016666 + + # Limits how many user codes can be submitted on the device link page, + # based on source IP address. + # This protects against brute-forcing the user code of the + # Device Authorization Grant. + device_code_link: + burst: 10 + per_second: 0.016666 ``` ## `telemetry` diff --git a/templates/pages/device_link.html b/templates/pages/device_link.html index aaf018b1c..b78e8a439 100644 --- a/templates/pages/device_link.html +++ b/templates/pages/device_link.html @@ -22,6 +22,14 @@ Please see LICENSE files in the repository root for full details.
+ {% if form_state.errors is not empty %} + {% for error in form_state.errors %} +
+ {{ errors.form_error_message(error=error) }} +
+ {% endfor %} + {% endif %} + {% call(f) field.field(label=_("mas.device_code_link.verification_code"), name="code", class="mb-4 self-center", form_state=form_state) %}
diff --git a/translations/en.json b/translations/en.json index e7df4b636..7efcc3739 100644 --- a/translations/en.json +++ b/translations/en.json @@ -6,11 +6,11 @@ }, "cancel": "Cancel", "@cancel": { - "context": "pages/consent.html:81:11-29, pages/device_consent.html:179:13-31, pages/device_link.html:45:33-51, pages/policy_violation.html:70:15-33, pages/reauth.html:37:13-31" + "context": "pages/consent.html:81:11-29, pages/device_consent.html:179:13-31, pages/device_link.html:53:33-51, pages/policy_violation.html:70:15-33, pages/reauth.html:37:13-31" }, "continue": "Continue", "@continue": { - "context": "form_post.html:26:28-48, pages/consent.html:71:28-48, pages/device_link.html:42:28-48, pages/login.html:68:30-50, pages/reauth.html:32:28-48, pages/recovery/start.html:38:26-46, pages/register/password.html:80:26-46, pages/register/steps/display_name.html:43:28-48, pages/register/steps/registration_token.html:41:28-48, pages/register/steps/verify_email.html:51:26-46, pages/sso.html:52:28-48" + "context": "form_post.html:26:28-48, pages/consent.html:71:28-48, pages/device_link.html:50:28-48, pages/login.html:68:30-50, pages/reauth.html:32:28-48, pages/recovery/start.html:38:26-46, pages/register/password.html:80:26-46, pages/register/steps/display_name.html:43:28-48, pages/register/steps/registration_token.html:41:28-48, pages/register/steps/verify_email.html:51:26-46, pages/sso.html:52:28-48" }, "create_account": "Create Account", "@create_account": { @@ -234,7 +234,7 @@ }, "verification_code": "Verification code", "@verification_code": { - "context": "pages/device_link.html:26:35-78", + "context": "pages/device_link.html:34:35-78", "description": "On the device link page, label of the text field where the user enters the verification code shown on the other device." } },