28 May, 2011

1 commit


27 May, 2011

1 commit

  • Since this cred was not created with copy_creds(), it needs to get
    initialized. Otherwise use of syscall(__NR_keyctl, KEYCTL_SESSION_TO_PARENT);
    can lead to a NULL deref. Thanks to Robert for finding this.

    But introduced by commit 47a150edc2a ("Cache user_ns in struct cred").

    Signed-off-by: Serge E. Hallyn
    Reported-by: Robert Święcki
    Cc: David Howells
    Cc: stable@kernel.org (2.6.39)
    Signed-off-by: Linus Torvalds

    Serge E. Hallyn
     

24 May, 2011

1 commit


20 May, 2011

1 commit


08 May, 2011

1 commit


17 Mar, 2011

2 commits

  • Make request_key() and co. return an error for a negative or rejected key. If
    the key was simply negated, then return ENOKEY, otherwise return the error
    with which it was rejected.

    Without this patch, the following command returns a key number (with the latest
    keyutils):

    [root@andromeda ~]# keyctl request2 user debug:foo rejected @s
    586569904

    Trying to print the key merely gets you a permission denied error:

    [root@andromeda ~]# keyctl print 586569904
    keyctl_read_alloc: Permission denied

    Doing another request_key() call does get you the error, as long as it hasn't
    expired yet:

    [root@andromeda ~]# keyctl request user debug:foo
    request_key: Key was rejected by service

    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells
     
  • Improve /proc/keys by:

    (1) Don't attempt to summarise the payload of a negated key. It won't have
    one. To this end, a helper function - key_is_instantiated() has been
    added that allows the caller to find out whether the key is positively
    instantiated (as opposed to being uninstantiated or negatively
    instantiated).

    (2) Do show keys that are negative, expired or revoked rather than hiding
    them. This requires an override flag (no_state_check) to be passed to
    search_my_process_keyrings() and keyring_search_aux() to suppress this
    check.

    Without this, keys that are possessed by the caller, but only grant
    permissions to the caller if possessed are skipped as the possession check
    fails.

    Keys that are visible due to user, group or other checks are visible with
    or without this patch.

    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells
     

08 Mar, 2011

4 commits

  • Add a keyctl op (KEYCTL_INSTANTIATE_IOV) that is like KEYCTL_INSTANTIATE, but
    takes an iovec array and concatenates the data in-kernel into one buffer.
    Since the KEYCTL_INSTANTIATE copies the data anyway, this isn't too much of a
    problem.

    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells
     
  • Add a new keyctl op to reject a key with a specified error code. This works
    much the same as negating a key, and so keyctl_negate_key() is made a special
    case of keyctl_reject_key(). The difference is that keyctl_negate_key()
    selects ENOKEY as the error to be reported.

    Typically the key would be rejected with EKEYEXPIRED, EKEYREVOKED or
    EKEYREJECTED, but this is not mandatory.

    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells
     
  • Add a key type operation to permit the key type to vet the description of a new
    key that key_alloc() is about to allocate. The operation may reject the
    description if it wishes with an error of its choosing. If it does this, the
    key will not be allocated.

    Signed-off-by: David Howells
    Reviewed-by: Mimi Zohar
    Signed-off-by: James Morris

    David Howells
     
  • Add an RCU payload dereference macro as this seems to be a common piece of code
    amongst key types that use RCU referenced payloads.

    Signed-off-by: David Howells
    Signed-off-by: Mimi Zohar
    Signed-off-by: James Morris

    David Howells
     

26 Jan, 2011

1 commit

  • Fix __key_link_end()'s attempt to fix up the quota if an error occurs.

    There are two erroneous cases: Firstly, we always decrease the quota if
    the preallocated replacement keyring needs cleaning up, irrespective of
    whether or not we should (we may have replaced a pointer rather than
    adding another pointer).

    Secondly, we never clean up the quota if we added a pointer without the
    keyring storage being extended (we allocate multiple pointers at a time,
    even if we're not going to use them all immediately).

    We handle this by setting the bottom bit of the preallocation pointer in
    __key_link_begin() to indicate that the quota needs fixing up, which is
    then passed to __key_link() (which clears the whole thing) and
    __key_link_end().

    Signed-off-by: David Howells
    Signed-off-by: Linus Torvalds

    David Howells
     

24 Jan, 2011

3 commits


22 Jan, 2011

2 commits

  • Fix up comments in the key management code. No functional changes.

    Signed-off-by: David Howells
    Signed-off-by: Linus Torvalds

    David Howells
     
  • Do a bit of a style clean up in the key management code. No functional
    changes.

    Done using:

    perl -p -i -e 's!^/[*]*/\n!!' security/keys/*.c
    perl -p -i -e 's!} /[*] end [a-z0-9_]*[(][)] [*]/\n!}\n!' security/keys/*.c
    sed -i -s -e ": next" -e N -e 's/^\n[}]$/}/' -e t -e P -e 's/^.*\n//' -e "b next" security/keys/*.c

    To remove /*****/ lines, remove comments on the closing brace of a
    function to name the function and remove blank lines before the closing
    brace of a function.

    Signed-off-by: David Howells
    Signed-off-by: Linus Torvalds

    David Howells
     

19 Jan, 2011

3 commits

  • We can avoid scattering va_end() within the

    va_start();
    for (;;) {

    }
    va_end();

    loop, assuming that crypto_shash_init()/crypto_shash_update() return 0 on
    success and negative value otherwise.

    Make TSS_authhmac()/TSS_checkhmac1()/TSS_checkhmac2() similar to TSS_rawhmac()
    by removing "va_end()/goto" from the loop.

    Signed-off-by: Tetsuo Handa
    Reviewed-by: Jesper Juhl
    Acked-by: Mimi Zohar
    Acked-by: David Howells
    Signed-off-by: James Morris

    Tetsuo Handa
     
  • TSS_rawhmac() checks for data != NULL before using it.
    We should do the same thing for TSS_authhmac().

    Signed-off-by: Tetsuo Handa
    Reviewed-by: Jesper Juhl
    Acked-by: Mimi Zohar
    Acked-by: David Howells
    Signed-off-by: James Morris

    Tetsuo Handa
     
  • TSS_rawhmac() forgot to call va_end()/kfree() when data == NULL and
    forgot to call va_end() when crypto_shash_update() < 0.
    Fix these bugs by escaping from the loop using "break"
    (rather than "return"/"goto") in order to make sure that
    va_end()/kfree() are always called.

    Signed-off-by: Tetsuo Handa
    Reviewed-by: Jesper Juhl
    Acked-by: Mimi Zohar
    Acked-by: David Howells
    Signed-off-by: James Morris

    Tetsuo Handa
     

14 Jan, 2011

1 commit

  • Add missing kfree(td) in tpm_seal() before the return, freeing
    td on error paths as well.

    Reported-by: Dan Carpenter
    Signed-off-by: Mimi Zohar
    Acked-by: David Safford
    Acked-by: David Howells
    Signed-off-by: Serge Hallyn
    Signed-off-by: James Morris

    Mimi Zohar
     

10 Jan, 2011

1 commit


24 Dec, 2010

1 commit

  • In construct_alloc_key(), up_write() is called in the error path if
    __key_link_begin() fails, but this is incorrect as __key_link_begin() only
    returns with the nominated keyring locked if it returns successfully.

    Without this patch, you might see the following in dmesg:

    =====================================
    [ BUG: bad unlock balance detected! ]
    -------------------------------------
    mount.cifs/5769 is trying to release lock (&key->sem) at:
    [] request_key_and_link+0x263/0x3fc
    but there are no more locks to release!

    other info that might help us debug this:
    3 locks held by mount.cifs/5769:
    #0: (&type->s_umount_key#41/1){+.+.+.}, at: [] sget+0x278/0x3e7
    #1: (&ret_buf->session_mutex){+.+.+.}, at: [] cifs_get_smb_ses+0x35a/0x443 [cifs]
    #2: (root_key_user.cons_lock){+.+.+.}, at: [] request_key_and_link+0x10a/0x3fc

    stack backtrace:
    Pid: 5769, comm: mount.cifs Not tainted 2.6.37-rc6+ #1
    Call Trace:
    [] ? request_key_and_link+0x263/0x3fc
    [] print_unlock_inbalance_bug+0xca/0xd5
    [] lock_release_non_nested+0xc1/0x263
    [] ? request_key_and_link+0x263/0x3fc
    [] ? request_key_and_link+0x263/0x3fc
    [] lock_release+0x17d/0x1a4
    [] up_write+0x23/0x3b
    [] request_key_and_link+0x263/0x3fc
    [] ? cifs_get_spnego_key+0x61/0x21f [cifs]
    [] request_key+0x41/0x74
    [] cifs_get_spnego_key+0x200/0x21f [cifs]
    [] CIFS_SessSetup+0x55d/0x1273 [cifs]
    [] cifs_setup_session+0x90/0x1ae [cifs]
    [] cifs_get_smb_ses+0x37f/0x443 [cifs]
    [] cifs_mount+0x1aa1/0x23f3 [cifs]
    [] ? alloc_debug_processing+0xdb/0x120
    [] ? cifs_get_spnego_key+0x1ef/0x21f [cifs]
    [] cifs_do_mount+0x165/0x2b3 [cifs]
    [] vfs_kern_mount+0xaf/0x1dc
    [] do_kern_mount+0x4d/0xef
    [] do_mount+0x6f4/0x733
    [] sys_mount+0x88/0xc2
    [] system_call_fastpath+0x16/0x1b

    Reported-by: Jeff Layton
    Signed-off-by: David Howells
    Reviewed-and-Tested-by: Jeff Layton
    Signed-off-by: Linus Torvalds

    David Howells
     

15 Dec, 2010

4 commits

  • Cleanup based on David Howells suggestions:
    - use static const char arrays instead of #define
    - rename init_sdesc to alloc_sdesc
    - convert 'unsigned int' definitions to 'size_t'
    - revert remaining 'const unsigned int' definitions to 'unsigned int'

    Signed-off-by: Mimi Zohar
    Acked-by: David Howells
    Signed-off-by: James Morris

    Mimi Zohar
     
  • Verify the hex ascii datablob length is correct before converting the IV,
    encrypted data, and HMAC to binary.

    Reported-by: David Howells
    Signed-off-by: Mimi Zohar
    Acked-by: David Howells
    Signed-off-by: James Morris

    Mimi Zohar
     
  • Cleanup based on David Howells suggestions:
    - replace kzalloc, where possible, with kmalloc
    - revert 'const unsigned int' definitions to 'unsigned int'

    Signed-off-by: David Safford
    Acked-by: Mimi Zohar
    Acked-by: David Howells
    Signed-off-by: James Morris

    Mimi Zohar
     
  • Previously not all TSS return codes were tested, as they were all eventually
    caught by the TPM. Now all returns are tested and handled immediately.

    This patch also fixes memory leaks in error and non-error paths.

    Signed-off-by: David Safford
    Acked-by: Mimi Zohar
    Acked-by: David Howells
    Acked-by: Serge E. Hallyn
    Signed-off-by: James Morris

    Mimi Zohar
     

30 Nov, 2010

1 commit


29 Nov, 2010

2 commits

  • Define a new kernel key-type called 'encrypted'. Encrypted keys are kernel
    generated random numbers, which are encrypted/decrypted with a 'trusted'
    symmetric key. Encrypted keys are created/encrypted/decrypted in the kernel.
    Userspace only ever sees/stores encrypted blobs.

    Changelog:
    - bug fix: replaced master-key rcu based locking with semaphore
    (reported by David Howells)
    - Removed memset of crypto_shash_digest() digest output
    - Replaced verification of 'key-type:key-desc' using strcspn(), with
    one based on string constants.
    - Moved documentation to Documentation/keys-trusted-encrypted.txt
    - Replace hash with shash (based on comments by David Howells)
    - Make lengths/counts size_t where possible (based on comments by David Howells)
    Could not convert most lengths, as crypto expects 'unsigned int'
    (size_t: on 32 bit is defined as unsigned int, but on 64 bit is unsigned long)
    - Add 'const' where possible (based on comments by David Howells)
    - allocate derived_buf dynamically to support arbitrary length master key
    (fixed by Roberto Sassu)
    - wait until late_initcall for crypto libraries to be registered
    - cleanup security/Kconfig
    - Add missing 'update' keyword (reported/fixed by Roberto Sassu)
    - Free epayload on failure to create key (reported/fixed by Roberto Sassu)
    - Increase the data size limit (requested by Roberto Sassu)
    - Crypto return codes are always 0 on success and negative on failure,
    remove unnecessary tests.
    - Replaced kzalloc() with kmalloc()

    Signed-off-by: Mimi Zohar
    Signed-off-by: David Safford
    Reviewed-by: Roberto Sassu
    Signed-off-by: James Morris

    Mimi Zohar
     
  • Define a new kernel key-type called 'trusted'. Trusted keys are random
    number symmetric keys, generated and RSA-sealed by the TPM. The TPM
    only unseals the keys, if the boot PCRs and other criteria match.
    Userspace can only ever see encrypted blobs.

    Based on suggestions by Jason Gunthorpe, several new options have been
    added to support additional usages.

    The new options are:
    migratable= designates that the key may/may not ever be updated
    (resealed under a new key, new pcrinfo or new auth.)

    pcrlock=n extends the designated PCR 'n' with a random value,
    so that a key sealed to that PCR may not be unsealed
    again until after a reboot.

    keyhandle= specifies the sealing/unsealing key handle.

    keyauth= specifies the sealing/unsealing key auth.

    blobauth= specifies the sealed data auth.

    Implementation of a kernel reserved locality for trusted keys will be
    investigated for a possible future extension.

    Changelog:
    - Updated and added examples to Documentation/keys-trusted-encrypted.txt
    - Moved generic TPM constants to include/linux/tpm_command.h
    (David Howell's suggestion.)
    - trusted_defined.c: replaced kzalloc with kmalloc, added pcrlock failure
    error handling, added const qualifiers where appropriate.
    - moved to late_initcall
    - updated from hash to shash (suggestion by David Howells)
    - reduced worst stack usage (tpm_seal) from 530 to 312 bytes
    - moved documentation to Documentation directory (suggestion by David Howells)
    - all the other code cleanups suggested by David Howells
    - Add pcrlock CAP_SYS_ADMIN dependency (based on comment by Jason Gunthorpe)
    - New options: migratable, pcrlock, keyhandle, keyauth, blobauth (based on
    discussions with Jason Gunthorpe)
    - Free payload on failure to create key(reported/fixed by Roberto Sassu)
    - Updated Kconfig and other descriptions (based on Serge Hallyn's suggestion)
    - Replaced kzalloc() with kmalloc() (reported by Serge Hallyn)

    Signed-off-by: David Safford
    Signed-off-by: Mimi Zohar
    Signed-off-by: James Morris

    Mimi Zohar
     

29 Oct, 2010

1 commit


10 Sep, 2010

2 commits

  • Fix a bug in keyctl_session_to_parent() whereby it tries to check the ownership
    of the parent process's session keyring whether or not the parent has a session
    keyring [CVE-2010-2960].

    This results in the following oops:

    BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
    IP: [] keyctl_session_to_parent+0x251/0x443
    ...
    Call Trace:
    [] ? keyctl_session_to_parent+0x67/0x443
    [] ? __do_fault+0x24b/0x3d0
    [] sys_keyctl+0xb4/0xb8
    [] system_call_fastpath+0x16/0x1b

    if the parent process has no session keyring.

    If the system is using pam_keyinit then it mostly protected against this as all
    processes derived from a login will have inherited the session keyring created
    by pam_keyinit during the log in procedure.

    To test this, pam_keyinit calls need to be commented out in /etc/pam.d/.

    Reported-by: Tavis Ormandy
    Signed-off-by: David Howells
    Acked-by: Tavis Ormandy
    Signed-off-by: Linus Torvalds

    David Howells
     
  • There's an protected access to the parent process's credentials in the middle
    of keyctl_session_to_parent(). This results in the following RCU warning:

    ===================================================
    [ INFO: suspicious rcu_dereference_check() usage. ]
    ---------------------------------------------------
    security/keys/keyctl.c:1291 invoked rcu_dereference_check() without protection!

    other info that might help us debug this:

    rcu_scheduler_active = 1, debug_locks = 0
    1 lock held by keyctl-session-/2137:
    #0: (tasklist_lock){.+.+..}, at: [] keyctl_session_to_parent+0x60/0x236

    stack backtrace:
    Pid: 2137, comm: keyctl-session- Not tainted 2.6.36-rc2-cachefs+ #1
    Call Trace:
    [] lockdep_rcu_dereference+0xaa/0xb3
    [] keyctl_session_to_parent+0xed/0x236
    [] sys_keyctl+0xb4/0xb6
    [] system_call_fastpath+0x16/0x1b

    The code should take the RCU read lock to make sure the parents credentials
    don't go away, even though it's holding a spinlock and has IRQ disabled.

    Signed-off-by: David Howells
    Signed-off-by: Linus Torvalds

    David Howells
     

13 Aug, 2010

1 commit


07 Aug, 2010

1 commit


02 Aug, 2010

4 commits

  • In commit bb952bb98a7e479262c7eb25d5592545a3af147d there was the accidental
    deletion of a statement from call_sbin_request_key() to render the process
    keyring ID to a text string so that it can be passed to /sbin/request-key.

    With gcc 4.6.0 this causes the following warning:

    CC security/keys/request_key.o
    security/keys/request_key.c: In function 'call_sbin_request_key':
    security/keys/request_key.c:102:15: warning: variable 'prkey' set but not used

    This patch reinstates that statement.

    Without this statement, /sbin/request-key will get some random rubbish from the
    stack as that parameter.

    Signed-off-by: Justin P. Mattock
    Signed-off-by: David Howells
    Signed-off-by: James Morris

    Justin P. Mattock
     
  • keyctl_describe_key() turns the key reference it gets into a usable key pointer
    and assigns that to a variable called 'key', which it then ignores in favour of
    recomputing the key pointer each time it needs it. Make it use the precomputed
    pointer instead.

    Without this patch, gcc 4.6 reports that the variable key is set but not used:

    building with gcc 4.6 I'm getting a warning message:
    CC security/keys/keyctl.o
    security/keys/keyctl.c: In function 'keyctl_describe_key':
    security/keys/keyctl.c:472:14: warning: variable 'key' set but not used

    Reported-by: Justin P. Mattock
    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells
     
  • Make /proc/keys check to see if the calling process possesses each key before
    performing the security check. The possession check can be skipped if the key
    doesn't have the possessor-view permission bit set.

    This causes the keys a process possesses to show up in /proc/keys, even if they
    don't have matching user/group/other view permissions.

    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells
     
  • Authorise a process to perform keyctl_set_timeout() on an uninstantiated key if
    that process has the authorisation key for it.

    This allows the instantiator to set the timeout on a key it is instantiating -
    provided it does it before instantiating the key.

    For instance, the test upcall script provided with the keyutils package could
    be modified to set the expiry to an hour hence before instantiating the key:

    [/usr/share/keyutils/request-key-debug.sh]
    if [ "$3" != "neg" ]
    then
    + keyctl timeout $1 3600
    keyctl instantiate $1 "Debug $3" $4 || exit 1
    else

    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells
     

27 Jun, 2010

1 commit

  • This is from a Smatch check I'm writing.

    strncpy_from_user() returns -EFAULT on error so the first change just
    silences a warning but doesn't change how the code works.

    The other change is a bug fix because install_thread_keyring_to_cred()
    can return a variety of errors such as -EINVAL, -EEXIST, -ENOMEM or
    -EKEYREVOKED.

    Signed-off-by: Dan Carpenter
    Signed-off-by: David Howells
    Signed-off-by: Linus Torvalds

    Dan Carpenter