chriskacerguis / codeigniter-restserver

A fully RESTful server implementation for CodeIgniter using one library, one config file and one controller.
MIT License
4.89k stars 2.86k forks source link

_generate_key() get a unreadeable string #535

Closed dansanti closed 9 years ago

dansanti commented 9 years ago

Hi! cloned last commit, and when a i use key controller, to make a new key, this new key is not readable, and a empty, or sometimes a few letters withs spaces was saved to database, and response get a status true, but key = null.

I used key controller in May , and it's works fine. Now debug echo, i noticed that $this->security->get_random_bytes(64); returns a unreadable string.

Manually i remove these line, and the hash options, and works fine.

What's supposed to response $this->security->get_random_bytes(64), a string like hash?, My be this was a CI3 Bugs.

Greetings..

dansanti commented 9 years ago

On CI Forum explains that get_random_bytes(64) no t return ASCII chars :

Not necessarily printable ASCII characters, to be more precise.

To make the result easier to work with (i.e. easy to copy into a config file), it may be converted to a hex string via bin2hex() and then unpacked via hex2bin() when used.

softwarespot commented 9 years ago

Why do you want it in a readable form e.g. ASCII characters only? Doesn't this diminish the randomness of it?

dansanti commented 9 years ago

the problem is, that result is used to authenticate and saved to database in that format, and when yo try to use it, key don't match. Key in that format can't be used.

softwarespot commented 9 years ago

Okay, so the issue isn't readability it's a script breaking change instead. Correct me if I am wrong, but I was under the impression that the key generation example was just an example? Your best best is to use your workaround, as we are using the correct approach now to generating a hash using CI3.

Look here for when the old way was change (do_hash is deprecated in CI3)

dansanti commented 9 years ago

Yea i know that it's example, the idea it's get it better, and if you dump the finally key stored, you noticed that it's practically blank, and no another change i make to this file that the last update, that change the deprecated hash method, i think,here be a missed line. (now i temporally set $salt to false to use hash)

kenjis commented 9 years ago

This must be bug. Because Key controller does not work.

How about this patch?

--- a/application/controllers/api/Key.php
+++ b/application/controllers/api/Key.php
@@ -213,7 +213,7 @@ class Key extends REST_Controller {
         do
         {
             // Generate a random salt
-            $salt = $this->security->get_random_bytes(64);
+            $salt = base_convert(bin2hex($this->security->get_random_bytes(64)), 16, 36);

             // If an error occurred, then fall back to the previous method
             if ($salt === FALSE)
softwarespot commented 9 years ago

A bug with CI or the library?

kenjis commented 9 years ago

@softwarespot A bug in sample controller.

[Edited]

softwarespot commented 9 years ago

Please submit a PR for @chriskacerguis to merge. Thanks

dansanti commented 9 years ago

Works fine ! thanks.

softwarespot commented 9 years ago

Doesn't this diminish the randomness though?

kenjis commented 9 years ago

@softwarespot

  1. $this->security->get_random_bytes(64) is the same code as original. So the same randomness.
  2. bin2hex() converts binary to hex (which is readable string) only.
  3. base_convert() converts a number between arbitrary bases. 16, 36 means converting base 16 (hex) to 36 (0-9,a-z).

Example:

<?php
foreach (range(0, 35) as $number) {
    echo base_convert($number, 10, 36).PHP_EOL;
}

Results:

0
1
2
3
4
5
6
7
8
9
a
b
c
d
e
f
g
h
i
j
k
l
m
n
o
p
q
r
s
t
u
v
w
x
y
z

But as PHP manual says "base_convert() may lose precision on large numbers due to properties related to the internal "double" or "float" type used." http://php.net/manual/en/function.base-convert.php, it surely loses precision. Because the number that $this->security->get_random_bytes(64) returns is like "3a1b22b6ea086d28ba6002c72e2355b3b1d204a2f06b1ab229b729b2a6abe203a2a359ac30427a88a3ad8f7bc815abd85b43adc86eb7086e2d83020059cd2aba" as hex. It is a very large number.

Doesn't this diminish the randomness though?

I'm not sure. Maybe it does a bit.

softwarespot commented 9 years ago

Thanks,