Forum | Documentation | Website | Blog

Skip to content
Snippets Groups Projects
  1. Jun 16, 2016
    • Dan Carpenter's avatar
      KEYS: potential uninitialized variable · 38327424
      Dan Carpenter authored
      If __key_link_begin() failed then "edit" would be uninitialized.  I've
      added a check to fix that.
      
      This allows a random user to crash the kernel, though it's quite
      difficult to achieve.  There are three ways it can be done as the user
      would have to cause an error to occur in __key_link():
      
       (1) Cause the kernel to run out of memory.  In practice, this is difficult
           to achieve without ENOMEM cropping up elsewhere and aborting the
           attempt.
      
       (2) Revoke the destination keyring between the keyring ID being looked up
           and it being tested for revocation.  In practice, this is difficult to
           time correctly because the KEYCTL_REJECT function can only be used
           from the request-key upcall process.  Further, users can only make use
           of what's in /sbin/request-key.conf, though this does including a
           rejection debugging test - which means that the destination keyring
           has to be the caller's session keyring in practice.
      
       (3) Have just enough key quota available to create a key, a new session
           keyring for the upcall and a link in the session keyring, but not then
           sufficient quota to create a link in the nominated destination keyring
           so that it fails with EDQUOT.
      
      The bug can be triggered using option (3) above using something like the
      following:
      
      	echo 80 >/proc/sys/kernel/keys/root_maxbytes
      	keyctl request2 user debug:fred negate @t
      
      The above sets the quota to something much lower (80) to make the bug
      easier to trigger, but this is dependent on the system.  Note also that
      the name of the keyring created contains a random number that may be
      between 1 and 10 characters in size, so may throw the test off by
      changing the amount of quota used.
      
      Assuming the failure occurs, something like the following will be seen:
      
      	kfree_debugcheck: out of range ptr 6b6b6b6b6b6b6b68h
      	------------[ cut here ]------------
      	kernel BUG at ../mm/slab.c:2821!
      	...
      	RIP: 0010:[<ffffffff811600f9>] kfree_debugcheck+0x20/0x25
      	RSP: 0018:ffff8804014a7de8  EFLAGS: 00010092
      	RAX: 0000000000000034 RBX: 6b6b6b6b6b6b6b68 RCX: 0000000000000000
      	RDX: 0000000000040001 RSI: 00000000000000f6 RDI: 0000000000000300
      	RBP: ffff8804014a7df0 R08: 0000000000000001 R09: 0000000000000000
      	R10: ffff8804014a7e68 R11: 0000000000000054 R12: 0000000000000202
      	R13: ffffffff81318a66 R14: 0000000000000000 R15: 0000000000000001
      	...
      	Call Trace:
      	  kfree+0xde/0x1bc
      	  assoc_array_cancel_edit+0x1f/0x36
      	  __key_link_end+0x55/0x63
      	  key_reject_and_link+0x124/0x155
      	  keyctl_reject_key+0xb6/0xe0
      	  keyctl_negate_key+0x10/0x12
      	  SyS_keyctl+0x9f/0xe7
      	  do_syscall_64+0x63/0x13a
      	  entry_SYSCALL64_slow_path+0x25/0x25
      
      Fixes: f70e2e06
      
       ('KEYS: Do preallocation for __key_link()')
      Signed-off-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: stable@vger.kernel.org
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      38327424
  2. Jun 03, 2016
  3. May 27, 2016
  4. May 25, 2016
    • Jann Horn's avatar
      Yama: fix double-spinlock and user access in atomic context · dca6b414
      Jann Horn authored
      Commit 8a56038c ("Yama: consolidate error reporting") causes lockups
      when someone hits a Yama denial. Call chain:
      
      process_vm_readv -> process_vm_rw -> process_vm_rw_core -> mm_access
      -> ptrace_may_access
      task_lock(...) is taken
      __ptrace_may_access -> security_ptrace_access_check
      -> yama_ptrace_access_check -> report_access -> kstrdup_quotable_cmdline
      -> get_cmdline -> access_process_vm -> get_task_mm
      task_lock(...) is taken again
      
      task_lock(p) just calls spin_lock(&p->alloc_lock), so at this point,
      spin_lock() is called on a lock that is already held by the current
      process.
      
      Also: Since the alloc_lock is a spinlock, sleeping inside
      security_ptrace_access_check hooks is probably not allowed at all? So it's
      not even possible to print the cmdline from in there because that might
      involve paging in userspace memory.
      
      It would be tempting to rewrite ptrace_may_access() to drop the alloc_lock
      before calling the LSM, but even then, ptrace_may_access...
      dca6b414
  5. May 20, 2016
  6. May 17, 2016
  7. May 04, 2016
  8. May 01, 2016
    • Mimi Zohar's avatar
      ima: add support for creating files using the mknodat syscall · 05d1a717
      Mimi Zohar authored
      Commit 3034a146
      
       "ima: pass 'opened' flag to identify newly created files"
      stopped identifying empty files as new files.  However new empty files
      can be created using the mknodat syscall.  On systems with IMA-appraisal
      enabled, these empty files are not labeled with security.ima extended
      attributes properly, preventing them from subsequently being opened in
      order to write the file data contents.  This patch defines a new hook
      named ima_post_path_mknod() to mark these empty files, created using
      mknodat, as new in order to allow the file data contents to be written.
      
      In addition, files with security.ima xattrs containing a file signature
      are considered "immutable" and can not be modified.  The file contents
      need to be written, before signing the file.  This patch relaxes this
      requirement for new files, allowing the file signature to be written
      before the file contents.
      
      Changelog:
      - defer identifying files with signatures stored as security.ima
        (based on Dmitry Rozhkov's comments)
      - removing tests (eg. dentry, dentry->d_inode, inode->i_size == 0)
        (based on Al's review)
      
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      Cc: Al Viro <<viro@zeniv.linux.org.uk>
      Tested-by: default avatarDmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
      05d1a717
    • Mimi Zohar's avatar
      ima: fix ima_inode_post_setattr · 42a4c603
      Mimi Zohar authored
      
      Changing file metadata (eg. uid, guid) could result in having to
      re-appraise a file's integrity, but does not change the "new file"
      status nor the security.ima xattr.  The IMA_PERMIT_DIRECTIO and
      IMA_DIGSIG_REQUIRED flags are policy rule specific.  This patch
      only resets these flags, not the IMA_NEW_FILE or IMA_DIGSIG flags.
      
      With this patch, changing the file timestamp will not remove the
      file signature on new files.
      
      Reported-by: default avatarDmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      Tested-by: default avatarDmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
      42a4c603
  9. Apr 26, 2016
  10. Apr 22, 2016
    • Baolin Wang's avatar
      security: Introduce security_settime64() · 457db29b
      Baolin Wang authored
      
      security_settime() uses a timespec, which is not year 2038 safe
      on 32bit systems. Thus this patch introduces the security_settime64()
      function with timespec64 type. We also convert the cap_settime() helper
      function to use the 64bit types.
      
      This patch then moves security_settime() to the header file as an
      inline helper function so that existing users can be iteratively
      converted.
      
      None of the existing hooks is using the timespec argument and therefor
      the patch is not making any functional changes.
      
      Cc: Serge Hallyn <serge.hallyn@canonical.com>,
      Cc: James Morris <james.l.morris@oracle.com>,
      Cc: "Serge E. Hallyn" <serge@hallyn.com>,
      Cc: Paul Moore <pmoore@redhat.com>
      Cc: Stephen Smalley <sds@tycho.nsa.gov>
      Cc: Kees Cook <keescook@chromium.org>
      Cc: Prarit Bhargava <prarit@redhat.com>
      Cc: Richard Cochran <richardcochran@gmail.com>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      Cc: Ingo Molnar <mingo@kernel.org>
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      Signed-off-by: default avatarBaolin Wang <baolin.wang@linaro.org>
      [jstultz: Reworded commit message]
      Signed-off-by: default avatarJohn Stultz <john.stultz@linaro.org>
      457db29b
  11. Apr 20, 2016
    • Kees Cook's avatar
      LSM: LoadPin for kernel file loading restrictions · 9b091556
      Kees Cook authored
      
      This LSM enforces that kernel-loaded files (modules, firmware, etc)
      must all come from the same filesystem, with the expectation that
      such a filesystem is backed by a read-only device such as dm-verity
      or CDROM. This allows systems that have a verified and/or unchangeable
      filesystem to enforce module and firmware loading restrictions without
      needing to sign the files individually.
      
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Acked-by: default avatarSerge Hallyn <serge.hallyn@canonical.com>
      Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
      9b091556
    • Kees Cook's avatar
      Yama: consolidate error reporting · 8a56038c
      Kees Cook authored
      
      Use a common error reporting function for Yama violation reports, and give
      more detail into the process command lines.
      
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
      8a56038c
    • Roopa Prabhu's avatar
      rtnetlink: add new RTM_GETSTATS message to dump link stats · 10c9ead9
      Roopa Prabhu authored
      This patch adds a new RTM_GETSTATS message to query link stats via netlink
      from the kernel. RTM_NEWLINK also dumps stats today, but RTM_NEWLINK
      returns a lot more than just stats and is expensive in some cases when
      frequent polling for stats from userspace is a common operation.
      
      RTM_GETSTATS is an attempt to provide a light weight netlink message
      to explicity query only link stats from the kernel on an interface.
      The idea is to also keep it extensible so that new kinds of stats can be
      added to it in the future.
      
      This patch adds the following attribute for NETDEV stats:
      struct nla_policy ifla_stats_policy[IFLA_STATS_MAX + 1] = {
              [IFLA_STATS_LINK_64]  = { .len = sizeof(struct rtnl_link_stats64) },
      };
      
      Like any other rtnetlink message, RTM_GETSTATS can be used to get stats of
      a single interface or all interfaces with NLM_F_DUMP.
      
      Future possible new types of stat attributes:
      link af stats:
          - IFLA_STATS_LINK_IPV6  (nested. for ipv6 stats)...
      10c9ead9
  12. Apr 19, 2016
  13. Apr 14, 2016
    • Prarit Bhargava's avatar
      selinux: Change bool variable name to index. · 0fd71a62
      Prarit Bhargava authored
      
      security_get_bool_value(int bool) argument "bool" conflicts with
      in-kernel macros such as BUILD_BUG().  This patch changes this to
      index which isn't a type.
      
      Cc: Paul Moore <paul@paul-moore.com>
      Cc: Stephen Smalley <sds@tycho.nsa.gov>
      Cc: Eric Paris <eparis@parisplace.org>
      Cc: James Morris <james.l.morris@oracle.com>
      Cc: "Serge E. Hallyn" <serge@hallyn.com>
      Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
      Cc: Andrew Perepechko <anserper@ya.ru>
      Cc: Jeff Vander Stoep <jeffv@google.com>
      Cc: selinux@tycho.nsa.gov
      Cc: Eric Paris <eparis@redhat.com>
      Cc: Paul Moore <pmoore@redhat.com>
      Cc: David Howells <dhowells@redhat.com>
      Signed-off-by: default avatarPrarit Bhargava <prarit@redhat.com>
      Acked-by: default avatarDavid Howells <dhowells@redhat.com>
      [PM: wrapped description for checkpatch.pl, use "selinux:..." as subj]
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      0fd71a62
  14. Apr 12, 2016
  15. Apr 11, 2016
    • David Howells's avatar
      IMA: Use the the system trusted keyrings instead of .ima_mok · 56104cf2
      David Howells authored
      Add a config option (IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY)
      that, when enabled, allows keys to be added to the IMA keyrings by
      userspace - with the restriction that each must be signed by a key in the
      system trusted keyrings.
      
      EPERM will be returned if this option is disabled, ENOKEY will be returned if
      no authoritative key can be found and EKEYREJECTED will be returned if the
      signature doesn't match.  Other errors such as ENOPKG may also be returned.
      
      If this new option is enabled, the builtin system keyring is searched, as is
      the secondary system keyring if that is also enabled.  Intermediate keys
      between the builtin system keyring and the key being added can be added to
      the secondary keyring (which replaces .ima_mok) to form a trust chain -
      provided they are also validly signed by a key in one of the trusted keyrings.
      
      The .ima_mok keyring is then removed and the IMA blacklist keyring gets its
      own config option (IMA_BLACKLIST_KEY...
      56104cf2
    • David Howells's avatar
      KEYS: Remove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED · 77f68bac
      David Howells authored
      
      Remove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED as they're no longer
      meaningful.  Also we can drop the trusted flag from the preparse structure.
      
      Given this, we no longer need to pass the key flags through to
      restrict_link().
      
      Further, we can now get rid of keyring_restrict_trusted_only() also.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      77f68bac
    • David Howells's avatar
      KEYS: Move the point of trust determination to __key_link() · a511e1af
      David Howells authored
      
      Move the point at which a key is determined to be trustworthy to
      __key_link() so that we use the contents of the keyring being linked in to
      to determine whether the key being linked in is trusted or not.
      
      What is 'trusted' then becomes a matter of what's in the keyring.
      
      Currently, the test is done when the key is parsed, but given that at that
      point we can only sensibly refer to the contents of the system trusted
      keyring, we can only use that as the basis for working out the
      trustworthiness of a new key.
      
      With this change, a trusted keyring is a set of keys that once the
      trusted-only flag is set cannot be added to except by verification through
      one of the contained keys.
      
      Further, adding a key into a trusted keyring, whilst it might grant
      trustworthiness in the context of that keyring, does not automatically
      grant trustworthiness in the context of a second keyring to which it could
      be secondarily linked.
      
      To accomplish this, the authentication data associated with the key source
      must now be retained.  For an X.509 cert, this means the contents of the
      AuthorityKeyIdentifier and the signature data.
      
      
      If system keyrings are disabled then restrict_link_by_builtin_trusted()
      resolves to restrict_link_reject().  The integrity digital signature code
      still works correctly with this as it was previously using
      KEY_FLAG_TRUSTED_ONLY, which doesn't permit anything to be added if there
      is no system keyring against which trust can be determined.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      a511e1af
    • David Howells's avatar
      KEYS: Add a facility to restrict new links into a keyring · 5ac7eace
      David Howells authored
      
      Add a facility whereby proposed new links to be added to a keyring can be
      vetted, permitting them to be rejected if necessary.  This can be used to
      block public keys from which the signature cannot be verified or for which
      the signature verification fails.  It could also be used to provide
      blacklisting.
      
      This affects operations like add_key(), KEYCTL_LINK and KEYCTL_INSTANTIATE.
      
      To this end:
      
       (1) A function pointer is added to the key struct that, if set, points to
           the vetting function.  This is called as:
      
      	int (*restrict_link)(struct key *keyring,
      			     const struct key_type *key_type,
      			     unsigned long key_flags,
      			     const union key_payload *key_payload),
      
           where 'keyring' will be the keyring being added to, key_type and
           key_payload will describe the key being added and key_flags[*] can be
           AND'ed with KEY_FLAG_TRUSTED.
      
           [*] This parameter will be removed in a later patch when
           	 KEY_FLAG_TRUSTED is removed.
      
           The function should return 0 to allow the link to take place or an
           error (typically -ENOKEY, -ENOPKG or -EKEYREJECTED) to reject the
           link.
      
           The pointer should not be set directly, but rather should be set
           through keyring_alloc().
      
           Note that if called during add_key(), preparse is called before this
           method, but a key isn't actually allocated until after this function
           is called.
      
       (2) KEY_ALLOC_BYPASS_RESTRICTION is added.  This can be passed to
           key_create_or_update() or key_instantiate_and_link() to bypass the
           restriction check.
      
       (3) KEY_FLAG_TRUSTED_ONLY is removed.  The entire contents of a keyring
           with this restriction emplaced can be considered 'trustworthy' by
           virtue of being in the keyring when that keyring is consulted.
      
       (4) key_alloc() and keyring_alloc() take an extra argument that will be
           used to set restrict_link in the new key.  This ensures that the
           pointer is set before the key is published, thus preventing a window
           of unrestrictedness.  Normally this argument will be NULL.
      
       (5) As a temporary affair, keyring_restrict_trusted_only() is added.  It
           should be passed to keyring_alloc() as the extra argument instead of
           setting KEY_FLAG_TRUSTED_ONLY on a keyring.  This will be replaced in
           a later patch with functions that look in the appropriate places for
           authoritative keys.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      5ac7eace
    • Al Viro's avatar
  16. Apr 10, 2016
  17. Apr 05, 2016
  18. Mar 28, 2016