On Wed, Dec 5, 2018 at 4:08 PM Kees Cook [off-list ref] wrote:

On Wed, Nov 21, 2018 at 8:54 AM [off-list ref] wrote:

quoted

From: Micah Morton <mortonm@chromium.org>

SafeSetID gates the setid family of syscalls to restrict UID/GID
transitions from a given UID/GID to only those approved by a
system-wide whitelist. These restrictions also prohibit the given
UIDs/GIDs from obtaining auxiliary privileges associated with
CAP_SET{U/G}ID, such as allowing a user to set up user namespace UID
mappings. For now, only gating the set*uid family of syscalls is
supported, with support for set*gid coming in a future patch set.

Signed-off-by: Micah Morton <mortonm@chromium.org>
---

Sending a patch developed against the 'next-general' branch of the
linux-security tree, since the previous patch versions wouldn't apply
cleanly to 'next-general'.

I'm finally getting back around to this. Sorry for the delay!

A few general process notes:
- Please "version" your patches in the Subject (e.g. "[PATCH v3] LSM:
add SafeSetID ..."). This helps track discussion.
- Please include a "changes since last version below the first "---"
line, to summarize what has changed. This makes review faster for
people that have read a specific version but need to catch up (like
me) :)

Ok thanks, will do in the future. The only code change since the
initial upload was to add a do_exit(SIGKILL) line to
setuid_policy_warning() in lsm.c, which will kill any process that
violates the whitelist policy. This way, there can never be a case
where a privileged program fails to drop privilege because of our
whitelist and continues running in an accidentally over-privileged
context.

quoted

+/*
+ * TODO: Figuring out whether the current syscall number (saved on the kernel
+ * stack) is one of the set*uid syscalls is an operation that requires checking
+ * the number against arch-specific constants as seen below. The need for this
+ * LSM to know about arch-specific syscall stuff is not ideal. Is it better to
+ * implement an arch-specific function that gets called from this file and
+ * update arch/Kconfig to mention that the HAVE_SAFESETID symbol should only be
+ * selected for architectures that implement the function? Any other ideas?
+ */

What would Stephen's solution for this problem end up looking like? I
think avoiding the arch-specific-ness would be quite valuable.

I sent a patch here that is an example of how it could be done:
https://www.spinics.net/lists/linux-security-module/msg24504.html.
AFAICT I think this is what Stephen had in mind.

I think adding a capability for this isn't the way to go (there is a
very painful history on adding capabilities). This feels much more
like a good mapping to an LSM (it's narrowing a privilege) with a very
specific policy.

--
Kees Cook