1

I am setting up a new PHP app and would like to learn to salt and secure user password. I am unsure about which step during registration I need to do this at. Also, do I need to change my login forms as well?

    if(isset($_POST['submit'])){
        //protect and then add the posted data to variables
        $username = protect($_POST['username']);
        $password = protect($_POST['password']);
        $passconf = protect($_POST['passconf']);
        $email = protect($_POST['email']);
        //check to see if any of the boxes were not filled in
        if(!$username || !$password || !$passconf || !$email){
            //if any weren't display the error message
            echo "<center>You need to fill in all of the required filds!</center>";
        }else{
            //if all were filled in continue checking
            //Check if the wanted username is more than 32 or less than 3 charcters long
            if(strlen($username) > 32 || strlen($username) < 3){
                //if it is display error message
                echo "<center>Your <b>Username</b> must be between 3 and 32 characters long!</center>";
            }else{
                //if not continue checking
                //select all the rows from out users table where the posted username matches the username stored
                $res = mysql_query("SELECT * FROM `users` WHERE `username` = '".$username."'");
                $num = mysql_num_rows($res);
                //check if theres a match
                if($num == 1){
                    //if yes the username is taken so display error message
                    echo  "<center>The <b>Username</b> you have chosen is already taken!</center>";
                }else{
                    //otherwise continue checking
                    //check if the password is less than 5 or more than 32 characters long
                    if(strlen($password) < 5 || strlen($password) > 32){
                        //if it is display error message
                        echo "<center>Your <b>Password</b> must be between 5 and 32 characters long!</center>";
                    }else{
                        //else continue checking
                        //check if the password and confirm password match
                        if($password != $passconf){
                            //if not display error message
                            echo "<center>The <b>Password</b> you supplied did not math the confirmation password!</center>";
                        }else{
                            //otherwise continue checking
                            //Set the format we want to check out email address against
                            $checkemail = "/^[a-z0-9]+([_\\.-][a-z0-9]+)*@([a-z0-9]+([\.-][a-z0-9]+)*)+\\.[a-z]{2,}$/i";
                            //check if the formats match
                            if(!preg_match($checkemail, $email)){
                                //if not display error message
                                echo "<center>The <b>E-mail</b> is not valid, must be name@server.tld!</center>";
                            }else{
                                //if they do, continue checking
                                //select all rows from our users table where the emails match
                                $res1 = mysql_query("SELECT * FROM `users` WHERE `email` = '".$email."'");
                                $num1 = mysql_num_rows($res1);
                                //if the number of matchs is 1
                                if($num1 == 1){
                                    //the email address supplied is taken so display error message
                                    echo "<center>The <b>E-mail</b> address you supplied is already taken</center>";
                                }else{
                                    //finally, otherwise register there account
                                    //time of register (unix)
                                    $registerTime = date('U');
                                    //make a code for our activation key
                                    $code = md5($username).$registerTime;
                                    //insert the row into the database
                                    $res2 = mysql_query("INSERT INTO `users` (`username`, `password`, `email`, `rtime`) VALUES('".$username."','".$password."','".$email."','".$registerTime."')");
                                    //send the email with an email containing the activation link to the supplied email address
user547794
  • 14,263
  • 36
  • 103
  • 152
  • 1
    possible duplicate of [Using hashing to safely store user passwords](http://stackoverflow.com/questions/2132887/using-hashing-to-safely-store-user-passwords) – Rob Jan 01 '11 at 21:20
  • 1
    First of all **get rid of your protect() function.** – Your Common Sense Jan 01 '11 at 21:21
  • 1
    As for your question, just store `md5($username.$password.$email)` in the password field and don't forget to regenerate it every time any of the paramers changed. You can add some reasonable iterations of md5, say 1000. Also check password for strongness. – Your Common Sense Jan 01 '11 at 21:25
  • 3
    As always MD5 is outdated (it is broken), you should use another hash algorithm. – Felix Kling Jan 01 '11 at 21:27
  • 1
    MD5 isn't really designed for password hashes :/ bcrypt, perhaps? – Matchu Jan 01 '11 at 21:27
  • @Felix no, it is not. it's okay for passwords – Your Common Sense Jan 01 '11 at 21:28
  • @Matchu however, it serves them well – Your Common Sense Jan 01 '11 at 21:29
  • @Col. Shrapnel: There is a reason why e.g. Mozilla does not use it anymore for hashing the user passwords. Broken is broken. I know you have a different opinion and I won't discuss this topic with you. – Felix Kling Jan 01 '11 at 21:30
  • @Felix: It's broken? Can you give me a link or so? – thejh Jan 01 '11 at 21:31
  • @thejh they have links but they don't have a proof. In fact, they merely do not understand texts behind their links. But it's scaring and mysterious indeed. So, everyone gets impressed. – Your Common Sense Jan 01 '11 at 21:32
  • @thejh: From the master himself: http://www.schneier.com/blog/archives/2005/03/more_hash_funct.html, http://www.schneier.com/blog/archives/2005/06/more_md5_collis.html and a paper: http://cryptography.hyperlink.cz/md5/MD5_collisions.pdf – Felix Kling Jan 01 '11 at 21:33
  • Broken MD5 report by US-CERT http://www.kb.cert.org/vuls/id/836068. Read the quote "Do not use the MD5 algorithm Software developers, Certification Authorities, website owners, and users should avoid using the MD5 algorithm in any capacity. As previous research has demonstrated, it should be considered cryptographically broken and unsuitable for further use." – Matt Asbury Jan 01 '11 at 21:35
  • 4
    @Col. Shrapnel: It is always interesting how you judge people you don't even know. Maybe you are the one who does not understand the significance of this? But of course Bruce Schneier must be wrong if he says that MD5 is broken and he probably does not know what he is talking about. – Felix Kling Jan 01 '11 at 21:35
  • 2
    While the ability to generate collisions is generally less of an issue for passwords (especially if the password field has a maximum length), MD5 is still not a cryptographic hash - the fact that it is a *fast* hash makes it less ideal for password applications, because fast means easier to brute force if a malicious entity ever gets their hands on the hashed version. Algorithms like SHA-1 are inherently *designed* to be slow, and thus take more time to brute-force. – Amber Jan 01 '11 at 21:38
  • 1
    @Felix I judge them by logic. How would a collision help you to recover or fake a salted password? – Your Common Sense Jan 01 '11 at 21:42
  • 1
    @Col. Shrapnel: What happens if someone gets somehow access to the usernames and hashed passwords? Big Badaboom! And even if this is very unlikely, just using another hash algorithm, that is known to be secure, from the beginning **costs nothing** and does **not hurt**. – Felix Kling Jan 01 '11 at 21:46
  • @Felix well, they can brute-force passwords. but collisions, of which you were talking about, has nothing to do here. So, strengthen passwords is the way to go. As for the speed - it can be compensated with number of iterations – Your Common Sense Jan 01 '11 at 21:50
  • @Felix nothing bad in using any other algorithm. I am just against mindless repeating of some rumors. – Your Common Sense Jan 01 '11 at 21:58
  • @Amber: I +1'd your comment because you touched on MD5's speed being an antifeature for password hashing---however, SHA-1 is still the wrong approach. (No general-purpose hash is good for use for password hashing.) To use something that's _really_ slow, Blowfish (or Eksblowfish, as used by BCrypt) is much closer to home. – C. K. Young Jan 01 '11 at 22:01
  • @Chris however it can't help with dictionary attack. So, strong passwords are still on demand. – Your Common Sense Jan 01 '11 at 22:10
  • @Col. Shrapnel: I'd like to think that requiring strong passwords is always a requirement. – C. K. Young Jan 01 '11 at 22:21
  • @Chris yet at the moment strong passwords of 8+ different case alphanum characters aren't bruteforceable as well, even with MD5, are they? :) – Your Common Sense Jan 01 '11 at 22:24
  • @Col. Shrapnel: I think people do routinely crack passwords that are hashed with one-shot MD5, but I'm not "in the (password cracking) trade", so I wouldn't know. Anyway, my point is that BCrypt will raise the password cracking bar by _several orders of magnitude_, and that alone makes it worth doing as a matter of routine. – C. K. Young Jan 01 '11 at 22:42
  • @Col. Shrapnel: For example, if doing a trial using one-shot MD5 takes 0.1 ms, and using an appropriately-slow BCrypt takes 100 ms, then trying 1 million passwords takes 100 seconds (about 1.5 minutes) and 100,000 seconds (over a day) respectively. – C. K. Young Jan 01 '11 at 22:44
  • @Chris still you need not a million nor billion but way more. – Your Common Sense Jan 01 '11 at 22:57
  • The appropriate method for securing passwords is simple: You have the user write the password on a slip of paper placed inside a sealed envelope, then hand it off to a Secret Service agent who puts it in a handcuffed briefcase. He then takes it and places the envelope in a mason jar on Funk & Wagnalls porch, where it will remain until noon that day. Then, the Mongol Hordes will drop by to pick it up and carry it to a top secret lab, where it will be salted with the entirety of War and Peace and a random string generated by 5 million bonobos. Only then, it will be inserted into the DB. – Phoenix Jan 01 '11 at 23:10

3 Answers3

3

You absolutely must read this article: Enough with the rainbow tables.

Summary: If you're not using BCrypt, you're doing it wrong. No ifs, no buts. (This also means that all the suggestions to use MD5 or SHA-1 or SHA-512 or anything else are wrong too.)

C. K. Young
  • 219,335
  • 46
  • 382
  • 435
0

As for when you do it, it should be sometime before you insert it into the DB but after you check it for errors.

Some suggestions though.

Instead of nesting the ifs during error checking so that if username fails, password doesn't get checked, and if password fails, passconf doesn't get checked try something like this:

$errors = array();

if(strlen($username) > 32 || strlen($username) < 3)
{
     $errors['username'] = "Username must be between 3 and 32 characters.";
}
else
{
     $res = mysql_query("SELECT * FROM `users` WHERE `username` = '".$username."'");
     $num = mysql_num_rows($res);
     if($num == 1)
     {
          $errors['username'] = "Username already exists!";
     }
}

if(strlen($password) < 5 || strlen($password) > 32)
{
    $errors['password'] = "Password must be between 5 and 32 characters.";
}
else if($password != $confpass)
{
    $errors['password'] = "Passwords do not match.";
}

etc. etc. etc. so that each field is checked and errors returned if there are any. Then you do something like this at the end:

if(!count($errors))  //or if(count($errors) == 0)
{
     //code to process login/registration/whatever  Do password hashing here.
}
else
{
     //There were errors, do something else
}

This way you get all errors, so you can tell the user everything that's wrong with their input at once, and the code isn't as deeply nested.

Also, the people having the flame war on what hashing algorithm to use above, just ignore them unless you're trying to create a US Government or Corporate application. No attackers will care enough to actually attack otherwise, unless your application gets popular enough to warrant an attack. It is important that you hash it in some way though.

Phoenix
  • 4,488
  • 1
  • 21
  • 13
  • Thanks, that makes a lot of sense. I'm still confused on one point though - how does my login scipt authenticate with a salted hash+password? I have seen a lot of example of how to create the salt, but how does a login know what credentials to accept? – user547794 Jan 02 '11 at 08:17
0

SECURITY IS HARD. Don't do it yourself but let the exports figure it out. You could read there specs/implementations(if open):

  • openid
  • google friend connect
  • facebook connect
  • twitter single sign in

just to name a few options.

Alfred
  • 60,935
  • 33
  • 147
  • 186