From 2a9adc043c0b63598f5581edaeb9a7a738b31f1d Mon Sep 17 00:00:00 2001 From: William Calliari <42240136+w1ll-i-code@users.noreply.github.com> Date: Thu, 15 May 2025 12:10:05 +0200 Subject: [PATCH 1/5] Allow modules to adjust the CSP headers through a dedicated hook. --- .../controllers/NavigationController.php | 55 +------ .../Config/General/ApplicationConfigForm.php | 9 ++ .../Application/Hook/CspDirectiveHook.php | 18 +++ library/Icinga/Util/Csp.php | 143 ++++++++++++++++-- library/Icinga/Util/NavigationItemHelper.php | 78 ++++++++++ 5 files changed, 241 insertions(+), 62 deletions(-) create mode 100644 library/Icinga/Application/Hook/CspDirectiveHook.php create mode 100644 library/Icinga/Util/NavigationItemHelper.php diff --git a/application/controllers/NavigationController.php b/application/controllers/NavigationController.php index 305ae90421..3838a64f13 100644 --- a/application/controllers/NavigationController.php +++ b/application/controllers/NavigationController.php @@ -7,9 +7,9 @@ use Icinga\Application\Config; use Icinga\Exception\NotFoundError; use Icinga\Data\DataArray\ArrayDatasource; -use Icinga\Data\Filter\FilterMatchCaseInsensitive; use Icinga\Forms\ConfirmRemovalForm; use Icinga\Forms\Navigation\NavigationConfigForm; +use Icinga\Util\NavigationItemHelper; use Icinga\Web\Controller; use Icinga\Web\Form; use Icinga\Web\Menu; @@ -65,63 +65,12 @@ protected function listItemTypes() return $types; } - /** - * Return all shared navigation item configurations - * - * @param string $owner A username if only items shared by a specific user are desired - * - * @return array - */ - protected function fetchSharedNavigationItemConfigs($owner = null) - { - $configs = array(); - foreach ($this->itemTypeConfig as $type => $_) { - $config = Config::navigation($type); - $config->getConfigObject()->setKeyColumn('name'); - $query = $config->select(); - if ($owner !== null) { - $query->applyFilter(new FilterMatchCaseInsensitive('owner', '=', $owner)); - } - - foreach ($query as $itemConfig) { - $configs[] = $itemConfig; - } - } - - return $configs; - } - - /** - * Return all user navigation item configurations - * - * @param string $username - * - * @return array - */ - protected function fetchUserNavigationItemConfigs($username) - { - $configs = array(); - foreach ($this->itemTypeConfig as $type => $_) { - $config = Config::navigation($type, $username); - $config->getConfigObject()->setKeyColumn('name'); - foreach ($config->select() as $itemConfig) { - $configs[] = $itemConfig; - } - } - - return $configs; - } - /** * Show the current user a list of their navigation items */ public function indexAction() { - $user = $this->Auth()->getUser(); - $ds = new ArrayDatasource(array_merge( - $this->fetchSharedNavigationItemConfigs($user->getUsername()), - $this->fetchUserNavigationItemConfigs($user->getUsername()) - )); + $ds = new ArrayDatasource(NavigationItemHelper::fetchUserNavigationItems($this->Auth()->getUser())); $query = $ds->select(); $this->view->types = $this->listItemTypes(); diff --git a/application/forms/Config/General/ApplicationConfigForm.php b/application/forms/Config/General/ApplicationConfigForm.php index b88ea5dfaf..ac2e507e70 100644 --- a/application/forms/Config/General/ApplicationConfigForm.php +++ b/application/forms/Config/General/ApplicationConfigForm.php @@ -4,8 +4,10 @@ namespace Icinga\Forms\Config\General; use Icinga\Application\Icinga; +use Icinga\Authentication\Auth; use Icinga\Data\ResourceFactory; use Icinga\Web\Form; +use Icinga\Util\Csp; /** * Configuration form for general application options @@ -60,6 +62,7 @@ public function createElements(array $formData) 'security_use_strict_csp', [ 'label' => $this->translate('Enable strict content security policy'), + 'autosubmit' => true, 'description' => $this->translate( 'Set whether to use strict content security policy (CSP).' . ' This setting helps to protect from cross-site scripting (XSS).' @@ -67,6 +70,12 @@ public function createElements(array $formData) ] ); + if ($formData['security_use_strict_csp']) { + Csp::createNonce(); + $header = Csp::getContentSecurityPolicy(Auth::getInstance()->getUser()); + $this->addHint("Content-Security-Policy: $header"); + } + $this->addElement( 'text', 'global_module_path', diff --git a/library/Icinga/Application/Hook/CspDirectiveHook.php b/library/Icinga/Application/Hook/CspDirectiveHook.php new file mode 100644 index 0000000000..43eb6c26eb --- /dev/null +++ b/library/Icinga/Application/Hook/CspDirectiveHook.php @@ -0,0 +1,18 @@ + [ 'https://*.media.tumblr.com', 'https://http.cat/' ] ] + * + * @return array The CSP directives are the keys and the policies the values. + */ + abstract public function getCspDirectives(): array; +} diff --git a/library/Icinga/Util/Csp.php b/library/Icinga/Util/Csp.php index c7fbf9a4c9..9ad28da9c7 100644 --- a/library/Icinga/Util/Csp.php +++ b/library/Icinga/Util/Csp.php @@ -4,6 +4,13 @@ namespace Icinga\Util; +use Icinga\Application\Hook; +use Icinga\Application\Hook\CspDirectiveHook; +use Icinga\Application\Icinga; +use Icinga\Application\Logger; +use Icinga\Authentication\Auth; +use Icinga\Data\ConfigObject; +use Icinga\User; use Icinga\Web\Response; use Icinga\Web\Window; use RuntimeException; @@ -45,17 +52,131 @@ private function __construct() */ public static function addHeader(Response $response): void { + $user = Auth::getInstance()->getUser(); + $header = static::getContentSecurityPolicy($user); + Logger::debug("Setting Content-Security-Policy header for user {$user->getUsername()} to $header"); + $response->setHeader('Content-Security-Policy', $header, true); + } + + /** + * Get the Content-Security-Policy for a specific user. + * + * @param User $user + * + * @throws RuntimeException If no nonce set for CSS + * + * @return string Returns the generated header value. + */ + public static function getContentSecurityPolicy(User $user): string { $csp = static::getInstance(); if (empty($csp->styleNonce)) { throw new RuntimeException('No nonce set for CSS'); } - $response->setHeader( - 'Content-Security-Policy', - "script-src 'self'; style-src 'self' 'nonce-$csp->styleNonce';", - true - ); + // These are the default directives that should always be enforced. 'self' is valid for all + // directives and will therefor not be listed here. + $cspDirectives = [ + 'style-src' => ["'nonce-{$csp->styleNonce}'"], + 'font-src' => ["data:"], + 'img-src' => ["data:"], + 'frame-src' => [] + ]; + + // Whitelist the hosts in the custom NavigationItems configured for the user, + // so that the iframes can be rendered properly. + /** @var array $navigationItems */ + $navigationItems = NavigationItemHelper::fetchUserNavigationItems($user); + foreach ($navigationItems as $navigationItem) { + + // Skip the host if the link gets opened in a new window. + if ($navigationItem->get("target", "") === "_blank") { + continue; + } + + $name = $navigationItem->get("name", ""); + $url = $navigationItem->get("url", ""); + + $scheme = parse_url($url, PHP_URL_SCHEME); + $host = parse_url($url, PHP_URL_HOST); + + if ($host === null || !static::validateCspPolicy("NavigationItem '$name'", "frame-src", $host)) { + continue; + } + + $policy = $host; + if ($scheme !== null) { + $policy = "$scheme://$host"; + } + + $cspDirectives['frame-src'][] = $policy; + } + + // Allow modules to add their own csp directives in a limited fashion. + /** @var CspDirectiveHook $hook */ + foreach (Hook::all('CspDirective') as $hook) { + foreach ($hook->getCspDirectives() as $directive => $policies) { + + // policy names contain only lowercase letters and '-'. Reject anything else. + if (!preg_match('|^[a-z\-]+$|', $directive)) { + $errorSource = get_class($hook); + Logger::debug("$errorSource: Invalid CSP directive found: $directive"); + continue; + } + + // The default-src can only ever be 'self'. Disallow any updates to it. + if ($directive === "default-src") { + $errorSource = get_class($hook); + Logger::debug("$errorSource: Changing default-src is forbidden."); + continue; + } + + $cspDirectives[$directive] = $cspDirectives[$directive] ?? []; + foreach ($policies as $policy) { + $source = get_class($hook); + if (!static::validateCspPolicy($source, $directive, $policy)) { + continue; + } + + $cspDirectives[$directive][] = $policy; + } + } + } + + $header = "default-src 'self'; "; + foreach ($cspDirectives as $directive => $policies) { + if (!empty($policies)) { + $header .= ' ' . implode(' ', array_merge([$directive, "'self'"], array_unique($policies))) . ';'; + } + } + + return $header; + } + + public static function validateCspPolicy(string $source, string $directive, string $policy): bool { + // We accept the following policies: + // 1. Hosts: Modules can whitelist certain domains as sources for the CSP header directives. + // - A host can have a specific scheme (http or https). + // - A host can whitelist all subdomains with * + // - A host can contain all alphanumeric characters as well as '+', '-', '_', '.', and ':' + // 2. Nonce: Modules are allowed to specify custom nonce for some directives. + // - A nonce is enclosed in single-quotes: "'" + // - A nonce begins with 'nonce-' followed by at least 22 significant characters of base64 encoded data. + // as recommended by the standard: https://content-security-policy.com/nonce/ + if (! preg_match("/^((https?:\/\/)?\*?[a-zA-Z0-9+._\-:]+|'nonce-[a-zA-Z0-9+\/]{22,}={0,3}')$/", $policy)) { + Logger::debug("$source: Invalid CSP policy found: $directive $policy"); + return false; + } + + // We refuse all overly aggressive whitelisting by default. This includes: + // 1. Whitelisting all Hosts with '*' + // 2. Whitelisting all Hosts in a tld, e.g. 'http://*.com' + if (preg_match('|\*(\.[a-zA-Z]+)?$|', $directive)) { + Logger::debug("$source: Disallowing whitelisting all hosts. $directive"); + return false; + } + + return true; } /** @@ -67,9 +188,10 @@ public static function addHeader(Response $response): void public static function createNonce(): void { $csp = static::getInstance(); - $csp->styleNonce = base64_encode(random_bytes(16)); - - Window::getInstance()->getSessionNamespace('csp')->set('style_nonce', $csp->styleNonce); + if ($csp->styleNonce === null) { + $csp->styleNonce = base64_encode(random_bytes(16)); + Window::getInstance()->getSessionNamespace('csp')->set('style_nonce', $csp->styleNonce); + } } /** @@ -79,7 +201,10 @@ public static function createNonce(): void */ public static function getStyleNonce(): ?string { - return static::getInstance()->styleNonce; + if (Icinga::app()->isWeb()) { + return static::getInstance()->styleNonce; + } + return null; } /** diff --git a/library/Icinga/Util/NavigationItemHelper.php b/library/Icinga/Util/NavigationItemHelper.php new file mode 100644 index 0000000000..c3cdb4c63f --- /dev/null +++ b/library/Icinga/Util/NavigationItemHelper.php @@ -0,0 +1,78 @@ +getUsername(); + self::$navigationItemCache = array_merge( + static::fetchSharedNavigationItemConfigs($itemTypeConfig, $username), + static::fetchUserNavigationItemConfigs($itemTypeConfig, $username) + ); + + return self::$navigationItemCache; + } + + /** + * Return all shared navigation item configurations + * + * @param string $owner A username if only items shared by a specific user are desired + * + * @return array + */ + protected static function fetchSharedNavigationItemConfigs($itemTypeConfig, string $owner) + { + $configs = array(); + foreach ($itemTypeConfig as $type => $_) { + $config = Config::navigation($type); + $config->getConfigObject()->setKeyColumn('name'); + $query = $config->select(); + if ($owner !== null) { + $query->applyFilter(new FilterMatchCaseInsensitive('owner', '=', $owner)); + } + + foreach ($query as $itemConfig) { + $configs[] = $itemConfig; + } + } + + return $configs; + } + + /** + * Return all user navigation item configurations + * + * @param string $username + * + * @return array + */ + protected static function fetchUserNavigationItemConfigs($itemTypeConfig, string $username) + { + $configs = array(); + foreach ($itemTypeConfig as $type => $_) { + $config = Config::navigation($type, $username); + $config->getConfigObject()->setKeyColumn('name'); + foreach ($config->select() as $itemConfig) { + $configs[] = $itemConfig; + } + } + + return $configs; + } + +} \ No newline at end of file From f373d3099e779beb7552015020a82ab94be574a9 Mon Sep 17 00:00:00 2001 From: William Calliari <42240136+w1ll-i-code@users.noreply.github.com> Date: Mon, 19 May 2025 15:18:04 +0200 Subject: [PATCH 2/5] Add additional validation for the url before using it in the frame-src scp header --- library/Icinga/Util/Csp.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/Icinga/Util/Csp.php b/library/Icinga/Util/Csp.php index 9ad28da9c7..7f239be676 100644 --- a/library/Icinga/Util/Csp.php +++ b/library/Icinga/Util/Csp.php @@ -85,7 +85,7 @@ public static function getContentSecurityPolicy(User $user): string { // Whitelist the hosts in the custom NavigationItems configured for the user, // so that the iframes can be rendered properly. - /** @var array $navigationItems */ + /** @var ConfigObject[] $navigationItems */ $navigationItems = NavigationItemHelper::fetchUserNavigationItems($user); foreach ($navigationItems as $navigationItem) { @@ -95,12 +95,19 @@ public static function getContentSecurityPolicy(User $user): string { } $name = $navigationItem->get("name", ""); + $errorSource = "NavigationItem '$name'"; $url = $navigationItem->get("url", ""); + // Make sure $url is actually valid; + if (filter_var($url, FILTER_VALIDATE_URL) === false) { + Logger::debug("$errorSource: Skipping invalid url: $host"); + continue; + } + $scheme = parse_url($url, PHP_URL_SCHEME); $host = parse_url($url, PHP_URL_HOST); - if ($host === null || !static::validateCspPolicy("NavigationItem '$name'", "frame-src", $host)) { + if ($host === null || !static::validateCspPolicy($errorSource, "frame-src", $host)) { continue; } From 78bfc90d222b39a5d6cdf2e1e8a6361c6954a280 Mon Sep 17 00:00:00 2001 From: Davide Zeni Date: Wed, 6 Aug 2025 19:37:03 +0200 Subject: [PATCH 3/5] Allow editing of the CSP trusted image sources --- .../controllers/NavigationController.php | 57 ++++++++- library/Icinga/Util/Csp.php | 116 +++++++++++++++--- library/Icinga/Util/NavigationItemHelper.php | 78 ------------ 3 files changed, 151 insertions(+), 100 deletions(-) delete mode 100644 library/Icinga/Util/NavigationItemHelper.php diff --git a/application/controllers/NavigationController.php b/application/controllers/NavigationController.php index 3838a64f13..31a8beb96e 100644 --- a/application/controllers/NavigationController.php +++ b/application/controllers/NavigationController.php @@ -7,9 +7,9 @@ use Icinga\Application\Config; use Icinga\Exception\NotFoundError; use Icinga\Data\DataArray\ArrayDatasource; +use Icinga\Data\Filter\FilterMatchCaseInsensitive; use Icinga\Forms\ConfirmRemovalForm; use Icinga\Forms\Navigation\NavigationConfigForm; -use Icinga\Util\NavigationItemHelper; use Icinga\Web\Controller; use Icinga\Web\Form; use Icinga\Web\Menu; @@ -65,12 +65,63 @@ protected function listItemTypes() return $types; } + /** + * Return all shared navigation item configurations + * + * @param string $owner A username if only items shared by a specific user are desired + * + * @return array + */ + protected function fetchSharedNavigationItemConfigs($owner = null) + { + $configs = array(); + foreach ($this->itemTypeConfig as $type => $_) { + $config = Config::navigation($type); + $config->getConfigObject()->setKeyColumn('name'); + $query = $config->select(); + if ($owner !== null) { + $query->applyFilter(new FilterMatchCaseInsensitive('owner', '=', $owner)); + } + + foreach ($query as $itemConfig) { + $configs[] = $itemConfig; + } + } + + return $configs; + } + + /** + * Return all user navigation item configurations + * + * @param string $username + * + * @return array + */ + protected function fetchUserNavigationItemConfigs($username) + { + $configs = array(); + foreach ($this->itemTypeConfig as $type => $_) { + $config = Config::navigation($type, $username); + $config->getConfigObject()->setKeyColumn('name'); + foreach ($config->select() as $itemConfig) { + $configs[] = $itemConfig; + } + } + + return $configs; + } + /** * Show the current user a list of their navigation items */ public function indexAction() { - $ds = new ArrayDatasource(NavigationItemHelper::fetchUserNavigationItems($this->Auth()->getUser())); + $user = $this->Auth()->getUser(); + $ds = new ArrayDatasource(array_merge( + $this->fetchSharedNavigationItemConfigs($user->getUsername()), + $this->fetchUserNavigationItemConfigs($user->getUsername()) + )); $query = $ds->select(); $this->view->types = $this->listItemTypes(); @@ -393,4 +444,4 @@ public function dashboardAction() $this->view->navigation = $navigation; $this->view->title = $navigation->getLabel(); } -} +} \ No newline at end of file diff --git a/library/Icinga/Util/Csp.php b/library/Icinga/Util/Csp.php index 7f239be676..38d872ef37 100644 --- a/library/Icinga/Util/Csp.php +++ b/library/Icinga/Util/Csp.php @@ -13,7 +13,10 @@ use Icinga\User; use Icinga\Web\Response; use Icinga\Web\Window; +use Icinga\Application\Config; use RuntimeException; +use Icinga\Web\Navigation\Navigation; +use Icinga\Web\Widget\Dashboard; use function ipl\Stdlib\get_php_type; @@ -53,7 +56,7 @@ private function __construct() public static function addHeader(Response $response): void { $user = Auth::getInstance()->getUser(); - $header = static::getContentSecurityPolicy($user); + $header = static::getContentSecurityPolicy(); Logger::debug("Setting Content-Security-Policy header for user {$user->getUsername()} to $header"); $response->setHeader('Content-Security-Policy', $header, true); } @@ -67,7 +70,8 @@ public static function addHeader(Response $response): void * * @return string Returns the generated header value. */ - public static function getContentSecurityPolicy(User $user): string { + public static function getContentSecurityPolicy(): string + { $csp = static::getInstance(); if (empty($csp->styleNonce)) { @@ -86,26 +90,19 @@ public static function getContentSecurityPolicy(User $user): string { // Whitelist the hosts in the custom NavigationItems configured for the user, // so that the iframes can be rendered properly. /** @var ConfigObject[] $navigationItems */ - $navigationItems = NavigationItemHelper::fetchUserNavigationItems($user); + $navigationItems = self::fetchDashletNavigationItemConfigs(); foreach ($navigationItems as $navigationItem) { + $errorSource = sprintf("Navigation item %s", $navigationItem['name']); - // Skip the host if the link gets opened in a new window. - if ($navigationItem->get("target", "") === "_blank") { - continue; - } - - $name = $navigationItem->get("name", ""); - $errorSource = "NavigationItem '$name'"; - $url = $navigationItem->get("url", ""); - + $host = parse_url($navigationItem["url"], PHP_URL_HOST); // Make sure $url is actually valid; - if (filter_var($url, FILTER_VALIDATE_URL) === false) { + if (filter_var($navigationItem["url"], FILTER_VALIDATE_URL) === false) { Logger::debug("$errorSource: Skipping invalid url: $host"); continue; } - $scheme = parse_url($url, PHP_URL_SCHEME); - $host = parse_url($url, PHP_URL_HOST); + $scheme = parse_url($navigationItem["url"], PHP_URL_SCHEME); + if ($host === null || !static::validateCspPolicy($errorSource, "frame-src", $host)) { continue; @@ -118,12 +115,10 @@ public static function getContentSecurityPolicy(User $user): string { $cspDirectives['frame-src'][] = $policy; } - // Allow modules to add their own csp directives in a limited fashion. /** @var CspDirectiveHook $hook */ foreach (Hook::all('CspDirective') as $hook) { foreach ($hook->getCspDirectives() as $directive => $policies) { - // policy names contain only lowercase letters and '-'. Reject anything else. if (!preg_match('|^[a-z\-]+$|', $directive)) { $errorSource = get_class($hook); @@ -156,11 +151,12 @@ public static function getContentSecurityPolicy(User $user): string { $header .= ' ' . implode(' ', array_merge([$directive, "'self'"], array_unique($policies))) . ';'; } } - + return $header; } - public static function validateCspPolicy(string $source, string $directive, string $policy): bool { + public static function validateCspPolicy(string $source, string $directive, string $policy): bool + { // We accept the following policies: // 1. Hosts: Modules can whitelist certain domains as sources for the CSP header directives. // - A host can have a specific scheme (http or https). @@ -240,4 +236,86 @@ protected static function getInstance(): self return static::$instance; } + + + /** + * Fetches and merges configurations for navigation menu items and dashlets. + * + * @return array An array containing both navigation items and dashlet configurations. + * // returns [['name' => 'Item Name', 'url' => 'https://example.com'], ...] + */ + protected static function fetchDashletNavigationItemConfigs() + { + return array_merge( + self::fetchNavigationItems(), + self::fetchDashletsItems() + ); + } + + /** + * Fetches navigation items for the current user. + * + * Iterates through all registered navigation types, loads both user-specific + * and shared configurations, and returns a list of menu items. + * + * @return array Each item is an associative array with 'name' and 'url' keys. + * Example: [ ['name' => 'Home', 'url' => '/'], ['name' => 'Profile', 'url' => '/profile'] ] + */ + protected static function fetchNavigationItems() + { + $username = Auth::getInstance()->getUser()->getUsername(); + $navigationType = Navigation::getItemTypeConfiguration(); + foreach ($navigationType as $type => $_) { + $config = Config::navigation($type, $username); + $config->getConfigObject()->setKeyColumn('name'); + $configShared = Config::navigation($type); + $configShared->getConfigObject()->setKeyColumn('name'); + foreach ($config->select() as $itemConfig) { + $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; + } + foreach ($configShared->select() as $itemConfig) { + $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; + } + } + return $menuItems; + } + + /** + * Fetches all dashlets for the current user that have an external URL. + * + * @return array A list of dashlets with their names and absolute URLs. + * // returns [['name' => 'Dashlet Name', 'url' => 'https://external.dashlet.com'], ...] + */ + protected static function fetchDashletsItems() + { + $dashboard = new Dashboard(); + $dashboard->setUser(Auth::getInstance()->getUser()); + $dashboard->load(); + $dashlets = []; + foreach ($dashboard->getPanes() as $pane) { + foreach ($pane->getDashlets() as $dashlet) { + $url = $dashlet->getUrl(); + // Prefer explicit external URL parameter if present + $externalUrl = $url->getParam("url"); + if ($externalUrl !== null) { + $dashlets[] = [ + "name" => $dashlet->getName(), + "url" => $externalUrl + ]; + continue; + } + + // Otherwise, check if the dashlet URL itself is external + if ($url->isExternal()) { + $dashlets[] = [ + "name" => $dashlet->getName(), + "url" => $url->getAbsoluteUrl() + ]; + } + } + } + + return $dashlets; + } + } diff --git a/library/Icinga/Util/NavigationItemHelper.php b/library/Icinga/Util/NavigationItemHelper.php deleted file mode 100644 index c3cdb4c63f..0000000000 --- a/library/Icinga/Util/NavigationItemHelper.php +++ /dev/null @@ -1,78 +0,0 @@ -getUsername(); - self::$navigationItemCache = array_merge( - static::fetchSharedNavigationItemConfigs($itemTypeConfig, $username), - static::fetchUserNavigationItemConfigs($itemTypeConfig, $username) - ); - - return self::$navigationItemCache; - } - - /** - * Return all shared navigation item configurations - * - * @param string $owner A username if only items shared by a specific user are desired - * - * @return array - */ - protected static function fetchSharedNavigationItemConfigs($itemTypeConfig, string $owner) - { - $configs = array(); - foreach ($itemTypeConfig as $type => $_) { - $config = Config::navigation($type); - $config->getConfigObject()->setKeyColumn('name'); - $query = $config->select(); - if ($owner !== null) { - $query->applyFilter(new FilterMatchCaseInsensitive('owner', '=', $owner)); - } - - foreach ($query as $itemConfig) { - $configs[] = $itemConfig; - } - } - - return $configs; - } - - /** - * Return all user navigation item configurations - * - * @param string $username - * - * @return array - */ - protected static function fetchUserNavigationItemConfigs($itemTypeConfig, string $username) - { - $configs = array(); - foreach ($itemTypeConfig as $type => $_) { - $config = Config::navigation($type, $username); - $config->getConfigObject()->setKeyColumn('name'); - foreach ($config->select() as $itemConfig) { - $configs[] = $itemConfig; - } - } - - return $configs; - } - -} \ No newline at end of file From ecba402ae09e5676e9fdf8d9abb65b2c1268e69b Mon Sep 17 00:00:00 2001 From: Davide Zeni Date: Mon, 25 Aug 2025 13:47:52 +0200 Subject: [PATCH 4/5] Refactor CSP validation logic and improve access control for shared navigation items --- library/Icinga/Application/Web.php | 2 +- library/Icinga/Util/Csp.php | 43 +++++------------------------- 2 files changed, 8 insertions(+), 37 deletions(-) diff --git a/library/Icinga/Application/Web.php b/library/Icinga/Application/Web.php index 934af0745d..5b5a3499cb 100644 --- a/library/Icinga/Application/Web.php +++ b/library/Icinga/Application/Web.php @@ -176,7 +176,7 @@ public function getViewRenderer() return $this->viewRenderer; } - private function hasAccessToSharedNavigationItem(&$config, Config $navConfig) + public function hasAccessToSharedNavigationItem(&$config, Config $navConfig) { // TODO: Provide a more sophisticated solution diff --git a/library/Icinga/Util/Csp.php b/library/Icinga/Util/Csp.php index 38d872ef37..3be746d0ef 100644 --- a/library/Icinga/Util/Csp.php +++ b/library/Icinga/Util/Csp.php @@ -104,7 +104,7 @@ public static function getContentSecurityPolicy(): string $scheme = parse_url($navigationItem["url"], PHP_URL_SCHEME); - if ($host === null || !static::validateCspPolicy($errorSource, "frame-src", $host)) { + if ($host === null) { continue; } @@ -135,11 +135,6 @@ public static function getContentSecurityPolicy(): string $cspDirectives[$directive] = $cspDirectives[$directive] ?? []; foreach ($policies as $policy) { - $source = get_class($hook); - if (!static::validateCspPolicy($source, $directive, $policy)) { - continue; - } - $cspDirectives[$directive][] = $policy; } } @@ -154,34 +149,6 @@ public static function getContentSecurityPolicy(): string return $header; } - - public static function validateCspPolicy(string $source, string $directive, string $policy): bool - { - // We accept the following policies: - // 1. Hosts: Modules can whitelist certain domains as sources for the CSP header directives. - // - A host can have a specific scheme (http or https). - // - A host can whitelist all subdomains with * - // - A host can contain all alphanumeric characters as well as '+', '-', '_', '.', and ':' - // 2. Nonce: Modules are allowed to specify custom nonce for some directives. - // - A nonce is enclosed in single-quotes: "'" - // - A nonce begins with 'nonce-' followed by at least 22 significant characters of base64 encoded data. - // as recommended by the standard: https://content-security-policy.com/nonce/ - if (! preg_match("/^((https?:\/\/)?\*?[a-zA-Z0-9+._\-:]+|'nonce-[a-zA-Z0-9+\/]{22,}={0,3}')$/", $policy)) { - Logger::debug("$source: Invalid CSP policy found: $directive $policy"); - return false; - } - - // We refuse all overly aggressive whitelisting by default. This includes: - // 1. Whitelisting all Hosts with '*' - // 2. Whitelisting all Hosts in a tld, e.g. 'http://*.com' - if (preg_match('|\*(\.[a-zA-Z]+)?$|', $directive)) { - Logger::debug("$source: Disallowing whitelisting all hosts. $directive"); - return false; - } - - return true; - } - /** * Set/recreate nonce for dynamic CSS * @@ -271,10 +238,14 @@ protected static function fetchNavigationItems() $configShared = Config::navigation($type); $configShared->getConfigObject()->setKeyColumn('name'); foreach ($config->select() as $itemConfig) { - $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; + if ( $itemConfig->get("target", "") !== "_blank") { + $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; + } } foreach ($configShared->select() as $itemConfig) { - $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; + if (Icinga::app()->hasAccessToSharedNavigationItem($itemConfig, $config) && $itemConfig->get("target", "") !== "_blank") { + $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; + } } } return $menuItems; From c35cf0265c8489d6532073dbd23d3d3728777360 Mon Sep 17 00:00:00 2001 From: Davide Zeni Date: Tue, 2 Sep 2025 11:36:40 +0200 Subject: [PATCH 5/5] Refactor CSP handling to improve user checks --- library/Icinga/Util/Csp.php | 76 +++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/library/Icinga/Util/Csp.php b/library/Icinga/Util/Csp.php index 3be746d0ef..591e6fe307 100644 --- a/library/Icinga/Util/Csp.php +++ b/library/Icinga/Util/Csp.php @@ -55,17 +55,13 @@ private function __construct() */ public static function addHeader(Response $response): void { - $user = Auth::getInstance()->getUser(); $header = static::getContentSecurityPolicy(); - Logger::debug("Setting Content-Security-Policy header for user {$user->getUsername()} to $header"); $response->setHeader('Content-Security-Policy', $header, true); } /** * Get the Content-Security-Policy for a specific user. * - * @param User $user - * * @throws RuntimeException If no nonce set for CSS * * @return string Returns the generated header value. @@ -230,18 +226,22 @@ protected static function fetchDashletNavigationItemConfigs() */ protected static function fetchNavigationItems() { - $username = Auth::getInstance()->getUser()->getUsername(); + $user = Auth::getInstance()->getUser(); + $menuItems = []; + if ($user === null) { + return $menuItems; + } $navigationType = Navigation::getItemTypeConfiguration(); foreach ($navigationType as $type => $_) { - $config = Config::navigation($type, $username); + $config = Config::navigation($type, $user->getUsername()); $config->getConfigObject()->setKeyColumn('name'); - $configShared = Config::navigation($type); - $configShared->getConfigObject()->setKeyColumn('name'); foreach ($config->select() as $itemConfig) { - if ( $itemConfig->get("target", "") !== "_blank") { + if ($itemConfig->get("target", "") !== "_blank") { $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; } } + $configShared = Config::navigation($type); + $configShared->getConfigObject()->setKeyColumn('name'); foreach ($configShared->select() as $itemConfig) { if (Icinga::app()->hasAccessToSharedNavigationItem($itemConfig, $config) && $itemConfig->get("target", "") !== "_blank") { $menuItems[] = ["name" => $itemConfig->get('name'), "url" => $itemConfig->get('url')]; @@ -259,34 +259,44 @@ protected static function fetchNavigationItems() */ protected static function fetchDashletsItems() { - $dashboard = new Dashboard(); - $dashboard->setUser(Auth::getInstance()->getUser()); - $dashboard->load(); - $dashlets = []; - foreach ($dashboard->getPanes() as $pane) { - foreach ($pane->getDashlets() as $dashlet) { - $url = $dashlet->getUrl(); - // Prefer explicit external URL parameter if present - $externalUrl = $url->getParam("url"); - if ($externalUrl !== null) { - $dashlets[] = [ - "name" => $dashlet->getName(), - "url" => $externalUrl - ]; - continue; - } + $user = Auth::getInstance()->getUser(); + $dashlets = []; + if ($user === null) { + return $dashlets; + } + + $dashboard = new Dashboard(); + $dashboard->setUser($user); + $dashboard->load(); + + foreach ($dashboard->getPanes() as $pane) { + foreach ($pane->getDashlets() as $dashlet) { + $url = $dashlet->getUrl(); + if ($url === null) { + continue; + } + + $externalUrl = $url->getParam("url"); + if ($externalUrl !== null && filter_var($externalUrl, FILTER_VALIDATE_URL) !== false) { + $dashlets[] = [ + "name" => $dashlet->getName(), + "url" => $externalUrl + ]; + continue; + } - // Otherwise, check if the dashlet URL itself is external - if ($url->isExternal()) { + if ($url->isExternal()) { + $absoluteUrl = $url->getAbsoluteUrl(); + if (filter_var($absoluteUrl, FILTER_VALIDATE_URL) !== false) { $dashlets[] = [ - "name" => $dashlet->getName(), - "url" => $url->getAbsoluteUrl() + "name" => $dashlet->getName(), + "url" => $absoluteUrl ]; } - } - } - - return $dashlets; + } + } + } + return $dashlets; } }