Browse Source

Tighten the KEY_CONNECTOR_URL validation

Url::parse alone also accepts schemes like javascript: that make no
sense here, so require http(s) like the other URL settings do. The CSP
entry now only contains the origin of the configured URL, since a query
or fragment would produce an invalid source expression. Also removes an
unused import from the playwright mock.
pull/7419/head
Luca Biemelt 1 week ago
parent
commit
7adf1b50cf
  1. 2
      playwright/tests/setups/keyconnector.ts
  2. 5
      src/config.rs
  3. 6
      src/util.rs

2
playwright/tests/setups/keyconnector.ts

@ -1,4 +1,4 @@
import { createServer, type Server } from 'node:http'; import { createServer } from 'node:http';
/** /**
* Barebone in-memory key connector, just enough for the clients to enroll and unlock. * Barebone in-memory key connector, just enough for the clients to enroll and unlock.

5
src/config.rs

@ -1104,8 +1104,9 @@ fn validate_config(cfg: &ConfigItems, on_update: bool) -> Result<(), Error> {
if cfg.key_connector_url.is_empty() { if cfg.key_connector_url.is_empty() {
err!("`KEY_CONNECTOR_URL` must be set when Key Connector is enabled") err!("`KEY_CONNECTOR_URL` must be set when Key Connector is enabled")
} }
if Url::parse(&cfg.key_connector_url).is_err() { let kc_url = cfg.key_connector_url.to_lowercase();
err!("Invalid URL format for `KEY_CONNECTOR_URL`."); if !kc_url.starts_with("http://") && !kc_url.starts_with("https://") || Url::parse(&kc_url).is_err() {
err!("`KEY_CONNECTOR_URL` must be a valid URL containing the protocol (http, https)")
} }
} }

6
src/util.rs

@ -135,9 +135,11 @@ impl Fairing for AppHeaders {
allowed_iframe_ancestors = CONFIG.allowed_iframe_ancestors(), allowed_iframe_ancestors = CONFIG.allowed_iframe_ancestors(),
allowed_connect_src = CONFIG.allowed_connect_src(), allowed_connect_src = CONFIG.allowed_connect_src(),
// The web vault fetches the master key from the key connector, so it // The web vault fetches the master key from the key connector, so it
// has to be allowed to connect to it. // has to be allowed to connect to it. Only use the origin here, a
// query or fragment in the URL would break the source expression.
key_connector_src = if CONFIG.key_connector_enabled() { key_connector_src = if CONFIG.key_connector_enabled() {
CONFIG.key_connector_url() url::Url::parse(&CONFIG.key_connector_url())
.map_or_else(|_| String::new(), |u| u.origin().ascii_serialization())
} else { } else {
String::new() String::new()
}, },

Loading…
Cancel
Save