Forum | Documentation | Website | Blog

Skip to content
Snippets Groups Projects
  1. Oct 09, 2023
  2. Sep 01, 2022
  3. Apr 19, 2021
  4. Mar 20, 2021
  5. Jan 30, 2021
  6. Feb 14, 2020
  7. Jul 10, 2019
  8. Jun 27, 2019
    • David Howells's avatar
      keys: Replace uid/gid/perm permissions checking with an ACL · 2e12256b
      David Howells authored
      
      Replace the uid/gid/perm permissions checking on a key with an ACL to allow
      the SETATTR and SEARCH permissions to be split.  This will also allow a
      greater range of subjects to represented.
      
      ============
      WHY DO THIS?
      ============
      
      The problem is that SETATTR and SEARCH cover a slew of actions, not all of
      which should be grouped together.
      
      For SETATTR, this includes actions that are about controlling access to a
      key:
      
       (1) Changing a key's ownership.
      
       (2) Changing a key's security information.
      
       (3) Setting a keyring's restriction.
      
      And actions that are about managing a key's lifetime:
      
       (4) Setting an expiry time.
      
       (5) Revoking a key.
      
      and (proposed) managing a key as part of a cache:
      
       (6) Invalidating a key.
      
      Managing a key's lifetime doesn't really have anything to do with
      controlling access to that key.
      
      Expiry time is awkward since it's more about the lifetime of the content
      and so, in some ways goes better with WRITE permission.  It can, however,
      be set unconditionally by a process with an appropriate authorisation token
      for instantiating a key, and can also be set by the key type driver when a
      key is instantiated, so lumping it with the access-controlling actions is
      probably okay.
      
      As for SEARCH permission, that currently covers:
      
       (1) Finding keys in a keyring tree during a search.
      
       (2) Permitting keyrings to be joined.
      
       (3) Invalidation.
      
      But these don't really belong together either, since these actions really
      need to be controlled separately.
      
      Finally, there are number of special cases to do with granting the
      administrator special rights to invalidate or clear keys that I would like
      to handle with the ACL rather than key flags and special checks.
      
      
      ===============
      WHAT IS CHANGED
      ===============
      
      The SETATTR permission is split to create two new permissions:
      
       (1) SET_SECURITY - which allows the key's owner, group and ACL to be
           changed and a restriction to be placed on a keyring.
      
       (2) REVOKE - which allows a key to be revoked.
      
      The SEARCH permission is split to create:
      
       (1) SEARCH - which allows a keyring to be search and a key to be found.
      
       (2) JOIN - which allows a keyring to be joined as a session keyring.
      
       (3) INVAL - which allows a key to be invalidated.
      
      The WRITE permission is also split to create:
      
       (1) WRITE - which allows a key's content to be altered and links to be
           added, removed and replaced in a keyring.
      
       (2) CLEAR - which allows a keyring to be cleared completely.  This is
           split out to make it possible to give just this to an administrator.
      
       (3) REVOKE - see above.
      
      
      Keys acquire ACLs which consist of a series of ACEs, and all that apply are
      unioned together.  An ACE specifies a subject, such as:
      
       (*) Possessor - permitted to anyone who 'possesses' a key
       (*) Owner - permitted to the key owner
       (*) Group - permitted to the key group
       (*) Everyone - permitted to everyone
      
      Note that 'Other' has been replaced with 'Everyone' on the assumption that
      you wouldn't grant a permit to 'Other' that you wouldn't also grant to
      everyone else.
      
      Further subjects may be made available by later patches.
      
      The ACE also specifies a permissions mask.  The set of permissions is now:
      
      	VIEW		Can view the key metadata
      	READ		Can read the key content
      	WRITE		Can update/modify the key content
      	SEARCH		Can find the key by searching/requesting
      	LINK		Can make a link to the key
      	SET_SECURITY	Can change owner, ACL, expiry
      	INVAL		Can invalidate
      	REVOKE		Can revoke
      	JOIN		Can join this keyring
      	CLEAR		Can clear this keyring
      
      
      The KEYCTL_SETPERM function is then deprecated.
      
      The KEYCTL_SET_TIMEOUT function then is permitted if SET_SECURITY is set,
      or if the caller has a valid instantiation auth token.
      
      The KEYCTL_INVALIDATE function then requires INVAL.
      
      The KEYCTL_REVOKE function then requires REVOKE.
      
      The KEYCTL_JOIN_SESSION_KEYRING function then requires JOIN to join an
      existing keyring.
      
      The JOIN permission is enabled by default for session keyrings and manually
      created keyrings only.
      
      
      ======================
      BACKWARD COMPATIBILITY
      ======================
      
      To maintain backward compatibility, KEYCTL_SETPERM will translate the
      permissions mask it is given into a new ACL for a key - unless
      KEYCTL_SET_ACL has been called on that key, in which case an error will be
      returned.
      
      It will convert possessor, owner, group and other permissions into separate
      ACEs, if each portion of the mask is non-zero.
      
      SETATTR permission turns on all of INVAL, REVOKE and SET_SECURITY.  WRITE
      permission turns on WRITE, REVOKE and, if a keyring, CLEAR.  JOIN is turned
      on if a keyring is being altered.
      
      The KEYCTL_DESCRIBE function translates the ACL back into a permissions
      mask to return depending on possessor, owner, group and everyone ACEs.
      
      It will make the following mappings:
      
       (1) INVAL, JOIN -> SEARCH
      
       (2) SET_SECURITY -> SETATTR
      
       (3) REVOKE -> WRITE if SETATTR isn't already set
      
       (4) CLEAR -> WRITE
      
      Note that the value subsequently returned by KEYCTL_DESCRIBE may not match
      the value set with KEYCTL_SETATTR.
      
      
      =======
      TESTING
      =======
      
      This passes the keyutils testsuite for all but a couple of tests:
      
       (1) tests/keyctl/dh_compute/badargs: The first wrong-key-type test now
           returns EOPNOTSUPP rather than ENOKEY as READ permission isn't removed
           if the type doesn't have ->read().  You still can't actually read the
           key.
      
       (2) tests/keyctl/permitting/valid: The view-other-permissions test doesn't
           work as Other has been replaced with Everyone in the ACL.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      2e12256b
  9. May 30, 2019
  10. Nov 04, 2017
  11. Oct 12, 2017
    • Eric Biggers's avatar
      ecryptfs: fix dereference of NULL user_key_payload · f66665c0
      Eric Biggers authored
      In eCryptfs, we failed to verify that the authentication token keys are
      not revoked before dereferencing their payloads, which is problematic
      because the payload of a revoked key is NULL.  request_key() *does* skip
      revoked keys, but there is still a window where the key can be revoked
      before we acquire the key semaphore.
      
      Fix it by updating ecryptfs_get_key_payload_data() to return
      -EKEYREVOKED if the key payload is NULL.  For completeness we check this
      for "encrypted" keys as well as "user" keys, although encrypted keys
      cannot be revoked currently.
      
      Alternatively we could use key_validate(), but since we'll also need to
      fix ecryptfs_get_key_payload_data() to validate the payload length, it
      seems appropriate to just check the payload pointer.
      
      Fixes: 237fead6
      
       ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      Cc: <stable@vger.kernel.org>    [v2.6.19+]
      Cc: Michael Halcrow <mhalcrow@google.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      f66665c0
  12. Apr 20, 2017
  13. Mar 01, 2017
    • David Howells's avatar
      KEYS: Differentiate uses of rcu_dereference_key() and user_key_payload() · 0837e49a
      David Howells authored
      rcu_dereference_key() and user_key_payload() are currently being used in
      two different, incompatible ways:
      
       (1) As a wrapper to rcu_dereference() - when only the RCU read lock used
           to protect the key.
      
       (2) As a wrapper to rcu_dereference_protected() - when the key semaphor is
           used to protect the key and the may be being modified.
      
      Fix this by splitting both of the key wrappers to produce:
      
       (1) RCU accessors for keys when caller has the key semaphore locked:
      
      	dereference_key_locked()
      	user_key_payload_locked()
      
       (2) RCU accessors for keys when caller holds the RCU read lock:
      
      	dereference_key_rcu()
      	user_key_payload_rcu()
      
      This should fix following warning in the NFS idmapper
      
        ===============================
        [ INFO: suspicious RCU usage. ]
        4.10.0 #1 Tainted: G        W
        -------------------------------
        ./include/keys/user-type.h:53 suspicious rcu_dereference_protected() usage!
        other info that might help us debu...
      0837e49a
  14. Oct 06, 2016
  15. May 27, 2016
  16. Apr 20, 2016
  17. Apr 11, 2016
  18. Feb 22, 2016
  19. Jan 27, 2016
  20. Oct 21, 2015
    • David Howells's avatar
      KEYS: Merge the type-specific data with the payload data · 146aa8b1
      David Howells authored
      
      Merge the type-specific data with the payload data into one four-word chunk
      as it seems pointless to keep them separate.
      
      Use user_key_payload() for accessing the payloads of overloaded
      user-defined keys.
      
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: linux-cifs@vger.kernel.org
      cc: ecryptfs@vger.kernel.org
      cc: linux-ext4@vger.kernel.org
      cc: linux-f2fs-devel@lists.sourceforge.net
      cc: linux-nfs@vger.kernel.org
      cc: ceph-devel@vger.kernel.org
      cc: linux-ima-devel@lists.sourceforge.net
      146aa8b1
  21. Feb 24, 2015
    • Colin Ian King's avatar
      eCryptfs: ensure copy to crypt_stat->cipher does not overrun · 2a559a8b
      Colin Ian King authored
      The patch 237fead6
      
      : "[PATCH] ecryptfs: fs/Makefile and
      fs/Kconfig" from Oct 4, 2006, leads to the following static checker
      warning:
      
        fs/ecryptfs/crypto.c:846 ecryptfs_new_file_context()
        error: off-by-one overflow 'crypt_stat->cipher' size 32.  rl = '0-32'
      
      There is a mismatch between the size of ecryptfs_crypt_stat.cipher
      and ecryptfs_mount_crypt_stat.global_default_cipher_name causing the
      copy of the cipher name to cause a off-by-one string copy error. This
      fix ensures the space reserved for this string is the same size including
      the trailing zero at the end throughout ecryptfs.
      
      This fix avoids increasing the size of ecryptfs_crypt_stat.cipher
      and also ecryptfs_parse_tag_70_packet_silly_stack.cipher_string and instead
      reduces the of ECRYPTFS_MAX_CIPHER_NAME_SIZE to 31 and includes the + 1 for
      the end of string terminator.
      
      NOTE: An overflow is not possible in practice since the value copied
      into global_default_cipher_name is validated by
      ecryptfs_code_for_cipher_string() at mount time. None of the allowed
      cipher strings are long enough to cause the potential buffer overflow
      fixed by this patch.
      
      Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
      Reported-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
      [tyhicks: Added the NOTE about the overflow not being triggerable]
      Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
      2a559a8b
  22. Oct 24, 2013
  23. Jun 29, 2013
  24. May 09, 2013
    • Tyler Hicks's avatar
      eCryptfs: Use the ablkcipher crypto API · 4dfea4f0
      Tyler Hicks authored
      
      Make the switch from the blkcipher kernel crypto interface to the
      ablkcipher interface.
      
      encrypt_scatterlist() and decrypt_scatterlist() now use the ablkcipher
      interface but, from the eCryptfs standpoint, still treat the crypto
      operation as a synchronous operation. They submit the async request and
      then wait until the operation is finished before they return. Most of
      the changes are contained inside those two functions.
      
      Despite waiting for the completion of the crypto operation, the
      ablkcipher interface provides performance increases in most cases when
      used on AES-NI capable hardware.
      
      Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
      Acked-by: default avatarColin King <colin.king@canonical.com>
      Reviewed-by: default avatarZeev Zilberman <zeev@annapurnaLabs.com>
      Cc: Dustin Kirkland <dustin.kirkland@gazzang.com>
      Cc: Tim Chen <tim.c.chen@intel.com>
      Cc: Ying Huang <ying.huang@intel.com>
      Cc: Thieu Le <thieule@google.com>
      Cc: Li Wang <dragonylffly@163.com>
      Cc: Jarkko Sakkinen <jarkko.sakkinen@iki.fi>
      4dfea4f0
  25. Mar 04, 2013
  26. Feb 26, 2013
  27. Jul 22, 2012
  28. Jul 08, 2012
    • Tyler Hicks's avatar
      eCryptfs: Initialize empty lower files when opening them · e3ccaa97
      Tyler Hicks authored
      Historically, eCryptfs has only initialized lower files in the
      ecryptfs_create() path. Lower file initialization is the act of writing
      the cryptographic metadata from the inode's crypt_stat to the header of
      the file. The ecryptfs_open() path already expects that metadata to be
      in the header of the file.
      
      A number of users have reported empty lower files in beneath their
      eCryptfs mounts. Most of the causes for those empty files being left
      around have been addressed, but the presence of empty files causes
      problems due to the lack of proper cryptographic metadata.
      
      To transparently solve this problem, this patch initializes empty lower
      files in the ecryptfs_open() error path. If the metadata is unreadable
      due to the lower inode size being 0, plaintext passthrough support is
      not in use, and the metadata is stored in the header of the file (as
      opposed to the user.ecryptfs extended attribute), the lower file will be
      initialized.
      
      The number of nested conditionals in ecryptfs_open() was getting out of
      hand, so a helper function was created. To avoid the same nested
      conditional problem, the conditional logic was reversed inside of the
      helper function.
      
      https://launchpad.net/bugs/911507
      
      
      
      Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
      Cc: John Johansen <john.johansen@canonical.com>
      Cc: Colin Ian King <colin.king@canonical.com>
      e3ccaa97
    • Tyler Hicks's avatar
      eCryptfs: Make all miscdev functions use daemon ptr in file private_data · 2ecaf55d
      Tyler Hicks authored
      
      Now that a pointer to a valid struct ecryptfs_daemon is stored in the
      private_data of an opened /dev/ecryptfs file, the remaining miscdev
      functions can utilize the pointer rather than looking up the
      ecryptfs_daemon at the beginning of each operation.
      
      The security model of /dev/ecryptfs is simplified a little bit with this
      patch. Upon opening /dev/ecryptfs, a per-user ecryptfs_daemon is
      registered. Another daemon cannot be registered for that user until the
      last file reference is released. During the lifetime of the
      ecryptfs_daemon, access checks are not performed on the /dev/ecryptfs
      operations because it is assumed that the application securely handles
      the opened file descriptor and does not unintentionally leak it to
      processes that are not trusted.
      
      Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
      Cc: Sasha Levin <levinsasha928@gmail.com>
      2ecaf55d
    • Tyler Hicks's avatar
      eCryptfs: Remove unused messaging declarations and function · 56696886
      Tyler Hicks authored
      
      These are no longer needed.
      
      Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
      Cc: Sasha Levin <levinsasha928@gmail.com>
      56696886
  29. Feb 16, 2012
  30. Jan 25, 2012
  31. Nov 23, 2011
    • Tyler Hicks's avatar
      eCryptfs: Prevent file create race condition · b59db43a
      Tyler Hicks authored
      The file creation path prematurely called d_instantiate() and
      unlock_new_inode() before the eCryptfs inode info was fully
      allocated and initialized and before the eCryptfs metadata was written
      to the lower file.
      
      This could result in race conditions in subsequent file and inode
      operations leading to unexpected error conditions or a null pointer
      dereference while attempting to use the unallocated memory.
      
      https://launchpad.net/bugs/813146
      
      
      
      Signed-off-by: default avatarTyler Hicks <tyhicks@canonical.com>
      Cc: stable@kernel.org
      b59db43a
  32. Oct 31, 2011
  33. Jun 27, 2011
  34. May 29, 2011
    • Tyler Hicks's avatar
      eCryptfs: Remove ecryptfs_header_cache_2 · 30632870
      Tyler Hicks authored
      
      Now that ecryptfs_lookup_interpose() is no longer using
      ecryptfs_header_cache_2 to read in metadata, the kmem_cache can be
      removed and the ecryptfs_header_cache_1 kmem_cache can be renamed to
      ecryptfs_header_cache.
      
      Signed-off-by: default avatarTyler Hicks <tyhicks@linux.vnet.ibm.com>
      30632870
    • Tyler Hicks's avatar
      eCryptfs: Cleanup and optimize ecryptfs_lookup_interpose() · 778aeb42
      Tyler Hicks authored
      
      ecryptfs_lookup_interpose() has turned into spaghetti code over the
      years. This is an effort to clean it up.
      
       - Shorten overly descriptive variable names such as ecryptfs_dentry
       - Simplify gotos and error paths
       - Create helper function for reading plaintext i_size from metadata
      
      It also includes an optimization when reading i_size from the metadata.
      A complete page-sized kmem_cache_alloc() was being done to read in 16
      bytes of metadata. The buffer for that is now statically declared.
      
      Signed-off-by: default avatarTyler Hicks <tyhicks@linux.vnet.ibm.com>
      778aeb42