feat(utils) generate random strings via CSPRNG

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)