1. 30 5月, 2020 2 次提交
    • E
      exec: Compute file based creds only once · 56305aa9
      Eric W. Biederman 提交于
      Move the computation of creds from prepare_binfmt into begin_new_exec
      so that the creds need only be computed once.  This is just code
      reorganization no semantic changes of any kind are made.
      
      Moving the computation is safe.  I have looked through the kernel and
      verified none of the binfmts look at bprm->cred directly, and that
      there are no helpers that look at bprm->cred indirectly.  Which means
      that it is not a problem to compute the bprm->cred later in the
      execution flow as it is not used until it becomes current->cred.
      
      A new function bprm_creds_from_file is added to contain the work that
      needs to be done.  bprm_creds_from_file first computes which file
      bprm->executable or most likely bprm->file that the bprm->creds
      will be computed from.
      
      The funciton bprm_fill_uid is updated to receive the file instead of
      accessing bprm->file.  The now unnecessary work needed to reset the
      bprm->cred->euid, and bprm->cred->egid is removed from brpm_fill_uid.
      A small comment to document that bprm_fill_uid now only deals with the
      work to handle suid and sgid files.  The default case is already
      heandled by prepare_exec_creds.
      
      The function security_bprm_repopulate_creds is renamed
      security_bprm_creds_from_file and now is explicitly passed the file
      from which to compute the creds.  The documentation of the
      bprm_creds_from_file security hook is updated to explain when the hook
      is called and what it needs to do.  The file is passed from
      cap_bprm_creds_from_file into get_file_caps so that the caps are
      computed for the appropriate file.  The now unnecessary work in
      cap_bprm_creds_from_file to reset the ambient capabilites has been
      removed.  A small comment to document that the work of
      cap_bprm_creds_from_file is to read capabilities from the files
      secureity attribute and derive capabilities from the fact the
      user had uid 0 has been added.
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Signed-off-by: N"Eric W. Biederman" <ebiederm@xmission.com>
      56305aa9
    • E
      exec: Add a per bprm->file version of per_clear · a7868323
      Eric W. Biederman 提交于
      There is a small bug in the code that recomputes parts of bprm->cred
      for every bprm->file.  The code never recomputes the part of
      clear_dangerous_personality_flags it is responsible for.
      
      Which means that in practice if someone creates a sgid script
      the interpreter will not be able to use any of:
      	READ_IMPLIES_EXEC
      	ADDR_NO_RANDOMIZE
      	ADDR_COMPAT_LAYOUT
      	MMAP_PAGE_ZERO.
      
      This accentially clearing of personality flags probably does
      not matter in practice because no one has complained
      but it does make the code more difficult to understand.
      
      Further remaining bug compatible prevents the recomputation from being
      removed and replaced by simply computing bprm->cred once from the
      final bprm->file.
      
      Making this change removes the last behavior difference between
      computing bprm->creds from the final file and recomputing
      bprm->cred several times.  Which allows this behavior change
      to be justified for it's own reasons, and for any but hunts
      looking into why the behavior changed to wind up here instead
      of in the code that will follow that computes bprm->cred
      from the final bprm->file.
      
      This small logic bug appears to have existed since the code
      started clearing dangerous personality bits.
      
      History Tree: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
      Fixes: 1bb0fa189c6a ("[PATCH] NX: clean up legacy binary support")
      Reviewed-by: NKees Cook <keescook@chromium.org>
      Signed-off-by: N"Eric W. Biederman" <ebiederm@xmission.com>
      a7868323
  2. 27 5月, 2020 1 次提交
    • E
      exec: Always set cap_ambient in cap_bprm_set_creds · a4ae32c7
      Eric W. Biederman 提交于
      An invariant of cap_bprm_set_creds is that every field in the new cred
      structure that cap_bprm_set_creds might set, needs to be set every
      time to ensure the fields does not get a stale value.
      
      The field cap_ambient is not set every time cap_bprm_set_creds is
      called, which means that if there is a suid or sgid script with an
      interpreter that has neither the suid nor the sgid bits set the
      interpreter should be able to accept ambient credentials.
      Unfortuantely because cap_ambient is not reset to it's original value
      the interpreter can not accept ambient credentials.
      
      Given that the ambient capability set is expected to be controlled by
      the caller, I don't think this is particularly serious.  But it is
      definitely worth fixing so the code works correctly.
      
      I have tested to verify my reading of the code is correct and the
      interpreter of a sgid can receive ambient capabilities with this
      change and cannot receive ambient capabilities without this change.
      
      Cc: stable@vger.kernel.org
      Cc: Andy Lutomirski <luto@kernel.org>
      Fixes: 58319057 ("capabilities: ambient capabilities")
      Signed-off-by: N"Eric W. Biederman" <ebiederm@xmission.com>
      a4ae32c7
  3. 21 5月, 2020 1 次提交
  4. 07 7月, 2019 1 次提交
  5. 12 6月, 2019 1 次提交
  6. 31 5月, 2019 1 次提交
  7. 26 2月, 2019 1 次提交
  8. 26 1月, 2019 1 次提交
  9. 11 1月, 2019 1 次提交
  10. 09 1月, 2019 1 次提交
  11. 13 12月, 2018 1 次提交
    • P
      security: audit and remove any unnecessary uses of module.h · 876979c9
      Paul Gortmaker 提交于
      Historically a lot of these existed because we did not have
      a distinction between what was modular code and what was providing
      support to modules via EXPORT_SYMBOL and friends.  That changed
      when we forked out support for the latter into the export.h file.
      This means we should be able to reduce the usage of module.h
      in code that is obj-y Makefile or bool Kconfig.
      
      The advantage in removing such instances is that module.h itself
      sources about 15 other headers; adding significantly to what we feed
      cpp, and it can obscure what headers we are effectively using.
      
      Since module.h might have been the implicit source for init.h
      (for __init) and for export.h (for EXPORT_SYMBOL) we consider each
      instance for the presence of either and replace as needed.
      
      Cc: James Morris <jmorris@namei.org>
      Cc: "Serge E. Hallyn" <serge@hallyn.com>
      Cc: John Johansen <john.johansen@canonical.com>
      Cc: Mimi Zohar <zohar@linux.ibm.com>
      Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
      Cc: David Howells <dhowells@redhat.com>
      Cc: linux-security-module@vger.kernel.org
      Cc: linux-integrity@vger.kernel.org
      Cc: keyrings@vger.kernel.org
      Signed-off-by: NPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: NJames Morris <james.morris@microsoft.com>
      876979c9
  12. 30 8月, 2018 1 次提交
  13. 11 8月, 2018 1 次提交
    • E
      cap_inode_getsecurity: use d_find_any_alias() instead of d_find_alias() · 355139a8
      Eddie.Horng 提交于
      The code in cap_inode_getsecurity(), introduced by commit 8db6c34f
      ("Introduce v3 namespaced file capabilities"), should use
      d_find_any_alias() instead of d_find_alias() do handle unhashed dentry
      correctly. This is needed, for example, if execveat() is called with an
      open but unlinked overlayfs file, because overlayfs unhashes dentry on
      unlink.
      This is a regression of real life application, first reported at
      https://www.spinics.net/lists/linux-unionfs/msg05363.html
      
      Below reproducer and setup can reproduce the case.
        const char* exec="echo";
        const char *newargv[] = { "echo", "hello", NULL};
        const char *newenviron[] = { NULL };
        int fd, err;
      
        fd = open(exec, O_PATH);
        unlink(exec);
        err = syscall(322/*SYS_execveat*/, fd, "", newargv, newenviron,
      AT_EMPTY_PATH);
        if(err<0)
          fprintf(stderr, "execveat: %s\n", strerror(errno));
      
      gcc compile into ~/test/a.out
      mount -t overlay -orw,lowerdir=/mnt/l,upperdir=/mnt/u,workdir=/mnt/w
      none /mnt/m
      cd /mnt/m
      cp /bin/echo .
      ~/test/a.out
      
      Expected result:
      hello
      Actually result:
      execveat: Invalid argument
      dmesg:
      Invalid argument reading file caps for /dev/fd/3
      
      The 2nd reproducer and setup emulates similar case but for
      regular filesystem:
        const char* exec="echo";
        int fd, err;
        char buf[256];
      
        fd = open(exec, O_RDONLY);
        unlink(exec);
        err = fgetxattr(fd, "security.capability", buf, 256);
        if(err<0)
          fprintf(stderr, "fgetxattr: %s\n", strerror(errno));
      
      gcc compile into ~/test_fgetxattr
      
      cd /tmp
      cp /bin/echo .
      ~/test_fgetxattr
      
      Result:
      fgetxattr: Invalid argument
      
      On regular filesystem, for example, ext4 read xattr from
      disk and return to execveat(), will not trigger this issue, however,
      the overlay attr handler pass real dentry to vfs_getxattr() will.
      This reproducer calls fgetxattr() with an unlinked fd, involkes
      vfs_getxattr() then reproduced the case that d_find_alias() in
      cap_inode_getsecurity() can't find the unlinked dentry.
      Suggested-by: NAmir Goldstein <amir73il@gmail.com>
      Acked-by: NAmir Goldstein <amir73il@gmail.com>
      Acked-by: NSerge E. Hallyn <serge@hallyn.com>
      Fixes: 8db6c34f ("Introduce v3 namespaced file capabilities")
      Cc: <stable@vger.kernel.org> # v4.14
      Signed-off-by: NEddie Horng <eddie.horng@mediatek.com>
      Signed-off-by: NEric W. Biederman <ebiederm@xmission.com>
      355139a8
  14. 25 5月, 2018 1 次提交
    • E
      capabilities: Allow privileged user in s_user_ns to set security.* xattrs · b1d749c5
      Eric W. Biederman 提交于
      A privileged user in s_user_ns will generally have the ability to
      manipulate the backing store and insert security.* xattrs into
      the filesystem directly. Therefore the kernel must be prepared to
      handle these xattrs from unprivileged mounts, and it makes little
      sense for commoncap to prevent writing these xattrs to the
      filesystem. The capability and LSM code have already been updated
      to appropriately handle xattrs from unprivileged mounts, so it
      is safe to loosen this restriction on setting xattrs.
      
      The exception to this logic is that writing xattrs to a mounted
      filesystem may also cause the LSM inode_post_setxattr or
      inode_setsecurity callbacks to be invoked. SELinux will deny the
      xattr update by virtue of applying mountpoint labeling to
      unprivileged userns mounts, and Smack will deny the writes for
      any user without global CAP_MAC_ADMIN, so loosening the
      capability check in commoncap is safe in this respect as well.
      Signed-off-by: NSeth Forshee <seth.forshee@canonical.com>
      Acked-by: NSerge Hallyn <serge@hallyn.com>
      Acked-by: NChristian Brauner <christian@brauner.io>
      Signed-off-by: NEric W. Biederman <ebiederm@xmission.com>
      b1d749c5
  15. 11 4月, 2018 1 次提交
  16. 02 1月, 2018 1 次提交
    • E
      capabilities: fix buffer overread on very short xattr · dc32b5c3
      Eric Biggers 提交于
      If userspace attempted to set a "security.capability" xattr shorter than
      4 bytes (e.g. 'setfattr -n security.capability -v x file'), then
      cap_convert_nscap() read past the end of the buffer containing the xattr
      value because it accessed the ->magic_etc field without verifying that
      the xattr value is long enough to contain that field.
      
      Fix it by validating the xattr value size first.
      
      This bug was found using syzkaller with KASAN.  The KASAN report was as
      follows (cleaned up slightly):
      
          BUG: KASAN: slab-out-of-bounds in cap_convert_nscap+0x514/0x630 security/commoncap.c:498
          Read of size 4 at addr ffff88002d8741c0 by task syz-executor1/2852
      
          CPU: 0 PID: 2852 Comm: syz-executor1 Not tainted 4.15.0-rc6-00200-gcc0aac99d977 #253
          Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
          Call Trace:
           __dump_stack lib/dump_stack.c:17 [inline]
           dump_stack+0xe3/0x195 lib/dump_stack.c:53
           print_address_description+0x73/0x260 mm/kasan/report.c:252
           kasan_report_error mm/kasan/report.c:351 [inline]
           kasan_report+0x235/0x350 mm/kasan/report.c:409
           cap_convert_nscap+0x514/0x630 security/commoncap.c:498
           setxattr+0x2bd/0x350 fs/xattr.c:446
           path_setxattr+0x168/0x1b0 fs/xattr.c:472
           SYSC_setxattr fs/xattr.c:487 [inline]
           SyS_setxattr+0x36/0x50 fs/xattr.c:483
           entry_SYSCALL_64_fastpath+0x18/0x85
      
      Fixes: 8db6c34f ("Introduce v3 namespaced file capabilities")
      Cc: <stable@vger.kernel.org> # v4.14+
      Signed-off-by: NEric Biggers <ebiggers@google.com>
      Reviewed-by: NSerge Hallyn <serge@hallyn.com>
      Signed-off-by: NJames Morris <james.l.morris@oracle.com>
      dc32b5c3
  17. 20 10月, 2017 10 次提交
  18. 19 10月, 2017 1 次提交
  19. 24 9月, 2017 1 次提交
  20. 02 9月, 2017 1 次提交
    • S
      Introduce v3 namespaced file capabilities · 8db6c34f
      Serge E. Hallyn 提交于
      Root in a non-initial user ns cannot be trusted to write a traditional
      security.capability xattr.  If it were allowed to do so, then any
      unprivileged user on the host could map his own uid to root in a private
      namespace, write the xattr, and execute the file with privilege on the
      host.
      
      However supporting file capabilities in a user namespace is very
      desirable.  Not doing so means that any programs designed to run with
      limited privilege must continue to support other methods of gaining and
      dropping privilege.  For instance a program installer must detect
      whether file capabilities can be assigned, and assign them if so but set
      setuid-root otherwise.  The program in turn must know how to drop
      partial capabilities, and do so only if setuid-root.
      
      This patch introduces v3 of the security.capability xattr.  It builds a
      vfs_ns_cap_data struct by appending a uid_t rootid to struct
      vfs_cap_data.  This is the absolute uid_t (that is, the uid_t in user
      namespace which mounted the filesystem, usually init_user_ns) of the
      root id in whose namespaces the file capabilities may take effect.
      
      When a task asks to write a v2 security.capability xattr, if it is
      privileged with respect to the userns which mounted the filesystem, then
      nothing should change.  Otherwise, the kernel will transparently rewrite
      the xattr as a v3 with the appropriate rootid.  This is done during the
      execution of setxattr() to catch user-space-initiated capability writes.
      Subsequently, any task executing the file which has the noted kuid as
      its root uid, or which is in a descendent user_ns of such a user_ns,
      will run the file with capabilities.
      
      Similarly when asking to read file capabilities, a v3 capability will
      be presented as v2 if it applies to the caller's namespace.
      
      If a task writes a v3 security.capability, then it can provide a uid for
      the xattr so long as the uid is valid in its own user namespace, and it
      is privileged with CAP_SETFCAP over its namespace.  The kernel will
      translate that rootid to an absolute uid, and write that to disk.  After
      this, a task in the writer's namespace will not be able to use those
      capabilities (unless rootid was 0), but a task in a namespace where the
      given uid is root will.
      
      Only a single security.capability xattr may exist at a time for a given
      file.  A task may overwrite an existing xattr so long as it is
      privileged over the inode.  Note this is a departure from previous
      semantics, which required privilege to remove a security.capability
      xattr.  This check can be re-added if deemed useful.
      
      This allows a simple setxattr to work, allows tar/untar to work, and
      allows us to tar in one namespace and untar in another while preserving
      the capability, without risking leaking privilege into a parent
      namespace.
      
      Example using tar:
      
       $ cp /bin/sleep sleepx
       $ mkdir b1 b2
       $ lxc-usernsexec -m b:0:100000:1 -m b:1:$(id -u):1 -- chown 0:0 b1
       $ lxc-usernsexec -m b:0:100001:1 -m b:1:$(id -u):1 -- chown 0:0 b2
       $ lxc-usernsexec -m b:0:100000:1000 -- tar --xattrs-include=security.capability --xattrs -cf b1/sleepx.tar sleepx
       $ lxc-usernsexec -m b:0:100001:1000 -- tar --xattrs-include=security.capability --xattrs -C b2 -xf b1/sleepx.tar
       $ lxc-usernsexec -m b:0:100001:1000 -- getcap b2/sleepx
         b2/sleepx = cap_sys_admin+ep
       # /opt/ltp/testcases/bin/getv3xattr b2/sleepx
         v3 xattr, rootid is 100001
      
      A patch to linux-test-project adding a new set of tests for this
      functionality is in the nsfscaps branch at github.com/hallyn/ltp
      
      Changelog:
         Nov 02 2016: fix invalid check at refuse_fcap_overwrite()
         Nov 07 2016: convert rootid from and to fs user_ns
         (From ebiederm: mar 28 2017)
           commoncap.c: fix typos - s/v4/v3
           get_vfs_caps_from_disk: clarify the fs_ns root access check
           nsfscaps: change the code split for cap_inode_setxattr()
         Apr 09 2017:
             don't return v3 cap for caps owned by current root.
            return a v2 cap for a true v2 cap in non-init ns
         Apr 18 2017:
            . Change the flow of fscap writing to support s_user_ns writing.
            . Remove refuse_fcap_overwrite().  The value of the previous
              xattr doesn't matter.
         Apr 24 2017:
            . incorporate Eric's incremental diff
            . move cap_convert_nscap to setxattr and simplify its usage
         May 8, 2017:
            . fix leaking dentry refcount in cap_inode_getsecurity
      Signed-off-by: NSerge Hallyn <serge@hallyn.com>
      Signed-off-by: NEric W. Biederman <ebiederm@xmission.com>
      8db6c34f
  21. 02 8月, 2017 2 次提交
    • K
      commoncap: Move cap_elevated calculation into bprm_set_creds · ee67ae7e
      Kees Cook 提交于
      Instead of a separate function, open-code the cap_elevated test, which
      lets us entirely remove bprm->cap_effective (to use the local "effective"
      variable instead), and more accurately examine euid/egid changes via the
      existing local "is_setid".
      
      The following LTP tests were run to validate the changes:
      
      	# ./runltp -f syscalls -s cap
      	# ./runltp -f securebits
      	# ./runltp -f cap_bounds
      	# ./runltp -f filecaps
      
      All kernel selftests for capabilities and exec continue to pass as well.
      Signed-off-by: NKees Cook <keescook@chromium.org>
      Reviewed-by: NJames Morris <james.l.morris@oracle.com>
      Acked-by: NSerge Hallyn <serge@hallyn.com>
      Reviewed-by: NAndy Lutomirski <luto@kernel.org>
      ee67ae7e
    • K
      commoncap: Refactor to remove bprm_secureexec hook · 46d98eb4
      Kees Cook 提交于
      The commoncap implementation of the bprm_secureexec hook is the only LSM
      that depends on the final call to its bprm_set_creds hook (since it may
      be called for multiple files, it ignores bprm->called_set_creds). As a
      result, it cannot safely _clear_ bprm->secureexec since other LSMs may
      have set it.  Instead, remove the bprm_secureexec hook by introducing a
      new flag to bprm specific to commoncap: cap_elevated. This is similar to
      cap_effective, but that is used for a specific subset of elevated
      privileges, and exists solely to track state from bprm_set_creds to
      bprm_secureexec. As such, it will be removed in the next patch.
      
      Here, set the new bprm->cap_elevated flag when setuid/setgid has happened
      from bprm_fill_uid() or fscapabilities have been prepared. This temporarily
      moves the bprm_secureexec hook to a static inline. The helper will be
      removed in the next patch; this makes the step easier to review and bisect,
      since this does not introduce any changes to inputs nor outputs to the
      "elevated privileges" calculation.
      
      The new flag is merged with the bprm->secureexec flag in setup_new_exec()
      since this marks the end of any further prepare_binprm() calls.
      
      Cc: Andy Lutomirski <luto@kernel.org>
      Signed-off-by: NKees Cook <keescook@chromium.org>
      Reviewed-by: NAndy Lutomirski <luto@kernel.org>
      Acked-by: NJames Morris <james.l.morris@oracle.com>
      Acked-by: NSerge Hallyn <serge@hallyn.com>
      46d98eb4
  22. 20 7月, 2017 1 次提交
  23. 06 3月, 2017 1 次提交
  24. 24 1月, 2017 3 次提交
  25. 19 1月, 2017 1 次提交
  26. 08 10月, 2016 1 次提交
  27. 24 6月, 2016 1 次提交
    • A
      fs: Treat foreign mounts as nosuid · 380cf5ba
      Andy Lutomirski 提交于
      If a process gets access to a mount from a different user
      namespace, that process should not be able to take advantage of
      setuid files or selinux entrypoints from that filesystem.  Prevent
      this by treating mounts from other mount namespaces and those not
      owned by current_user_ns() or an ancestor as nosuid.
      
      This will make it safer to allow more complex filesystems to be
      mounted in non-root user namespaces.
      
      This does not remove the need for MNT_LOCK_NOSUID.  The setuid,
      setgid, and file capability bits can no longer be abused if code in
      a user namespace were to clear nosuid on an untrusted filesystem,
      but this patch, by itself, is insufficient to protect the system
      from abuse of files that, when execed, would increase MAC privilege.
      
      As a more concrete explanation, any task that can manipulate a
      vfsmount associated with a given user namespace already has
      capabilities in that namespace and all of its descendents.  If they
      can cause a malicious setuid, setgid, or file-caps executable to
      appear in that mount, then that executable will only allow them to
      elevate privileges in exactly the set of namespaces in which they
      are already privileges.
      
      On the other hand, if they can cause a malicious executable to
      appear with a dangerous MAC label, running it could change the
      caller's security context in a way that should not have been
      possible, even inside the namespace in which the task is confined.
      
      As a hardening measure, this would have made CVE-2014-5207 much
      more difficult to exploit.
      Signed-off-by: NAndy Lutomirski <luto@amacapital.net>
      Signed-off-by: NSeth Forshee <seth.forshee@canonical.com>
      Acked-by: NJames Morris <james.l.morris@oracle.com>
      Acked-by: NSerge Hallyn <serge.hallyn@canonical.com>
      Signed-off-by: NEric W. Biederman <ebiederm@xmission.com>
      380cf5ba