Skip to content

Commit 20e86bf

Browse files
committed
Merge branch 'development' of github.com:BookStackApp/BookStack into development
2 parents f9e0873 + b072077 commit 20e86bf

File tree

4 files changed

+75
-1
lines changed

4 files changed

+75
-1
lines changed

.env.example.complete

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ LDAP_USER_FILTER="(&(uid={user}))"
219219
LDAP_VERSION=false
220220
LDAP_START_TLS=false
221221
LDAP_TLS_INSECURE=false
222+
LDAP_TLS_CA_CERT=false
222223
LDAP_ID_ATTRIBUTE=uid
223224
LDAP_EMAIL_ATTRIBUTE=mail
224225
LDAP_DISPLAY_NAME_ATTRIBUTE=cn

app/Access/LdapService.php

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,12 @@ protected function getConnection()
209209
$this->ldap->setOption(null, LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER);
210210
}
211211

212+
// Configure any user-provided CA cert files for LDAP.
213+
// This option works globally and must be set before a connection is created.
214+
if ($this->config['tls_ca_cert']) {
215+
$this->configureTlsCaCerts($this->config['tls_ca_cert']);
216+
}
217+
212218
$ldapHost = $this->parseServerString($this->config['server']);
213219
$ldapConnection = $this->ldap->connect($ldapHost);
214220

@@ -223,7 +229,14 @@ protected function getConnection()
223229

224230
// Start and verify TLS if it's enabled
225231
if ($this->config['start_tls']) {
226-
$started = $this->ldap->startTls($ldapConnection);
232+
try {
233+
$started = $this->ldap->startTls($ldapConnection);
234+
} catch (\Exception $exception) {
235+
$error = $exception->getMessage() . ' :: ' . ldap_error($ldapConnection);
236+
ldap_get_option($ldapConnection, LDAP_OPT_DIAGNOSTIC_MESSAGE, $detail);
237+
Log::info("LDAP STARTTLS failure: {$error} {$detail}");
238+
throw new LdapException('Could not start TLS connection. Further details in the application log.');
239+
}
227240
if (!$started) {
228241
throw new LdapException('Could not start TLS connection');
229242
}
@@ -234,6 +247,33 @@ protected function getConnection()
234247
return $this->ldapConnection;
235248
}
236249

250+
/**
251+
* Configure TLS CA certs globally for ldap use.
252+
* This will detect if the given path is a directory or file, and set the relevant
253+
* LDAP TLS options appropriately otherwise throw an exception if no file/folder found.
254+
*
255+
* Note: When using a folder, certificates are expected to be correctly named by hash
256+
* which can be done via the c_rehash utility.
257+
*
258+
* @throws LdapException
259+
*/
260+
protected function configureTlsCaCerts(string $caCertPath): void
261+
{
262+
$errMessage = "Provided path [{$caCertPath}] for LDAP TLS CA certs could not be resolved to an existing location";
263+
$path = realpath($caCertPath);
264+
if ($path === false) {
265+
throw new LdapException($errMessage);
266+
}
267+
268+
if (is_dir($path)) {
269+
$this->ldap->setOption(null, LDAP_OPT_X_TLS_CACERTDIR, $path);
270+
} else if (is_file($path)) {
271+
$this->ldap->setOption(null, LDAP_OPT_X_TLS_CACERTFILE, $path);
272+
} else {
273+
throw new LdapException($errMessage);
274+
}
275+
}
276+
237277
/**
238278
* Parse an LDAP server string and return the host suitable for a connection.
239279
* Is flexible to formats such as 'ldap.example.com:8069' or 'ldaps://ldap.example.com'.

app/Config/services.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'),
134134
'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS', false),
135135
'tls_insecure' => env('LDAP_TLS_INSECURE', false),
136+
'tls_ca_cert' => env('LDAP_TLS_CA_CERT', false),
136137
'start_tls' => env('LDAP_START_TLS', false),
137138
'thumbnail_attribute' => env('LDAP_THUMBNAIL_ATTRIBUTE', null),
138139
],

tests/Auth/LdapTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use BookStack\Access\Ldap;
66
use BookStack\Access\LdapService;
7+
use BookStack\Exceptions\LdapException;
78
use BookStack\Users\Models\Role;
89
use BookStack\Users\Models\User;
910
use Illuminate\Testing\TestResponse;
@@ -35,6 +36,7 @@ protected function setUp(): void
3536
'services.ldap.user_filter' => '(&(uid={user}))',
3637
'services.ldap.follow_referrals' => false,
3738
'services.ldap.tls_insecure' => false,
39+
'services.ldap.tls_ca_cert' => false,
3840
'services.ldap.thumbnail_attribute' => null,
3941
]);
4042
$this->mockLdap = $this->mock(Ldap::class);
@@ -799,4 +801,34 @@ public function test_thumbnail_attribute_used_as_user_avatar_if_configured()
799801
$this->assertNotNull($user->avatar);
800802
$this->assertEquals('8c90748342f19b195b9c6b4eff742ded', md5_file(public_path($user->avatar->path)));
801803
}
804+
805+
public function test_tls_ca_cert_option_throws_if_set_to_invalid_location()
806+
{
807+
$path = 'non_found_' . time();
808+
config()->set(['services.ldap.tls_ca_cert' => $path]);
809+
810+
$this->commonLdapMocks(0, 0, 0, 0, 0);
811+
812+
$this->assertThrows(function () {
813+
$this->withoutExceptionHandling()->mockUserLogin();
814+
}, LdapException::class, "Provided path [{$path}] for LDAP TLS CA certs could not be resolved to an existing location");
815+
}
816+
817+
public function test_tls_ca_cert_option_used_if_set_to_a_folder()
818+
{
819+
$path = $this->files->testFilePath('');
820+
config()->set(['services.ldap.tls_ca_cert' => $path]);
821+
822+
$this->mockLdap->shouldReceive('setOption')->once()->with(null, LDAP_OPT_X_TLS_CACERTDIR, rtrim($path, '/'))->andReturn(true);
823+
$this->runFailedAuthLogin();
824+
}
825+
826+
public function test_tls_ca_cert_option_used_if_set_to_a_file()
827+
{
828+
$path = $this->files->testFilePath('test-file.txt');
829+
config()->set(['services.ldap.tls_ca_cert' => $path]);
830+
831+
$this->mockLdap->shouldReceive('setOption')->once()->with(null, LDAP_OPT_X_TLS_CACERTFILE, $path)->andReturn(true);
832+
$this->runFailedAuthLogin();
833+
}
802834
}

0 commit comments

Comments
 (0)