03 Aug, 2018

1 commit

  • [ Upstream commit 36dd26e0c8d42699eeba87431246c07c28075bae ]

    Improve fscrypt read performance by switching the decryption workqueue
    from bound to unbound. With the bound workqueue, when multiple bios
    completed on the same CPU, they were decrypted on that same CPU. But
    with the unbound queue, they are now decrypted in parallel on any CPU.

    Although fscrypt read performance can be tough to measure due to the
    many sources of variation, this change is most beneficial when
    decryption is slow, e.g. on CPUs without AES instructions. For example,
    I timed tarring up encrypted directories on f2fs. On x86 with AES-NI
    instructions disabled, the unbound workqueue improved performance by
    about 25-35%, using 1 to NUM_CPUs jobs with 4 or 8 CPUs available. But
    with AES-NI enabled, performance was unchanged to within ~2%.

    I also did the same test on a quad-core ARM CPU using xts-speck128-neon
    encryption. There performance was usually about 10% better with the
    unbound workqueue, bringing it closer to the unencrypted speed.

    The unbound workqueue may be worse in some cases due to worse locality,
    but I think it's still the better default. dm-crypt uses an unbound
    workqueue by default too, so this change makes fscrypt match.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

30 Nov, 2017

1 commit

  • commit a0b3bc855374c50b5ea85273553485af48caf2f7 upstream.

    fscrypt_initialize(), which allocates the global bounce page pool when
    an encrypted file is first accessed, uses "double-checked locking" to
    try to avoid locking fscrypt_init_mutex. However, it doesn't use any
    memory barriers, so it's theoretically possible for a thread to observe
    a bounce page pool which has not been fully initialized. This is a
    classic bug with "double-checked locking".

    While "only a theoretical issue" in the latest kernel, in pre-4.8
    kernels the pointer that was checked was not even the last to be
    initialized, so it was easily possible for a crash (NULL pointer
    dereference) to happen. This was changed only incidentally by the large
    refactor to use fs/crypto/.

    Solve both problems in a trivial way that can easily be backported: just
    always take the mutex. It's theoretically less efficient, but it
    shouldn't be noticeable in practice as the mutex is only acquired very
    briefly once per encrypted file.

    Later I'd like to make this use a helper macro like DO_ONCE(). However,
    DO_ONCE() runs in atomic context, so we'd need to add a new macro that
    allows blocking.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

02 Nov, 2017

1 commit

  • Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the 'GPL-2.0'
    SPDX license identifier. The SPDX identifier is a legally binding
    shorthand, which can be used instead of the full boiler plate text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart and
    Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset of
    the use cases:
    - file had no licensing information it it.
    - file was a */uapi/* one with no licensing information in it,
    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to license
    had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied to
    a file was done in a spreadsheet of side by side results from of the
    output of two independent scanners (ScanCode & Windriver) producing SPDX
    tag:value files created by Philippe Ombredanne. Philippe prepared the
    base worksheet, and did an initial spot review of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537 files
    assessed. Kate Stewart did a file by file comparison of the scanner
    results in the spreadsheet to determine which SPDX license identifier(s)
    to be applied to the file. She confirmed any determination that was not
    immediately clear with lawyers working with the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:
    - Files considered eligible had to be source code files.
    - Make and config files were included as candidates if they contained >5
    lines of source
    - File already had some variant of a license header in it (even if
    Reviewed-by: Philippe Ombredanne
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

13 Oct, 2017

1 commit

  • When an fscrypt-encrypted file is opened, we request the file's master
    key from the keyrings service as a logon key, then access its payload.
    However, a revoked key has a NULL payload, and we failed to check for
    this. request_key() *does* skip revoked keys, but there is still a
    window where the key can be revoked before we acquire its semaphore.

    Fix it by checking for a NULL payload, treating it like a key which was
    already revoked at the time it was requested.

    Fixes: 88bd6ccdcdd6 ("ext4 crypto: add encryption key management facilities")
    Reviewed-by: James Morris
    Cc: [v4.1+]
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells

    Eric Biggers
     

24 Aug, 2017

1 commit

  • This way we don't need a block_device structure to submit I/O. The
    block_device has different life time rules from the gendisk and
    request_queue and is usually only available when the block device node
    is open. Other callers need to explicitly create one (e.g. the lightnvm
    passthrough code, or the new nvme multipathing code).

    For the actual I/O path all that we need is the gendisk, which exists
    once per block device. But given that the block layer also does
    partition remapping we additionally need a partition index, which is
    used for said remapping in generic_make_request.

    Note that all the block drivers generally want request_queue or
    sometimes the gendisk, so this removes a layer of indirection all
    over the stack.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Jens Axboe

    Christoph Hellwig
     

10 Jul, 2017

2 commits

  • Pull ext4 updates from Ted Ts'o:
    "The first major feature for ext4 this merge window is the largedir
    feature, which allows ext4 directories to support over 2 billion
    directory entries (assuming ~64 byte file names; in practice, users
    will run into practical performance limits first.) This feature was
    originally written by the Lustre team, and credit goes to Artem
    Blagodarenko from Seagate for getting this feature upstream.

    The second major major feature allows ext4 to support extended
    attribute values up to 64k. This feature was also originally from
    Lustre, and has been enhanced by Tahsin Erdogan from Google with a
    deduplication feature so that if multiple files have the same xattr
    value (for example, Windows ACL's stored by Samba), only one copy will
    be stored on disk for encoding and caching efficiency.

    We also have the usual set of bug fixes, cleanups, and optimizations"

    * tag 'ext4_for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4: (47 commits)
    ext4: fix spelling mistake: "prellocated" -> "preallocated"
    ext4: fix __ext4_new_inode() journal credits calculation
    ext4: skip ext4_init_security() and encryption on ea_inodes
    fs: generic_block_bmap(): initialize all of the fields in the temp bh
    ext4: change fast symlink test to not rely on i_blocks
    ext4: require key for truncate(2) of encrypted file
    ext4: don't bother checking for encryption key in ->mmap()
    ext4: check return value of kstrtoull correctly in reserved_clusters_store
    ext4: fix off-by-one fsmap error on 1k block filesystems
    ext4: return EFSBADCRC if a bad checksum error is found in ext4_find_entry()
    ext4: return EIO on read error in ext4_find_entry
    ext4: forbid encrypting root directory
    ext4: send parallel discards on commit completions
    ext4: avoid unnecessary stalls in ext4_evict_inode()
    ext4: add nombcache mount option
    ext4: strong binding of xattr inode references
    ext4: eliminate xattr entry e_hash recalculation for removes
    ext4: reserve space for xattr entries/names
    quota: add get_inode_usage callback to transfer multi-inode charges
    ext4: xattr inode deduplication
    ...

    Linus Torvalds
     
  • Pull fscrypt updates from Ted Ts'o:
    "Add support for 128-bit AES and some cleanups to fscrypt"

    * tag 'fscrypt_for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/fscrypt:
    fscrypt: make ->dummy_context() return bool
    fscrypt: add support for AES-128-CBC
    fscrypt: inline fscrypt_free_filename()

    Linus Torvalds
     

06 Jul, 2017

1 commit

  • ea_inode feature allows creating extended attributes that are up to
    64k in size. Update __ext4_new_inode() to pick increased credit limits.

    To avoid overallocating too many journal credits, update
    __ext4_xattr_set_credits() to make a distinction between xattr create
    vs update. This helps __ext4_new_inode() because all attributes are
    known to be new, so we can save credits that are normally needed to
    delete old values.

    Also, have fscrypt specify its maximum context size so that we don't
    end up allocating credits for 64k size.

    Signed-off-by: Tahsin Erdogan
    Signed-off-by: Theodore Ts'o

    Tahsin Erdogan
     

24 Jun, 2017

2 commits

  • fscrypt provides facilities to use different encryption algorithms which
    are selectable by userspace when setting the encryption policy. Currently,
    only AES-256-XTS for file contents and AES-256-CBC-CTS for file names are
    implemented. This is a clear case of kernel offers the mechanism and
    userspace selects a policy. Similar to what dm-crypt and ecryptfs have.

    This patch adds support for using AES-128-CBC for file contents and
    AES-128-CBC-CTS for file name encryption. To mitigate watermarking
    attacks, IVs are generated using the ESSIV algorithm. While AES-CBC is
    actually slightly less secure than AES-XTS from a security point of view,
    there is more widespread hardware support. Using AES-CBC gives us the
    acceptable performance while still providing a moderate level of security
    for persistent storage.

    Especially low-powered embedded devices with crypto accelerators such as
    CAAM or CESA often only support AES-CBC. Since using AES-CBC over AES-XTS
    is basically thought of a last resort, we use AES-128-CBC over AES-256-CBC
    since it has less encryption rounds and yields noticeable better
    performance starting from a file size of just a few kB.

    Signed-off-by: Daniel Walter
    [david@sigma-star.at: addressed review comments]
    Signed-off-by: David Gstir
    Reviewed-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Daniel Walter
     
  • fscrypt_free_filename() only needs to do a kfree() of crypto_buf.name,
    which works well as an inline function. We can skip setting the various
    pointers to NULL, since no user cares about it (the name is always freed
    just before it goes out of scope).

    Signed-off-by: Eric Biggers
    Reviewed-by: David Gstir
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     

09 Jun, 2017

1 commit

  • Replace bi_error with a new bi_status to allow for a clear conversion.
    Note that device mapper overloaded bi_error with a private value, which
    we'll have to keep arround at least for now and thus propagate to a
    proper blk_status_t value.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Jens Axboe

    Christoph Hellwig
     

04 May, 2017

3 commits

  • Introduce a helper function fscrypt_match_name() which tests whether a
    fscrypt_name matches a directory entry. Also clean up the magic numbers
    and document things properly.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • When accessing an encrypted directory without the key, userspace must
    operate on filenames derived from the ciphertext names, which contain
    arbitrary bytes. Since we must support filenames as long as NAME_MAX,
    we can't always just base64-encode the ciphertext, since that may make
    it too long. Currently, this is solved by presenting long names in an
    abbreviated form containing any needed filesystem-specific hashes (e.g.
    to identify a directory block), then the last 16 bytes of ciphertext.
    This needs to be sufficient to identify the actual name on lookup.

    However, there is a bug. It seems to have been assumed that due to the
    use of a CBC (ciphertext block chaining)-based encryption mode, the last
    16 bytes (i.e. the AES block size) of ciphertext would depend on the
    full plaintext, preventing collisions. However, we actually use CBC
    with ciphertext stealing (CTS), which handles the last two blocks
    specially, causing them to appear "flipped". Thus, it's actually the
    second-to-last block which depends on the full plaintext.

    This caused long filenames that differ only near the end of their
    plaintexts to, when observed without the key, point to the wrong inode
    and be undeletable. For example, with ext4:

    # echo pass | e4crypt add_key -p 16 edir/
    # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
    # find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
    100000
    # sync
    # echo 3 > /proc/sys/vm/drop_caches
    # keyctl new_session
    # find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
    2004
    # rm -rf edir/
    rm: cannot remove 'edir/_A7nNFi3rhkEQlJ6P,hdzluhODKOeWx5V': Structure needs cleaning
    ...

    To fix this, when presenting long encrypted filenames, encode the
    second-to-last block of ciphertext rather than the last 16 bytes.

    Although it would be nice to solve this without depending on a specific
    encryption mode, that would mean doing a cryptographic hash like SHA-256
    which would be much less efficient. This way is sufficient for now, and
    it's still compatible with encryption modes like HEH which are strong
    pseudorandom permutations. Also, changing the presented names is still
    allowed at any time because they are only provided to allow applications
    to do things like delete encrypted directories. They're not designed to
    be used to persistently identify files --- which would be hard to do
    anyway, given that they're encrypted after all.

    For ease of backports, this patch only makes the minimal fix to both
    ext4 and f2fs. It leaves ubifs as-is, since ubifs doesn't compare the
    ciphertext block yet. Follow-on patches will clean things up properly
    and make the filesystems use a shared helper function.

    Fixes: 5de0b4d0cd15 ("ext4 crypto: simplify and speed up filename encryption")
    Reported-by: Gwendal Grignou
    Cc: stable@vger.kernel.org
    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • To mitigate some types of offline attacks, filesystem encryption is
    designed to enforce that all files in an encrypted directory tree use
    the same encryption policy (i.e. the same encryption context excluding
    the nonce). However, the fscrypt_has_permitted_context() function which
    enforces this relies on comparing struct fscrypt_info's, which are only
    available when we have the encryption keys. This can cause two
    incorrect behaviors:

    1. If we have the parent directory's key but not the child's key, or
    vice versa, then fscrypt_has_permitted_context() returned false,
    causing applications to see EPERM or ENOKEY. This is incorrect if
    the encryption contexts are in fact consistent. Although we'd
    normally have either both keys or neither key in that case since the
    master_key_descriptors would be the same, this is not guaranteed
    because keys can be added or removed from keyrings at any time.

    2. If we have neither the parent's key nor the child's key, then
    fscrypt_has_permitted_context() returned true, causing applications
    to see no error (or else an error for some other reason). This is
    incorrect if the encryption contexts are in fact inconsistent, since
    in that case we should deny access.

    To fix this, retrieve and compare the fscrypt_contexts if we are unable
    to set up both fscrypt_infos.

    While this slightly hurts performance when accessing an encrypted
    directory tree without the key, this isn't a case we really need to be
    optimizing for; access *with* the key is much more important.
    Furthermore, the performance hit is barely noticeable given that we are
    already retrieving the fscrypt_context and doing two keyring searches in
    fscrypt_get_encryption_info(). If we ever actually wanted to optimize
    this case we might start by caching the fscrypt_contexts.

    Cc: stable@vger.kernel.org # 4.0+
    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     

30 Apr, 2017

2 commits

  • This commit exposes the necessary constants and structures for a
    userspace program to pass filesystem encryption keys into the keyring.
    The fscrypt_key structure was already part of the kernel ABI, this
    change just makes it so programs no longer have to redeclare these
    structures (like e4crypt in e2fsprogs currently does).

    Note that we do not expose the other FS_*_KEY_SIZE constants as they are
    not necessary. Only XTS is supported for contents_encryption_mode, so
    currently FS_MAX_KEY_SIZE bytes of key material must always be passed to
    the kernel.

    This commit also removes __packed from fscrypt_key as it does not
    contain any implicit padding and does not refer to an on-disk structure.

    Signed-off-by: Joe Richey
    Signed-off-by: Theodore Ts'o

    Joe Richey
     
  • The functions in fs/crypto/*.c are only called by filesystems configured
    with encryption support. Since the ->get_context(), ->set_context(),
    and ->empty_dir() operations are always provided in that case (and must
    be, otherwise there would be no way to get/set encryption policies, or
    in the case of ->get_context() even access encrypted files at all),
    there is no need to check for these operations being NULL and we can
    remove these unneeded checks.

    Signed-off-by: Eric Biggers
    Reviewed-by: Richard Weinberger
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     

26 Mar, 2017

1 commit


16 Mar, 2017

2 commits

  • The only use of the ->prepare_context() fscrypt operation was to allow
    ext4 to evict inline data from the inode before ->set_context().
    However, there is no reason why this cannot be done as simply the first
    step in ->set_context(), and in fact it makes more sense to do it that
    way because then the policy modes and flags get validated before any
    real work is done. Therefore, merge ext4_prepare_context() into
    ext4_set_context(), and remove ->prepare_context().

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • Filesystem encryption ostensibly supported revoking a keyring key that
    had been used to "unlock" encrypted files, causing those files to become
    "locked" again. This was, however, buggy for several reasons, the most
    severe of which was that when key revocation happened to be detected for
    an inode, its fscrypt_info was immediately freed, even while other
    threads could be using it for encryption or decryption concurrently.
    This could be exploited to crash the kernel or worse.

    This patch fixes the use-after-free by removing the code which detects
    the keyring key having been revoked, invalidated, or expired. Instead,
    an encrypted inode that is "unlocked" now simply remains unlocked until
    it is evicted from memory. Note that this is no worse than the case for
    block device-level encryption, e.g. dm-crypt, and it still remains
    possible for a privileged user to evict unused pages, inodes, and
    dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
    simply unmounting the filesystem. In fact, one of those actions was
    already needed anyway for key revocation to work even somewhat sanely.
    This change is not expected to break any applications.

    In the future I'd like to implement a real API for fscrypt key
    revocation that interacts sanely with ongoing filesystem operations ---
    waiting for existing operations to complete and blocking new operations,
    and invalidating and sanitizing key material and plaintext from the VFS
    caches. But this is a hard problem, and for now this bug must be fixed.

    This bug affected almost all versions of ext4, f2fs, and ubifs
    encryption, and it was potentially reachable in any kernel configured
    with encryption support (CONFIG_EXT4_ENCRYPTION=y,
    CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
    CONFIG_UBIFS_FS_ENCRYPTION=y). Note that older kernels did not use the
    shared fs/crypto/ code, but due to the potential security implications
    of this bug, it may still be worthwhile to backport this fix to them.

    Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
    Cc: stable@vger.kernel.org # v4.2+
    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o
    Acked-by: Michael Halcrow

    Eric Biggers
     

02 Mar, 2017

1 commit

  • 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 debug this:
    rcu_scheduler_active = 2, debug_locks = 0
    1 lock held by mount.nfs/5987:
    #0: (rcu_read_lock){......}, at: [] nfs_idmap_get_key+0x15c/0x420 [nfsv4]
    stack backtrace:
    CPU: 1 PID: 5987 Comm: mount.nfs Tainted: G W 4.10.0 #1
    Call Trace:
    dump_stack+0xe8/0x154 (unreliable)
    lockdep_rcu_suspicious+0x140/0x190
    nfs_idmap_get_key+0x380/0x420 [nfsv4]
    nfs_map_name_to_uid+0x2a0/0x3b0 [nfsv4]
    decode_getfattr_attrs+0xfac/0x16b0 [nfsv4]
    decode_getfattr_generic.constprop.106+0xbc/0x150 [nfsv4]
    nfs4_xdr_dec_lookup_root+0xac/0xb0 [nfsv4]
    rpcauth_unwrap_resp+0xe8/0x140 [sunrpc]
    call_decode+0x29c/0x910 [sunrpc]
    __rpc_execute+0x140/0x8f0 [sunrpc]
    rpc_run_task+0x170/0x200 [sunrpc]
    nfs4_call_sync_sequence+0x68/0xa0 [nfsv4]
    _nfs4_lookup_root.isra.44+0xd0/0xf0 [nfsv4]
    nfs4_lookup_root+0xe0/0x350 [nfsv4]
    nfs4_lookup_root_sec+0x70/0xa0 [nfsv4]
    nfs4_find_root_sec+0xc4/0x100 [nfsv4]
    nfs4_proc_get_rootfh+0x5c/0xf0 [nfsv4]
    nfs4_get_rootfh+0x6c/0x190 [nfsv4]
    nfs4_server_common_setup+0xc4/0x260 [nfsv4]
    nfs4_create_server+0x278/0x3c0 [nfsv4]
    nfs4_remote_mount+0x50/0xb0 [nfsv4]
    mount_fs+0x74/0x210
    vfs_kern_mount+0x78/0x220
    nfs_do_root_mount+0xb0/0x140 [nfsv4]
    nfs4_try_mount+0x60/0x100 [nfsv4]
    nfs_fs_mount+0x5ec/0xda0 [nfs]
    mount_fs+0x74/0x210
    vfs_kern_mount+0x78/0x220
    do_mount+0x254/0xf70
    SyS_mount+0x94/0x100
    system_call+0x38/0xe0

    Reported-by: Jan Stancek
    Signed-off-by: David Howells
    Tested-by: Jan Stancek
    Signed-off-by: James Morris

    David Howells
     

07 Feb, 2017

3 commits

  • When a completion is declared on-stack we have to use
    COMPLETION_INITIALIZER_ONSTACK().

    Fixes: 0b81d07790726 ("fs crypto: move per-file encryption from f2fs
    tree to fs/crypto")
    Signed-off-by: Richard Weinberger
    Signed-off-by: Theodore Ts'o

    Richard Weinberger
     
  • Previously, each filesystem configured without encryption support would
    define all the public fscrypt functions to their notsupp_* stubs. This
    list of #defines had to be updated in every filesystem whenever a change
    was made to the public fscrypt functions. To make things more
    maintainable now that we have three filesystems using fscrypt, split the
    old header fscrypto.h into several new headers. fscrypt_supp.h contains
    the real declarations and is included by filesystems when configured
    with encryption support, whereas fscrypt_notsupp.h contains the inline
    stubs and is included by filesystems when configured without encryption
    support. fscrypt_common.h contains common declarations needed by both.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • res is assigned to sizeof(ctx), however, this is unused and res
    is updated later on without that assigned value to res ever being
    used. Remove this redundant assignment.

    Fixes CoverityScan CID#1395546 "Unused value"

    Signed-off-by: Colin Ian King
    Signed-off-by: Theodore Ts'o

    Colin Ian King
     

08 Jan, 2017

1 commit

  • There was an unnecessary amount of complexity around requesting the
    filesystem-specific key prefix. It was unclear why; perhaps it was
    envisioned that different instances of the same filesystem type could
    use different key prefixes, or that key prefixes could be binary.
    However, neither of those things were implemented or really make sense
    at all. So simplify the code by making key_prefix a const char *.

    Signed-off-by: Eric Biggers
    Reviewed-by: Richard Weinberger
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     

03 Jan, 2017

1 commit

  • Currently, the test_dummy_encryption ext4 mount option, which exists
    only to test encrypted I/O paths with xfstests, overrides all
    per-inode encryption keys with a fixed key.

    This change minimizes test_dummy_encryption-specific code path changes
    by supplying a fake context for directories which are not encrypted
    for use when creating new directories, files, or symlinks. This
    allows us to properly exercise the keyring lookup, derivation, and
    context inheritance code paths.

    Before mounting a file system using test_dummy_encryption, userspace
    must execute the following shell commands:

    mode='\x00\x00\x00\x00'
    raw="$(printf ""\\\\x%02x"" $(seq 0 63))"
    if lscpu | grep "Byte Order" | grep -q Little ; then
    size='\x40\x00\x00\x00'
    else
    size='\x00\x00\x00\x40'
    fi
    key="${mode}${raw}${size}"
    keyctl new_session
    echo -n -e "${key}" | keyctl padd logon fscrypt:4242424242424242 @s

    Signed-off-by: Theodore Ts'o

    Theodore Ts'o
     

02 Jan, 2017

1 commit

  • That way we can get rid of the direct dependency on CONFIG_BLOCK.

    Fixes: d475a507457b ("ubifs: Add skeleton for fscrypto")
    Reported-by: Arnd Bergmann
    Reported-by: Randy Dunlap
    Reviewed-by: Eric Biggers
    Reviewed-by: Christoph Hellwig
    Reviewed-by: David Gstir
    Signed-off-by: Richard Weinberger
    Signed-off-by: Theodore Ts'o

    Richard Weinberger
     

01 Jan, 2017

5 commits

  • It was possible for the ->get_context() operation to fail with a
    specific error code, which was then not returned to the caller of
    FS_IOC_SET_ENCRYPTION_POLICY or FS_IOC_GET_ENCRYPTION_POLICY. Make sure
    to pass through these error codes. Also reorganize the code so that
    ->get_context() only needs to be called one time when setting an
    encryption policy, and handle contexts of unrecognized sizes more
    appropriately.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • Several warning messages were not rate limited and were user-triggerable
    from FS_IOC_SET_ENCRYPTION_POLICY. These shouldn't really have been
    there in the first place, but either way they aren't as useful now that
    the error codes have been improved. So just remove them.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • As part of an effort to clean up fscrypt-related error codes, make
    FS_IOC_SET_ENCRYPTION_POLICY fail with EEXIST when the file already uses
    a different encryption policy. This is more descriptive than EINVAL,
    which was ambiguous with some of the other error cases.

    I am not aware of any users who might be relying on the previous error
    code of EINVAL, which was never documented anywhere.

    This failure case will be exercised by an xfstest.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • As part of an effort to clean up fscrypt-related error codes, make
    FS_IOC_SET_ENCRYPTION_POLICY fail with ENOTDIR when the file descriptor
    does not refer to a directory. This is more descriptive than EINVAL,
    which was ambiguous with some of the other error cases.

    I am not aware of any users who might be relying on the previous error
    code of EINVAL, which was never documented anywhere, and in some buggy
    kernels did not exist at all as the S_ISDIR() check was missing.

    This failure case will be exercised by an xfstest.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     
  • As part of an effort to clean up fscrypt-related error codes, make
    attempting to create a file in an encrypted directory that hasn't been
    "unlocked" fail with ENOKEY. Previously, several error codes were used
    for this case, including ENOENT, EACCES, and EPERM, and they were not
    consistent between and within filesystems. ENOKEY is a better choice
    because it expresses that the failure is due to lacking the encryption
    key. It also matches the error code returned when trying to open an
    encrypted regular file without the key.

    I am not aware of any users who might be relying on the previous
    inconsistent error codes, which were never documented anywhere.

    This failure case will be exercised by an xfstest.

    Signed-off-by: Eric Biggers
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     

31 Dec, 2016

1 commit

  • Attempting to link a device node, named pipe, or socket file into an
    encrypted directory through rename(2) or link(2) always failed with
    EPERM. This happened because fscrypt_has_permitted_context() saw that
    the file was unencrypted and forbid creating the link. This behavior
    was unexpected because such files are never encrypted; only regular
    files, directories, and symlinks can be encrypted.

    To fix this, make fscrypt_has_permitted_context() always return true on
    special files.

    This will be covered by a test in my encryption xfstests patchset.

    Fixes: 9bd8212f981e ("ext4 crypto: add encryption policy and password salt support")
    Signed-off-by: Eric Biggers
    Reviewed-by: Richard Weinberger
    Cc: stable@vger.kernel.org
    Signed-off-by: Theodore Ts'o

    Eric Biggers
     

28 Dec, 2016

1 commit

  • Commit f1c131b45410a: "crypto: xts - Convert to skcipher" now fails
    the setkey operation if the AES key is the same as the tweak key.
    Previously this check was only done if FIPS mode is enabled. Now this
    check is also done if weak key checking was requested. This is
    reasonable, but since we were using the dummy key which was a constant
    series of 0x42 bytes, it now caused dummy encrpyption test mode to
    fail.

    Fix this by using 0x42... and 0x24... for the two keys, so they are
    different.

    Fixes: f1c131b45410a202eb45cc55980a7a9e4e4b4f40
    Cc: stable@vger.kernel.org
    Signed-off-by: Theodore Ts'o

    Theodore Ts'o
     

12 Dec, 2016

7 commits