25 Feb, 2018

3 commits

  • commit 3cd18d1981731d5f74b8e437009124ac99905d14 upstream.

    The recent rework introduced a possible randconfig build failure
    when CONFIG_CRYPTO configured to only allow modules:

    security/keys/big_key.o: In function `big_key_crypt':
    big_key.c:(.text+0x29f): undefined reference to `crypto_aead_setkey'
    security/keys/big_key.o: In function `big_key_init':
    big_key.c:(.init.text+0x1a): undefined reference to `crypto_alloc_aead'
    big_key.c:(.init.text+0x45): undefined reference to `crypto_aead_setauthsize'
    big_key.c:(.init.text+0x77): undefined reference to `crypto_destroy_tfm'
    crypto/gcm.o: In function `gcm_hash_crypt_remain_continue':
    gcm.c:(.text+0x167): undefined reference to `crypto_ahash_finup'
    crypto/gcm.o: In function `crypto_gcm_exit_tfm':
    gcm.c:(.text+0x847): undefined reference to `crypto_destroy_tfm'

    When we 'select CRYPTO' like the other users, we always get a
    configuration that builds.

    Fixes: 428490e38b2e ("security/keys: rewrite all of big_key crypto")
    Signed-off-by: Arnd Bergmann
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Arnd Bergmann
     
  • commit 4b14752ec4e0d87126e636384cf37c8dd9df157c upstream.

    We can't do anything reasonable in security_bounded_transition() if we
    don't have a policy loaded, and in fact we could run into problems
    with some of the code inside expecting a policy. Fix these problems
    like we do many others in security/selinux/ss/services.c by checking
    to see if the policy is loaded (ss_initialized) and returning quickly
    if it isn't.

    Reported-by: syzbot
    Signed-off-by: Paul Moore
    Acked-by: Stephen Smalley
    Reviewed-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Paul Moore
     
  • commit ef28df55ac27e1e5cd122e19fa311d886d47a756 upstream.

    The syzbot/syzkaller automated tests found a problem in
    security_context_to_sid_core() during early boot (before we load the
    SELinux policy) where we could potentially feed context strings without
    NUL terminators into the strcmp() function.

    We already guard against this during normal operation (after the SELinux
    policy has been loaded) by making a copy of the context strings and
    explicitly adding a NUL terminator to the end. The patch extends this
    protection to the early boot case (no loaded policy) by moving the context
    copy earlier in security_context_to_sid_core().

    Reported-by: syzbot
    Signed-off-by: Paul Moore
    Reviewed-By: William Roberts
    Signed-off-by: Greg Kroah-Hartman

    Paul Moore
     

13 Feb, 2018

1 commit

  • commit 794b4bc292f5d31739d89c0202c54e7dc9bc3add upstream.

    With the 'encrypted' key type it was possible for userspace to provide a
    data blob ending with a master key description shorter than expected,
    e.g. 'keyctl add encrypted desc "new x" @s'. When validating such a
    master key description, validate_master_desc() could read beyond the end
    of the buffer. Fix this by using strncmp() instead of memcmp(). [Also
    clean up the code to deduplicate some logic.]

    Cc: Mimi Zohar
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: James Morris
    Signed-off-by: Jin Qian
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

05 Jan, 2018

6 commits

  • This renames CONFIG_KAISER to CONFIG_PAGE_TABLE_ISOLATION.

    Signed-off-by: Kees Cook
    Signed-off-by: Greg Kroah-Hartman

    Kees Cook
     
  • Now that the required bits have been addressed, reenable
    PARAVIRT.

    Signed-off-by: Borislav Petkov
    Signed-off-by: Greg Kroah-Hartman

    Borislav Petkov
     
  • We fail to see what CONFIG_KAISER_REAL_SWITCH is for: it seems to be
    left over from early development, and now just obscures tricky parts
    of the code. Delete it before adding PCIDs, or nokaiser boot option.

    (Or if there is some good reason to keep the option, then it needs
    a help text - and a "depends on KAISER", so that all those without
    KAISER are not asked the question. But we'd much rather delete it.)

    Signed-off-by: Hugh Dickins
    Signed-off-by: Greg Kroah-Hartman

    Hugh Dickins
     
  • It is absurd that KAISER should depend on SMP, but apparently nobody
    has tried a UP build before: which breaks on implicit declaration of
    function 'per_cpu_offset' in arch/x86/mm/kaiser.c.

    Now, you would expect that to be trivially fixed up; but looking at
    the System.map when that block is #ifdef'ed out of kaiser_init(),
    I see that in a UP build __per_cpu_user_mapped_end is precisely at
    __per_cpu_user_mapped_start, and the items carefully gathered into
    that section for user-mapping on SMP, dispersed elsewhere on UP.

    So, some other kind of section assignment will be needed on UP,
    but implementing that is not a priority: just make KAISER depend
    on SMP for now.

    Also inserted a blank line before the option, tidied up the
    brief Kconfig help message, and added an "If unsure, Y".

    Signed-off-by: Hugh Dickins
    Signed-off-by: Greg Kroah-Hartman

    Hugh Dickins
     
  • Merged fixes and cleanups, rebased to 4.9.51 tree (no 5-level paging).

    Signed-off-by: Dave Hansen
    Signed-off-by: Hugh Dickins
    Signed-off-by: Greg Kroah-Hartman

    Dave Hansen
     
  • This patch introduces our implementation of KAISER (Kernel Address Isolation to
    have Side-channels Efficiently Removed), a kernel isolation technique to close
    hardware side channels on kernel address information.

    More information about the patch can be found on:

    https://github.com/IAIK/KAISER

    From: Richard Fellner
    From: Daniel Gruss
    Subject: [RFC, PATCH] x86_64: KAISER - do not map kernel in user mode
    Date: Thu, 4 May 2017 14:26:50 +0200
    Link: http://marc.info/?l=linux-kernel&m=149390087310405&w=2
    Kaiser-4.10-SHA1: c4b1831d44c6144d3762ccc72f0c4e71a0c713e5

    To:
    To:
    Cc:
    Cc:
    Cc: Michael Schwarz
    Cc: Richard Fellner
    Cc: Ingo Molnar
    Cc:
    Cc:

    After several recent works [1,2,3] KASLR on x86_64 was basically
    considered dead by many researchers. We have been working on an
    efficient but effective fix for this problem and found that not mapping
    the kernel space when running in user mode is the solution to this
    problem [4] (the corresponding paper [5] will be presented at ESSoS17).

    With this RFC patch we allow anybody to configure their kernel with the
    flag CONFIG_KAISER to add our defense mechanism.

    If there are any questions we would love to answer them.
    We also appreciate any comments!

    Cheers,
    Daniel (+ the KAISER team from Graz University of Technology)

    [1] http://www.ieee-security.org/TC/SP2013/papers/4977a191.pdf
    [2] https://www.blackhat.com/docs/us-16/materials/us-16-Fogh-Using-Undocumented-CPU-Behaviour-To-See-Into-Kernel-Mode-And-Break-KASLR-In-The-Process.pdf
    [3] https://www.blackhat.com/docs/us-16/materials/us-16-Jang-Breaking-Kernel-Address-Space-Layout-Randomization-KASLR-With-Intel-TSX.pdf
    [4] https://github.com/IAIK/KAISER
    [5] https://gruss.cc/files/kaiser.pdf

    [patch based also on
    https://raw.githubusercontent.com/IAIK/KAISER/master/KAISER/0001-KAISER-Kernel-Address-Isolation.patch]

    Signed-off-by: Richard Fellner
    Signed-off-by: Moritz Lipp
    Signed-off-by: Daniel Gruss
    Signed-off-by: Michael Schwarz
    Acked-by: Jiri Kosina
    Signed-off-by: Hugh Dickins
    Signed-off-by: Greg Kroah-Hartman

    Richard Fellner
     

14 Dec, 2017

1 commit

  • commit 4dca6ea1d9432052afb06baf2e3ae78188a4410b upstream.

    When the request_key() syscall is not passed a destination keyring, it
    links the requested key (if constructed) into the "default" request-key
    keyring. This should require Write permission to the keyring. However,
    there is actually no permission check.

    This can be abused to add keys to any keyring to which only Search
    permission is granted. This is because Search permission allows joining
    the keyring. keyctl_set_reqkey_keyring(KEY_REQKEY_DEFL_SESSION_KEYRING)
    then will set the default request-key keyring to the session keyring.
    Then, request_key() can be used to add keys to the keyring.

    Both negatively and positively instantiated keys can be added using this
    method. Adding negative keys is trivial. Adding a positive key is a
    bit trickier. It requires that either /sbin/request-key positively
    instantiates the key, or that another thread adds the key to the process
    keyring at just the right time, such that request_key() misses it
    initially but then finds it in construct_alloc_key().

    Fix this bug by checking for Write permission to the keyring in
    construct_get_dest_keyring() when the default keyring is being used.

    We don't do the permission check for non-default keyrings because that
    was already done by the earlier call to lookup_user_key(). Also,
    request_key_and_link() is currently passed a 'struct key *' rather than
    a key_ref_t, so the "possessed" bit is unavailable.

    We also don't do the permission check for the "requestor keyring", to
    continue to support the use case described by commit 8bbf4976b59f
    ("KEYS: Alter use of key instantiation link-to-keyring argument") where
    /sbin/request-key recursively calls request_key() to add keys to the
    original requestor's destination keyring. (I don't know of any users
    who actually do that, though...)

    Fixes: 3e30148c3d52 ("[PATCH] Keys: Make request-key create an authorisation key")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

10 Dec, 2017

1 commit

  • [ Upstream commit ebe7c0a7be92bbd34c6ff5b55810546a0ee05bee ]

    The hash_setup function always sets the hash_setup_done flag, even
    when the hash algorithm is invalid. This prevents the default hash
    algorithm defined as CONFIG_IMA_DEFAULT_HASH from being used.

    This patch sets hash_setup_done flag only for valid hash algorithms.

    Fixes: e7a2ad7eb6f4 "ima: enable support for larger default filedata hash algorithms"
    Signed-off-by: Boshi Wang
    Signed-off-by: Mimi Zohar
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    Boshi Wang
     

24 Nov, 2017

1 commit

  • commit 020aae3ee58c1af0e7ffc4e2cc9fe4dc630338cb upstream.

    Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters")
    moved the call of ima_file_check() from may_open() to do_filp_open() at a
    point where the file descriptor is already opened.

    This breaks the assumption made by IMA that file descriptors being closed
    belong to files whose access was granted by ima_file_check(). The
    consequence is that security.ima and security.evm are updated with good
    values, regardless of the current appraisal status.

    For example, if a file does not have security.ima, IMA will create it after
    opening the file for writing, even if access is denied. Access to the file
    will be allowed afterwards.

    Avoid this issue by checking the appraisal status before updating
    security.ima.

    Signed-off-by: Roberto Sassu
    Signed-off-by: Mimi Zohar
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Roberto Sassu
     

18 Nov, 2017

1 commit

  • commit 47b2c3fff4932e6fc17ce13d51a43c6969714e20 upstream.

    CONFIG_KEYS_COMPAT is defined in arch-specific Kconfigs and is missing for
    several 64-bit architectures : mips, parisc, tile.

    At the moment and for those architectures, calling in 32-bit userspace the
    keyctl syscall would return an ENOSYS error.

    This patch moves the CONFIG_KEYS_COMPAT option to security/keys/Kconfig, to
    make sure the compatibility wrapper is registered by default for any 64-bit
    architecture as long as it is configured with CONFIG_COMPAT.

    [DH: Modified to remove arm64 compat enablement also as requested by Eric
    Biggers]

    Signed-off-by: Bilal Amarni
    Signed-off-by: David Howells
    Reviewed-by: Arnd Bergmann
    cc: Eric Biggers
    Signed-off-by: James Morris
    Cc: James Cowgill
    Signed-off-by: Greg Kroah-Hartman

    Bilal Amarni
     

15 Nov, 2017

3 commits

  • commit a3c812f7cfd80cf51e8f5b7034f7418f6beb56c1 upstream.

    When calling keyctl_read() on a key of type "trusted", if the
    user-supplied buffer was too small, the kernel ignored the buffer length
    and just wrote past the end of the buffer, potentially corrupting
    userspace memory. Fix it by instead returning the size required, as per
    the documentation for keyctl_read().

    We also don't even fill the buffer at all in this case, as this is
    slightly easier to implement than doing a short read, and either
    behavior appears to be permitted. It also makes it match the behavior
    of the "encrypted" key type.

    Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
    Reported-by: Ben Hutchings
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Reviewed-by: Mimi Zohar
    Reviewed-by: James Morris
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit ee618b4619b72527aaed765f0f0b74072b281159 upstream.

    As the previous patch did for encrypted-keys, zero sensitive any
    potentially sensitive data related to the "trusted" key type before it
    is freed. Notably, we were not zeroing the tpm_buf structures in which
    the actual key is stored for TPM seal and unseal, nor were we zeroing
    the trusted_key_payload in certain error paths.

    Cc: Mimi Zohar
    Cc: David Safford
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • [ Upstream commit 3ccb76c5dfe0d25c1d0168d5b726d0b43d19a485 ]

    The kernel build bot turned up a bad config combination when
    CONFIG_SECURITY_APPARMOR is y and CONFIG_SECURITY_APPARMOR_HASH is n,
    resulting in the build error
    security/built-in.o: In function `aa_unpack':
    (.text+0x841e2): undefined reference to `aa_g_hash_policy'

    Signed-off-by: John Johansen
    Signed-off-by: Sasha Levin
    Signed-off-by: Greg Kroah-Hartman

    John Johansen
     

08 Nov, 2017

1 commit

  • commit 3239b6f29bdfb4b0a2ba59df995fc9e6f4df7f1f upstream.

    Commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer
    in keyring_read()") made keyring_read() stop corrupting userspace memory
    when the user-supplied buffer is too small. However it also made the
    return value in that case be the short buffer size rather than the size
    required, yet keyctl_read() is actually documented to return the size
    required. Therefore, switch it over to the documented behavior.

    Note that for now we continue to have it fill the short buffer, since it
    did that before (pre-v3.13) and dump_key_tree_aux() in keyutils arguably
    relies on it.

    Fixes: e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()")
    Reported-by: Ben Hutchings
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Reviewed-by: James Morris
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

27 Oct, 2017

3 commits

  • commit 363b02dab09b3226f3bd1420dad9c72b79a42a76 upstream.

    Consolidate KEY_FLAG_INSTANTIATED, KEY_FLAG_NEGATIVE and the rejection
    error into one field such that:

    (1) The instantiation state can be modified/read atomically.

    (2) The error can be accessed atomically with the state.

    (3) The error isn't stored unioned with the payload pointers.

    This deals with the problem that the state is spread over three different
    objects (two bits and a separate variable) and reading or updating them
    atomically isn't practical, given that not only can uninstantiated keys
    change into instantiated or rejected keys, but rejected keys can also turn
    into instantiated keys - and someone accessing the key might not be using
    any locking.

    The main side effect of this problem is that what was held in the payload
    may change, depending on the state. For instance, you might observe the
    key to be in the rejected state. You then read the cached error, but if
    the key semaphore wasn't locked, the key might've become instantiated
    between the two reads - and you might now have something in hand that isn't
    actually an error code.

    The state is now KEY_IS_UNINSTANTIATED, KEY_IS_POSITIVE or a negative error
    code if the key is negatively instantiated. The key_is_instantiated()
    function is replaced with key_is_positive() to avoid confusion as negative
    keys are also 'instantiated'.

    Additionally, barriering is included:

    (1) Order payload-set before state-set during instantiation.

    (2) Order state-read before payload-read when using the key.

    Further separate barriering is necessary if RCU is being used to access the
    payload content after reading the payload pointers.

    Fixes: 146aa8b1453b ("KEYS: Merge the type-specific data with the payload data")
    Reported-by: Eric Biggers
    Signed-off-by: David Howells
    Reviewed-by: Eric Biggers
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     
  • commit 60ff5b2f547af3828aebafd54daded44cfb0807a upstream.

    Currently, when passed a key that already exists, add_key() will call the
    key's ->update() method if such exists. But this is heavily broken in the
    case where the key is uninstantiated because it doesn't call
    __key_instantiate_and_link(). Consequently, it doesn't do most of the
    things that are supposed to happen when the key is instantiated, such as
    setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and
    awakening tasks waiting on it, and incrementing key->user->nikeys.

    It also never takes key_construction_mutex, which means that
    ->instantiate() can run concurrently with ->update() on the same key. In
    the case of the "user" and "logon" key types this causes a memory leak, at
    best. Maybe even worse, the ->update() methods of the "encrypted" and
    "trusted" key types actually just dereference a NULL pointer when passed an
    uninstantiated key.

    Change key_create_or_update() to wait interruptibly for the key to finish
    construction before continuing.

    This patch only affects *uninstantiated* keys. For now we still allow a
    negatively instantiated key to be updated (thereby positively
    instantiating it), although that's broken too (the next patch fixes it)
    and I'm not sure that anyone actually uses that functionality either.

    Here is a simple reproducer for the bug using the "encrypted" key type
    (requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug
    pertained to more than just the "encrypted" key type:

    #include
    #include
    #include

    int main(void)
    {
    int ringid = keyctl_join_session_keyring(NULL);

    if (fork()) {
    for (;;) {
    const char payload[] = "update user:foo 32";

    usleep(rand() % 10000);
    add_key("encrypted", "desc", payload, sizeof(payload), ringid);
    keyctl_clear(ringid);
    }
    } else {
    for (;;)
    request_key("encrypted", "desc", "callout_info", ringid);
    }
    }

    It causes:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
    IP: encrypted_update+0xb0/0x170
    PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0
    PREEMPT SMP
    CPU: 0 PID: 340 Comm: reproduce Tainted: G D 4.14.0-rc1-00025-g428490e38b2e #796
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
    task: ffff8a467a39a340 task.stack: ffffb15c40770000
    RIP: 0010:encrypted_update+0xb0/0x170
    RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246
    RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000
    RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303
    RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17
    R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000
    R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f
    FS: 00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0
    Call Trace:
    key_create_or_update+0x2bc/0x460
    SyS_add_key+0x10c/0x1d0
    entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x7f5d7f211259
    RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
    RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259
    RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04
    RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004
    R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868
    R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000
    Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b
    RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8
    CR2: 0000000000000018

    Reported-by: Eric Biggers
    Signed-off-by: David Howells
    cc: Eric Biggers
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     
  • commit 13923d0865ca96312197962522e88bc0aedccd74 upstream.

    A key of type "encrypted" references a "master key" which is used to
    encrypt and decrypt the encrypted key's payload. However, when we
    accessed the master key's payload, we failed to handle the case where
    the master key has been revoked, which sets the payload pointer to NULL.
    Note that 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.

    This was an issue for master keys of type "user" only. Master keys can
    also be of type "trusted", but those cannot be revoked.

    Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
    Reviewed-by: James Morris
    Cc: Mimi Zohar
    Cc: David Safford
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

12 Oct, 2017

1 commit

  • commit 57e7ba04d422c3d41c8426380303ec9b7533ded9 upstream.

    security_inode_getsecurity() provides the text string value
    of a security attribute. It does not provide a "secctx".
    The code in xattr_getsecurity() that calls security_inode_getsecurity()
    and then calls security_release_secctx() happened to work because
    SElinux and Smack treat the attribute and the secctx the same way.
    It fails for cap_inode_getsecurity(), because that module has no
    secctx that ever needs releasing. It turns out that Smack is the
    one that's doing things wrong by not allocating memory when instructed
    to do so by the "alloc" parameter.

    The fix is simple enough. Change the security_release_secctx() to
    kfree() because it isn't a secctx being returned by
    security_inode_getsecurity(). Change Smack to allocate the string when
    told to do so.

    Note: this also fixes memory leaks for LSMs which implement
    inode_getsecurity but not release_secctx, such as capabilities.

    Signed-off-by: Casey Schaufler
    Reported-by: Konstantin Khlebnikov
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Casey Schaufler
     

05 Oct, 2017

5 commits

  • commit 37863c43b2c6464f252862bf2e9768264e961678 upstream.

    Because keyctl_read_key() looks up the key with no permissions
    requested, it may find a negatively instantiated key. If the key is
    also possessed, we went ahead and called ->read() on the key. But the
    key payload will actually contain the ->reject_error rather than the
    normal payload. Thus, the kernel oopses trying to read the
    user_key_payload from memory address (int)-ENOKEY = 0x00000000ffffff82.

    Fortunately the payload data is stored inline, so it shouldn't be
    possible to abuse this as an arbitrary memory read primitive...

    Reproducer:
    keyctl new_session
    keyctl request2 user desc '' @s
    keyctl read $(keyctl show | awk '/user: desc/ {print $1}')

    It causes a crash like the following:
    BUG: unable to handle kernel paging request at 00000000ffffff92
    IP: user_read+0x33/0xa0
    PGD 36a54067 P4D 36a54067 PUD 0
    Oops: 0000 [#1] SMP
    CPU: 0 PID: 211 Comm: keyctl Not tainted 4.14.0-rc1 #337
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
    task: ffff90aa3b74c3c0 task.stack: ffff9878c0478000
    RIP: 0010:user_read+0x33/0xa0
    RSP: 0018:ffff9878c047bee8 EFLAGS: 00010246
    RAX: 0000000000000001 RBX: ffff90aa3d7da340 RCX: 0000000000000017
    RDX: 0000000000000000 RSI: 00000000ffffff82 RDI: ffff90aa3d7da340
    RBP: ffff9878c047bf00 R08: 00000024f95da94f R09: 0000000000000000
    R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
    R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
    FS: 00007f58ece69740(0000) GS:ffff90aa3e200000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 00000000ffffff92 CR3: 0000000036adc001 CR4: 00000000003606f0
    Call Trace:
    keyctl_read_key+0xac/0xe0
    SyS_keyctl+0x99/0x120
    entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x7f58ec787bb9
    RSP: 002b:00007ffc8d401678 EFLAGS: 00000206 ORIG_RAX: 00000000000000fa
    RAX: ffffffffffffffda RBX: 00007ffc8d402800 RCX: 00007f58ec787bb9
    RDX: 0000000000000000 RSI: 00000000174a63ac RDI: 000000000000000b
    RBP: 0000000000000004 R08: 00007ffc8d402809 R09: 0000000000000020
    R10: 0000000000000000 R11: 0000000000000206 R12: 00007ffc8d402800
    R13: 00007ffc8d4016e0 R14: 0000000000000000 R15: 0000000000000000
    Code: e5 41 55 49 89 f5 41 54 49 89 d4 53 48 89 fb e8 a4 b4 ad ff 85 c0 74 09 80 3d b9 4c 96 00 00 74 43 48 8b b3 20 01 00 00 4d 85 ed b7 5e 10 74 29 4d 85 e4 74 24 4c 39 e3 4c 89 e2 4c 89 ef 48
    RIP: user_read+0x33/0xa0 RSP: ffff9878c047bee8
    CR2: 00000000ffffff92

    Fixes: 61ea0c0ba904 ("KEYS: Skip key state checks when checking for possession")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 237bbd29f7a049d310d907f4b2716a7feef9abf3 upstream.

    It was possible for an unprivileged user to create the user and user
    session keyrings for another user. For example:

    sudo -u '#3000' sh -c 'keyctl add keyring _uid.4000 "" @u
    keyctl add keyring _uid_ses.4000 "" @u
    sleep 15' &
    sleep 1
    sudo -u '#4000' keyctl describe @u
    sudo -u '#4000' keyctl describe @us

    This is problematic because these "fake" keyrings won't have the right
    permissions. In particular, the user who created them first will own
    them and will have full access to them via the possessor permissions,
    which can be used to compromise the security of a user's keys:

    -4: alswrv-----v------------ 3000 0 keyring: _uid.4000
    -5: alswrv-----v------------ 3000 0 keyring: _uid_ses.4000

    Fix it by marking user and user session keyrings with a flag
    KEY_FLAG_UID_KEYRING. Then, when searching for a user or user session
    keyring by name, skip all keyrings that don't have the flag set.

    Fixes: 69664cf16af4 ("keys: don't generate user and user session keyrings unless they're accessed")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit e645016abc803dafc75e4b8f6e4118f088900ffb upstream.

    Userspace can call keyctl_read() on a keyring to get the list of IDs of
    keys in the keyring. But if the user-supplied buffer is too small, the
    kernel would write the full list anyway --- which will corrupt whatever
    userspace memory happened to be past the end of the buffer. Fix it by
    only filling the space that is available.

    Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 428490e38b2e352812e0b765d8bceafab0ec441d upstream.

    This started out as just replacing the use of crypto/rng with
    get_random_bytes_wait, so that we wouldn't use bad randomness at boot
    time. But, upon looking further, it appears that there were even deeper
    underlying cryptographic problems, and that this seems to have been
    committed with very little crypto review. So, I rewrote the whole thing,
    trying to keep to the conventions introduced by the previous author, to
    fix these cryptographic flaws.

    It makes no sense to seed crypto/rng at boot time and then keep
    using it like this, when in fact there's already get_random_bytes_wait,
    which can ensure there's enough entropy and be a much more standard way
    of generating keys. Since this sensitive material is being stored
    untrusted, using ECB and no authentication is simply not okay at all. I
    find it surprising and a bit horrifying that this code even made it past
    basic crypto review, which perhaps points to some larger issues. This
    patch moves from using AES-ECB to using AES-GCM. Since keys are uniquely
    generated each time, we can set the nonce to zero. There was also a race
    condition in which the same key would be reused at the same time in
    different threads. A mutex fixes this issue now.

    So, to summarize, this commit fixes the following vulnerabilities:

    * Low entropy key generation, allowing an attacker to potentially
    guess or predict keys.
    * Unauthenticated encryption, allowing an attacker to modify the
    cipher text in particular ways in order to manipulate the plaintext,
    which is is even more frightening considering the next point.
    * Use of ECB mode, allowing an attacker to trivially swap blocks or
    compare identical plaintext blocks.
    * Key re-use.
    * Faulty memory zeroing.

    [Note that in backporting this commit to 4.9, get_random_bytes_wait was
    replaced with get_random_bytes, since 4.9 does not have the former
    function. This might result in slightly worse entropy in key generation,
    but common use cases of big_keys makes that likely not a huge deal. And,
    this is the best we can do with this old kernel. Alas.]

    Signed-off-by: Jason A. Donenfeld
    Reviewed-by: Eric Biggers
    Signed-off-by: David Howells
    Cc: Herbert Xu
    Cc: Kirill Marinushkin
    Cc: security@kernel.org
    Signed-off-by: Greg Kroah-Hartman

    Jason A. Donenfeld
     
  • commit 910801809b2e40a4baedd080ef5d80b4a180e70e upstream.

    Error paths forgot to zero out sensitive material, so this patch changes
    some kfrees into a kzfrees.

    Signed-off-by: Jason A. Donenfeld
    Signed-off-by: David Howells
    Reviewed-by: Eric Biggers
    Cc: Herbert Xu
    Cc: Kirill Marinushkin
    Cc: security@kernel.org
    Signed-off-by: Greg Kroah-Hartman

    Jason A. Donenfeld
     

12 Jul, 2017

1 commit

  • commit 57cb17e764ba0aaa169d07796acce54ccfbc6cae upstream.

    This function has two callers and neither are able to handle a NULL
    return. Really, -EINVAL is the correct thing return here anyway. This
    fixes some static checker warnings like:

    security/keys/encrypted-keys/encrypted.c:709 encrypted_key_decrypt()
    error: uninitialized symbol 'master_key'.

    Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
    Signed-off-by: Dan Carpenter
    Acked-by: Mimi Zohar
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Dan Carpenter
     

14 Jun, 2017

3 commits

  • commit e9ff56ac352446f55141aaef1553cee662b2e310 upstream.

    Since v4.9, the crypto API cannot (normally) be used to encrypt/decrypt
    stack buffers because the stack may be virtually mapped. Fix this for
    the padding buffers in encrypted-keys by using ZERO_PAGE for the
    encryption padding and by allocating a temporary heap buffer for the
    decryption padding.

    Tested with CONFIG_DEBUG_SG=y:
    keyctl new_session
    keyctl add user master "abcdefghijklmnop" @s
    keyid=$(keyctl add encrypted desc "new user:master 25" @s)
    datablob="$(keyctl pipe $keyid)"
    keyctl unlink $keyid
    keyid=$(keyctl add encrypted desc "load $datablob" @s)
    datablob2="$(keyctl pipe $keyid)"
    [ "$datablob" = "$datablob2" ] && echo "Success!"

    Cc: Andy Lutomirski
    Cc: Herbert Xu
    Cc: Mimi Zohar
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 63a0b0509e700717a59f049ec6e4e04e903c7fe2 upstream.

    key_update() freed the key_preparsed_payload even if it was not
    initialized first. This would cause a crash if userspace called
    keyctl_update() on a key with type like "asymmetric" that has a
    ->preparse() method but not an ->update() method. Possibly it could
    even be triggered for other key types by racing with keyctl_setperm() to
    make the KEY_NEED_WRITE check fail (the permission was already checked,
    so normally it wouldn't fail there).

    Reproducer with key type "asymmetric", given a valid cert.der:

    keyctl new_session
    keyid=$(keyctl padd asymmetric desc @s < cert.der)
    keyctl setperm $keyid 0x3f000000
    keyctl update $keyid data

    [ 150.686666] BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
    [ 150.687601] IP: asymmetric_key_free_kids+0x12/0x30
    [ 150.688139] PGD 38a3d067
    [ 150.688141] PUD 3b3de067
    [ 150.688447] PMD 0
    [ 150.688745]
    [ 150.689160] Oops: 0000 [#1] SMP
    [ 150.689455] Modules linked in:
    [ 150.689769] CPU: 1 PID: 2478 Comm: keyctl Not tainted 4.11.0-rc4-xfstests-00187-ga9f6b6b8cd2f #742
    [ 150.690916] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
    [ 150.692199] task: ffff88003b30c480 task.stack: ffffc90000350000
    [ 150.692952] RIP: 0010:asymmetric_key_free_kids+0x12/0x30
    [ 150.693556] RSP: 0018:ffffc90000353e58 EFLAGS: 00010202
    [ 150.694142] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000004
    [ 150.694845] RDX: ffffffff81ee3920 RSI: ffff88003d4b0700 RDI: 0000000000000001
    [ 150.697569] RBP: ffffc90000353e60 R08: ffff88003d5d2140 R09: 0000000000000000
    [ 150.702483] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
    [ 150.707393] R13: 0000000000000004 R14: ffff880038a4d2d8 R15: 000000000040411f
    [ 150.709720] FS: 00007fcbcee35700(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000
    [ 150.711504] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [ 150.712733] CR2: 0000000000000001 CR3: 0000000039eab000 CR4: 00000000003406e0
    [ 150.714487] Call Trace:
    [ 150.714975] asymmetric_key_free_preparse+0x2f/0x40
    [ 150.715907] key_update+0xf7/0x140
    [ 150.716560] ? key_default_cmp+0x20/0x20
    [ 150.717319] keyctl_update_key+0xb0/0xe0
    [ 150.718066] SyS_keyctl+0x109/0x130
    [ 150.718663] entry_SYSCALL_64_fastpath+0x1f/0xc2
    [ 150.719440] RIP: 0033:0x7fcbce75ff19
    [ 150.719926] RSP: 002b:00007ffd5d167088 EFLAGS: 00000206 ORIG_RAX: 00000000000000fa
    [ 150.720918] RAX: ffffffffffffffda RBX: 0000000000404d80 RCX: 00007fcbce75ff19
    [ 150.721874] RDX: 00007ffd5d16785e RSI: 000000002866cd36 RDI: 0000000000000002
    [ 150.722827] RBP: 0000000000000006 R08: 000000002866cd36 R09: 00007ffd5d16785e
    [ 150.723781] R10: 0000000000000004 R11: 0000000000000206 R12: 0000000000404d80
    [ 150.724650] R13: 00007ffd5d16784d R14: 00007ffd5d167238 R15: 000000000040411f
    [ 150.725447] Code: 83 c4 08 31 c0 5b 41 5c 41 5d 41 5e 41 5f 5d c3 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 85 ff 74 23 55 48 89 e5 53 48 89 fb 8b 3f e8 06 21 c5 ff 48 8b 7b 08 e8 fd 20 c5 ff 48 89 df e8
    [ 150.727489] RIP: asymmetric_key_free_kids+0x12/0x30 RSP: ffffc90000353e58
    [ 150.728117] CR2: 0000000000000001
    [ 150.728430] ---[ end trace f7f8fe1da2d5ae8d ]---

    Fixes: 4d8c0250b841 ("KEYS: Call ->free_preparse() even after ->preparse() returns an error")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit 5649645d725c73df4302428ee4e02c869248b4c5 upstream.

    sys_add_key() and the KEYCTL_UPDATE operation of sys_keyctl() allowed a
    NULL payload with nonzero length to be passed to the key type's
    ->preparse(), ->instantiate(), and/or ->update() methods. Various key
    types including asymmetric, cifs.idmap, cifs.spnego, and pkcs7_test did
    not handle this case, allowing an unprivileged user to trivially cause a
    NULL pointer dereference (kernel oops) if one of these key types was
    present. Fix it by doing the copy_from_user() when 'plen' is nonzero
    rather than when '_payload' is non-NULL, causing the syscall to fail
    with EFAULT as expected when an invalid buffer is specified.

    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: James Morris
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     

25 May, 2017

1 commit

  • commit 1ac202e978e18f045006d75bd549612620c6ec3a upstream.

    Modifying the attributes of a file makes ima_inode_post_setattr reset
    the IMA cache flags. So if the file, which has just been created,
    is opened a second time before the first file descriptor is closed,
    verification fails since the security.ima xattr has not been written
    yet. We therefore have to look at the IMA_NEW_FILE even if the file
    already existed.

    With this patch there should no longer be an error when cat tries to
    open testfile:

    $ rm -f testfile
    $ ( echo test >&3 ; touch testfile ; cat testfile ) 3>testfile

    A file being new is no reason to accept that it is missing a digital
    signature demanded by the policy.

    Signed-off-by: Daniel Glöckner
    Signed-off-by: Mimi Zohar
    Signed-off-by: Greg Kroah-Hartman

    Daniel Glöckner
     

27 Apr, 2017

3 commits

  • commit c9f838d104fed6f2f61d68164712e3204bf5271b upstream.

    This fixes CVE-2017-7472.

    Running the following program as an unprivileged user exhausts kernel
    memory by leaking thread keyrings:

    #include

    int main()
    {
    for (;;)
    keyctl_set_reqkey_keyring(KEY_REQKEY_DEFL_THREAD_KEYRING);
    }

    Fix it by only creating a new thread keyring if there wasn't one before.
    To make things more consistent, make install_thread_keyring_to_cred()
    and install_process_keyring_to_cred() both return 0 if the corresponding
    keyring is already present.

    Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials")
    Signed-off-by: Eric Biggers
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Eric Biggers
     
  • commit c1644fe041ebaf6519f6809146a77c3ead9193af upstream.

    This fixes CVE-2017-6951.

    Userspace should not be able to do things with the "dead" key type as it
    doesn't have some of the helper functions set upon it that the kernel
    needs. Attempting to use it may cause the kernel to crash.

    Fix this by changing the name of the type to ".dead" so that it's rejected
    up front on userspace syscalls by key_get_type_from_user().

    Though this doesn't seem to affect recent kernels, it does affect older
    ones, certainly those prior to:

    commit c06cfb08b88dfbe13be44a69ae2fdc3a7c902d81
    Author: David Howells
    Date: Tue Sep 16 17:36:06 2014 +0100
    KEYS: Remove key_type::match in favour of overriding default by match_preparse

    which went in before 3.18-rc1.

    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     
  • commit ee8f844e3c5a73b999edf733df1c529d6503ec2f upstream.

    This fixes CVE-2016-9604.

    Keyrings whose name begin with a '.' are special internal keyrings and so
    userspace isn't allowed to create keyrings by this name to prevent
    shadowing. However, the patch that added the guard didn't fix
    KEYCTL_JOIN_SESSION_KEYRING. Not only can that create dot-named keyrings,
    it can also subscribe to them as a session keyring if they grant SEARCH
    permission to the user.

    This, for example, allows a root process to set .builtin_trusted_keys as
    its session keyring, at which point it has full access because now the
    possessor permissions are added. This permits root to add extra public
    keys, thereby bypassing module verification.

    This also affects kexec and IMA.

    This can be tested by (as root):

    keyctl session .builtin_trusted_keys
    keyctl add user a a @s
    keyctl list @s

    which on my test box gives me:

    2 keys in keyring:
    180010936: ---lswrv 0 0 asymmetric: Build time autogenerated kernel key: ae3d4a31b82daa8e1a75b49dc2bba949fd992a05
    801382539: --alswrv 0 0 user: a

    Fix this by rejecting names beginning with a '.' in the keyctl.

    Signed-off-by: David Howells
    Acked-by: Mimi Zohar
    cc: linux-ima-devel@lists.sourceforge.net
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

12 Mar, 2017

1 commit

  • commit bc15ed663e7e53ee4dc3e60f8d09c93a0528c694 upstream.

    On failure to return a pathname from ima_d_path(), a pointer to
    dname is returned, which is subsequently used in the IMA measurement
    list, the IMA audit records, and other audit logging. Saving the
    pointer to dname for later use has the potential to race with rename.

    Intead of returning a pointer to dname on failure, this patch returns
    a pointer to a copy of the filename.

    Reported-by: Al Viro
    Signed-off-by: Mimi Zohar
    Signed-off-by: Greg Kroah-Hartman

    Mimi Zohar
     

15 Feb, 2017

1 commit

  • commit 0c461cb727d146c9ef2d3e86214f498b78b7d125 upstream.

    SELinux tries to support setting/clearing of /proc/pid/attr attributes
    from the shell by ignoring terminating newlines and treating an
    attribute value that begins with a NUL or newline as an attempt to
    clear the attribute. However, the test for clearing attributes has
    always been wrong; it has an off-by-one error, and this could further
    lead to reading past the end of the allocated buffer since commit
    bb646cdb12e75d82258c2f2e7746d5952d3e321a ("proc_pid_attr_write():
    switch to memdup_user()"). Fix the off-by-one error.

    Even with this fix, setting and clearing /proc/pid/attr attributes
    from the shell is not straightforward since the interface does not
    support multiple write() calls (so shells that write the value and
    newline separately will set and then immediately clear the attribute,
    requiring use of echo -n to set the attribute), whereas trying to use
    echo -n "" to clear the attribute causes the shell to skip the
    write() call altogether since POSIX says that a zero-length write
    causes no side effects. Thus, one must use echo -n to set and echo
    without -n to clear, as in the following example:
    $ echo -n unconfined_u:object_r:user_home_t:s0 > /proc/$$/attr/fscreate
    $ cat /proc/$$/attr/fscreate
    unconfined_u:object_r:user_home_t:s0
    $ echo "" > /proc/$$/attr/fscreate
    $ cat /proc/$$/attr/fscreate

    Note the use of /proc/$$ rather than /proc/self, as otherwise
    the cat command will read its own attribute value, not that of the shell.

    There are no users of this facility to my knowledge; possibly we
    should just get rid of it.

    UPDATE: Upon further investigation it appears that a local process
    with the process:setfscreate permission can cause a kernel panic as a
    result of this bug. This patch fixes CVE-2017-2618.

    Signed-off-by: Stephen Smalley
    [PM: added the update about CVE-2017-2618 to the commit description]
    Signed-off-by: Paul Moore
    Signed-off-by: Greg Kroah-Hartman

    Signed-off-by: James Morris

    Stephen Smalley
     

12 Jan, 2017

1 commit

  • commit 9a11a18902bc3b904353063763d06480620245a6 upstream.

    When the "policy" securityfs file is opened for read, it is opened as a
    sequential file. However, when it is eventually released, there is no
    cleanup for the sequential file, therefore some memory is leaked.

    This patch adds a call to seq_release() in ima_release_policy() to clean up
    the memory when the file is opened for read.

    Fixes: 80eae209d63a IMA: allow reading back the current policy
    Reported-by: Colin Ian King
    Signed-off-by: Eric Richter
    Tested-by: Colin Ian King
    Signed-off-by: Mimi Zohar
    Signed-off-by: Greg Kroah-Hartman

    Eric Richter
     

21 Nov, 2016

1 commit

  • After a policy replacement, the task cred may be out of date and need
    to be updated. However change_hat is using the stale profiles from
    the out of date cred resulting in either: a stale profile being applied
    or, incorrect failure when searching for a hat profile as it has been
    migrated to the new parent profile.

    Fixes: 01e2b670aa898a39259bc85c78e3d74820f4d3b6 (failure to find hat)
    Fixes: 898127c34ec03291c86f4ff3856d79e9e18952bc (stale policy being applied)
    Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1000287
    Cc: stable@vger.kernel.org
    Signed-off-by: John Johansen
    Signed-off-by: James Morris

    John Johansen
     

27 Oct, 2016

1 commit

  • Since BIG_KEYS can't be compiled as module it requires one of the "stdrng"
    providers to be compiled into kernel. Otherwise big_key_crypto_init() fails
    on crypto_alloc_rng step and next dereference of big_key_skcipher (e.g. in
    big_key_preparse()) results in a NULL pointer dereference.

    Fixes: 13100a72f40f5748a04017e0ab3df4cf27c809ef ('Security: Keys: Big keys stored encrypted')
    Signed-off-by: Artem Savkov
    Signed-off-by: David Howells
    cc: Stephan Mueller
    cc: Kirill Marinushkin
    cc: stable@vger.kernel.org
    Signed-off-by: James Morris

    Artem Savkov