First user login function: Security and efficency opinions

Question

I am writing an application for work. This application will sit on a closed network but I still have to be concerned about security. There will be a maximum of three types of user for this application: User (read only), admin (data edits only) and super admin (access all areas). But at the moment I am trying to get a feel for security on a basic level.

Just so I am clear about the user options: I meant that only 3 people will be permitted to use the application. So, there will only ever be ONE basic "User" and not a number of basic users.

Would one of you kind people please look at the following code to review its efficiency and security with the possibility of offering some tips on improvement.

function loginUser($username, $password)
{

    include('assets/con.php');

    /* Set the salt. */
    $salt = 'JwethYGkuuygTYFIYTFrfkuyGUTf75kUY4uyk6';

    /* Apply SHA512 password and salt encryption. */
    $password = hash('SHA512', $password);
    $salt = hash('SHA512', $salt);

    $query = 'SELECT username FROM users WHERE username = ? AND password = ? AND pass_salt = ?';
    if( $statement = $con->prepare($query) )
    {
        $statement -> bind_param( 'sss', $username, $password, $salt );
        $statement -> execute();
        $statement -> store_result();
        $num_row = $statement -> num_rows;
        $statement -> bind_result($username);
        $statement -> fetch();
        $statement -> close();
    } 

    else die( "Failed to prepare query." );

    if( $num_row === 1 )
    {
        $_SESSION['userName'] = $username;
        return true;
    }
    return false;
}

Answer 1

Hashing

Let's start with the correct solution for hashing passwords: use bcrypt; PHP offers it with password_hash.

It's secure, and it's really easy to use (it's just a simple function call, and it manages salts etc for you).

What is wrong with your approach:

• simple sha512 is way too fast; it's still better than plaintext, but it's really not the correct approach.
• you don't have a per-user salt.
• you don't even have a per-site salt. pass_salt doesn't serve any purpose, the check will always match as you are using it wrong and are thus still open to rainbow attacks etc (it should be something like $password = hash('SHA512', $password . $salt);, but again, site-salts are not enough).

Misc

• your comment talks about encryption, but you are never encrypting, only hashing.
• putting spaces around -> is unusual and leads to harder to read code.
• don't die in functions, it makes it impossible for the calling code to recover. Just throw an exception.
• pass the connection to the function instead of including it to make your function easier to reuse and test.

Apart from these minor issues and the very weak hashing, your code looks good to me.

Answer 2

IMHO it's a pretty good code except one thing. Currently you have a fixed salt, for every user the same. You should consider to change it. I would recommend to generate a random salt for every user, you already have a salt field in the user table.

You could also consider the "new" way with http://php.net/manual/de/function.password-hash.php

Here is what we've done in the old fashioned way.

Generate a unique salted password for each user with this.

$randomSalt = hash('SHA512', uniqid(null, true));
$passwordSalt = hash('SHA512', $password . $randomSalt);

So that a possible attacker need to brute force the passwords seperatly for each user.

In the login check first get the salt for the entered username, something like this: SELECT salt FROM users WHERE username = ?.

With this salt you verify the password with something like this.

$passwordSalt = hash('SHA512', $password . $saltFromUserTable);
$query = "SELECT salt FROM users WHERE username = ? && password = ?yoursalt";

Hope this give you some idea of improving password logins. For sure this is not a "Just do it so" guide with the top-best security available but for most of applications this should be good. Security always also depends on the application and use-case.