mirror of
https://github.com/BookStackApp/BookStack.git
synced 2024-11-23 03:12:32 +01:00
LDAP: Review, testing and update of LDAP TLS CA cert control
Review of #4913 Added testing to cover option. Updated option so it can be used for a CA directory, or a CA file. Updated option name to be somewhat abstracted from original underling PHP option. Tested against Jumpcloud. Testing took hours due to instability which was due to these settings sticking and being unstable on change until php process restart. Also due to little documentation for these options. X_TLS_CACERTDIR option needs cert files to be named via specific hashes which can be achieved via c_rehash utility. This also adds detail on STARTTLS failure, which took a long time to discover due to little detail out there for deeper PHP LDAP debugging.
This commit is contained in:
parent
18269f2c60
commit
8087123f2e
@ -219,7 +219,7 @@ LDAP_USER_FILTER=false
|
||||
LDAP_VERSION=false
|
||||
LDAP_START_TLS=false
|
||||
LDAP_TLS_INSECURE=false
|
||||
LDAP_TLS_CACERTFILE=false
|
||||
LDAP_TLS_CA_CERT=false
|
||||
LDAP_ID_ATTRIBUTE=uid
|
||||
LDAP_EMAIL_ATTRIBUTE=mail
|
||||
LDAP_DISPLAY_NAME_ATTRIBUTE=cn
|
||||
|
@ -209,10 +209,10 @@ class LdapService
|
||||
$this->ldap->setOption(null, LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER);
|
||||
}
|
||||
|
||||
// Specify CA Cert file for LDAP.
|
||||
// Configure any user-provided CA cert files for LDAP.
|
||||
// This option works globally and must be set before a connection is created.
|
||||
if ($this->config['tls_cacertfile']) {
|
||||
$this->ldap->setOption(null, LDAP_OPT_X_TLS_CACERTFILE, $this->config['tls_cacertfile']);
|
||||
if ($this->config['tls_ca_cert']) {
|
||||
$this->configureTlsCaCerts($this->config['tls_ca_cert']);
|
||||
}
|
||||
|
||||
$ldapHost = $this->parseServerString($this->config['server']);
|
||||
@ -229,7 +229,14 @@ class LdapService
|
||||
|
||||
// Start and verify TLS if it's enabled
|
||||
if ($this->config['start_tls']) {
|
||||
$started = $this->ldap->startTls($ldapConnection);
|
||||
try {
|
||||
$started = $this->ldap->startTls($ldapConnection);
|
||||
} catch (\Exception $exception) {
|
||||
$error = $exception->getMessage() . ' :: ' . ldap_error($ldapConnection);
|
||||
ldap_get_option($ldapConnection, LDAP_OPT_DIAGNOSTIC_MESSAGE, $detail);
|
||||
Log::info("LDAP STARTTLS failure: {$error} {$detail}");
|
||||
throw new LdapException('Could not start TLS connection. Further details in the application log.');
|
||||
}
|
||||
if (!$started) {
|
||||
throw new LdapException('Could not start TLS connection');
|
||||
}
|
||||
@ -240,6 +247,33 @@ class LdapService
|
||||
return $this->ldapConnection;
|
||||
}
|
||||
|
||||
/**
|
||||
* Configure TLS CA certs globally for ldap use.
|
||||
* This will detect if the given path is a directory or file, and set the relevant
|
||||
* LDAP TLS options appropriately otherwise throw an exception if no file/folder found.
|
||||
*
|
||||
* Note: When using a folder, certificates are expected to be correctly named by hash
|
||||
* which can be done via the c_rehash utility.
|
||||
*
|
||||
* @throws LdapException
|
||||
*/
|
||||
protected function configureTlsCaCerts(string $caCertPath): void
|
||||
{
|
||||
$errMessage = "Provided path [{$caCertPath}] for LDAP TLS CA certs could not be resolved to an existing location";
|
||||
$path = realpath($caCertPath);
|
||||
if ($path === false) {
|
||||
throw new LdapException($errMessage);
|
||||
}
|
||||
|
||||
if (is_dir($path)) {
|
||||
$this->ldap->setOption(null, LDAP_OPT_X_TLS_CACERTDIR, $path);
|
||||
} else if (is_file($path)) {
|
||||
$this->ldap->setOption(null, LDAP_OPT_X_TLS_CACERTFILE, $path);
|
||||
} else {
|
||||
throw new LdapException($errMessage);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse an LDAP server string and return the host suitable for a connection.
|
||||
* Is flexible to formats such as 'ldap.example.com:8069' or 'ldaps://ldap.example.com'.
|
||||
|
@ -133,7 +133,7 @@ return [
|
||||
'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'),
|
||||
'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS', false),
|
||||
'tls_insecure' => env('LDAP_TLS_INSECURE', false),
|
||||
'tls_cacertfile' => env('LDAP_TLS_CACERTFILE', false),
|
||||
'tls_ca_cert' => env('LDAP_TLS_CA_CERT', false),
|
||||
'start_tls' => env('LDAP_START_TLS', false),
|
||||
'thumbnail_attribute' => env('LDAP_THUMBNAIL_ATTRIBUTE', null),
|
||||
],
|
||||
|
@ -4,6 +4,7 @@ namespace Tests\Auth;
|
||||
|
||||
use BookStack\Access\Ldap;
|
||||
use BookStack\Access\LdapService;
|
||||
use BookStack\Exceptions\LdapException;
|
||||
use BookStack\Users\Models\Role;
|
||||
use BookStack\Users\Models\User;
|
||||
use Illuminate\Testing\TestResponse;
|
||||
@ -35,6 +36,7 @@ class LdapTest extends TestCase
|
||||
'services.ldap.user_filter' => '(&(uid=${user}))',
|
||||
'services.ldap.follow_referrals' => false,
|
||||
'services.ldap.tls_insecure' => false,
|
||||
'services.ldap.tls_ca_cert' => false,
|
||||
'services.ldap.thumbnail_attribute' => null,
|
||||
]);
|
||||
$this->mockLdap = $this->mock(Ldap::class);
|
||||
@ -767,4 +769,34 @@ EBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=')],
|
||||
$this->assertNotNull($user->avatar);
|
||||
$this->assertEquals('8c90748342f19b195b9c6b4eff742ded', md5_file(public_path($user->avatar->path)));
|
||||
}
|
||||
|
||||
public function test_tls_ca_cert_option_throws_if_set_to_invalid_location()
|
||||
{
|
||||
$path = 'non_found_' . time();
|
||||
config()->set(['services.ldap.tls_ca_cert' => $path]);
|
||||
|
||||
$this->commonLdapMocks(0, 0, 0, 0, 0);
|
||||
|
||||
$this->assertThrows(function () {
|
||||
$this->withoutExceptionHandling()->mockUserLogin();
|
||||
}, LdapException::class, "Provided path [{$path}] for LDAP TLS CA certs could not be resolved to an existing location");
|
||||
}
|
||||
|
||||
public function test_tls_ca_cert_option_used_if_set_to_a_folder()
|
||||
{
|
||||
$path = $this->files->testFilePath('');
|
||||
config()->set(['services.ldap.tls_ca_cert' => $path]);
|
||||
|
||||
$this->mockLdap->shouldReceive('setOption')->once()->with(null, LDAP_OPT_X_TLS_CACERTDIR, rtrim($path, '/'))->andReturn(true);
|
||||
$this->runFailedAuthLogin();
|
||||
}
|
||||
|
||||
public function test_tls_ca_cert_option_used_if_set_to_a_file()
|
||||
{
|
||||
$path = $this->files->testFilePath('test-file.txt');
|
||||
config()->set(['services.ldap.tls_ca_cert' => $path]);
|
||||
|
||||
$this->mockLdap->shouldReceive('setOption')->once()->with(null, LDAP_OPT_X_TLS_CACERTFILE, $path)->andReturn(true);
|
||||
$this->runFailedAuthLogin();
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user