View Issue Details

IDProjectCategoryView StatusLast Update
0015361mantisbtldappublic2022-04-18 20:17
Reporterillmnec Assigned Tocommunity  
PrioritynormalSeverityfeatureReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.12 
Target Version2.25.0Fixed in Version2.25.0 
Summary0015361: Add STARTTLS Support to LDAP
Description

It would be great if Mantis supported STARTTLS as this is the officially supported method for encrypted LDAP connections (LDAPS is being deprecated).

This can be done very simply by adding
ldap_start_tls($t_ds);
somwhere after the connection has been made (line 530 in core/ldap_api.php of version 1.2.12). This will silently enable the TLS connection, if the LDAP server accepts such a connection.

Obviously, the proper method would require adding a new variable to the configuraton ($g_ldap_starttls) and then checking if the connection is really encrypted before continuing.

Tagspatch
Attached Files
mantis_patch_ldap_starttls.patch (2,079 bytes)   
diff -rupN mantisbt-1.2.17/config_defaults_inc.php mantisbt-1.2.17-ldap-starttls/config_defaults_inc.php
--- mantisbt-1.2.17/config_defaults_inc.php	2014-03-03 14:19:50.000000000 -0500
+++ mantisbt-1.2.17-ldap-starttls/config_defaults_inc.php	2014-09-18 14:37:37.000000000 -0400
@@ -1765,6 +1765,13 @@
 	$g_ldap_bind_passwd		= '';
 
 	/**
+	 * Should the connection use STARTTLS (use ldap:// url for server address)
+	 *
+	 * @global string $g_ldap_starttls
+	 */
+	$g_ldap_starttls		= FALSE;
+
+	/**
 	 * Should we send to the LDAP email address or what MySql tells us
 	 * @global int $g_use_ldap_email
 	 */
diff -rupN mantisbt-1.2.17/core/constant_inc.php mantisbt-1.2.17-ldap-starttls/core/constant_inc.php
--- mantisbt-1.2.17/core/constant_inc.php	2014-03-03 14:19:50.000000000 -0500
+++ mantisbt-1.2.17-ldap-starttls/core/constant_inc.php	2014-09-18 14:37:37.000000000 -0400
@@ -312,6 +312,7 @@ define( 'ERROR_LDAP_SERVER_CONNECT_FAILE
 define( 'ERROR_LDAP_UPDATE_FAILED', 1402 );
 define( 'ERROR_LDAP_USER_NOT_FOUND', 1403 );
 define( 'ERROR_LDAP_EXTENSION_NOT_LOADED', 1404 );
+define( 'ERROR_LDAP_UNABLE_TO_STARTTLS', 1405 );
 
 # ERROR_CATEGORY_*
 define( 'ERROR_CATEGORY_DUPLICATE', 1500 );
diff -rupN mantisbt-1.2.17/core/ldap_api.php mantisbt-1.2.17-ldap-starttls/core/ldap_api.php
--- mantisbt-1.2.17/core/ldap_api.php	2014-03-03 14:19:50.000000000 -0500
+++ mantisbt-1.2.17-ldap-starttls/core/ldap_api.php	2014-09-18 14:37:37.000000000 -0400
@@ -50,6 +50,13 @@ function ldap_connect_bind( $p_binddn = 
     log_event( LOG_LDAP, "Attempting connection to LDAP URI '{$t_ldap_server}'." );
     $t_ds = @ldap_connect( $t_ldap_server );
     
+	$t_ldap_starttls = config_get( 'ldap_starttls');
+	if ($t_ldap_starttls) {
+		if (! @ldap_start_tls($t_ds)){
+			log_event( LOG_LDAP, "Error: Cannot initiate STARTTLS on LDAP Server" );
+			trigger_error( ERROR_LDAP_UNABLE_TO_STARTTLS, ERROR );
+		}
+	}
 	if ( $t_ds !== false && $t_ds > 0 ) {
 		log_event( LOG_LDAP, "Connection accepted by LDAP server" );
 		$t_protocol_version = config_get( 'ldap_protocol_version' );

Relationships

related to 0028328 closeddregad After Upgrade I got "APPLICATION ERROR #1406" with ERROR_LDAP_UNABLE_TO_STARTTLS after login 
related to 0029861 closeddregad LDAPS - no new users possible & password in cleartext 

Activities

illmnec

illmnec

2014-09-18 14:43

reporter   ~0041247

Added a patch that adds the functionality to mantisbt 1.2.17.
Three files are modified:
core/ldap_api.php (connection handling)
core/constant_inc.php (new ERROR definition)
config_defaults_inc.php (new g_ldap_starttls config variable requires default value)

tvleavitt

tvleavitt

2019-05-14 07:02

reporter   ~0062060

Bump. Can someone please take 5 minutes and integrate this? It's pretty damn trivial (just got it working myself). The request dates back six years, the code has been there since late 2014, and the effort required is nil.

dregad

dregad

2019-05-14 09:32

developer   ~0062061

just got it working myself

@tvleavitt would you mind submitting a Pull Request on our Github repository [1] with the changes, making sure that your submissions adhere to our Coding Guidelines [2] ? Thanks !

Note that the provided patch is not complete - an error message for ERROR_LDAP_UNABLE_TO_STARTTLS should be defined in strings_english.txt, and the Admin Guide should document the new config as well.

[1] https://github.com/mantisbt/mantisbt
[2] http://www.mantisbt.org/wiki/doku.php/mantisbt:coding_guidelines

tvleavitt

tvleavitt

2019-05-14 11:32

reporter   ~0062063

I'll do my best to get to it this week. I'm not overly familiar with Git and pull requests, etc. but I'm sure I can figure it out. How do I edit / update the Admin Guide? Adhering to the Coding Guidelines should be pretty trivial, the patches are tiny and basically exact duplicates of similar functions. I presume that the lang/strings_english.txt bit would map to the item specified in core/constant_inc.php in the patch.

dregad

dregad

2019-05-14 14:41

developer   ~0062064

I'll do my best to get to it this week.

Awesome, thanks

I'm not overly familiar with Git and pull requests, etc. but I'm sure I can figure it out.

If you need help just let me know. Feel free to ping me on Gitter

How do I edit / update the Admin Guide?

That's XML files (docbook format)

I presume that the lang/strings_english.txt bit would map to the item specified in core/constant_inc.php in the patch

Correct, just copy paste the line for another error message and update it as needed

rogueresearch

rogueresearch

2020-12-23 21:07

reporter   ~0064836

Was this patch ever integrated?

dregad

dregad

2020-12-24 05:43

developer   ~0064839

@rogueresearch I don't believe @tvleavitt ever submitted a pull request for this (or if he did, I missed it).

Feel free to finalize the patch per my indications in 0015361:0062061 and submit it yourself if you want; I'll gladly review and merge it.

rogueresearch

rogueresearch

2021-01-01 19:04

reporter   ~0064908

OK, I've started work on it...

dregad

dregad

2021-01-03 18:53

developer   ~0064923

Thanks, let me know if you need any help.

rogueresearch

rogueresearch

2021-01-04 12:36

reporter   ~0064925

@dregad see draft at: https://github.com/mantisbt/mantisbt/pull/1727

Note that I haven't tested it yet. Also, though I'm a professional programmer, I don't know PHP or web development at all.

dregad

dregad

2021-02-16 08:41

developer   ~0065135

Many thanks @rogueresearch for taking care of this, it always feel good to be able to close long-standing issues like this :-)

Related Changesets

MantisBT: master 94462f8c

2021-01-04 07:29

Sean McBride

Committer: dregad


Details Diff
Review of LDAP code; added StartTLS support

- added StartTLS support for LDAP, based on illmnec's patch
(fixes 0015361).
- added new ldap_tls_protocol_min option to specify minimun TLS version.
- changed default $g_ldap_protocol_version from 0 to 3 (fixes 0027848).
- improved Admin Guide and config_defaults_inc.php PHPDoc comments
- corrected log output for ldap_connect, which, despite its name,
doesn't actually perform a network connection, according to its docs.
- added an Admin Check to ensure that ldap_server config option is in
URI form (fixes 0027849).

Signed-off-by: Damien Regad <dregad@mantisbt.org>

PR https://github.com/mantisbt/mantisbt/pull/1727
Affected Issues
0015361, 0027848, 0027849
mod - admin/check/check_config_inc.php Diff File
mod - config_defaults_inc.php Diff File
mod - core/constant_inc.php Diff File
mod - core/ldap_api.php Diff File
mod - docbook/Admin_Guide/en-US/config/auth.xml Diff File
mod - lang/strings_english.txt Diff File