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
     

28 May, 2010

2 commits

  • No functional changes.

    keyctl_session_to_parent() is the only user of signal->count which needs
    the correct value. Change it to use thread_group_empty() instead, this
    must be strictly equivalent under tasklist, and imho looks better.

    Signed-off-by: Oleg Nesterov
    Acked-by: David Howells
    Cc: Peter Zijlstra
    Acked-by: Roland McGrath
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Oleg Nesterov
     
  • call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
    subprocess_info->cred in advance. Now that we have info->init() we can
    change this code to set tgcred->session_keyring in context of execing
    kernel thread.

    Note: since currently call_usermodehelper_keys() is never called with
    UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
    are not really needed, we could rely on install_session_keyring_to_cred()
    which does key_get() on success.

    Signed-off-by: Oleg Nesterov
    Acked-by: Neil Horman
    Acked-by: David Howells
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Oleg Nesterov
     

25 May, 2010

1 commit


18 May, 2010

1 commit


06 May, 2010

3 commits

  • Do preallocation for __key_link() so that the various callers in request_key.c
    can deal with any errors from this source before attempting to construct a key.
    This allows them to assume that the actual linkage step is guaranteed to be
    successful.

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

    David Howells
     
  • Conflicts:
    security/keys/keyring.c

    Resolved conflict with whitespace fix in find_keyring_by_name()

    Signed-off-by: James Morris

    James Morris
     
  • Errors from construct_alloc_key() shouldn't just be ignored in the way they are
    by construct_key_and_link(). The only error that can be ignored so is
    EINPROGRESS as that is used to indicate that we've found a key and don't need
    to construct one.

    We don't, however, handle ENOMEM, EDQUOT or EACCES to indicate allocation
    failures of one sort or another.

    Reported-by: Vegard Nossum
    Signed-off-by: David Howells
    Signed-off-by: James Morris

    David Howells