diff --git a/.env.example.complete b/.env.example.complete index 3f1a8ab94..cfc968192 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -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 diff --git a/app/Access/LdapService.php b/app/Access/LdapService.php index 56e7aba04..e0d7f2615 100644 --- a/app/Access/LdapService.php +++ b/app/Access/LdapService.php @@ -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'. diff --git a/app/Config/services.php b/app/Config/services.php index a407b5dc8..f93ca44f6 100644 --- a/app/Config/services.php +++ b/app/Config/services.php @@ -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), ], diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 34900ce6f..717167729 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -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(); + } }