Security of API Keygen for a cryptocurrency trading platform
up vote
7
down vote
favorite
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
|
show 2 more comments
up vote
7
down vote
favorite
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
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 fromuserCake1.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
|
show 2 more comments
up vote
7
down vote
favorite
up vote
7
down vote
favorite
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
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
php mysql api cryptocurrency
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 fromuserCake1.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
|
show 2 more comments
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 fromuserCake1.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
|
show 2 more comments
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:
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.
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
add a comment |
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).
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
add a comment |
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:
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.
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
add a comment |
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:
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.
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
add a comment |
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:
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.
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.
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
add a comment |
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
add a comment |
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).
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
add a comment |
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).
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
add a comment |
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).
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).
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
add a comment |
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
add a comment |
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.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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
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 fromuserCake1.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