Skip to content

Commit 5ebb39b

Browse files
tatankattatankat
authored
[4.x] Fix LDAP over SSL (#37962)
* [4.x] Fix LDAP over SSL * More precise WHERE clause and a bit code style. * Select correct encryption type with uri ldaps:// and fix element * Phase 1 convert BRANCH to PSR-12 * Phase 2 convert BRANCH to PSR-12 * remove leftover codestyle error * Use more correct namings, based on Thunderbird * rename DB updates for newer Joomla version * Renamed SQL files to update on latest release * Enable the test fixed by this PR * added certificate configuration options * Some cleaning ---------
1 parent e91b58a commit 5ebb39b

File tree

6 files changed

+146
-70
lines changed

6 files changed

+146
-70
lines changed

‎administrator/components/com_admin/sql/updates/mysql/4.3.0-2023-01-30.sql‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
UPDATE `#__extensions`
2+
SET `params` = REPLACE(`params`, '"negotiate_tls":1', '"encryption":"tls"')
3+
WHERE `name` = 'plg_authentication_ldap'
4+
AND `type` = 'plugin'
5+
AND `element` = 'ldap'
6+
AND `folder` = 'authentication'
7+
AND `client_id` = 0
8+
AND `params` LIKE '{%"negotiate_tls":1%}';
9+
10+
UPDATE `#__extensions`
11+
SET `params` = REPLACE(`params`, '"negotiate_tls":0', '"encryption":"none"')
12+
WHERE `name` = 'plg_authentication_ldap'
13+
AND `type` = 'plugin'
14+
AND `element` = 'ldap'
15+
AND `folder` = 'authentication'
16+
AND `client_id` = 0
17+
AND `params` LIKE '{%"negotiate_tls":0%}';
18+
19+
UPDATE `#__extensions`
20+
SET `params` = REPLACE(`params`, '"encryption":"none"', '"encryption":"ssl"')
21+
WHERE `name` = 'plg_authentication_ldap'
22+
AND `type` = 'plugin'
23+
AND `element` = 'ldap'
24+
AND `folder` = 'authentication'
25+
AND `client_id` = 0
26+
AND `params` LIKE '{%"host":"ldaps:\\\\/\\\\/%}';
27+
28+
UPDATE `#__extensions`
29+
SET `params` = REPLACE(`params`, '"host":"ldaps:\\/\\/', '"host":"')
30+
WHERE `name` = 'plg_authentication_ldap'
31+
AND `type` = 'plugin'
32+
AND `element` = 'ldap'
33+
AND `folder` = 'authentication'
34+
AND `client_id` = 0
35+
AND `params` LIKE '{%"host":"ldaps:\\\\/\\\\/%}';
36+

‎administrator/components/com_admin/sql/updates/postgresql/4.3.0-2023-01-30.sql‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
UPDATE "#__extensions"
2+
SET "params" = REPLACE("params", '"negotiate_tls":1', '"encryption":"tls"')
3+
WHERE "name" = 'plg_authentication_ldap'
4+
AND "type" = 'plugin'
5+
AND "element" = 'ldap'
6+
AND "folder" = 'authentication'
7+
AND "client_id" = 0
8+
AND "params" LIKE '{%"negotiate_tls":1%}';
9+
10+
UPDATE "#__extensions"
11+
SET "params" = REPLACE("params", '"negotiate_tls":0', '"encryption":"none"')
12+
WHERE "name" = 'plg_authentication_ldap'
13+
AND "type" = 'plugin'
14+
AND "element" = 'ldap'
15+
AND "folder" = 'authentication'
16+
AND "client_id" = 0
17+
AND "params" LIKE '{%"negotiate_tls":0%}';
18+
19+
UPDATE "#__extensions"
20+
SET "params" = REPLACE("params", '"encryption":"none"', '"encryption":"ssl"')
21+
WHERE "name" = 'plg_authentication_ldap'
22+
AND "type" = 'plugin'
23+
AND "element" = 'ldap'
24+
AND "folder" = 'authentication'
25+
AND "client_id" = 0
26+
AND "params" LIKE '{%"host":"ldaps:\\/\\/%}';
27+
28+
UPDATE "#__extensions"
29+
SET "params" = REPLACE("params", '"host":"ldaps:\/\/', '"host":"')
30+
WHERE "name" = 'plg_authentication_ldap'
31+
AND "type" = 'plugin'
32+
AND "element" = 'ldap'
33+
AND "folder" = 'authentication'
34+
AND "client_id" = 0
35+
AND "params" LIKE '{%"host":"ldaps:\\/\\/%}';
36+

‎administrator/language/en-GB/plg_authentication_ldap.ini‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@
66
PLG_AUTHENTICATION_LDAP="Authentication - LDAP"
77
PLG_LDAP_FIELD_AUTHMETHOD_LABEL="Authorisation Method"
88
PLG_LDAP_FIELD_BASEDN_LABEL="Base DN"
9+
PLG_LDAP_FIELD_CACERT_LABEL="Path to the server certificate"
10+
PLG_LDAP_FIELD_CACERT_DESC="Full path to the file or directory where Joomla can find the LDAP server's (Certificate Authority) certificate. If this is empty, the system defaults for the TLS_CACERT and TLS_CACERTDIR LDAP client options are used."
11+
PLG_LDAP_FIELD_ENCRYPTION_LABEL="Connection Security"
912
PLG_LDAP_FIELD_EMAIL_DESC="LDAP attribute which has the User's email address."
1013
PLG_LDAP_FIELD_EMAIL_LABEL="Map: Email"
1114
PLG_LDAP_FIELD_FULLNAME_DESC="LDAP attribute which has the User's full name."
1215
PLG_LDAP_FIELD_FULLNAME_LABEL="Map: Full Name"
1316
PLG_LDAP_FIELD_HOST_LABEL="Host"
17+
PLG_LDAP_FIELD_IGNORE_REQCERT_TLS_LABEL="Ignore Certificate"
1418
PLG_LDAP_FIELD_LDAPDEBUG_DESC="Enables debug hardcoded to level 7"
1519
PLG_LDAP_FIELD_LDAPDEBUG_LABEL="Debug"
16-
PLG_LDAP_FIELD_NEGOCIATE_LABEL="Negotiate TLS"
1720
PLG_LDAP_FIELD_PASSWORD_DESC="The Connect Password is the password of an administrative account."
1821
PLG_LDAP_FIELD_PASSWORD_LABEL="Connect Password"
1922
PLG_LDAP_FIELD_PORT_LABEL="Port"
@@ -31,4 +34,7 @@ PLG_LDAP_FIELD_V3_DESC="Default is LDAP2, but the latest versions of OpenLdap re
3134
PLG_LDAP_FIELD_V3_LABEL="LDAP V3"
3235
PLG_LDAP_FIELD_VALUE_BINDSEARCH="Bind and Search"
3336
PLG_LDAP_FIELD_VALUE_BINDUSER="Bind Directly as User"
37+
PLG_LDAP_FIELD_VALUE_ENCRYPTIONNONE="None"
38+
PLG_LDAP_FIELD_VALUE_ENCRYPTIONSSL="SSL/TLS"
39+
PLG_LDAP_FIELD_VALUE_ENCRYPTIONTLS="STARTTLS"
3440
PLG_LDAP_XML_DESCRIPTION="<p>Handles User Authentication against an LDAP server.</p><p><strong>Warning! You must have at least one authentication plugin enabled or you will lose all access to your site.</strong></p>"

‎plugins/authentication/ldap/ldap.xml‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,25 +45,47 @@
4545
type="radio"
4646
layout="joomla.form.field.radio.switcher"
4747
label="PLG_LDAP_FIELD_V3_LABEL"
48-
default="0"
48+
default="1"
4949
filter="integer"
5050
>
5151
<option value="0">JNO</option>
5252
<option value="1">JYES</option>
5353
</field>
5454

5555
<field
56-
name="negotiate_tls"
56+
name="encryption"
57+
type="list"
58+
label="PLG_LDAP_FIELD_ENCRYPTION_LABEL"
59+
default="none"
60+
validate="options"
61+
>
62+
<option value="none">PLG_LDAP_FIELD_VALUE_ENCRYPTIONNONE</option>
63+
<option value="tls">PLG_LDAP_FIELD_VALUE_ENCRYPTIONTLS</option>
64+
<option value="ssl">PLG_LDAP_FIELD_VALUE_ENCRYPTIONSSL</option>
65+
</field>
66+
67+
<field
68+
name="ignore_reqcert_tls"
5769
type="radio"
58-
label="PLG_LDAP_FIELD_NEGOCIATE_LABEL"
59-
default="0"
60-
filter="integer"
70+
label="PLG_LDAP_FIELD_IGNORE_REQCERT_TLS_LABEL"
6171
layout="joomla.form.field.radio.switcher"
72+
default="1"
73+
filter="boolean"
74+
showon="encryption!:none"
6275
>
6376
<option value="0">JNO</option>
6477
<option value="1">JYES</option>
6578
</field>
6679

80+
<field
81+
name="cacert"
82+
type="text"
83+
label="PLG_LDAP_FIELD_CACERT_LABEL"
84+
description="PLG_LDAP_FIELD_CACERT_DESC"
85+
required="false"
86+
showon="encryption!:none[AND]ignore_reqcert_tls:0"
87+
/>
88+
6789
<field
6890
name="no_referrals"
6991
type="radio"

‎plugins/authentication/ldap/src/Extension/Ldap.php‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,43 @@ public function onUserAuthenticate($credentials, $options, &$response)
9595
$ldap_fullname = $this->params->get('ldap_fullname', '');
9696
$ldap_uid = $this->params->get('ldap_uid', '');
9797
$auth_method = $this->params->get('auth_method', '');
98+
// Load certificate info
99+
$ignore_reqcert_tls = (bool) $this->params->get('ignore_reqcert_tls', '1');
100+
$cacert = $this->params->get('cacert', '');
101+
102+
// getting certificate file and certificate directory options (both need to be set)
103+
if (!$ignore_reqcert_tls && !empty($cacert)) {
104+
if (is_dir($cacert)) {
105+
$cacertdir = $cacert;
106+
$cacertfile = "";
107+
} elseif (is_file($cacert)) {
108+
$cacertfile = $cacert;
109+
$cacertdir = dirname($cacert);
110+
} else {
111+
$cacertfile = $cacert;
112+
$cacertdir = $cacert;
113+
Log::add(sprintf('Certificate path for LDAP client is neither an existing file nor directory: "%s"', $cacert), Log::ERROR, $logcategory);
114+
}
115+
} else {
116+
Log::add(sprintf('Not setting any LDAP TLS CA certificate options because %s, system wide settings are used', $ignore_reqcert_tls ? "certificate is ignored" : "no certificate location is configured"), Log::DEBUG, $logcategory);
117+
}
98118

99119
$options = [
100120
'host' => $this->params->get('host', ''),
101121
'port' => (int) $this->params->get('port', ''),
102-
'version' => $this->params->get('use_ldapV3', '0') == '1' ? 3 : 2,
122+
'version' => $this->params->get('use_ldapV3', '1') == '1' ? 3 : 2,
103123
'referrals' => (bool) $this->params->get('no_referrals', '0'),
104-
'encryption' => $this->params->get('negotiate_tls', '0') == '1' ? 'tls' : 'none',
124+
'encryption' => $this->params->get('encryption', 'none'),
125+
'options' => [
126+
'x_tls_require_cert' => $ignore_reqcert_tls ? LDAP_OPT_X_TLS_NEVER : LDAP_OPT_X_TLS_DEMAND,
127+
],
105128
];
129+
// if these are not set, the system defaults are used
130+
if (isset($cacertdir) && isset($cacertfile)) {
131+
$options['options']['x_tls_cacertdir'] = $cacertdir;
132+
$options['options']['x_tls_cacertfile'] = $cacertfile;
133+
}
134+
106135
Log::add(sprintf('Creating LDAP session with options: %s', json_encode($options)), Log::DEBUG, $logcategory);
107136
$connection_string = sprintf('ldap%s://%s:%s', 'ssl' === $options['encryption'] ? 's' : '', $options['host'], $options['port']);
108137
Log::add(sprintf('Creating LDAP session to connect to "%s" while binding', $connection_string), Log::DEBUG, $logcategory);

‎tests/Integration/Plugin/Authentication/Ldap/LdapPluginTest.php‎

Lines changed: 9 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@
2323
/**
2424
* Test class for Ldap plugin
2525
*
26+
* Not testing for:
27+
* * different certificate options
28+
* these can't be properly automatically tested as the LDAP_OPT_X_ settings can only be set once in a running process
29+
* * working ldap debug option.
30+
* this can only be tested if phpunit stderr is redirected/duplicated/configured to a file
31+
*
2632
* @package Joomla.IntegrationTest
2733
* @subpackage Ldap
2834
*
@@ -72,38 +78,6 @@ private function getPlugin($options): LdapPlugin
7278
return $plugin;
7379
}
7480

75-
private function acceptCertificates(): void
76-
{
77-
//TODO make this (and LDAP_OPT_X_CERTFILE and LDAP_OPT_X_TLS_REQUIRE_CERT) Joomla ldap setting
78-
$cert = JPATH_ROOT . '/' . JTEST_LDAP_CACERTFILE;
79-
ldap_set_option(null, LDAP_OPT_X_TLS_CACERTDIR, dirname($cert));
80-
ldap_set_option(null, LDAP_OPT_X_TLS_CACERTFILE, $cert);
81-
}
82-
83-
private function getAdminConnection(array $options): Ldap
84-
{
85-
$admin_options = [
86-
'host' => $options['host'],
87-
'port' => (int) $options['port'],
88-
'version' => $options['use_ldapV3'] == '1' ? 3 : 2,
89-
'referrals' => (bool) $options['no_referrals'],
90-
'encryption' => $options['encryption'],
91-
'debug' => (bool) $options['ldap_debug'],
92-
];
93-
$ldap = Ldap::create(
94-
'ext_ldap',
95-
$admin_options
96-
);
97-
$ldap->bind("cn=admin,cn=config", "configpassword");
98-
return $ldap;
99-
}
100-
101-
private function requireEncryption($encryption, $options): void
102-
{
103-
//$ldap = $this->getAdminConnection($options);
104-
//TODO configure openldap (only if given permission in phpunit.xml, so people can use their own ldap server) to require the requested encryption to be sure encryption is used
105-
}
106-
10781
private function skipIfAskedFor($options): void
10882
{
10983
if (empty($options["host"])) {
@@ -136,6 +110,9 @@ public function setUp(): void
136110
'ldap_email' => JTEST_LDAP_EMAIL,
137111
'ldap_uid' => JTEST_LDAP_UID,
138112
'ldap_debug' => 0,
113+
/* the security options can only be set once, these are the best practice settings */
114+
'ignore_reqcert_tls' => 0,
115+
'cacert' => JPATH_ROOT . '/' . JTEST_LDAP_CACERTFILE,
139116
/* changing options to test all code */
140117
'port' => self::LDAPPORT,
141118
'encryption' => "none",
@@ -235,9 +212,6 @@ public function testOnUserAuthenticateBindAndSearchTLS()
235212
$this->skipIfAskedFor($options);
236213
$plugin = $this->getPlugin($options);
237214

238-
$this->acceptCertificates();
239-
$this->requireEncryption("tls", $options);
240-
241215
$response = new AuthenticationResponse();
242216
$plugin->onUserAuthenticate($this->default_credentials, [], $response);
243217
$this->assertEquals(Authentication::STATUS_SUCCESS, $response->status);
@@ -252,42 +226,15 @@ public function testOnUserAuthenticateBindAndSearchTLS()
252226
*/
253227
public function testOnUserAuthenticateBindAndSearchSSL()
254228
{
255-
$this->markTestSkipped("Fix provided in PR #37962");
256-
257229
$options = $this->default_options;
258230
$options["auth_method"] = "search";
259231
$options["encryption"] = "ssl";
260232
$options["port"] = self::SSLPORT;
261233
$this->skipIfAskedFor($options);
262234
$plugin = $this->getPlugin($options);
263235

264-
$this->acceptCertificates();
265-
$this->requireEncryption("ssl", $options);
266-
267-
$response = new AuthenticationResponse();
268-
$plugin->onUserAuthenticate($this->default_credentials, [], $response);
269-
$this->assertEquals(Authentication::STATUS_SUCCESS, $response->status);
270-
}
271-
272-
/**
273-
* @testdox does log ldap client calls and errors
274-
* can only be tested if phpunit stderr is redirected/duplicated/configured to a file
275-
* then, we can check if ldap_ calls are present in that file
276-
*
277-
* @return void
278-
*
279-
* @since 4.3.0
280-
*/
281-
/*
282-
public function testOnUserAuthenticateWithDebug()
283-
{
284-
$options = $this->default_options;
285-
$options["ldap_debug"] = 1;
286-
$plugin = $this->getPlugin($options);
287-
288236
$response = new AuthenticationResponse();
289237
$plugin->onUserAuthenticate($this->default_credentials, [], $response);
290238
$this->assertEquals(Authentication::STATUS_SUCCESS, $response->status);
291239
}
292-
*/
293240
}

0 commit comments

Comments
 (0)