Replace MD5 hash function

knight-of-ni commented 8 years ago

ZoneMinder uses MD5 for its authentication. Problem is that it is known to be compromised. I don't recall what the recommended hash function should be. We simply need to find what the library is, add support for it, and make that the default instead of other libraries that use md5 like openssl or gnutls.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

kylejohnson commented 8 years ago

We should be hashing and salting passwords. For now, I say we can ignore the salt as these are local installs and aren't always publicly exposed (read: to save us a lot of time completely revamping the password functions).

As for the hash function, SHA256 is currently recommended.

knight-of-ni commented 8 years ago

We also need to be careful with existing users when they upgrade to a version that uses the new hash. We don't want to lock them out of the web portal.

kylejohnson commented 8 years ago

Something like this:

$hashed_password =  hash('sha256', "$the_users_password");

connortechnology commented 8 years ago

We will have to try to auth properly, then try to auth using old md5, and then auto-update their db.

Linwood-F commented 8 years ago

This is perhaps too much history but... why doesn't Zoneminder just let the web server do authentication, and use zoneminder's database for authorization. Stop rolling our own (even with libraries)? At least that way when security patches are issued for the web, it is automatic here? Or does that change too many things?

kylejohnson commented 8 years ago

@Linwood-F You mean using something like auth_basic (or some other apache module)?

Linwood-F commented 8 years ago

Yes. I'm not engaged enough to know the risks of various ways choices, but it just strikes me that building it into Zoneminder, when (well, +/- the API) Zoneminder always lives behind a web server with authentication built in, seemed a bit redundant.

SteveGilvarry commented 8 years ago

Previous related discussion https://github.com/ZoneMinder/ZoneMinder/pull/916

kylejohnson commented 8 years ago

We'll want to verify that SHA256 is 'the best' for us right now.

As for not locking out previous users, I suggest we:

Add a new column to the Users table called hashing_scheme with a default value of our current scheme (md5).
When a user attempts to log in, hash the password they submitted with whatever's in that user's hashing_scheme.
- If the password was incorrect, fail.
- If the password was correct, and this user's hashing_scheme value is different to the application's current_hashing_scheme, rehash the user's password with the new scheme, then update the password and hashing_scheme value in the database.

kylejohnson commented 8 years ago

If no one else wants to grab this one, I'd like to take it. Maybe for 1.30 or 1.31.

knight-of-ni commented 8 years ago

Good idea. And I realized over lunch that gnutls is a collection of crypto functions, not just md5, duh. It does have sha256 so we don't need to go hunting for any new libraries.

If you can get to this before I do, I won't complain. I promised @whorfin months ago that I'd help him make ZoneMinder more friendly for Solaris so that is my next priority.

kylejohnson commented 8 years ago

I think the correct path is to go with php's password_hash and password_verify functions, as opposed to calling external libraries.

connortechnology commented 8 years ago

A while back I was upgrading the password hashes in my perl-based web app. What I read at the time was the Blowfish is the thing to do for passwords. https://en.wikipedia.org/wiki/Blowfish_(cipher)

There is an rfc for how to store hashed passwords that even tells you how they are ciphered (rfc2307).

I would be tempted to use the built-in php hashing functions, but as I recall there is some problem with also doing the hashing with perl and C.

alager commented 8 years ago

Sorry if this is too naive, not knowing how all the security stuff is structured, but can you just use the DB directly? No library needed. mysql 5.5 supports sha256 and many others. Below is an example using sha1 . Sha256 just adds a few parameters.

$stmnt =
        "select count(*) as count from users where name = $name and password = " .
        "cast( sha1( concat( $salt, $password ) ) )";

kylejohnson commented 8 years ago

@alager While we /could/ use the DB, it doesn't offer bcrypt / blowfish. In addition, it might not be as portable as we want, as we'd like to finish support for postgres some time this decade.

alager commented 8 years ago

Hmm, maybe I misunderstood the topic. Seemed that using multiple hash algorithms wasn't the issue. From the php docs on password_hash(), using the default bcrypt will evolve and return different results over time (different php versions). That seems like a maintenance headache. And blowfish, according to the Wikipedia link above (at the bottom of the article), is not recommended either, it's been succeeded by two-fish and three-fish. Postgres also supports multiple hashes, including sha2 family, like mysql does. Just my two cents. Any of the choices will be more secure than md5, from a crypto point of view.

andrewmcguinness commented 8 years ago

Security experts recommend not using SHA for password storage, as it is too fast. Recommendations are PBKDF2 or bcrypt See e.g. http://www.troyhunt.com/2012/06/our-password-hashing-has-no-clothes.html

@alager : the different results over time is not a problem; you don't check the password by calling password_hash() on the submitted password and comparing, you use password_verify() for checking the password, and it will correctly verify a password hashed using an older password_hash() default.

http://php.net/manual/en/function.password-verify.php

The evolving of bcrypt is an essential feature because the speed of hashing in hardware is increasing so fast.

Blowfish is no longer recommended for encryption, but using bcrypt (which is a function involving repeated application of blowfish, designed specifically for hashing passwords) is still seen as fine. Since it's so well supported in php, it seems the best way to go.

I'd have a go at it, but I have no php experience.

SteveGilvarry commented 7 years ago

PHP have said they are implementing lib sodium in 7.2 https://paragonie.com/book/pecl-libsodium/read/00-intro.md

JedMeister commented 7 years ago

+1 for bcrypt (aka what PHP appear to call "blowfish" from what I've read) -1 for SHAx (any version)

Post on bcypt/blowfish vs SHA512 (WRT to "hashing algorithms"): https://stackoverflow.com/a/1561245/3363571

Post on bcypt vs blowfish usage in PHP: https://security.stackexchange.com/a/110293/81392

PS IIRC bcypt manages salting for you... So AFAIK usage of PHP's password_hash and password_verify should also provide a salting mechanism, as well as password hashing. Note though, I'm not a PHP user so it is possible that my experience of bcrypt in Python doesn't transfer...

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mnoorenberghe commented 5 years ago

So some of the open issues:

I believe we sometimes need access to do authentication from outside PHP code e.g. c++/Perl which could be a challenge for some paths. Currently it's all done in MySQL.
If we move to doing auth in PHP, what versions of PHP do we have to support?

I'm wondering if we shouldn't let the perfect be the enemy of the good here as I am also looking into new front-end frameworks and we would likely get stronger authentication for free as part of that. In that vein we should maybe keep doing auth in MySQL for an easy transition and use what is available (SHA2 and AES)…

This is also now more important since PASSWORD() was removed in MySQL 8.0.11.

JedMeister commented 5 years ago

As noted before, I'm no PHP coder (nor Perl or C++). I can't speak at all for implementation re C++/Perl, but a quick google suggests that there are a ton of modules/libraries for doing bcrypt/blowfish related stuff in both these languages.

I have a little experience with Python's bcrypt library and that works a treat with authenticating via DB stored passwords created with PHP's password_hash so I would expect that to also be pretty straight forward with other languages.

IMO, using bcrypt/blowfish via PHP for password hashing/authentication is a no brainer. My 2c anyway...

ZoneMinder / zoneminder

Replace MD5 hash function #1235