Skip to content

Commit

Permalink
feat(conf): safe base64 and tests
Browse files Browse the repository at this point in the history 
* base64 conversion is moved later in the flow in order to make
  it safer, so that values like system are not attempted to
  be decoded
* test coverage for the content of the created files
* refactoring
  • Loading branch information
@samugi @bungle
samugi authored and bungle committed Oct 4, 2022
1 parent 23ff6bd commit c15556f
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 97 deletions.
46 changes: 27 additions & 19 deletions kong/cmd/utils/prefix_handler.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,12 +465,17 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
end
end

-- create certs files and assign paths if needed
-- create certs files and assign paths when they are passed as content
do

local function propagate_dhparam(path)
kong_config["nginx_http_ssl_dhparam"] = path
kong_config["nginx_stream_ssl_dhparam"] = path
local function set_dhparam_path(path)
if kong_config["nginx_http_ssl_dhparam"] then
kong_config["nginx_http_ssl_dhparam"] = path
end

if kong_config["nginx_stream_ssl_dhparam"] then
kong_config["nginx_stream_ssl_dhparam"] = path
end

for _, directive in ipairs(kong_config["nginx_http_directives"]) do
if directive.name == "ssl_dhparam" and directive.value then
Expand All @@ -496,7 +501,9 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
})
end

local function write_file_set_path(
-- ensure the property value is a "content" (not a path),
-- write the content to a file and set the path in the configuration
local function write_content_set_path(
contents,
format,
write_func,
Expand All @@ -513,11 +520,11 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
write_func(path, contents)
kong_config[config_key] = path
if target == "ssl-dhparam" then
propagate_dhparam(path)
set_dhparam_path(path)
end
end

else
elseif type(contents) == "table" then
for i, content in ipairs(contents) do
if not exists(content) then
if not exists(ssl_path) then
Expand All @@ -531,17 +538,16 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
end
end

local ssl_path = join(kong_config.prefix, "ssl")
for _, target in ipairs({
"proxy",
"admin",
"status",
"client",
"cluster",
"ssl-dhparam",
"lua-ssl-trusted",
"cluster-ca"
}) do
local ssl_path = join(kong_config.prefix, "ssl")
local cert_name
local key_name
local ssl_cert
Expand All @@ -555,33 +561,35 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
key_name = target .. "_cert_key"
elseif target == "cluster-ca" then
cert_name = "cluster_ca_cert"
elseif target == "ssl-dhparam" then
cert_name = "ssl_dhparam"
elseif target == "lua-ssl-trusted" then
cert_name = "lua_ssl_trusted_certificate"
else
cert_name = target .. "_ssl_cert"
key_name = target .. "_ssl_cert_key"
end

if cert_name and (cert_name ~= "ssl_dhparam" or
not is_predefined_dhgroup(kong_config[cert_name])) then
ssl_cert = kong_config[cert_name]
end
ssl_cert = cert_name and kong_config[cert_name]
ssl_key = key_name and kong_config[key_name]

if ssl_cert and #ssl_cert > 0 then
write_file_set_path(ssl_cert, ".crt", write_ssl_cert, ssl_path, target,
cert_name)
write_content_set_path(ssl_cert, ".crt", write_ssl_cert, ssl_path,
target, cert_name)
end

if ssl_key and #ssl_key > 0 then
write_file_set_path(ssl_key, ".key", write_ssl_cert_key, ssl_path,
target, key_name)
write_content_set_path(ssl_key, ".key", write_ssl_cert_key, ssl_path,
target, key_name)
end
end

local dhparam_value = kong_config["ssl_dhparam"]
if dhparam_value and not is_predefined_dhgroup(dhparam_value) then
write_content_set_path(dhparam_value, ".pem", write_ssl_cert, ssl_path,
"ssl-dhparam", "ssl_dhparam")
end
end


if kong_config.lua_ssl_trusted_certificate_combined then
gen_trusted_certs_combined_file(
kong_config.lua_ssl_trusted_certificate_combined,
Expand Down
83 changes: 42 additions & 41 deletions kong/conf_loader/init.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,30 +625,32 @@ local function infer_value(value, typ, opts)
end


local function try_base64_decode(vals)
-- names that we should not attempt decoding
local decode_blacklist = {
system = "system"
}
local function decode_base64_str(str)
if type(str) == "string" then
return decode_base64(str)
or decode_base64url(str)
or nil, "base64 decoding failed: invalid input"

if type(vals) == "table" then
for i, v in ipairs(vals) do
vals[i] = decode_blacklist[v]
or decode_base64(v)
or decode_base64url(v)
or v
else
return nil, "base64 decoding failed: not a string"
end
end


local function try_decode_base64(value)
if type(value) == "table" then
for i, v in ipairs(value) do
value[i] = decode_base64_str(v) or v
end
return vals

return value
end

if type(vals) == "string" then
return decode_blacklist[vals]
or decode_base64(vals)
or decode_base64url(vals)
or vals
if type(value) == "string" then
return decode_base64_str(value) or value
end

return vals
return value
end


Expand Down Expand Up @@ -676,25 +678,6 @@ local function check_and_infer(conf, opts)
conf[k] = value
end

-- decode base64 for supported properties
for cert_name, has_key in pairs({
ssl_cert = true,
admin_ssl_cert = true,
status_ssl_cert = true,
client_ssl_cert = true,
cluster_cert = true,
ssl_dhparam = false,
lua_ssl_trusted_certificate = false,
cluster_ca_cert = false
}) do
conf[cert_name] = try_base64_decode(conf[cert_name])

if has_key then
local key_name = cert_name .. "_key"
conf[key_name] = try_base64_decode(conf[key_name])
end
end

---------------------
-- custom validations
---------------------
Expand Down Expand Up @@ -797,25 +780,31 @@ local function check_and_infer(conf, opts)
end

if ssl_cert then
for _, cert in ipairs(ssl_cert) do
for i, cert in ipairs(ssl_cert) do
if not exists(cert) then
cert = try_decode_base64(cert)
ssl_cert[i] = cert
local _, err = openssl_x509.new(cert)
if err then
errors[#errors + 1] = prefix .. "ssl_cert: failed loading certificate from " .. cert
end
end
end
conf[prefix .. "ssl_cert"] = ssl_cert
end

if ssl_cert_key then
for _, cert_key in ipairs(ssl_cert_key) do
for i, cert_key in ipairs(ssl_cert_key) do
if not exists(cert_key) then
cert_key = try_decode_base64(cert_key)
ssl_cert_key[i] = cert_key
local _, err = openssl_pkey.new(cert_key)
if err then
errors[#errors + 1] = prefix .. "ssl_cert_key: failed loading key from " .. cert_key
end
end
end
conf[prefix .. "ssl_cert_key"] = ssl_cert_key
end
end
end
Expand All @@ -832,13 +821,17 @@ local function check_and_infer(conf, opts)
end

if client_ssl_cert and not exists(client_ssl_cert) then
client_ssl_cert = try_decode_base64(client_ssl_cert)
conf.client_ssl_cert = client_ssl_cert
local _, err = openssl_x509.new(client_ssl_cert)
if err then
errors[#errors + 1] = "client_ssl_cert: failed loading certificate from " .. client_ssl_cert
end
end

if client_ssl_cert_key and not exists(client_ssl_cert_key) then
client_ssl_cert_key = try_decode_base64(client_ssl_cert_key)
conf.client_ssl_cert_key = client_ssl_cert_key
local _, err = openssl_pkey.new(client_ssl_cert_key)
if err then
errors[#errors + 1] = "client_ssl_cert_key: failed loading key from " ..
Expand All @@ -865,11 +858,12 @@ local function check_and_infer(conf, opts)

if trusted_cert ~= "system" then
if not exists(trusted_cert) then
trusted_cert = try_decode_base64(trusted_cert)
local _, err = openssl_x509.new(trusted_cert)
if err then
errors[#errors + 1] = "lua_ssl_trusted_certificate: " ..
"failed loading certificate from " ..
trusted_cert
"failed loading certificate from " ..
trusted_cert
end
end

Expand Down Expand Up @@ -906,6 +900,7 @@ local function check_and_infer(conf, opts)
if conf.ssl_dhparam then
if not is_predefined_dhgroup(conf.ssl_dhparam)
and not exists(conf.ssl_dhparam) then
conf.ssl_dhparam = try_decode_base64(conf.ssl_dhparam)
local _, err = openssl_pkey.new(
{
type = "DH",
Expand Down Expand Up @@ -1076,13 +1071,17 @@ local function check_and_infer(conf, opts)

else
if not exists(cluster_cert) then
cluster_cert = try_decode_base64(cluster_cert)
conf.cluster_cert = cluster_cert
local _, err = openssl_x509.new(cluster_cert)
if err then
errors[#errors + 1] = "cluster_cert: failed loading certificate from " .. cluster_cert
end
end

if not exists(cluster_cert_key) then
cluster_cert_key = try_decode_base64(cluster_cert_key)
conf.cluster_cert_key = cluster_cert_key
local _, err = openssl_pkey.new(cluster_cert_key)
if err then
errors[#errors + 1] = "cluster_cert_key: failed loading key from " .. cluster_cert_key
Expand All @@ -1091,6 +1090,8 @@ local function check_and_infer(conf, opts)
end

if cluster_ca_cert and not exists(cluster_ca_cert) then
cluster_ca_cert = try_decode_base64(cluster_ca_cert)
conf.cluster_ca_cert = cluster_ca_cert
local _, err = openssl_x509.new(cluster_ca_cert)
if err then
errors[#errors + 1] = "cluster_ca_cert: failed loading certificate from " ..
Expand Down
10 changes: 7 additions & 3 deletions spec/01-unit/03-conf_loader_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -750,14 +750,18 @@ describe("Configuration loader", function()
ssl_dhparam = dhparam,
lua_ssl_trusted_certificate = cacert
}

local conf_params = {
ssl_cipher_suite = "old"
ssl_cipher_suite = "old",
client_ssl = "on",
role = "control_plane",
status_listen = "127.0.0.1:123 ssl",
proxy_listen = "127.0.0.1:456 ssl",
admin_listen = "127.0.0.1:789 ssl"
}

for n, v in pairs(properties) do
conf_params[n] = ngx.encode_base64(v)
end

local conf, err = conf_loader(nil, conf_params)

assert.is_nil(err)
Expand Down
Loading

0 comments on commit c15556f

Please sign in to comment.