Security of API Keygen for a cryptocurrency trading platform











up vote
7
down vote

favorite
1












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">';
}
}
}









share|improve this question




















  • 1




    Three quick comments besides answering the actual question on security... First, mysql_query is deprecated and you should use newer implementations suggested here us2.php.net/mysql_query, unless you are using a much older PHP version. Second, is $id 'clean' enough to prevent SQL injection attacks in "...WHERE User_Id = '$id'"? Third, why will you want to force a page-refresh if there is a key collision, instead of just looping within the logic? In the extremely unlikely scenario of 10 key collisions, are you expecting the users to face 10 page refreshes as well?
    – h.j.k.
    Dec 30 '13 at 4:18










  • about mysql: i know. its faster to write it though and go back and change it later. about $id. yes, it comes from userCake1.4.2 standard function library. basically the system is built on top of userCake, with modified password hashing and extra functions. by default, all of userCake functions are run through stringent sanitization. Third, as i stated, i have < 6 months experience in php/mysql. i haven't figured out a way to loop back through yet without a page refresh. can you provide me an example would i just return or something, i'm not sure exactly how to accomplish it in php.
    – r3wt
    Dec 30 '13 at 4:47












  • I'd recommend something less ambitious than a cryptocurrency trading platform as a beginner project, unless you're doing it privately just for fun.
    – 200_success
    Dec 30 '13 at 10:16












  • there's a difference between a beginner project and "my first real project".
    – r3wt
    Dec 30 '13 at 19:27






  • 1




    @.r3wt I agree with @200_success that you're not ready for such a high risk project. Choose a project where a mistake doesn't mean that cryptocurrency worth thousands of USD get stolen.
    – CodesInChaos
    Jan 24 '14 at 12:30















up vote
7
down vote

favorite
1












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">';
}
}
}









share|improve this question




















  • 1




    Three quick comments besides answering the actual question on security... First, mysql_query is deprecated and you should use newer implementations suggested here us2.php.net/mysql_query, unless you are using a much older PHP version. Second, is $id 'clean' enough to prevent SQL injection attacks in "...WHERE User_Id = '$id'"? Third, why will you want to force a page-refresh if there is a key collision, instead of just looping within the logic? In the extremely unlikely scenario of 10 key collisions, are you expecting the users to face 10 page refreshes as well?
    – h.j.k.
    Dec 30 '13 at 4:18










  • about mysql: i know. its faster to write it though and go back and change it later. about $id. yes, it comes from userCake1.4.2 standard function library. basically the system is built on top of userCake, with modified password hashing and extra functions. by default, all of userCake functions are run through stringent sanitization. Third, as i stated, i have < 6 months experience in php/mysql. i haven't figured out a way to loop back through yet without a page refresh. can you provide me an example would i just return or something, i'm not sure exactly how to accomplish it in php.
    – r3wt
    Dec 30 '13 at 4:47












  • I'd recommend something less ambitious than a cryptocurrency trading platform as a beginner project, unless you're doing it privately just for fun.
    – 200_success
    Dec 30 '13 at 10:16












  • there's a difference between a beginner project and "my first real project".
    – r3wt
    Dec 30 '13 at 19:27






  • 1




    @.r3wt I agree with @200_success that you're not ready for such a high risk project. Choose a project where a mistake doesn't mean that cryptocurrency worth thousands of USD get stolen.
    – CodesInChaos
    Jan 24 '14 at 12:30













up vote
7
down vote

favorite
1









up vote
7
down vote

favorite
1






1





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">';
}
}
}









share|improve this 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">';
}
}
}






php mysql api cryptocurrency






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Jan 24 at 1:25









Jamal

30.2k11115226




30.2k11115226










asked Dec 30 '13 at 3:04









r3wt

307315




307315








  • 1




    Three quick comments besides answering the actual question on security... First, mysql_query is deprecated and you should use newer implementations suggested here us2.php.net/mysql_query, unless you are using a much older PHP version. Second, is $id 'clean' enough to prevent SQL injection attacks in "...WHERE User_Id = '$id'"? Third, why will you want to force a page-refresh if there is a key collision, instead of just looping within the logic? In the extremely unlikely scenario of 10 key collisions, are you expecting the users to face 10 page refreshes as well?
    – h.j.k.
    Dec 30 '13 at 4:18










  • about mysql: i know. its faster to write it though and go back and change it later. about $id. yes, it comes from userCake1.4.2 standard function library. basically the system is built on top of userCake, with modified password hashing and extra functions. by default, all of userCake functions are run through stringent sanitization. Third, as i stated, i have < 6 months experience in php/mysql. i haven't figured out a way to loop back through yet without a page refresh. can you provide me an example would i just return or something, i'm not sure exactly how to accomplish it in php.
    – r3wt
    Dec 30 '13 at 4:47












  • I'd recommend something less ambitious than a cryptocurrency trading platform as a beginner project, unless you're doing it privately just for fun.
    – 200_success
    Dec 30 '13 at 10:16












  • there's a difference between a beginner project and "my first real project".
    – r3wt
    Dec 30 '13 at 19:27






  • 1




    @.r3wt I agree with @200_success that you're not ready for such a high risk project. Choose a project where a mistake doesn't mean that cryptocurrency worth thousands of USD get stolen.
    – CodesInChaos
    Jan 24 '14 at 12:30














  • 1




    Three quick comments besides answering the actual question on security... First, mysql_query is deprecated and you should use newer implementations suggested here us2.php.net/mysql_query, unless you are using a much older PHP version. Second, is $id 'clean' enough to prevent SQL injection attacks in "...WHERE User_Id = '$id'"? Third, why will you want to force a page-refresh if there is a key collision, instead of just looping within the logic? In the extremely unlikely scenario of 10 key collisions, are you expecting the users to face 10 page refreshes as well?
    – h.j.k.
    Dec 30 '13 at 4:18










  • about mysql: i know. its faster to write it though and go back and change it later. about $id. yes, it comes from userCake1.4.2 standard function library. basically the system is built on top of userCake, with modified password hashing and extra functions. by default, all of userCake functions are run through stringent sanitization. Third, as i stated, i have < 6 months experience in php/mysql. i haven't figured out a way to loop back through yet without a page refresh. can you provide me an example would i just return or something, i'm not sure exactly how to accomplish it in php.
    – r3wt
    Dec 30 '13 at 4:47












  • I'd recommend something less ambitious than a cryptocurrency trading platform as a beginner project, unless you're doing it privately just for fun.
    – 200_success
    Dec 30 '13 at 10:16












  • there's a difference between a beginner project and "my first real project".
    – r3wt
    Dec 30 '13 at 19:27






  • 1




    @.r3wt I agree with @200_success that you're not ready for such a high risk project. Choose a project where a mistake doesn't mean that cryptocurrency worth thousands of USD get stolen.
    – CodesInChaos
    Jan 24 '14 at 12:30








1




1




Three quick comments besides answering the actual question on security... First, mysql_query is deprecated and you should use newer implementations suggested here us2.php.net/mysql_query, unless you are using a much older PHP version. Second, is $id 'clean' enough to prevent SQL injection attacks in "...WHERE User_Id = '$id'"? Third, why will you want to force a page-refresh if there is a key collision, instead of just looping within the logic? In the extremely unlikely scenario of 10 key collisions, are you expecting the users to face 10 page refreshes as well?
– h.j.k.
Dec 30 '13 at 4:18




Three quick comments besides answering the actual question on security... First, mysql_query is deprecated and you should use newer implementations suggested here us2.php.net/mysql_query, unless you are using a much older PHP version. Second, is $id 'clean' enough to prevent SQL injection attacks in "...WHERE User_Id = '$id'"? Third, why will you want to force a page-refresh if there is a key collision, instead of just looping within the logic? In the extremely unlikely scenario of 10 key collisions, are you expecting the users to face 10 page refreshes as well?
– h.j.k.
Dec 30 '13 at 4:18












about mysql: i know. its faster to write it though and go back and change it later. about $id. yes, it comes from userCake1.4.2 standard function library. basically the system is built on top of userCake, with modified password hashing and extra functions. by default, all of userCake functions are run through stringent sanitization. Third, as i stated, i have < 6 months experience in php/mysql. i haven't figured out a way to loop back through yet without a page refresh. can you provide me an example would i just return or something, i'm not sure exactly how to accomplish it in php.
– r3wt
Dec 30 '13 at 4:47






about mysql: i know. its faster to write it though and go back and change it later. about $id. yes, it comes from userCake1.4.2 standard function library. basically the system is built on top of userCake, with modified password hashing and extra functions. by default, all of userCake functions are run through stringent sanitization. Third, as i stated, i have < 6 months experience in php/mysql. i haven't figured out a way to loop back through yet without a page refresh. can you provide me an example would i just return or something, i'm not sure exactly how to accomplish it in php.
– r3wt
Dec 30 '13 at 4:47














I'd recommend something less ambitious than a cryptocurrency trading platform as a beginner project, unless you're doing it privately just for fun.
– 200_success
Dec 30 '13 at 10:16






I'd recommend something less ambitious than a cryptocurrency trading platform as a beginner project, unless you're doing it privately just for fun.
– 200_success
Dec 30 '13 at 10:16














there's a difference between a beginner project and "my first real project".
– r3wt
Dec 30 '13 at 19:27




there's a difference between a beginner project and "my first real project".
– r3wt
Dec 30 '13 at 19:27




1




1




@.r3wt I agree with @200_success that you're not ready for such a high risk project. Choose a project where a mistake doesn't mean that cryptocurrency worth thousands of USD get stolen.
– CodesInChaos
Jan 24 '14 at 12:30




@.r3wt I agree with @200_success that you're not ready for such a high risk project. Choose a project where a mistake doesn't mean that cryptocurrency worth thousands of USD get stolen.
– CodesInChaos
Jan 24 '14 at 12:30










2 Answers
2






active

oldest

votes

















up vote
16
down vote



accepted










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.






share|improve this answer



















  • 1




    That is a great explanation you put together, you made it very simple and easy to understand
    – bumperbox
    Jan 24 '14 at 21:59










  • +1, except for "a SHA-256 of any word processing document would do". If an attacker got access to your word processing documents, they could then find the key. It would be better to use a secure source of random numbers, such as /dev/random.
    – Gareth Rees
    Jan 28 '14 at 13:21


















up vote
5
down vote













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-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}$/i",
"N" => "/^[a-fd]{8}([a-fd]{4}){4}[a-fd]{8}$/i",
"B" => "/^({)?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)})$/i",
"P" => "/^(()?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)))$/i",
"X" => "/^({0x)[a-fd]{8}((,0x)[a-fd]{4}){2}(,{0x)[a-fd]{2}((,0x)[a-fd]{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).






share|improve this answer





















  • i disagree. why would you not check for collision? that's just lazy
    – r3wt
    Dec 30 '13 at 19:25










  • Generate with my code 1 billion identifier you will see.
    – Peter Kiss
    Dec 30 '13 at 19:36










  • Ok thank's Peter Kiss. i will consider using your method for creating tradekeys. they fit the bill perfectly for that, and i can use a similar scheme of polling the db to ensure uniqueness, just to be safe. this is a trading site, margin for error == NULL. i went with 200_success's answer. your's was very good, but his provided something a little more to my liking.
    – r3wt
    Dec 31 '13 at 1:58










  • Read UUID generation techniques and probability of duplicates. A properly generated UUID is so unlikely to collide that if you want to worry about collisions, you'll also want to worry about memory/CPU defects and cosmic rays making your computer behave nondeterministically.
    – 200_success
    Dec 31 '13 at 2:55










  • i would have used it if the class worked.
    – r3wt
    Jan 13 '14 at 9:42











Your Answer





StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");

StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});


}
});














draft saved

draft discarded


















StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f38309%2fsecurity-of-api-keygen-for-a-cryptocurrency-trading-platform%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown

























2 Answers
2






active

oldest

votes








2 Answers
2






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
16
down vote



accepted










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.






share|improve this answer



















  • 1




    That is a great explanation you put together, you made it very simple and easy to understand
    – bumperbox
    Jan 24 '14 at 21:59










  • +1, except for "a SHA-256 of any word processing document would do". If an attacker got access to your word processing documents, they could then find the key. It would be better to use a secure source of random numbers, such as /dev/random.
    – Gareth Rees
    Jan 28 '14 at 13:21















up vote
16
down vote



accepted










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.






share|improve this answer



















  • 1




    That is a great explanation you put together, you made it very simple and easy to understand
    – bumperbox
    Jan 24 '14 at 21:59










  • +1, except for "a SHA-256 of any word processing document would do". If an attacker got access to your word processing documents, they could then find the key. It would be better to use a secure source of random numbers, such as /dev/random.
    – Gareth Rees
    Jan 28 '14 at 13:21













up vote
16
down vote



accepted







up vote
16
down vote



accepted






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.






share|improve this answer














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.







share|improve this answer














share|improve this answer



share|improve this answer








edited Jan 24 '14 at 20:56

























answered Dec 30 '13 at 9:44









200_success

127k15149412




127k15149412








  • 1




    That is a great explanation you put together, you made it very simple and easy to understand
    – bumperbox
    Jan 24 '14 at 21:59










  • +1, except for "a SHA-256 of any word processing document would do". If an attacker got access to your word processing documents, they could then find the key. It would be better to use a secure source of random numbers, such as /dev/random.
    – Gareth Rees
    Jan 28 '14 at 13:21














  • 1




    That is a great explanation you put together, you made it very simple and easy to understand
    – bumperbox
    Jan 24 '14 at 21:59










  • +1, except for "a SHA-256 of any word processing document would do". If an attacker got access to your word processing documents, they could then find the key. It would be better to use a secure source of random numbers, such as /dev/random.
    – Gareth Rees
    Jan 28 '14 at 13:21








1




1




That is a great explanation you put together, you made it very simple and easy to understand
– bumperbox
Jan 24 '14 at 21:59




That is a great explanation you put together, you made it very simple and easy to understand
– bumperbox
Jan 24 '14 at 21:59












+1, except for "a SHA-256 of any word processing document would do". If an attacker got access to your word processing documents, they could then find the key. It would be better to use a secure source of random numbers, such as /dev/random.
– Gareth Rees
Jan 28 '14 at 13:21




+1, except for "a SHA-256 of any word processing document would do". If an attacker got access to your word processing documents, they could then find the key. It would be better to use a secure source of random numbers, such as /dev/random.
– Gareth Rees
Jan 28 '14 at 13:21












up vote
5
down vote













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-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}$/i",
"N" => "/^[a-fd]{8}([a-fd]{4}){4}[a-fd]{8}$/i",
"B" => "/^({)?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)})$/i",
"P" => "/^(()?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)))$/i",
"X" => "/^({0x)[a-fd]{8}((,0x)[a-fd]{4}){2}(,{0x)[a-fd]{2}((,0x)[a-fd]{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).






share|improve this answer





















  • i disagree. why would you not check for collision? that's just lazy
    – r3wt
    Dec 30 '13 at 19:25










  • Generate with my code 1 billion identifier you will see.
    – Peter Kiss
    Dec 30 '13 at 19:36










  • Ok thank's Peter Kiss. i will consider using your method for creating tradekeys. they fit the bill perfectly for that, and i can use a similar scheme of polling the db to ensure uniqueness, just to be safe. this is a trading site, margin for error == NULL. i went with 200_success's answer. your's was very good, but his provided something a little more to my liking.
    – r3wt
    Dec 31 '13 at 1:58










  • Read UUID generation techniques and probability of duplicates. A properly generated UUID is so unlikely to collide that if you want to worry about collisions, you'll also want to worry about memory/CPU defects and cosmic rays making your computer behave nondeterministically.
    – 200_success
    Dec 31 '13 at 2:55










  • i would have used it if the class worked.
    – r3wt
    Jan 13 '14 at 9:42















up vote
5
down vote













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-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}$/i",
"N" => "/^[a-fd]{8}([a-fd]{4}){4}[a-fd]{8}$/i",
"B" => "/^({)?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)})$/i",
"P" => "/^(()?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)))$/i",
"X" => "/^({0x)[a-fd]{8}((,0x)[a-fd]{4}){2}(,{0x)[a-fd]{2}((,0x)[a-fd]{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).






share|improve this answer





















  • i disagree. why would you not check for collision? that's just lazy
    – r3wt
    Dec 30 '13 at 19:25










  • Generate with my code 1 billion identifier you will see.
    – Peter Kiss
    Dec 30 '13 at 19:36










  • Ok thank's Peter Kiss. i will consider using your method for creating tradekeys. they fit the bill perfectly for that, and i can use a similar scheme of polling the db to ensure uniqueness, just to be safe. this is a trading site, margin for error == NULL. i went with 200_success's answer. your's was very good, but his provided something a little more to my liking.
    – r3wt
    Dec 31 '13 at 1:58










  • Read UUID generation techniques and probability of duplicates. A properly generated UUID is so unlikely to collide that if you want to worry about collisions, you'll also want to worry about memory/CPU defects and cosmic rays making your computer behave nondeterministically.
    – 200_success
    Dec 31 '13 at 2:55










  • i would have used it if the class worked.
    – r3wt
    Jan 13 '14 at 9:42













up vote
5
down vote










up vote
5
down vote









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-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}$/i",
"N" => "/^[a-fd]{8}([a-fd]{4}){4}[a-fd]{8}$/i",
"B" => "/^({)?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)})$/i",
"P" => "/^(()?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)))$/i",
"X" => "/^({0x)[a-fd]{8}((,0x)[a-fd]{4}){2}(,{0x)[a-fd]{2}((,0x)[a-fd]{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).






share|improve this answer












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-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}$/i",
"N" => "/^[a-fd]{8}([a-fd]{4}){4}[a-fd]{8}$/i",
"B" => "/^({)?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)})$/i",
"P" => "/^(()?[a-fd]{8}(-[a-fd]{4}){4}[a-fd]{8}(?(1)))$/i",
"X" => "/^({0x)[a-fd]{8}((,0x)[a-fd]{4}){2}(,{0x)[a-fd]{2}((,0x)[a-fd]{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).







share|improve this answer












share|improve this answer



share|improve this answer










answered Dec 30 '13 at 9:32









Peter Kiss

2,3131717




2,3131717












  • i disagree. why would you not check for collision? that's just lazy
    – r3wt
    Dec 30 '13 at 19:25










  • Generate with my code 1 billion identifier you will see.
    – Peter Kiss
    Dec 30 '13 at 19:36










  • Ok thank's Peter Kiss. i will consider using your method for creating tradekeys. they fit the bill perfectly for that, and i can use a similar scheme of polling the db to ensure uniqueness, just to be safe. this is a trading site, margin for error == NULL. i went with 200_success's answer. your's was very good, but his provided something a little more to my liking.
    – r3wt
    Dec 31 '13 at 1:58










  • Read UUID generation techniques and probability of duplicates. A properly generated UUID is so unlikely to collide that if you want to worry about collisions, you'll also want to worry about memory/CPU defects and cosmic rays making your computer behave nondeterministically.
    – 200_success
    Dec 31 '13 at 2:55










  • i would have used it if the class worked.
    – r3wt
    Jan 13 '14 at 9:42


















  • i disagree. why would you not check for collision? that's just lazy
    – r3wt
    Dec 30 '13 at 19:25










  • Generate with my code 1 billion identifier you will see.
    – Peter Kiss
    Dec 30 '13 at 19:36










  • Ok thank's Peter Kiss. i will consider using your method for creating tradekeys. they fit the bill perfectly for that, and i can use a similar scheme of polling the db to ensure uniqueness, just to be safe. this is a trading site, margin for error == NULL. i went with 200_success's answer. your's was very good, but his provided something a little more to my liking.
    – r3wt
    Dec 31 '13 at 1:58










  • Read UUID generation techniques and probability of duplicates. A properly generated UUID is so unlikely to collide that if you want to worry about collisions, you'll also want to worry about memory/CPU defects and cosmic rays making your computer behave nondeterministically.
    – 200_success
    Dec 31 '13 at 2:55










  • i would have used it if the class worked.
    – r3wt
    Jan 13 '14 at 9:42
















i disagree. why would you not check for collision? that's just lazy
– r3wt
Dec 30 '13 at 19:25




i disagree. why would you not check for collision? that's just lazy
– r3wt
Dec 30 '13 at 19:25












Generate with my code 1 billion identifier you will see.
– Peter Kiss
Dec 30 '13 at 19:36




Generate with my code 1 billion identifier you will see.
– Peter Kiss
Dec 30 '13 at 19:36












Ok thank's Peter Kiss. i will consider using your method for creating tradekeys. they fit the bill perfectly for that, and i can use a similar scheme of polling the db to ensure uniqueness, just to be safe. this is a trading site, margin for error == NULL. i went with 200_success's answer. your's was very good, but his provided something a little more to my liking.
– r3wt
Dec 31 '13 at 1:58




Ok thank's Peter Kiss. i will consider using your method for creating tradekeys. they fit the bill perfectly for that, and i can use a similar scheme of polling the db to ensure uniqueness, just to be safe. this is a trading site, margin for error == NULL. i went with 200_success's answer. your's was very good, but his provided something a little more to my liking.
– r3wt
Dec 31 '13 at 1:58












Read UUID generation techniques and probability of duplicates. A properly generated UUID is so unlikely to collide that if you want to worry about collisions, you'll also want to worry about memory/CPU defects and cosmic rays making your computer behave nondeterministically.
– 200_success
Dec 31 '13 at 2:55




Read UUID generation techniques and probability of duplicates. A properly generated UUID is so unlikely to collide that if you want to worry about collisions, you'll also want to worry about memory/CPU defects and cosmic rays making your computer behave nondeterministically.
– 200_success
Dec 31 '13 at 2:55












i would have used it if the class worked.
– r3wt
Jan 13 '14 at 9:42




i would have used it if the class worked.
– r3wt
Jan 13 '14 at 9:42


















draft saved

draft discarded




















































Thanks for contributing an answer to Code Review Stack Exchange!


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


Use MathJax to format equations. MathJax reference.


To learn more, see our tips on writing great answers.





Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


Please pay close attention to the following guidance:


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


To learn more, see our tips on writing great answers.




draft saved


draft discarded














StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f38309%2fsecurity-of-api-keygen-for-a-cryptocurrency-trading-platform%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown





















































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown

































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown







Popular posts from this blog

Morgemoulin

Scott Moir

Souastre