linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2]
@ 2014-09-15 20:44 David Howells
  2014-09-15 20:45 ` [PATCH 01/10] Provide a binary to hex conversion function " David Howells
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:44 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel


Here are some patches to improve the matching of asymmetric keys and to
improve the handling of PKCS#7 certificates:

 (1) Provide a method to preparse the data supplied for matching a key.  This
     permits they key type to extract out the bits it needs for matching once
     only.

     Further, the type of search (direct lookup or iterative) can be set and
     the function used to actually check the match can be set by preparse
     rather than being hard coded for the type.

 (2) Improves asymmetric keys identification.

     Keys derived from X.509 certs now get labelled with IDs derived from their
     issuer and certificate number (required to match PKCS#7) and from their
     SKID and subject (required to match X.509).

     IDs are now binary and match criterion preparsing is provided so that
     criteria can be turned into binary blobs to make matching faster.

 (3) Improves PKCS#7 message handling to permit PKCS#7 messages without X.509
     cert lists to be matched to trusted keys, thereby allowing minimally sized
     PKCS#7 certs to be used.

 (4) Improves PKCS#7 message handling to better handle certificate chains that
     are broken due to unsupported crypto that can otherwise by used to
     intersect a trust keyring.

They can be found here also:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-pkcs7

Changes:

 (*) Documentation for the keys API changes.

 (*) Doc comments and exports put on new key ID functions that are used across
     modules.

 (*) The four debugging 0xff bytes are removed from the middle of key IDs.

 (*) Attempt to suppress the warning about not checking the redundant return
     of hex2bin().

 (*) pkcs->unsupported_crypto can be removed in favour of working the error
     code out in pkcs7_validate_trust() from the error codes of
     pkcs7_validate_trust_one().

 (*) bin2hex() now uses hex_byte_pack().

 (*) KEYRING_SEARCH_LOOKUP_TYPE is unused and got removed.

 (*) ctx.match_data.cmp will always be set in keyring_search() so the check
     for NULL there can be removed.

David
---
David Howells (10):
      Provide a binary to hex conversion function
      KEYS: Preparse match data
      KEYS: Remove key_type::def_lookup_type
      KEYS: Remove key_type::match in favour of overriding default by match_preparse
      KEYS: Make the key matching functions return bool
      KEYS: Update the keyrings documentation for match changes
      KEYS: Implement binary asymmetric key ID handling
      KEYS: Overhaul key identification when searching for asymmetric keys
      PKCS#7: Better handling of unsupported crypto
      PKCS#7: Handle PKCS#7 messages that contain no X.509 certs


 Documentation/security/keys.txt           |   65 +++++++-
 crypto/asymmetric_keys/asymmetric_keys.h  |    8 +
 crypto/asymmetric_keys/asymmetric_type.c  |  222 +++++++++++++++++++++--------
 crypto/asymmetric_keys/pkcs7_key_type.c   |    2 
 crypto/asymmetric_keys/pkcs7_parser.c     |   38 ++++-
 crypto/asymmetric_keys/pkcs7_parser.h     |    6 -
 crypto/asymmetric_keys/pkcs7_trust.c      |   82 +++++++----
 crypto/asymmetric_keys/pkcs7_verify.c     |  102 +++++++++----
 crypto/asymmetric_keys/x509_cert_parser.c |   55 ++++---
 crypto/asymmetric_keys/x509_parser.h      |    6 +
 crypto/asymmetric_keys/x509_public_key.c  |  102 ++++++++-----
 fs/cifs/cifs_spnego.c                     |    1 
 fs/cifs/cifsacl.c                         |    1 
 fs/nfs/idmap.c                            |    2 
 include/crypto/public_key.h               |    5 -
 include/keys/asymmetric-type.h            |   38 +++++
 include/keys/user-type.h                  |    1 
 include/linux/kernel.h                    |    1 
 include/linux/key-type.h                  |   34 ++++
 lib/hexdump.c                             |   16 ++
 net/ceph/crypto.c                         |    1 
 net/dns_resolver/dns_key.c                |   18 ++
 net/rxrpc/ar-key.c                        |    2 
 security/keys/big_key.c                   |    2 
 security/keys/encrypted-keys/encrypted.c  |    1 
 security/keys/internal.h                  |   21 +--
 security/keys/key.c                       |    2 
 security/keys/keyring.c                   |   58 +++++---
 security/keys/proc.c                      |    8 +
 security/keys/process_keys.c              |   13 +-
 security/keys/request_key.c               |   21 ++-
 security/keys/request_key_auth.c          |    6 -
 security/keys/trusted.c                   |    1 
 security/keys/user_defined.c              |   14 --
 34 files changed, 649 insertions(+), 306 deletions(-)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/10] Provide a binary to hex conversion function [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 02/10] KEYS: Preparse match data " David Howells
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

Provide a function to convert a buffer of binary data into an unterminated
ascii hex string representation of that data.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/kernel.h |    1 +
 lib/hexdump.c          |   16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 95624bed87ef..7c0ad8e38510 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -496,6 +496,7 @@ static inline char *hex_byte_pack_upper(char *buf, u8 byte)
 
 extern int hex_to_bin(char ch);
 extern int __must_check hex2bin(u8 *dst, const char *src, size_t count);
+extern char *bin2hex(char *dst, const void *src, size_t count);
 
 bool mac_pton(const char *s, u8 *mac);
 
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 8499c810909a..270773b91923 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -59,6 +59,22 @@ int hex2bin(u8 *dst, const char *src, size_t count)
 EXPORT_SYMBOL(hex2bin);
 
 /**
+ * bin2hex - convert binary data to an ascii hexadecimal string
+ * @dst: ascii hexadecimal result
+ * @src: binary data
+ * @count: binary data length
+ */
+char *bin2hex(char *dst, const void *src, size_t count)
+{
+	const unsigned char *_src = src;
+
+	while (count--)
+		dst = hex_byte_pack(dst, *_src++);
+	return dst;
+}
+EXPORT_SYMBOL(bin2hex);
+
+/**
  * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
  * @buf: data blob to dump
  * @len: number of bytes in the @buf


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/10] KEYS: Preparse match data [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
  2014-09-15 20:45 ` [PATCH 01/10] Provide a binary to hex conversion function " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 03/10] KEYS: Remove key_type::def_lookup_type " David Howells
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

Preparse the match data.  This provides several advantages:

 (1) The preparser can reject invalid criteria up front.

 (2) The preparser can convert the criteria to binary data if necessary (the
     asymmetric key type really wants to do binary comparison of the key IDs).

 (3) The preparser can set the type of search to be performed.  This means
     that it's not then a one-off setting in the key type.

 (4) The preparser can set an appropriate comparator function.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/asymmetric_type.c |   31 ++++++++++++++++++-
 include/keys/user-type.h                 |    4 ++
 include/linux/key-type.h                 |   31 +++++++++++++++++--
 net/dns_resolver/dns_key.c               |    5 ++-
 security/keys/internal.h                 |    8 ++---
 security/keys/keyring.c                  |   49 ++++++++++++++++++------------
 security/keys/proc.c                     |    8 ++---
 security/keys/process_keys.c             |   13 ++++----
 security/keys/request_key.c              |   21 ++++++++++---
 security/keys/request_key_auth.c         |    6 ++--
 security/keys/user_defined.c             |    4 +-
 11 files changed, 129 insertions(+), 51 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index eb8cd46961a5..f666b4e8d256 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -59,9 +59,11 @@ EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
  *	"id:<id>"	- request a key matching the ID
  *	"<subtype>:<id>" - request a key of a subtype
  */
-static int asymmetric_key_match(const struct key *key, const void *description)
+static int asymmetric_key_match(const struct key *key,
+				const struct key_match_data *match_data)
 {
 	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
+	const char *description = match_data->raw_data;
 	const char *spec = description;
 	const char *id;
 	ptrdiff_t speclen;
@@ -94,6 +96,31 @@ static int asymmetric_key_match(const struct key *key, const void *description)
 }
 
 /*
+ * Preparse the match criterion.  If we don't set lookup_type and cmp,
+ * the default will be an exact match on the key description.
+ *
+ * There are some specifiers for matching key IDs rather than by the key
+ * description:
+ *
+ *	"id:<id>" - request a key by any available ID
+ *
+ * These have to be searched by iteration rather than by direct lookup because
+ * the key is hashed according to its description.
+ */
+static int asymmetric_key_match_preparse(struct key_match_data *match_data)
+{
+	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	return 0;
+}
+
+/*
+ * Free the preparsed the match criterion.
+ */
+static void asymmetric_key_match_free(struct key_match_data *match_data)
+{
+}
+
+/*
  * Describe the asymmetric key
  */
 static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
@@ -196,7 +223,9 @@ struct key_type key_type_asymmetric = {
 	.preparse	= asymmetric_key_preparse,
 	.free_preparse	= asymmetric_key_free_preparse,
 	.instantiate	= generic_key_instantiate,
+	.match_preparse	= asymmetric_key_match_preparse,
 	.match		= asymmetric_key_match,
+	.match_free	= asymmetric_key_match_free,
 	.destroy	= asymmetric_key_destroy,
 	.describe	= asymmetric_key_describe,
 	.def_lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE,
diff --git a/include/keys/user-type.h b/include/keys/user-type.h
index 3ab1873a4bfa..66d92af30e7c 100644
--- a/include/keys/user-type.h
+++ b/include/keys/user-type.h
@@ -36,11 +36,13 @@ extern struct key_type key_type_user;
 extern struct key_type key_type_logon;
 
 struct key_preparsed_payload;
+struct key_match_data;
 
 extern int user_preparse(struct key_preparsed_payload *prep);
 extern void user_free_preparse(struct key_preparsed_payload *prep);
 extern int user_update(struct key *key, struct key_preparsed_payload *prep);
-extern int user_match(const struct key *key, const void *criterion);
+extern int user_match(const struct key *key,
+		      const struct key_match_data *match_data);
 extern void user_revoke(struct key *key);
 extern void user_destroy(struct key *key);
 extern void user_describe(const struct key *user, struct seq_file *m);
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 44792ee649de..8aba688a451a 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -53,6 +53,22 @@ typedef int (*request_key_actor_t)(struct key_construction *key,
 				   const char *op, void *aux);
 
 /*
+ * Preparsed matching criterion.
+ */
+struct key_match_data {
+	/* Comparison function, defaults to type->match, but can be replaced by
+	 * type->match_preparse(). */
+	int (*cmp)(const struct key *key,
+		   const struct key_match_data *match_data);
+
+	const void	*raw_data;	/* Raw match data */
+	void		*preparsed;	/* For ->match_preparse() to stash stuff */
+	unsigned	lookup_type;	/* Type of lookup for this search. */
+#define KEYRING_SEARCH_LOOKUP_DIRECT	0x0000	/* Direct lookup by description. */
+#define KEYRING_SEARCH_LOOKUP_ITERATE	0x0001	/* Iterative search. */
+};
+
+/*
  * kernel managed key type definition
  */
 struct key_type {
@@ -67,8 +83,6 @@ struct key_type {
 
 	/* Default key search algorithm. */
 	unsigned def_lookup_type;
-#define KEYRING_SEARCH_LOOKUP_DIRECT	0x0000	/* Direct lookup by description. */
-#define KEYRING_SEARCH_LOOKUP_ITERATE	0x0001	/* Iterative search. */
 
 	/* vet a description */
 	int (*vet_description)(const char *description);
@@ -96,8 +110,19 @@ struct key_type {
 	 */
 	int (*update)(struct key *key, struct key_preparsed_payload *prep);
 
+	/* Preparse the data supplied to ->match() (optional).  The
+	 * data to be preparsed can be found in match_data->raw_data.
+	 * The lookup type can also be set by this function.
+	 */
+	int (*match_preparse)(struct key_match_data *match_data);
+
 	/* match a key against a description */
-	int (*match)(const struct key *key, const void *desc);
+	int (*match)(const struct key *key,
+		     const struct key_match_data *match_data);
+
+	/* Free preparsed match data (optional).  This should be supplied it
+	 * ->match_preparse() is supplied. */
+	void (*match_free)(struct key_match_data *match_data);
 
 	/* clear some of the data from a key on revokation (optional)
 	 * - the key's semaphore will be write-locked by the caller
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index f380b2c58178..92df6e508ae7 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -177,10 +177,11 @@ static void dns_resolver_free_preparse(struct key_preparsed_payload *prep)
  * should end with a period).  The domain name is case-independent.
  */
 static int
-dns_resolver_match(const struct key *key, const void *description)
+dns_resolver_match(const struct key *key,
+		   const struct key_match_data *match_data)
 {
 	int slen, dlen, ret = 0;
-	const char *src = key->description, *dsp = description;
+	const char *src = key->description, *dsp = match_data->raw_data;
 
 	kenter("%s,%s", src, dsp);
 
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 5f20da01fd8d..805e60b0b87e 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -107,13 +107,10 @@ extern int iterate_over_keyring(const struct key *keyring,
 				int (*func)(const struct key *key, void *data),
 				void *data);
 
-typedef int (*key_match_func_t)(const struct key *, const void *);
-
 struct keyring_search_context {
 	struct keyring_index_key index_key;
 	const struct cred	*cred;
-	key_match_func_t	match;
-	const void		*match_data;
+	struct key_match_data	match_data;
 	unsigned		flags;
 #define KEYRING_SEARCH_LOOKUP_TYPE	0x0001	/* [as type->def_lookup_type] */
 #define KEYRING_SEARCH_NO_STATE_CHECK	0x0002	/* Skip state checks */
@@ -152,7 +149,8 @@ extern struct key *request_key_and_link(struct key_type *type,
 					struct key *dest_keyring,
 					unsigned long flags);
 
-extern int lookup_user_key_possessed(const struct key *key, const void *target);
+extern int lookup_user_key_possessed(const struct key *key,
+				     const struct key_match_data *match_data);
 extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
 				 key_perm_t perm);
 #define KEY_LOOKUP_CREATE	0x01
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 8314a7d2104d..10f0a5f2d362 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -545,7 +545,7 @@ static int keyring_search_iterator(const void *object, void *iterator_data)
 	}
 
 	/* keys that don't match */
-	if (!ctx->match(key, ctx->match_data)) {
+	if (!ctx->match_data.cmp(key, &ctx->match_data)) {
 		kleave(" = 0 [!match]");
 		return 0;
 	}
@@ -585,8 +585,7 @@ skipped:
  */
 static int search_keyring(struct key *keyring, struct keyring_search_context *ctx)
 {
-	if ((ctx->flags & KEYRING_SEARCH_LOOKUP_TYPE) ==
-	    KEYRING_SEARCH_LOOKUP_DIRECT) {
+	if (ctx->match_data.lookup_type == KEYRING_SEARCH_LOOKUP_DIRECT) {
 		const void *object;
 
 		object = assoc_array_find(&keyring->keys,
@@ -627,7 +626,7 @@ static bool search_nested_keyrings(struct key *keyring,
 	/* Check to see if this top-level keyring is what we are looking for
 	 * and whether it is valid or not.
 	 */
-	if (ctx->flags & KEYRING_SEARCH_LOOKUP_ITERATE ||
+	if (ctx->match_data.lookup_type == KEYRING_SEARCH_LOOKUP_ITERATE ||
 	    keyring_compare_object(keyring, &ctx->index_key)) {
 		ctx->skipped_ret = 2;
 		ctx->flags |= KEYRING_SEARCH_DO_STATE_CHECK;
@@ -885,16 +884,28 @@ key_ref_t keyring_search(key_ref_t keyring,
 		.index_key.type		= type,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match			= type->match,
-		.match_data		= description,
-		.flags			= (type->def_lookup_type |
-					   KEYRING_SEARCH_DO_STATE_CHECK),
+		.match_data.cmp		= type->match,
+		.match_data.raw_data	= description,
+		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
 	};
+	key_ref_t key;
+	int ret;
 
-	if (!ctx.match)
+	if (!ctx.match_data.cmp)
 		return ERR_PTR(-ENOKEY);
 
-	return keyring_search_aux(keyring, &ctx);
+	if (type->match_preparse) {
+		ret = type->match_preparse(&ctx.match_data);
+		if (ret < 0)
+			return ERR_PTR(ret);
+	}
+
+	key = keyring_search_aux(keyring, &ctx);
+
+	if (type->match_free)
+		type->match_free(&ctx.match_data);
+	return key;
 }
 EXPORT_SYMBOL(keyring_search);
 
@@ -1014,7 +1025,7 @@ static int keyring_detect_cycle_iterator(const void *object,
 
 	/* We might get a keyring with matching index-key that is nonetheless a
 	 * different keyring. */
-	if (key != ctx->match_data)
+	if (key != ctx->match_data.raw_data)
 		return 0;
 
 	ctx->result = ERR_PTR(-EDEADLK);
@@ -1031,14 +1042,14 @@ static int keyring_detect_cycle_iterator(const void *object,
 static int keyring_detect_cycle(struct key *A, struct key *B)
 {
 	struct keyring_search_context ctx = {
-		.index_key	= A->index_key,
-		.match_data	= A,
-		.iterator	= keyring_detect_cycle_iterator,
-		.flags		= (KEYRING_SEARCH_LOOKUP_DIRECT |
-				   KEYRING_SEARCH_NO_STATE_CHECK |
-				   KEYRING_SEARCH_NO_UPDATE_TIME |
-				   KEYRING_SEARCH_NO_CHECK_PERM |
-				   KEYRING_SEARCH_DETECT_TOO_DEEP),
+		.index_key		= A->index_key,
+		.match_data.raw_data	= A,
+		.match_data.lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT,
+		.iterator		= keyring_detect_cycle_iterator,
+		.flags			= (KEYRING_SEARCH_NO_STATE_CHECK |
+					   KEYRING_SEARCH_NO_UPDATE_TIME |
+					   KEYRING_SEARCH_NO_CHECK_PERM |
+					   KEYRING_SEARCH_DETECT_TOO_DEEP),
 	};
 
 	rcu_read_lock();
diff --git a/security/keys/proc.c b/security/keys/proc.c
index d3f6f2fd21db..972eeb336b81 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -194,10 +194,10 @@ static int proc_keys_show(struct seq_file *m, void *v)
 		.index_key.type		= key->type,
 		.index_key.description	= key->description,
 		.cred			= current_cred(),
-		.match			= lookup_user_key_possessed,
-		.match_data		= key,
-		.flags			= (KEYRING_SEARCH_NO_STATE_CHECK |
-					   KEYRING_SEARCH_LOOKUP_DIRECT),
+		.match_data.cmp		= lookup_user_key_possessed,
+		.match_data.raw_data	= key,
+		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.flags			= KEYRING_SEARCH_NO_STATE_CHECK,
 	};
 
 	key_ref = make_key_ref(key, 0);
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 0cf8a130a267..08bd533d014f 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -489,9 +489,10 @@ found:
 /*
  * See if the key we're looking at is the target key.
  */
-int lookup_user_key_possessed(const struct key *key, const void *target)
+int lookup_user_key_possessed(const struct key *key,
+			      const struct key_match_data *match_data)
 {
-	return key == target;
+	return key == match_data->raw_data;
 }
 
 /*
@@ -516,9 +517,9 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 			  key_perm_t perm)
 {
 	struct keyring_search_context ctx = {
-		.match	= lookup_user_key_possessed,
-		.flags	= (KEYRING_SEARCH_NO_STATE_CHECK |
-			   KEYRING_SEARCH_LOOKUP_DIRECT),
+		.match_data.cmp		= lookup_user_key_possessed,
+		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.flags			= KEYRING_SEARCH_NO_STATE_CHECK,
 	};
 	struct request_key_auth *rka;
 	struct key *key;
@@ -673,7 +674,7 @@ try_again:
 		ctx.index_key.type		= key->type;
 		ctx.index_key.description	= key->description;
 		ctx.index_key.desc_len		= strlen(key->description);
-		ctx.match_data			= key;
+		ctx.match_data.raw_data		= key;
 		kdebug("check possessed");
 		skey_ref = search_process_keyrings(&ctx);
 		kdebug("possessed=%p", skey_ref);
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 26a94f18af94..5dd2088ade23 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -513,9 +513,9 @@ struct key *request_key_and_link(struct key_type *type,
 		.index_key.type		= type,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match			= type->match,
-		.match_data		= description,
-		.flags			= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.match_data.cmp		= type->match,
+		.match_data.raw_data	= description,
+		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	};
 	struct key *key;
 	key_ref_t key_ref;
@@ -525,6 +525,14 @@ struct key *request_key_and_link(struct key_type *type,
 	       ctx.index_key.type->name, ctx.index_key.description,
 	       callout_info, callout_len, aux, dest_keyring, flags);
 
+	if (type->match_preparse) {
+		ret = type->match_preparse(&ctx.match_data);
+		if (ret < 0) {
+			key = ERR_PTR(ret);
+			goto error;
+		}
+	}
+
 	/* search all the process keyrings for a key */
 	key_ref = search_process_keyrings(&ctx);
 
@@ -537,7 +545,7 @@ struct key *request_key_and_link(struct key_type *type,
 			if (ret < 0) {
 				key_put(key);
 				key = ERR_PTR(ret);
-				goto error;
+				goto error_free;
 			}
 		}
 	} else if (PTR_ERR(key_ref) != -EAGAIN) {
@@ -547,12 +555,15 @@ struct key *request_key_and_link(struct key_type *type,
 		 * should consult userspace if we can */
 		key = ERR_PTR(-ENOKEY);
 		if (!callout_info)
-			goto error;
+			goto error_free;
 
 		key = construct_key_and_link(&ctx, callout_info, callout_len,
 					     aux, dest_keyring, flags);
 	}
 
+error_free:
+	if (type->match_free)
+		type->match_free(&ctx.match_data);
 error:
 	kleave(" = %p", key);
 	return key;
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 739e7455d388..9ae02819cc06 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -246,9 +246,9 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
 		.index_key.type		= &key_type_request_key_auth,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match			= user_match,
-		.match_data		= description,
-		.flags			= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.match_data.cmp		= user_match,
+		.match_data.raw_data	= description,
+		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	};
 	struct key *authkey;
 	key_ref_t authkey_ref;
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index eee340011f2b..ec8a56063b02 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -141,9 +141,9 @@ EXPORT_SYMBOL_GPL(user_update);
 /*
  * match users on their name
  */
-int user_match(const struct key *key, const void *description)
+int user_match(const struct key *key, const struct key_match_data *match_data)
 {
-	return strcmp(key->description, description) == 0;
+	return strcmp(key->description, match_data->raw_data) == 0;
 }
 
 EXPORT_SYMBOL_GPL(user_match);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/10] KEYS: Remove key_type::def_lookup_type [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
  2014-09-15 20:45 ` [PATCH 01/10] Provide a binary to hex conversion function " David Howells
  2014-09-15 20:45 ` [PATCH 02/10] KEYS: Preparse match data " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 04/10] KEYS: Remove key_type::match in favour of overriding default by match_preparse " David Howells
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

Remove key_type::def_lookup_type as it's no longer used.  The information now
defaults to KEYRING_SEARCH_LOOKUP_DIRECT but may be overridden by
type->match_preparse().

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/asymmetric_type.c |    1 -
 crypto/asymmetric_keys/pkcs7_key_type.c  |    1 -
 include/linux/key-type.h                 |    3 ---
 security/keys/big_key.c                  |    1 -
 security/keys/internal.h                 |   11 +++++------
 security/keys/user_defined.c             |    2 --
 6 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index f666b4e8d256..9d78ad7754d9 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -228,7 +228,6 @@ struct key_type key_type_asymmetric = {
 	.match_free	= asymmetric_key_match_free,
 	.destroy	= asymmetric_key_destroy,
 	.describe	= asymmetric_key_describe,
-	.def_lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE,
 };
 EXPORT_SYMBOL_GPL(key_type_asymmetric);
 
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
index 3de5fb011de0..d1faa1df1dec 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -72,7 +72,6 @@ error:
  */
 static struct key_type key_type_pkcs7 = {
 	.name			= "pkcs7_test",
-	.def_lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	.preparse		= pkcs7_preparse,
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 8aba688a451a..bf93ea609273 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -81,9 +81,6 @@ struct key_type {
 	 */
 	size_t def_datalen;
 
-	/* Default key search algorithm. */
-	unsigned def_lookup_type;
-
 	/* vet a description */
 	int (*vet_description)(const char *description);
 
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index c2f91a0cf889..4045c13a761a 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -33,7 +33,6 @@ MODULE_LICENSE("GPL");
  */
 struct key_type key_type_big_key = {
 	.name			= "big_key",
-	.def_lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	.preparse		= big_key_preparse,
 	.free_preparse		= big_key_free_preparse,
 	.instantiate		= generic_key_instantiate,
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 805e60b0b87e..b47cc532be1e 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -112,12 +112,11 @@ struct keyring_search_context {
 	const struct cred	*cred;
 	struct key_match_data	match_data;
 	unsigned		flags;
-#define KEYRING_SEARCH_LOOKUP_TYPE	0x0001	/* [as type->def_lookup_type] */
-#define KEYRING_SEARCH_NO_STATE_CHECK	0x0002	/* Skip state checks */
-#define KEYRING_SEARCH_DO_STATE_CHECK	0x0004	/* Override NO_STATE_CHECK */
-#define KEYRING_SEARCH_NO_UPDATE_TIME	0x0008	/* Don't update times */
-#define KEYRING_SEARCH_NO_CHECK_PERM	0x0010	/* Don't check permissions */
-#define KEYRING_SEARCH_DETECT_TOO_DEEP	0x0020	/* Give an error on excessive depth */
+#define KEYRING_SEARCH_NO_STATE_CHECK	0x0001	/* Skip state checks */
+#define KEYRING_SEARCH_DO_STATE_CHECK	0x0002	/* Override NO_STATE_CHECK */
+#define KEYRING_SEARCH_NO_UPDATE_TIME	0x0004	/* Don't update times */
+#define KEYRING_SEARCH_NO_CHECK_PERM	0x0008	/* Don't check permissions */
+#define KEYRING_SEARCH_DETECT_TOO_DEEP	0x0010	/* Give an error on excessive depth */
 
 	int (*iterator)(const void *object, void *iterator_data);
 
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index ec8a56063b02..cd7e726e8646 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -26,7 +26,6 @@ static int logon_vet_description(const char *desc);
  */
 struct key_type key_type_user = {
 	.name			= "user",
-	.def_lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	.preparse		= user_preparse,
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
@@ -48,7 +47,6 @@ EXPORT_SYMBOL_GPL(key_type_user);
  */
 struct key_type key_type_logon = {
 	.name			= "logon",
-	.def_lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	.preparse		= user_preparse,
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/10] KEYS: Remove key_type::match in favour of overriding default by match_preparse [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (2 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 03/10] KEYS: Remove key_type::def_lookup_type " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 05/10] KEYS: Make the key matching functions return bool " David Howells
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

A previous patch added a ->match_preparse() method to the key type.  This is
allowed to override the function called by the iteration algorithm.
Therefore, we can just set a default that simply checks for an exact match of
the key description with the original criterion data and allow match_preparse
to override it as needed.

The key_type::match op is then redundant and can be removed, as can the
user_match() function.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/asymmetric_type.c |    6 +++---
 crypto/asymmetric_keys/pkcs7_key_type.c  |    1 -
 fs/cifs/cifs_spnego.c                    |    1 -
 fs/cifs/cifsacl.c                        |    1 -
 fs/nfs/idmap.c                           |    2 --
 include/keys/user-type.h                 |    3 ---
 include/linux/key-type.h                 |    4 ----
 net/ceph/crypto.c                        |    1 -
 net/dns_resolver/dns_key.c               |   17 +++++++++++++----
 net/rxrpc/ar-key.c                       |    2 --
 security/keys/big_key.c                  |    1 -
 security/keys/encrypted-keys/encrypted.c |    1 -
 security/keys/internal.h                 |    2 ++
 security/keys/key.c                      |    2 +-
 security/keys/keyring.c                  |   15 ++++++++++-----
 security/keys/request_key.c              |    2 +-
 security/keys/request_key_auth.c         |    2 +-
 security/keys/trusted.c                  |    1 -
 security/keys/user_defined.c             |   12 ------------
 19 files changed, 31 insertions(+), 45 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 9d78ad7754d9..7c0498968975 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -59,8 +59,8 @@ EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
  *	"id:<id>"	- request a key matching the ID
  *	"<subtype>:<id>" - request a key of a subtype
  */
-static int asymmetric_key_match(const struct key *key,
-				const struct key_match_data *match_data)
+static int asymmetric_key_cmp(const struct key *key,
+			      const struct key_match_data *match_data)
 {
 	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
 	const char *description = match_data->raw_data;
@@ -110,6 +110,7 @@ static int asymmetric_key_match(const struct key *key,
 static int asymmetric_key_match_preparse(struct key_match_data *match_data)
 {
 	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	match_data->cmp = asymmetric_key_cmp;
 	return 0;
 }
 
@@ -224,7 +225,6 @@ struct key_type key_type_asymmetric = {
 	.free_preparse	= asymmetric_key_free_preparse,
 	.instantiate	= generic_key_instantiate,
 	.match_preparse	= asymmetric_key_match_preparse,
-	.match		= asymmetric_key_match,
 	.match_free	= asymmetric_key_match_free,
 	.destroy	= asymmetric_key_destroy,
 	.describe	= asymmetric_key_describe,
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
index d1faa1df1dec..751f8fd7335d 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -75,7 +75,6 @@ static struct key_type key_type_pkcs7 = {
 	.preparse		= pkcs7_preparse,
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
-	.match			= user_match,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
 	.describe		= user_describe,
diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c
index a3e932547617..f4cf200b3c76 100644
--- a/fs/cifs/cifs_spnego.c
+++ b/fs/cifs/cifs_spnego.c
@@ -62,7 +62,6 @@ cifs_spnego_key_destroy(struct key *key)
 struct key_type cifs_spnego_key_type = {
 	.name		= "cifs.spnego",
 	.instantiate	= cifs_spnego_key_instantiate,
-	.match		= user_match,
 	.destroy	= cifs_spnego_key_destroy,
 	.describe	= user_describe,
 };
diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 7ff866dbb89e..6d00c419cbae 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -84,7 +84,6 @@ static struct key_type cifs_idmap_key_type = {
 	.instantiate = cifs_idmap_key_instantiate,
 	.destroy     = cifs_idmap_key_destroy,
 	.describe    = user_describe,
-	.match       = user_match,
 };
 
 static char *
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 7dd55b745c4d..2f5db844c172 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -177,7 +177,6 @@ static struct key_type key_type_id_resolver = {
 	.preparse	= user_preparse,
 	.free_preparse	= user_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.revoke		= user_revoke,
 	.destroy	= user_destroy,
 	.describe	= user_describe,
@@ -401,7 +400,6 @@ static struct key_type key_type_id_resolver_legacy = {
 	.preparse	= user_preparse,
 	.free_preparse	= user_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.revoke		= user_revoke,
 	.destroy	= user_destroy,
 	.describe	= user_describe,
diff --git a/include/keys/user-type.h b/include/keys/user-type.h
index 66d92af30e7c..cebefb069c44 100644
--- a/include/keys/user-type.h
+++ b/include/keys/user-type.h
@@ -36,13 +36,10 @@ extern struct key_type key_type_user;
 extern struct key_type key_type_logon;
 
 struct key_preparsed_payload;
-struct key_match_data;
 
 extern int user_preparse(struct key_preparsed_payload *prep);
 extern void user_free_preparse(struct key_preparsed_payload *prep);
 extern int user_update(struct key *key, struct key_preparsed_payload *prep);
-extern int user_match(const struct key *key,
-		      const struct key_match_data *match_data);
 extern void user_revoke(struct key *key);
 extern void user_destroy(struct key *key);
 extern void user_describe(const struct key *user, struct seq_file *m);
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index bf93ea609273..c14816bd3b44 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -113,10 +113,6 @@ struct key_type {
 	 */
 	int (*match_preparse)(struct key_match_data *match_data);
 
-	/* match a key against a description */
-	int (*match)(const struct key *key,
-		     const struct key_match_data *match_data);
-
 	/* Free preparsed match data (optional).  This should be supplied it
 	 * ->match_preparse() is supplied. */
 	void (*match_free)(struct key_match_data *match_data);
diff --git a/net/ceph/crypto.c b/net/ceph/crypto.c
index ffeba8f9dda9..62fc5e7a9acf 100644
--- a/net/ceph/crypto.c
+++ b/net/ceph/crypto.c
@@ -476,7 +476,6 @@ struct key_type key_type_ceph = {
 	.preparse	= ceph_key_preparse,
 	.free_preparse	= ceph_key_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.destroy	= ceph_key_destroy,
 };
 
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 92df6e508ae7..a07b9ba7e0b7 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -176,9 +176,8 @@ static void dns_resolver_free_preparse(struct key_preparsed_payload *prep)
  * The domain name may be a simple name or an absolute domain name (which
  * should end with a period).  The domain name is case-independent.
  */
-static int
-dns_resolver_match(const struct key *key,
-		   const struct key_match_data *match_data)
+static int dns_resolver_cmp(const struct key *key,
+			    const struct key_match_data *match_data)
 {
 	int slen, dlen, ret = 0;
 	const char *src = key->description, *dsp = match_data->raw_data;
@@ -210,6 +209,16 @@ no_match:
 }
 
 /*
+ * Preparse the match criterion.
+ */
+static int dns_resolver_match_preparse(struct key_match_data *match_data)
+{
+	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	match_data->cmp = dns_resolver_cmp;
+	return 0;
+}
+
+/*
  * Describe a DNS key
  */
 static void dns_resolver_describe(const struct key *key, struct seq_file *m)
@@ -243,7 +252,7 @@ struct key_type key_type_dns_resolver = {
 	.preparse	= dns_resolver_preparse,
 	.free_preparse	= dns_resolver_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= dns_resolver_match,
+	.match_preparse	= dns_resolver_match_preparse,
 	.revoke		= user_revoke,
 	.destroy	= user_destroy,
 	.describe	= dns_resolver_describe,
diff --git a/net/rxrpc/ar-key.c b/net/rxrpc/ar-key.c
index b45d080e64a7..c08d4649157a 100644
--- a/net/rxrpc/ar-key.c
+++ b/net/rxrpc/ar-key.c
@@ -44,7 +44,6 @@ struct key_type key_type_rxrpc = {
 	.preparse	= rxrpc_preparse,
 	.free_preparse	= rxrpc_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.destroy	= rxrpc_destroy,
 	.describe	= rxrpc_describe,
 	.read		= rxrpc_read,
@@ -61,7 +60,6 @@ struct key_type key_type_rxrpc_s = {
 	.preparse	= rxrpc_preparse_s,
 	.free_preparse	= rxrpc_free_preparse_s,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.destroy	= rxrpc_destroy_s,
 	.describe	= rxrpc_describe,
 };
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 4045c13a761a..b6adb94f6d52 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -36,7 +36,6 @@ struct key_type key_type_big_key = {
 	.preparse		= big_key_preparse,
 	.free_preparse		= big_key_free_preparse,
 	.instantiate		= generic_key_instantiate,
-	.match			= user_match,
 	.revoke			= big_key_revoke,
 	.destroy		= big_key_destroy,
 	.describe		= big_key_describe,
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 5fe443d120af..db9675db1026 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -970,7 +970,6 @@ struct key_type key_type_encrypted = {
 	.name = "encrypted",
 	.instantiate = encrypted_instantiate,
 	.update = encrypted_update,
-	.match = user_match,
 	.destroy = encrypted_destroy,
 	.describe = user_describe,
 	.read = encrypted_read,
diff --git a/security/keys/internal.h b/security/keys/internal.h
index b47cc532be1e..e66a16cb63e1 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -127,6 +127,8 @@ struct keyring_search_context {
 	struct timespec		now;
 };
 
+extern int key_default_cmp(const struct key *key,
+			   const struct key_match_data *match_data);
 extern key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 				    struct keyring_search_context *ctx);
 
diff --git a/security/keys/key.c b/security/keys/key.c
index 6d0cad16f002..e17ba6aefdc0 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -799,7 +799,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 	}
 
 	key_ref = ERR_PTR(-EINVAL);
-	if (!index_key.type->match || !index_key.type->instantiate ||
+	if (!index_key.type->instantiate ||
 	    (!index_key.description && !index_key.type->preparse))
 		goto error_put_type;
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 10f0a5f2d362..253c9a0eb092 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -89,7 +89,6 @@ struct key_type key_type_keyring = {
 	.preparse	= keyring_preparse,
 	.free_preparse	= keyring_free_preparse,
 	.instantiate	= keyring_instantiate,
-	.match		= user_match,
 	.revoke		= keyring_revoke,
 	.destroy	= keyring_destroy,
 	.describe	= keyring_describe,
@@ -512,6 +511,15 @@ struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t gid,
 EXPORT_SYMBOL(keyring_alloc);
 
 /*
+ * By default, we keys found by getting an exact match on their descriptions.
+ */
+int key_default_cmp(const struct key *key,
+		    const struct key_match_data *match_data)
+{
+	return strcmp(key->description, match_data->raw_data) == 0;
+}
+
+/*
  * Iteration function to consider each key found.
  */
 static int keyring_search_iterator(const void *object, void *iterator_data)
@@ -884,7 +892,7 @@ key_ref_t keyring_search(key_ref_t keyring,
 		.index_key.type		= type,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match_data.cmp		= type->match,
+		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
@@ -892,9 +900,6 @@ key_ref_t keyring_search(key_ref_t keyring,
 	key_ref_t key;
 	int ret;
 
-	if (!ctx.match_data.cmp)
-		return ERR_PTR(-ENOKEY);
-
 	if (type->match_preparse) {
 		ret = type->match_preparse(&ctx.match_data);
 		if (ret < 0)
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 5dd2088ade23..bb4337c7ae1b 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -513,7 +513,7 @@ struct key *request_key_and_link(struct key_type *type,
 		.index_key.type		= type,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match_data.cmp		= type->match,
+		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	};
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 9ae02819cc06..6639e2cb8853 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -246,7 +246,7 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
 		.index_key.type		= &key_type_request_key_auth,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match_data.cmp		= user_match,
+		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	};
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 6b804aa4529a..c0594cb07ada 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1096,7 +1096,6 @@ struct key_type key_type_trusted = {
 	.name = "trusted",
 	.instantiate = trusted_instantiate,
 	.update = trusted_update,
-	.match = user_match,
 	.destroy = trusted_destroy,
 	.describe = user_describe,
 	.read = trusted_read,
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index cd7e726e8646..36b47bbd3d8c 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -30,7 +30,6 @@ struct key_type key_type_user = {
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
 	.update			= user_update,
-	.match			= user_match,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
 	.describe		= user_describe,
@@ -51,7 +50,6 @@ struct key_type key_type_logon = {
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
 	.update			= user_update,
-	.match			= user_match,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
 	.describe		= user_describe,
@@ -137,16 +135,6 @@ error:
 EXPORT_SYMBOL_GPL(user_update);
 
 /*
- * match users on their name
- */
-int user_match(const struct key *key, const struct key_match_data *match_data)
-{
-	return strcmp(key->description, match_data->raw_data) == 0;
-}
-
-EXPORT_SYMBOL_GPL(user_match);
-
-/*
  * dispose of the links from a revoked keyring
  * - called with the key sem write-locked
  */


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/10] KEYS: Make the key matching functions return bool [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (3 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 04/10] KEYS: Remove key_type::match in favour of overriding default by match_preparse " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 06/10] KEYS: Update the keyrings documentation for match changes " David Howells
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

Make the key matching functions pointed to by key_match_data::cmp return bool
rather than int.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/asymmetric_type.c |    4 ++--
 include/linux/key-type.h                 |   10 ++++++----
 net/dns_resolver/dns_key.c               |    4 ++--
 security/keys/internal.h                 |    8 ++++----
 security/keys/keyring.c                  |    4 ++--
 security/keys/process_keys.c             |    4 ++--
 6 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 7c0498968975..7755f918e8d9 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -59,8 +59,8 @@ EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
  *	"id:<id>"	- request a key matching the ID
  *	"<subtype>:<id>" - request a key of a subtype
  */
-static int asymmetric_key_cmp(const struct key *key,
-			      const struct key_match_data *match_data)
+static bool asymmetric_key_cmp(const struct key *key,
+			       const struct key_match_data *match_data)
 {
 	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
 	const char *description = match_data->raw_data;
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index c14816bd3b44..ff9f1d394235 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -56,10 +56,12 @@ typedef int (*request_key_actor_t)(struct key_construction *key,
  * Preparsed matching criterion.
  */
 struct key_match_data {
-	/* Comparison function, defaults to type->match, but can be replaced by
-	 * type->match_preparse(). */
-	int (*cmp)(const struct key *key,
-		   const struct key_match_data *match_data);
+	/* Comparison function, defaults to exact description match, but can be
+	 * overridden by type->match_preparse().  Should return true if a match
+	 * is found and false if not.
+	 */
+	bool (*cmp)(const struct key *key,
+		    const struct key_match_data *match_data);
 
 	const void	*raw_data;	/* Raw match data */
 	void		*preparsed;	/* For ->match_preparse() to stash stuff */
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index a07b9ba7e0b7..31cd4fd75486 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -176,8 +176,8 @@ static void dns_resolver_free_preparse(struct key_preparsed_payload *prep)
  * The domain name may be a simple name or an absolute domain name (which
  * should end with a period).  The domain name is case-independent.
  */
-static int dns_resolver_cmp(const struct key *key,
-			    const struct key_match_data *match_data)
+static bool dns_resolver_cmp(const struct key *key,
+			     const struct key_match_data *match_data)
 {
 	int slen, dlen, ret = 0;
 	const char *src = key->description, *dsp = match_data->raw_data;
diff --git a/security/keys/internal.h b/security/keys/internal.h
index e66a16cb63e1..b8960c4959a5 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -127,8 +127,8 @@ struct keyring_search_context {
 	struct timespec		now;
 };
 
-extern int key_default_cmp(const struct key *key,
-			   const struct key_match_data *match_data);
+extern bool key_default_cmp(const struct key *key,
+			    const struct key_match_data *match_data);
 extern key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 				    struct keyring_search_context *ctx);
 
@@ -150,8 +150,8 @@ extern struct key *request_key_and_link(struct key_type *type,
 					struct key *dest_keyring,
 					unsigned long flags);
 
-extern int lookup_user_key_possessed(const struct key *key,
-				     const struct key_match_data *match_data);
+extern bool lookup_user_key_possessed(const struct key *key,
+				      const struct key_match_data *match_data);
 extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
 				 key_perm_t perm);
 #define KEY_LOOKUP_CREATE	0x01
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 253c9a0eb092..8177010174f7 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -513,8 +513,8 @@ EXPORT_SYMBOL(keyring_alloc);
 /*
  * By default, we keys found by getting an exact match on their descriptions.
  */
-int key_default_cmp(const struct key *key,
-		    const struct key_match_data *match_data)
+bool key_default_cmp(const struct key *key,
+		     const struct key_match_data *match_data)
 {
 	return strcmp(key->description, match_data->raw_data) == 0;
 }
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 08bd533d014f..bd536cb221e2 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -489,8 +489,8 @@ found:
 /*
  * See if the key we're looking at is the target key.
  */
-int lookup_user_key_possessed(const struct key *key,
-			      const struct key_match_data *match_data)
+bool lookup_user_key_possessed(const struct key *key,
+			       const struct key_match_data *match_data)
 {
 	return key == match_data->raw_data;
 }


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/10] KEYS: Update the keyrings documentation for match changes [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (4 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 05/10] KEYS: Make the key matching functions return bool " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 07/10] KEYS: Implement binary asymmetric key ID handling " David Howells
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel


---

 Documentation/security/keys.txt |   65 +++++++++++++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 13 deletions(-)

diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 8727c194ca16..821c936e1a63 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -888,11 +888,11 @@ payload contents" for more information.
 				const char *callout_info);
 
     This is used to request a key or keyring with a description that matches
-    the description specified according to the key type's match function. This
-    permits approximate matching to occur. If callout_string is not NULL, then
-    /sbin/request-key will be invoked in an attempt to obtain the key from
-    userspace. In that case, callout_string will be passed as an argument to
-    the program.
+    the description specified according to the key type's match_preparse()
+    method. This permits approximate matching to occur. If callout_string is
+    not NULL, then /sbin/request-key will be invoked in an attempt to obtain
+    the key from userspace. In that case, callout_string will be passed as an
+    argument to the program.
 
     Should the function fail error ENOKEY, EKEYEXPIRED or EKEYREVOKED will be
     returned.
@@ -1170,7 +1170,7 @@ The structure has a number of fields, some of which are mandatory:
      The method should return 0 if successful or a negative error code
      otherwise.
 
-     
+
  (*) void (*free_preparse)(struct key_preparsed_payload *prep);
 
      This method is only required if the preparse() method is provided,
@@ -1225,16 +1225,55 @@ The structure has a number of fields, some of which are mandatory:
      It is safe to sleep in this method.
 
 
- (*) int (*match)(const struct key *key, const void *desc);
+ (*) int (*match_preparse)(struct key_match_data *match_data);
+
+     This method is optional.  It is called when a key search is about to be
+     performed.  It is given the following structure:
 
-     This method is called to match a key against a description. It should
-     return non-zero if the two match, zero if they don't.
+	struct key_match_data {
+		bool (*cmp)(const struct key *key,
+			    const struct key_match_data *match_data);
+		const void	*raw_data;
+		void		*preparsed;
+		unsigned	lookup_type;
+	};
 
-     This method should not need to lock the key in any way. The type and
-     description can be considered invariant, and the payload should not be
-     accessed (the key may not yet be instantiated).
+     On entry, raw_data will be pointing to the criteria to be used in matching
+     a key by the caller and should not be modified.  (*cmp)() will be pointing
+     to the default matcher function (which does an exact description match
+     against raw_data) and lookup_type will be set to indicate a direct lookup.
 
-     It is not safe to sleep in this method; the caller may hold spinlocks.
+     The following lookup_type values are available:
+
+      [*] KEYRING_SEARCH_LOOKUP_DIRECT - A direct lookup hashes the type and
+      	  description to narrow down the search to a small number of keys.
+
+      [*] KEYRING_SEARCH_LOOKUP_ITERATE - An iterative lookup walks all the
+      	  keys in the keyring until one is matched.  This must be used for any
+      	  search that's not doing a simple direct match on the key description.
+
+     The method may set cmp to point to a function of its choice that does some
+     other form of match, may set lookup_type to KEYRING_SEARCH_LOOKUP_ITERATE
+     and may attach something to the preparsed pointer for use by (*cmp)().
+     (*cmp)() should return true if a key matches and false otherwise.
+
+     If preparsed is set, it may be necessary to use the match_free() method to
+     clean it up.
+
+     The method should return 0 if successful or a negative error code
+     otherwise.
+
+     It is permitted to sleep in this method, but (*cmp)() may not sleep as
+     locks will be held over it.
+
+     If match_preparse() is not provided, keys of this type will be matched
+     exactly by their description.
+
+
+ (*) void (*match_free)(struct key_match_data *match_data);
+
+     This method is optional.  If given, it called to clean up
+     match_data->preparsed after a successful call to match_preparse().
 
 
  (*) void (*revoke)(struct key *key);


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/10] KEYS: Implement binary asymmetric key ID handling [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (5 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 06/10] KEYS: Update the keyrings documentation for match changes " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 08/10] KEYS: Overhaul key identification when searching for asymmetric keys " David Howells
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

Implement the first step in using binary key IDs for asymmetric keys rather
than hex string keys.

The previously added match data preparsing will be able to convert hex
criterion strings into binary which can then be compared more rapidly.

Further, we actually want more then one ID string per public key.  The problem
is that X.509 certs refer to other X.509 certs by matching Issuer + AuthKeyId
to Subject + SubjKeyId, but PKCS#7 messages match against X.509 Issuer +
SerialNumber.

This patch just provides facilities for a later patch to make use of.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/asymmetric_keys.h |    4 +
 crypto/asymmetric_keys/asymmetric_type.c |   89 ++++++++++++++++++++++++++++++
 include/keys/asymmetric-type.h           |   38 +++++++++++++
 3 files changed, 131 insertions(+)

diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
index a63c551c6557..917be6b985e7 100644
--- a/crypto/asymmetric_keys/asymmetric_keys.h
+++ b/crypto/asymmetric_keys/asymmetric_keys.h
@@ -10,6 +10,10 @@
  */
 
 int asymmetric_keyid_match(const char *kid, const char *id);
+extern bool asymmetric_match_key_ids(const struct asymmetric_key_ids *kids,
+				     const struct asymmetric_key_id *match_id);
+
+extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
 
 static inline const char *asymmetric_key_id(const struct key *key)
 {
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 7755f918e8d9..92bfc438dd1d 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -15,6 +15,7 @@
 #include <linux/seq_file.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/ctype.h>
 #include "asymmetric_keys.h"
 
 MODULE_LICENSE("GPL");
@@ -22,6 +23,94 @@ MODULE_LICENSE("GPL");
 static LIST_HEAD(asymmetric_key_parsers);
 static DECLARE_RWSEM(asymmetric_key_parsers_sem);
 
+/**
+ * asymmetric_key_generate_id: Construct an asymmetric key ID
+ * @val_1: First binary blob
+ * @len_1: Length of first binary blob
+ * @val_2: Second binary blob
+ * @len_2: Length of second binary blob
+ *
+ * Construct an asymmetric key ID from a pair of binary blobs.
+ */
+struct asymmetric_key_id *asymmetric_key_generate_id(const void *val_1,
+						     size_t len_1,
+						     const void *val_2,
+						     size_t len_2)
+{
+	struct asymmetric_key_id *kid;
+
+	kid = kmalloc(sizeof(struct asymmetric_key_id) + len_1 + len_2,
+		      GFP_KERNEL);
+	if (!kid)
+		return ERR_PTR(-ENOMEM);
+	kid->len = len_1 + len_2;
+	memcpy(kid->data, val_1, len_1);
+	memcpy(kid->data + len_1, val_2, len_2);
+	return kid;
+}
+EXPORT_SYMBOL_GPL(asymmetric_key_generate_id);
+
+/**
+ * asymmetric_key_id_same - Return true if two asymmetric keys IDs are the same.
+ * @kid_1, @kid_2: The key IDs to compare
+ */
+bool asymmetric_key_id_same(const struct asymmetric_key_id *kid1,
+			    const struct asymmetric_key_id *kid2)
+{
+	if (!kid1 || !kid2)
+		return false;
+	if (kid1->len != kid2->len)
+		return false;
+	return memcmp(kid1->data, kid2->data, kid1->len) == 0;
+}
+EXPORT_SYMBOL_GPL(asymmetric_key_id_same);
+
+/**
+ * asymmetric_match_key_ids - Search asymmetric key IDs
+ * @kids: The list of key IDs to check
+ * @match_id: The key ID we're looking for
+ */
+bool asymmetric_match_key_ids(const struct asymmetric_key_ids *kids,
+			      const struct asymmetric_key_id *match_id)
+{
+	if (!kids || !match_id)
+		return false;
+	if (asymmetric_key_id_same(kids->id[0], match_id))
+		return true;
+	if (asymmetric_key_id_same(kids->id[1], match_id))
+		return true;
+	return false;
+}
+EXPORT_SYMBOL_GPL(asymmetric_match_key_ids);
+
+/**
+ * asymmetric_key_hex_to_key_id - Convert a hex string into a key ID.
+ * @id: The ID as a hex string.
+ */
+struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
+{
+	struct asymmetric_key_id *match_id;
+	const char *p;
+	ptrdiff_t hexlen;
+
+	if (!*id)
+		return ERR_PTR(-EINVAL);
+	for (p = id; *p; p++)
+		if (!isxdigit(*p))
+			return ERR_PTR(-EINVAL);
+	hexlen = p - id;
+	if (hexlen & 1)
+		return ERR_PTR(-EINVAL);
+
+	match_id = kmalloc(sizeof(struct asymmetric_key_id) + hexlen / 2,
+			   GFP_KERNEL);
+	if (!match_id)
+		return ERR_PTR(-ENOMEM);
+	match_id->len = hexlen / 2;
+	(void)hex2bin(match_id->data, id, hexlen / 2);
+	return match_id;
+}
+
 /*
  * Match asymmetric key id with partial match
  * @id:		key id to match in a form "id:<id>"
diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h
index 7dd473496180..044ab0d3aa45 100644
--- a/include/keys/asymmetric-type.h
+++ b/include/keys/asymmetric-type.h
@@ -19,6 +19,44 @@
 extern struct key_type key_type_asymmetric;
 
 /*
+ * Identifiers for an asymmetric key ID.  We have three ways of looking up a
+ * key derived from an X.509 certificate:
+ *
+ * (1) Serial Number & Issuer.  Non-optional.  This is the only valid way to
+ *     map a PKCS#7 signature to an X.509 certificate.
+ *
+ * (2) Issuer & Subject Unique IDs.  Optional.  These were the original way to
+ *     match X.509 certificates, but have fallen into disuse in favour of (3).
+ *
+ * (3) Auth & Subject Key Identifiers.  Optional.  SKIDs are only provided on
+ *     CA keys that are intended to sign other keys, so don't appear in end
+ *     user certificates unless forced.
+ *
+ * We could also support an PGP key identifier, which is just a SHA1 sum of the
+ * public key and certain parameters, but since we don't support PGP keys at
+ * the moment, we shall ignore those.
+ *
+ * What we actually do is provide a place where binary identifiers can be
+ * stashed and then compare against them when checking for an id match.
+ */
+struct asymmetric_key_id {
+	unsigned short	len;
+	unsigned char	data[];
+};
+
+struct asymmetric_key_ids {
+	void		*id[2];
+};
+
+extern bool asymmetric_key_id_same(const struct asymmetric_key_id *kid1,
+				   const struct asymmetric_key_id *kid2);
+
+extern struct asymmetric_key_id *asymmetric_key_generate_id(const void *val_1,
+							    size_t len_1,
+							    const void *val_2,
+							    size_t len_2);
+
+/*
  * The payload is at the discretion of the subtype.
  */
 


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/10] KEYS: Overhaul key identification when searching for asymmetric keys [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (6 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 07/10] KEYS: Implement binary asymmetric key ID handling " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 09/10] PKCS#7: Better handling of unsupported crypto " David Howells
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

Make use of the new match string preparsing to overhaul key identification
when searching for asymmetric keys.  The following changes are made:

 (1) Use the previously created asymmetric_key_id struct to hold the following
     key IDs derived from the X.509 certificate or PKCS#7 message:

	id: serial number + issuer
	skid: subjKeyId + subject
	authority: authKeyId + issuer

 (2) Replace the hex fingerprint attached to key->type_data[1] with an
     asymmetric_key_ids struct containing the id and the skid (if present).

 (3) Make the asymmetric_type match data preparse select one of two searches:

     (a) An iterative search for the key ID given if prefixed with "id:".  The
     	 prefix is expected to be followed by a hex string giving the ID to
     	 search for.  The criterion key ID is checked against all key IDs
     	 recorded on the key.

     (b) A direct search if the key ID is not prefixed with "id:".  This will
     	 look for an exact match on the key description.

 (4) Make x509_request_asymmetric_key() take a key ID.  This is then converted
     into "id:<hex>" and passed into keyring_search() where match preparsing
     will turn it back into a binary ID.

 (5) X.509 certificate verification then takes the authority key ID and looks
     up a key that matches it to find the public key for the certificate
     signature.

 (6) PKCS#7 certificate verification then takes the id key ID and looks up a
     key that matches it to find the public key for the signed information
     block signature.

Additional changes:

 (1) Multiple subjKeyId and authKeyId values on an X.509 certificate cause the
     cert to be rejected with -EBADMSG.

 (2) The 'fingerprint' ID is gone.  This was primarily intended to convey PGP
     public key fingerprints.  If PGP is supported in future, this should
     generate a key ID that carries the fingerprint.

 (3) Th ca_keyid= kernel command line option is now converted to a key ID and
     used to match the authority key ID.  Possibly this should only match the
     actual authKeyId part and not the issuer as well.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/asymmetric_keys.h  |    4 -
 crypto/asymmetric_keys/asymmetric_type.c  |  133 ++++++++++++-----------------
 crypto/asymmetric_keys/pkcs7_parser.c     |   38 ++++++--
 crypto/asymmetric_keys/pkcs7_parser.h     |    5 -
 crypto/asymmetric_keys/pkcs7_trust.c      |    6 -
 crypto/asymmetric_keys/pkcs7_verify.c     |   44 ++++------
 crypto/asymmetric_keys/x509_cert_parser.c |   55 +++++++-----
 crypto/asymmetric_keys/x509_parser.h      |    5 +
 crypto/asymmetric_keys/x509_public_key.c  |   89 +++++++++++--------
 include/crypto/public_key.h               |    5 +
 10 files changed, 198 insertions(+), 186 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
index 917be6b985e7..fd21ac28e0a0 100644
--- a/crypto/asymmetric_keys/asymmetric_keys.h
+++ b/crypto/asymmetric_keys/asymmetric_keys.h
@@ -9,13 +9,13 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
-int asymmetric_keyid_match(const char *kid, const char *id);
 extern bool asymmetric_match_key_ids(const struct asymmetric_key_ids *kids,
 				     const struct asymmetric_key_id *match_id);
 
 extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
 
-static inline const char *asymmetric_key_id(const struct key *key)
+static inline
+const struct asymmetric_key_ids *asymmetric_key_ids(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 92bfc438dd1d..718e779a010e 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -112,76 +112,15 @@ struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
 }
 
 /*
- * 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;
-}
-EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
-
-/*
- * Match asymmetric keys on (part of) their name
- * We have some shorthand methods for matching keys.  We allow:
- *
- *	"<desc>"	- request a key by description
- *	"id:<id>"	- request a key matching the ID
- *	"<subtype>:<id>" - request a key of a subtype
+ * Match asymmetric keys by ID.
  */
 static bool asymmetric_key_cmp(const struct key *key,
 			       const struct key_match_data *match_data)
 {
-	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
-	const char *description = match_data->raw_data;
-	const char *spec = description;
-	const char *id;
-	ptrdiff_t speclen;
-
-	if (!subtype || !spec || !*spec)
-		return 0;
-
-	/* See if the full key description matches as is */
-	if (key->description && strcmp(key->description, description) == 0)
-		return 1;
-
-	/* All tests from here on break the criterion description into a
-	 * specifier, a colon and then an identifier.
-	 */
-	id = strchr(spec, ':');
-	if (!id)
-		return 0;
-
-	speclen = id - spec;
-	id++;
-
-	if (speclen == 2 && memcmp(spec, "id", 2) == 0)
-		return asymmetric_keyid_match(asymmetric_key_id(key), id);
+	const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
+	const struct asymmetric_key_id *match_id = match_data->preparsed;
 
-	if (speclen == subtype->name_len &&
-	    memcmp(spec, subtype->name, speclen) == 0)
-		return 1;
-
-	return 0;
+	return asymmetric_match_key_ids(kids, match_id);
 }
 
 /*
@@ -198,8 +137,30 @@ static bool asymmetric_key_cmp(const struct key *key,
  */
 static int asymmetric_key_match_preparse(struct key_match_data *match_data)
 {
-	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	struct asymmetric_key_id *match_id;
+	const char *spec = match_data->raw_data;
+	const char *id;
+
+	if (!spec || !*spec)
+		return -EINVAL;
+	if (spec[0] == 'i' &&
+	    spec[1] == 'd' &&
+	    spec[2] == ':') {
+		id = spec + 3;
+	} else {
+		goto default_match;
+	}
+
+	match_id = asymmetric_key_hex_to_key_id(id);
+	if (!match_id)
+		return -ENOMEM;
+
+	match_data->preparsed = match_id;
 	match_data->cmp = asymmetric_key_cmp;
+	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	return 0;
+
+default_match:
 	return 0;
 }
 
@@ -208,6 +169,7 @@ static int asymmetric_key_match_preparse(struct key_match_data *match_data)
  */
 static void asymmetric_key_match_free(struct key_match_data *match_data)
 {
+	kfree(match_data->preparsed);
 }
 
 /*
@@ -216,8 +178,10 @@ static void asymmetric_key_match_free(struct key_match_data *match_data)
 static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
 {
 	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
-	const char *kid = asymmetric_key_id(key);
-	size_t n;
+	const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
+	const struct asymmetric_key_id *kid;
+	const unsigned char *p;
+	int n;
 
 	seq_puts(m, key->description);
 
@@ -225,13 +189,16 @@ static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
 		seq_puts(m, ": ");
 		subtype->describe(key, m);
 
-		if (kid) {
+		if (kids && kids->id[0]) {
+			kid = kids->id[0];
 			seq_putc(m, ' ');
-			n = strlen(kid);
-			if (n <= 8)
-				seq_puts(m, kid);
-			else
-				seq_puts(m, kid + n - 8);
+			n = kid->len;
+			p = kid->data;
+			if (n > 8) {
+				p += n - 8;
+				n = 8;
+			}
+			seq_printf(m, "%*phN", n, p);
 		}
 
 		seq_puts(m, " [");
@@ -282,6 +249,7 @@ static int asymmetric_key_preparse(struct key_preparsed_payload *prep)
 static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
 {
 	struct asymmetric_key_subtype *subtype = prep->type_data[0];
+	struct asymmetric_key_ids *kids = prep->type_data[1];
 
 	pr_devel("==>%s()\n", __func__);
 
@@ -289,7 +257,11 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
 		subtype->destroy(prep->payload[0]);
 		module_put(subtype->owner);
 	}
-	kfree(prep->type_data[1]);
+	if (kids) {
+		kfree(kids->id[0]);
+		kfree(kids->id[1]);
+		kfree(kids);
+	}
 	kfree(prep->description);
 }
 
@@ -299,13 +271,20 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
 static void asymmetric_key_destroy(struct key *key)
 {
 	struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
+	struct asymmetric_key_ids *kids = key->type_data.p[1];
+
 	if (subtype) {
 		subtype->destroy(key->payload.data);
 		module_put(subtype->owner);
 		key->type_data.p[0] = NULL;
 	}
-	kfree(key->type_data.p[1]);
-	key->type_data.p[1] = NULL;
+
+	if (kids) {
+		kfree(kids->id[0]);
+		kfree(kids->id[1]);
+		kfree(kids);
+		key->type_data.p[1] = NULL;
+	}
 }
 
 struct key_type key_type_asymmetric = {
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 1e9861da7ee4..3bd5a1e4c493 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -29,6 +29,10 @@ struct pkcs7_parse_context {
 	enum OID	last_oid;		/* Last OID encountered */
 	unsigned	x509_index;
 	unsigned	sinfo_index;
+	const void	*raw_serial;
+	unsigned	raw_serial_size;
+	unsigned	raw_issuer_size;
+	const void	*raw_issuer;
 };
 
 /*
@@ -39,6 +43,7 @@ static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo)
 	if (sinfo) {
 		mpi_free(sinfo->sig.mpi[0]);
 		kfree(sinfo->sig.digest);
+		kfree(sinfo->signing_cert_id);
 		kfree(sinfo);
 	}
 }
@@ -251,10 +256,10 @@ int pkcs7_extract_cert(void *context, size_t hdrlen,
 	if (IS_ERR(x509))
 		return PTR_ERR(x509);
 
-	pr_debug("Got cert for %s\n", x509->subject);
-	pr_debug("- fingerprint %s\n", x509->fingerprint);
-
 	x509->index = ++ctx->x509_index;
+	pr_debug("Got cert %u for %s\n", x509->index, x509->subject);
+	pr_debug("- fingerprint %*phN\n", x509->id->len, x509->id->data);
+
 	*ctx->ppcerts = x509;
 	ctx->ppcerts = &x509->next;
 	return 0;
@@ -343,8 +348,8 @@ int pkcs7_sig_note_serial(void *context, size_t hdrlen,
 			  const void *value, size_t vlen)
 {
 	struct pkcs7_parse_context *ctx = context;
-	ctx->sinfo->raw_serial = value;
-	ctx->sinfo->raw_serial_size = vlen;
+	ctx->raw_serial = value;
+	ctx->raw_serial_size = vlen;
 	return 0;
 }
 
@@ -356,8 +361,8 @@ int pkcs7_sig_note_issuer(void *context, size_t hdrlen,
 			  const void *value, size_t vlen)
 {
 	struct pkcs7_parse_context *ctx = context;
-	ctx->sinfo->raw_issuer = value;
-	ctx->sinfo->raw_issuer_size = vlen;
+	ctx->raw_issuer = value;
+	ctx->raw_issuer_size = vlen;
 	return 0;
 }
 
@@ -390,10 +395,21 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
 			   const void *value, size_t vlen)
 {
 	struct pkcs7_parse_context *ctx = context;
-
-	ctx->sinfo->index = ++ctx->sinfo_index;
-	*ctx->ppsinfo = ctx->sinfo;
-	ctx->ppsinfo = &ctx->sinfo->next;
+	struct pkcs7_signed_info *sinfo = ctx->sinfo;
+	struct asymmetric_key_id *kid;
+
+	/* Generate cert issuer + serial number key ID */
+	kid = asymmetric_key_generate_id(ctx->raw_serial,
+					 ctx->raw_serial_size,
+					 ctx->raw_issuer,
+					 ctx->raw_issuer_size);
+	if (IS_ERR(kid))
+		return PTR_ERR(kid);
+
+	sinfo->signing_cert_id = kid;
+	sinfo->index = ++ctx->sinfo_index;
+	*ctx->ppsinfo = sinfo;
+	ctx->ppsinfo = &sinfo->next;
 	ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
 	if (!ctx->sinfo)
 		return -ENOMEM;
diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
index d25f4d15370f..91949f92bc72 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -33,10 +33,7 @@ struct pkcs7_signed_info {
 	const void	*authattrs;
 
 	/* Issuing cert serial number and issuer's name */
-	const void	*raw_serial;
-	unsigned	raw_serial_size;
-	unsigned	raw_issuer_size;
-	const void	*raw_issuer;
+	struct asymmetric_key_id *signing_cert_id;
 
 	/* Message signature.
 	 *
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index fad888ea4fad..09197e50fa82 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -49,8 +49,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 		/* Look to see if this certificate is present in the trusted
 		 * keys.
 		 */
-		key = x509_request_asymmetric_key(trust_keyring, x509->subject,
-						  x509->fingerprint);
+		key = x509_request_asymmetric_key(trust_keyring, x509->id);
 		if (!IS_ERR(key))
 			/* One of the X.509 certificates in the PKCS#7 message
 			 * is apparently the same as one we already trust.
@@ -82,8 +81,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 		return -ENOKEY;
 	}
 
-	key = x509_request_asymmetric_key(trust_keyring, last->issuer,
-					  last->authority);
+	key = x509_request_asymmetric_key(trust_keyring, last->authority);
 	if (IS_ERR(key))
 		return PTR_ERR(key) == -ENOMEM ? -ENOMEM : -ENOKEY;
 	x509 = last;
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index c62cf8006e1f..57e90fa17f2b 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -131,8 +131,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
 	struct x509_certificate *x509;
 	unsigned certix = 1;
 
-	kenter("%u,%u,%u",
-	       sinfo->index, sinfo->raw_serial_size, sinfo->raw_issuer_size);
+	kenter("%u", sinfo->index);
 
 	for (x509 = pkcs7->certs; x509; x509 = x509->next, certix++) {
 		/* I'm _assuming_ that the generator of the PKCS#7 message will
@@ -140,21 +139,11 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
 		 * PKCS#7 message - but I can't be 100% sure of that.  It's
 		 * possible this will need element-by-element comparison.
 		 */
-		if (x509->raw_serial_size != sinfo->raw_serial_size ||
-		    memcmp(x509->raw_serial, sinfo->raw_serial,
-			   sinfo->raw_serial_size) != 0)
+		if (!asymmetric_key_id_same(x509->id, sinfo->signing_cert_id))
 			continue;
 		pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
 			 sinfo->index, certix);
 
-		if (x509->raw_issuer_size != sinfo->raw_issuer_size ||
-		    memcmp(x509->raw_issuer, sinfo->raw_issuer,
-			   sinfo->raw_issuer_size) != 0) {
-			pr_warn("Sig %u: X.509 subject and PKCS#7 issuer don't match\n",
-				sinfo->index);
-			continue;
-		}
-
 		if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
 			pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
 				sinfo->index);
@@ -164,8 +153,10 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
 		sinfo->signer = x509;
 		return 0;
 	}
+
 	pr_warn("Sig %u: Issuing X.509 cert not found (#%*ph)\n",
-		sinfo->index, sinfo->raw_serial_size, sinfo->raw_serial);
+		sinfo->index,
+		sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
 	return -ENOKEY;
 }
 
@@ -184,7 +175,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		p->seen = false;
 
 	for (;;) {
-		pr_debug("verify %s: %s\n", x509->subject, x509->fingerprint);
+		pr_debug("verify %s: %*phN\n",
+			 x509->subject,
+			 x509->raw_serial_size, x509->raw_serial);
 		x509->seen = true;
 		ret = x509_get_sig_params(x509);
 		if (ret < 0)
@@ -192,7 +185,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 
 		pr_debug("- issuer %s\n", x509->issuer);
 		if (x509->authority)
-			pr_debug("- authkeyid %s\n", x509->authority);
+			pr_debug("- authkeyid %*phN\n",
+				 x509->authority->len, x509->authority->data);
 
 		if (!x509->authority ||
 		    strcmp(x509->subject, x509->issuer) == 0) {
@@ -218,13 +212,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		/* Look through the X.509 certificates in the PKCS#7 message's
 		 * list to see if the next one is there.
 		 */
-		pr_debug("- want %s\n", x509->authority);
+		pr_debug("- want %*phN\n",
+			 x509->authority->len, x509->authority->data);
 		for (p = pkcs7->certs; p; p = p->next) {
-			pr_debug("- cmp [%u] %s\n", p->index, p->fingerprint);
-			if (p->raw_subject_size == x509->raw_issuer_size &&
-			    strcmp(p->fingerprint, x509->authority) == 0 &&
-			    memcmp(p->raw_subject, x509->raw_issuer,
-				   x509->raw_issuer_size) == 0)
+			if (!p->skid)
+				continue;
+			pr_debug("- cmp [%u] %*phN\n",
+				 p->index, p->skid->len, p->skid->data);
+			if (asymmetric_key_id_same(p->skid, x509->authority))
 				goto found_issuer;
 		}
 
@@ -233,7 +228,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		return 0;
 
 	found_issuer:
-		pr_debug("- issuer %s\n", p->subject);
+		pr_debug("- subject %s\n", p->subject);
 		if (p->seen) {
 			pr_warn("Sig %u: X.509 chain contains loop\n",
 				sinfo->index);
@@ -304,7 +299,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
 		ret = x509_get_sig_params(x509);
 		if (ret < 0)
 			return ret;
-		pr_debug("X.509[%u] %s\n", n, x509->authority);
+		pr_debug("X.509[%u] %*phN\n",
+			 n, x509->authority->len, x509->authority->data);
 	}
 
 	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index ac72348c186a..96151b2b91a2 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -46,7 +46,8 @@ void x509_free_certificate(struct x509_certificate *cert)
 		public_key_destroy(cert->pub);
 		kfree(cert->issuer);
 		kfree(cert->subject);
-		kfree(cert->fingerprint);
+		kfree(cert->id);
+		kfree(cert->skid);
 		kfree(cert->authority);
 		kfree(cert->sig.digest);
 		mpi_free(cert->sig.rsa.s);
@@ -62,6 +63,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 {
 	struct x509_certificate *cert;
 	struct x509_parse_context *ctx;
+	struct asymmetric_key_id *kid;
 	long ret;
 
 	ret = -ENOMEM;
@@ -89,6 +91,17 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 	if (ret < 0)
 		goto error_decode;
 
+	/* Generate cert issuer + serial number key ID */
+	kid = asymmetric_key_generate_id(cert->raw_serial,
+					 cert->raw_serial_size,
+					 cert->raw_issuer,
+					 cert->raw_issuer_size);
+	if (IS_ERR(kid)) {
+		ret = PTR_ERR(kid);
+		goto error_decode;
+	}
+	cert->id = kid;
+
 	kfree(ctx);
 	return cert;
 
@@ -407,36 +420,34 @@ int x509_process_extension(void *context, size_t hdrlen,
 			   const void *value, size_t vlen)
 {
 	struct x509_parse_context *ctx = context;
+	struct asymmetric_key_id *kid;
 	const unsigned char *v = value;
-	char *f;
 	int i;
 
 	pr_debug("Extension: %u\n", ctx->last_oid);
 
 	if (ctx->last_oid == OID_subjectKeyIdentifier) {
 		/* Get hold of the key fingerprint */
-		if (vlen < 3)
+		if (ctx->cert->skid || vlen < 3)
 			return -EBADMSG;
 		if (v[0] != ASN1_OTS || v[1] != vlen - 2)
 			return -EBADMSG;
 		v += 2;
 		vlen -= 2;
 
-		f = kmalloc(vlen * 2 + 1, GFP_KERNEL);
-		if (!f)
-			return -ENOMEM;
-		for (i = 0; i < vlen; i++)
-			sprintf(f + i * 2, "%02x", v[i]);
-		pr_debug("fingerprint %s\n", f);
-		ctx->cert->fingerprint = f;
+		kid = asymmetric_key_generate_id(v, vlen,
+						 ctx->cert->raw_subject,
+						 ctx->cert->raw_subject_size);
+		if (IS_ERR(kid))
+			return PTR_ERR(kid);
+		ctx->cert->skid = kid;
+		pr_debug("subjkeyid %*phN\n", kid->len, kid->data);
 		return 0;
 	}
 
 	if (ctx->last_oid == OID_authorityKeyIdentifier) {
-		size_t key_len;
-
 		/* Get hold of the CA key fingerprint */
-		if (vlen < 5)
+		if (ctx->cert->authority || vlen < 5)
 			return -EBADMSG;
 
 		/* Authority Key Identifier must be a Constructed SEQUENCE */
@@ -454,7 +465,7 @@ int x509_process_extension(void *context, size_t hdrlen,
 			    v[3] > vlen - 4)
 				return -EBADMSG;
 
-			key_len = v[3];
+			vlen = v[3];
 			v += 4;
 		} else {
 			/* Long Form length */
@@ -476,17 +487,17 @@ int x509_process_extension(void *context, size_t hdrlen,
 			    v[sub + 1] > vlen - 4 - sub)
 				return -EBADMSG;
 
-			key_len = v[sub + 1];
+			vlen = v[sub + 1];
 			v += (sub + 2);
 		}
 
-		f = kmalloc(key_len * 2 + 1, GFP_KERNEL);
-		if (!f)
-			return -ENOMEM;
-		for (i = 0; i < key_len; i++)
-			sprintf(f + i * 2, "%02x", v[i]);
-		pr_debug("authority   %s\n", f);
-		ctx->cert->authority = f;
+		kid = asymmetric_key_generate_id(v, vlen,
+						 ctx->cert->raw_issuer,
+						 ctx->cert->raw_issuer_size);
+		if (IS_ERR(kid))
+			return PTR_ERR(kid);
+		pr_debug("authkeyid %*phN\n", kid->len, kid->data);
+		ctx->cert->authority = kid;
 		return 0;
 	}
 
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 1b76f207c1f3..0e8d59b010fb 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -19,8 +19,9 @@ struct x509_certificate {
 	struct public_key_signature sig;	/* Signature parameters */
 	char		*issuer;		/* Name of certificate issuer */
 	char		*subject;		/* Name of certificate subject */
-	char		*fingerprint;		/* Key fingerprint as hex */
-	char		*authority;		/* Authority key fingerprint as hex */
+	struct asymmetric_key_id *id;		/* Issuer + serial number */
+	struct asymmetric_key_id *skid;		/* Subject key identifier */
+	struct asymmetric_key_id *authority;	/* Authority key identifier */
 	struct tm	valid_from;
 	struct tm	valid_to;
 	const void	*tbs;			/* Signed data */
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index f3d62307e6ee..c60905c3f4d2 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -25,7 +25,7 @@
 #include "x509_parser.h"
 
 static bool use_builtin_keys;
-static char *ca_keyid;
+static struct asymmetric_key_id *ca_keyid;
 
 #ifndef MODULE
 static int __init ca_keys_setup(char *str)
@@ -33,10 +33,16 @@ static int __init ca_keys_setup(char *str)
 	if (!str)		/* default system keyring */
 		return 1;
 
-	if (strncmp(str, "id:", 3) == 0)
-		ca_keyid = str;	/* owner key 'id:xxxxxx' */
-	else if (strcmp(str, "builtin") == 0)
+	if (strncmp(str, "id:", 3) == 0) {
+		struct asymmetric_key_id *p;
+		p = asymmetric_key_hex_to_key_id(str);
+		if (p == ERR_PTR(-EINVAL))
+			pr_err("Unparsable hex string in ca_keys\n");
+		else if (!IS_ERR(p))
+			ca_keyid = p;	/* owner key 'id:xxxxxx' */
+	} else if (strcmp(str, "builtin") == 0) {
 		use_builtin_keys = true;
+	}
 
 	return 1;
 }
@@ -46,31 +52,28 @@ __setup("ca_keys=", ca_keys_setup);
 /**
  * x509_request_asymmetric_key - Request a key by X.509 certificate params.
  * @keyring: The keys to search.
- * @subject: The name of the subject to whom the key belongs.
- * @key_id: The subject key ID as a hex string.
+ * @kid: The key ID.
  *
  * Find a key in the given keyring by subject name and key ID.  These might,
  * for instance, be the issuer name and the authority key ID of an X.509
  * certificate that needs to be verified.
  */
 struct key *x509_request_asymmetric_key(struct key *keyring,
-					const char *subject,
-					const char *key_id)
+					const struct asymmetric_key_id *kid)
 {
 	key_ref_t key;
-	size_t subject_len = strlen(subject), key_id_len = strlen(key_id);
-	char *id;
+	char *id, *p;
 
-	/* Construct an identifier "<subjname>:<keyid>". */
-	id = kmalloc(subject_len + 2 + key_id_len + 1, GFP_KERNEL);
+	/* Construct an identifier "id:<keyid>". */
+	p = id = kmalloc(2 + 1 + kid->len * 2 + 1, GFP_KERNEL);
 	if (!id)
 		return ERR_PTR(-ENOMEM);
 
-	memcpy(id, subject, subject_len);
-	id[subject_len + 0] = ':';
-	id[subject_len + 1] = ' ';
-	memcpy(id + subject_len + 2, key_id, key_id_len);
-	id[subject_len + 2 + key_id_len] = 0;
+	*p++ = 'i';
+	*p++ = 'd';
+	*p++ = ':';
+	p = bin2hex(p, kid->data, kid->len);
+	*p = 0;
 
 	pr_debug("Look up: \"%s\"\n", id);
 
@@ -195,11 +198,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
 	if (!trust_keyring)
 		return -EOPNOTSUPP;
 
-	if (ca_keyid && !asymmetric_keyid_match(cert->authority, ca_keyid))
+	if (ca_keyid && !asymmetric_key_id_same(cert->authority, ca_keyid))
 		return -EPERM;
 
-	key = x509_request_asymmetric_key(trust_keyring,
-					  cert->issuer, cert->authority);
+	key = x509_request_asymmetric_key(trust_keyring, cert->authority);
 	if (!IS_ERR(key))  {
 		if (!use_builtin_keys
 		    || test_bit(KEY_FLAG_BUILTIN, &key->flags))
@@ -214,9 +216,11 @@ static int x509_validate_trust(struct x509_certificate *cert,
  */
 static int x509_key_preparse(struct key_preparsed_payload *prep)
 {
+	struct asymmetric_key_ids *kids;
 	struct x509_certificate *cert;
+	const char *q;
 	size_t srlen, sulen;
-	char *desc = NULL;
+	char *desc = NULL, *p;
 	int ret;
 
 	cert = x509_cert_parse(prep->data, prep->datalen);
@@ -249,19 +253,12 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 		 pkey_algo_name[cert->sig.pkey_algo],
 		 hash_algo_name[cert->sig.pkey_hash_algo]);
 
-	if (!cert->fingerprint) {
-		pr_warn("Cert for '%s' must have a SubjKeyId extension\n",
-			cert->subject);
-		ret = -EKEYREJECTED;
-		goto error_free_cert;
-	}
-
 	cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
 	cert->pub->id_type = PKEY_ID_X509;
 
 	/* Check the signature on the key if it appears to be self-signed */
 	if (!cert->authority ||
-	    strcmp(cert->fingerprint, cert->authority) == 0) {
+	    asymmetric_key_id_same(cert->skid, cert->authority)) {
 		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;
@@ -273,31 +270,47 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 
 	/* Propose a description */
 	sulen = strlen(cert->subject);
-	srlen = strlen(cert->fingerprint);
+	srlen = cert->raw_serial_size;
+	q = cert->raw_serial;
+	if (srlen > 1 && *q == 0) {
+		srlen--;
+		q++;
+	}
+
 	ret = -ENOMEM;
-	desc = kmalloc(sulen + 2 + srlen + 1, GFP_KERNEL);
+	desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
 	if (!desc)
 		goto error_free_cert;
-	memcpy(desc, cert->subject, sulen);
-	desc[sulen] = ':';
-	desc[sulen + 1] = ' ';
-	memcpy(desc + sulen + 2, cert->fingerprint, srlen);
-	desc[sulen + 2 + srlen] = 0;
+	p = memcpy(desc, cert->subject, sulen);
+	p += sulen;
+	*p++ = ':';
+	*p++ = ' ';
+	p = bin2hex(p, q, srlen);
+	*p = 0;
+
+	kids = kmalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
+	if (!kids)
+		goto error_free_desc;
+	kids->id[0] = cert->id;
+	kids->id[1] = cert->skid;
 
 	/* We're pinning the module by being linked against it */
 	__module_get(public_key_subtype.owner);
 	prep->type_data[0] = &public_key_subtype;
-	prep->type_data[1] = cert->fingerprint;
+	prep->type_data[1] = kids;
 	prep->payload[0] = cert->pub;
 	prep->description = desc;
 	prep->quotalen = 100;
 
 	/* We've finished with the certificate */
 	cert->pub = NULL;
-	cert->fingerprint = NULL;
+	cert->id = NULL;
+	cert->skid = NULL;
 	desc = NULL;
 	ret = 0;
 
+error_free_desc:
+	kfree(desc);
 error_free_cert:
 	x509_free_certificate(cert);
 	return ret;
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 0d164c6af539..fa73a6fd536c 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -15,6 +15,7 @@
 #define _LINUX_PUBLIC_KEY_H
 
 #include <linux/mpi.h>
+#include <keys/asymmetric-type.h>
 #include <crypto/hash_info.h>
 
 enum pkey_algo {
@@ -98,8 +99,8 @@ struct key;
 extern int verify_signature(const struct key *key,
 			    const struct public_key_signature *sig);
 
+struct asymmetric_key_id;
 extern struct key *x509_request_asymmetric_key(struct key *keyring,
-					       const char *issuer,
-					       const char *key_id);
+					       const struct asymmetric_key_id *kid);
 
 #endif /* _LINUX_PUBLIC_KEY_H */


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/10] PKCS#7: Better handling of unsupported crypto [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (7 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 08/10] KEYS: Overhaul key identification when searching for asymmetric keys " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 20:45 ` [PATCH 10/10] PKCS#7: Handle PKCS#7 messages that contain no X.509 certs " David Howells
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

Provide better handling of unsupported crypto when verifying a PKCS#7 message.
If we can't bridge the gap between a pair of X.509 certs or between a signed
info block and an X.509 cert because it involves some crypto we don't support,
that's not necessarily the end of the world as there may be other ways points
at which we can intersect with a ring of trusted keys.

Instead, only produce ENOPKG immediately if all the signed info blocks in a
PKCS#7 message require unsupported crypto to bridge to the first X.509 cert.
Otherwise, we defer the generation of ENOPKG until we get ENOKEY during trust
validation.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/pkcs7_parser.h    |    1 +
 crypto/asymmetric_keys/pkcs7_trust.c     |   30 ++++++++++++--------
 crypto/asymmetric_keys/pkcs7_verify.c    |   46 +++++++++++++++++++++++++++---
 crypto/asymmetric_keys/x509_parser.h     |    1 +
 crypto/asymmetric_keys/x509_public_key.c |   13 +++++++-
 5 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
index 91949f92bc72..efc7dc9b8f9c 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -23,6 +23,7 @@ struct pkcs7_signed_info {
 	struct x509_certificate *signer; /* Signing certificate (in msg->certs) */
 	unsigned index;
 	bool trusted;
+	bool unsupported_crypto;	/* T if not usable due to missing crypto */
 
 	/* Message digest - the digest of the Content Data (or NULL) */
 	const void	*msgdigest;
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 09197e50fa82..2f953158a1a4 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -35,6 +35,11 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 
 	kenter(",%u,", sinfo->index);
 
+	if (sinfo->unsupported_crypto) {
+		kleave(" = -ENOPKG [cached]");
+		return -ENOPKG;
+	}
+
 	for (x509 = sinfo->signer; x509; x509 = x509->signer) {
 		if (x509->seen) {
 			if (x509->verified) {
@@ -139,24 +144,27 @@ int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
 {
 	struct pkcs7_signed_info *sinfo;
 	struct x509_certificate *p;
-	int cached_ret = 0, ret;
+	int cached_ret = -ENOKEY;
+	int ret;
 
 	for (p = pkcs7->certs; p; p = p->next)
 		p->seen = false;
 
 	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
-		ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring);
-		if (ret < 0) {
-			if (ret == -ENOPKG) {
+		switch (pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring)) {
+		case -ENOKEY:
+			continue;
+		case -ENOPKG:
+			if (cached_ret == -ENOKEY)
 				cached_ret = -ENOPKG;
-			} else if (ret == -ENOKEY) {
-				if (cached_ret == 0)
-					cached_ret = -ENOKEY;
-			} else {
-				return ret;
-			}
+			continue;
+		case 0:
+			*_trusted |= sinfo->trusted;
+			cached_ret = 0;
+			continue;
+		default:
+			return ret;
 		}
-		*_trusted |= sinfo->trusted;
 	}
 
 	return cached_ret;
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 57e90fa17f2b..bd264052f751 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -181,7 +181,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		x509->seen = true;
 		ret = x509_get_sig_params(x509);
 		if (ret < 0)
-			return ret;
+			goto maybe_missing_crypto_in_x509;
 
 		pr_debug("- issuer %s\n", x509->issuer);
 		if (x509->authority)
@@ -203,7 +203,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 
 			ret = x509_check_signature(x509->pub, x509);
 			if (ret < 0)
-				return ret;
+				goto maybe_missing_crypto_in_x509;
 			x509->signer = x509;
 			pr_debug("- self-signed\n");
 			return 0;
@@ -245,6 +245,17 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		x509 = p;
 		might_sleep();
 	}
+
+maybe_missing_crypto_in_x509:
+	/* Just prune the certificate chain at this point if we lack some
+	 * crypto module to go further.  Note, however, we don't want to set
+	 * sinfo->missing_crypto as the signed info block may still be
+	 * validatable against an X.509 cert lower in the chain that we have a
+	 * trusted copy of.
+	 */
+	if (ret == -ENOPKG)
+		return 0;
+	return ret;
 }
 
 /*
@@ -286,11 +297,33 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
 /**
  * pkcs7_verify - Verify a PKCS#7 message
  * @pkcs7: The PKCS#7 message to be verified
+ *
+ * Verify a PKCS#7 message is internally consistent - that is, the data digest
+ * matches the digest in the AuthAttrs and any signature in the message or one
+ * of the X.509 certificates it carries that matches another X.509 cert in the
+ * message can be verified.
+ *
+ * This does not look to match the contents of the PKCS#7 message against any
+ * external public keys.
+ *
+ * Returns, in order of descending priority:
+ *
+ *  (*) -EKEYREJECTED if a signature failed to match for which we found an
+ *	appropriate X.509 certificate, or:
+ *
+ *  (*) -EBADMSG if some part of the message was invalid, or:
+ *
+ *  (*) -ENOPKG if none of the signature chains are verifiable because suitable
+ *	crypto modules couldn't be found, or:
+ *
+ *  (*) 0 if all the signature chains that don't incur -ENOPKG can be verified
+ *	(note that a signature chain may be of zero length), or:
  */
 int pkcs7_verify(struct pkcs7_message *pkcs7)
 {
 	struct pkcs7_signed_info *sinfo;
 	struct x509_certificate *x509;
+	int enopkg = -ENOPKG;
 	int ret, n;
 
 	kenter("");
@@ -306,12 +339,17 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
 	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
 		ret = pkcs7_verify_one(pkcs7, sinfo);
 		if (ret < 0) {
+			if (ret == -ENOPKG) {
+				sinfo->unsupported_crypto = true;
+				continue;
+			}
 			kleave(" = %d", ret);
 			return ret;
 		}
+		enopkg = 0;
 	}
 
-	kleave(" = 0");
-	return 0;
+	kleave(" = %d", enopkg);
+	return enopkg;
 }
 EXPORT_SYMBOL_GPL(pkcs7_verify);
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 0e8d59b010fb..4e1a384901ed 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -38,6 +38,7 @@ struct x509_certificate {
 	bool		seen;			/* Infinite recursion prevention */
 	bool		verified;
 	bool		trusted;
+	bool		unsupported_crypto;	/* T if can't be verified due to missing crypto */
 };
 
 /*
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index c60905c3f4d2..1d9a4c555376 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -115,6 +115,8 @@ int x509_get_sig_params(struct x509_certificate *cert)
 
 	pr_devel("==>%s()\n", __func__);
 
+	if (cert->unsupported_crypto)
+		return -ENOPKG;
 	if (cert->sig.rsa.s)
 		return 0;
 
@@ -127,8 +129,13 @@ int x509_get_sig_params(struct x509_certificate *cert)
 	 * big the hash operational data will be.
 	 */
 	tfm = crypto_alloc_shash(hash_algo_name[cert->sig.pkey_hash_algo], 0, 0);
-	if (IS_ERR(tfm))
-		return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
+	if (IS_ERR(tfm)) {
+		if (PTR_ERR(tfm) == -ENOENT) {
+			cert->unsupported_crypto = true;
+			return -ENOPKG;
+		}
+		return PTR_ERR(tfm);
+	}
 
 	desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
 	digest_size = crypto_shash_digestsize(tfm);
@@ -175,6 +182,8 @@ int x509_check_signature(const struct public_key *pub,
 		return ret;
 
 	ret = public_key_verify_signature(pub, &cert->sig);
+	if (ret == -ENOPKG)
+		cert->unsupported_crypto = true;
 	pr_debug("Cert Verification: %d\n", ret);
 	return ret;
 }


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/10] PKCS#7: Handle PKCS#7 messages that contain no X.509 certs [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (8 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 09/10] PKCS#7: Better handling of unsupported crypto " David Howells
@ 2014-09-15 20:45 ` David Howells
  2014-09-15 22:30 ` [PATCH 09/10] PKCS#7: Better handling of unsupported crypto " David Howells
  2014-09-21 23:32 ` [PATCH 11/10] Check hex2bin()'s return when generating an asymmetric key ID David Howells
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 20:45 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

The X.509 certificate list in a PKCS#7 message is optional.  To save space, we
can omit the inclusion of any X.509 certificates if we are sure that we can
look the relevant public key up by the serial number and issuer given in a
signed info block.

This also supports use of a signed info block for which we can't find a
matching X.509 cert in the certificate list, though it be populated.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/pkcs7_trust.c  |   48 +++++++++++++++++++++++++--------
 crypto/asymmetric_keys/pkcs7_verify.c |   16 ++++++++---
 2 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 2f953158a1a4..6fdbc3a33d66 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -55,13 +55,16 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 		 * keys.
 		 */
 		key = x509_request_asymmetric_key(trust_keyring, x509->id);
-		if (!IS_ERR(key))
+		if (!IS_ERR(key)) {
 			/* One of the X.509 certificates in the PKCS#7 message
 			 * is apparently the same as one we already trust.
 			 * Verify that the trusted variant can also validate
 			 * the signature on the descendant.
 			 */
+			pr_devel("sinfo %u: Cert %u as key %x\n",
+				 sinfo->index, x509->index, key_serial(key));
 			goto matched;
+		}
 		if (key == ERR_PTR(-ENOMEM))
 			return -ENOMEM;
 
@@ -81,15 +84,34 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 	/* No match - see if the root certificate has a signer amongst the
 	 * trusted keys.
 	 */
-	if (!last || !last->issuer || !last->authority) {
-		kleave(" = -ENOKEY [no backref]");
-		return -ENOKEY;
+	if (last && last->authority) {
+		key = x509_request_asymmetric_key(trust_keyring, last->authority);
+		if (!IS_ERR(key)) {
+			x509 = last;
+			pr_devel("sinfo %u: Root cert %u signer is key %x\n",
+				 sinfo->index, x509->index, key_serial(key));
+			goto matched;
+		}
+		if (PTR_ERR(key) != -ENOKEY)
+			return PTR_ERR(key);
 	}
 
-	key = x509_request_asymmetric_key(trust_keyring, last->authority);
-	if (IS_ERR(key))
-		return PTR_ERR(key) == -ENOMEM ? -ENOMEM : -ENOKEY;
-	x509 = last;
+	/* As a last resort, see if we have a trusted public key that matches
+	 * the signed info directly.
+	 */
+	key = x509_request_asymmetric_key(trust_keyring,
+					  sinfo->signing_cert_id);
+	if (!IS_ERR(key)) {
+		pr_devel("sinfo %u: Direct signer is key %x\n",
+			 sinfo->index, key_serial(key));
+		x509 = NULL;
+		goto matched;
+	}
+	if (PTR_ERR(key) != -ENOKEY)
+		return PTR_ERR(key);
+
+	kleave(" = -ENOKEY [no backref]");
+	return -ENOKEY;
 
 matched:
 	ret = verify_signature(key, sig);
@@ -103,10 +125,12 @@ matched:
 	}
 
 verified:
-	x509->verified = true;
-	for (p = sinfo->signer; p != x509; p = p->signer) {
-		p->verified = true;
-		p->trusted = trusted;
+	if (x509) {
+		x509->verified = true;
+		for (p = sinfo->signer; p != x509; p = p->signer) {
+			p->verified = true;
+			p->trusted = trusted;
+		}
 	}
 	sinfo->trusted = trusted;
 	kleave(" = 0");
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index bd264052f751..cd455450b069 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -154,10 +154,13 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
 		return 0;
 	}
 
-	pr_warn("Sig %u: Issuing X.509 cert not found (#%*ph)\n",
-		sinfo->index,
-		sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
-	return -ENOKEY;
+	/* The relevant X.509 cert isn't found here, but it might be found in
+	 * the trust keyring.
+	 */
+	pr_debug("Sig %u: Issuing X.509 cert not found (#%*phN)\n",
+		 sinfo->index,
+		 sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
+	return 0;
 }
 
 /*
@@ -275,11 +278,14 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
 	if (ret < 0)
 		return ret;
 
-	/* Find the key for the signature */
+	/* Find the key for the signature if there is one */
 	ret = pkcs7_find_key(pkcs7, sinfo);
 	if (ret < 0)
 		return ret;
 
+	if (!sinfo->signer)
+		return 0;
+
 	pr_devel("Using X.509[%u] for sig %u\n",
 		 sinfo->signer->index, sinfo->index);
 


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 09/10] PKCS#7: Better handling of unsupported crypto [ver #2]
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (9 preceding siblings ...)
  2014-09-15 20:45 ` [PATCH 10/10] PKCS#7: Handle PKCS#7 messages that contain no X.509 certs " David Howells
@ 2014-09-15 22:30 ` David Howells
  2014-09-21 23:32 ` [PATCH 11/10] Check hex2bin()'s return when generating an asymmetric key ID David Howells
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-15 22:30 UTC (permalink / raw)
  Cc: dhowells, vgoyal, keyrings, linux-kernel

David Howells <dhowells@redhat.com> wrote:

> +		switch (pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring)) {

This should be:

+		ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring);
+		switch (ret) {

David

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 11/10] Check hex2bin()'s return when generating an asymmetric key ID
  2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
                   ` (10 preceding siblings ...)
  2014-09-15 22:30 ` [PATCH 09/10] PKCS#7: Better handling of unsupported crypto " David Howells
@ 2014-09-21 23:32 ` David Howells
  11 siblings, 0 replies; 13+ messages in thread
From: David Howells @ 2014-09-21 23:32 UTC (permalink / raw)
  To: vgoyal; +Cc: dhowells, keyrings, linux-kernel

As it stands, the code to generate an asymmetric key ID prechecks the hex
string it is given whilst determining the length, before it allocates the
buffer for hex2bin() to translate into - which mean that checking the result of
hex2bin() is redundant.

Unfortunately, hex2bin() is marked as __must_check, which means that the
following warning may be generated if the return value isn't checked:

	crypto/asymmetric_keys/asymmetric_type.c: In function
	asymmetric_key_hex_to_key_id:
	crypto/asymmetric_keys/asymmetric_type.c:110: warning: ignoring return
	value of hex2bin, declared with attribute warn_unused_result

The warning can't be avoided by casting the result to void.

Instead, use strlen() to check the length of the string and ignore the fact
that the string might not be entirely valid hex until after the allocation has
been done - in which case we can use the result of hex2bin() for this.

Signed-off-by: David Howells <dhowells@redhat.com>
---
 asymmetric_type.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 718e779a010e..f0f2111d2c66 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -90,15 +90,12 @@ EXPORT_SYMBOL_GPL(asymmetric_match_key_ids);
 struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
 {
 	struct asymmetric_key_id *match_id;
-	const char *p;
-	ptrdiff_t hexlen;
+	size_t hexlen;
+	int ret;
 
 	if (!*id)
 		return ERR_PTR(-EINVAL);
-	for (p = id; *p; p++)
-		if (!isxdigit(*p))
-			return ERR_PTR(-EINVAL);
-	hexlen = p - id;
+	hexlen = strlen(id);
 	if (hexlen & 1)
 		return ERR_PTR(-EINVAL);
 
@@ -107,7 +104,11 @@ struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id)
 	if (!match_id)
 		return ERR_PTR(-ENOMEM);
 	match_id->len = hexlen / 2;
-	(void)hex2bin(match_id->data, id, hexlen / 2);
+	ret = hex2bin(match_id->data, id, hexlen / 2);
+	if (ret < 0) {
+		kfree(match_id);
+		return ERR_PTR(-EINVAL);
+	}
 	return match_id;
 }
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-09-21 23:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-15 20:44 [PATCH 00/10] KEYS: Improve asymmetric key and PKCS#7 handling [ver #2] David Howells
2014-09-15 20:45 ` [PATCH 01/10] Provide a binary to hex conversion function " David Howells
2014-09-15 20:45 ` [PATCH 02/10] KEYS: Preparse match data " David Howells
2014-09-15 20:45 ` [PATCH 03/10] KEYS: Remove key_type::def_lookup_type " David Howells
2014-09-15 20:45 ` [PATCH 04/10] KEYS: Remove key_type::match in favour of overriding default by match_preparse " David Howells
2014-09-15 20:45 ` [PATCH 05/10] KEYS: Make the key matching functions return bool " David Howells
2014-09-15 20:45 ` [PATCH 06/10] KEYS: Update the keyrings documentation for match changes " David Howells
2014-09-15 20:45 ` [PATCH 07/10] KEYS: Implement binary asymmetric key ID handling " David Howells
2014-09-15 20:45 ` [PATCH 08/10] KEYS: Overhaul key identification when searching for asymmetric keys " David Howells
2014-09-15 20:45 ` [PATCH 09/10] PKCS#7: Better handling of unsupported crypto " David Howells
2014-09-15 20:45 ` [PATCH 10/10] PKCS#7: Handle PKCS#7 messages that contain no X.509 certs " David Howells
2014-09-15 22:30 ` [PATCH 09/10] PKCS#7: Better handling of unsupported crypto " David Howells
2014-09-21 23:32 ` [PATCH 11/10] Check hex2bin()'s return when generating an asymmetric key ID David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).