diff options
| author | Daniel Silverstone <dsilvers@digital-scurf.org> | 2019-01-19 11:43:26 +0000 |
|---|---|---|
| committer | Daniel Silverstone <dsilvers@digital-scurf.org> | 2019-01-19 11:43:26 +0000 |
| commit | be9f2dcd6bf695c8c5f65b1a8a0f30ff58f8866b (patch) | |
| tree | a5d2d040a8d7ab6accf2e5af02ad1fab00641c2b | |
| parent | 1a862c7c24638bfe0c7982b728049a162e2ad7e6 (diff) | |
| download | gitano-be9f2dcd6bf695c8c5f65b1a8a0f30ff58f8866b.tar.gz | |
admincommand: Fix 'as' to better hide bad usernames
Sometimes commands in their `_prep()` might need to assume
the user they're running as does actually exist. Since there's
only one user name guaranteed to exist (gitano-bypass) ensure
that when `as` runs against a user which doesn't exist, we run
the original command's `_prep()` against that user so that we
don't leak the non-existence of a username via a crash/traceback.
This fixes Debian bug #876078
| -rw-r--r-- | lib/gitano/admincommand.lua | 26 | ||||
| -rw-r--r-- | testing/02-commands-as.yarn | 2 |
2 files changed, 22 insertions, 6 deletions
diff --git a/lib/gitano/admincommand.lua b/lib/gitano/admincommand.lua index 5f1bcf9..e74d605 100644 --- a/lib/gitano/admincommand.lua +++ b/lib/gitano/admincommand.lua @@ -103,13 +103,29 @@ local function builtin_as_prep(conf, _, cmdline, context) context["as_" .. k] = v end context.user = cmdline[2] + local target_user_name = cmdline[2] + local target_user_exists = conf.users[cmdline[2]] ~= nil + if not target_user_exists then + -- If the target user is not in existence then pretend that + -- the caller intended to impersonate gitano-bypass. Since + -- that user really should exist. Also we set it so that + -- they would have to run 'user list' which means that if + -- we subsequently say the user doesn't exist, it's quite + -- safe. Any other case will get a permission denied from + -- the lace ruleset which should be quite safe. + cmdline[2] = "gitano-bypass" + cmdline[3] = "user" + cmdline[4] = "list" + while cmdline[5] ~= nil do + table.remove(cmdline, 5) + end + end -- Okay, we're now ready to chain through to the called command local res, msg = cmdline.cmd.prep(conf, cmdline.repo, cmdline.copy, context) - if res == "allow" then - -- Check to see if the user exists - if conf.users[cmdline[2]] == nil then - res, msg = "deny", "User '" .. cmdline[2] .. "' does not exist." - end + if res == "allow" and not target_user_exists then + -- Normally we wouldn't leak this, but if the user *could* run a command + -- as gitano-admin + res, msg = "deny", "User '" .. target_user_name .. "' does not exist." end return res, msg end diff --git a/testing/02-commands-as.yarn b/testing/02-commands-as.yarn index 92d2428..15f82ff 100644 --- a/testing/02-commands-as.yarn +++ b/testing/02-commands-as.yarn @@ -60,7 +60,7 @@ in any way. WHEN testinstance, using adminkey, adds user other, using testinstance other AND testinstance adminkey runs as other whoami THEN stderr is empty - WHEN testinstance other, expecting failure, runs as badger whoami + WHEN testinstance other, expecting failure, runs as badger sshkey list THEN stdout is empty AND stderr does not contain badger |