Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On 30 May 2014 22:12, Mimi Zohar wrote:
> On Fri, 2014-05-30 at 21:24 +0300, Dmitry Kasatkin wrote:
>> On 30 May 2014 20:58, "Mimi Zohar" wrote:
>> >
>> > On Fri, 2014-05-30 at 18:58 +0300, Dmitry Kasatkin wrote:
>> > > On 28 May 2014 18:09, Mimi Zohar wrote:
>> > > > Dot prefixed keyring names are supposed to be reserved for the
>> > > > kernel, but add_key() calls key_get_type_from_user(), which
>> > > > incorrectly verifies the 'type' field, not the 'description' field.
>> > > > This patch verifies the 'description' field isn't dot prefixed,
>> > > > when creating a new keyring, and removes the dot prefix test in
>> > > > key_get_type_from_user().
>> > > >
>> > > > Reported-by: Dmitry Kasatkin
>> > > > Cc: David Howells
>> > > > Signed-off-by: Mimi Zohar
>> > > > ---
>> > > > security/keys/keyctl.c | 6 --
>> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
>> > > >
>> > > > diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
>> > > > index cd5bd0c..9e9a762 100644
>> > > > --- a/security/keys/keyctl.c
>> > > > +++ b/security/keys/keyctl.c
>> > > > @@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
>> > > > return ret;
>> > > > if (ret == 0 || ret >= len)
>> > > > return -EINVAL;
>> > > > - if (type[0] == '.')
>> > > > - return -EPERM;
>> > > > type[len - 1] = '\0';
>> > > > return 0;
>> > > > }
>> > > > @@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *,
>> _type,
>> > > > kfree(description);
>> > > > description = NULL;
>> > > > }
>> > > > + if (description[0] == '.') {
>> > > > + ret = -EPERM;
>> > > > + goto error2;
>> > > > + }
>> > >
>> > > 1. 3 lines above "discription = NULL" will cause kernel oops...
>> > > It happens when using empty description... like:
>> > >
>> > > cat x509_ima.der | keyctl padd asymmetric "" keyid
>> >
>> > Right, that should be 'else if'.
>> >
>> > > 2. It prevents adding trusted keys to ".ima" from user space...
>> > > This is NOT what we want... right?
>> >
>> > It prevents creating a dot prefixed keyring.
>> >
>>
>> May be. But it prevents also adding the key
>> It needs to distinguish key adding and keyring adding then...
>
> Perhaps, but assuming you created a keyring on @u, you would still need
> to look up the keyid and use it. The same is true here. Instead of
> using "keyctl search @u keyring _ima", you would use "keyctl describe %
> keyring:.ima". The first field is the keyring id.
>
> Mimi
>
Not perhaps, but for sure.
In the case of adding the key, "description" is not a keyring name...
it is a key name.
In the case of adding keyring, "description" is a keyring name...
- Dmitry
--
Thanks,
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On Fri, 2014-05-30 at 21:24 +0300, Dmitry Kasatkin wrote:
> On 30 May 2014 20:58, "Mimi Zohar" wrote:
> >
> > On Fri, 2014-05-30 at 18:58 +0300, Dmitry Kasatkin wrote:
> > > On 28 May 2014 18:09, Mimi Zohar wrote:
> > > > Dot prefixed keyring names are supposed to be reserved for the
> > > > kernel, but add_key() calls key_get_type_from_user(), which
> > > > incorrectly verifies the 'type' field, not the 'description' field.
> > > > This patch verifies the 'description' field isn't dot prefixed,
> > > > when creating a new keyring, and removes the dot prefix test in
> > > > key_get_type_from_user().
> > > >
> > > > Reported-by: Dmitry Kasatkin
> > > > Cc: David Howells
> > > > Signed-off-by: Mimi Zohar
> > > > ---
> > > > security/keys/keyctl.c | 6 --
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> > > > index cd5bd0c..9e9a762 100644
> > > > --- a/security/keys/keyctl.c
> > > > +++ b/security/keys/keyctl.c
> > > > @@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
> > > > return ret;
> > > > if (ret == 0 || ret >= len)
> > > > return -EINVAL;
> > > > - if (type[0] == '.')
> > > > - return -EPERM;
> > > > type[len - 1] = '\0';
> > > > return 0;
> > > > }
> > > > @@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *,
> _type,
> > > > kfree(description);
> > > > description = NULL;
> > > > }
> > > > + if (description[0] == '.') {
> > > > + ret = -EPERM;
> > > > + goto error2;
> > > > + }
> > >
> > > 1. 3 lines above "discription = NULL" will cause kernel oops...
> > > It happens when using empty description... like:
> > >
> > > cat x509_ima.der | keyctl padd asymmetric "" keyid
> >
> > Right, that should be 'else if'.
> >
> > > 2. It prevents adding trusted keys to ".ima" from user space...
> > > This is NOT what we want... right?
> >
> > It prevents creating a dot prefixed keyring.
> >
>
> May be. But it prevents also adding the key
> It needs to distinguish key adding and keyring adding then...
Perhaps, but assuming you created a keyring on @u, you would still need
to look up the keyid and use it. The same is true here. Instead of
using "keyctl search @u keyring _ima", you would use "keyctl describe %
keyring:.ima". The first field is the keyring id.
Mimi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On Fri, 2014-05-30 at 18:58 +0300, Dmitry Kasatkin wrote:
> On 28 May 2014 18:09, Mimi Zohar wrote:
> > Dot prefixed keyring names are supposed to be reserved for the
> > kernel, but add_key() calls key_get_type_from_user(), which
> > incorrectly verifies the 'type' field, not the 'description' field.
> > This patch verifies the 'description' field isn't dot prefixed,
> > when creating a new keyring, and removes the dot prefix test in
> > key_get_type_from_user().
> >
> > Reported-by: Dmitry Kasatkin
> > Cc: David Howells
> > Signed-off-by: Mimi Zohar
> > ---
> > security/keys/keyctl.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> > index cd5bd0c..9e9a762 100644
> > --- a/security/keys/keyctl.c
> > +++ b/security/keys/keyctl.c
> > @@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
> > return ret;
> > if (ret == 0 || ret >= len)
> > return -EINVAL;
> > - if (type[0] == '.')
> > - return -EPERM;
> > type[len - 1] = '\0';
> > return 0;
> > }
> > @@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
> > kfree(description);
> > description = NULL;
> > }
> > + if (description[0] == '.') {
> > + ret = -EPERM;
> > + goto error2;
> > + }
>
> 1. 3 lines above "discription = NULL" will cause kernel oops...
> It happens when using empty description... like:
>
> cat x509_ima.der | keyctl padd asymmetric "" keyid
Right, that should be 'else if'.
> 2. It prevents adding trusted keys to ".ima" from user space...
> This is NOT what we want... right?
It prevents creating a dot prefixed keyring.
Mimi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On 28 May 2014 18:09, Mimi Zohar wrote:
> Dot prefixed keyring names are supposed to be reserved for the
> kernel, but add_key() calls key_get_type_from_user(), which
> incorrectly verifies the 'type' field, not the 'description' field.
> This patch verifies the 'description' field isn't dot prefixed,
> when creating a new keyring, and removes the dot prefix test in
> key_get_type_from_user().
>
> Reported-by: Dmitry Kasatkin
> Cc: David Howells
> Signed-off-by: Mimi Zohar
> ---
> security/keys/keyctl.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index cd5bd0c..9e9a762 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
> return ret;
> if (ret == 0 || ret >= len)
> return -EINVAL;
> - if (type[0] == '.')
> - return -EPERM;
> type[len - 1] = '\0';
> return 0;
> }
> @@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
> kfree(description);
> description = NULL;
> }
> + if (description[0] == '.') {
> + ret = -EPERM;
> + goto error2;
> + }
1. 3 lines above "discription = NULL" will cause kernel oops...
It happens when using empty description... like:
cat x509_ima.der | keyctl padd asymmetric "" keyid
2. It prevents adding trusted keys to ".ima" from user space...
This is NOT what we want... right?
- Dmitry
> }
>
> /* pull the payload in if one was supplied */
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On 28 May 2014 18:09, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
Dot prefixed keyring names are supposed to be reserved for the
kernel, but add_key() calls key_get_type_from_user(), which
incorrectly verifies the 'type' field, not the 'description' field.
This patch verifies the 'description' field isn't dot prefixed,
when creating a new keyring, and removes the dot prefix test in
key_get_type_from_user().
Reported-by: Dmitry Kasatkin d.kasat...@samsung.com
Cc: David Howells dhowe...@redhat.com
Signed-off-by: Mimi Zohar zo...@linux.vnet.ibm.com
---
security/keys/keyctl.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index cd5bd0c..9e9a762 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
return ret;
if (ret == 0 || ret = len)
return -EINVAL;
- if (type[0] == '.')
- return -EPERM;
type[len - 1] = '\0';
return 0;
}
@@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
kfree(description);
description = NULL;
}
+ if (description[0] == '.') {
+ ret = -EPERM;
+ goto error2;
+ }
1. 3 lines above discription = NULL will cause kernel oops...
It happens when using empty description... like:
cat x509_ima.der | keyctl padd asymmetric keyid
2. It prevents adding trusted keys to .ima from user space...
This is NOT what we want... right?
- Dmitry
}
/* pull the payload in if one was supplied */
--
1.8.1.4
--
To unsubscribe from this list: send the line unsubscribe
linux-security-module in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On Fri, 2014-05-30 at 18:58 +0300, Dmitry Kasatkin wrote:
On 28 May 2014 18:09, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
Dot prefixed keyring names are supposed to be reserved for the
kernel, but add_key() calls key_get_type_from_user(), which
incorrectly verifies the 'type' field, not the 'description' field.
This patch verifies the 'description' field isn't dot prefixed,
when creating a new keyring, and removes the dot prefix test in
key_get_type_from_user().
Reported-by: Dmitry Kasatkin d.kasat...@samsung.com
Cc: David Howells dhowe...@redhat.com
Signed-off-by: Mimi Zohar zo...@linux.vnet.ibm.com
---
security/keys/keyctl.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index cd5bd0c..9e9a762 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
return ret;
if (ret == 0 || ret = len)
return -EINVAL;
- if (type[0] == '.')
- return -EPERM;
type[len - 1] = '\0';
return 0;
}
@@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
kfree(description);
description = NULL;
}
+ if (description[0] == '.') {
+ ret = -EPERM;
+ goto error2;
+ }
1. 3 lines above discription = NULL will cause kernel oops...
It happens when using empty description... like:
cat x509_ima.der | keyctl padd asymmetric keyid
Right, that should be 'else if'.
2. It prevents adding trusted keys to .ima from user space...
This is NOT what we want... right?
It prevents creating a dot prefixed keyring.
Mimi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On Fri, 2014-05-30 at 21:24 +0300, Dmitry Kasatkin wrote:
On 30 May 2014 20:58, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
On Fri, 2014-05-30 at 18:58 +0300, Dmitry Kasatkin wrote:
On 28 May 2014 18:09, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
Dot prefixed keyring names are supposed to be reserved for the
kernel, but add_key() calls key_get_type_from_user(), which
incorrectly verifies the 'type' field, not the 'description' field.
This patch verifies the 'description' field isn't dot prefixed,
when creating a new keyring, and removes the dot prefix test in
key_get_type_from_user().
Reported-by: Dmitry Kasatkin d.kasat...@samsung.com
Cc: David Howells dhowe...@redhat.com
Signed-off-by: Mimi Zohar zo...@linux.vnet.ibm.com
---
security/keys/keyctl.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index cd5bd0c..9e9a762 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
return ret;
if (ret == 0 || ret = len)
return -EINVAL;
- if (type[0] == '.')
- return -EPERM;
type[len - 1] = '\0';
return 0;
}
@@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *,
_type,
kfree(description);
description = NULL;
}
+ if (description[0] == '.') {
+ ret = -EPERM;
+ goto error2;
+ }
1. 3 lines above discription = NULL will cause kernel oops...
It happens when using empty description... like:
cat x509_ima.der | keyctl padd asymmetric keyid
Right, that should be 'else if'.
2. It prevents adding trusted keys to .ima from user space...
This is NOT what we want... right?
It prevents creating a dot prefixed keyring.
May be. But it prevents also adding the key
It needs to distinguish key adding and keyring adding then...
Perhaps, but assuming you created a keyring on @u, you would still need
to look up the keyid and use it. The same is true here. Instead of
using keyctl search @u keyring _ima, you would use keyctl describe %
keyring:.ima. The first field is the keyring id.
Mimi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v4 1/4] KEYS: special dot prefixed keyring name bug fix
On 30 May 2014 22:12, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
On Fri, 2014-05-30 at 21:24 +0300, Dmitry Kasatkin wrote:
On 30 May 2014 20:58, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
On Fri, 2014-05-30 at 18:58 +0300, Dmitry Kasatkin wrote:
On 28 May 2014 18:09, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
Dot prefixed keyring names are supposed to be reserved for the
kernel, but add_key() calls key_get_type_from_user(), which
incorrectly verifies the 'type' field, not the 'description' field.
This patch verifies the 'description' field isn't dot prefixed,
when creating a new keyring, and removes the dot prefix test in
key_get_type_from_user().
Reported-by: Dmitry Kasatkin d.kasat...@samsung.com
Cc: David Howells dhowe...@redhat.com
Signed-off-by: Mimi Zohar zo...@linux.vnet.ibm.com
---
security/keys/keyctl.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index cd5bd0c..9e9a762 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type,
return ret;
if (ret == 0 || ret = len)
return -EINVAL;
- if (type[0] == '.')
- return -EPERM;
type[len - 1] = '\0';
return 0;
}
@@ -87,6 +85,10 @@ SYSCALL_DEFINE5(add_key, const char __user *,
_type,
kfree(description);
description = NULL;
}
+ if (description[0] == '.') {
+ ret = -EPERM;
+ goto error2;
+ }
1. 3 lines above discription = NULL will cause kernel oops...
It happens when using empty description... like:
cat x509_ima.der | keyctl padd asymmetric keyid
Right, that should be 'else if'.
2. It prevents adding trusted keys to .ima from user space...
This is NOT what we want... right?
It prevents creating a dot prefixed keyring.
May be. But it prevents also adding the key
It needs to distinguish key adding and keyring adding then...
Perhaps, but assuming you created a keyring on @u, you would still need
to look up the keyid and use it. The same is true here. Instead of
using keyctl search @u keyring _ima, you would use keyctl describe %
keyring:.ima. The first field is the keyring id.
Mimi
Not perhaps, but for sure.
In the case of adding the key, description is not a keyring name...
it is a key name.
In the case of adding keyring, description is a keyring name...
- Dmitry
--
Thanks,
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
