This project is mirrored from https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git. Pull mirroring updated .
  1. 03 Sep, 2021 1 commit
    • Eric Biggers's avatar
      fscrypt: add fscrypt_symlink_getattr() for computing st_size · 38342724
      Eric Biggers authored
      commit d1876056 upstream.
      
      Add a helper function fscrypt_symlink_getattr() which will be called
      from the various filesystems' ->getattr() methods to read and decrypt
      the target of encrypted symlinks in order to report the correct st_size.
      
      Detailed explanation:
      
      As required by POSIX and as documented in various man pages, st_size for
      a symlink is supposed to be the length of the symlink target.
      Unfortunately, st_size has always been wrong for encrypted symlinks
      because st_size is populated from i_size from disk, which intentionally
      contains the length of the encrypted symlink target.  That's slightly
      greater than the length of the decrypted symlink target (which is the
      symlink target that userspace usually sees), and usually won't match the
      length of the no-key encoded symlink target either.
      
      This hadn't been fixed yet because reporting the correct st_size would
      require reading the symlink target from disk and decrypting or encoding
      it, which historically has been considered too heavyweight to do in
      ->getattr().  Also historically, the wrong st_size had only broken a
      test (LTP lstat03) and there were no known complaints from real users.
      (This is probably because the st_size of symlinks isn't used too often,
      and when it is, typically it's for a hint for what buffer size to pass
      to readlink() -- which a slightly-too-large size still works for.)
      
      However, a couple things have changed now.  First, there have recently
      been complaints about the current behavior from real users:
      
      - Breakage in rpmbuild:
        https://github.com/rpm-software-management/rpm/issues/1682
        https://github.com/google/fscrypt/issues/305
      
      - Breakage in toybox cpio:
        https://www.mail-archive.com/toybox@lists.landley.net/msg07193.html
      
      - Breakage in libgit2: https://issuetracker.google.com/issues/189629152
        (on Android public issue tracker, requires login)
      
      Second, we now cache decrypted symlink targets in ->i_link.  Therefore,
      taking the performance hit of reading and decrypting the symlink target
      in ->getattr() wouldn't be as big a deal as it used to be, since usually
      it will just save having to do the same thing later.
      
      Also note that eCryptfs ended up having to read and decrypt symlink
      targets in ->getattr() as well, to fix this same issue; see
      commit 3a60a168 ("eCryptfs: Decrypt symlink target for stat size").
      
      So, let's just bite the bullet, and read and decrypt the symlink target
      in ->getattr() in order to report the correct st_size.  Add a function
      fscrypt_symlink_getattr() which the filesystems will call to do this.
      
      (Alternatively, we could store the decrypted size of symlinks on-disk.
      But there isn't a great place to do so, and encryption is meant to hide
      the original size to some extent; that property would be lost.)
      
      Cc: stable@vger.kernel.org
      Link: https://lore.kernel.org/r/20210702065350.209646-2-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      38342724
  2. 03 Dec, 2020 4 commits
    • Daniel Rosenberg's avatar
      fscrypt: Have filesystems handle their d_ops · bb9cd910
      Daniel Rosenberg authored
      
      
      This shifts the responsibility of setting up dentry operations from
      fscrypt to the individual filesystems, allowing them to have their own
      operations while still setting fscrypt's d_revalidate as appropriate.
      
      Most filesystems can just use generic_set_encrypted_ci_d_ops, unless
      they have their own specific dentry operations as well. That operation
      will set the minimal d_ops required under the circumstances.
      
      Since the fscrypt d_ops are set later on, we must set all d_ops there,
      since we cannot adjust those later on. This should not result in any
      change in behavior.
      Signed-off-by: default avatarDaniel Rosenberg <drosen@google.com>
      Acked-by: default avatarTheodore Ts'o <tytso@mit.edu>
      Acked-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarJaegeuk Kim <jaegeuk@kernel.org>
      bb9cd910
    • Eric Biggers's avatar
      fscrypt: allow deleting files with unsupported encryption policy · a14d0b67
      Eric Biggers authored
      
      
      Currently it's impossible to delete files that use an unsupported
      encryption policy, as the kernel will just return an error when
      performing any operation on the top-level encrypted directory, even just
      a path lookup into the directory or opening the directory for readdir.
      
      More specifically, this occurs in any of the following cases:
      
      - The encryption context has an unrecognized version number.  Current
        kernels know about v1 and v2, but there could be more versions in the
        future.
      
      - The encryption context has unrecognized encryption modes
        (FSCRYPT_MODE_*) or flags (FSCRYPT_POLICY_FLAG_*), an unrecognized
        combination of modes, or reserved bits set.
      
      - The encryption key has been added and the encryption modes are
        recognized but aren't available in the crypto API -- for example, a
        directory is encrypted with FSCRYPT_MODE_ADIANTUM but the kernel
        doesn't have CONFIG_CRYPTO_ADIANTUM enabled.
      
      It's desirable to return errors for most operations on files that use an
      unsupported encryption policy, but the current behavior is too strict.
      We need to allow enough to delete files, so that people can't be stuck
      with undeletable files when downgrading kernel versions.  That includes
      allowing directories to be listed and allowing dentries to be looked up.
      
      Fix this by modifying the key setup logic to treat an unsupported
      encryption policy in the same way as "key unavailable" in the cases that
      are required for a recursive delete to work: preparing for a readdir or
      a dentry lookup, revalidating a dentry, or checking whether an inode has
      the same encryption policy as its parent directory.
      Reviewed-by: default avatarAndreas Dilger <adilger@dilger.ca>
      Link: https://lore.kernel.org/r/20201203022041.230976-10-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      a14d0b67
    • Eric Biggers's avatar
      fscrypt: move body of fscrypt_prepare_setattr() out-of-line · 7622350e
      Eric Biggers authored
      
      
      In preparation for reducing the visibility of fscrypt_require_key() by
      moving it to fscrypt_private.h, move the call to it from
      fscrypt_prepare_setattr() to an out-of-line function.
      Reviewed-by: default avatarAndreas Dilger <adilger@dilger.ca>
      Link: https://lore.kernel.org/r/20201203022041.230976-7-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      7622350e
    • Eric Biggers's avatar
      fscrypt: introduce fscrypt_prepare_readdir() · ec0caa97
      Eric Biggers authored
      
      
      The last remaining use of fscrypt_get_encryption_info() from filesystems
      is for readdir (->iterate_shared()).  Every other call is now in
      fs/crypto/ as part of some other higher-level operation.
      
      We need to add a new argument to fscrypt_get_encryption_info() to
      indicate whether the encryption policy is allowed to be unrecognized or
      not.  Doing this is easier if we can work with high-level operations
      rather than direct filesystem use of fscrypt_get_encryption_info().
      
      So add a function fscrypt_prepare_readdir() which wraps the call to
      fscrypt_get_encryption_info() for the readdir use case.
      Reviewed-by: default avatarAndreas Dilger <adilger@dilger.ca>
      Link: https://lore.kernel.org/r/20201203022041.230976-6-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      ec0caa97
  3. 24 Nov, 2020 3 commits
    • Eric Biggers's avatar
      fscrypt: simplify master key locking · 4a4b8721
      Eric Biggers authored
      The stated reasons for separating fscrypt_master_key::mk_secret_sem from
      the standard semaphore contained in every 'struct key' no longer apply.
      
      First, due to commit a992b20c ("fscrypt: add
      fscrypt_prepare_new_inode() and fscrypt_set_context()"),
      fscrypt_get_encryption_info() is no longer called from within a
      filesystem transaction.
      
      Second, due to commit d3ec10aa ("KEYS: Don't write out to userspace
      while holding key semaphore"), the semaphore for the "keyring" key type
      no longer ranks above page faults.
      
      That leaves performance as the only possible reason to keep the separate
      mk_secret_sem.  Specifically, having mk_secret_sem reduces the
      contention between setup_file_encryption_key() and
      FS_IOC_{ADD,REMOVE}_ENCRYPTION_KEY.  However, these ioctls aren't
      executed often, so this doesn't seem to be worth the extra complexity.
      
      Therefore, simplify the locking design by just using key->sem instead of
      mk_secret_sem.
      
      Link: https://lore.kernel.org/r/20201117032626.320275-1-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      4a4b8721
    • Eric Biggers's avatar
      fscrypt: remove unnecessary calls to fscrypt_require_key() · 234f1b7f
      Eric Biggers authored
      In an encrypted directory, a regular dentry (one that doesn't have the
      no-key name flag) can only be created if the directory's encryption key
      is available.
      
      Therefore the calls to fscrypt_require_key() in __fscrypt_prepare_link()
      and __fscrypt_prepare_rename() are unnecessary, as these functions
      already check that the dentries they're given aren't no-key names.
      
      Remove these unnecessary calls to fscrypt_require_key().
      
      Link: https://lore.kernel.org/r/20201118075609.120337-6-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      234f1b7f
    • Eric Biggers's avatar
      fscrypt: add fscrypt_is_nokey_name() · 159e1de2
      Eric Biggers authored
      It's possible to create a duplicate filename in an encrypted directory
      by creating a file concurrently with adding the encryption key.
      
      Specifically, sys_open(O_CREAT) (or sys_mkdir(), sys_mknod(), or
      sys_symlink()) can lookup the target filename while the directory's
      encryption key hasn't been added yet, resulting in a negative no-key
      dentry.  The VFS then calls ->create() (or ->mkdir(), ->mknod(), or
      ->symlink()) because the dentry is negative.  Normally, ->create() would
      return -ENOKEY due to the directory's key being unavailable.  However,
      if the key was added between the dentry lookup and ->create(), then the
      filesystem will go ahead and try to create the file.
      
      If the target filename happens to already exist as a normal name (not a
      no-key name), a duplicate filename may be added to the directory.
      
      In order to fix this, we need to fix the filesystems to prevent
      ->create(), ->mkdir(), ->mknod(), and ->symlink() on no-key names.
      (->rename() and ->link() need it too, but those are already handled
      correctly by fscrypt_prepare_rename() and fscrypt_prepare_link().)
      
      In preparation for this, add a helper function fscrypt_is_nokey_name()
      that filesystems can use to do this check.  Use this helper function for
      the existing checks that fs/crypto/ does for rename and link.
      
      Cc: stable@vger.kernel.org
      Link: https://lore.kernel.org/r/20201118075609.120337-2-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      159e1de2
  4. 24 Sep, 2020 2 commits
  5. 22 Sep, 2020 3 commits
    • Eric Biggers's avatar
      fscrypt: handle test_dummy_encryption in more logical way · ac4acb1f
      Eric Biggers authored
      The behavior of the test_dummy_encryption mount option is that when a
      new file (or directory or symlink) is created in an unencrypted
      directory, it's automatically encrypted using a dummy encryption policy.
      That's it; in particular, the encryption (or lack thereof) of existing
      files (or directories or symlinks) doesn't change.
      
      Unfortunately the implementation of test_dummy_encryption is a bit weird
      and confusing.  When test_dummy_encryption is enabled and a file is
      being created in an unencrypted directory, we set up an encryption key
      (->i_crypt_info) for the directory.  This isn't actually used to do any
      encryption, however, since the directory is still unencrypted!  Instead,
      ->i_crypt_info is only used for inheriting the encryption policy.
      
      One consequence of this is that the filesystem ends up providing a
      "dummy context" (policy + nonce) instead of a "dummy policy".  In
      commit ed318a6c
      
       ("fscrypt: support test_dummy_encryption=v2"), I
      mistakenly thought this was required.  However, actually the nonce only
      ends up being used to derive a key that is never used.
      
      Another consequence of this implementation is that it allows for
      'inode->i_crypt_info != NULL && !IS_ENCRYPTED(inode)', which is an edge
      case that can be forgotten about.  For example, currently
      FS_IOC_GET_ENCRYPTION_POLICY on an unencrypted directory may return the
      dummy encryption policy when the filesystem is mounted with
      test_dummy_encryption.  That seems like the wrong thing to do, since
      again, the directory itself is not actually encrypted.
      
      Therefore, switch to a more logical and maintainable implementation
      where the dummy encryption policy inheritance is done without setting up
      keys for unencrypted directories.  This involves:
      
      - Adding a function fscrypt_policy_to_inherit() which returns the
        encryption policy to inherit from a directory.  This can be a real
        policy, a dummy policy, or no policy.
      
      - Replacing struct fscrypt_dummy_context, ->get_dummy_context(), etc.
        with struct fscrypt_dummy_policy, ->get_dummy_policy(), etc.
      
      - Making fscrypt_fname_encrypted_size() take an fscrypt_policy instead
        of an inode.
      Acked-by: default avatarJaegeuk Kim <jaegeuk@kernel.org>
      Acked-by: default avatarJeff Layton <jlayton@kernel.org>
      Link: https://lore.kernel.org/r/20200917041136.178600-13-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      ac4acb1f
    • Eric Biggers's avatar
      fscrypt: move fscrypt_prepare_symlink() out-of-line · 31114726
      Eric Biggers authored
      
      
      In preparation for moving the logic for "get the encryption policy
      inherited by new files in this directory" to a single place, make
      fscrypt_prepare_symlink() a regular function rather than an inline
      function that wraps __fscrypt_prepare_symlink().
      
      This way, the new function fscrypt_policy_to_inherit() won't need to be
      exported to filesystems.
      Acked-by: default avatarJeff Layton <jlayton@kernel.org>
      Link: https://lore.kernel.org/r/20200917041136.178600-12-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      31114726
    • Eric Biggers's avatar
      fscrypt: require that fscrypt_encrypt_symlink() already has key · 4cc1a3e7
      Eric Biggers authored
      
      
      Now that all filesystems have been converted to use
      fscrypt_prepare_new_inode(), the encryption key for new symlink inodes
      is now already set up whenever we try to encrypt the symlink target.
      Enforce this rather than try to set up the key again when it may be too
      late to do so safely.
      Acked-by: default avatarJeff Layton <jlayton@kernel.org>
      Link: https://lore.kernel.org/r/20200917041136.178600-9-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      4cc1a3e7
  6. 07 Sep, 2020 1 commit
  7. 12 May, 2020 1 commit
    • Eric Biggers's avatar
      fscrypt: fix all kerneldoc warnings · d2fe9754
      Eric Biggers authored
      Fix all kerneldoc warnings in fs/crypto/ and include/linux/fscrypt.h.
      Most of these were due to missing documentation for function parameters.
      
      Detected with:
      
          scripts/kernel-doc -v -none fs/crypto/*.{c,h} include/linux/fscrypt.h
      
      This cleanup makes it possible to check new patches for kerneldoc
      warnings without having to filter out all the existing ones.
      
      For consistency, also adjust some function "brief descriptions" to
      include the parentheses and to wrap at 80 characters.  (The latter
      matches the checkpatch expectation.)
      
      Link: https://lore.kernel.org/r/20200511191358.53096-2-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      d2fe9754
  8. 22 Jan, 2020 3 commits
    • Daniel Rosenberg's avatar
      fscrypt: derive dirhash key for casefolded directories · aa408f83
      Daniel Rosenberg authored
      
      
      When we allow indexed directories to use both encryption and
      casefolding, for the dirhash we can't just hash the ciphertext filenames
      that are stored on-disk (as is done currently) because the dirhash must
      be case insensitive, but the stored names are case-preserving.  Nor can
      we hash the plaintext names with an unkeyed hash (or a hash keyed with a
      value stored on-disk like ext4's s_hash_seed), since that would leak
      information about the names that encryption is meant to protect.
      
      Instead, if we can accept a dirhash that's only computable when the
      fscrypt key is available, we can hash the plaintext names with a keyed
      hash using a secret key derived from the directory's fscrypt master key.
      We'll use SipHash-2-4 for this purpose.
      
      Prepare for this by deriving a SipHash key for each casefolded encrypted
      directory.  Make sure to handle deriving the key not only when setting
      up the directory's fscrypt_info, but also in the case where the casefold
      flag is enabled after the fscrypt_info was already set up.  (We could
      just always derive the key regardless of casefolding, but that would
      introduce unnecessary overhead for people not using casefolding.)
      Signed-off-by: default avatarDaniel Rosenberg <drosen@google.com>
      [EB: improved commit message, updated fscrypt.rst, squashed with change
       that avoids unnecessarily deriving the key, and many other cleanups]
      Link: https://lore.kernel.org/r/20200120223201.241390-3-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      aa408f83
    • Daniel Rosenberg's avatar
      fscrypt: don't allow v1 policies with casefolding · 6e1918cf
      Daniel Rosenberg authored
      
      
      Casefolded encrypted directories will use a new dirhash method that
      requires a secret key.  If the directory uses a v2 encryption policy,
      it's easy to derive this key from the master key using HKDF.  However,
      v1 encryption policies don't provide a way to derive additional keys.
      
      Therefore, don't allow casefolding on directories that use a v1 policy.
      Specifically, make it so that trying to enable casefolding on a
      directory that has a v1 policy fails, trying to set a v1 policy on a
      casefolded directory fails, and trying to open a casefolded directory
      that has a v1 policy (if one somehow exists on-disk) fails.
      Signed-off-by: default avatarDaniel Rosenberg <drosen@google.com>
      [EB: improved commit message, updated fscrypt.rst, and other cleanups]
      Link: https://lore.kernel.org/r/20200120223201.241390-2-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      6e1918cf
    • Eric Biggers's avatar
      fscrypt: add "fscrypt_" prefix to fname_encrypt() · 1b3b827e
      Eric Biggers authored
      fname_encrypt() is a global function, due to being used in both fname.c
      and hooks.c.  So it should be prefixed with "fscrypt_", like all the
      other global functions in fs/crypto/.
      
      Link: https://lore.kernel.org/r/20200120071736.45915-1-ebiggers@kernel.org
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      1b3b827e
  9. 13 Aug, 2019 1 commit
  10. 11 Jun, 2019 1 commit
  11. 21 May, 2019 1 commit
  12. 17 Apr, 2019 5 commits
    • Eric Biggers's avatar
      fscrypt: cache decrypted symlink target in ->i_link · 2c58d548
      Eric Biggers authored
      
      
      Path lookups that traverse encrypted symlink(s) are very slow because
      each encrypted symlink needs to be decrypted each time it's followed.
      This also involves dropping out of rcu-walk mode.
      
      Make encrypted symlinks faster by caching the decrypted symlink target
      in ->i_link.  The first call to fscrypt_get_symlink() sets it.  Then,
      the existing VFS path lookup code uses the non-NULL ->i_link to take the
      fast path where ->get_link() isn't called, and lookups in rcu-walk mode
      remain in rcu-walk mode.
      
      Also set ->i_link immediately when a new encrypted symlink is created.
      
      To safely free the symlink target after an RCU grace period has elapsed,
      introduce a new function fscrypt_free_inode(), and make the relevant
      filesystems call it just before actually freeing the inode.
      
      Cc: Al Viro <viro@zeniv.linux.org.uk>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      2c58d548
    • Eric Biggers's avatar
      fscrypt: fix race where ->lookup() marks plaintext dentry as ciphertext · b01531db
      Eric Biggers authored
      ->lookup() in an encrypted directory begins as follows:
      
      1. fscrypt_prepare_lookup():
          a. Try to load the directory's encryption key.
          b. If the key is unavailable, mark the dentry as a ciphertext name
             via d_flags.
      2. fscrypt_setup_filename():
          a. Try to load the directory's encryption key.
          b. If the key is available, encrypt the name (treated as a plaintext
             name) to get the on-disk name.  Otherwise decode the name
             (treated as a ciphertext name) to get the on-disk name.
      
      But if the key is concurrently added, it may be found at (2a) but not at
      (1a).  In this case, the dentry will be wrongly marked as a ciphertext
      name even though it was actually treated as plaintext.
      
      This will cause the dentry to be wrongly invalidated on the next lookup,
      potentially causing problems.  For example, if the racy ->lookup() was
      part of sys_mount(), then the new mount will be detached when anything
      tries to access it.  This is despite the mountpoint having a plaintext
      path, which should remain valid now that the key was added.
      
      Of course, this is only possible if there's a userspace race.  Still,
      the additional kernel-side race is confusing and unexpected.
      
      Close the kernel-side race by changing fscrypt_prepare_lookup() to also
      set the on-disk filename (step 2b), consistent with the d_flags update.
      
      Fixes: 28b4c263
      
       ("ext4 crypto: revalidate dentry after adding or removing the key")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      b01531db
    • Eric Biggers's avatar
      fscrypt: only set dentry_operations on ciphertext dentries · d456a33f
      Eric Biggers authored
      
      
      Plaintext dentries are always valid, so only set fscrypt_d_ops on
      ciphertext dentries.
      
      Besides marginally improved performance, this allows overlayfs to use an
      fscrypt-encrypted upperdir, provided that all the following are true:
      
          (1) The fscrypt encryption key is placed in the keyring before
      	mounting overlayfs, and remains while the overlayfs is mounted.
      
          (2) The overlayfs workdir uses the same encryption policy.
      
          (3) No dentries for the ciphertext names of subdirectories have been
      	created in the upperdir or workdir yet.  (Since otherwise
      	d_splice_alias() will reuse the old dentry with ->d_op set.)
      
      One potential use case is using an ephemeral encryption key to encrypt
      all files created or changed by a container, so that they can be
      securely erased ("crypto-shredded") after the container stops.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      d456a33f
    • Eric Biggers's avatar
      fscrypt: fix race allowing rename() and link() of ciphertext dentries · 968dd6d0
      Eric Biggers authored
      
      
      Close some race conditions where fscrypt allowed rename() and link() on
      ciphertext dentries that had been looked up just prior to the key being
      concurrently added.  It's better to return -ENOKEY in this case.
      
      This avoids doing the nonsensical thing of encrypting the names a second
      time when searching for the actual on-disk dir entries.  It also
      guarantees that DCACHE_ENCRYPTED_NAME dentries are never rename()d, so
      the dcache won't have support all possible combinations of moving
      DCACHE_ENCRYPTED_NAME around during __d_move().
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      968dd6d0
    • Eric Biggers's avatar
      fscrypt: clean up and improve dentry revalidation · 6cc24868
      Eric Biggers authored
      
      
      Make various improvements to fscrypt dentry revalidation:
      
      - Don't try to handle the case where the per-directory key is removed,
        as this can't happen without the inode (and dentries) being evicted.
      
      - Flag ciphertext dentries rather than plaintext dentries, since it's
        ciphertext dentries that need the special handling.
      
      - Avoid doing unnecessary work for non-ciphertext dentries.
      
      - When revalidating ciphertext dentries, try to set up the directory's
        i_crypt_info to make sure the key is really still absent, rather than
        invalidating all negative dentries as the previous code did.  An old
        comment suggested we can't do this for locking reasons, but AFAICT
        this comment was outdated and it actually works fine.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      6cc24868
  13. 24 Jan, 2019 1 commit
    • Eric Biggers's avatar
      fscrypt: return -EXDEV for incompatible rename or link into encrypted dir · f5e55e77
      Eric Biggers authored
      Currently, trying to rename or link a regular file, directory, or
      symlink into an encrypted directory fails with EPERM when the source
      file is unencrypted or is encrypted with a different encryption policy,
      and is on the same mountpoint.  It is correct for the operation to fail,
      but the choice of EPERM breaks tools like 'mv' that know to copy rather
      than rename if they see EXDEV, but don't know what to do with EPERM.
      
      Our original motivation for EPERM was to encourage users to securely
      handle their data.  Encrypting files by "moving" them into an encrypted
      directory can be insecure because the unencrypted data may remain in
      free space on disk, where it can later be recovered by an attacker.
      It's much better to encrypt the data from the start, or at least try to
      securely delete the source data e.g. using the 'shred' program.
      
      However, the current behavior hasn't been effective at achieving its
      goal because users tend to be confused, hack around it, and complain;
      see e.g. https://github.com/google/fscrypt/issues/76
      
      .  And in some cases
      it's actually inconsistent or unnecessary.  For example, 'mv'-ing files
      between differently encrypted directories doesn't work even in cases
      where it can be secure, such as when in userspace the same passphrase
      protects both directories.  Yet, you *can* already 'mv' unencrypted
      files into an encrypted directory if the source files are on a different
      mountpoint, even though doing so is often insecure.
      
      There are probably better ways to teach users to securely handle their
      files.  For example, the 'fscrypt' userspace tool could provide a
      command that migrates unencrypted files into an encrypted directory,
      acting like 'shred' on the source files and providing appropriate
      warnings depending on the type of the source filesystem and disk.
      
      Receiving errors on unimportant files might also force some users to
      disable encryption, thus making the behavior counterproductive.  It's
      desirable to make encryption as unobtrusive as possible.
      
      Therefore, change the error code from EPERM to EXDEV so that tools
      looking for EXDEV will fall back to a copy.
      
      This, of course, doesn't prevent users from still doing the right things
      to securely manage their files.  Note that this also matches the
      behavior when a file is renamed between two project quota hierarchies;
      so there's precedent for using EXDEV for things other than mountpoints.
      
      xfstests generic/398 will require an update with this change.
      
      [Rewritten from an earlier patch series by Michael Halcrow.]
      
      Cc: Michael Halcrow <mhalcrow@google.com>
      Cc: Joe Richey <joerichey@google.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      f5e55e77
  14. 20 May, 2018 1 commit
  15. 01 Feb, 2018 1 commit
  16. 12 Jan, 2018 4 commits
    • Eric Biggers's avatar
      fscrypt: fix up fscrypt_fname_encrypted_size() for internal use · b9db0b4a
      Eric Biggers authored
      
      
      Filesystems don't need fscrypt_fname_encrypted_size() anymore, so
      unexport it and move it to fscrypt_private.h.
      
      We also never calculate the encrypted size of a filename without having
      the fscrypt_info present since it is needed to know the amount of
      NUL-padding which is determined by the encryption policy, and also we
      will always truncate the NUL-padding to the maximum filename length.
      Therefore, also make fscrypt_fname_encrypted_size() assume that the
      fscrypt_info is present, and make it truncate the returned length to the
      specified max_len.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      b9db0b4a
    • Eric Biggers's avatar
      fscrypt: calculate NUL-padding length in one place only · 50c961de
      Eric Biggers authored
      
      
      Currently, when encrypting a filename (either a real filename or a
      symlink target) we calculate the amount of NUL-padding twice: once
      before encryption and once during encryption in fname_encrypt().  It is
      needed before encryption to allocate the needed buffer size as well as
      calculate the size the symlink target will take up on-disk before
      creating the symlink inode.  Calculating the size during encryption as
      well is redundant.
      
      Remove this redundancy by always calculating the exact size beforehand,
      and making fname_encrypt() just add as much NUL padding as is needed to
      fill the output buffer.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      50c961de
    • Eric Biggers's avatar
      fscrypt: new helper function - fscrypt_get_symlink() · 3b0d8837
      Eric Biggers authored
      
      
      Filesystems also have duplicate code to support ->get_link() on
      encrypted symlinks.  Factor it out into a new function
      fscrypt_get_symlink().  It takes in the contents of the encrypted
      symlink on-disk and provides the target (decrypted or encoded) that
      should be returned from ->get_link().
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      3b0d8837
    • Eric Biggers's avatar
      fscrypt: new helper functions for ->symlink() · 76e81d6d
      Eric Biggers authored
      
      
      Currently, filesystems supporting fscrypt need to implement some tricky
      logic when creating encrypted symlinks, including handling a peculiar
      on-disk format (struct fscrypt_symlink_data) and correctly calculating
      the size of the encrypted symlink.  Introduce helper functions to make
      things a bit easier:
      
      - fscrypt_prepare_symlink() computes and validates the size the symlink
        target will require on-disk.
      - fscrypt_encrypt_symlink() creates the encrypted target if needed.
      
      The new helpers actually fix some subtle bugs.  First, when checking
      whether the symlink target was too long, filesystems didn't account for
      the fact that the NUL padding is meant to be truncated if it would cause
      the maximum length to be exceeded, as is done for filenames in
      directories.  Consequently users would receive ENAMETOOLONG when
      creating symlinks close to what is supposed to be the maximum length.
      For example, with EXT4 with a 4K block size, the maximum symlink target
      length in an encrypted directory is supposed to be 4093 bytes (in
      comparison to 4095 in an unencrypted directory), but in
      FS_POLICY_FLAGS_PAD_32-mode only up to 4064 bytes were accepted.
      
      Second, symlink targets of "." and ".." were not being encrypted, even
      though they should be, as these names are special in *directory entries*
      but not in symlink targets.  Fortunately, we can fix this simply by
      starting to encrypt them, as old kernels already accept them in
      encrypted form.
      
      Third, the output string length the filesystems were providing when
      doing the actual encryption was incorrect, as it was forgotten to
      exclude 'sizeof(struct fscrypt_symlink_data)'.  Fortunately though, this
      bug didn't make a difference.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
      76e81d6d
  17. 18 Oct, 2017 4 commits