18 Sep, 2019

1 commit

  • The first time a rule set is configured for SafeSetID, we shouldn't be
    trying to release the previously configured ruleset, since there isn't
    one. Currently, the pointer that would point to a previously configured
    ruleset is uninitialized on first rule set configuration, leading to a
    crash when we try to call release_ruleset with that pointer.

    Acked-by: Jann Horn
    Signed-off-by: Micah Morton

    Micah Morton
     

15 Jul, 2019

10 commits

  • The capable() hook returns an error number. -EPERM is actually the same as
    -1, so this doesn't make a difference in behavior.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • Someone might write a ruleset like the following, expecting that it
    securely constrains UID 1 to UIDs 1, 2 and 3:

    1:2
    1:3

    However, because no constraints are applied to UIDs 2 and 3, an attacker
    with UID 1 can simply first switch to UID 2, then switch to any UID from
    there. The secure way to write this ruleset would be:

    1:2
    1:3
    2:2
    3:3

    , which uses "transition to self" as a way to inhibit the default-allow
    policy without allowing anything specific.

    This is somewhat unintuitive. To make sure that policy authors don't
    accidentally write insecure policies because of this, let the kernel verify
    that a new ruleset does not contain any entries that are constrained, but
    transitively unconstrained.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • For debugging a running system, it is very helpful to be able to see what
    policy the system is using. Add a read handler that can dump out a copy of
    the loaded policy.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • The current API of the SafeSetID LSM uses one write() per rule, and applies
    each written rule instantly. This has several downsides:

    - While a policy is being loaded, once a single parent-child pair has been
    loaded, the parent is restricted to that specific child, even if
    subsequent rules would allow transitions to other child UIDs. This means
    that during policy loading, set*uid() can randomly fail.
    - To replace the policy without rebooting, it is necessary to first flush
    all old rules. This creates a time window in which no constraints are
    placed on the use of CAP_SETUID.
    - If we want to perform sanity checks on the final policy, this requires
    that the policy isn't constructed in a piecemeal fashion without telling
    the kernel when it's done.

    Other kernel APIs - including things like the userns code and netfilter -
    avoid this problem by performing updates atomically. Luckily, SafeSetID
    hasn't landed in a stable (upstream) release yet, so maybe it's not too
    late to completely change the API.

    The new API for SafeSetID is: If you want to change the policy, open
    "safesetid/whitelist_policy" and write the entire policy,
    newline-delimited, in there.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • Looking at current_cred() in write handlers is bad form, stop doing that.

    Also, let's just require that the write is coming from the initial user
    namespace. Especially SAFESETID_WHITELIST_FLUSH requires privilege over all
    namespaces, and SAFESETID_WHITELIST_ADD should probably require it as well.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • In preparation for changing the policy parsing logic, refactor the line
    parsing logic to be less verbose and move it into a separate function.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • At the moment, safesetid_security_capable() has two nested conditional
    blocks, and one big comment for all the logic. Chop it up and reduce the
    amount of indentation.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • parent_kuid and child_kuid are kuids, there is no reason to make them
    uint64_t. (And anyway, in the kernel, the normal name for that would be
    u64, not uint64_t.)

    check_setuid_policy_hashtable_key() and
    check_setuid_policy_hashtable_key_value() are basically the same thing,
    merge them.

    Also fix the comment that claimed that (1<
    Signed-off-by: Micah Morton

    Jann Horn
     
  • With the old code, when a process with the (real,effective,saved) UID set
    (1,1,1) calls setresuid(2,3,4), safesetid_task_fix_setuid() only checks
    whether the transition 1->2 is permitted; the transitions 1->3 and 1->4 are
    not checked. Fix this.

    This is also a good opportunity to refactor safesetid_task_fix_setuid() to
    be less verbose - having one branch per set*uid() syscall is unnecessary.

    Note that this slightly changes semantics: The UID transition check for
    UIDs that were not in the old cred struct is now always performed against
    the policy of the RUID. I think that's more consistent anyway, since the
    RUID is also the one that decides whether any policy is enforced at all.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     
  • Fix the pr_warn() calls in the SafeSetID LSM to have newlines at the end.
    Without this, denial messages will be buffered as incomplete lines in
    log_output(), and will then only show up once something else prints into
    dmesg.

    Signed-off-by: Jann Horn
    Signed-off-by: Micah Morton

    Jann Horn
     

09 Jul, 2019

1 commit

  • …iederm/user-namespace

    Pull force_sig() argument change from Eric Biederman:
    "A source of error over the years has been that force_sig has taken a
    task parameter when it is only safe to use force_sig with the current
    task.

    The force_sig function is built for delivering synchronous signals
    such as SIGSEGV where the userspace application caused a synchronous
    fault (such as a page fault) and the kernel responded with a signal.

    Because the name force_sig does not make this clear, and because the
    force_sig takes a task parameter the function force_sig has been
    abused for sending other kinds of signals over the years. Slowly those
    have been fixed when the oopses have been tracked down.

    This set of changes fixes the remaining abusers of force_sig and
    carefully rips out the task parameter from force_sig and friends
    making this kind of error almost impossible in the future"

    * 'siginfo-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace: (27 commits)
    signal/x86: Move tsk inside of CONFIG_MEMORY_FAILURE in do_sigbus
    signal: Remove the signal number and task parameters from force_sig_info
    signal: Factor force_sig_info_to_task out of force_sig_info
    signal: Generate the siginfo in force_sig
    signal: Move the computation of force into send_signal and correct it.
    signal: Properly set TRACE_SIGNAL_LOSE_INFO in __send_signal
    signal: Remove the task parameter from force_sig_fault
    signal: Use force_sig_fault_to_task for the two calls that don't deliver to current
    signal: Explicitly call force_sig_fault on current
    signal/unicore32: Remove tsk parameter from __do_user_fault
    signal/arm: Remove tsk parameter from __do_user_fault
    signal/arm: Remove tsk parameter from ptrace_break
    signal/nds32: Remove tsk parameter from send_sigtrap
    signal/riscv: Remove tsk parameter from do_trap
    signal/sh: Remove tsk parameter from force_sig_info_fault
    signal/um: Remove task parameter from send_sigtrap
    signal/x86: Remove task parameter from send_sigtrap
    signal: Remove task parameter from force_sig_mceerr
    signal: Remove task parameter from force_sig
    signal: Remove task parameter from force_sigsegv
    ...

    Linus Torvalds
     

27 May, 2019

1 commit


21 May, 2019

1 commit


13 Feb, 2019

1 commit

  • In case of error, the function securityfs_create_dir() returns ERR_PTR()
    and never returns NULL. The NULL test in the return value check should
    be replaced with IS_ERR().

    Fixes: aeca4e2ca65c ("LSM: add SafeSetID module that gates setid calls")
    Signed-off-by: Wei Yongjun
    Acked-by: Kees Cook
    Signed-off-by: James Morris

    Wei Yongjun
     

31 Jan, 2019

1 commit

  • The include for asm/syscall.h was needed in a prior version of lsm.c
    that checked return values of syscall_get_nr, but since we did away with
    that part of the code this include is no longer necessary. Take out this
    include since it breaks builds for certain architectures. We no longer
    have any arch-specific code in SafeSetID.

    Signed-off-by: Micah Morton
    Signed-off-by: James Morris

    Micah Morton
     

30 Jan, 2019

1 commit

  • This patch changes the Kconfig file for the SafeSetID LSM to depend on
    CONFIG_SECURITY as well as select CONFIG_SECURITYFS, since the policies
    for the LSM are configured through writing to securityfs.

    Signed-off-by: Micah Morton
    Signed-off-by: James Morris

    Micah Morton
     

29 Jan, 2019

1 commit

  • Without this, system boot was crashing with:

    [0.174285] LSM: Security Framework initializing
    [0.175277] BUG: unable to handle kernel NULL pointer dereference
    ...
    [0.176272] Call Trace:
    [0.176272] ordered_lsm_parse+0x112/0x20b
    [0.176272] security_init+0x9b/0x3ab
    [0.176272] start_kernel+0x413/0x479
    [0.176272] secondary_startup_64+0xa4/0xb0

    Signed-off-by: Micah Morton
    Fixed-by: Kees Cook
    Signed-off-by: James Morris

    Micah Morton
     

26 Jan, 2019

1 commit

  • 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
    Acked-by: Kees Cook
    Signed-off-by: James Morris

    Micah Morton