dont follow more than 2 redirects for URL previews for security reasons
Signed-off-by: strawberry <strawberry@puppygock.gay>
This commit is contained in:
parent
fb2eba722d
commit
8e6d52e7dd
2 changed files with 40 additions and 1 deletions
|
@ -432,7 +432,7 @@ fn url_request_allowed(addr: &IpAddr) -> bool {
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn request_url_preview(url: &str) -> Result<UrlPreviewData> {
|
async fn request_url_preview(url: &str) -> Result<UrlPreviewData> {
|
||||||
let client = services().globals.default_client();
|
let client = services().globals.url_preview_client();
|
||||||
let response = client.head(url).send().await?;
|
let response = client.head(url).send().await?;
|
||||||
|
|
||||||
if !response
|
if !response
|
||||||
|
|
|
@ -62,6 +62,7 @@ pub struct Service<'a> {
|
||||||
keypair: Arc<ruma::signatures::Ed25519KeyPair>,
|
keypair: Arc<ruma::signatures::Ed25519KeyPair>,
|
||||||
dns_resolver: TokioAsyncResolver,
|
dns_resolver: TokioAsyncResolver,
|
||||||
jwt_decoding_key: Option<jsonwebtoken::DecodingKey>,
|
jwt_decoding_key: Option<jsonwebtoken::DecodingKey>,
|
||||||
|
url_preview_client: reqwest::Client,
|
||||||
federation_client: reqwest::Client,
|
federation_client: reqwest::Client,
|
||||||
default_client: reqwest::Client,
|
default_client: reqwest::Client,
|
||||||
pub stable_room_versions: Vec<RoomVersionId>,
|
pub stable_room_versions: Vec<RoomVersionId>,
|
||||||
|
@ -171,6 +172,7 @@ impl Service<'_> {
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.map(|secret| jsonwebtoken::DecodingKey::from_secret(secret.as_bytes()));
|
.map(|secret| jsonwebtoken::DecodingKey::from_secret(secret.as_bytes()));
|
||||||
|
|
||||||
|
let url_preview_client = url_preview_reqwest_client_builder(&config)?.build()?;
|
||||||
let default_client = reqwest_client_builder(&config)?.build()?;
|
let default_client = reqwest_client_builder(&config)?.build()?;
|
||||||
let federation_client = reqwest_client_builder(&config)?
|
let federation_client = reqwest_client_builder(&config)?
|
||||||
.dns_resolver(Arc::new(Resolver::new(tls_name_override.clone())))
|
.dns_resolver(Arc::new(Resolver::new(tls_name_override.clone())))
|
||||||
|
@ -212,6 +214,7 @@ impl Service<'_> {
|
||||||
})?,
|
})?,
|
||||||
actual_destination_cache: Arc::new(RwLock::new(WellKnownMap::new())),
|
actual_destination_cache: Arc::new(RwLock::new(WellKnownMap::new())),
|
||||||
tls_name_override,
|
tls_name_override,
|
||||||
|
url_preview_client,
|
||||||
federation_client,
|
federation_client,
|
||||||
default_client,
|
default_client,
|
||||||
jwt_decoding_key,
|
jwt_decoding_key,
|
||||||
|
@ -250,6 +253,13 @@ impl Service<'_> {
|
||||||
&self.keypair
|
&self.keypair
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns a reqwest client which can be used to send requests for URL previews
|
||||||
|
/// This is the same as `default_client()` except a redirect policy of max 2 is set
|
||||||
|
pub fn url_preview_client(&self) -> reqwest::Client {
|
||||||
|
// Client is cheap to clone (Arc wrapper) and avoids lifetime issues
|
||||||
|
self.url_preview_client.clone()
|
||||||
|
}
|
||||||
|
|
||||||
/// Returns a reqwest client which can be used to send requests
|
/// Returns a reqwest client which can be used to send requests
|
||||||
pub fn default_client(&self) -> reqwest::Client {
|
pub fn default_client(&self) -> reqwest::Client {
|
||||||
// Client is cheap to clone (Arc wrapper) and avoids lifetime issues
|
// Client is cheap to clone (Arc wrapper) and avoids lifetime issues
|
||||||
|
@ -596,3 +606,32 @@ fn reqwest_client_builder(config: &Config) -> Result<reqwest::ClientBuilder> {
|
||||||
|
|
||||||
Ok(reqwest_client_builder)
|
Ok(reqwest_client_builder)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn url_preview_reqwest_client_builder(config: &Config) -> Result<reqwest::ClientBuilder> {
|
||||||
|
// for security reasons (e.g. malicious open redirect), we do not want to follow too many redirects when generating URL previews.
|
||||||
|
// let's keep it at least 2 to account for HTTP -> HTTPS upgrades, if it becomes an issue we can consider raising it to 3.
|
||||||
|
let redirect_policy = reqwest::redirect::Policy::custom(|attempt| {
|
||||||
|
if attempt.previous().len() > 2 {
|
||||||
|
attempt.error("Too many redirects (max is 2)")
|
||||||
|
} else {
|
||||||
|
attempt.follow()
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
let mut reqwest_client_builder = reqwest::Client::builder()
|
||||||
|
.pool_max_idle_per_host(0)
|
||||||
|
.connect_timeout(Duration::from_secs(60))
|
||||||
|
.timeout(Duration::from_secs(60 * 5))
|
||||||
|
.redirect(redirect_policy)
|
||||||
|
.user_agent(concat!(
|
||||||
|
env!("CARGO_PKG_NAME"),
|
||||||
|
"/",
|
||||||
|
env!("CARGO_PKG_VERSION")
|
||||||
|
));
|
||||||
|
|
||||||
|
if let Some(proxy) = config.proxy.to_proxy()? {
|
||||||
|
reqwest_client_builder = reqwest_client_builder.proxy(proxy);
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(reqwest_client_builder)
|
||||||
|
}
|
||||||
|
|
Loading…
Add table
Reference in a new issue