From 43a9bd1fb8b3f57e7935a6a6bc48975e2063b01b Mon Sep 17 00:00:00 2001 From: Angel Fernando Quiroz Campos Date: Fri, 11 Oct 2024 13:19:47 -0500 Subject: [PATCH] Security: Add 'auth_openid_allowed_providers' configuration setting to fix potential unauthenticated blind SSRF via openid. --- main/auth/openid/login.php | 33 +++++++++++++++++++++++++++-- main/inc/lib/template.lib.php | 2 +- main/inc/local.inc.php | 18 ++++++++++------ main/install/configuration.dist.php | 6 ++++++ 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/main/auth/openid/login.php b/main/auth/openid/login.php index f3cda60ac6..3fa1359379 100755 --- a/main/auth/openid/login.php +++ b/main/auth/openid/login.php @@ -14,7 +14,7 @@ require_once 'openid.lib.php'; require_once 'xrds.lib.php'; -function openid_form() +function openid_form(): FormValidator { $form = new FormValidator( 'openid_login', @@ -25,8 +25,10 @@ function openid_form() ); $form -> addElement('text', 'openid_url', array(get_lang('OpenIDURL'), Display::url(get_lang('OpenIDWhatIs'), 'main/auth/openid/whatis.php')), array('class' => 'openid_input')); $form -> addElement('button', 'submit', get_lang('Login')); + $form->applyFilter('openid_url', 'trim'); + $form->protect(); - return $form->returnForm(); + return $form; } /** @@ -459,3 +461,30 @@ function openid_http_request($url, $headers = array(), $method = 'GET', $data = $result->code = $code; return $result; } + +function openid_is_allowed_provider($identityUrl): bool +{ + $allowedProviders = api_get_configuration_value('auth_openid_allowed_providers'); + + if (false === $allowedProviders) { + return true; + } + + $host = parse_url($identityUrl, PHP_URL_HOST) ?: $identityUrl; + + foreach ($allowedProviders as $provider) { + if (strpos($provider, '*') !== false) { + $regex = '/^' . str_replace('\*', '.*', preg_quote($provider, '/')) . '$/'; + + if (preg_match($regex, $host)) { + return true; + } + } else { + if ($host === $provider) { + return true; + } + } + } + + return false; +} diff --git a/main/inc/lib/template.lib.php b/main/inc/lib/template.lib.php index 83121e6796..f6c1fc3587 100755 --- a/main/inc/lib/template.lib.php +++ b/main/inc/lib/template.lib.php @@ -1318,7 +1318,7 @@ class Template $html = $form->returnForm(); if (api_get_setting('openid_authentication') == 'true') { include_once api_get_path(SYS_CODE_PATH).'auth/openid/login.php'; - $html .= '
'.openid_form().'
'; + $html .= '
'.openid_form()->returnForm().'
'; } $pluginKeycloak = api_get_plugin_setting('keycloak', 'tool_enable') === 'true'; diff --git a/main/inc/local.inc.php b/main/inc/local.inc.php index e9a2acb226..9119fee0e9 100755 --- a/main/inc/local.inc.php +++ b/main/inc/local.inc.php @@ -971,13 +971,19 @@ if (!empty($_SESSION['_user']['user_id']) && !($login || $logout)) { $osso->logout(); //redirects and exits } } elseif (api_get_setting('openid_authentication') == 'true') { - if (!empty($_POST['openid_url'])) { - include api_get_path(SYS_CODE_PATH).'auth/openid/login.php'; - openid_begin(trim($_POST['openid_url']), api_get_path(WEB_PATH).'index.php'); - //this last function should trigger a redirect, so we can die here safely - exit('Openid login redirection should be in progress'); + include api_get_path(SYS_CODE_PATH).'auth/openid/login.php'; + $openidForm = openid_form(); + if ($openidForm->validate() && $openidForm->isSubmitted()) { + $openidUrl = $openidForm->exportValue('openid_url'); + + if (openid_is_allowed_provider($openidUrl)) { + openid_begin($openidUrl, api_get_path(WEB_PATH).'index.php'); + //this last function should trigger a redirect, so we can die here safely + exit('Openid login redirection should be in progress'); + } else { + $loginFailed = true; + } } elseif (!empty($_GET['openid_identity'])) { //it's usual for PHP to replace '.' (dot) by '_' (underscore) in URL parameters - include api_get_path(SYS_CODE_PATH).'auth/openid/login.php'; $res = openid_complete($_GET); if ($res['status'] == 'success') { $id1 = Database::escape_string($res['openid.identity']); diff --git a/main/install/configuration.dist.php b/main/install/configuration.dist.php index a2dd68ef01..5f1c6f7ea0 100644 --- a/main/install/configuration.dist.php +++ b/main/install/configuration.dist.php @@ -2260,6 +2260,12 @@ VALUES (21, 13, 'send_notification_at_a_specific_date', 'Send notification at a // Salt to use for admin ldap password decryption //$_configuration['ldap_admin_password_salt'] = 'salt'; +// Limit providers for OpenID (classic) authentication +/*$_configuration['auth_openid_allowed_providers'] = [ + 'example.com', + '*.example.com', +];*/ + // Option to hide the teachers info on courses about info page. //$_configuration['course_about_teacher_name_hide'] = false;