#1
  1. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Aug 2012
    Location
    Burb of Detroit, Michigan
    Posts
    81
    Rep Power
    76

    I think this is a secure way of doing?


    All day long I've been checking out php.net crypt and other resources on the internet trying to write a better registration/login script. I think I'm close, but I want the guru's opinions on the following script:

    PHP Code:
    <?php
    class GenerateCryptedPassword {
        
        protected 
    $salt;
            
        public function 
    createHashedPassword($password) {
         
          if (
    CRYPT_SHA512 != 1) {
            throw new 
    Exception('Hashing mechanism not supported.');
          }
                 
         
    // For CRYPT_SHA512, but it should give you an idea of how crypt() works.
         
    $this->salt uniqid(); // Could use the second parameter to give it more entropy.
         
    $Algo '6'// This is CRYPT_SHA512 as shown on http://php.net/crypt
         
    $Rounds '5000'// The more, the more secure it is!
         
         // This is the "salt" string we give to crypt().
         
    $CryptSalt '$' $Algo '$rounds=' $Rounds '$' $this->salt
         return 
    $HashedPassword crypt($password$CryptSalt);        
        }
        
      
        
        public function 
    validate($pass$HashedPassword) {
         
          if (
    CRYPT_SHA512 != 1) {
            throw new 
    Exception('Hashing mechanism not supported.');
          }
         
          return (
    crypt($pass$HashedPassword) == $HashedPassword) ? true false;
         
        }
        
    }
    I spent all day monkeying around with script, I never knew it would be that hard. Anyways, I hope I'm on the right track, I believe I have to make a better random salt?

    John
  2. #2
  3. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,904
    Rep Power
    1045
    Hi,

    why didn't you simply use one of the great password libraries?

    For modern PHP, there's the excellent password_compat, which has the nice feature of being compatible with the new password API in PHP 5.5. So as soon as PHP 5.5 is out, you can simply remove the library and use the native functions without having to change the code. For legacy PHP, there's PHPass.

    As a layman (which we all are), you should never fumble with low-level cryptography. Functions like crypt() aren't meant to be used directly in application code, they're written for password libraries. Trying to use the "raw" function is very risky, because there's a lot of mistakes you can make. You can easily break the whole thing.

    For example, you totally forgot the error checking, so you cannot even tell what you're storing and comparing there. Could be some garbage strings. The underlying hash algorithm is not secure (even if this is just a demo). The salt is not secure (as you already mentioned). Maybe there are other issues -- I don't know, because like all of us, I'm not a cryptographer.

    I strongly suggest getting away from home-made functions and using an established and well-tested library like the ones above. They've been written by people who know this stuff, and they've proven themselves in reality many time. So you can be pretty sure they actually work. That's not even remotely true for home-made code. Even if we told you that everything is fine, that wouldn't mean a thing.

    I think one of the most important qualities of a good developer is that they know their limits and know when to use a library. Cryptography is something we should keep our hands off and leave to the experts -- just like we leave heart surgeries to actual doctors. I mean, when you're having a lot of trouble with the implementation, isn't that already a warning sign?

    Comments on this post

    • Strider64 agrees : Thanks for pointing me in the right direction...
  4. #3
  5. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Aug 2012
    Location
    Burb of Detroit, Michigan
    Posts
    81
    Rep Power
    76
    This is exactly why I posted this and password.php looks simple to use. In the back of my mind I knew something was wrong spending the whole day trying to figure it out, which is another reason why I probably posted this. I learn my lesson a while back ago, never post anything live before making sure. That's why I love having a local server to do all the testing. Thanks once again.

    Too funny I spent all day yesterday and all I had to do was require one file and do this:

    PHP Code:
    $data['pass'] = password_hash($passwordPASSWORD_BCRYPT, array("cost" => 15)); 

    Comments on this post

    • Jacques1 agrees

IMN logo majestic logo threadwatch logo seochat tools logo