feat(utils) generate random strings via CSPRNG

The current implementation of utils.random_string() leverages
the LuaJIT math.random() under the hood to generate bytes; this is
unsuitable for cases where cryptographically secure random data
is needed, such as fallback values for authentication plugin
secret values. To correct this, we introduce a wrapper around
the kernel CSPRNG (via /dev/urandom) to read random bytes, and
wrap utils.random_string around this. We also return these bytes
in a modified Base64 encoding (replacing non-alphanumeric characters
with random alphanumeric replacements); this serves to increase
the size of the keyspace significantly while attempting to
maintain some backwards compatibility with previous generated string
parameters (e.g. by generating a string of the same size and a
somewhat matching pattern).

The underlying get_rand_bytes implementation is modified to read
from /dev/urandom when explicitly requested, and falling back to
OpenSSL's RAND_bytes when reading from urandom fails. The blocking
read from urandom is acceptable when explicitly requested, as this
is typically done in asynchronous environments (e.g. admin API
requests), where the need for strong psuedorandomness outweighs
the overhead of I/O and talking to the kernel.

kong-0.10.3-0.rockspec

@@ -23,7 +23,7 @@ dependencies = {
   "lua-cassandra == 1.2.2",
   "pgmoon-mashape == 2.0.1",
   "luatz == 0.3",
-  "lua_system_constants == 0.1.1",
+  "lua_system_constants == 0.1.2",
   "lua-resty-iputils == 0.2.1",
   "luacrypto == 0.3.2",
   "luasyslog == 1.0.0",

kong/plugins/file-log/handler.lua

@@ -18,10 +18,7 @@ local oflags = bit.bor(O_WRONLY, O_CREAT, O_APPEND)
 local mode = bit.bor(S_IRUSR, S_IWUSR, S_IRGRP, S_IROTH)
 
 ffi.cdef[[
-int open(const char * filename, int flags, int mode);
 int write(int fd, const void * ptr, int numbytes);
-int close(int fd);
-char *strerror(int errnum);
 ]]
 
 -- fd tracking utility functions

kong/tools/utils.lua

@@ -43,6 +43,11 @@ void ERR_load_crypto_strings(void);
 void ERR_free_strings(void);
 
 const char *ERR_reason_error_string(unsigned long e);
+
+int open(const char * filename, int flags, int mode);
+size_t read(int fd, void *buf, size_t count);
+int close(int fd);
+char *strerror(int errnum);
 ]]
 
 local _M = {}
@@ -115,12 +120,57 @@ do
   end
 end
 
+local get_rand_bytes
+
 do
+  local ngx_log = ngx.log
+  local WARN    = ngx.WARN
+
+  local system_constants = require "lua_system_constants"
+  local O_RDONLY = system_constants.O_RDONLY()
   local bytes_buf_t = ffi.typeof "char[?]"
 
-  function _M.get_rand_bytes(n_bytes)
+  local function urandom_bytes(buf, size)
+    local fd = ffi.C.open("/dev/urandom", O_RDONLY, 0) -- mode is ignored
+    if fd < 0 then
+      ngx_log(WARN, "Error opening random fd: ",
+                    ffi_str(ffi.C.strerror(ffi.errno())))
+
+      return false
+    end
+
+    local res = ffi.C.read(fd, buf, size)
+    if res <= 0 then
+      ngx_log(WARN, "Error reading from urandom: ",
+                    ffi_str(ffi.C.strerror(ffi.errno())))
+
+      return false
+    end
+
+    if ffi.C.close(fd) ~= 0 then
+      ngx_log(WARN, "Error closing urandom: ",
+                    ffi_str(ffi.C.strerror(ffi.errno())))
+    end
+
+    return true
+  end
+
+  -- try to get n_bytes of CSPRNG data, first via /dev/urandom,
+  -- and then falling back to OpenSSL if necessary
+  get_rand_bytes = function(n_bytes, urandom)
     local buf = ffi_new(bytes_buf_t, n_bytes)
 
+    -- only read from urandom if we were explicitly asked
+    if urandom then
+      local rc = urandom_bytes(buf, n_bytes)
+
+      -- if the read of urandom was successful, we returned true
+      -- and buf is filled with our bytes, so return it as a string
+      if rc then
+        return ffi_str(buf, n_bytes)
+      end
+    end
+
     if C.RAND_bytes(buf, n_bytes) == 0 then
       -- get error code
       local err_code = C.ERR_get_error()
@@ -139,19 +189,39 @@ do
 
     return ffi_str(buf, n_bytes)
   end
-end
 
-local v4_uuid = uuid.generate_v4
+  _M.get_rand_bytes = get_rand_bytes
+end
 
 --- Generates a v4 uuid.
 -- @function uuid
 -- @return string with uuid
 _M.uuid = uuid.generate_v4
 
 --- Generates a random unique string
--- @return string  The random string (a uuid without hyphens)
-function _M.random_string()
-  return v4_uuid():gsub("-", "")
+-- @return string  The random string (a chunk of base64ish-encoded random bytes)
+do
+  local char = string.char
+  local rand = math.random
+  local encode_base64 = ngx.encode_base64
+
+  -- generate a random-looking string by retrieving a chunk of bytes and
+  -- replacing non-alphanumeric characters with random alphanumeric replacements
+  -- (we dont care about deriving these bytes securely)
+  -- this serves to attempt to maintain some backward compatibility with the
+  -- previous implementation (stripping a UUID of its hyphens), while significantly
+  -- expanding the size of the keyspace.
+  local function random_string()
+    -- get 24 bytes, which will return a 32 char string after encoding
+    -- this is done in attempt to maintain backwards compatibility as
+    -- much as possible while improving the strength of this function
+    return encode_base64(get_rand_bytes(24, true))
+           :gsub("/", char(rand(48, 57)))  -- 0 - 10
+           :gsub("+", char(rand(65, 90)))  -- A - Z
+           :gsub("=", char(rand(97, 122))) -- a - z
+  end
+
+  _M.random_string = random_string
 end
 
 local uuid_regex = "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$"

spec/01-unit/04-utils_spec.lua

@@ -135,9 +135,15 @@ describe("Utils", function()
     describe("random_string()", function()
       it("should return a random string", function()
         local first = utils.random_string()
-        assert.truthy(first)
-        assert.falsy(first:find("-"))
+        assert.is_string(first)
 
+        -- build the same length string as previous implementations
+        assert.equals(32, #first)
+
+        -- ensure we don't find anything that isnt alphanumeric
+        assert.not_matches("^[^%a%d]+$", first)
+
+        -- at some point in the universe this test will fail ;)
         local second = utils.random_string()
         assert.not_equal(first, second)
       end)

spec/03-plugins/04-file-log/01-log_spec.lua

@@ -65,7 +65,7 @@ describe("Plugin: file-log (log)", function()
   end)
 
   it("reopens file on each request", function()
-    local uuid1 = utils.random_string()
+    local uuid1 = utils.uuid()
 
     -- Making the request
     local res = assert(client:send({
@@ -86,7 +86,7 @@ describe("Plugin: file-log (log)", function()
     os.remove(FILE_LOG_PATH)
 
     -- Making the next request
-    local uuid2 = utils.random_string()
+    local uuid2 = utils.uuid()
     local res = assert(client:send({
       method = "GET",
       path = "/status/200",
@@ -97,7 +97,7 @@ describe("Plugin: file-log (log)", function()
     }))
     assert.res_status(200, res)
 
-    local uuid3 = utils.random_string()
+    local uuid3 = utils.uuid()
     local res = assert(client:send({
       method = "GET",
       path = "/status/200",

spec/03-plugins/05-syslog/01-log_spec.lua

@@ -71,7 +71,7 @@ describe("#ci Plugin: syslog (log)", function()
   end)
 
   local function do_test(host, expecting_same)
-    local uuid = utils.random_string()
+    local uuid = utils.uuid()
 
     local response = assert(client:send {
       method = "GET",

spec/03-plugins/17-jwt/02-api_spec.lua

@@ -151,7 +151,7 @@ describe("Plugin: jwt (API)", function()
         local json = assert.response(res).has.jsonbody()
         assert.string(json.secret)
         assert.equals(32, #json.secret)
-        assert.matches("^%x+$", json.secret)
+        assert.matches("^[%a%d]+$", json.secret)
       end)
     end)