[Trusty,SRU,CVE-2016-9604] KEYS: Disallow keyrings beginning with '.' to be joined as session keyrings

Message ID 20170905093358.28935-1-kleber.souza@canonical.com
State New
Headers show
Series
  • [Trusty,SRU,CVE-2016-9604] KEYS: Disallow keyrings beginning with '.' to be joined as session keyrings
Related show

Commit Message

Kleber Souza Sept. 5, 2017, 9:33 a.m.
From: David Howells <dhowells@redhat.com>

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 <dhowells@redhat.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
cc: linux-ima-devel@lists.sourceforge.net
cc: stable@vger.kernel.org
(cherry picked from commit ee8f844e3c5a73b999edf733df1c529d6503ec2f)
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
---
 security/keys/keyctl.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Colin King Sept. 5, 2017, 9:47 a.m. | #1
On 05/09/17 10:33, Kleber Sacilotto de Souza wrote:
> From: David Howells <dhowells@redhat.com>
> 
> 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 <dhowells@redhat.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> cc: linux-ima-devel@lists.sourceforge.net
> cc: stable@vger.kernel.org
> (cherry picked from commit ee8f844e3c5a73b999edf733df1c529d6503ec2f)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
> ---
>  security/keys/keyctl.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index 9360394b3c10..4e3fecc72f43 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -271,7 +271,8 @@ error:
>   * Create and join an anonymous session keyring or join a named session
>   * keyring, creating it if necessary.  A named session keyring must have Search
>   * permission for it to be joined.  Session keyrings without this permit will
> - * be skipped over.
> + * be skipped over.  It is not permitted for userspace to create or join
> + * keyrings whose name begin with a dot.
>   *
>   * If successful, the ID of the joined session keyring will be returned.
>   */
> @@ -288,12 +289,16 @@ long keyctl_join_session_keyring(const char __user *_name)
>  			ret = PTR_ERR(name);
>  			goto error;
>  		}
> +
> +		ret = -EPERM;
> +		if (name[0] == '.')
> +			goto error_name;
>  	}
>  
>  	/* join the session */
>  	ret = join_session_keyring(name);
> +error_name:
>  	kfree(name);
> -
>  error:
>  	return ret;
>  }
> 
Clean cherry pick. Makes sense.

Acked-by: Colin Ian King <colin.king@canonical.com>
Stefan Bader Sept. 5, 2017, 1:12 p.m. | #2
On 05.09.2017 11:33, Kleber Sacilotto de Souza wrote:
> From: David Howells <dhowells@redhat.com>
> 
> 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 <dhowells@redhat.com>
> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> cc: linux-ima-devel@lists.sourceforge.net
> cc: stable@vger.kernel.org

CVE-2016-9604

> (cherry picked from commit ee8f844e3c5a73b999edf733df1c529d6503ec2f)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---

Again with repeated CVE number.

>  security/keys/keyctl.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index 9360394b3c10..4e3fecc72f43 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -271,7 +271,8 @@ error:
>   * Create and join an anonymous session keyring or join a named session
>   * keyring, creating it if necessary.  A named session keyring must have Search
>   * permission for it to be joined.  Session keyrings without this permit will
> - * be skipped over.
> + * be skipped over.  It is not permitted for userspace to create or join
> + * keyrings whose name begin with a dot.
>   *
>   * If successful, the ID of the joined session keyring will be returned.
>   */
> @@ -288,12 +289,16 @@ long keyctl_join_session_keyring(const char __user *_name)
>  			ret = PTR_ERR(name);
>  			goto error;
>  		}
> +
> +		ret = -EPERM;
> +		if (name[0] == '.')
> +			goto error_name;
>  	}
>  
>  	/* join the session */
>  	ret = join_session_keyring(name);
> +error_name:
>  	kfree(name);
> -
>  error:
>  	return ret;
>  }
>
Kleber Souza Sept. 5, 2017, 2:31 p.m. | #3
Applied to trusty/master-next branch, adding the CVE number in the SOB
area as pointed out by Stefan. Thanks.

Patch

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 9360394b3c10..4e3fecc72f43 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -271,7 +271,8 @@  error:
  * Create and join an anonymous session keyring or join a named session
  * keyring, creating it if necessary.  A named session keyring must have Search
  * permission for it to be joined.  Session keyrings without this permit will
- * be skipped over.
+ * be skipped over.  It is not permitted for userspace to create or join
+ * keyrings whose name begin with a dot.
  *
  * If successful, the ID of the joined session keyring will be returned.
  */
@@ -288,12 +289,16 @@  long keyctl_join_session_keyring(const char __user *_name)
 			ret = PTR_ERR(name);
 			goto error;
 		}
+
+		ret = -EPERM;
+		if (name[0] == '.')
+			goto error_name;
 	}
 
 	/* join the session */
 	ret = join_session_keyring(name);
+error_name:
 	kfree(name);
-
 error:
 	return ret;
 }