[RFC PATCH 0/7] KEYS: Adjust public key signature handling

[RFC PATCH 0/7] KEYS: Adjust public key signature handling

[David Howells]

These patches do the following:

 (1) Retain a signature in an asymmetric-type key and associate with it the
     identifiers that will match a key that can be used to verify it.

 (2) Differentiate an X.509 cert that cannot be used versus one that cannot
     be verified due to unavailable crypto.  This is noted in the
     structures involved.

 (3) Determination of the self-signedness of an X.509 cert is improved to
     include checks on the subject/issuer names and the key
     algorithm/signature algorithm types.

 (4) Self-signed X.509 certificates are consistency checked early on if the
     appropriate crypto is available.

This set of patches is a prelude to a set that changes how trustworthiness
is determined.

David
---
David Howells (7):
      X.509: Whitespace cleanup
      KEYS: Allow authentication data to be stored in an asymmetric key
      KEYS: Add identifier pointers to public_key_signature struct
      X.509: Retain the key verification data
      PKCS#7: Make the signature a pointer rather than embedding it
      X.509: Extract signature digest and make self-signed cert checks earlier
      There's a bug in the code determining whether a certificate is self-signed


 crypto/asymmetric_keys/asymmetric_type.c  |    7 +
 crypto/asymmetric_keys/pkcs7_parser.c     |   38 ++++--
 crypto/asymmetric_keys/pkcs7_parser.h     |   10 +-
 crypto/asymmetric_keys/pkcs7_trust.c      |   12 +-
 crypto/asymmetric_keys/pkcs7_verify.c     |  107 +++++++----------
 crypto/asymmetric_keys/public_key.c       |   20 ++-
 crypto/asymmetric_keys/signature.c        |   18 +++
 crypto/asymmetric_keys/x509_cert_parser.c |   52 +++++---
 crypto/asymmetric_keys/x509_parser.h      |   11 +-
 crypto/asymmetric_keys/x509_public_key.c  |  182 +++++++++++++++++++----------
 include/crypto/public_key.h               |    6 +
 include/keys/asymmetric-subtype.h         |    2 
 include/keys/asymmetric-type.h            |    7 +
 13 files changed, 281 insertions(+), 191 deletions(-)

[RFC PATCH 1/7] X.509: Whitespace cleanup

[David Howells]

Clean up some whitespace.

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

 crypto/asymmetric_keys/x509_public_key.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 733c046aacc6..2fcf707fb208 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -88,7 +88,7 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
 		lookup = skid->data;
 		len = skid->len;
 	}
-	
+
 	/* Construct an identifier "id:<keyid>". */
 	p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
 	if (!req)
@@ -137,7 +137,7 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
 			goto reject;
 		}
 	}
-	
+
 	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key));
 	return key;
 

[RFC PATCH 2/7] KEYS: Allow authentication data to be stored in an asymmetric key

[David Howells]

Allow authentication data to be stored in an asymmetric key in the 4th
element of the key payload and provide a way for it to be destroyed.

For the public key subtype, this will be a public_key_signature struct.

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

 crypto/asymmetric_keys/asymmetric_type.c  |    7 +++++--
 crypto/asymmetric_keys/public_key.c       |   20 ++++++++++++++------
 crypto/asymmetric_keys/signature.c        |   14 ++++++++++++++
 crypto/asymmetric_keys/x509_cert_parser.c |    2 +-
 include/crypto/public_key.h               |    5 ++++-
 include/keys/asymmetric-subtype.h         |    2 +-
 include/keys/asymmetric-type.h            |    7 ++++---
 7 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 9f2165b27d52..a79d30128821 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -331,7 +331,8 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
 	pr_devel("==>%s()\n", __func__);
 
 	if (subtype) {
-		subtype->destroy(prep->payload.data[asym_crypto]);
+		subtype->destroy(prep->payload.data[asym_crypto],
+				 prep->payload.data[asym_auth]);
 		module_put(subtype->owner);
 	}
 	asymmetric_key_free_kids(kids);
@@ -346,13 +347,15 @@ static void asymmetric_key_destroy(struct key *key)
 	struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
 	struct asymmetric_key_ids *kids = key->payload.data[asym_key_ids];
 	void *data = key->payload.data[asym_crypto];
+	void *auth = key->payload.data[asym_auth];
 
 	key->payload.data[asym_crypto] = NULL;
 	key->payload.data[asym_subtype] = NULL;
 	key->payload.data[asym_key_ids] = NULL;
+	key->payload.data[asym_auth] = NULL;
 
 	if (subtype) {
-		subtype->destroy(data);
+		subtype->destroy(data, auth);
 		module_put(subtype->owner);
 	}
 
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 0f8b264b3961..fd76b5fc3b3a 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -39,15 +39,23 @@ static void public_key_describe(const struct key *asymmetric_key,
 /*
  * Destroy a public key algorithm key.
  */
-void public_key_destroy(void *payload)
+void public_key_free(struct public_key *key)
 {
-	struct public_key *key = payload;
-
-	if (key)
+	if (key) {
 		kfree(key->key);
-	kfree(key);
+		kfree(key);
+	}
+}
+EXPORT_SYMBOL_GPL(public_key_free);
+
+/*
+ * Destroy a public key algorithm key.
+ */
+static void public_key_destroy(void *payload0, void *payload3)
+{
+	public_key_free(payload0);
+	public_key_signature_free(payload3);
 }
-EXPORT_SYMBOL_GPL(public_key_destroy);
 
 struct public_key_completion {
 	struct completion completion;
diff --git a/crypto/asymmetric_keys/signature.c b/crypto/asymmetric_keys/signature.c
index 004d5fc8e56b..3beee3976ed5 100644
--- a/crypto/asymmetric_keys/signature.c
+++ b/crypto/asymmetric_keys/signature.c
@@ -15,9 +15,23 @@
 #include <keys/asymmetric-subtype.h>
 #include <linux/export.h>
 #include <linux/err.h>
+#include <linux/slab.h>
 #include <crypto/public_key.h>
 #include "asymmetric_keys.h"
 
+/*
+ * Destroy a public key signature.
+ */
+void public_key_signature_free(struct public_key_signature *sig)
+{
+	if (sig) {
+		kfree(sig->s);
+		kfree(sig->digest);
+		kfree(sig);
+	}
+}
+EXPORT_SYMBOL_GPL(public_key_signature_free);
+
 /**
  * verify_signature - Initiate the use of an asymmetric key to verify a signature
  * @key: The asymmetric key to verify against
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 4a29bac70060..05251c7f9a03 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -47,7 +47,7 @@ struct x509_parse_context {
 void x509_free_certificate(struct x509_certificate *cert)
 {
 	if (cert) {
-		public_key_destroy(cert->pub);
+		public_key_free(cert->pub);
 		kfree(cert->issuer);
 		kfree(cert->subject);
 		kfree(cert->id);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index aa730ea7faf8..19f557ca50ba 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -41,7 +41,7 @@ struct public_key {
 	const char *pkey_algo;
 };
 
-extern void public_key_destroy(void *payload);
+extern void public_key_free(struct public_key *key);
 
 /*
  * Public key cryptography signature data
@@ -55,7 +55,10 @@ struct public_key_signature {
 	const char *hash_algo;
 };
 
+extern void public_key_signature_free(struct public_key_signature *sig);
+
 extern struct asymmetric_key_subtype public_key_subtype;
+
 struct key;
 extern int verify_signature(const struct key *key,
 			    const struct public_key_signature *sig);
diff --git a/include/keys/asymmetric-subtype.h b/include/keys/asymmetric-subtype.h
index 4915d40d3c3c..2480469ce8fb 100644
--- a/include/keys/asymmetric-subtype.h
+++ b/include/keys/asymmetric-subtype.h
@@ -32,7 +32,7 @@ struct asymmetric_key_subtype {
 	void (*describe)(const struct key *key, struct seq_file *m);
 
 	/* Destroy a key of this subtype */
-	void (*destroy)(void *payload);
+	void (*destroy)(void *payload_crypto, void *payload_auth);
 
 	/* Verify the signature on a key of this subtype (optional) */
 	int (*verify_signature)(const struct key *key,
diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h
index 59c1df9cf922..70a8775bb444 100644
--- a/include/keys/asymmetric-type.h
+++ b/include/keys/asymmetric-type.h
@@ -23,9 +23,10 @@ extern struct key_type key_type_asymmetric;
  * follows:
  */
 enum asymmetric_payload_bits {
-	asym_crypto,
-	asym_subtype,
-	asym_key_ids,
+	asym_crypto,		/* The data representing the key */
+	asym_subtype,		/* Pointer to an asymmetric_key_subtype struct */
+	asym_key_ids,		/* Pointer to an asymmetric_key_ids struct */
+	asym_auth		/* The key's authorisation (signature, parent key ID) */
 };
 
 /*

[RFC PATCH 3/7] KEYS: Add identifier pointers to public_key_signature struct

[David Howells]

Add key identifier pointers to public_key_signature struct so that they can
be used to retain the identifier of the key to be used to verify the
signature in both PKCS#7 and X.509.

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

 crypto/asymmetric_keys/signature.c |    4 ++++
 include/crypto/public_key.h        |    1 +
 2 files changed, 5 insertions(+)

diff --git a/crypto/asymmetric_keys/signature.c b/crypto/asymmetric_keys/signature.c
index 3beee3976ed5..11b7ba170904 100644
--- a/crypto/asymmetric_keys/signature.c
+++ b/crypto/asymmetric_keys/signature.c
@@ -24,7 +24,11 @@
  */
 void public_key_signature_free(struct public_key_signature *sig)
 {
+	int i;
+
 	if (sig) {
+		for (i = 0; i < ARRAY_SIZE(sig->auth_ids); i++)
+			kfree(sig->auth_ids[i]);
 		kfree(sig->s);
 		kfree(sig->digest);
 		kfree(sig);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 19f557ca50ba..2f5de5c1a3a0 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -47,6 +47,7 @@ extern void public_key_free(struct public_key *key);
  * Public key cryptography signature data
  */
 struct public_key_signature {
+	struct asymmetric_key_id *auth_ids[2];
 	u8 *s;			/* Signature */
 	u32 s_size;		/* Number of bytes in signature */
 	u8 *digest;

[RFC PATCH 4/7] X.509: Retain the key verification data

[David Howells]

Retain the key verification data (ie. the struct public_key_signature)
including the digest and the key identifiers.

Note that this means that we need to take a separate copy of the digest in
x509_get_sig_params() rather than lumping it in with the crypto layer data.

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

 crypto/asymmetric_keys/pkcs7_trust.c      |    8 ++--
 crypto/asymmetric_keys/pkcs7_verify.c     |   20 +++++-----
 crypto/asymmetric_keys/x509_cert_parser.c |   40 ++++++++++---------
 crypto/asymmetric_keys/x509_parser.h      |    4 --
 crypto/asymmetric_keys/x509_public_key.c  |   61 +++++++++++++++--------------
 5 files changed, 67 insertions(+), 66 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 3bbdcc79a3d3..bf89e006bd07 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -80,16 +80,16 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 
 		might_sleep();
 		last = x509;
-		sig = &last->sig;
+		sig = last->sig;
 	}
 
 	/* No match - see if the root certificate has a signer amongst the
 	 * trusted keys.
 	 */
-	if (last && (last->akid_id || last->akid_skid)) {
+	if (last && (last->sig->auth_ids[0] || last->sig->auth_ids[1])) {
 		key = x509_request_asymmetric_key(trust_keyring,
-						  last->akid_id,
-						  last->akid_skid,
+						  last->sig->auth_ids[0],
+						  last->sig->auth_ids[1],
 						  false);
 		if (!IS_ERR(key)) {
 			x509 = last;
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 50be2a15e531..d8d8d234874e 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -174,6 +174,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
 static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 				  struct pkcs7_signed_info *sinfo)
 {
+	struct public_key_signature *sig;
 	struct x509_certificate *x509 = sinfo->signer, *p;
 	struct asymmetric_key_id *auth;
 	int ret;
@@ -193,14 +194,15 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 			goto maybe_missing_crypto_in_x509;
 
 		pr_debug("- issuer %s\n", x509->issuer);
-		if (x509->akid_id)
+		sig = x509->sig;
+		if (sig->auth_ids[0])
 			pr_debug("- authkeyid.id %*phN\n",
-				 x509->akid_id->len, x509->akid_id->data);
-		if (x509->akid_skid)
+				 sig->auth_ids[0]->len, sig->auth_ids[0]->data);
+		if (sig->auth_ids[1])
 			pr_debug("- authkeyid.skid %*phN\n",
-				 x509->akid_skid->len, x509->akid_skid->data);
+				 sig->auth_ids[1]->len, sig->auth_ids[1]->data);
 
-		if ((!x509->akid_id && !x509->akid_skid) ||
+		if ((!x509->sig->auth_ids[0] && !x509->sig->auth_ids[1]) ||
 		    strcmp(x509->subject, x509->issuer) == 0) {
 			/* If there's no authority certificate specified, then
 			 * the certificate must be self-signed and is the root
@@ -224,7 +226,7 @@ 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.
 		 */
-		auth = x509->akid_id;
+		auth = sig->auth_ids[0];
 		if (auth) {
 			pr_debug("- want %*phN\n", auth->len, auth->data);
 			for (p = pkcs7->certs; p; p = p->next) {
@@ -234,7 +236,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 					goto found_issuer_check_skid;
 			}
 		} else {
-			auth = x509->akid_skid;
+			auth = sig->auth_ids[1];
 			pr_debug("- want %*phN\n", auth->len, auth->data);
 			for (p = pkcs7->certs; p; p = p->next) {
 				if (!p->skid)
@@ -254,8 +256,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		/* We matched issuer + serialNumber, but if there's an
 		 * authKeyId.keyId, that must match the CA subjKeyId also.
 		 */
-		if (x509->akid_skid &&
-		    !asymmetric_key_id_same(p->skid, x509->akid_skid)) {
+		if (sig->auth_ids[1] &&
+		    !asymmetric_key_id_same(p->skid, sig->auth_ids[1])) {
 			pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
 				sinfo->index, x509->index, p->index);
 			return -EKEYREJECTED;
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 05251c7f9a03..a2fefa713614 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -48,14 +48,11 @@ void x509_free_certificate(struct x509_certificate *cert)
 {
 	if (cert) {
 		public_key_free(cert->pub);
+		public_key_signature_free(cert->sig);
 		kfree(cert->issuer);
 		kfree(cert->subject);
 		kfree(cert->id);
 		kfree(cert->skid);
-		kfree(cert->akid_id);
-		kfree(cert->akid_skid);
-		kfree(cert->sig.digest);
-		kfree(cert->sig.s);
 		kfree(cert);
 	}
 }
@@ -78,6 +75,9 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 	cert->pub = kzalloc(sizeof(struct public_key), GFP_KERNEL);
 	if (!cert->pub)
 		goto error_no_ctx;
+	cert->sig = kzalloc(sizeof(struct public_key_signature), GFP_KERNEL);
+	if (!cert->sig)
+		goto error_no_ctx;
 	ctx = kzalloc(sizeof(struct x509_parse_context), GFP_KERNEL);
 	if (!ctx)
 		goto error_no_ctx;
@@ -188,33 +188,33 @@ int x509_note_pkey_algo(void *context, size_t hdrlen,
 		return -ENOPKG; /* Unsupported combination */
 
 	case OID_md4WithRSAEncryption:
-		ctx->cert->sig.hash_algo = "md4";
-		ctx->cert->sig.pkey_algo = "rsa";
+		ctx->cert->sig->hash_algo = "md4";
+		ctx->cert->sig->pkey_algo = "rsa";
 		break;
 
 	case OID_sha1WithRSAEncryption:
-		ctx->cert->sig.hash_algo = "sha1";
-		ctx->cert->sig.pkey_algo = "rsa";
+		ctx->cert->sig->hash_algo = "sha1";
+		ctx->cert->sig->pkey_algo = "rsa";
 		break;
 
 	case OID_sha256WithRSAEncryption:
-		ctx->cert->sig.hash_algo = "sha256";
-		ctx->cert->sig.pkey_algo = "rsa";
+		ctx->cert->sig->hash_algo = "sha256";
+		ctx->cert->sig->pkey_algo = "rsa";
 		break;
 
 	case OID_sha384WithRSAEncryption:
-		ctx->cert->sig.hash_algo = "sha384";
-		ctx->cert->sig.pkey_algo = "rsa";
+		ctx->cert->sig->hash_algo = "sha384";
+		ctx->cert->sig->pkey_algo = "rsa";
 		break;
 
 	case OID_sha512WithRSAEncryption:
-		ctx->cert->sig.hash_algo = "sha512";
-		ctx->cert->sig.pkey_algo = "rsa";
+		ctx->cert->sig->hash_algo = "sha512";
+		ctx->cert->sig->pkey_algo = "rsa";
 		break;
 
 	case OID_sha224WithRSAEncryption:
-		ctx->cert->sig.hash_algo = "sha224";
-		ctx->cert->sig.pkey_algo = "rsa";
+		ctx->cert->sig->hash_algo = "sha224";
+		ctx->cert->sig->pkey_algo = "rsa";
 		break;
 	}
 
@@ -572,14 +572,14 @@ int x509_akid_note_kid(void *context, size_t hdrlen,
 
 	pr_debug("AKID: keyid: %*phN\n", (int)vlen, value);
 
-	if (ctx->cert->akid_skid)
+	if (ctx->cert->sig->auth_ids[1])
 		return 0;
 
 	kid = asymmetric_key_generate_id(value, vlen, "", 0);
 	if (IS_ERR(kid))
 		return PTR_ERR(kid);
 	pr_debug("authkeyid %*phN\n", kid->len, kid->data);
-	ctx->cert->akid_skid = kid;
+	ctx->cert->sig->auth_ids[1] = kid;
 	return 0;
 }
 
@@ -611,7 +611,7 @@ int x509_akid_note_serial(void *context, size_t hdrlen,
 
 	pr_debug("AKID: serial: %*phN\n", (int)vlen, value);
 
-	if (!ctx->akid_raw_issuer || ctx->cert->akid_id)
+	if (!ctx->akid_raw_issuer || ctx->cert->sig->auth_ids[0])
 		return 0;
 
 	kid = asymmetric_key_generate_id(value,
@@ -622,6 +622,6 @@ int x509_akid_note_serial(void *context, size_t hdrlen,
 		return PTR_ERR(kid);
 
 	pr_debug("authkeyid %*phN\n", kid->len, kid->data);
-	ctx->cert->akid_id = kid;
+	ctx->cert->sig->auth_ids[0] = kid;
 	return 0;
 }
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index dbeed6018e63..26a4d83e4e6d 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -17,13 +17,11 @@ struct x509_certificate {
 	struct x509_certificate *next;
 	struct x509_certificate *signer;	/* Certificate that signed this one */
 	struct public_key *pub;			/* Public key details */
-	struct public_key_signature sig;	/* Signature parameters */
+	struct public_key_signature *sig;	/* Signature parameters */
 	char		*issuer;		/* Name of certificate issuer */
 	char		*subject;		/* Name of certificate subject */
 	struct asymmetric_key_id *id;		/* Issuer + Serial number */
 	struct asymmetric_key_id *skid;		/* Subject + subjectKeyId (optional) */
-	struct asymmetric_key_id *akid_id;	/* CA AuthKeyId matching ->id (optional) */
-	struct asymmetric_key_id *akid_skid;	/* CA AuthKeyId matching ->skid (optional) */
 	time64_t	valid_from;
 	time64_t	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 2fcf707fb208..4cd102de174c 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -153,30 +153,29 @@ EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
  */
 int x509_get_sig_params(struct x509_certificate *cert)
 {
+	struct public_key_signature *sig = cert->sig;
 	struct crypto_shash *tfm;
 	struct shash_desc *desc;
-	size_t digest_size, desc_size;
-	void *digest;
+	size_t desc_size;
 	int ret;
 
 	pr_devel("==>%s()\n", __func__);
 
 	if (cert->unsupported_crypto)
 		return -ENOPKG;
-	if (cert->sig.s)
+	if (sig->s)
 		return 0;
 
-	cert->sig.s = kmemdup(cert->raw_sig, cert->raw_sig_size,
-			      GFP_KERNEL);
-	if (!cert->sig.s)
+	sig->s = kmemdup(cert->raw_sig, cert->raw_sig_size, GFP_KERNEL);
+	if (!sig->s)
 		return -ENOMEM;
 
-	cert->sig.s_size = cert->raw_sig_size;
+	sig->s_size = cert->raw_sig_size;
 
 	/* Allocate the hashing algorithm we're going to need and find out how
 	 * big the hash operational data will be.
 	 */
-	tfm = crypto_alloc_shash(cert->sig.hash_algo, 0, 0);
+	tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
 	if (IS_ERR(tfm)) {
 		if (PTR_ERR(tfm) == -ENOENT) {
 			cert->unsupported_crypto = true;
@@ -186,29 +185,28 @@ int x509_get_sig_params(struct x509_certificate *cert)
 	}
 
 	desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
-	digest_size = crypto_shash_digestsize(tfm);
+	sig->digest_size = crypto_shash_digestsize(tfm);
 
-	/* We allocate the hash operational data storage on the end of the
-	 * digest storage space.
-	 */
 	ret = -ENOMEM;
-	digest = kzalloc(ALIGN(digest_size, __alignof__(*desc)) + desc_size,
-			 GFP_KERNEL);
-	if (!digest)
+	sig->digest = kmalloc(sig->digest_size, GFP_KERNEL);
+	if (!sig->digest)
 		goto error;
 
-	cert->sig.digest = digest;
-	cert->sig.digest_size = digest_size;
+	desc = kzalloc(desc_size, GFP_KERNEL);
+	if (!desc)
+		goto error;
 
-	desc = PTR_ALIGN(digest + digest_size, __alignof__(*desc));
 	desc->tfm = tfm;
 	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 
 	ret = crypto_shash_init(desc);
 	if (ret < 0)
-		goto error;
+		goto error_2;
 	might_sleep();
-	ret = crypto_shash_finup(desc, cert->tbs, cert->tbs_size, digest);
+	ret = crypto_shash_finup(desc, cert->tbs, cert->tbs_size, sig->digest);
+
+error_2:
+	kfree(desc);
 error:
 	crypto_free_shash(tfm);
 	pr_devel("<==%s() = %d\n", __func__, ret);
@@ -230,7 +228,7 @@ int x509_check_signature(const struct public_key *pub,
 	if (ret < 0)
 		return ret;
 
-	ret = public_key_verify_signature(pub, &cert->sig);
+	ret = public_key_verify_signature(pub, cert->sig);
 	if (ret == -ENOPKG)
 		cert->unsupported_crypto = true;
 	pr_debug("Cert Verification: %d\n", ret);
@@ -250,17 +248,18 @@ EXPORT_SYMBOL_GPL(x509_check_signature);
 static int x509_validate_trust(struct x509_certificate *cert,
 			       struct key *trust_keyring)
 {
+	struct public_key_signature *sig = cert->sig;
 	struct key *key;
 	int ret = 1;
 
 	if (!trust_keyring)
 		return -EOPNOTSUPP;
 
-	if (ca_keyid && !asymmetric_key_id_partial(cert->akid_skid, ca_keyid))
+	if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
 		return -EPERM;
 
 	key = x509_request_asymmetric_key(trust_keyring,
-					  cert->akid_id, cert->akid_skid,
+					  sig->auth_ids[0], sig->auth_ids[1],
 					  false);
 	if (!IS_ERR(key))  {
 		if (!use_builtin_keys
@@ -292,8 +291,8 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	pr_devel("Cert Subject: %s\n", cert->subject);
 
 	if (!cert->pub->pkey_algo ||
-	    !cert->sig.pkey_algo ||
-	    !cert->sig.hash_algo) {
+	    !cert->sig->pkey_algo ||
+	    !cert->sig->hash_algo) {
 		ret = -ENOPKG;
 		goto error_free_cert;
 	}
@@ -301,15 +300,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	pr_devel("Cert Key Algo: %s\n", cert->pub->pkey_algo);
 	pr_devel("Cert Valid period: %lld-%lld\n", cert->valid_from, cert->valid_to);
 	pr_devel("Cert Signature: %s + %s\n",
-		 cert->sig.pkey_algo,
-		 cert->sig.hash_algo);
+		 cert->sig->pkey_algo,
+		 cert->sig->hash_algo);
 
 	cert->pub->id_type = "X509";
 
 	/* Check the signature on the key if it appears to be self-signed */
-	if ((!cert->akid_skid && !cert->akid_id) ||
-	    asymmetric_key_id_same(cert->skid, cert->akid_skid) ||
-	    asymmetric_key_id_same(cert->id, cert->akid_id)) {
+	if ((!cert->sig->auth_ids[0] && !cert->sig->auth_ids[1]) ||
+	    asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]) ||
+	    asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0])) {
 		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;
@@ -353,6 +352,7 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	prep->payload.data[asym_subtype] = &public_key_subtype;
 	prep->payload.data[asym_key_ids] = kids;
 	prep->payload.data[asym_crypto] = cert->pub;
+	prep->payload.data[asym_auth] = cert->sig;
 	prep->description = desc;
 	prep->quotalen = 100;
 
@@ -360,6 +360,7 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	cert->pub = NULL;
 	cert->id = NULL;
 	cert->skid = NULL;
+	cert->sig = NULL;
 	desc = NULL;
 	ret = 0;
 

[RFC PATCH 5/7] PKCS#7: Make the signature a pointer rather than embedding it

[David Howells]

Point to the public_key_signature struct from the pkcs7_signed_info struct
rather than embedding it.  This makes the code consistent with the X.509
signature handling and makes it possible to have a common cleanup function.

We also save a copy of the digest in the signature without sharing the
memory with the crypto layer metadata.

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

 crypto/asymmetric_keys/pkcs7_parser.c |   38 +++++++++++++++----------
 crypto/asymmetric_keys/pkcs7_parser.h |   10 +++---
 crypto/asymmetric_keys/pkcs7_trust.c  |    4 +--
 crypto/asymmetric_keys/pkcs7_verify.c |   51 +++++++++++++++++----------------
 4 files changed, 55 insertions(+), 48 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 40de03f49ff8..835701613125 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -44,9 +44,7 @@ struct pkcs7_parse_context {
 static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo)
 {
 	if (sinfo) {
-		kfree(sinfo->sig.s);
-		kfree(sinfo->sig.digest);
-		kfree(sinfo->signing_cert_id);
+		public_key_signature_free(sinfo->sig);
 		kfree(sinfo);
 	}
 }
@@ -125,6 +123,10 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
 	ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
 	if (!ctx->sinfo)
 		goto out_no_sinfo;
+	ctx->sinfo->sig = kzalloc(sizeof(struct public_key_signature),
+				  GFP_KERNEL);
+	if (!ctx->sinfo->sig)
+		goto out_no_sig;
 
 	ctx->data = (unsigned long)data;
 	ctx->ppcerts = &ctx->certs;
@@ -150,6 +152,7 @@ out:
 		ctx->certs = cert->next;
 		x509_free_certificate(cert);
 	}
+out_no_sig:
 	pkcs7_free_signed_info(ctx->sinfo);
 out_no_sinfo:
 	pkcs7_free_message(ctx->msg);
@@ -218,25 +221,26 @@ int pkcs7_sig_note_digest_algo(void *context, size_t hdrlen,
 
 	switch (ctx->last_oid) {
 	case OID_md4:
-		ctx->sinfo->sig.hash_algo = "md4";
+		ctx->sinfo->sig->hash_algo = "md4";
 		break;
 	case OID_md5:
-		ctx->sinfo->sig.hash_algo = "md5";
+		ctx->sinfo->sig->hash_algo = "md5";
 		break;
 	case OID_sha1:
-		ctx->sinfo->sig.hash_algo = "sha1";
+		ctx->sinfo->sig->hash_algo = "sha1";
 		break;
 	case OID_sha256:
-		ctx->sinfo->sig.hash_algo = "sha256";
+		ctx->sinfo->sig->hash_algo = "sha256";
 		break;
 	case OID_sha384:
-		ctx->sinfo->sig.hash_algo = "sha384";
+		ctx->sinfo->sig->hash_algo = "sha384";
 		break;
 	case OID_sha512:
-		ctx->sinfo->sig.hash_algo = "sha512";
+		ctx->sinfo->sig->hash_algo = "sha512";
 		break;
 	case OID_sha224:
-		ctx->sinfo->sig.hash_algo = "sha224";
+		ctx->sinfo->sig->hash_algo = "sha224";
+		break;
 	default:
 		printk("Unsupported digest algo: %u\n", ctx->last_oid);
 		return -ENOPKG;
@@ -255,7 +259,7 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen,
 
 	switch (ctx->last_oid) {
 	case OID_rsaEncryption:
-		ctx->sinfo->sig.pkey_algo = "rsa";
+		ctx->sinfo->sig->pkey_algo = "rsa";
 		break;
 	default:
 		printk("Unsupported pkey algo: %u\n", ctx->last_oid);
@@ -615,11 +619,11 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen,
 {
 	struct pkcs7_parse_context *ctx = context;
 
-	ctx->sinfo->sig.s = kmemdup(value, vlen, GFP_KERNEL);
-	if (!ctx->sinfo->sig.s)
+	ctx->sinfo->sig->s = kmemdup(value, vlen, GFP_KERNEL);
+	if (!ctx->sinfo->sig->s)
 		return -ENOMEM;
 
-	ctx->sinfo->sig.s_size = vlen;
+	ctx->sinfo->sig->s_size = vlen;
 	return 0;
 }
 
@@ -655,12 +659,16 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
 
 	pr_devel("SINFO KID: %u [%*phN]\n", kid->len, kid->len, kid->data);
 
-	sinfo->signing_cert_id = kid;
+	sinfo->sig->auth_ids[0] = 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;
+	ctx->sinfo->sig = kzalloc(sizeof(struct public_key_signature),
+				  GFP_KERNEL);
+	if (!ctx->sinfo->sig)
+		return -ENOMEM;
 	return 0;
 }
diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
index a66b19ebcf47..d5eec31e95b6 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -41,19 +41,17 @@ struct pkcs7_signed_info {
 #define	sinfo_has_ms_statement_type	5
 	time64_t	signing_time;
 
-	/* Issuing cert serial number and issuer's name [PKCS#7 or CMS ver 1]
-	 * or issuing cert's SKID [CMS ver 3].
-	 */
-	struct asymmetric_key_id *signing_cert_id;
-
 	/* Message signature.
 	 *
 	 * This contains the generated digest of _either_ the Content Data or
 	 * the Authenticated Attributes [RFC2315 9.3].  If the latter, one of
 	 * the attributes contains the digest of the the Content Data within
 	 * it.
+	 *
+	 * THis also contains the issuing cert serial number and issuer's name
+	 * [PKCS#7 or CMS ver 1] or issuing cert's SKID [CMS ver 3].
 	 */
-	struct public_key_signature sig;
+	struct public_key_signature *sig;
 };
 
 struct pkcs7_message {
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index bf89e006bd07..554200284ea8 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -27,7 +27,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 				    struct pkcs7_signed_info *sinfo,
 				    struct key *trust_keyring)
 {
-	struct public_key_signature *sig = &sinfo->sig;
+	struct public_key_signature *sig = sinfo->sig;
 	struct x509_certificate *x509, *last = NULL, *p;
 	struct key *key;
 	bool trusted;
@@ -105,7 +105,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 	 * the signed info directly.
 	 */
 	key = x509_request_asymmetric_key(trust_keyring,
-					  sinfo->signing_cert_id,
+					  sinfo->sig->auth_ids[0],
 					  NULL,
 					  false);
 	if (!IS_ERR(key)) {
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index d8d8d234874e..1426f03e630b 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -25,34 +25,36 @@
 static int pkcs7_digest(struct pkcs7_message *pkcs7,
 			struct pkcs7_signed_info *sinfo)
 {
+	struct public_key_signature *sig = sinfo->sig;
 	struct crypto_shash *tfm;
 	struct shash_desc *desc;
-	size_t digest_size, desc_size;
-	void *digest;
+	size_t desc_size;
 	int ret;
 
-	kenter(",%u,%s", sinfo->index, sinfo->sig.hash_algo);
+	kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
 
-	if (!sinfo->sig.hash_algo)
+	if (!sinfo->sig->hash_algo)
 		return -ENOPKG;
 
 	/* Allocate the hashing algorithm we're going to need and find out how
 	 * big the hash operational data will be.
 	 */
-	tfm = crypto_alloc_shash(sinfo->sig.hash_algo, 0, 0);
+	tfm = crypto_alloc_shash(sinfo->sig->hash_algo, 0, 0);
 	if (IS_ERR(tfm))
 		return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
 
 	desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
-	sinfo->sig.digest_size = digest_size = crypto_shash_digestsize(tfm);
+	sig->digest_size = crypto_shash_digestsize(tfm);
 
 	ret = -ENOMEM;
-	digest = kzalloc(ALIGN(digest_size, __alignof__(*desc)) + desc_size,
-			 GFP_KERNEL);
-	if (!digest)
+	sig->digest = kmalloc(sig->digest_size, GFP_KERNEL);
+	if (!sig->digest)
+		goto error_no_desc;
+
+	desc = kzalloc(desc_size, GFP_KERNEL);
+	if (!desc)
 		goto error_no_desc;
 
-	desc = PTR_ALIGN(digest + digest_size, __alignof__(*desc));
 	desc->tfm   = tfm;
 	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 
@@ -60,10 +62,11 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
 	ret = crypto_shash_init(desc);
 	if (ret < 0)
 		goto error;
-	ret = crypto_shash_finup(desc, pkcs7->data, pkcs7->data_len, digest);
+	ret = crypto_shash_finup(desc, pkcs7->data, pkcs7->data_len,
+				 sig->digest);
 	if (ret < 0)
 		goto error;
-	pr_devel("MsgDigest = [%*ph]\n", 8, digest);
+	pr_devel("MsgDigest = [%*ph]\n", 8, sig->digest);
 
 	/* However, if there are authenticated attributes, there must be a
 	 * message digest attribute amongst them which corresponds to the
@@ -78,14 +81,15 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
 			goto error;
 		}
 
-		if (sinfo->msgdigest_len != sinfo->sig.digest_size) {
+		if (sinfo->msgdigest_len != sig->digest_size) {
 			pr_debug("Sig %u: Invalid digest size (%u)\n",
 				 sinfo->index, sinfo->msgdigest_len);
 			ret = -EBADMSG;
 			goto error;
 		}
 
-		if (memcmp(digest, sinfo->msgdigest, sinfo->msgdigest_len) != 0) {
+		if (memcmp(sig->digest, sinfo->msgdigest,
+			   sinfo->msgdigest_len) != 0) {
 			pr_debug("Sig %u: Message digest doesn't match\n",
 				 sinfo->index);
 			ret = -EKEYREJECTED;
@@ -97,7 +101,7 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
 		 * convert the attributes from a CONT.0 into a SET before we
 		 * hash it.
 		 */
-		memset(digest, 0, sinfo->sig.digest_size);
+		memset(sig->digest, 0, sig->digest_size);
 
 		ret = crypto_shash_init(desc);
 		if (ret < 0)
@@ -107,17 +111,14 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
 		if (ret < 0)
 			goto error;
 		ret = crypto_shash_finup(desc, sinfo->authattrs,
-					 sinfo->authattrs_len, digest);
+					 sinfo->authattrs_len, sig->digest);
 		if (ret < 0)
 			goto error;
-		pr_devel("AADigest = [%*ph]\n", 8, digest);
+		pr_devel("AADigest = [%*ph]\n", 8, sig->digest);
 	}
 
-	sinfo->sig.digest = digest;
-	digest = NULL;
-
 error:
-	kfree(digest);
+	kfree(desc);
 error_no_desc:
 	crypto_free_shash(tfm);
 	kleave(" = %d", ret);
@@ -144,12 +145,12 @@ 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 (!asymmetric_key_id_same(x509->id, sinfo->signing_cert_id))
+		if (!asymmetric_key_id_same(x509->id, sinfo->sig->auth_ids[0]))
 			continue;
 		pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
 			 sinfo->index, certix);
 
-		if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
+		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);
 			continue;
@@ -164,7 +165,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
 	 */
 	pr_debug("Sig %u: Issuing X.509 cert not found (#%*phN)\n",
 		 sinfo->index,
-		 sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
+		 sinfo->sig->auth_ids[0]->len, sinfo->sig->auth_ids[0]->data);
 	return 0;
 }
 
@@ -334,7 +335,7 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
 	}
 
 	/* Verify the PKCS#7 binary against the key */
-	ret = public_key_verify_signature(sinfo->signer->pub, &sinfo->sig);
+	ret = public_key_verify_signature(sinfo->signer->pub, sinfo->sig);
 	if (ret < 0)
 		return ret;
 

[RFC PATCH 6/7] X.509: Extract signature digest and make self-signed cert checks earlier

[David Howells]

Extract the signature digest for an X.509 certificate earlier, at the end
of x509_cert_parse() rather than leaving it to the callers thereof since it
has to be called anyway.

Further, immediately after that, check the signature on self-signed
certificates, also rather in the callers of x509_cert_parse().

We note in the x509_certificate struct the following bits of information:

 (1) Whether the signature is self-signed (even if we can't check the
     signature due to missing crypto).

 (2) Whether the key held in the certificate needs unsupported crypto to be
     used.  We may get a PKCS#7 message with X.509 certs that we can't make
     use of - we just ignore them and give ENOPKG at the end it we couldn't
     verify anything if at least one of these unusable certs are in the
     chain of trust.

 (3) Whether the signature held in the certificate needs unsupported crypto
     to be checked.  We can still use the key held in this certificate,
     even if we can't check the signature on it - if it is held in the
     system trusted keyring, for instance.  We just can't add it to a ring
     of trusted keys or follow it further up the chain of trust.

Making these checks earlier allows x509_check_signature() to be removed and
replaced with direct calls to public_key_verify_signature().

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

 crypto/asymmetric_keys/pkcs7_verify.c     |   38 ++-------
 crypto/asymmetric_keys/x509_cert_parser.c |   10 ++
 crypto/asymmetric_keys/x509_parser.h      |    7 +-
 crypto/asymmetric_keys/x509_public_key.c  |  126 ++++++++++++++++++++---------
 4 files changed, 110 insertions(+), 71 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 1426f03e630b..44b746e9df1b 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -190,9 +190,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 			 x509->subject,
 			 x509->raw_serial_size, x509->raw_serial);
 		x509->seen = true;
-		ret = x509_get_sig_params(x509);
-		if (ret < 0)
-			goto maybe_missing_crypto_in_x509;
+		if (x509->unsupported_key)
+			goto unsupported_crypto_in_x509;
 
 		pr_debug("- issuer %s\n", x509->issuer);
 		sig = x509->sig;
@@ -203,22 +202,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 			pr_debug("- authkeyid.skid %*phN\n",
 				 sig->auth_ids[1]->len, sig->auth_ids[1]->data);
 
-		if ((!x509->sig->auth_ids[0] && !x509->sig->auth_ids[1]) ||
-		    strcmp(x509->subject, x509->issuer) == 0) {
+		if (x509->self_signed) {
 			/* If there's no authority certificate specified, then
 			 * the certificate must be self-signed and is the root
 			 * of the chain.  Likewise if the cert is its own
 			 * authority.
 			 */
-			pr_debug("- no auth?\n");
-			if (x509->raw_subject_size != x509->raw_issuer_size ||
-			    memcmp(x509->raw_subject, x509->raw_issuer,
-				   x509->raw_issuer_size) != 0)
-				return 0;
-
-			ret = x509_check_signature(x509->pub, x509);
-			if (ret < 0)
-				goto maybe_missing_crypto_in_x509;
+			if (x509->unsupported_sig)
+				goto unsupported_crypto_in_x509;
 			x509->signer = x509;
 			pr_debug("- self-signed\n");
 			return 0;
@@ -270,7 +261,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 				sinfo->index);
 			return 0;
 		}
-		ret = x509_check_signature(p->pub, x509);
+		ret = public_key_verify_signature(p->pub, p->sig);
 		if (ret < 0)
 			return ret;
 		x509->signer = p;
@@ -282,16 +273,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		might_sleep();
 	}
 
-maybe_missing_crypto_in_x509:
+unsupported_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
+	 * sinfo->unsupported_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;
+	return 0;
 }
 
 /*
@@ -378,9 +367,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
 		 enum key_being_used_for usage)
 {
 	struct pkcs7_signed_info *sinfo;
-	struct x509_certificate *x509;
 	int enopkg = -ENOPKG;
-	int ret, n;
+	int ret;
 
 	kenter("");
 
@@ -422,12 +410,6 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
 		return -EINVAL;
 	}
 
-	for (n = 0, x509 = pkcs7->certs; x509; x509 = x509->next, n++) {
-		ret = x509_get_sig_params(x509);
-		if (ret < 0)
-			return ret;
-	}
-
 	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
 		ret = pkcs7_verify_one(pkcs7, sinfo);
 		if (ret < 0) {
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index a2fefa713614..865f46ea724f 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -108,6 +108,11 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 
 	cert->pub->keylen = ctx->key_size;
 
+	/* Grab the signature bits */
+	ret = x509_get_sig_params(cert);
+	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,
@@ -119,6 +124,11 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 	}
 	cert->id = kid;
 
+	/* Detect self-signed certificates */
+	ret = x509_check_for_self_signed(cert);
+	if (ret < 0)
+		goto error_decode;
+
 	kfree(ctx);
 	return cert;
 
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 26a4d83e4e6d..f24f4d808e7f 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -40,7 +40,9 @@ 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 */
+	bool		self_signed;		/* T if self-signed (check unsupported_sig too) */
+	bool		unsupported_key;	/* T if key uses unsupported crypto */
+	bool		unsupported_sig;	/* T if signature uses unsupported crypto */
 };
 
 /*
@@ -56,5 +58,4 @@ extern int x509_decode_time(time64_t *_t,  size_t hdrlen,
  * x509_public_key.c
  */
 extern int x509_get_sig_params(struct x509_certificate *cert);
-extern int x509_check_signature(const struct public_key *pub,
-				struct x509_certificate *cert);
+extern int x509_check_for_self_signed(struct x509_certificate *cert);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 4cd102de174c..0d5b9add313f 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -161,10 +161,17 @@ int x509_get_sig_params(struct x509_certificate *cert)
 
 	pr_devel("==>%s()\n", __func__);
 
-	if (cert->unsupported_crypto)
-		return -ENOPKG;
-	if (sig->s)
+	if (!cert->pub->pkey_algo)
+		cert->unsupported_key = true;
+
+	if (!sig->pkey_algo)
+		cert->unsupported_sig = true;
+
+	/* We check the hash if we can - even if we can't then verify it */
+	if (!sig->hash_algo) {
+		cert->unsupported_sig = true;
 		return 0;
+	}
 
 	sig->s = kmemdup(cert->raw_sig, cert->raw_sig_size, GFP_KERNEL);
 	if (!sig->s)
@@ -178,8 +185,8 @@ int x509_get_sig_params(struct x509_certificate *cert)
 	tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
 	if (IS_ERR(tfm)) {
 		if (PTR_ERR(tfm) == -ENOENT) {
-			cert->unsupported_crypto = true;
-			return -ENOPKG;
+			cert->unsupported_sig = true;
+			return 0;
 		}
 		return PTR_ERR(tfm);
 	}
@@ -212,29 +219,53 @@ error:
 	pr_devel("<==%s() = %d\n", __func__, ret);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(x509_get_sig_params);
 
 /*
- * Check the signature on a certificate using the provided public key
+ * Check for self-signedness in an X.509 cert and if found, check the signature
+ * immediately if we can.
  */
-int x509_check_signature(const struct public_key *pub,
-			 struct x509_certificate *cert)
+int x509_check_for_self_signed(struct x509_certificate *cert)
 {
-	int ret;
+	int ret = 0;
 
 	pr_devel("==>%s()\n", __func__);
 
-	ret = x509_get_sig_params(cert);
-	if (ret < 0)
-		return ret;
+	if (cert->sig->auth_ids[0] || cert->sig->auth_ids[1]) {
+		/* If the AKID is present it may have one or two parts.  If
+		 * both are supplied, both must match.
+		 */
+		bool a = asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]);
+		bool b = asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0]);
+
+		if (!a && !b)
+			goto not_self_signed;
+
+		ret = -EKEYREJECTED;
+		if (((a && !b) || (b && !a)) &&
+		    cert->sig->auth_ids[0] && cert->sig->auth_ids[1])
+			goto out;
+	}
+
+	ret = public_key_verify_signature(cert->pub, cert->sig);
+	if (ret < 0) {
+		if (ret == -ENOPKG) {
+			cert->unsupported_sig = true;
+			ret = 0;
+		}
+		goto out;
+	}
+
+	pr_devel("Cert Self-signature verified");
+	cert->self_signed = true;
 
-	ret = public_key_verify_signature(pub, cert->sig);
-	if (ret == -ENOPKG)
-		cert->unsupported_crypto = true;
-	pr_debug("Cert Verification: %d\n", ret);
+out:
+	pr_devel("<==%s() = %d\n", __func__, ret);
 	return ret;
+
+not_self_signed:
+	pr_devel("<==%s() = 0 [not]\n", __func__);
+	return 0;
 }
-EXPORT_SYMBOL_GPL(x509_check_signature);
 
 /*
  * Check the new certificate against the ones in the trust keyring.  If one of
@@ -252,22 +283,30 @@ static int x509_validate_trust(struct x509_certificate *cert,
 	struct key *key;
 	int ret = 1;
 
+	if (!cert->sig->auth_ids[0] && !cert->sig->auth_ids[1])
+		return 1;
+
 	if (!trust_keyring)
 		return -EOPNOTSUPP;
-
 	if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
 		return -EPERM;
+	if (cert->unsupported_sig)
+		return -ENOPKG;
 
 	key = x509_request_asymmetric_key(trust_keyring,
 					  sig->auth_ids[0], sig->auth_ids[1],
 					  false);
-	if (!IS_ERR(key))  {
-		if (!use_builtin_keys
-		    || test_bit(KEY_FLAG_BUILTIN, &key->flags))
-			ret = x509_check_signature(key->payload.data[asym_crypto],
-						   cert);
-		key_put(key);
+	if (IS_ERR(key))
+		return PTR_ERR(key);
+
+	if (!use_builtin_keys ||
+	    test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
+		ret = public_key_verify_signature(
+			key->payload.data[asym_crypto], cert->sig);
+		if (ret == -ENOPKG)
+			cert->unsupported_sig = true;
 	}
+	key_put(key);
 	return ret;
 }
 
@@ -290,34 +329,41 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	pr_devel("Cert Issuer: %s\n", cert->issuer);
 	pr_devel("Cert Subject: %s\n", cert->subject);
 
-	if (!cert->pub->pkey_algo ||
-	    !cert->sig->pkey_algo ||
-	    !cert->sig->hash_algo) {
+	if (cert->unsupported_key) {
 		ret = -ENOPKG;
 		goto error_free_cert;
 	}
 
 	pr_devel("Cert Key Algo: %s\n", cert->pub->pkey_algo);
 	pr_devel("Cert Valid period: %lld-%lld\n", cert->valid_from, cert->valid_to);
-	pr_devel("Cert Signature: %s + %s\n",
-		 cert->sig->pkey_algo,
-		 cert->sig->hash_algo);
 
 	cert->pub->id_type = "X509";
 
-	/* Check the signature on the key if it appears to be self-signed */
-	if ((!cert->sig->auth_ids[0] && !cert->sig->auth_ids[1]) ||
-	    asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]) ||
-	    asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0])) {
-		ret = x509_check_signature(cert->pub, cert); /* self-signed */
-		if (ret < 0)
-			goto error_free_cert;
-	} else if (!prep->trusted) {
+	/* See if we can derive the trustability of this certificate.
+	 *
+	 * When it comes to self-signed certificates, we cannot evaluate
+	 * trustedness except by the fact that we obtained it from a trusted
+	 * location.  So we just rely on x509_validate_trust() failing in this
+	 * case.
+	 *
+	 * Note that there's a possibility of a self-signed cert matching a
+	 * cert that we have (most likely a duplicate that we already trust) -
+	 * in which case it will be marked trusted.
+	 */
+	if (cert->unsupported_sig || cert->self_signed) {
+		public_key_signature_free(cert->sig);
+		cert->sig = NULL;
+	} else {
+		pr_devel("Cert Signature: %s + %s\n",
+			 cert->sig->pkey_algo, cert->sig->hash_algo);
+
 		ret = x509_validate_trust(cert, get_system_trusted_keyring());
 		if (ret)
 			ret = x509_validate_trust(cert, get_ima_mok_keyring());
+		if (ret == -EKEYREJECTED)
+			goto error_free_cert;
 		if (!ret)
-			prep->trusted = 1;
+			prep->trusted = true;
 	}
 
 	/* Propose a description */

[RFC PATCH 7/7] There's a bug in the code determining whether a certificate is self-signed

[David Howells]

or not: if they have neither AKID nor SKID then we just assume that the
cert is self-signed, which may not be true.

Fix this by checking that the raw subject name matches the raw issuer name
and that the public key algorithm for the key and signature are both the
same in addition to requiring that the AKID bits match.

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

 crypto/asymmetric_keys/x509_public_key.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 0d5b9add313f..da42acd492b6 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -230,6 +230,11 @@ int x509_check_for_self_signed(struct x509_certificate *cert)
 
 	pr_devel("==>%s()\n", __func__);
 
+	if (cert->raw_subject_size != cert->raw_issuer_size ||
+	    memcmp(cert->raw_subject, cert->raw_issuer,
+		   cert->raw_issuer_size) != 0)
+		goto not_self_signed;
+
 	if (cert->sig->auth_ids[0] || cert->sig->auth_ids[1]) {
 		/* If the AKID is present it may have one or two parts.  If
 		 * both are supplied, both must match.
@@ -246,6 +251,10 @@ int x509_check_for_self_signed(struct x509_certificate *cert)
 			goto out;
 	}
 
+	ret = -EKEYREJECTED;
+	if (cert->pub->pkey_algo != cert->sig->pkey_algo)
+		goto out;
+
 	ret = public_key_verify_signature(cert->pub, cert->sig);
 	if (ret < 0) {
 		if (ret == -ENOPKG) {

Re: [RFC PATCH 0/7] KEYS: Adjust public key signature handling

[Mimi Zohar]

Hi David,

On Mon, 2016-02-29 at 18:21 +0000, David Howells wrote:
> These patches do the following:
> 
>  (1) Retain a signature in an asymmetric-type key and associate with it the
>      identifiers that will match a key that can be used to verify it.
> 
>  (2) Differentiate an X.509 cert that cannot be used versus one that cannot
>      be verified due to unavailable crypto.  This is noted in the
>      structures involved.
> 
>  (3) Determination of the self-signedness of an X.509 cert is improved to
>      include checks on the subject/issuer names and the key
>      algorithm/signature algorithm types.
> 
>  (4) Self-signed X.509 certificates are consistency checked early on if the
>      appropriate crypto is available.
> 
> This set of patches is a prelude to a set that changes how trustworthiness
> is determined.

These patches don't apply directly on top of linux-security.   Maybe
they apply on top of an updated version of the  "X.509: Software public
key subtype changes" patch set.   In which branch are these patches?

thanks,

Mimi

Re: [RFC PATCH 0/7] KEYS: Adjust public key signature handling

[David Howells]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> These patches don't apply directly on top of linux-security.   Maybe
> they apply on top of an updated version of the  "X.509: Software public
> key subtype changes" patch set.   In which branch are these patches?

They're based on security/next as was a while ago.  If you look here:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git

you can see five branches that form a chain: keys-next, keys-rsa, keys-sig,
keys-trust and keys-blacklist.  keys-sig is this set.

David

Re: [RFC PATCH 0/7] KEYS: Adjust public key signature handling

[Mimi Zohar]

On Wed, 2016-03-02 at 12:24 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > These patches don't apply directly on top of linux-security.   Maybe
> > they apply on top of an updated version of the  "X.509: Software public
> > key subtype changes" patch set.   In which branch are these patches?
> 
> They're based on security/next as was a while ago.  If you look here:
> 
> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git
> 
> you can see five branches that form a chain: keys-next, keys-rsa, keys-sig,
> keys-trust and keys-blacklist.  keys-sig is this set.

Thanks, that works.

Mimi

Re: [RFC PATCH 0/7] KEYS: Adjust public key signature handling

[David Howells]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> These patches don't apply directly on top of linux-security.   Maybe
> they apply on top of an updated version of the  "X.509: Software public
> key subtype changes" patch set.   In which branch are these patches?

I've rebased my keys-sig branch on top of today's security/next and fixed the
patch description for the last patch to have a subject line.

David