Skip to content

Commit

Permalink
Merge branch 'hotfix/urandom-seed'
Browse files Browse the repository at this point in the history
  • Loading branch information
thibaultcha committed Oct 28, 2016
2 parents 04523a9 + 06de2da commit 573577b
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 56 deletions.
45 changes: 34 additions & 11 deletions kong/api/routes/kong.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
local utils = require "kong.tools.utils"
local singletons = require "kong.singletons"

local sub = string.sub
local find = string.find
local pairs = pairs
local ipairs = ipairs
Expand All @@ -14,19 +15,40 @@ local lua_version = jit and jit.version or _VERSION
return {
["/"] = {
GET = function(self, dao, helpers)
local rows, err = dao.plugins:find_all()
if err then
return helpers.responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end
local distinct_plugins = {}
local prng_seeds = {}

local m = {}
for _, row in ipairs(rows) do
m[row.name] = true
do
local rows, err = dao.plugins:find_all()
if err then
return helpers.responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
end

local map = {}
for _, row in ipairs(rows) do
if not map[row.name] then
distinct_plugins[#distinct_plugins+1] = row.name
end
map[row.name] = true
end
end

local distinct_plugins = {}
for plugin_name in pairs(m) do
distinct_plugins[#distinct_plugins + 1] = plugin_name
do
local kong_shm = ngx.shared.kong
local shm_prefix = "pid: "
local keys, err = kong_shm:get_keys()
if not keys then
ngx.log(ngx.ERR, "could not get kong shm keys: ", err)
else
for i = 1, #keys do
if sub(keys[i], 1, #shm_prefix) == shm_prefix then
prng_seeds[keys[i]], err = kong_shm:get(keys[i])
if err then
ngx.log(ngx.ERR, "could not get PRNG seed from kong shm")
end
end
end
end
end

return helpers.responses.send_HTTP_OK {
Expand All @@ -42,7 +64,8 @@ return {
enabled_in_cluster = distinct_plugins
},
lua_version = lua_version,
configuration = singletons.configuration
configuration = singletons.configuration,
prng_seeds = prng_seeds,
}
end
},
Expand Down
5 changes: 3 additions & 2 deletions kong/cmd/init.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "kong.core.globalpatches"
require("kong.core.globalpatches")({cli = true})

math.randomseed()
local prng_seed = math.randomseed()

local pl_app = require "pl.lapp"
local log = require "kong.cmd.utils.log"
Expand Down Expand Up @@ -84,6 +84,7 @@ return function(args)
log.debug("ngx_lua: %s", ngx.config.ngx_lua_version)
log.debug("nginx: %s", ngx.config.nginx_version)
log.debug("Lua: %s", jit and jit.version or _VERSION)
log.debug("PRNG seed: %d", prng_seed)

xpcall(function() cmd_exec(args) end, function(err)
if not (args.v or args.vv) then
Expand Down
108 changes: 69 additions & 39 deletions kong/core/globalpatches.lua
Original file line number Diff line number Diff line change
@@ -1,43 +1,73 @@
local meta = require "kong.meta"
local randomseed = math.randomseed

_G._KONG = {
_NAME = meta._NAME,
_VERSION = meta._VERSION
}

local seed

--- Seeds the random generator, use with care.
-- Once - properly - seeded, this method is replaced with a stub
-- one. This is to enforce best-practises for seeding in ngx_lua,
-- and prevents third-party modules from overriding our correct seed
-- (many modules make a wrong usage of `math.randomseed()` by calling
-- it multiple times or do not use unique seed for Nginx workers).
--
-- This patched method will create a unique seed per worker process,
-- using a combination of both time and the worker's pid.
-- luacheck: globals math
_G.math.randomseed = function()
if not seed then
-- If we're in runtime nginx, we have multiple workers so we _only_
-- accept seeding when in the 'init_worker' phase.
-- That is because that phase is the earliest one before the
-- workers have a chance to process business logic, and because
-- if we'd do that in the 'init' phase, the Lua VM is not forked
-- yet and all workers would end-up using the same seed.
if not ngx.RESTY_CLI and ngx.get_phase() ~= "init_worker" then
error("math.randomseed() must be called in init_worker", 2)
end
return function(opts)
opts = opts or {}

do
local meta = require "kong.meta"

seed = ngx.time() + ngx.worker.pid()
ngx.log(ngx.DEBUG, "random seed: ", seed, " for worker nb ", ngx.worker.id(),
" (pid: ", ngx.worker.pid(), ")")
randomseed(seed)
else
ngx.log(ngx.DEBUG, "attempt to seed random number generator, but ",
"already seeded with ", seed)
_G._KONG = {
_NAME = meta._NAME,
_VERSION = meta._VERSION
}
end

return seed
do
local util = require "kong.tools.utils"
local seed
local randomseed = math.randomseed

_G.math.randomseed = function()
if not seed then
if not opts.cli and ngx.get_phase() ~= "init_worker" then
ngx.log(ngx.WARN, "math.randomseed() must be called in init_worker ",
"context\n", debug.traceback('', 2)) -- nil [message] arg doesn't work with level
end

local bytes, err = util.get_rand_bytes(8)
if bytes then
ngx.log(ngx.DEBUG, "seeding PRNG from OpenSSL RAND_bytes()")

local t = {}
for i = 1, #bytes do
local byte = string.byte(bytes, i)
t[#t+1] = byte
end
local str = table.concat(t)
if #str > 12 then
-- truncate the final number to prevent integer overflow,
-- since math.randomseed() could get cast to a platform-specific
-- integer with a different size and get truncated, hence, lose
-- randomness.
-- double-precision floating point should be able to represent numbers
-- without rounding with up to 15/16 digits but let's use 12 of them.
str = string.sub(str, 1, 12)
end
seed = tonumber(str)
else
ngx.log(ngx.ERR, "could not seed from OpenSSL RAND_bytes, seeding ",
"PRNG with time and worker pid instead (this can ",
"result to duplicated seeds): ", err)

seed = ngx.now()*1000 + ngx.worker.pid()
end

ngx.log(ngx.DEBUG, "random seed: ", seed, " for worker nb ",
ngx.worker.id())

if not opts.cli then
local ok, err = ngx.shared.kong:safe_set("pid: " .. ngx.worker.pid(), seed)
if not ok then
ngx.log(ngx.WARN, "could not store PRNG seed in kong shm: ", err)
end
end

randomseed(seed)
else
ngx.log(ngx.DEBUG, "attempt to seed random number generator, but ",
"already seeded with: ", seed, "\n",
debug.traceback('', 2)) -- nil [message] arg doesn't work with level
end

return seed
end
end
end
2 changes: 1 addition & 1 deletion kong/kong.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
-- |[[ ]]|
-- ==========

require "kong.core.globalpatches"
require("kong.core.globalpatches")()

local core = require "kong.core.handler"
local Serf = require "kong.serf"
Expand Down
3 changes: 2 additions & 1 deletion kong/templates/nginx_kong.lua
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ lua_package_cpath '${{LUA_PACKAGE_CPATH}};;';
lua_code_cache ${{LUA_CODE_CACHE}};
lua_max_running_timers 4096;
lua_max_pending_timers 16384;
lua_shared_dict kong 4m;
lua_shared_dict cache ${{MEM_CACHE_SIZE}};
lua_shared_dict reports_locks 100k;
lua_shared_dict cluster_locks 100k;
Expand Down Expand Up @@ -141,4 +142,4 @@ server {
return 200 'User-agent: *\nDisallow: /';
}
}
]]
]]
42 changes: 40 additions & 2 deletions kong/tools/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ local uuid = require "resty.jit-uuid"
local pl_stringx = require "pl.stringx"

local C = ffi.C
local ffi_new = ffi.new
local ffi_str = ffi.string
local fmt = string.format
local type = type
local pairs = pairs
Expand All @@ -27,7 +29,17 @@ local find = string.find
local gsub = string.gsub

ffi.cdef[[
typedef unsigned char u_char;

int gethostname(char *name, size_t len);

int RAND_bytes(u_char *buf, int num);

unsigned long ERR_get_error(void);
void ERR_load_crypto_strings(void);
void ERR_free_strings(void);

const char *ERR_reason_error_string(unsigned long e);
]]

local _M = {}
Expand All @@ -38,11 +50,11 @@ function _M.get_hostname()
local result
local SIZE = 128

local buf = ffi.new("unsigned char[?]", SIZE)
local buf = ffi_new("unsigned char[?]", SIZE)
local res = C.gethostname(buf, SIZE)

if res == 0 then
local hostname = ffi.string(buf, SIZE)
local hostname = ffi_str(buf, SIZE)
result = gsub(hostname, "%z+$", "")
else
local f = io.popen("/bin/hostname")
Expand All @@ -54,6 +66,32 @@ function _M.get_hostname()
return result
end

do
local bytes_buf_t = ffi.typeof "char[?]"

function _M.get_rand_bytes(n_bytes)
local buf = ffi_new(bytes_buf_t, n_bytes)

if C.RAND_bytes(buf, n_bytes) == 0 then
-- get error code
local err_code = C.ERR_get_error()
if err_code == 0 then
return nil, "could not get SSL error code from the queue"
end

-- get human-readable error string
C.ERR_load_crypto_strings()
local err = C.ERR_reason_error_string(err_code)
C.ERR_free_strings()

return nil, "could not get random bytes (" ..
"reason:" .. ffi_str(err) .. ") "
end

return ffi_str(buf, n_bytes)
end
end

local v4_uuid = uuid.generate_v4

--- Generates a v4 uuid.
Expand Down
13 changes: 13 additions & 0 deletions spec/02-integration/03-admin_api/01-kong_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ describe("Admin API", function()
assert.equal([[{"message":"Method not allowed"}]], body)
end
end)
it("returns PRNG seeds", function()
local res = assert(client:send {
method = "GET",
path = "/",
})
local body = assert.response(res).has.status(200)
local json = cjson.decode(body)
assert.is_table(json.prng_seeds)
for k, v in pairs(json.prng_seeds) do
assert.matches("pid: %d+", k)
assert.matches("%d+", k)
end
end)
end)
end)

Expand Down

0 comments on commit 573577b

Please sign in to comment.