Review of LDAP code; added StartTLS support

- added StartTLS support for LDAP, based on illmnec's patch
  (fixes #15361).
- added new ldap_tls_protocol_min option to specify minimun TLS version.
- changed default $g_ldap_protocol_version from 0 to 3 (fixes #27848).
- 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 #27849).

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

PR #1727

admin/check/check_config_inc.php

@@ -117,5 +117,13 @@
 	array( false => 'Use "limit_view_unless_threshold" instead.' )
 );
 
+# Check that 'ldap_server' is a proper URI, starting with either ldap:// or ldaps://
+$t_ldap_server = config_get_global( 'ldap_server' );
+check_print_test_row(
+	'"ldap_server" must be a valid, full LDAP URI',
+	( preg_match( '~^ldaps?://~', $t_ldap_server ) == 1 ),
+	array( false => '"ldap_server" must be a proper URI, starting with either "ldap://" or "ldaps://"' )
+);
+
 # Obsolete Settings
 require_api( 'obsolete.php' );

config_defaults_inc.php

@@ -2086,60 +2086,89 @@
 
 
 /**
- * Specifies the LDAP or Active Directory server to connect to.
+ * Specifies the LDAP or Active Directory server(s) to connect to.
+ * Multiple servers can be specified as a space-separated list.
  *
  * This must be a full LDAP URI (ldap[s]://hostname:port)
- * - Protocol can be either ldap or ldaps (for SSL encryption). If omitted,
- *   then an unencrypted connection will be established on port 389.
+ * - Protocol must be either:
+ *   - ldap (unencrypted or opportunistic StartTLS)
+ *   - ldaps (for TLS encryption)
  * - Port number is optional, and defaults to 389. If this doesn't work, try
  *   using one of the following standard port numbers: 636 (ldaps); for Active
  *   Directory Global Catalog forest-wide search, use 3268 (ldap) or 3269 (ldaps)
  *
  * Examples of valid URI:
- *   ldap.example.com
  *   ldap://ldap.example.com
  *   ldaps://ldap.example.com:3269/
  *
  * @global string $g_ldap_server
  */
-$g_ldap_server = 'ldaps://ldap.example.com/';
+$g_ldap_server = 'ldap://ldap.example.com/';
 
 /**
- * The root distinguished name for LDAP searches
+ * Decides if the connection will attempt an opportunistic upgrade to
+ * a TLS connection (STARTTLS).
+ * Be sure to use the ldap:// scheme and not ldaps://
+ * if setting this to ON.  For security, a failure aborts the entire connection.
+ *
+ * @global integer $g_ldap_use_starttls
+ */
+$g_ldap_use_starttls = ON;
+
+/**
+ * The minimum version of the TLS protocol to allow.
+ * For example, LDAP_OPT_X_TLS_PROTOCOL_TLS1_2. If OFF, then the protocol version
+ * is not set. This maps to the LDAP_OPT_X_TLS_PROTOCOL_MIN ldap library option.
+ * Requires PHP 7.1 or later. For security, a failure aborts the entire connection.
+ * @see https://www.php.net/manual/en/ldap.constants.php#constant.ldap-opt-x-tls-protocol-min
+ *
+ * @global int $g_ldap_tls_protocol_min
+ */
+$g_ldap_tls_protocol_min = OFF;
+
+/**
+ * The root distinguished name for LDAP searches.
  * @global string $g_ldap_root_dn
  */
 $g_ldap_root_dn = 'dc=example,dc=com';
 
 /**
- * LDAP search filter for the organization
+ * LDAP search filter for the organization.
  * e.g. '(organizationname=*Traffic)'
  * @global string $g_ldap_organization
  */
 $g_ldap_organization = '';
 
 /**
- * The LDAP Protocol Version, if 0, then the protocol version is not set.
- * For Active Directory use version 3.
+ * The LDAP Protocol Version.
+ * If 0, then the protocol version is not set, and you get whatever default
+ * the underlying ldap library uses. This maps to the LDAP_OPT_PROTOCOL_VERSION
+ * ldap library option. In almost all cases you should use 3. LDAPv3 was
+ * introduced back in 1997. LDAPv2 was deprecated in 2003 by RFC3494.
  *
  * @global integer $g_ldap_protocol_version
  */
-$g_ldap_protocol_version = 0;
+$g_ldap_protocol_version = 3;
 
 /**
  * Duration of the timeout for TCP connection to the LDAP server (in seconds).
+ * This maps to the LDAP_OPT_NETWORK_TIMEOUT ldap library option.
  * Set this to a low value when the hostname defined in $g_ldap_server resolves
  * to multiple IP addresses, allowing rapid failover to the next available LDAP
  * server.
- * Defaults to 0 (infinite)
+ * Defaults to 0 (infinite).
  *
  * @global int $g_ldap_network_timeout
  */
 $g_ldap_network_timeout = 0;
 
 /**
  * Determines whether the LDAP library automatically follows referrals returned
- * by LDAP servers or not. This maps to LDAP_OPT_REFERRALS ldap library option.
+ * by LDAP servers or not.
+ * This maps to the LDAP_OPT_REFERRALS ldap library option.
  * For Active Directory, this should be set to OFF.
+ * If you have only one LDAP server, setting to this to OFF is advisible to prevent
+ * any man-in-the-middle attacks.
  *
  * @global integer $g_ldap_follow_referrals
  */
@@ -2148,7 +2177,8 @@
 /**
  * The distinguished name of the service account to use for binding to the
  * LDAP server.
- * For example, 'CN=ldap,OU=Administrators,DC=example,DC=com'.
+ * For anonymous binding, leave empty.
+ * For example, 'cn=ldap,ou=Administrators,dc=example,dc=com'.
  *
  * @global string $g_ldap_bind_dn
  */
@@ -2157,14 +2187,15 @@
 /**
  * The password for the service account used to establish the connection to
  * the LDAP server.
+ * For anonymous binding, leave empty.
  *
  * @global string $g_ldap_bind_passwd
  */
 $g_ldap_bind_passwd = '';
 
 /**
- * The LDAP field for username
- * Use 'sAMAccountName' for Active Directory
+ * The LDAP field for username.
+ * Use 'sAMAccountName' for Active Directory.
  * @global string $g_ldap_uid_field
  */
 $g_ldap_uid_field = 'uid';
@@ -2178,19 +2209,23 @@
 /**
  * Use the realname specified in LDAP (ON) rather than the one stored in the
  * database (OFF).
+ * Note that MantisBT will update the database with the data retrieved
+ * from LDAP when ON.
  * @global integer $g_use_ldap_realname
  */
 $g_use_ldap_realname = OFF;
 
 /**
  * Use the email address specified in LDAP (ON) rather than the one stored
  * in the database (OFF).
+ * Note that MantisBT will update the database with the data retrieved
+ * from LDAP when ON.
  * @global integer $g_use_ldap_email
  */
 $g_use_ldap_email = OFF;
 
 /**
- * This configuration option allows replacing the ldap server with a comma-
+ * This configuration option allows replacing the LDAP server with a comma-
  * delimited text file for development or testing purposes.
  * The LDAP simulation file format is as follows:
  *   - One line per user
@@ -2201,7 +2236,7 @@
  *        - password
  *   - Any extra fields are ignored
  * On production systems, this option should be set to ''.
- * @global integer $g_ldap_simulation_file_path
+ * @global string $g_ldap_simulation_file_path
  */
 $g_ldap_simulation_file_path = '';
 
@@ -4426,7 +4461,9 @@
 	'ldap_root_dn',
 	'ldap_server',
 	'ldap_simulation_file_path',
+	'ldap_tls_protocol_min',
 	'ldap_uid_field',
+	'ldap_use_starttls',
 	'library_path',
 	'login_method',
 	'logo_image',

core/constant_inc.php

@@ -357,6 +357,8 @@
 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_SET_MIN_TLS', 1405 );
+define( 'ERROR_LDAP_UNABLE_TO_STARTTLS', 1406 );
 
 # ERROR_CATEGORY_*
 define( 'ERROR_CATEGORY_DUPLICATE', 1500 );

core/ldap_api.php

@@ -64,16 +64,16 @@ function ldap_connect_bind( $p_binddn = '', $p_password = '' ) {
 
 	$t_ldap_server = config_get_global( 'ldap_server' );
 
-	log_event( LOG_LDAP, 'Attempting connection to LDAP server/URI \'' . $t_ldap_server . '\'.' );
+	log_event( LOG_LDAP, 'Checking syntax of LDAP server URI \'' . $t_ldap_server . '\'.' );
 	$t_ds = @ldap_connect( $t_ldap_server );
 	if( $t_ds === false ) {
-		log_event( LOG_LDAP, 'Connection to LDAP server failed' );
+		log_event( LOG_LDAP, 'LDAP server URI syntax check failed, make sure its in URI form' );
 		trigger_error( ERROR_LDAP_SERVER_CONNECT_FAILED, ERROR );
 		# Return required as function may be called with error suppressed
 		return false;
 	}
 
-	log_event( LOG_LDAP, 'Connection accepted by LDAP server' );
+	log_event( LOG_LDAP, 'LDAP server URI syntax check succeeded' );
 
 	$t_network_timeout = config_get_global( 'ldap_network_timeout' );
 	if( $t_network_timeout > 0 ) {
@@ -100,8 +100,39 @@ function ldap_connect_bind( $p_binddn = '', $p_password = '' ) {
 		ldap_log_error( $t_ds );
 	}
 
+	# Set minimum TLS protocol version flag (ex: LDAP_OPT_X_TLS_PROTOCOL_TLS1_2).
+	if( version_compare( PHP_VERSION, '7.1.0', '>=' ) ) {
+		$t_tls_protocol_min = config_get_global( 'ldap_tls_protocol_min' );
+		if( $t_tls_protocol_min > 0 ) {
+			log_event( LOG_LDAP, 'Attempting to set minimum TLS protocol' );
+			$t_result = @ldap_set_option( $t_ds, LDAP_OPT_X_TLS_PROTOCOL_MIN, $t_tls_protocol_min );
+			if( !$t_result ) {
+				ldap_log_error( $t_ds );
+				log_event( LOG_LDAP, "Error: Failed to set minimum TLS version on LDAP server" );
+				trigger_error( ERROR_LDAP_UNABLE_TO_SET_MIN_TLS, ERROR );
+
+				# Return required as function may be called with error suppressed
+				return false;
+			}
+		}
+	}
+
+	$t_use_starttls = config_get_global( 'ldap_use_starttls' );
+	if ( $t_use_starttls ) {
+		log_event( LOG_LDAP, 'Attempting StartTLS' );
+		$t_result = @ldap_start_tls( $t_ds );
+		if( !$t_result ) {
+			ldap_log_error( $t_ds );
+			log_event( LOG_LDAP, "Error: Cannot initiate StartTLS on LDAP server" );
+			trigger_error( ERROR_LDAP_UNABLE_TO_STARTTLS, ERROR );
+
+			# Return required as function may be called with error suppressed
+			return false;
+		}
+	}
+
 	# If no Bind DN and Password is set, attempt to login as the configured
-	#  Bind DN.
+	# Bind DN.
 	if( is_blank( $p_binddn ) && is_blank( $p_password ) ) {
 		$p_binddn = config_get_global( 'ldap_bind_dn', '' );
 		$p_password = config_get_global( 'ldap_bind_passwd', '' );
@@ -204,7 +235,7 @@ function ldap_escape_string( $p_string ) {
 function ldap_cache_user_data( $p_username ) {
 	global $g_cache_ldap_data;
 
-	# Returne cached data if available
+	# Return cached data if available
 	if( isset( $g_cache_ldap_data[$p_username] ) ) {
 		return $g_cache_ldap_data[$p_username];
 	}

docbook/Admin_Guide/en-US/config/auth.xml

@@ -91,10 +91,15 @@
 
 					<itemizedlist>
 						<listitem>
-							<para>Protocol can be either
-								<filename>ldap</filename> or
-								<filename>ldaps</filename> (for SSL/TLS encryption).
-								If omitted, then an unencrypted connection will be established on port 389.
+							<para>Protocol must be either:
+								<itemizedlist>
+									<listitem>
+										<para>ldap - unencrypted or opportunistic StartTLS</para>
+									</listitem>
+									<listitem>
+										<para>ldaps - TLS encryption</para>
+									</listitem>
+								</itemizedlist>
 							</para>
 						</listitem>
 						<listitem>
@@ -109,13 +114,36 @@
 
 					<para>Examples of valid URI:</para>
 					<programlisting>
-ldap.example.com
 ldap://ldap.example.com/
 ldaps://ldap.example.com:3269/
 </programlisting>
 				</listitem>
 			</varlistentry>
 
+			<varlistentry>
+				<term>$g_ldap_use_starttls</term>
+
+				<listitem>
+					<para>A boolean indicating if the connection will attempt opportunistic
+					upgrade to a TLS connection (STARTTLS).  Be sure to use the ldap:// scheme
+					and not ldaps:// if setting this to ON.  For security, a failure aborts
+					the entire connection. Defaults to <emphasis>ON</emphasis>.</para>
+				</listitem>
+			</varlistentry>
+
+			<varlistentry>
+				<term>$g_ldap_tls_protocol_min</term>
+
+				<listitem>
+					<para>An integer indicating the minimum version of the
+					TLS protocol to allow. For example, LDAP_OPT_X_TLS_PROTOCOL_TLS1_2.
+					If 0, then the protocol version is not set.
+					This maps to LDAP_OPT_X_TLS_PROTOCOL_MIN ldap library option.
+					Requires PHP 7.1 or later.
+					Defaults to <emphasis>0</emphasis>.</para>
+				</listitem>
+			</varlistentry>
+
 			<varlistentry>
 				<term>$g_ldap_root_dn</term>
 
@@ -139,12 +167,12 @@ ldaps://ldap.example.com:3269/
 				<term>$g_ldap_protocol_version</term>
 
 				<listitem>
-					<para>The LDAP Protocol Version. If 0, then the
-					protocol version is not set. Defaults to
-					<emphasis>0</emphasis>.</para>
-
-					<para>For Active Directory use protocol version
-					3.</para>
+					<para>The LDAP Protocol Version. If 0, then the protocol version
+					is not set, and you get whatever default the underlying ldap
+					library uses. This maps to the LDAP_OPT_PROTOCOL_VERSION ldap
+					library option. In almost all cases you should use 3. LDAPv3 was
+					introduced back in 1997. LDAPv2 was deprecated in 2003 by RFC3494.
+					Defaults to <emphasis>3</emphasis>.</para>
 				</listitem>
 			</varlistentry>
 
@@ -153,7 +181,8 @@ ldaps://ldap.example.com:3269/
 
 				<listitem>
 					<para>Duration of the timeout for TCP connection to the
-					LDAP server (in seconds). Defaults to
+					LDAP server (in seconds). This maps to LDAP_OPT_NETWORK_TIMEOUT ldap
+					library option. Defaults to
 					<emphasis>0</emphasis> (infinite).</para>
 
 					<para>Set this to a low value when the hostname defined
@@ -174,7 +203,9 @@ ldaps://ldap.example.com:3269/
 					<emphasis>ON</emphasis>.</para>
 
 					<para>For Active Directory, this should be set to
-					OFF.</para>
+					OFF. If you have only one LDAP server, setting to
+					this to OFF is advisible to prevent any
+					man-in-the-middle attacks.</para>
 				</listitem>
 			</varlistentry>
 
@@ -184,7 +215,8 @@ ldaps://ldap.example.com:3269/
 				<listitem>
 					<para>The distinguished name of the service account to
 					use for binding to the LDAP server. For example,
-					'CN=ldap,OU=Administrators,DC=example,DC=com'.</para>
+					'cn=ldap,ou=Administrators,dc=example,dc=com'.
+					For anonymous binding, leave empty.</para>
 				</listitem>
 			</varlistentry>
 
@@ -193,7 +225,8 @@ ldaps://ldap.example.com:3269/
 
 				<listitem>
 					<para>The password for the service account used to
-					establish the connection to the LDAP server.</para>
+					establish the connection to the LDAP server.
+					For anonymous binding, leave empty.</para>
 				</listitem>
 			</varlistentry>
 

lang/strings_english.txt

@@ -1670,6 +1670,8 @@ $MANTIS_ERROR[ERROR_LDAP_SERVER_CONNECT_FAILED] = 'LDAP Server Connection Failed
 $MANTIS_ERROR[ERROR_LDAP_UPDATE_FAILED] = 'LDAP Record Update has failed.';
 $MANTIS_ERROR[ERROR_LDAP_USER_NOT_FOUND] = 'LDAP User Record Not Found.';
 $MANTIS_ERROR[ERROR_LDAP_EXTENSION_NOT_LOADED] = 'LDAP Extension Not Loaded.';
+$MANTIS_ERROR[ERROR_LDAP_UNABLE_TO_SET_MIN_TLS] = 'Failed to set minimum TLS version on LDAP server.';
+$MANTIS_ERROR[ERROR_LDAP_UNABLE_TO_STARTTLS] = 'Cannot initiate StartTLS on LDAP server.';
 $MANTIS_ERROR[ERROR_DB_CONNECT_FAILED] = 'Database connection failed. Error received from database was #%1$d: %2$s.';
 $MANTIS_ERROR[ERROR_DB_QUERY_FAILED] = 'Database query failed. Error received from database was #%1$d: %2$s for the query: %3$s.';
 $MANTIS_ERROR[ERROR_DB_SELECT_FAILED] = 'Database selection failed. Error received from database was #%1$d: %2$s.';