Issue #10730: Use crypto_api for generating nonces and improve hashing

A new Crypto API function crypto_generate_uri_safe_nonce has been added
which generates base64 encoded URI safe alphabet nonces according to
RFC4648. This nonce creation function can thus be used throughout
MantisBT where we need a random nonce. The primary use at the moment is
with form_api tokens.

Hashing throughout the codebase has been improved to use the newly
implemented $g_crypto_master_salt configuration option. This deprecates
a number of older salt configuration options as we now derive salts
from the master salt as needed. The Whirlpool hashing function is used
to generate stronger hashes (instead of the original md5 hashing that is
now deprecated).

RSS keys, cookie strings, lost password confirmation hashes, CAPTCHA
keys, form CSRF tokens and so forth have all been upgraded to make use
of the new Crypto API infrastructure and better hashing/salting methods.

config_defaults_inc.php

@@ -340,15 +340,6 @@
  */
 $g_send_reset_password	= ON;
 
-/**
- * String used to generate the confirm_hash for the 'lost password' feature and
- * captcha code for 'signup'
- * ATTENTION: CHANGE IT TO WHATEVER VALUE YOU PREFER
- * @global int $g_password_confirm_hash_magic_string
- * @todo randomize + admin check
- */
-$g_password_confirm_hash_magic_string = 'blowfish';
-
 /**
  * use captcha image to validate subscription it requires GD library installed
  * @global int $g_signup_use_captcha
@@ -3459,15 +3450,6 @@
 $g_rss_enabled = ON;
 
 
-/**
- * This seed is used as part of the inputs for calculating the authentication
- * key for the RSS feeds. If this seed changes, all the existing keys for the
- * RSS feeds will become invalid.  This is defaulted to the database user name,
- * but it is recommended to overwrite it with a specific value on installation.
- * @global string $g_rss_key_seed
- */
-$g_rss_key_seed = '%db_username%';
-
 /*********************
  * Bug Relationships *
  *********************/
@@ -4021,7 +4003,6 @@
 	'minimal_jscss',
 	'plugins_enabled',
 	'plugins_installed',
-	'rss_key_seed',
 	'session_',
 	'show_detailed_errors',
 	'show_queries_',

core/authentication_api.php

@@ -26,6 +26,7 @@
  * @uses access_api.php
  * @uses config_api.php
  * @uses constant_inc.php
+ * @uses crypto_api.php
  * @uses current_user_api.php
  * @uses database_api.php
  * @uses error_api.php
@@ -45,6 +46,7 @@
 require_api( 'access_api.php' );
 require_api( 'config_api.php' );
 require_api( 'constant_inc.php' );
+require_api( 'crypto_api.php' );
 require_api( 'current_user_api.php' );
 require_api( 'database_api.php' );
 require_api( 'error_api.php' );
@@ -449,31 +451,32 @@ function auth_process_plain_password( $p_password, $p_salt = null, $p_method = n
 }
 
 /**
- * Generate a random 12 character password
+ * Generate a random 16 character password.
  * @todo Review use of $p_email within mantis
  * @param string $p_email unused
- * @return string 12 character random password
+ * @return string 16 character random password
  * @access public
  */
 function auth_generate_random_password( $p_email ) {
-	$t_val = mt_rand( 0, mt_getrandmax() ) + mt_rand( 0, mt_getrandmax() );
-	$t_val = md5( $t_val );
-
-	return utf8_substr( $t_val, 0, 12 );
+	# !TODO: create memorable passwords?
+	return crypto_generate_uri_safe_nonce( 16 );
 }
 
 /**
- * Generate a confirm_hash 12 character to valide the password reset request
- * @param int $p_user_id user id
- * @return string representing MD5 hash
+ * Generate a confirmation code to validate password reset requests.
+ * @param int $p_user_id User ID to generate a confirmation code for
+ * @return string Confirmation code (384bit) encoded according to the base64 with URI safe alphabet approach described in RFC4648
  * @access public
  */
 function auth_generate_confirm_hash( $p_user_id ) {
-	$t_confirm_hash_generator = config_get( 'password_confirm_hash_magic_string' );
 	$t_password = user_get_field( $p_user_id, 'password' );
 	$t_last_visit = user_get_field( $p_user_id, 'last_visit' );
 
-	$t_confirm_hash = md5( $t_confirm_hash_generator . $t_password . $t_last_visit );
+	$t_confirm_hash_raw = hash( 'whirlpool', 'confirm_hash' . config_get_global( 'crypto_master_salt' ) . $t_password . $t_last_visit, true );
+	# Note: We truncate the last 8 bits from the hash output so that base64
+	# encoding can be performed without any trailing padding.
+	$t_confirm_hash_base64_encoded = base64_encode( substr( $t_confirm_hash_raw, 0, 63 ) );
+	$t_confirm_hash = strtr( $t_confirm_hash_base64_encoded, '+/', '-_' );
 
 	return $t_confirm_hash;
 }
@@ -524,27 +527,14 @@ function auth_clear_cookies() {
 }
 
 /**
- * Generate a string to use as the identifier for the login cookie
- * It is not guaranteed to be unique and should be checked
- * The string returned should be 64 characters in length
- * @return string 64 character cookie string
- * @access public
- */
-function auth_generate_cookie_string() {
-	$t_val = mt_rand( 0, mt_getrandmax() ) + mt_rand( 0, mt_getrandmax() );
-	$t_val = md5( $t_val ) . md5( time() );
-	return $t_val;
-}
-
-/**
- * Generate a UNIQUE string to use as the identifier for the login cookie
- * The string returned should be 64 characters in length
- * @return string 64 character cookie string
+ * Generate a random and unique string to use as the identifier for the login
+ * cookie.
+ * @return string Random and unique 384bit cookie string of encoded according to the base64 with URI safe alphabet approach described in RFC4648
  * @access public
  */
 function auth_generate_unique_cookie_string() {
 	do {
-		$t_cookie_string = auth_generate_cookie_string();
+		$t_cookie_string = crypto_generate_uri_safe_nonce( 64 );
 	}
 	while( !auth_is_cookie_string_unique( $t_cookie_string ) );
 

core/config_api.php

@@ -589,7 +589,7 @@ function config_is_private( $p_config_var ) {
 		case 'database_name':
 		case 'db_schema':
 		case 'db_type':
-		case 'password_confirm_hash_magic_string':
+		case 'master_crypto_salt':
 		case 'smtp_host':
 		case 'smtp_username':
 		case 'smtp_password':
@@ -632,7 +632,6 @@ function config_is_private( $p_config_var ) {
 		case 'meta_include_file':
 		case 'log_level':
 		case 'log_destination':
-		case 'rss_key_seed':
 		case 'dot_tool':
 		case 'neato_tool':
 		case 'twitter_username':

core/crypto_api.php

@@ -143,3 +143,27 @@ function crypto_generate_strong_random_string( $p_bytes ) {
 	}
 	return $t_random_string;
 }
+
+/**
+ * Generate a nonce encoded using the base64 with URI safe alphabet approach
+ * described in RFC4648. Note that the minimum length is rounded up to the next
+ * number with a factor of 4 so that padding is never added to the end of the
+ * base64 output. This means the '=' padding character is never present in the
+ * output. Due to the reduced character set of base64 encoding, the actual
+ * amount of entropy produced by this function for a given output string length
+ * is 3/4 (0.75) that of raw unencoded output produced with the
+ * crypto_generate_strong_random_string( $p_bytes ) function.
+ * @param int $p_minimum_length Minimum number of characters required for the nonce
+ * @return string Nonce encoded according to the base64 with URI safe alphabet approach described in RFC4648
+ */
+function crypto_generate_uri_safe_nonce( $p_minimum_length ) {
+	$t_length_mod4 = $p_minimum_length % 4;
+	$t_adjusted_length = $p_minimum_length + 4 - ($t_length_mod4 ? $t_length_mod4 : 4);
+	$t_raw_bytes_required = ( $t_adjusted_length / 4 ) * 3;
+	$t_random_bytes = crypto_generate_strong_random_string( $t_raw_bytes_required );
+	$t_base64_encoded = base64_encode( $t_random_bytes );
+	# Note: no need to translate trailing = padding characters because our
+	# length rounding ensures that padding is never required.
+	$t_random_nonce = strtr( $t_base64_encoded, '+/', '-_' );
+	return $t_random_nonce;
+}

core/form_api.php

@@ -29,13 +29,15 @@
  *
  * @uses config_api.php
  * @uses constant_inc.php
+ * @uses crypto_api.php
  * @uses gpc_api.php
  * @uses php_api.php
  * @uses session_api.php
  */
 
 require_api( 'config_api.php' );
 require_api( 'constant_inc.php' );
+require_api( 'crypto_api.php' );
 require_api( 'gpc_api.php' );
 require_api( 'php_api.php' );
 require_api( 'session_api.php' );
@@ -59,10 +61,11 @@ function form_security_token( $p_form_name ) {
 		$t_tokens[$p_form_name] = array();
 	}
 
-	# Generate a random security token prefixed by date.
-	# mt_rand() returns an int between 0 and RAND_MAX as extra entropy
+	# Generate a nonce prefixed by date.
+	# With a base64 output encoded nonce length of 32 characters, we are
+	# generating a 192bit nonce.
 	$t_date = date( 'Ymd' );
-	$t_string = $t_date . sha1( time() . mt_rand() );
+	$t_string = $t_date . crypto_generate_uri_safe_nonce( 32 );
 
 	# Add the token to the user's session
 	if ( !isset( $t_tokens[$p_form_name][$t_date] ) ) {

core/obsolete.php

@@ -141,3 +141,5 @@
 
 #changes in 1.3.0dev
 config_obsolete( 'bugnote_allow_user_edit_delete', '' );
+config_obsolete( 'password_confirm_hash_magic_string', 'crypto_master_salt' );
+config_obsolete( 'rss_key_seed', 'crypto_master_salt' );

core/rss_api.php

@@ -27,6 +27,7 @@
  * @uses authentication_api.php
  * @uses config_api.php
  * @uses constant_inc.php
+ * @uses crypto_api.php
  * @uses current_user_api.php
  * @uses helper_api.php
  * @uses user_api.php
@@ -35,15 +36,17 @@
 require_api( 'authentication_api.php' );
 require_api( 'config_api.php' );
 require_api( 'constant_inc.php' );
+require_api( 'crypto_api.php' );
 require_api( 'current_user_api.php' );
 require_api( 'helper_api.php' );
 require_api( 'user_api.php' );
 
 /**
- * Calculates a key to be used for RSS authentication based on user name, cookie and password.
- * if the user changes his user name or password, then the key becomes invalid.
- * @param int $p_user_id
- * @return string
+ * Calculates a key to be used for RSS authentication based on user name,
+ * cookie and password. If the user changes their user name or password, this
+ * RSS authentication key will become invalidated.
+ * @param int $p_user_id User ID for the user which the key is being calculated for
+ * @return string RSS authentication key (384bit) encoded according to the base64 with URI safe alphabet approach described in RFC4648
  */
 function rss_calculate_key( $p_user_id = null ) {
 	if( $p_user_id === null ) {
@@ -52,13 +55,17 @@ function rss_calculate_key( $p_user_id = null ) {
 		$t_user_id = $p_user_id;
 	}
 
-	$t_seed = config_get_global( 'rss_key_seed' );
-
 	$t_username = user_get_field( $t_user_id, 'username' );
 	$t_password = user_get_field( $t_user_id, 'password' );
 	$t_cookie = user_get_field( $t_user_id, 'cookie_string' );
 
-	return md5( $t_seed . $t_username . $t_cookie . $t_password );
+	$t_key_raw = hash( 'whirlpool', 'rss_key' . config_get_global( 'crypto_master_salt' ) . $t_username . $t_password . $t_cookie, true );
+	# Note: We truncate the last 8 bits from the hash output so that base64
+	# encoding can be performed without any trailing padding.
+	$t_key_base64_encoded = base64_encode( substr( $t_key_raw, 0, 63 ) );
+	$t_key = strtr( $t_key_base64_encoded, '+/', '-_' );
+
+	return $t_key;
 }
 
 /**

docbook/adminguide/en/configuration.sgml

@@ -250,13 +250,6 @@
                 </listitem>
             </varlistentry>
 
-            <varlistentry>
-                <term>$g_password_confirm_hash_magic_string</term>
-                <listitem>
-                    <para>TODO</para>
-                </listitem>
-            </varlistentry>
-
             <varlistentry>
                 <term>$g_signup_use_captcha</term>
                 <listitem>

make_captcha_img.php

@@ -23,18 +23,20 @@
  *
  * @uses core.php
  * @uses config_api.php
+ * @uses crypto_api.php
  * @uses gpc_api.php
  * @uses utility_api.php
  */
 
 require_once( 'core.php' );
 require_api( 'config_api.php' );
+require_api( 'crypto_api.php' );
 require_api( 'gpc_api.php' );
 require_api( 'utility_api.php' );
 
-$f_public_key = gpc_get_int( 'public_key' );
+$f_public_key = gpc_get_string( 'public_key' );
 
-$t_key = utf8_strtolower( utf8_substr( md5( config_get( 'password_confirm_hash_magic_string' ) . $f_public_key ), 1, 5) );
+$t_private_key = substr( hash( 'whirlpool', 'captcha' . config_get_global( 'crypto_master_salt' ) . $f_public_key, false ), 0, 5 );
 $t_system_font_folder = get_font_path();
 $t_font_per_captcha = config_get( 'font_per_captcha' );
 
@@ -44,7 +46,7 @@
 );
 
 $captcha = new masc_captcha( $t_captcha_init );
-$captcha->make_captcha( $t_key );
+$captcha->make_captcha( $t_private_key );
 
 #
 # The class below was derived from

signup.php

@@ -24,6 +24,7 @@
  * @uses authentication_api.php
  * @uses config_api.php
  * @uses constant_inc.php
+ * @uses crypto_api.php
  * @uses email_api.php
  * @uses form_api.php
  * @uses gpc_api.php
@@ -38,6 +39,7 @@
 require_api( 'authentication_api.php' );
 require_api( 'config_api.php' );
 require_api( 'constant_inc.php' );
+require_api( 'crypto_api.php' );
 require_api( 'email_api.php' );
 require_api( 'form_api.php' );
 require_api( 'gpc_api.php' );
@@ -52,7 +54,7 @@
 $f_username		= strip_tags( gpc_get_string( 'username' ) );
 $f_email		= strip_tags( gpc_get_string( 'email' ) );
 $f_captcha		= gpc_get_string( 'captcha', '' );
-$f_public_key	= gpc_get_int( 'public_key', '' );
+$f_public_key	= gpc_get_string( 'public_key', '' );
 
 $f_username = trim( $f_username );
 $f_email = email_append_domain( trim( $f_email ) );
@@ -72,9 +74,9 @@
 if( ON == config_get( 'signup_use_captcha' ) && get_gd_version() > 0 	&&
 			helper_call_custom_function( 'auth_can_change_password', array() ) ) {
 	# captcha image requires GD library and related option to ON
-	$t_key = utf8_strtolower( utf8_substr( md5( config_get( 'password_confirm_hash_magic_string' ) . $f_public_key ), 1, 5) );
+	$t_private_key = substr( hash( 'whirlpool', 'captcha' . config_get_global( 'crypto_master_salt' ) . $f_public_key, false ), 0, 5 );
 
-	if ( $t_key != $f_captcha ) {
+	if ( $t_private_key != $f_captcha ) {
 		trigger_error( ERROR_SIGNUP_NOT_MATCHING_CAPTCHA, ERROR );
 	}
 }

signup_page.php

@@ -23,6 +23,7 @@
  * @uses core.php
  * @uses config_api.php
  * @uses constant_inc.php
+ * @uses crypto_api.php
  * @uses form_api.php
  * @uses helper_api.php
  * @uses html_api.php
@@ -34,6 +35,7 @@
 require_once( 'core.php' );
 require_api( 'config_api.php' );
 require_api( 'constant_inc.php' );
+require_api( 'crypto_api.php' );
 require_api( 'form_api.php' );
 require_api( 'helper_api.php' );
 require_api( 'html_api.php' );
@@ -52,7 +54,7 @@
 html_page_top1();
 html_page_top2a();
 
-$t_key = mt_rand( 0,99999 );
+$t_public_key = crypto_generate_uri_safe_nonce( 64 );
 ?>
 
 <br />
@@ -94,8 +96,8 @@
 		<?php print_captcha_input( 'captcha', '' ) ?>
 	</td>
 	<td>
-		<img src="make_captcha_img.php?public_key=<?php echo $t_key ?>" alt="visual captcha" />
-		<input type="hidden" name="public_key" value="<?php echo $t_key ?>" />
+		<img src="make_captcha_img.php?public_key=<?php echo $t_public_key ?>" alt="visual captcha" />
+		<input type="hidden" name="public_key" value="<?php echo $t_public_key ?>" />
 	</td>
 </tr>
 <?php