Skip to content

Commit

Permalink
Improve filesystem permissions and use more restrictive umask.
Browse files Browse the repository at this point in the history
perp was starting processes with a umask of 0, so various directories
and files had permissions that were too wide open (eg, db directory and
files were world readable/writable). This sets more explicit permissions
everywhere to mitigate this and fix existing installations.

We'll also start using a more restrictive umask everywhere and set the
effective gid of the process to try and have better default permissions.

This also gets rid of some of the recursive "chown -R" commands that
were previously in our startup sequence. This could slow down startup
performance as the number of recursive files grew, and shouldn't be
necessary with the more explicit permissions setup. I think the
recursive command could also possibly lead to race conditions (only seen
in the CI environment, but a situation where the chown failed on a
database file that didn't exist, possibly because the database was being
modified at the same time as the chown).
  • Loading branch information
GUI committed May 20, 2018
1 parent 86af57a commit 2e595ce
Show file tree
Hide file tree
Showing 39 changed files with 398 additions and 49 deletions.
1 change: 1 addition & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,4 @@ ban:
ember_server:
port: 14050
live_reload_port: 14051
umask: "0027"
34 changes: 30 additions & 4 deletions src/api-umbrella/cli/read_config.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ local nillify_yaml_nulls = require "api-umbrella.utils.nillify_yaml_nulls"
local path = require "pl.path"
local plutils = require "pl.utils"
local random_token = require "api-umbrella.utils.random_token"
local stat = require "posix.sys.stat"
local stringx = require "pl.stringx"
local types = require "pl.types"
local unistd = require "posix.unistd"
local url = require "socket.url"

local chmod = stat.chmod
local chown = unistd.chown
local is_empty = types.is_empty
local split = plutils.split
local strip = stringx.strip
Expand Down Expand Up @@ -153,20 +157,24 @@ local function set_computed_config()
config["etc_dir"] = path.join(config["root_dir"], "etc")
end

if not config["var_dir"] then
config["var_dir"] = path.join(config["root_dir"], "var")
end

if not config["log_dir"] then
config["log_dir"] = path.join(config["root_dir"], "var/log")
config["log_dir"] = path.join(config["var_dir"], "log")
end

if not config["run_dir"] then
config["run_dir"] = path.join(config["root_dir"], "var/run")
config["run_dir"] = path.join(config["var_dir"], "run")
end

if not config["tmp_dir"] then
config["tmp_dir"] = path.join(config["root_dir"], "var/tmp")
config["tmp_dir"] = path.join(config["var_dir"], "tmp")
end

if not config["db_dir"] then
config["db_dir"] = path.join(config["root_dir"], "var/db")
config["db_dir"] = path.join(config["var_dir"], "db")
end

local trusted_proxies = config["router"]["trusted_proxies"] or {}
Expand Down Expand Up @@ -359,6 +367,11 @@ local function set_computed_config()
end
end

local function set_process_permissions()
unistd.setpid("g", "api-umbrella")
stat.umask(tonumber(config["umask"], 8))
end

-- Handle setup of random secret tokens that should be be unique for API
-- Umbrella installations, but should be persisted across restarts.
--
Expand Down Expand Up @@ -401,6 +414,11 @@ local function set_cached_random_tokens()
-- Persist the cached tokens.
dir.makepath(config["run_dir"])
file.write(cached_path, lyaml.dump({ cached }))
chmod(cached_path, tonumber("0640", 8))
if config["group"] then
chown(cached_path, nil, config["group"])
end

deep_defaults(config, cached)
end
end
Expand All @@ -415,6 +433,10 @@ local function write_runtime_config()
local runtime_config_path = path.join(config["run_dir"], "runtime_config.yml")
dir.makepath(config["run_dir"])
file.write(runtime_config_path, lyaml.dump({config}))
chmod(runtime_config_path, tonumber("0640", 8))
if config["group"] then
chown(runtime_config_path, nil, config["group"])
end
end

return function(options)
Expand All @@ -431,12 +453,16 @@ return function(options)
read_default_config()
read_system_config()
set_computed_config()
set_process_permissions()

set_cached_random_tokens()

if options and options["write"] then
write_runtime_config()
end
end

set_process_permissions()

return config
end
85 changes: 54 additions & 31 deletions src/api-umbrella/cli/setup.lua
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ local stat = require "posix.sys.stat"
local tablex = require "pl.tablex"
local unistd = require "posix.unistd"

local chmod = stat.chmod
local chown = unistd.chown

local config
local template_config

Expand Down Expand Up @@ -95,19 +98,8 @@ local function prepare()
config["log_dir"],
config["run_dir"],
config["tmp_dir"],
path.join(config["db_dir"], "elasticsearch"),
path.join(config["etc_dir"], "elasticsearch_scripts"),
path.join(config["db_dir"], "mongodb"),
path.join(config["db_dir"], "rsyslog"),
path.join(config["etc_dir"], "trafficserver/snapshots"),
path.join(config["log_dir"], "trafficserver"),
path.join(config["root_dir"], "var/trafficserver"),
}

if config["app_env"] == "test" then
table.insert(dirs, path.join(config["run_dir"], "test-env-mongo-orchestration"))
end

for _, directory in ipairs(dirs) do
dir.makepath(directory)
end
Expand Down Expand Up @@ -149,6 +141,10 @@ local function ensure_geoip_db()
local default_city_db_path = path.join(config["_embedded_root_dir"], "var/db/geoip/city-v6.dat")
dir.makepath(path.dirname(city_db_path))
file.copy(default_city_db_path, city_db_path)
chmod(city_db_path, tonumber("0640", 8))
if config["group"] then
chown(city_db_path, nil, config["group"])
end
end
end

Expand Down Expand Up @@ -188,7 +184,16 @@ local function write_templates()

dir.makepath(path.dirname(install_path))
file.write(install_path, content)
stat.chmod(install_path, stat.stat(template_path).st_mode)
if config["group"] then
chown(install_path, nil, config["group"])
end

local install_filename = path.basename(install_path)
if install_filename == "rc.log" or install_filename == "rc.main" or install_filename == "rc.perp" then
chmod(install_path, tonumber("0750", 8))
else
chmod(install_path, tonumber("0640", 8))
end
end
end
end
Expand All @@ -214,18 +219,37 @@ local function write_static_site_key()
end

local function set_permissions()
local _, err
_, _, err = run_command("chmod 1777 " .. config["tmp_dir"])
if err then
print("chmod failed: ", err)
os.exit(1)
end
chmod(config["tmp_dir"], tonumber("1777", 8))

if config["user"] and config["group"] then
_, _, err = run_command("chown -R " .. config["user"] .. ":" .. config["group"] .. " " .. path.join(config["etc_dir"], "trafficserver") .. " " .. path.join(config["root_dir"], "var"))
if err then
print(err)
os.exit(1)
local user = config["user"]
local group = config["group"]
chown(config["db_dir"], nil, group)
chown(config["log_dir"], nil, group)
chown(config["run_dir"], user, group)
chown(config["tmp_dir"], user, group)
chown(config["var_dir"], nil, group)
chown(config["etc_dir"], nil, group)
chown(path.join(config["db_dir"], "geoip"), nil, group)
chown(path.join(config["etc_dir"], "elasticsearch"), nil, group)
chown(path.join(config["etc_dir"], "nginx"), nil, group)
chown(path.join(config["etc_dir"], "perp"), nil, group)
chown(path.join(config["etc_dir"], "trafficserver"), nil, group)

if config["app_env"] == "test" then
chown(path.join(config["etc_dir"], "test-env"), nil, group)
chown(path.join(config["etc_dir"], "test-env/mongo-orchestration"), nil, group)
chown(path.join(config["etc_dir"], "test-env/nginx"), nil, group)
chown(path.join(config["etc_dir"], "test-env/openldap"), nil, group)
chown(path.join(config["etc_dir"], "test-env/unbound"), nil, group)
end
end

local service_dirs = dir.getdirectories(path.join(config["etc_dir"], "perp"))
for _, service_dir in ipairs(service_dirs) do
chmod(service_dir, tonumber("0750", 8))
if config["group"] then
chown(service_dir, nil, config["group"])
end
end
end
Expand Down Expand Up @@ -311,17 +335,16 @@ local function activate_services()

-- Set the sticky bit for any active services.
if is_active then
local _, _, err = run_command("chmod +t " .. service_dir)
if err then
print(err)
os.exit(1)
chmod(service_dir, tonumber("1750", 8))

local log_dir = path.join(config["log_dir"], service_name)
dir.makepath(log_dir)
chmod(log_dir, tonumber("0750", 8))
if config["user"] and config["group"] then
chown(log_dir, config["user"], config["group"])
end
else
local _, _, err = run_command("chmod -t " .. service_dir)
if err then
print(err)
os.exit(1)
end
chmod(service_dir, tonumber("0750", 8))
end
end
end
Expand Down
1 change: 1 addition & 0 deletions templates/etc/perp/dev-env-ember-server/rc.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/usr/bin/env bash
set -e -u
exec ../rc.log "$@"
3 changes: 3 additions & 0 deletions templates/etc/perp/dev-env-ember-server/rc.main.mustache
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#!/usr/bin/env bash
set -e -u

# Redirect stderr to stdout
exec 2>&1

umask "{{umask}}"

if [ "${1}" = "start" ]; then
echo "starting ${2}..."
api_umbrella_user="{{user}}"
Expand Down
1 change: 1 addition & 0 deletions templates/etc/perp/elasticsearch/rc.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/usr/bin/env bash
set -e -u
exec ../rc.log "$@"
11 changes: 11 additions & 0 deletions templates/etc/perp/elasticsearch/rc.main.mustache
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
#!/usr/bin/env bash
set -e -u

# Redirect stderr to stdout
exec 2>&1

umask "{{umask}}"

if [ "${1}" = "start" ]; then
echo "starting ${2}..."
api_umbrella_user="{{user}}"
api_umbrella_group="{{group}}"

run_args=("-e" "rc.env")
if [ -n "$api_umbrella_user" ]; then
run_args+=("-u" "$api_umbrella_user")
fi

dirs=("{{db_dir}}/elasticsearch" "{{etc_dir}}/elasticsearch_scripts")
mkdir -p "${dirs[@]}"
chmod 750 "${dirs[@]}"
if [ -n "$api_umbrella_user" ]; then
chown $api_umbrella_user:$api_umbrella_group "${dirs[@]}"
fi

exec runtool "${run_args[@]}" elasticsearch
fi

Expand Down
1 change: 1 addition & 0 deletions templates/etc/perp/geoip-auto-updater/rc.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/usr/bin/env bash
set -e -u
exec ../rc.log "$@"
3 changes: 3 additions & 0 deletions templates/etc/perp/geoip-auto-updater/rc.main.mustache
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#!/usr/bin/env bash
set -e -u

# Redirect stderr to stdout
exec 2>&1

umask "{{umask}}"

if [ "${1}" = "start" ]; then
echo "starting ${2}..."

Expand Down
1 change: 1 addition & 0 deletions templates/etc/perp/mongod/rc.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/usr/bin/env bash
set -e -u
exec ../rc.log "$@"
11 changes: 11 additions & 0 deletions templates/etc/perp/mongod/rc.main.mustache
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
#!/usr/bin/env bash
set -e -u

# Redirect stderr to stdout
exec 2>&1

umask "{{umask}}"

if [ "${1}" = "start" ]; then
echo "starting ${2}..."
api_umbrella_user="{{user}}"
api_umbrella_group="{{group}}"

run_args=()
if [ -n "$api_umbrella_user" ]; then
run_args+=("-u" "$api_umbrella_user")
fi

dirs=("{{db_dir}}/mongodb")
mkdir -p "${dirs[@]}"
chmod 750 "${dirs[@]}"
if [ -n "$api_umbrella_user" ]; then
chown $api_umbrella_user:$api_umbrella_group "${dirs[@]}"
fi

exec runtool "${run_args[@]}" mongod --config "{{etc_dir}}/mongod.conf"
fi

Expand Down
1 change: 1 addition & 0 deletions templates/etc/perp/mora/rc.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/usr/bin/env bash
set -e -u
exec ../rc.log "$@"
3 changes: 3 additions & 0 deletions templates/etc/perp/mora/rc.main.mustache
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#!/usr/bin/env bash
set -e -u

# Redirect stderr to stdout
exec 2>&1

umask "{{umask}}"

if [ "${1}" = "start" ]; then
echo "starting ${2}..."
api_umbrella_user="{{user}}"
Expand Down
1 change: 1 addition & 0 deletions templates/etc/perp/nginx-reloader/rc.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/usr/bin/env bash
set -e -u
exec ../rc.log "$@"
3 changes: 3 additions & 0 deletions templates/etc/perp/nginx-reloader/rc.main.mustache
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#!/usr/bin/env bash
set -e -u

# Redirect stderr to stdout
exec 2>&1

umask "{{umask}}"

if [ "${1}" = "start" ]; then
echo "starting ${2}..."

Expand Down
1 change: 1 addition & 0 deletions templates/etc/perp/nginx/rc.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#!/usr/bin/env bash
set -e -u
exec ../rc.log "$@"
3 changes: 3 additions & 0 deletions templates/etc/perp/nginx/rc.main.mustache
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#!/usr/bin/env bash
set -e -u

# Redirect stderr to stdout
exec 2>&1

umask "{{umask}}"

if [ "${1}" = "start" ]; then
echo "starting ${2}..."

Expand Down
Loading

0 comments on commit 2e595ce

Please sign in to comment.