Skip to content

Commit 43a9bd1

Browse files
authored
Security: Add 'auth_openid_allowed_providers' configuration setting to fix potential unauthenticated blind SSRF via openid.
1 parent df47eac commit 43a9bd1

File tree

4 files changed

+50
-9
lines changed

4 files changed

+50
-9
lines changed

main/auth/openid/login.php

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
require_once 'openid.lib.php';
1515
require_once 'xrds.lib.php';
1616

17-
function openid_form()
17+
function openid_form(): FormValidator
1818
{
1919
$form = new FormValidator(
2020
'openid_login',
@@ -25,8 +25,10 @@ function openid_form()
2525
);
2626
$form -> addElement('text', 'openid_url', array(get_lang('OpenIDURL'), Display::url(get_lang('OpenIDWhatIs'), 'main/auth/openid/whatis.php')), array('class' => 'openid_input'));
2727
$form -> addElement('button', 'submit', get_lang('Login'));
28+
$form->applyFilter('openid_url', 'trim');
29+
$form->protect();
2830

29-
return $form->returnForm();
31+
return $form;
3032
}
3133

3234
/**
@@ -459,3 +461,30 @@ function openid_http_request($url, $headers = array(), $method = 'GET', $data =
459461
$result->code = $code;
460462
return $result;
461463
}
464+
465+
function openid_is_allowed_provider($identityUrl): bool
466+
{
467+
$allowedProviders = api_get_configuration_value('auth_openid_allowed_providers');
468+
469+
if (false === $allowedProviders) {
470+
return true;
471+
}
472+
473+
$host = parse_url($identityUrl, PHP_URL_HOST) ?: $identityUrl;
474+
475+
foreach ($allowedProviders as $provider) {
476+
if (strpos($provider, '*') !== false) {
477+
$regex = '/^' . str_replace('\*', '.*', preg_quote($provider, '/')) . '$/';
478+
479+
if (preg_match($regex, $host)) {
480+
return true;
481+
}
482+
} else {
483+
if ($host === $provider) {
484+
return true;
485+
}
486+
}
487+
}
488+
489+
return false;
490+
}

main/inc/lib/template.lib.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1318,7 +1318,7 @@ public static function displayLoginForm()
13181318
$html = $form->returnForm();
13191319
if (api_get_setting('openid_authentication') == 'true') {
13201320
include_once api_get_path(SYS_CODE_PATH).'auth/openid/login.php';
1321-
$html .= '<div>'.openid_form().'</div>';
1321+
$html .= '<div>'.openid_form()->returnForm().'</div>';
13221322
}
13231323

13241324
$pluginKeycloak = api_get_plugin_setting('keycloak', 'tool_enable') === 'true';

main/inc/local.inc.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -971,13 +971,19 @@
971971
$osso->logout(); //redirects and exits
972972
}
973973
} elseif (api_get_setting('openid_authentication') == 'true') {
974-
if (!empty($_POST['openid_url'])) {
975-
include api_get_path(SYS_CODE_PATH).'auth/openid/login.php';
976-
openid_begin(trim($_POST['openid_url']), api_get_path(WEB_PATH).'index.php');
977-
//this last function should trigger a redirect, so we can die here safely
978-
exit('Openid login redirection should be in progress');
974+
include api_get_path(SYS_CODE_PATH).'auth/openid/login.php';
975+
$openidForm = openid_form();
976+
if ($openidForm->validate() && $openidForm->isSubmitted()) {
977+
$openidUrl = $openidForm->exportValue('openid_url');
978+
979+
if (openid_is_allowed_provider($openidUrl)) {
980+
openid_begin($openidUrl, api_get_path(WEB_PATH).'index.php');
981+
//this last function should trigger a redirect, so we can die here safely
982+
exit('Openid login redirection should be in progress');
983+
} else {
984+
$loginFailed = true;
985+
}
979986
} elseif (!empty($_GET['openid_identity'])) { //it's usual for PHP to replace '.' (dot) by '_' (underscore) in URL parameters
980-
include api_get_path(SYS_CODE_PATH).'auth/openid/login.php';
981987
$res = openid_complete($_GET);
982988
if ($res['status'] == 'success') {
983989
$id1 = Database::escape_string($res['openid.identity']);

main/install/configuration.dist.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2260,6 +2260,12 @@
22602260
// Salt to use for admin ldap password decryption
22612261
//$_configuration['ldap_admin_password_salt'] = 'salt';
22622262

2263+
// Limit providers for OpenID (classic) authentication
2264+
/*$_configuration['auth_openid_allowed_providers'] = [
2265+
'example.com',
2266+
'*.example.com',
2267+
];*/
2268+
22632269
// Option to hide the teachers info on courses about info page.
22642270
//$_configuration['course_about_teacher_name_hide'] = false;
22652271

0 commit comments

Comments
 (0)