| 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 | expand |
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>
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; > } >
Applied to trusty/master-next branch, adding the CVE number in the SOB area as pointed out by Stefan. Thanks.
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; }