Is this using PDO and prepared statements correctly for a secure login?
Once again looking for some help with PHP security and a login system. Wondering if i did this correctly here. If i wasn't specific enough anywhere please ask, Any help is greatly appreciated. I am trying to create a secure login system, just for purposes of learning. here is the code:
require("constants.php");
$DBH = new mysqli($dbhost, $dbuser, $dbpass, $dbname);
function createSalt() {
$length = mt_rand(64, 128);
$salt = '';
for ($i = 0; $i < $length; $i++) {
$salt .= chr(mt_rand(33, 255));
}
return $salt;
}
//Salt function created by ircmaxell
function registerNewUser() {
//Check to see if Username Is In Use//
$q = $DBH->prepare("SELECT id FROM users WHERE username = ?");
$username = filter_var($username, FILTER_SANITIZE_STRING);
$data = array($username);
$q->execute($data);
$row = $q->fetch();
if ($row === false) {
//If Username Is Not Already In Use Insert Data//
$hash = hash('sha256', $pass);
$salt = createSalt();
$hash = hash('sha256', $salt . $hash . $pass); //UPDATED
$data = array($username, $hash, $salt);
$qInsert = $DBH->prepare(
"INSERT INTO users (username, password, salt) values (?, ?, ?)"
);
$qInsert->execut开发者_如何转开发e($data); //Inserts User Data Into Table//
}
}
That looks good so far. I have three suggestions, though:
- Choose a longer salt
- Don't store salt and password digest separately
- If your database connection is not to
localhost
use a different database connector: PDO doesn't (yet) support SSL connections
EDIT: Also, validate the input a client provides as a "username". But since your code sample is just an excerpt I guess you're doing that.
EDIT #2: When I said, "don't store salt and password separately" I meant incorporating the salt into the stored password hash. Since the hash algorithm of your choice produces quite a long string (64 character) composed of [0-9a-f] you might want to contemplate generating a salt of random length (credits to ircmaxell here) and either concatenate that to the beginning or end of the password hash. So you'll end up storing variable length values (96 - 128 characters) worth of meaninglessness (to outsiders):
$hash = hash('sha256', $pass);
$salt = substr(hash('sha256', mt_rand(0, 1337)), mt_rand(0, 31), 32);
$hash = $salt . hash('sha256', $salt . $hash . $pass);
One suggestion. Add the password into the salting hash run:
$hash = hash('sha256', $pass);
$salt = createSalt();
$hash = hash('sha256', $salt . $hash . $pass);
The reason is to avoid collisions (albeit with SHA-256
that's pretty unlikely). Let's say there is a string foo
that collides with your password bar
when run through sha256
... $hash
from the first round would be identical, so the second hash round will also produce identical results:
$hash = hash('sha256', 'foo'); // "test" for example
$hash = hash('sha256', 'bar'); // "test" since it's a collision
$newHash = hash('sha256', $salt . $hash); //The same for both foo and bar!
Whereas if you re-introduce the the password in the second round, it won't directly collide since the string is different for each hash round...
Edit: As far as salting, I'd recommend something like this (cross-platform):
function createSalt() {
$length = mt_rand(64, 128);
$salt = '';
for ($i = 0; $i < $length; $i++) {
$salt .= chr(mt_rand(33, 255));
}
return $salt;
}
It uses characters outside the normal range (much higher), but excludes the common control and whitespace characters that might be stripped by the database (or trimmed out). Note that it's returning a valid ISO-8859-1 (Latin-1) string. It's not a valid UTF-8 string. So make sure the character set of the column is good, or in the worst case change 255
to 127
in the code above (but this greatly reduces the strength of the salt)...
精彩评论