HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpModerate

Security of API Keygen for a cryptocurrency trading platform

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
platformcryptocurrencykeygensecuritytradingforapi

Problem

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 'Your Api Key is:';
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);

Solution

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:

-
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.



-
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:

  • Cryptographic hashes are one-way (irreversible) functions, so knowing the hash reveals nothing about the input that was hashed. (Unguessable)



  • The input varies by time and includes randomness, so it is not repeatable. (Distinct)



  • The input includes the user ID, so no two users should be able to generate the same key. (Distinct)



  • 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)



  • 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.

Code Snippets

$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);
$api_key = hash('sha256', (time() . $id . $some_sitewide_secret . rand()));

Context

StackExchange Code Review Q#38309, answer score: 17

Revisions (0)

No revisions yet.