[PATCH v2 0/3] KEYS: validate certificate trust with selected owner or builtin key

[PATCH v2 0/3] KEYS: validate certificate trust with selected owner or builtin key

[Dmitry Kasatkin]

Instead of allowing public keys, with certificates signed by any key on
the system trusted keyring, to be added to a trusted keyring, this patch
set further restricts the certificates to those signed by a particular key
or builtin keys on the system keyring.

This patch defines a new kernel parameter 'keys_ownerid={id: | builtin}'
to use specific key or any builtin key.

Changes to v1:
* key id matching code from asymmetric_type.c is reused in the patch

Thanks,
Dmitry


Dmitry Kasatkin (3):
  KEYS: make key id matching as a dedicated function
  KEYS: validate certificate trust only with selected owner key
  KEYS: validate certificate trust only with builtin keys

 Documentation/kernel-parameters.txt      |  5 ++++
 crypto/asymmetric_keys/asymmetric_keys.h |  2 ++
 crypto/asymmetric_keys/asymmetric_type.c | 50 ++++++++++++++++++++------------
 crypto/asymmetric_keys/x509_public_key.c | 26 +++++++++++++++--
 include/linux/key.h                      |  1 +
 kernel/system_keyring.c                  |  1 +
 6 files changed, 64 insertions(+), 21 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] KEYS: make key id matching as a dedicated function

[Dmitry Kasatkin]

Key id matching will also be used in the following patch.
To avoid code duplication this patch moves functionality
to a separate function.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 crypto/asymmetric_keys/asymmetric_keys.h |  2 ++
 crypto/asymmetric_keys/asymmetric_type.c | 50 ++++++++++++++++++++------------
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
index 515b634..a63c551 100644
--- a/crypto/asymmetric_keys/asymmetric_keys.h
+++ b/crypto/asymmetric_keys/asymmetric_keys.h
@@ -9,6 +9,8 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
+int asymmetric_keyid_match(const char *kid, const char *id);
+
 static inline const char *asymmetric_key_id(const struct key *key)
 {
 	return key->type_data.p[1];
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index b77eb53..1fd1d30 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -23,6 +23,34 @@ static LIST_HEAD(asymmetric_key_parsers);
 static DECLARE_RWSEM(asymmetric_key_parsers_sem);
 
 /*
+ * Match asymmetric key id with partial match
+ * @id:		key id to match in a form "id:<id>"
+ */
+int asymmetric_keyid_match(const char *kid, const char *id)
+{
+	size_t idlen, kidlen;
+
+	if (!kid || !id)
+		return 0;
+
+	/* make it possible to use id as in the request: "id:<id>" */
+	if (strncmp(id, "id:", 3) == 0)
+		id += 3;
+
+	/* Anything after here requires a partial match on the ID string */
+	idlen = strlen(id);
+	kidlen = strlen(kid);
+	if (idlen > kidlen)
+		return 0;
+
+	kid += kidlen - idlen;
+	if (strcasecmp(id, kid) != 0)
+		return 0;
+
+	return 1;
+}
+
+/*
  * Match asymmetric keys on (part of) their name
  * We have some shorthand methods for matching keys.  We allow:
  *
@@ -34,9 +62,8 @@ static int asymmetric_key_match(const struct key *key, const void *description)
 {
 	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
 	const char *spec = description;
-	const char *id, *kid;
+	const char *id;
 	ptrdiff_t speclen;
-	size_t idlen, kidlen;
 
 	if (!subtype || !spec || !*spec)
 		return 0;
@@ -55,23 +82,8 @@ static int asymmetric_key_match(const struct key *key, const void *description)
 	speclen = id - spec;
 	id++;
 
-	/* Anything after here requires a partial match on the ID string */
-	kid = asymmetric_key_id(key);
-	if (!kid)
-		return 0;
-
-	idlen = strlen(id);
-	kidlen = strlen(kid);
-	if (idlen > kidlen)
-		return 0;
-
-	kid += kidlen - idlen;
-	if (strcasecmp(id, kid) != 0)
-		return 0;
-
-	if (speclen == 2 &&
-	    memcmp(spec, "id", 2) == 0)
-		return 1;
+	if (speclen == 2 && memcmp(spec, "id", 2) == 0)
+		return asymmetric_keyid_match(asymmetric_key_id(key), id);
 
 	if (speclen == subtype->name_len &&
 	    memcmp(spec, subtype->name, speclen) == 0)
-- 
1.9.1

[PATCH v2 2/3] KEYS: validate certificate trust only with selected owner key

[Dmitry Kasatkin]

Instead of allowing public keys, with certificates signed by any
key on the system trusted keyring, to be added to a trusted keyring,
this patch further restricts the certificates to those signed by a
particular key on the system keyring.

This patch defines a new kernel parameter 'keys_ownerid' to identify
the owner's key which must be used for trust validation of certificates.

Based on Mimi's "KEYS: define an owner trusted keyring" patch.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 Documentation/kernel-parameters.txt      |  5 +++++
 crypto/asymmetric_keys/x509_public_key.c | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7116fda..7a810d3 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1434,6 +1434,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			use the HighMem zone if it exists, and the Normal
 			zone if it does not.
 
+	keys_ownerid=[KEYS] This parameter identifies a specific key(s) on
+			the system trusted keyring to be used for certificate
+			trust validation.
+			format: id:<keyid>
+
 	kgdbdbgp=	[KGDB,HW] kgdb over EHCI usb debug port.
 			Format: <Controller#>[,poll interval]
 			The controller # is the number of the ehci usb debug
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 7a9b386..8482d23 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -24,6 +24,19 @@
 #include "public_key.h"
 #include "x509_parser.h"
 
+static char *owner_keyid;
+static int __init default_owner_keyid_set(char *str)
+{
+	if (!str)		/* default system keyring */
+		return 1;
+
+	if (strncmp(str, "id:", 3) == 0)
+		owner_keyid = str;	/* owner local key 'id:xxxxxx' */
+
+	return 1;
+}
+__setup("keys_ownerid=", default_owner_keyid_set);
+
 /*
  * Find a key in the given keyring by issuer and authority.
  */
@@ -169,6 +182,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
 	if (!trust_keyring)
 		return -EOPNOTSUPP;
 
+	if (owner_keyid &&
+		   !asymmetric_keyid_match(cert->authority, owner_keyid))
+			return -EPERM;
+
 	key = x509_request_asymmetric_key(trust_keyring,
 					  cert->issuer, strlen(cert->issuer),
 					  cert->authority,
-- 
1.9.1

[PATCH v2 3/3] KEYS: validate certificate trust only with builtin keys

[Dmitry Kasatkin]

Instead of allowing public keys, with certificates signed by any
key on the system trusted keyring, to be added to a trusted keyring,
this patch further restricts the certificates to those signed only by
builtin keys on the system keyring.

This patch defines a new option 'builtin' for the kernel parameter
'keys_ownerid' to allow trust validation using builtin keys.

Based on Mimi's "KEYS: define an owner trusted keyring" patch.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 Documentation/kernel-parameters.txt      | 2 +-
 crypto/asymmetric_keys/x509_public_key.c | 9 +++++++--
 include/linux/key.h                      | 1 +
 kernel/system_keyring.c                  | 1 +
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7a810d3..336dabe 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1437,7 +1437,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	keys_ownerid=[KEYS] This parameter identifies a specific key(s) on
 			the system trusted keyring to be used for certificate
 			trust validation.
-			format: id:<keyid>
+			format: { id:<keyid> | builtin }
 
 	kgdbdbgp=	[KGDB,HW] kgdb over EHCI usb debug port.
 			Format: <Controller#>[,poll interval]
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 8482d23..bf14bd8 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -24,6 +24,7 @@
 #include "public_key.h"
 #include "x509_parser.h"
 
+static bool builtin_keys;
 static char *owner_keyid;
 static int __init default_owner_keyid_set(char *str)
 {
@@ -32,6 +33,8 @@ static int __init default_owner_keyid_set(char *str)
 
 	if (strncmp(str, "id:", 3) == 0)
 		owner_keyid = str;	/* owner local key 'id:xxxxxx' */
+	else if (strcmp(str, "builtin") == 0)
+		builtin_keys = true;
 
 	return 1;
 }
@@ -191,8 +194,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
 					  cert->authority,
 					  strlen(cert->authority));
 	if (!IS_ERR(key))  {
-		pk = key->payload.data;
-		ret = x509_check_signature(pk, cert);
+		if (!builtin_keys || test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
+			pk = key->payload.data;
+			ret = x509_check_signature(pk, cert);
+		}
 		key_put(key);
 	}
 	return ret;
diff --git a/include/linux/key.h b/include/linux/key.h
index cd0abb8..67c8e7e 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -170,6 +170,7 @@ struct key {
 #define KEY_FLAG_INVALIDATED	7	/* set if key has been invalidated */
 #define KEY_FLAG_TRUSTED	8	/* set if key is trusted */
 #define KEY_FLAG_TRUSTED_ONLY	9	/* set if keyring only accepts links to trusted keys */
+#define KEY_FLAG_BUILTIN	10	/* set if key is builtin */
 
 	/* the key type and key description string
 	 * - the desc is used to match a key against search criteria
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index 52ebc70..875f64e 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -89,6 +89,7 @@ static __init int load_system_certificate_list(void)
 			pr_err("Problem loading in-kernel X.509 certificate (%ld)\n",
 			       PTR_ERR(key));
 		} else {
+			set_bit(KEY_FLAG_BUILTIN, &key_ref_to_ptr(key)->flags);
 			pr_notice("Loaded X.509 cert '%s'\n",
 				  key_ref_to_ptr(key)->description);
 			key_ref_put(key);
-- 
1.9.1

Re: [PATCH v2 0/3] KEYS: validate certificate trust with selected owner or builtin key

[Mimi Zohar]

On Tue, 2014-06-17 at 11:56 +0300, Dmitry Kasatkin wrote: 
> Instead of allowing public keys, with certificates signed by any key on
> the system trusted keyring, to be added to a trusted keyring, this patch
> set further restricts the certificates to those signed by a particular key
> or builtin keys on the system keyring.
> 
> This patch defines a new kernel parameter 'keys_ownerid={id: | builtin}'
> to use specific key or any builtin key.
> 
> Changes to v1:
> * key id matching code from asymmetric_type.c is reused in the patch

Nice!  The first two we'll upstream, but defer the builtin patch until
the UEFI key patches are upstreamed.

thanks,

Mimi

Re: [PATCH v2 1/3] KEYS: make key id matching as a dedicated function

[Mimi Zohar]

On Tue, 2014-06-17 at 11:56 +0300, Dmitry Kasatkin wrote: 
> Key id matching will also be used in the following patch.
> To avoid code duplication this patch moves functionality
> to a separate function.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

In refactoring asymmetric_key_match(), its changed.  The reason for this
change should be included in the patch description.  (Bug fixes should
be documented.)

thanks,

Mimi

> ---
>  crypto/asymmetric_keys/asymmetric_keys.h |  2 ++
>  crypto/asymmetric_keys/asymmetric_type.c | 50 ++++++++++++++++++++------------
>  2 files changed, 33 insertions(+), 19 deletions(-)
> 
> diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
> index 515b634..a63c551 100644
> --- a/crypto/asymmetric_keys/asymmetric_keys.h
> +++ b/crypto/asymmetric_keys/asymmetric_keys.h
> @@ -9,6 +9,8 @@
>   * 2 of the Licence, or (at your option) any later version.
>   */
> 
> +int asymmetric_keyid_match(const char *kid, const char *id);
> +
>  static inline const char *asymmetric_key_id(const struct key *key)
>  {
>  	return key->type_data.p[1];
> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> index b77eb53..1fd1d30 100644
> --- a/crypto/asymmetric_keys/asymmetric_type.c
> +++ b/crypto/asymmetric_keys/asymmetric_type.c
> @@ -23,6 +23,34 @@ static LIST_HEAD(asymmetric_key_parsers);
>  static DECLARE_RWSEM(asymmetric_key_parsers_sem);
> 
>  /*
> + * Match asymmetric key id with partial match
> + * @id:		key id to match in a form "id:<id>"
> + */
> +int asymmetric_keyid_match(const char *kid, const char *id)
> +{
> +	size_t idlen, kidlen;
> +
> +	if (!kid || !id)
> +		return 0;
> +
> +	/* make it possible to use id as in the request: "id:<id>" */
> +	if (strncmp(id, "id:", 3) == 0)
> +		id += 3;
> +
> +	/* Anything after here requires a partial match on the ID string */
> +	idlen = strlen(id);
> +	kidlen = strlen(kid);
> +	if (idlen > kidlen)
> +		return 0;
> +
> +	kid += kidlen - idlen;
> +	if (strcasecmp(id, kid) != 0)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +/*
>   * Match asymmetric keys on (part of) their name
>   * We have some shorthand methods for matching keys.  We allow:
>   *
> @@ -34,9 +62,8 @@ static int asymmetric_key_match(const struct key *key, const void *description)
>  {
>  	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
>  	const char *spec = description;
> -	const char *id, *kid;
> +	const char *id;
>  	ptrdiff_t speclen;
> -	size_t idlen, kidlen;
> 
>  	if (!subtype || !spec || !*spec)
>  		return 0;
> @@ -55,23 +82,8 @@ static int asymmetric_key_match(const struct key *key, const void *description)
>  	speclen = id - spec;
>  	id++;
> 
> -	/* Anything after here requires a partial match on the ID string */
> -	kid = asymmetric_key_id(key);
> -	if (!kid)
> -		return 0;
> -
> -	idlen = strlen(id);
> -	kidlen = strlen(kid);
> -	if (idlen > kidlen)
> -		return 0;
> -
> -	kid += kidlen - idlen;
> -	if (strcasecmp(id, kid) != 0)
> -		return 0;
> -
> -	if (speclen == 2 &&
> -	    memcmp(spec, "id", 2) == 0)
> -		return 1;
> +	if (speclen == 2 && memcmp(spec, "id", 2) == 0)
> +		return asymmetric_keyid_match(asymmetric_key_id(key), id);
> 
>  	if (speclen == subtype->name_len &&
>  	    memcmp(spec, subtype->name, speclen) == 0)