Skip to contentSkip to conversation

KEYS: validate certificate trust with selected owner or builtin key

PATCH In review Dmitry Kasatkin 2 participants 6 messages Jun 17, 2014 – Jun 18, 2014

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover KEYS: validate certificate trust with selected owner or builtin key

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(-)
Signature · 1 line
1.9.1
#1 lore raw

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

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

---
 crypto/asymmetric_keys/asymmetric_keys.h |  2 ++
 crypto/asymmetric_keys/asymmetric_type.c | 50 ++++++++++++++++++++------------
 2 files changed, 33 insertions(+), 19 deletions(-)
crypto/asymmetric_keys/asymmetric_keys.h +2 -0
index 515b634..a63c551 100644
@@ -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];
crypto/asymmetric_keys/asymmetric_type.c +31 -19

69 lines not on this page · raw message

Signature · 1 line
1.9.1
#2 lore raw

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

Quoted 5 lines · 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.

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

Quoted 94 lines
---
 crypto/asymmetric_keys/asymmetric_keys.h |  2 ++
 crypto/asymmetric_keys/asymmetric_type.c | 50 ++++++++++++++++++++------------
 2 files changed, 33 insertions(+), 19 deletions(-)
crypto/asymmetric_keys/asymmetric_keys.h +2 -0
index 515b634..a63c551 100644
@@ -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];
crypto/asymmetric_keys/asymmetric_type.c +31 -19
index b77eb53..1fd1d30 100644
@@ -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)
#3 lore raw

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

In reply to 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.

---
 Documentation/kernel-parameters.txt      |  5 +++++
 crypto/asymmetric_keys/x509_public_key.c | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)
Documentation/kernel-parameters.txt +5 -0
index 7116fda..7a810d3 100644
@@ -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
crypto/asymmetric_keys/x509_public_key.c +17 -0
index 7a9b386..8482d23 100644
@@ -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,
Signature · 1 line
1.9.1
#4 lore raw

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

In reply to 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.

---
 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(-)
Documentation/kernel-parameters.txt +1 -1
index 7a810d3..336dabe 100644
@@ -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]
crypto/asymmetric_keys/x509_public_key.c +7 -2
index 8482d23..bf14bd8 100644
@@ -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;
include/linux/key.h +1 -0
index cd0abb8..67c8e7e 100644
@@ -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
kernel/system_keyring.c +1 -0
index 52ebc70..875f64e 100644
@@ -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);
Signature · 1 line
1.9.1
#5 lore raw
In reply to Dmitry Kasatkin
Quoted 10 lines · 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

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

thanks,

Mimi