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

apparmor: provide userspace flag indicating binfmt_elf_mmap change

2019-05-02T20:42:07+00:00

commit 34c426acb75cc21bdf84685e106db0c1a3565057 upstream.

Commit 9f834ec18def ("binfmt_elf: switch to new creds when switching to new mm")
changed when the creds are installed by the binfmt_elf handler. This
affects which creds are used to mmap the executable into the address
space. Which can have an affect on apparmor policy.

Add a flag to apparmor at
/sys/kernel/security/apparmor/features/domain/fix_binfmt_elf_mmap

to make it possible to detect this semantic change so that the userspace
tools and the regression test suite can correctly deal with the change.

BugLink: http://bugs.launchpad.net/bugs/1630069
Signed-off-by: John Johansen 
Signed-off-by: Ben Hutchings

KEYS: always initialize keyring_index_key::desc_len

2019-05-02T20:41:58+00:00

commit ede0fa98a900e657d1fcd80b50920efc896c1a4c upstream.

syzbot hit the 'BUG_ON(index_key->desc_len == 0);' in __key_link_begin()
called from construct_alloc_key() during sys_request_key(), because the
length of the key description was never calculated.

The problem is that we rely on ->desc_len being initialized by
search_process_keyrings(), specifically by search_nested_keyrings().
But, if the process isn't subscribed to any keyrings that never happens.

Fix it by always initializing keyring_index_key::desc_len as soon as the
description is set, like we already do in some places.

The following program reproduces the BUG_ON() when it's run as root and
no session keyring has been installed.  If it doesn't work, try removing
pam_keyinit.so from /etc/pam.d/login and rebooting.

    #include 
    #include 
    #include 

    int main(void)
    {
            int id = add_key("keyring", "syz", NULL, 0, KEY_SPEC_USER_KEYRING);

            keyctl_setperm(id, KEY_OTH_WRITE);
            setreuid(5000, 5000);
            request_key("user", "desc", "", id);
    }

Reported-by: syzbot+ec24e95ea483de0a24da@syzkaller.appspotmail.com
Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
Signed-off-by: Eric Biggers 
Signed-off-by: David Howells 
Signed-off-by: James Morris 
Signed-off-by: Ben Hutchings

KEYS: restrict /proc/keys by credentials at open time

2019-05-02T20:41:58+00:00

commit 4aa68e07d845562561f5e73c04aa521376e95252 upstream.

When checking for permission to view keys whilst reading from
/proc/keys, we should use the credentials with which the /proc/keys file
was opened.  This is because, in a classic type of exploit, it can be
possible to bypass checks for the *current* credentials by passing the
file descriptor to a suid program.

Following commit 34dbbcdbf633 ("Make file credentials available to the
seqfile interfaces") we can finally fix it.  So let's do it.

Signed-off-by: Eric Biggers 
Signed-off-by: David Howells 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings

KEYS: allow reaching the keys quotas exactly

2019-05-02T20:41:56+00:00

commit a08bf91ce28ed3ae7b6fef35d843fef8dc8c2cd9 upstream.

If the sysctl 'kernel.keys.maxkeys' is set to some number n, then
actually users can only add up to 'n - 1' keys.  Likewise for
'kernel.keys.maxbytes' and the root_* versions of these sysctls.  But
these sysctls are apparently supposed to be *maximums*, as per their
names and all documentation I could find -- the keyrings(7) man page,
Documentation/security/keys/core.rst, and all the mentions of EDQUOT
meaning that the key quota was *exceeded* (as opposed to reached).

Thus, fix the code to allow reaching the quotas exactly.

Fixes: 0b77f5bfb45c ("keys: make the keyring quotas controllable through /proc/sys")
Signed-off-by: Eric Biggers 
Signed-off-by: David Howells 
Signed-off-by: James Morris 
Signed-off-by: Ben Hutchings

Yama: Check for pid death before checking ancestry

2019-05-02T20:41:23+00:00

commit 9474f4e7cd71a633fa1ef93b7daefd44bbdfd482 upstream.

It's possible that a pid has died before we take the rcu lock, in which
case we can't walk the ancestry list as it may be detached. Instead, check
for death first before doing the walk.

Reported-by: syzbot+a9ac39bf55329e206219@syzkaller.appspotmail.com
Fixes: 2d514487faf1 ("security: Yama LSM")
Suggested-by: Oleg Nesterov 
Signed-off-by: Kees Cook 
Signed-off-by: James Morris 
Signed-off-by: Ben Hutchings

ima: fix showing large 'violations' or 'runtime_measurements_count'

2019-02-11T17:53:30+00:00

commit 1e4c8dafbb6bf72fb5eca035b861e39c5896c2b7 upstream.

The 12 character temporary buffer is not necessarily long enough to hold
a 'long' value.  Increase it.

Signed-off-by: Eric Biggers 
Signed-off-by: Mimi Zohar 
Signed-off-by: Ben Hutchings

KEYS: encrypted: fix buffer overread in valid_master_desc()

2018-12-16T22:09:44+00:00

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

apparmor: remove no-op permission check in policy_unpack

2018-12-16T22:08:51+00:00

commit c037bd615885f1d9d3bdb48531bace79fae1505d upstream.

The patch 736ec752d95e: "AppArmor: policy routines for loading and
unpacking policy" from Jul 29, 2010, leads to the following static
checker warning:

    security/apparmor/policy_unpack.c:410 verify_accept()
    warn: bitwise AND condition is false here

    security/apparmor/policy_unpack.c:413 verify_accept()
    warn: bitwise AND condition is false here

security/apparmor/policy_unpack.c
   392  #define DFA_VALID_PERM_MASK             0xffffffff
   393  #define DFA_VALID_PERM2_MASK            0xffffffff
   394
   395  /**
   396   * verify_accept - verify the accept tables of a dfa
   397   * @dfa: dfa to verify accept tables of (NOT NULL)
   398   * @flags: flags governing dfa
   399   *
   400   * Returns: 1 if valid accept tables else 0 if error
   401   */
   402  static bool verify_accept(struct aa_dfa *dfa, int flags)
   403  {
   404          int i;
   405
   406          /* verify accept permissions */
   407          for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
   408                  int mode = ACCEPT_TABLE(dfa)[i];
   409
   410                  if (mode & ~DFA_VALID_PERM_MASK)
   411                          return 0;
   412
   413                  if (ACCEPT_TABLE2(dfa)[i] & ~DFA_VALID_PERM2_MASK)
   414                          return 0;

fixes: 736ec752d95e ("AppArmor: policy routines for loading and unpacking policy")
Reported-by: Dan Carpenter 
Signed-off-by: John Johansen 
Signed-off-by: Ben Hutchings

selinux: KASAN: slab-out-of-bounds in xattr_getsecurity

2018-10-21T07:46:21+00:00

commit efe3de79e0b52ca281ef6691480c8c68c82a4657 upstream.

Call trace:
 [] dump_backtrace+0x0/0x428
 [] show_stack+0x28/0x38
 [] dump_stack+0xd4/0x124
 [] print_address_description+0x68/0x258
 [] kasan_report.part.2+0x228/0x2f0
 [] kasan_report+0x5c/0x70
 [] check_memory_region+0x12c/0x1c0
 [] memcpy+0x34/0x68
 [] xattr_getsecurity+0xe0/0x160
 [] vfs_getxattr+0xc8/0x120
 [] getxattr+0x100/0x2c8
 [] SyS_fgetxattr+0x64/0xa0
 [] el0_svc_naked+0x24/0x28

If user get root access and calls security.selinux setxattr() with an
embedded NUL on a file and then if some process performs a getxattr()
on that file with a length greater than the actual length of the string,
it would result in a panic.

To fix this, add the actual length of the string to the security context
instead of the length passed by the userspace process.

Signed-off-by: Sachin Grover 
Signed-off-by: Paul Moore 
Signed-off-by: Ben Hutchings

ima: relax requiring a file signature for new files with zero length

2018-06-16T21:21:43+00:00

commit b7e27bc1d42e8e0cc58b602b529c25cd0071b336 upstream.

Custom policies can require file signatures based on LSM labels.  These
files are normally created and only afterwards labeled, requiring them
to be signed.

Instead of requiring file signatures based on LSM labels, entire
filesystems could require file signatures.  In this case, we need the
ability of writing new files without requiring file signatures.

The definition of a "new" file was originally defined as any file with
a length of zero.  Subsequent patches redefined a "new" file to be based
on the FILE_CREATE open flag.  By combining the open flag with a file
size of zero, this patch relaxes the file signature requirement.

Fixes: 1ac202e978e1 ima: accept previously set IMA_NEW_FILE
Signed-off-by: Mimi Zohar 
Signed-off-by: Ben Hutchings