linux-stable.git/security, branch v3.16.52

KEYS: add missing permission check for request_key() destination

2018-01-01T20:52:12+00:00

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 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings

security: let security modules use PTRACE_MODE_* with bitmasks

2018-01-01T20:52:10+00:00

commit 3dfb7d8cdbc7ea0c2970450e60818bb3eefbad69 upstream.

It looks like smack and yama weren't aware that the ptrace mode
can have flags ORed into it - PTRACE_MODE_NOAUDIT until now, but
only for /proc/$pid/stat, and with the PTRACE_MODE_*CREDS patch,
all modes have flags ORed into them.

Signed-off-by: Jann Horn 
Acked-by: Kees Cook 
Acked-by: Casey Schaufler 
Cc: Oleg Nesterov 
Cc: Ingo Molnar 
Cc: James Morris 
Cc: "Serge E. Hallyn" 
Cc: Andy Shevchenko 
Cc: Andy Lutomirski 
Cc: Al Viro 
Cc: "Eric W. Biederman" 
Cc: Willy Tarreau 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
Signed-off-by: Ben Hutchings

KEYS: trusted: fix writing past end of buffer in trusted_read()

2018-01-01T20:52:03+00:00

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: Ben Hutchings

KEYS: trusted: sanitize all key material

2018-01-01T20:52:03+00:00

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 
[bwh: Backported to 3.16:
 - Drop one unapplicable change
 - Adjust context]
Signed-off-by: Ben Hutchings

KEYS: return full count in keyring_read() if buffer is too small

2018-01-01T20:52:03+00:00

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: Ben Hutchings

KEYS: encrypted: fix dereference of NULL user_key_payload

2018-01-01T20:51:52+00:00

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 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings

lsm: fix smack_inode_removexattr and xattr_getsecurity memleak

2018-01-01T20:51:48+00:00

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 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings

Smack: remove unneeded NULL-termination from securtity label

2018-01-01T20:51:48+00:00

commit da1b63566c469bf3e2b24182114422e16b1aa34c upstream.

Values of extended attributes are stored as binary blobs. NULL-termination
of them isn't required. It just wastes disk space and confuses command-line
tools like getfattr because they have to print that zero byte at the end.

This patch removes terminating zero byte from initial security label in
smack_inode_init_security and cuts it out in function smack_inode_getsecurity
which is used by syscall getxattr. This change seems completely safe, because
function smk_parse_smack ignores everything after first zero byte.

Signed-off-by: Konstantin Khlebnikov 
Signed-off-by: Ben Hutchings

security/keys: properly zero out sensitive key material in big_key

2018-01-01T20:51:41+00:00

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
[bwh: Backported to 3.16: there's only one kfree() to change]
Signed-off-by: Ben Hutchings

KEYS: prevent creating a different user's keyrings

2018-01-01T20:51:40+00:00

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 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings