Security of API Keygen for a cryptocurrency trading platform

Question

Here is an API key gen script for a cryptocurrency trading platform I am building.

First it checks to see if a key exists in the db for the user ID. If it does exist, it displays the key. If it doesn't, it creates one.

Next it checks to make sure the key is unique, by polling the db for identical keys. If one is found to be identical, the page is refreshed via meta refresh. If no key collision is found, it inserts the key in the database and refreshes the page via meta refresh. When the page is reloaded, there is now a key in the database for the user ID, and the key is displayed.

Any tips or feedback is greatly appreciated. I am a fairly inexperienced programmer and this is my first serious project.

require_once("models/config.php");
$id = $loggedInUser->user_id;
$api_select = mysql_query("SELECT * FROM userCake_Users WHERE `User_Id`='$id'");
while($row = mysql_fetch_assoc($api_select)) {
if($row["api_key"] !== null) {
    echo '<h3>Your Api Key is:</h3><br/>';
    echo $row["api_key"];
}else{
    $user = $row["Username"];
    $pass = $row["Password"];
    $length = 128;
    $time = date("F j, Y, g:i a");
    $salt1 = $time . hash('sha512', (sha1 .$time));
    $salt2 = substr(md5(uniqid(rand(), true)), 0, 25);
    $salt3 = substr(md5(uniqid(rand(), true)), 0, 25);
    $salt4 = hash('sha256', (md5 .$time));
    $user_hash = hash('sha512', ($salt2 . $user . $salt1));
    $pass_hash = hash('sha512', ($salt1 . $pass . $salt2));
    $keyhash_a = hash('sha512', ($user_hash . $salt3));
    $keyhash_b = hash('sha512', ($pass_hash . $salt4));
    $hash_a = str_split($keyhash_a);
    $hash_b = str_split($keyhash_b);
    foreach($hash_a as $key => $value) {
        $hashed_a[] = $salt2 . hash('sha512', ($salt3 . $value)) . $salt1 . hash('sha256', ($salt4 . $key));
    }
    foreach($hash_a as $key => $value) {
        $hashed_b[] = $salt2 . hash('sha512', ($salt3 . $value)) . $salt1 . hash('sha256', ($salt4 . $key));
    }
    $hash_merge = array_merge($hashed_b, $hashed_a);
    $from_merge = implode($hash_merge);
    $exploded_2 = str_split($from_merge);
    $key_hash_last = implode($exploded_2);
    $key0 = str_shuffle($key_hash_last);
    $key1 = str_split($key0);
    $key2 = array_unique($key1);
    $key3 = implode($key2);
    $key4 = str_shuffle($key3);
    $key5 = str_shuffle($key4);
    $api_key0 = str_shuffle($key3.$key4.$key5.$key2);
    $api_key_prepped = mysql_real_escape_string($api_key0);
    $api_check_no_collision = mysql_query("SELECT * FROM userCake_Users WHERE `api_key` = '$api_key_prepped'"); //check to see if an identical key exists
    if(mysql_num_rows($api_check_no_collision) > 0) {
        echo '<meta http-equiv="refresh" content="0; URL=index.php?page=api">';//an identical key exists in the database, refresh the page and generate a new key.
    }else{
    $api_insert = mysql_query("UPDATE  `testing`.`userCake_Users` SET  `api_key` =  '$api_key_prepped' WHERE  `userCake_Users`.`User_ID` ='$id';"); //the key is unique, submit it to the database.
    echo '<meta http-equiv="refresh" content="0; URL=index.php?page=api">';
    }
}
}

Answer 1

It feels like you are combining lots of hashing and string transformation in hopes that complexity will make your scheme more secure. That's not engineering or computer science, though — it's Cargo Cult Programming.

There are two ways that an API key scheme could work:

1. To generate the API key, concatenate the username with a salt and site-wide secret, then hash it. When it is later presented, verify it by concatenating the username, salt, and secret, and see if the hashes match.

   Advantages:

   • It may be possible to verify the API key without a database lookup.
   • You can immediately revoke all API keys ever issued by simply changing the site-wide secret.

   Disadvantages:

   • It's tricky to revoke the API key for a particular user without a database lookup.

2. Generate any distinct, unguessable string. Store it in the database, associated with the user. When it is later presented, verify it by looking it up in the database.

   Advantages:

   • It's easy to understand how it works.

   Disadvantages:

   • Verification requires a database lookup.

What you have done is a combination of the two strategies. You store the generated API key in the database, associated with the user. However, while you could have used just any random string, you used a convoluted process involving lots of randomizing, hashing, concatenating, and shuffling, all without much justification, and in a way that offers no additional security benefits.

Consider, for example:

$hash_merge = array_merge($hashed_b, $hashed_a);
$from_merge = implode($hash_merge);
$exploded_2 = str_split($from_merge);
$key_hash_last = implode($exploded_2);

That just converts an array to a string, back into an array, and reconstitutes the same string again.

If you consider the properties of cryptographic hash functions, you will notice that much of the manipulations are unnecessary. In particular, cryptographic hash functions are designed such that if inputs a and b differ by as little as one bit, hash(a) and hash(b) will not resemble each other at all.

Therefore, if you choose option (2), you can generate an API key securely using something as simple as

$api_key = hash('sha256', (time() . $id . $some_sitewide_secret . rand()));

The result will be secure because:

1. Cryptographic hashes are one-way (irreversible) functions, so knowing the hash reveals nothing about the input that was hashed. (Unguessable)
2. The input varies by time and includes randomness, so it is not repeatable. (Distinct)
3. The input includes the user ID, so no two users should be able to generate the same key. (Distinct)
4. The input includes some site-wide secret string (which you can set in your configuration file), so even if an attacker tried to generate a lot of keys using the same technique in an attempt to brute-force a match, knowing the approximate time at which the key was generated and even cracking the pseudorandom number generator would not help. Pick a very long random string for the site-wide secret (a SHA-256 of any word processing document would do). (Unguessable)
5. The output has 256 bits of entropy, which is impossible to crack by exhaustive enumeration. (Unguessable)

Furthermore, the chance that the same API key would be generated twice is about 2^-250. It's more likely that your server would be struck by an asteroid tomorrow than that it would generate the same key twice using this technique, so don't worry about accidental collisions.

Answer 2

An API key only have to be unique enough to avoid collison nothing more and here you are doing to much to achieve this. For example the rehashing will incrase the chance for a collison.

I have created a class for generating unique keys for example an API key usage or simply for primary key in a database table.

<?php

final class Guid {

    private static $_empty = array("00000000", "0000", "0000", "0000", "000000000000");

    private static $_parseFormats = array(
        "D" => "/^[a-f\d]{8}(-[a-f\d]{4}){4}[a-f\d]{8}$/i",
        "N" => "/^[a-f\d]{8}([a-f\d]{4}){4}[a-f\d]{8}$/i",
        "B" => "/^(\{)?[a-f\d]{8}(-[a-f\d]{4}){4}[a-f\d]{8}(?(1)\})$/i",
        "P" => "/^(\()?[a-f\d]{8}(-[a-f\d]{4}){4}[a-f\d]{8}(?(1)\))$/i",
        "X" => "/^(\{0x)[a-f\d]{8}((,0x)[a-f\d]{4}){2}(,\{0x)[a-f\d]{2}((,0x)[a-f\d]{2}){7}(\}\})$/i"
    );

    public static function NewGuid() {
        $data = openssl_random_pseudo_bytes(16);

        $data[6] = chr(ord($data[6]) & 0x0f | 0x40); // set version to 0010
        $data[8] = chr(ord($data[8]) & 0x3f | 0x80); // set bits 6-7 to 10

        $parts = str_split(bin2hex($data), 4);
        $guid = new Guid();
        $guid->_parts = array(
            $parts[0] . $parts[1],
            $parts[2],
            $parts[3],
            $parts[4],
            $parts[5] . $parts[6] . $parts[7]
        );

        return $guid;
    }

    public static function TryParse($asString, &$out_guid) {
        $out_guid = NULL;

        foreach (self::$_parseFormats as $format) {
            if (1 == preg_match($format, $asString)) {
                $clean = strtolower(str_replace(array("-", "{", "}", "(", ")", "0x", ","), "", $asString));
                $out_guid = new Guid();
                $out_guid->_parts = array(
                    substr($clean, 0, 8),
                    substr($clean, 8, 4),
                    substr($clean, 12, 4),
                    substr($clean, 16, 4),
                    substr($clean, 20, 12),
                );

                return true;
            }
        }

        return false;
    }

    public static function Parse($asString) {
        if (self::TryParse($asString, $out_guid)) {
            return $out_guid;
        }

        throw new Exception("Invalid Guid: " . $asString);
    }

    private $_parts;

    public function __construct() {
        $this->_parts = self::$_empty;
    }

    private static function _comparer(Guid $guid1, Guid $guid2) {
        return $guid1->_parts == $guid2->_parts;
    }

    public function Equals(ObjectBase $obj) {
        return self::_comparer($this, $obj);
    }

    public function ToString($format = NULL) {
        switch ($format) {
            case "";
            case "D";
                return implode("-", $this->_parts);
            case "N";
                return implode("", $this->_parts);
            case "B";
                return "{" . implode("-", $this->_parts) . "}";
            case "P";
                return "(" . implode("-", $this->_parts) . ")";
            case "X";
                $tmp = array(
                    "0x" . $this->_parts[0],
                    "0x" . $this->_parts[1],
                    "0x" . $this->_parts[2],
                    "{0x" . implode(",0x", str_split($this->_parts[3] . $this->_parts[4], 2)) . "}"
                );

                return "{" . implode(",", $tmp) . "}";
            default:
                throw new \Exception("Invalid Guid format" . $format);
        }
    }

    public function __toString() {
        return $this->ToString("D");
    }

}

The usage is easy:

$apiKey = Guid::NewGuid();

Yes you can have different formats if you like, the default __toString() call will result in something like:

f9168c5e-ceb2-4faa-b6bf-329bf39fa1e4

And you don't have to check for collisons becouse it has a really small chance to have two identical Guid generated with openssl_random_pseudo_bytes(). The only thing you have to have is a PHP installation with version 5.3.0 or higher but it's recommended anyway to have the most recent PHP version (now 5.5.7).