View Issue Details

IDProjectCategoryView StatusLast Update
0010172mantisbtauthenticationpublic2017-05-16 14:22
ReporterthosjoAssigned Todregad 
PrioritynormalSeverityfeatureReproducibilityN/A
Status closedResolutionwon't fix 
Product Version1.2.0a3 
Target VersionFixed in Version 
Summary0010172: Passwords in SHA256 using a static salt
Description

To increase the security of the stored password hashes I've implemented support for SHA256 with a static salt.

The transition is handled by changing $g_login_method from MD5 to SHA256.

Tagspatch, schema

Relationships

related to 0011250 closeddregad Allow SHA1 passwords 
has duplicate 0009171 closedvboctor Implement secure/salted hashing algorithm for passwords 
has duplicate 0013273 closedatrol Store salted passwd only 
has duplicate 0021859 closedatrol Passwords using MD5 
related to 0022839 assigneddregad Deprecate MD5 login method and replace with BCRYPT hash 

Activities

2009-02-28 19:07

 

mantisbt-1.2.0a3-sha256.diff (3,630 bytes)
diff -uNr -x .svn mantisbt-1.2.0a3/admin/schema.php mantisbt-lab/admin/schema.php
--- mantisbt-1.2.0a3/admin/schema.php	2009-01-15 17:04:51.000000000 +0100
+++ mantisbt-lab/admin/schema.php	2009-02-28 23:55:02.000000000 +0100
@@ -301,7 +301,7 @@
   username 		C(32) NOTNULL DEFAULT \" '' \",
   realname 		C(64) NOTNULL DEFAULT \" '' \",
   email 		C(64) NOTNULL DEFAULT \" '' \",
-  password 		C(32) NOTNULL DEFAULT \" '' \",
+  password 		C(128) NOTNULL DEFAULT \" '' \",
   date_created 		T NOTNULL DEFAULT '" . db_null_date() . "',
   last_visit 		T NOTNULL DEFAULT '" . db_null_date() . "',
   enabled		L NOTNULL DEFAULT \" '1' \",
diff -uNr -x .svn mantisbt-1.2.0a3/config_defaults_inc.php mantisbt-lab/config_defaults_inc.php
--- mantisbt-1.2.0a3/config_defaults_inc.php	2009-01-15 17:04:59.000000000 +0100
+++ mantisbt-lab/config_defaults_inc.php	2009-02-28 23:56:44.000000000 +0100
@@ -2149,13 +2149,21 @@
 
 	/**
 	 * login method
-	 * CRYPT or PLAIN or MD5 or LDAP or BASIC_AUTH
+	 * CRYPT or PLAIN or MD5 or LDAP or BASIC_AUTH or SHA256
 	 * You can simply change this at will. MantisBT will try to figure out how the passwords were encrypted.
 	 * @global int $g_login_method
 	 */	
 	$g_login_method				= MD5;
 
 	/**
+	* password salt
+ 	* Static salt for user passwords
+	* Make this as long and complicated as you can
+	* NEW PASSWORDS HAS TO BE GENERATED WHEN THIS IS MODIFIED
+ 	*/
+	$g_password_static_salt		= 'blowfish';
+
+	/**
 	 * limit reporters
 	 * Set to ON if you wish to limit reporters to only viewing bugs that they report.
 	 * @global int $g_limit_reporters
diff -uNr -x .svn mantisbt-1.2.0a3/core/authentication_api.php mantisbt-lab/core/authentication_api.php
--- mantisbt-1.2.0a3/core/authentication_api.php	2009-01-15 17:04:59.000000000 +0100
+++ mantisbt-lab/core/authentication_api.php	2009-02-28 23:55:03.000000000 +0100
@@ -340,6 +340,7 @@
 	$t_password = user_get_field( $p_user_id, 'password' );
 	$t_login_methods = Array(
 		MD5,
+		SHA256,
 		CRYPT,
 		PLAIN,
 	);
@@ -399,6 +400,11 @@
 		case MD5:
 			$t_processed_password = md5( $p_password );
 			break;
+		case SHA256:
+			$p_salt = config_get( 'password_static_salt' );
+			$t_processed_password = sha256( $p_salt . $p_password );
+			break;
+
 		case BASIC_AUTH:
 		case PLAIN:
 		default:
@@ -406,7 +412,7 @@
 			break;
 	}
 
-	# cut this off to PASSLEN cahracters which the largest possible string in the database
+	# cut this off to PASSLEN characters which the largest possible string in the database
 	return substr( $t_processed_password, 0, PASSLEN );
 }
 
diff -uNr -x .svn mantisbt-1.2.0a3/core/constant_inc.php mantisbt-lab/core/constant_inc.php
--- mantisbt-1.2.0a3/core/constant_inc.php	2009-01-15 17:53:27.000000000 +0100
+++ mantisbt-lab/core/constant_inc.php	2009-02-28 23:55:03.000000000 +0100
@@ -115,6 +115,7 @@
 define( 'LDAP', 4 );
 define( 'BASIC_AUTH', 5 );
 define( 'HTTP_AUTH', 6 );
+define( 'SHA256', 7 );
 
 # file upload methods
 define( 'DISK', 1 );
@@ -476,4 +477,4 @@
 # Lengths - NOTE: these may represent hard-coded values in db schema and should not be changed.
 define( 'USERLEN', 32);
 define( 'REALLEN', 64);
-define( 'PASSLEN', 32);
+define( 'PASSLEN', 128);
diff -uNr -x .svn mantisbt-1.2.0a3/core/custom_function_api.php mantisbt-lab/core/custom_function_api.php
--- mantisbt-1.2.0a3/core/custom_function_api.php	2009-01-15 17:04:52.000000000 +0100
+++ mantisbt-lab/core/custom_function_api.php	2009-02-28 23:55:03.000000000 +0100
@@ -196,6 +196,7 @@
 		CRYPT,
 		CRYPT_FULL_SALT,
 		MD5,
+		SHA256,
 	);
 	if( in_array( config_get( 'login_method' ), $t_can_change ) ) {
 		return true;
jreese

jreese

2009-03-02 15:04

reporter   ~0020979

Two major troubles with your patch:

  • you shouldn't be modifying existing database schema entries. If you need to modify existing schema, create a new entry that alters the table in question appropriately. Otherwise, existing installations will not be upgraded correctly when applying the new code.

  • I don't think a static salt is useful. If we are going to implement salted passwords, we should make the effort to generate and store a unique salt in the database for each user. Preferably, the salt for each user would be at least 4 characters, randomly created from lower and upper case letters, numbers, and basic symbols (!, @, #, etc). This will give a large enough sample space to increase the complexity of hash-generation by multiple orders of magnitude.
thosjo

thosjo

2009-03-03 03:40

reporter   ~0020986

Last edited: 2009-03-03 03:40

I agree, the schema was modified to work with my lab but shouldnt be used when upgrading of course.

A static salt isn't as useful as a randomly generated salt but it's better than the current md5().

0009171 Has PHPass (http://cvsweb.openwall.com/cgi/cvsweb.cgi/projects/phpass/) been look at as an alternative?

vboctor

vboctor

2009-06-15 03:54

manager   ~0022163

Agreed with the concept. There are two possible approaches:

  1. Just add a new login method (e.g. SHA1).

  2. Do 1 + allow for existing installations to move to it. This means that the code will upgrade existing users from MD5 (or whatever it is) to SHA1 as the users login.

Ideally we should have number 2.

dhx

dhx

2009-08-07 10:55

reporter   ~0022697

The salt should be randomly generated per-password so that an attacker can't create a new rainbow table to crack all passwords from a Mantis database in parallel.

It might be best to add a new crypto_api that handles this sort of stuff using more modern PHP methods such as hash()?

Danez

Danez

2009-09-19 11:21

reporter   ~0022978

Where does the function sha256() in the patch come from? It's not a php core function. sha1() exists.

I would also suggest to use sha265/sha512 with dynamic salt and hash().
and if hash() is not available fallback to md5() with salt.

CarstenGrohmann

CarstenGrohmann

2011-08-29 07:46

reporter   ~0029587

What the state of this request?
Could you implement this request a little bit faster?

j_schultz

j_schultz

2017-04-30 17:07

reporter   ~0056744

What's the status on this? The fact that MD5 is still the default option in 2017 is alarming, especially since it is preferred over crypt() (which could offer much more secure hashes like bcrypt) without any obvious explanation.

dregad

dregad

2017-05-06 17:31

developer   ~0056784

It does not make much sense to implement SHA1 / SHA256 nowadays. BCRYPT is a much better option for hashing passwords, and is now offered by PHP as default method via password_hash() function.

I am therefore resolving this issue as "won't fix"; please follow-up in 0022839 for implementation of BCRYPT as default login method in future releases of MantisBT.

Issue History

Date Modified Username Field Change
2009-02-28 19:07 thosjo New Issue
2009-02-28 19:07 thosjo File Added: mantisbt-1.2.0a3-sha256.diff
2009-03-02 14:55 jreese Relationship added related to 0009171
2009-03-02 15:04 jreese Note Added: 0020979
2009-03-03 03:40 thosjo Note Added: 0020986
2009-03-03 03:40 thosjo Note Edited: 0020986
2009-06-15 03:51 vboctor Tag Attached: patch
2009-06-15 03:52 vboctor Relationship replaced has duplicate 0009171
2009-06-15 03:54 vboctor Note Added: 0022163
2009-06-15 03:54 vboctor Status new => acknowledged
2009-08-07 10:55 dhx Note Added: 0022697
2009-09-19 11:21 Danez Note Added: 0022978
2011-07-26 04:43 dregad Relationship added related to 0011250
2011-08-29 06:56 atrol Relationship added has duplicate 0013273
2011-08-29 07:46 CarstenGrohmann Note Added: 0029587
2016-11-04 14:33 atrol Relationship added has duplicate 0021859
2017-01-15 18:15 vboctor Tag Attached: schema
2017-04-30 17:07 j_schultz Note Added: 0056744
2017-05-06 17:26 dregad Relationship added related to 0022839
2017-05-06 17:31 dregad Note Added: 0056784
2017-05-06 17:31 dregad Status acknowledged => resolved
2017-05-06 17:31 dregad Resolution open => won't fix
2017-05-06 17:31 dregad Assigned To => dregad
2017-05-16 14:22 atrol Status resolved => closed