Skip to content
Snippets Groups Projects
Commit f0da3722 authored by Conrad Lara's avatar Conrad Lara
Browse files

Security Fix: Authentication bypass when default plugin is not configured.

parent f9d69386
No related branches found
No related tags found
No related merge requests found
......@@ -98,6 +98,27 @@ Now you should be ready to configure the TFA module.
* Visit your account's TFA tab: `user/[uid]/security/tfa`
* Configure the selected Validation Plugins as desired for your account.
##### TFA Security Advisory Related Configuration
###### Configured Plugins Enforce TFA Requiered
Prior to 8.x-1.4 the TFA module would only check if a user had configured the
current default validation plugin to determine if TFA was required for login.
This could lead to TFA not being required for users if a site administrator
changed the default plugin to one a user had not yet configured.
In order to mitigate this risk TFA now must validate that no plugins indicate
a provisioned status, this includes disabled plugins. This can create a
situation where a user is unable to log in due to having a disabled plugin
provisioned without any enabled plugins provisioned.
If an adminsitrator is unable to re-enable a disabled plugin that prevents
authentciation, or belives that this restriction is not neccessary for their
site they may limit the validation to only test enabled plugins by adding the
following into the sites settings.php file.
`$settings['tfa.only_restrict_with_enabled_plugins'] = TRUE;`
##### TFA apps
Users will need an application for their phone, tablet or computer that is
capable of generating authentication codes. As of the date of this ReadMe,
......
......@@ -13,6 +13,7 @@ use Drupal\user\UserDataInterface;
use Drupal\user\Entity\User;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
/**
......@@ -184,6 +185,31 @@ class EntryForm extends FormBase {
// Get current validation plugin form.
$this->tfaValidationPlugin = $this->tfaValidationManager->createInstance($validation_plugin, ['uid' => $uid]);
// If the current plugin isn't ready we need to find another plugin.
if (!$this->tfaValidationPlugin->ready()) {
// Find a new plugin.
$enabled_plugins = $this->config('tfa.settings')->get('allowed_validation_plugins');
foreach ($enabled_plugins as $plugin_name) {
if (!empty($user_enabled_validation_plugins[$plugin_name]) && $plugin_name != $validation_plugin) {
/** @var \Drupal\tfa\Plugin\TfaValidationInterface $plugin */
$plugin = $this->tfaValidationManager->createInstance($plugin_name, ['uid' => $uid]);
if ($plugin->ready()) {
$validation_plugin = $plugin_name;
$this->tfaValidationPlugin = $plugin;
break;
}
}
}
if (!$this->tfaValidationPlugin->ready()) {
// This should never happen. The EntryForm should never be rendered if
// no plugins are ready.
$message = $this->t('An unexpected error occurred attempting to display the TFA Entry form.');
$this->messenger()->addError($message);
return new RedirectResponse(Url::fromRoute('user.login')->toString());
}
}
$form = $this->tfaValidationPlugin->getForm($form, $form_state);
$this->tfaLoginPlugins = $this->tfaLoginManager->getPlugins(['uid' => $uid]);
......
......@@ -3,6 +3,7 @@
namespace Drupal\tfa;
use Drupal\Component\Plugin\Exception\PluginException;
use Drupal\Core\Site\Settings;
use Drupal\user\UserInterface;
use Psr\Log\LoggerInterface;
use Drupal\Core\Url;
......@@ -129,7 +130,26 @@ trait TfaLoginContextTrait {
}
}
catch (PluginException $e) {
return FALSE;
// No error as we want to try other plugins.
}
}
// Check alternate plugins.
$enabled_plugins = $this->tfaSettings->get('allowed_validation_plugins');
foreach ($enabled_plugins as $plugin_name) {
if ($plugin_name == $default_validation_plugin) {
// We checked the default plugin already.
continue;
}
try {
/** @var \Drupal\tfa\Plugin\TfaValidationInterface $validation_plugin */
$validation_plugin = $this->tfaValidationManager->createInstance($plugin_name, ['uid' => $this->user->id()]);
if ($validation_plugin->ready()) {
return TRUE;
}
}
catch (PluginException $e) {
// No error as we want to try other plugins.
}
}
......@@ -218,10 +238,35 @@ trait TfaLoginContextTrait {
* otherwise return false.
*/
public function canLoginWithoutTfa(LoggerInterface $logger) {
$user = $this->getUser();
// Users that have configured a TFA method should not be allowed to skip.
$user_settings = $this->userData->get('tfa', $user->id(), 'tfa_user_settings');
$user_enabled_validation_plugins = $user_settings['data']['plugins'] ?? [];
$enabled_validation_plugins = $this->tfaSettings->get('allowed_validation_plugins');
$enabled_validation_plugins = is_array($enabled_validation_plugins) ? $enabled_validation_plugins : [];
foreach ($this->tfaValidationManager->getDefinitions() as $plugin_id => $definition) {
if (
Settings::get('tfa.only_restrict_with_enabled_plugins', FALSE) === TRUE
&& !array_key_exists($plugin_id, $enabled_validation_plugins)
) {
// Skip checking admin disabled plugins.
continue;
}
if (array_key_exists($plugin_id, $user_enabled_validation_plugins)) {
/** @var \Drupal\tfa\Plugin\TfaValidationInterface $plugin */
$plugin = $this->tfaValidationManager->createInstance($plugin_id, ['uid' => $user->id()]);
if ($plugin->ready()) {
$message = $this->config('tfa.settings')->get('help_text');
$this->messenger()->addError($message);
$logger->notice('@name has attempted login without using a configured second factor authentication plugin.', ['@name' => $user->getAccountName()]);
return FALSE;
}
}
}
// User may be able to skip TFA, depending on module settings and number of
// prior attempts.
$remaining = $this->remainingSkips();
$user = $this->getUser();
if ($remaining) {
$tfa_setup_link = Url::fromRoute('tfa.overview', [
'user' => $user->id(),
......
<?php
namespace Drupal\tfa_test_plugins\Plugin\TfaValidation;
use Drupal\Core\Form\FormStateInterface;
use Drupal\tfa\Plugin\TfaBasePlugin;
use Drupal\tfa\Plugin\TfaValidationInterface;
/**
* TFA Test Validation Plugin - FALSE.
*
* Provides a plugin that will return FALSE when interrogated.
*
* @package Drupal\tfa_test_plugins
*
* @TfaValidation(
* id = "tfa_test_plugins_validation_false",
* label = @Translation("TFA Test Validation Plugin - FALSE Response"),
* description = @Translation("TFA Test Validation Plugin - FALSE Response"),
* )
*/
class TfaTestValidationFalsePlugin extends TfaBasePlugin implements TfaValidationInterface {
/**
* {@inheritdoc}
*/
public function getForm(array $form, FormStateInterface $form_state) {
return $form;
}
/**
* {@inheritdoc}
*/
public function validateForm(array $form, FormStateInterface $form_state) {
return FALSE;
}
/**
* {@inheritdoc}
*/
public function ready() {
return FALSE;
}
}
......@@ -2,12 +2,15 @@
namespace Drupal\Tests\tfa\Functional;
use Drupal\tfa\TfaUserDataTrait;
/**
* Tests for the tfa login process.
*
* @group Tfa
*/
class TfaLoginTest extends TfaTestBase {
use TfaUserDataTrait;
/**
* User doing the TFA Validation.
......@@ -135,4 +138,41 @@ class TfaLoginTest extends TfaTestBase {
$assert_session->pageTextContains('TFA Setup for ' . $another_user->getDisplayName());
}
/**
* Tests login when the user has the Default plugin disabled.
*/
public function testDefaultPluginDisabled() {
$test_user = $this->createUser();
$settings = $this->config('tfa.settings');
$settings->set('enabled', TRUE);
$enabled_plugins = [
'tfa_test_plugins_validation' => 'tfa_test_plugins_validation',
'tfa_test_plugins_validation_false' => 'tfa_test_plugins_validation_false',
];
$settings->set('allowed_validation_plugins', $enabled_plugins);
$settings->set('default_validation_plugin', 'tfa_test_plugins_validation_false');
$settings->save();
/** @var \Drupal\user\UserDataInterface $user_data_service */
$user_data_service = $this->container->get('user.data');
// This will be the users 'configured and ready' plugin, it is however
// not the 'default' plugin.
$this->tfaSaveTfaData($test_user->id(), $user_data_service, ['plugins' => 'tfa_test_plugins_validation']);
// This will be an unknown/invalid/uninstalled plugin to ensure
// that no exceptions occur on unknown plugins.
$this->tfaSaveTfaData($test_user->id(), $user_data_service, ['plugins' => 'tfa_plugin_does_not_exist']);
$this->drupalLogout();
$edit = [
'name' => $test_user->getAccountName(),
'pass' => $test_user->passRaw,
];
$this->drupalGet('user/login');
$this->submitForm($edit, 'Log in');
$assert_session = $this->assertSession();
$assert_session->statusCodeEquals(200);
$this->assertNotEmpty($this->getSessionCookies());
$this->matchesRegularExpression('/.*\/user\/' . $test_user->id() . '.*/', $this->getSession()->getCurrentUrl());
}
}
......@@ -218,6 +218,7 @@ class TfaContextTest extends UnitTestCase {
// Not ready.
$settings = $this->prophesize(ImmutableConfig::class);
$settings->get('default_validation_plugin')->willReturn(FALSE);
$settings->get('allowed_validation_plugins')->willReturn([]);
$config_factory = $this->prophesize(ConfigFactoryInterface::class);
$config_factory->get('tfa.settings')->willReturn($settings->reveal());
$this->configFactory = $config_factory->reveal();
......@@ -226,6 +227,7 @@ class TfaContextTest extends UnitTestCase {
// Is ready.
$settings->get('default_validation_plugin')->willReturn('foo');
$settings->get('allowed_validation_plugins')->willReturn(['foo' => 'foo']);
$config_factory = $this->prophesize(ConfigFactoryInterface::class);
$config_factory->get('tfa.settings')->willReturn($settings->reveal());
$this->configFactory = $config_factory->reveal();
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment