4
\$\begingroup\$

I googled around for secure random number generation and random string generation, combining them with some user data.

Is this good or am I totally off-base? I don't know much about cryptography but I do not see many alternatives, other than some bad code that gets copied around a lot with mt_rand and uniqid.

//csrf tokens
    public function csrf_token($regen = false)
    {
        if($regen === true) {
            //we need to give the user a token
            if(isset($_SESSION["__csrf_token"])) {
                unset($_SESSION["__csrf_token"]);
            }
            $max = mt_rand(0, mt_getrandmax());
            $rand_num = floor($max*(hexdec(bin2hex(openssl_random_pseudo_bytes(4)))/0xffffffff));
            $rand_string = "";
            for($i=0; $i < 11; $i++) {
                $x = mt_rand(0, 2);
                switch($x) {
                    case 0: $rand_string.= chr(mt_rand(97,122));break;
                    case 1: $rand_string.= chr(mt_rand(65,90));break;
                    case 2: $rand_string.= chr(mt_rand(48,57));break;
                }
            }

            $_SESSION["__csrf_token"] = hash('whirlpool', $rand_num . $this->username . $rand_string . $this->hash_pw);
            $this->csrf_token = $_SESSION["__csrf_token"];
            return $this->csrf_token;
        }else{
            //the user already has a token
            return $this->csrf_token;
        }
    }
Improve this question
\$\endgroup\$
  • 1
    \$\begingroup\$ To me, it looks like it's doing too much. I'd just generate 16 random bytes, convert them to hex, and be done with it. \$\endgroup\$ – icktoofay Apr 13 '14 at 1:06
  • \$\begingroup\$ thanks icktoofay. you're probably right. i will consider this when refactoring the code. thank you for your time. \$\endgroup\$ – r3wt Apr 13 '14 at 1:34
3
\$\begingroup\$

If CSRF stands for Cross Site Request Forgery, then it's hard to imagine why I should help.

In any case, simply doing a cryptographic Whirlpool hash of a user-supplied string with a random seed value should be sufficiently random for most every purpose. The rest is just obfuscation and doesn't add to security.

Improve this answer
\$\endgroup\$
  • \$\begingroup\$ I appreciate your reply. i am not the sharpest knife in the drawer and it is hard for me to decide how much is good enough. so what i infer from your post, i should just hash the hash_pw(user supplied string) with a random number(Random seed), or should i hash it with the random string as well. \$\endgroup\$ – r3wt Apr 13 '14 at 1:33
  • \$\begingroup\$ Related meta post on malicious code. \$\endgroup\$ – syb0rg Apr 13 '14 at 2:36
  • \$\begingroup\$ @syb0rg how is that related? \$\endgroup\$ – r3wt Apr 13 '14 at 6:09
  • \$\begingroup\$ @r3wt It was more related to Edward's comment about why he found it hard to help. \$\endgroup\$ – syb0rg Apr 13 '14 at 16:18
  • 1
    \$\begingroup\$ i'm lost.. what about my code is malicious? \$\endgroup\$ – r3wt Apr 13 '14 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.