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.

Author: ssddanbrown

.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

app/Access/LdapService.php

@@ -203,16 +203,18 @@ protected function getConnection()
  throw new LdapException(trans('errors.ldap_extension_not_installed'));
  }
 
+ $this->ldap->setOption(null, LDAP_OPT_DEBUG_LEVEL, 7);
+
  // Disable certificate verification.
  // This option works globally and must be set before a connection is created.
  if ($this->config['tls_insecure']) {
  $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 +231,14 @@ protected function getConnection()
 
  // 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 +249,33 @@ protected function getConnection()
  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'.

app/Config/services.php

@@ -133,7 +133,7 @@
  '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),
  ],

tests/Auth/LdapTest.php

@@ -4,6 +4,7 @@
 
 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 @@ protected function setUp(): void
  '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 @@ public function test_thumbnail_attribute_used_as_user_avatar_if_configured()
  $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();
+ }
 }