One way encoding a password

Question

I wrote a script that one way encrypts a user's password by generating a key, and multiplies by the ASCII value of the character and the ASCII value of the key character at (the position that the character has in the password string) and repeats this for the previous positions in the string meaning a key string with ASCII values of:

49, 50, 51, 52, 53, 54, 55, 56, 57, 48

and a password string of

112, 97, 115, 115, 119, 111, 114, 100, 46, 49

Would be multiplied like:

[(112 * 49)] + [(97 * 50) + (97 * 49)] + ...

and so on.

As an example, the string password.1 encrypted with the key 1234567890 (auto generated) would return 203456.

$pass = $_GET["pass"];
$key = $_GET['key'];
$newPass = go($pass, $key);
echo $newPass;
function go($pass, $key){
    $passArray = str_split($pass);
    $keyArray = str_split($key);

    $total = 0;
    $pos = 0;
    foreach ($passArray as $char){
        $posInitial = $pos;
        while($pos !== 0){
            $total = $total + (ord($keyArray[$pos]) * ord($char));
            $pos--;
        }
        $pos = $posInitial + 1;
     }
     return $total;
}
function authorise($pass, $passEncode, $key){
    $passEncodeTest = go($pass, $key);
    if ($passEncode == $passEncodeTest){
        return true;
    } else {
        return false;
    }
}
authorise('password.2', $newPass, $key); ==> returns false
authorise('password.1', $newPass, $key); ==> returns true

(I will add a password sanitiser in later, but for now, plaintext)

My question is, what can I do to improve this code?

Answer 1

Reinventing the wheel

Please note that encryption should, in principle, always be two way. A 'one way encryption' just sounds weird. Normally something is encrypted with the intention to decrypt it at a later stage. This is not what you do. What you do is called hashing or perhaps 'encoding' as you indicated in your title.

You could improve the formatting of your code. I really miss the empty lines before and after the functions, and a space before { in a function. Your code is also not working because of the ==> in it at the end. This should be // ==>. Is is just nice if your code is working like published here.

The naming of your variables seems somewhat sloppy. In the beginning you call an encoded password $newPass. Is it a new password? I don't think so. Then in the authorise() function you rename it correctly to $passEncode, by which you probably mean $passEncoded.

So the big question is, why do you define your own hashing routine? Is it any better than the existing routines? PHP offers many of them!

See this link to tutsplus which addresses some issues that are encountered in hashing passwords. Is your routine resistent to all these issues? I don't think so.

It is far better to use a good existing password hashing library that is present in PHP. Have a look here: http://php.net/manual/en/faq.passwords.php

There's a lot of information about hashing and PHP out there. Find it and use it. I do really support new and inventive ways to solve problems but in this case it is definately not a good idea to reinvent the wheel.