[v6,6/6] KEYS: Parse keyring payload for restrict options
diff mbox

Message ID 20160721233139.4085-7-mathew.j.martineau@linux.intel.com
State New
Headers show

Commit Message

Mat Martineau July 21, 2016, 11:31 p.m. UTC
The restrict_link hook was recently added to keyrings so that keys may
be validated prior to linking.  This functionality was only available
using internal kernel APIs.

Configuring keyring restrictions from userspace must work around these
constraints:

 - The restrict_link pointer must be set through keyring_alloc().
 - The add_key system call has a fixed set of parameters.

When creating keyrings, the payload was previously required to be
NULL.  To support restrict_link configuration, the payload is now
parsed for keyring options if a payload is provided.  The expected
option payload format is:

  "restrict_type=<type> restrict_by=<method> restrict_key=<key serial>"

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 Documentation/crypto/asymmetric-keys.txt |  45 ++++++++++
 Documentation/security/keys.txt          |  13 ++-
 security/keys/keyring.c                  | 138 ++++++++++++++++++++++++++++++-
 3 files changed, 191 insertions(+), 5 deletions(-)

Comments

David Howells July 22, 2016, 7:34 a.m. UTC | #1
Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:

> +     - Options used when creating the keyring:
> +       - restrict_type=asymmetric
> +       - restrict_by=keyring
> +       - restrict_key=<keyring serial number>

Looking at this, I can't help thinking that restrict_by is superfluous.  You
could say:

	restrict_key=builtin_trusted
	restrict_key=secondary_trusted
	restrict_key=<key-id>

There's also no particular reason is has to be a keyring.  You could nominate
a single key to act as the gatekeeper.

Further, it might look better collapsed thusly:

	restrict=<key-type>[:<parameter>[,<parameter>*]]
	restrict=asymmetric:builtin_trusted
	restrict=asymmetric:secondary_trusted
	restrict=asymmetric:<key-id>

so that it's shorter to write (I can't decide whether a colon or a comma looks
better, but that's neither here nor there).  You can pass the bit after the
colon, if present, to the handler to parse for itself.

Another possibility would be that you pass the entire payload off to the type
handler so that it can parse it for itself.

There are also some other thoughts:

 (1) Do we need to potentially support multiple restrictions:

	restrict=asymmetric:secondary_trusted restrict=blacklist:1234

 (2) Do we need to think about restrictions that are supported by an
     unloadable type?  I would say that we probably don't want to go there for
     the moment.

 (3) How do we handle the restriction key becoming invalidated/revoked?
     Should the GC check the restriction pointers and if invalidated, detach
     the key and remove Write permission from the keyring?  Note that
     involving the GC would also allow us to handle dead keys from a removed
     key type.

Sorry for potentially imposing more changes on you.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mat Martineau July 22, 2016, 9:36 p.m. UTC | #2
David,

On Fri, 22 Jul 2016, David Howells wrote:

> Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
>
>> +     - Options used when creating the keyring:
>> +       - restrict_type=asymmetric
>> +       - restrict_by=keyring
>> +       - restrict_key=<keyring serial number>
>
> Looking at this, I can't help thinking that restrict_by is superfluous.  You
> could say:
>
> 	restrict_key=builtin_trusted
> 	restrict_key=secondary_trusted
> 	restrict_key=<key-id>

My intent was to provide a more generic interface that was applicable to 
other key types (not just asymmetric signature checks), and could have a 
variety of restrict functions. Note that I couldn't use 
restrict_link_by_signature as-is because of the ca_keys options that 
change its behavior.

Another useful restrict function would be to look for the signing key in 
both the referenced restrict_key and the keyring being linked to. This 
would make it easy to build up a trust chain.

I think it's important to include the key type, restriction method, and 
(optional) parameters for the restriction method.

> There's also no particular reason is has to be a keyring.  You could nominate
> a single key to act as the gatekeeper.

I agree.

> Further, it might look better collapsed thusly:
>
> 	restrict=<key-type>[:<parameter>[,<parameter>*]]
> 	restrict=asymmetric:builtin_trusted
> 	restrict=asymmetric:secondary_trusted
> 	restrict=asymmetric:<key-id>
>
> so that it's shorter to write (I can't decide whether a colon or a comma looks
> better, but that's neither here nor there).  You can pass the bit after the
> colon, if present, to the handler to parse for itself.

Sure, I can make the syntax more compact. Handing off the parsing would 
limit the added code in keyring.c, and would allow for type-specific 
options to be used rather than forcing restrict_by/restrict_key on every 
key type.

This would also make it easier to put the restrict_key pointer in struct 
asymmetric_key_subtype instead of struct key.

> Another possibility would be that you pass the entire payload off to the type
> handler so that it can parse it for itself.
>
> There are also some other thoughts:
>
> (1) Do we need to potentially support multiple restrictions:
>
> 	restrict=asymmetric:secondary_trusted restrict=blacklist:1234

This isn't needed for my use cases, my goal was to provide a userspace API 
for the existing single restrict_link hook that's added in 4.7.

Restrictions could be composed in different ways, but how they should do 
so is not self-evident. Are restrictions for multiple types allowed? Are 
they all evaluated in sequence? Is one accept enough to link, or one 
reject enough to not link? Do restrict functions then need to conform to a 
more complex set of rules (return distinct values for 
accept/reject/dontcare)? We don't rule out future extension to multiple 
rules by starting with one, so I think it works to begin with the simple 
single-restriction case.

> (2) Do we need to think about restrictions that are supported by an
>     unloadable type?  I would say that we probably don't want to go there for
>     the moment.

Ok, let's not go there.

> (3) How do we handle the restriction key becoming invalidated/revoked?
>     Should the GC check the restriction pointers and if invalidated, detach
>     the key and remove Write permission from the keyring?  Note that
>     involving the GC would also allow us to handle dead keys from a removed
>     key type.

I'd prefer a 'lazy' approach - we don't want to walk the entire key serial 
tree every time a key is invalidated or revoked. Each restrict function 
should check that a key is usable first, and if it is invalid/revoked/etc 
it can release the reference and clear the restriction key pointer. How it 
behaves afterward depends on what it's restricting: a signature 
verification method would start rejecting everything, but revoking a 
blacklist key would maybe start accepting everything.

--
Mat Martineau
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar July 24, 2016, 2:34 p.m. UTC | #3
On Fr, 2016-07-22 at 14:36 -0700, Mat Martineau wrote:
> David,
> 
> On Fri, 22 Jul 2016, David Howells wrote:
> 
> > Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:

> >
> > (1) Do we need to potentially support multiple restrictions:
> >
> > 	restrict=asymmetric:secondary_trusted restrict=blacklist:1234

Other than the blacklist, are there other examples of multiple
restrictions?   If we're only talking about the blacklist keyring, then
it could always be checked.

> This isn't needed for my use cases, my goal was to provide a userspace API 
> for the existing single restrict_link hook that's added in 4.7.

> Restrictions could be composed in different ways, but how they should do 
> so is not self-evident. Are restrictions for multiple types allowed? Are 
> they all evaluated in sequence? Is one accept enough to link, or one 
> reject enough to not link? Do restrict functions then need to conform to a 
> more complex set of rules (return distinct values for 
> accept/reject/dontcare)? We don't rule out future extension to multiple 
> rules by starting with one, so I think it works to begin with the simple 
> single-restriction case.
> 
> > (2) Do we need to think about restrictions that are supported by an
> >     unloadable type?  I would say that we probably don't want to go there for
> >     the moment.

Other than the keys on the the blacklist keyring are there other
keyrings/keys that are unloadable?

> Ok, let's not go there.
> 
> > (3) How do we handle the restriction key becoming invalidated/revoked?
> >     Should the GC check the restriction pointers and if invalidated, detach
> >     the key and remove Write permission from the keyring?  Note that
> >     involving the GC would also allow us to handle dead keys from a removed
> >     key type.
> 
> I'd prefer a 'lazy' approach - we don't want to walk the entire key serial 
> tree every time a key is invalidated or revoked. Each restrict function 
> should check that a key is usable first, 

Agreed.   For this reason, at least until there is a usecase other than
the blacklist keyring, I don't see a need to support multiple
restrictions.

> and if it is invalid/revoked/etc 
> it can release the reference and clear the restriction key pointer. How it 
> behaves afterward depends on what it's restricting: a signature 
> verification method would start rejecting everything, but revoking a 
> blacklist key would maybe start accepting everything.

Right, the current and any future validation using that key would fail.
Instead of removing the key at that point, it might be easier to force
the key to be "expired".  Having the key on the blacklist will prevent
the key from being re-added.

Suppose the key being blacklisted/revoked is the "restrict-key" used to
validate other keys being added to the keyring.   If the "restrict-key"
is a single key, should all the keys on the keyring be removed?  What
happens if the "restrict-key" is not a single key, but a keyring?  Is
there enough information in the key structure to be able to correlate
which key verified the certificate before the key was added to the
keyring?

Keys cannot be removed from the blacklist keyring, so this should not be
an issue.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Howells July 25, 2016, 2:39 p.m. UTC | #4
Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:

> > so that it's shorter to write (I can't decide whether a colon or a comma
> > looks better, but that's neither here nor there).  You can pass the bit
> > after the colon, if present, to the handler to parse for itself.
> 
> Sure, I can make the syntax more compact. Handing off the parsing would limit
> the added code in keyring.c, and would allow for type-specific options to be
> used rather than forcing restrict_by/restrict_key on every key type.
> 
> This would also make it easier to put the restrict_key pointer in struct
> asymmetric_key_subtype instead of struct key.

I wonder if key->restrict_link should be replaced with a pointer to a:

	struct key_restriction {
		int (*restrict_link)(struct key *keyring,
				     const struct key_type *type,
				     const union key_payload *payload,
				     void *data);
		void free_data(void *data);
		void *data;
	};

so that we don't inflate struct key too much.

Your key->type->lookup_restrict() function could then return a key_restriction
struct.

I'm not sure you want to put this in struct asymmetric_key_subtype - why does
the subtype need to be involved beyond the actual mechanism for doing the
crypto?

> > (1) Do we need to potentially support multiple restrictions:
> ...
> This isn't needed for my use cases, my goal was to provide a userspace API for
> the existing single restrict_link hook that's added in 4.7.

Let's leave this for now, then.  It probably makes sense for the system
blacklist to apply unconditionally, especially as it can refer to PKCS#7
hashes as well as X.509 internal hashes.

> > (3) How do we handle the restriction key becoming invalidated/revoked?
> >     Should the GC check the restriction pointers and if invalidated, detach
> >     the key and remove Write permission from the keyring?  Note that
> >     involving the GC would also allow us to handle dead keys from a removed
> >     key type.
> 
> I'd prefer a 'lazy' approach - we don't want to walk the entire key serial
> tree every time a key is invalidated or revoked.

The keyrings gc does this already (well, on revocation only after a grace
period).  It makes the locking a lot simpler because the only place keys can
be removed from the tree is in the gc.  Besides, if a key is invalidated,
expired or dead, we need to unlink it from all keyrings before we can destroy
it.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Howells July 25, 2016, 2:40 p.m. UTC | #5
Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> > > (2) Do we need to think about restrictions that are supported by an
> > >     unloadable type?  I would say that we probably don't want to go there for
> > >     the moment.
> 
> Other than the keys on the the blacklist keyring are there other
> keyrings/keys that are unloadable?

Blacklist-type keys won't be unloadable.  By unloadable, I mean the key_type
can be unregistered.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mat Martineau July 26, 2016, 4:24 a.m. UTC | #6
On Mon, 25 Jul 2016, David Howells wrote:

> Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
>
>>> so that it's shorter to write (I can't decide whether a colon or a comma
>>> looks better, but that's neither here nor there).  You can pass the bit
>>> after the colon, if present, to the handler to parse for itself.
>>
>> Sure, I can make the syntax more compact. Handing off the parsing would limit
>> the added code in keyring.c, and would allow for type-specific options to be
>> used rather than forcing restrict_by/restrict_key on every key type.
>>
>> This would also make it easier to put the restrict_key pointer in struct
>> asymmetric_key_subtype instead of struct key.
>
> I wonder if key->restrict_link should be replaced with a pointer to a:
>
> 	struct key_restriction {
> 		int (*restrict_link)(struct key *keyring,
> 				     const struct key_type *type,
> 				     const union key_payload *payload,
> 				     void *data);
> 		void free_data(void *data);
> 		void *data;
> 	};
>
> so that we don't inflate struct key too much.
>
> Your key->type->lookup_restrict() function could then return a key_restriction
> struct.

This would work well, it's very flexible and low overhead.

>
> I'm not sure you want to put this in struct asymmetric_key_subtype - why does
> the subtype need to be involved beyond the actual mechanism for doing the
> crypto?

I was thinking it would allow more specialization in the restrict 
functions and prevent bloat in struct key, but struct key_restriction is 
better.

>
>>> (1) Do we need to potentially support multiple restrictions:
>> ...
>> This isn't needed for my use cases, my goal was to provide a userspace API for
>> the existing single restrict_link hook that's added in 4.7.
>
> Let's leave this for now, then.  It probably makes sense for the system
> blacklist to apply unconditionally, especially as it can refer to PKCS#7
> hashes as well as X.509 internal hashes.
>
>>> (3) How do we handle the restriction key becoming invalidated/revoked?
>>>     Should the GC check the restriction pointers and if invalidated, detach
>>>     the key and remove Write permission from the keyring?  Note that
>>>     involving the GC would also allow us to handle dead keys from a removed
>>>     key type.
>>
>> I'd prefer a 'lazy' approach - we don't want to walk the entire key serial
>> tree every time a key is invalidated or revoked.
>
> The keyrings gc does this already (well, on revocation only after a grace
> period).  It makes the locking a lot simpler because the only place keys can
> be removed from the tree is in the gc.  Besides, if a key is invalidated,
> expired or dead, we need to unlink it from all keyrings before we can destroy
> it.

If gc is already visiting all of the keyrings, then it's no significant 
performance hit to also check the restrict_key pointer. I'm not sure yet 
how this meshes with the struct key_restriction proposal above. It might 
need another function pointer for the garbage collector to call so the 
opaque data structure can be checked.

--
Mat Martineau
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mat Martineau July 26, 2016, 10:44 p.m. UTC | #7
On Sun, 24 Jul 2016, Mimi Zohar wrote:

> On Fr, 2016-07-22 at 14:36 -0700, Mat Martineau wrote:
>> David,
>>
>> On Fri, 22 Jul 2016, David Howells wrote:
>>
>>> Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
>
>>>
>>> (1) Do we need to potentially support multiple restrictions:
>>>
>>> 	restrict=asymmetric:secondary_trusted restrict=blacklist:1234
>
> Other than the blacklist, are there other examples of multiple
> restrictions?   If we're only talking about the blacklist keyring, then
> it could always be checked.

Other non-signature restrictions could be desirable, like date ranges or 
encryption strength. Certain restrictions methods could make use of the 
kernel's blacklist keyring, other methods may want a separate blacklist.

>
>> This isn't needed for my use cases, my goal was to provide a userspace API
>> for the existing single restrict_link hook that's added in 4.7.
>
>> Restrictions could be composed in different ways, but how they should do
>> so is not self-evident. Are restrictions for multiple types allowed? Are
>> they all evaluated in sequence? Is one accept enough to link, or one
>> reject enough to not link? Do restrict functions then need to conform to a
>> more complex set of rules (return distinct values for
>> accept/reject/dontcare)? We don't rule out future extension to multiple
>> rules by starting with one, so I think it works to begin with the simple
>> single-restriction case.
>>
>>> (2) Do we need to think about restrictions that are supported by an
>>>     unloadable type?  I would say that we probably don't want to go there for
>>>     the moment.
>
> Other than the keys on the the blacklist keyring are there other
> keyrings/keys that are unloadable?
>
>> Ok, let's not go there.
>>
>>> (3) How do we handle the restriction key becoming invalidated/revoked?
>>>     Should the GC check the restriction pointers and if invalidated, detach
>>>     the key and remove Write permission from the keyring?  Note that
>>>     involving the GC would also allow us to handle dead keys from a removed
>>>     key type.
>>
>> I'd prefer a 'lazy' approach - we don't want to walk the entire key serial
>> tree every time a key is invalidated or revoked. Each restrict function
>> should check that a key is usable first,
>
> Agreed.   For this reason, at least until there is a usecase other than
> the blacklist keyring, I don't see a need to support multiple
> restrictions.
>
>> and if it is invalid/revoked/etc
>> it can release the reference and clear the restriction key pointer. How it
>> behaves afterward depends on what it's restricting: a signature
>> verification method would start rejecting everything, but revoking a
>> blacklist key would maybe start accepting everything.
>
> Right, the current and any future validation using that key would fail.
> Instead of removing the key at that point, it might be easier to force
> the key to be "expired".  Having the key on the blacklist will prevent
> the key from being re-added.
>
> Suppose the key being blacklisted/revoked is the "restrict-key" used to
> validate other keys being added to the keyring.   If the "restrict-key"
> is a single key, should all the keys on the keyring be removed?  What
> happens if the "restrict-key" is not a single key, but a keyring?

In a scenario where the userspace process has control of the keyrings 
involved in each role (signing keys or local blacklist), I think it's 
reasonable to have that program re-evaulate trust when it decides it is 
necessary. It's trickier when making use of shared keyrings that can 
change unexpectedly. Right now we have a system for evaluating keys at the 
moment they are linked to a keyring. I think this is useful in its own 
right, and a fully dynamic trust keyring would be a great extension or 
evolution of the restriction system.

> Is there enough information in the key structure to be able to correlate 
> which key verified the certificate before the key was added to the 
> keyring?

Each x509 key has an authority key identifier that indicates which key 
signed it.

> Keys cannot be removed from the blacklist keyring, so this should not be
> an issue.

Good to know, thanks!

--
Mat Martineau
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/Documentation/crypto/asymmetric-keys.txt b/Documentation/crypto/asymmetric-keys.txt
index 80d45b5..84616d0 100644
--- a/Documentation/crypto/asymmetric-keys.txt
+++ b/Documentation/crypto/asymmetric-keys.txt
@@ -327,3 +327,48 @@  Functions are provided to register and unregister parsers:
 
 Parsers may not have the same name.  The names are otherwise only used for
 displaying in debugging messages.
+
+
+=========================
+KEYRING LINK RESTRICTIONS
+=========================
+
+Keyrings created from userspace using add_key can be configured to check the
+signature of the key being linked.
+
+Several restriction methods are available:
+
+ (1) Restrict using a separate keyring
+
+     - Options used when creating the keyring:
+       - restrict_type=asymmetric
+       - restrict_by=keyring
+       - restrict_key=<keyring serial number>
+
+     Whenever a key link is requested, the keyring identified in restrict_key
+     will be searched for the signing key.
+
+ (2) Restrict using the kernel builtin trusted keyring
+
+     - Options used when creating the keyring:
+       - restrict_type=asymmetric
+       - restrict_by=builtin_trusted
+
+     The kernel builtin trusted keyring will be searched for the signing key.
+
+ (3) Restrict using the kernel builtin and secondary trusted keyrings
+
+     - Options used when creating the keyring:
+       - restrict_type=asymmetric
+       - restrict_by=builtin_and_secondary_trusted
+
+     The kernel builtin and secondary trusted keyrings will be searched for the
+     signing key.
+
+In all of these cases, if the signing key is found the signature of the key to
+be linked will be verified using the signing key.  The requested key is added
+to the keyring only if the signature is successfully verified.  -ENOKEY is
+returned if the parent certificate could not be found, or -EKEYREJECTED is
+returned if the signature check fails or the key is blacklisted.  Other errors
+may be returned if the signature check could not be performed.
+
diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 45f5ff8e..b80c7c6 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -418,8 +418,17 @@  The main syscalls are:
      the type. The payload is plen in size, and plen can be zero for an empty
      payload.
 
-     A new keyring can be generated by setting type "keyring", the keyring name
-     as the description (or NULL) and setting the payload to NULL.
+     A new keyring can be generated by setting type "keyring" and the keyring
+     name as the description (or NULL).  The payload can be NULL or contain
+     keyring link restriction options using the following format:
+
+	"restrict_type=<type> restrict_by=<method> restrict_key=<key serial>"
+
+     "type" is a registered key type.  The type and method will populate
+     restrict_link in the keyring structure using the lookup_restrict function
+     for the requested type.  restrict_key is optional when creating the
+     keyring, but some restrict methods depend on the presence of a
+     restrict_key value when linking keys.
 
      User defined keys can be created by specifying type "user". It is
      recommended that a user defined key's description by prefixed with a type
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index c91e4e0..a0f1148 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -20,9 +20,18 @@ 
 #include <keys/user-type.h>
 #include <linux/assoc_array_priv.h>
 #include <linux/uaccess.h>
+#include <linux/parser.h>
 #include "internal.h"
 
 /*
+ * Layout of preparse payload
+ */
+enum {
+	keyring_restrict_link,
+	keyring_restrict_key,
+};
+
+/*
  * When plumbing the depths of the key tree, this sets a hard limit
  * set on how deep we're willing to go.
  */
@@ -125,29 +134,151 @@  static void keyring_publish_name(struct key *keyring)
 	}
 }
 
+enum {
+	Opt_restrict_type,
+	Opt_restrict_by,
+	Opt_restrict_key,
+	Opt_err,
+};
+
+static const match_table_t keyring_tokens = {
+	{ Opt_restrict_type,	"restrict_type=%s" },
+	{ Opt_restrict_by,	"restrict_by=%s" },
+	{ Opt_restrict_key,	"restrict_key=%d" },
+	{ Opt_err,		NULL },
+};
+
+/*
+ * Parse the keyring options and fill in the required keyring members.
+ *
+ * Returns 0 if the option string is valid (including the empty case),
+ * otherwise -EINVAL.
+ */
+static int keyring_datablob_parse(char *datablob,
+				  struct key_preparsed_payload *prep)
+{
+	substring_t args[MAX_OPT_ARGS];
+	char *c;
+	int token;
+	unsigned long token_mask = 0;
+	struct key_type *restrict_type = NULL;
+	char *restrict_by = NULL;
+	restrict_link_func_t restrict_link;
+	key_serial_t serial = 0;
+	int result;
+	key_ref_t key_ref;
+
+	while ((c = strsep(&datablob, " \t"))) {
+		if (*c == '\0')
+			continue;
+
+		token = match_token(c, keyring_tokens, args);
+		if (test_and_set_bit(token, &token_mask))
+			goto error;
+
+		switch (token) {
+		case Opt_restrict_type:
+			restrict_type = key_type_lookup(args[0].from);
+			if (IS_ERR(restrict_type)) {
+				restrict_type = NULL;
+				goto error;
+			}
+			break;
+		case Opt_restrict_by:
+			restrict_by = args[0].from;
+			break;
+		case Opt_restrict_key:
+			result = kstrtos32(args[0].from, 0, &serial);
+			if (result < 0)
+				goto error;
+			break;
+		case Opt_err:
+			goto error;
+		}
+	}
+
+	if (restrict_type && restrict_by) {
+		if (!restrict_type->lookup_restrict)
+			goto error;
+
+		restrict_link = restrict_type->lookup_restrict(restrict_by);
+		if (IS_ERR(restrict_link))
+			goto error;
+
+		if (test_bit(Opt_restrict_key, &token_mask)) {
+			key_ref = lookup_user_key(serial, 0, 0);
+			if (IS_ERR(key_ref))
+				goto error;
+
+			prep->payload.data[keyring_restrict_key] =
+				key_ref_to_ptr(key_ref);
+		}
+
+		prep->payload.data[keyring_restrict_link] = restrict_link;
+	} else if (test_bit(Opt_restrict_type, &token_mask) ||
+		   test_bit(Opt_restrict_by, &token_mask) ||
+		   test_bit(Opt_restrict_key, &token_mask)) {
+		goto error;
+	}
+
+	if (restrict_type)
+		key_type_put(restrict_type);
+
+	return 0;
+
+error:
+	if (restrict_type)
+		key_type_put(restrict_type);
+
+	return -EINVAL;
+}
+
 /*
  * Preparse a keyring payload
  */
 static int keyring_preparse(struct key_preparsed_payload *prep)
 {
-	return prep->datalen != 0 ? -EINVAL : 0;
+	char *datablob;
+	size_t datalen = prep->datalen;
+	int ret = 0;
+
+	if (datalen) {
+		datablob = kmalloc(datalen + 1, GFP_KERNEL);
+		if (!datablob)
+			return -ENOMEM;
+
+		memcpy(datablob, prep->data, datalen);
+		datablob[datalen] = '\0';
+
+		ret = keyring_datablob_parse(datablob, prep);
+
+		kfree(datablob);
+	}
+
+	return ret;
 }
 
 /*
- * Free a preparse of a user defined key payload
+ * Free a preparse of a keyring payload
  */
 static void keyring_free_preparse(struct key_preparsed_payload *prep)
 {
+	key_put(prep->payload.data[keyring_restrict_key]);
 }
 
 /*
  * Initialise a keyring.
  *
- * Returns 0 on success, -EINVAL if given any data.
+ * Returns 0 on success.
  */
 static int keyring_instantiate(struct key *keyring,
 			       struct key_preparsed_payload *prep)
 {
+	keyring->restrict_link = prep->payload.data[keyring_restrict_link];
+	keyring->restrict_key = prep->payload.data[keyring_restrict_key];
+	/* Don't let keyring_free_preparse do a key_put() on the key pointer */
+	prep->payload.data[keyring_restrict_key] = NULL;
+
 	assoc_array_init(&keyring->keys);
 	/* make the keyring available by name if it has one */
 	keyring_publish_name(keyring);
@@ -394,6 +525,7 @@  static void keyring_destroy(struct key *keyring)
 		write_unlock(&keyring_name_lock);
 	}
 
+	key_put(keyring->restrict_key);
 	assoc_array_destroy(&keyring->keys, &keyring_assoc_array_ops);
 }