diff --git a/.changes/http-scope-redirects-default.md b/.changes/http-scope-redirects-default.md new file mode 100644 index 000000000..bf993f159 --- /dev/null +++ b/.changes/http-scope-redirects-default.md @@ -0,0 +1,6 @@ +--- +"http": major +"http-js": major +--- + +**Breaking:** The URL scope is now always checked on every hop of a redirect chain, so every redirect target must be allowed by the scope or the request fails with `url not allowed on the configured scope`. This was previously opt-in through the `scopeRedirects` plugin configuration, which has been removed along with the `Config` struct: `tauri_plugin_http::init()` returns `TauriPlugin` again, and any `plugins > http` object must be removed from `tauri.conf.json`. diff --git a/plugins/http/guest-js/index.ts b/plugins/http/guest-js/index.ts index 5406e03f2..e7666a6a0 100644 --- a/plugins/http/guest-js/index.ts +++ b/plugins/http/guest-js/index.ts @@ -77,9 +77,8 @@ export interface ClientOptions { * Defines the maximum number of redirects the client should follow. * If set to 0, no redirects will be followed. * - * When the `scopeRedirects` plugin configuration is enabled, every redirect must - * also be allowed by the configured scope, otherwise the request fails - * instead of being followed. + * Every redirect must also be allowed by the configured scope, + * otherwise the request fails instead of being followed. */ maxRedirections?: number /** Timeout in milliseconds */ diff --git a/plugins/http/src/commands.rs b/plugins/http/src/commands.rs index 4387bae30..db51da87f 100644 --- a/plugins/http/src/commands.rs +++ b/plugins/http/src/commands.rs @@ -180,25 +180,17 @@ fn attach_proxy( /// Builds the redirect policy for the request. /// -/// When `scope` is [`Some`], **every** hop is checked against it. Validating only the URL the -/// frontend asked for is not enough: a server on an allowed origin can answer with a redirect to -/// any other origin (an open redirect, or a server the attacker controls), and following it would -/// give the webview access to a URL the scope denies. -/// -/// That check is opt-in through the `scopeRedirects` configuration because it breaks applications -/// that rely on being redirected outside of their scope. -// TODO(v3): always check the scope and take it by value instead of `Option` -fn redirect_policy(scope: Option, max_redirections: Option) -> Policy { +/// **Every** hop is checked against the scope. Validating only the URL the frontend asked for is +/// not enough: a server on an allowed origin can answer with a redirect to any other origin (an +/// open redirect, or a server the attacker controls), and following it would give the webview +/// access to a URL the scope denies. +fn redirect_policy(scope: Scope, max_redirections: Option) -> Policy { if max_redirections == Some(0) { return Policy::none(); } let max_redirections = max_redirections.unwrap_or(DEFAULT_MAX_REDIRECTIONS); - let Some(scope) = scope else { - return Policy::limited(max_redirections); - }; - Policy::custom(move |attempt| { // the first URL in `previous` is the initial request, so it must be excluded if attempt.previous().len() > max_redirections { @@ -228,6 +220,8 @@ fn map_request_error(error: reqwest::Error) -> Error { Error::Network(error) } +// `state` is only read when the `cookies` feature is enabled +#[cfg_attr(not(feature = "cookies"), allow(unused_variables))] #[command] pub async fn fetch( webview: Webview, @@ -311,7 +305,6 @@ pub async fn fetch( builder = builder.connect_timeout(Duration::from_millis(timeout)); } - let scope = state.config.scope_redirects.then_some(scope); builder = builder.redirect(redirect_policy(scope, max_redirections)); if let Some(proxy_config) = proxy { @@ -605,11 +598,7 @@ mod tests { port } - fn get( - url: &str, - scope: Option, - max_redirections: Option, - ) -> Result { + fn get(url: &str, scope: Scope, max_redirections: Option) -> Result { let client = reqwest::ClientBuilder::new() .redirect(redirect_policy(scope, max_redirections)) .build() @@ -620,9 +609,9 @@ mod tests { ) } - fn localhost_scope(port: u16) -> Option { + fn localhost_scope(port: u16) -> Scope { let entry = Arc::new(format!("http://localhost:{port}/*").parse().unwrap()); - Some(Scope::new(vec![entry], Vec::new())) + Scope::new(vec![entry], Vec::new()) } #[test] @@ -644,27 +633,12 @@ mod tests { } } - #[test] - fn redirect_outside_of_scope_is_followed_when_not_configured() { - let port = spawn_server(); - - // `scopeRedirects` is disabled, so only the URL requested by the frontend is checked - let response = get( - &format!("http://localhost:{port}/redirect-external"), - None, - None, - ) - .unwrap(); - - assert_eq!(response.status(), StatusCode::OK); - } - #[test] fn redirect_denied_by_the_scope_is_denied() { let port = spawn_server(); let allow = Arc::new(format!("http://localhost:{port}/*").parse().unwrap()); let deny = Arc::new(format!("http://localhost:{port}/target").parse().unwrap()); - let scope = Some(Scope::new(vec![allow], vec![deny])); + let scope = Scope::new(vec![allow], vec![deny]); let err = get(&format!("http://localhost:{port}/redirect/0"), scope, None).unwrap_err(); @@ -690,16 +664,14 @@ mod tests { fn max_redirections_is_enforced() { let port = spawn_server(); - for scope in [localhost_scope(port), None] { - let err = get( - &format!("http://localhost:{port}/redirect/5"), - scope, - Some(2), - ) - .unwrap_err(); + let err = get( + &format!("http://localhost:{port}/redirect/5"), + localhost_scope(port), + Some(2), + ) + .unwrap_err(); - assert!(matches!(err, Error::Network(e) if e.is_redirect())); - } + assert!(matches!(err, Error::Network(e) if e.is_redirect())); } #[test] diff --git a/plugins/http/src/config.rs b/plugins/http/src/config.rs deleted file mode 100644 index 02f4e9285..000000000 --- a/plugins/http/src/config.rs +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2019-2023 Tauri Programme within The Commons Conservancy -// SPDX-License-Identifier: Apache-2.0 -// SPDX-License-Identifier: MIT - -use serde::Deserialize; - -/// HTTP plugin configuration, defined on the `plugins > http` object of your `tauri.conf.json`. -#[derive(Debug, Clone, Default, Deserialize)] -#[serde(rename_all = "camelCase", deny_unknown_fields)] -pub struct Config { - /// Whether the scope is checked on every hop of a redirect chain instead of only on the - /// URL requested by the frontend. Defaults to `false`. - /// - /// When disabled, a server on an allowed origin can answer with a redirect to any other - /// origin - an open redirect, or a server an attacker controls - and the plugin follows it, - /// handing the webview a response from a URL the scope denies, such as a `localhost` - /// service, an internal host or a cloud metadata endpoint. - /// - /// When enabled, a redirect to a URL that is not allowed by the scope fails with - /// [`Error::UrlNotAllowed`](crate::Error::UrlNotAllowed) instead of being followed, so every - /// redirect target must also be part of the scope. This is opt-in because it breaks - /// applications that rely on being redirected outside of their configured scope. - // TODO(v3): enforce the scope on redirects by default and remove this option - #[serde(default)] - pub scope_redirects: bool, -} - -#[cfg(test)] -mod tests { - use super::Config; - - #[test] - fn deserializes_the_plugin_configuration() { - // the plugin configuration is optional, and the scope check is opt-in - let config: Option = serde_json::from_value(serde_json::Value::Null).unwrap(); - assert!(config.is_none()); - - let config: Config = serde_json::from_str("{}").unwrap(); - assert!(!config.scope_redirects); - - let config: Config = serde_json::from_str(r#"{ "scopeRedirects": true }"#).unwrap(); - assert!(config.scope_redirects); - - // a typo must not silently disable the scope check - assert!(serde_json::from_str::(r#"{ "scopeRedirect": true }"#).is_err()); - } -} diff --git a/plugins/http/src/lib.rs b/plugins/http/src/lib.rs index e33275476..4d19b0048 100644 --- a/plugins/http/src/lib.rs +++ b/plugins/http/src/lib.rs @@ -44,20 +44,12 @@ //! - **unsafe-headers**: Allows webview requests to send any headers. //! - **dangerous-settings**: Allows dangerous client settings such as accepting invalid certificates or hostnames. //! -//! ## Configuration +//! ## Security //! -//! See [`Config`] for the options that can be set on the `plugins > http` object of your -//! `tauri.conf.json`: -//! -//! ```json -//! { -//! "plugins": { -//! "http": { -//! "scopeRedirects": true -//! } -//! } -//! } -//! ``` +//! The URL scope is checked on every hop of a redirect chain, not only on the URL requested by +//! the frontend, so every redirect target must also be allowed by the scope. Otherwise a server on +//! an allowed origin could redirect the request to any other origin - a `localhost` service, an +//! internal host or a cloud metadata endpoint - and hand its response to the webview. pub use reqwest; use tauri::{ @@ -65,11 +57,9 @@ use tauri::{ Manager, Runtime, }; -pub use config::Config; pub use error::{Error, Result}; mod commands; -mod config; mod error; #[cfg(feature = "cookies")] mod reqwest_cookie_store; @@ -79,14 +69,13 @@ mod scope; const COOKIES_FILENAME: &str = ".cookies"; pub(crate) struct Http { - pub(crate) config: Config, #[cfg(feature = "cookies")] cookies_jar: std::sync::Arc, } -pub fn init() -> TauriPlugin> { - Builder::>::new("http") - .setup(|app, api| { +pub fn init() -> TauriPlugin { + Builder::new("http") + .setup(|app, _api| { #[cfg(feature = "cookies")] let cookies_jar = { use crate::reqwest_cookie_store::*; @@ -114,7 +103,6 @@ pub fn init() -> TauriPlugin> { }; let state = Http { - config: api.config().clone().unwrap_or_default(), #[cfg(feature = "cookies")] cookies_jar: std::sync::Arc::new(cookies_jar), }; @@ -138,6 +126,9 @@ pub fn init() -> TauriPlugin> { } } } + + #[cfg(not(feature = "cookies"))] + let _ = (app, event); }) .invoke_handler(tauri::generate_handler![ commands::fetch,