Use modern hash function when save user's password

[fetus-hina]

Today, the MD5 hash algorithm is very weak and Mantis's current hash algorithm does not use salt and key stretching. The hashed password that created by current algorithm is seriously weak. (The hashed password that stored in database maybe contained in a rainbow-table, or may calculates quickly with CPU and/or GPU)

This pull-request adds new password hashing algorithm and switch default to it. As of today, new algorithm uses bcrypt via PHP-built-in password_hash() function. (This function available from PHP 5.5) If bcrypt becomes "weak", the PHP team (or the distros' maintainer) will upgrade algorithm to safer one. Then, stored hashed-password will upgrade automatically when user logged in after update PHP engine. This means "this new algorithm is maintenance-free."

Backward compatibility info: The hashed-password that created by MD5 or other older algorithms are still accepted for authentication. Older password will upgrade automatically when first log in.

Note: The password_hash() function requires PHP 5.5+ and Mantis's current minimal requirement is 5.5.9+. But, the documentation that linked from download page says "5.3.2+" (it looks for Mantis v1.3.x, but Mantis website links to that version now). If we should support PHP 5.3.2-5.4.x, we should include the ircmaxell/password_compat library. (This PR does not so.)

As you may have noticed, I'm not good at English. I feel sorry for my difficult-to-read English sentences. I will saved when feedback is simple and easy English.

Thank you.

Bugtracker reference: #22839

[dregad]

Hello @fetus-hina many thanks for your contribution, this is greatly appreciated as I've had this on my todo-list for ages, but somehow never got around to coding it. I will review your pull request later.

See also related #50 (yes that's old)

Don't worry about your English, it is anyway much better than my Japanese ;)

the documentation that linked from download page says "5.3.2+" (it looks for Mantis v1.3.x, but Mantis website links to that version now).

Thanks for pointing that out; I will fix it. (for the record, even though you won't have access to it #22511)

[dregad]

I will fix it. (for the record, even though you won't have access to it #22511)

It's done.

[vboctor]

Is there a way to incorporate the hashing mechanism into the AuthPlugin? #1070

[dregad]

@fetus-hina I rebased your branch on top of latest master, made a few adjustments and updated documentation. Many thanks for your work, let me know if you have any objections with the changes I made.

I ran some basic tests, everything seems to work fine when "upgrading" the login method e.g. from MD5, but additional testing is required to ensure everything works as expected in various conversion scenarios as well.

Is there a way to incorporate the hashing mechanism into the AuthPlugin? #1070

@vboctor, not sure what you mean by that, can you clarify ?

[fetus-hina]

@dregad I have no objections about your work. However, we seem to need discussion about length limitation. Also I have no opinion about it, at this time.

[dregad]

@vboctor do you have any further comments on my replies following your initial review ?

Also, it would be nice if you could provide feedback on https://github.com/mantisbt/mantisbt/commit/8e466bb3dc84bf30fa893ca206ddf89032dacf11#r21980157

[vboctor]

@dregad I responded to your replies. I think these should be addressed or get to an agreement on.

[dregad]

@vboctor thanks. I'll make adjustments per my replies above, and let you know when it's ready for the next round of review.

Note that you have not responded to

Also, it would be nice if you could provide feedback on https://github.com/mantisbt/mantisbt/commit/8e466bb3dc84bf30fa893ca206ddf89032dacf11#r21980157

[vboctor]

@dregad I replied to your comment about LDAP check positioning.

[dregad]

@vboctor I just pushed updated code which should address your comments. I have not fully tested it yet.

[atrol]

Maybe we should change the password in the following schema step to prevent any issue we might get when trying to update the hash on first login of a fresh install.

$g_upgrade[51] = array( 'InsertData', array( db_get_table( 'user' ), "(
		username, realname, email, password,
		date_created, last_visit, enabled, protected, access_level,
		login_count, lost_password_request_count, failed_login_count,
		cookie_string
	)
	VALUES (
		'administrator', '', 'root@localhost', '63a9f0ea7bb98050796b649e85481845',
		$t_timestamp, $t_timestamp, '1', '0', 90,
		3, 0, 0, '"
		. md5( mt_rand( 0, mt_getrandmax() ) + mt_rand( 0, mt_getrandmax() ) ) . md5( time() )
		. "'
	)" ) );

[dregad]

prevent any issue we might get when trying to update the hash on first login of a fresh install

I'm not really sure what issues you are thinking about here, but in any case I think it should be safe to be consistent and update the default administrator password's hash from md5 to bcrypt.

[atrol]

I'm not really sure what issues you are thinking about here

Nothing special, just the fact that code which is not executed can't cause a problem.

[dregad]

For the record, I am still working on this, I am not satisfied with the current handling of the password upgrade mechanism and would like to implement automated testing too.

[Magissia]

Could we merge this as current hashing is clearly lacking security ?

[dregad]

@Magissia it is not mergeable in its current form, and I have not gotten around to fixing the issues I have found in password upgrade logic. Too many things to do, and not enough time, unfortunately.

[limesurvey-translations]

By todays standards a plain MD5 hashing would be considered to be a major vulnerability. If a break-in happens, most passwords could be easily decrypted with rainbow tables. When is this finally going to be fixed?

[dododedodonl]

Any updates regarding the acceptance of this (or a similair) PR?

[luciditee]

Is there an update to this PR? I ended up using Bugzilla for a deployment instead of Mantis because of the MD5 usage, and I much prefer the Mantis interface.

[dregad]

Just to let everyone know that this is still on my todo list; unfortunately real life got in the way, and I am having a hard time investing time on open-source development these days so I'm no able to commit on a date for fixing this issue.

[seanm]

Seems this bug got some press today:

https://it.slashdot.org/story/19/06/17/182208/a-quarter-of-major-cmss-use-outdated-md5-as-the-default-password-hashing-scheme

https://www.zdnet.com/article/a-quarter-of-major-cmss-use-outdated-md5-as-the-default-password-hashing-scheme/

[kaizen1]

So is Mantis still using ancient MD5 hash? Major security hole as everyone knows.

[istiak101]

So is Mantis still using ancient MD5 hash? Major security hole as everyone knows.

Yes.