[PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

Here's a set of patches that does the following:

 (1) Extracts both parts of an X.509 AuthorityKeyIdentifier (AKID) extension.
     We already extract the bit that can match the subjectKeyIdentifier (SKID)
     of the parent X.509 cert, but we currently ignore the bits that can match
     the issuer and serialNumber.

     Looks up an X.509 cert by issuer and serialNumber if those are provided in
     the AKID.  If the keyIdentifier is also provided, checks that the
     subjectKeyIdentifier of the cert found matches that also.

     If no issuer and serialNumber are provided in the AKID, looks up an X.509
     cert by SKID using the AKID keyIdentifier.

     This allows module signing to be done with certificates that don't have an
     SKID by which they can be looked up.

 (2) Makes use of the PKCS#7 facility to provide module signatures.

     sign-file is replaced with a program that generates a PKCS#7 message that
     has no X.509 certs embedded and that has detached data (the module
     content) and adds it onto the message with magic string and descriptor.

 (3) The PKCS#7 message (and matching X.509 cert) supply all the information
     that is needed to select the X.509 cert to be used to verify the signature
     by standard means (including selection of digest algorithm and public key
     algorithm).  No kernel-specific magic values are required.

Note that the revised sign-file program no longer supports the "-s <signature>"
option as I'm not sure what the best way to deal with this is.  Do we generate
a PKCS#7 cert from the signature given, or do we get given a PKCS#7 cert?  I
lean towards the latter.

They can be found here also:

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

These patches are based on the security tree's next branch.

David
---
David Howells (5):
      X.509: Extract both parts of the AuthorityKeyIdentifier
      X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
      PKCS#7: Allow detached data to be supplied for signature checking purposes
      MODSIGN: Provide a utility to append a PKCS#7 signature to a module
      MODSIGN: Use PKCS#7 messages as module signatures


 crypto/asymmetric_keys/Makefile           |    8 -
 crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
 crypto/asymmetric_keys/pkcs7_verify.c     |   81 ++++--
 crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
 crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
 crypto/asymmetric_keys/x509_parser.h      |    3 
 crypto/asymmetric_keys/x509_public_key.c  |   85 ++++--
 include/crypto/pkcs7.h                    |    3 
 include/crypto/public_key.h               |    4 
 init/Kconfig                              |    1 
 kernel/module_signing.c                   |  220 +++------------
 scripts/Makefile                          |    2 
 scripts/sign-file                         |  421 -----------------------------
 scripts/sign-file.c                       |  189 +++++++++++++
 14 files changed, 505 insertions(+), 699 deletions(-)
 create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
 delete mode 100755 scripts/sign-file
 create mode 100755 scripts/sign-file.c

[PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier

[David Howells]

Extract both parts of the AuthorityKeyIdentifier, not just the keyIdentifier,
as the second part can be used to match X.509 certificates by issuer and
serialNumber.

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

 crypto/asymmetric_keys/Makefile           |    8 +-
 crypto/asymmetric_keys/pkcs7_trust.c      |    4 -
 crypto/asymmetric_keys/pkcs7_verify.c     |   12 +-
 crypto/asymmetric_keys/x509_akid.asn1     |   35 +++++++
 crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++++++++++++++-----------
 crypto/asymmetric_keys/x509_parser.h      |    3 -
 crypto/asymmetric_keys/x509_public_key.c  |    8 +-
 7 files changed, 144 insertions(+), 68 deletions(-)
 create mode 100644 crypto/asymmetric_keys/x509_akid.asn1

diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index e47fcd9ac5e8..cd1406f9b14a 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -15,15 +15,21 @@ obj-$(CONFIG_PUBLIC_KEY_ALGO_RSA) += rsa.o
 obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o
 x509_key_parser-y := \
 	x509-asn1.o \
+	x509_akid-asn1.o \
 	x509_rsakey-asn1.o \
 	x509_cert_parser.o \
 	x509_public_key.o
 
-$(obj)/x509_cert_parser.o: $(obj)/x509-asn1.h $(obj)/x509_rsakey-asn1.h
+$(obj)/x509_cert_parser.o: \
+	$(obj)/x509-asn1.h \
+	$(obj)/x509_akid-asn1.h \
+	$(obj)/x509_rsakey-asn1.h
 $(obj)/x509-asn1.o: $(obj)/x509-asn1.c $(obj)/x509-asn1.h
+$(obj)/x509_akid-asn1.o: $(obj)/x509_akid-asn1.c $(obj)/x509_akid-asn1.h
 $(obj)/x509_rsakey-asn1.o: $(obj)/x509_rsakey-asn1.c $(obj)/x509_rsakey-asn1.h
 
 clean-files	+= x509-asn1.c x509-asn1.h
+clean-files	+= x509_akid-asn1.c x509_akid-asn1.h
 clean-files	+= x509_rsakey-asn1.c x509_rsakey-asn1.h
 
 #
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 1d29376072da..f802cf118053 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -85,8 +85,8 @@ 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->authority) {
-		key = x509_request_asymmetric_key(trust_keyring, last->authority,
+	if (last && last->auth_skid) {
+		key = x509_request_asymmetric_key(trust_keyring, last->auth_skid,
 						  false);
 		if (!IS_ERR(key)) {
 			x509 = last;
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index cd455450b069..5e956c5b9071 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -187,11 +187,11 @@ 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->authority)
+		if (x509->auth_skid)
 			pr_debug("- authkeyid %*phN\n",
-				 x509->authority->len, x509->authority->data);
+				 x509->auth_skid->len, x509->auth_skid->data);
 
-		if (!x509->authority ||
+		if (!x509->auth_skid ||
 		    strcmp(x509->subject, x509->issuer) == 0) {
 			/* If there's no authority certificate specified, then
 			 * the certificate must be self-signed and is the root
@@ -216,13 +216,13 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		 * list to see if the next one is there.
 		 */
 		pr_debug("- want %*phN\n",
-			 x509->authority->len, x509->authority->data);
+			 x509->auth_skid->len, x509->auth_skid->data);
 		for (p = pkcs7->certs; p; p = p->next) {
 			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))
+			if (asymmetric_key_id_same(p->skid, x509->auth_skid))
 				goto found_issuer;
 		}
 
@@ -338,8 +338,6 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
 		ret = x509_get_sig_params(x509);
 		if (ret < 0)
 			return ret;
-		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_akid.asn1 b/crypto/asymmetric_keys/x509_akid.asn1
new file mode 100644
index 000000000000..1a33231a75a8
--- /dev/null
+++ b/crypto/asymmetric_keys/x509_akid.asn1
@@ -0,0 +1,35 @@
+-- X.509 AuthorityKeyIdentifier
+-- rfc5280 section 4.2.1.1
+
+AuthorityKeyIdentifier ::= SEQUENCE {
+	keyIdentifier			[0] IMPLICIT KeyIdentifier		OPTIONAL,
+	authorityCertIssuer		[1] IMPLICIT GeneralNames		OPTIONAL,
+	authorityCertSerialNumber	[2] IMPLICIT CertificateSerialNumber	OPTIONAL
+	}
+
+KeyIdentifier ::= OCTET STRING ({ x509_akid_note_kid })
+
+CertificateSerialNumber ::= INTEGER ({ x509_akid_note_serial })
+
+GeneralNames ::= SEQUENCE OF GeneralName
+
+GeneralName ::= CHOICE {
+	otherName			[0] ANY,
+	rfc822Name			[1] IA5String,
+	dNSName				[2] IA5String,
+	x400Address			[3] ANY,
+	directoryName			[4] Name ({ x509_akid_note_name }),
+	ediPartyName			[5] ANY,
+	uniformResourceIdentifier	[6] IA5String,
+	iPAddress			[7] OCTET STRING,
+	registeredID			[8] OBJECT IDENTIFIER
+	}
+
+Name ::= SEQUENCE OF RelativeDistinguishedName
+
+RelativeDistinguishedName ::= SET OF AttributeValueAssertion
+
+AttributeValueAssertion ::= SEQUENCE {
+	attributeType		OBJECT IDENTIFIER ({ x509_note_OID }),
+	attributeValue		ANY ({ x509_extract_name_segment })
+	}
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index a668d90302d3..e9d6586fdf89 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -18,6 +18,7 @@
 #include "public_key.h"
 #include "x509_parser.h"
 #include "x509-asn1.h"
+#include "x509_akid-asn1.h"
 #include "x509_rsakey-asn1.h"
 
 struct x509_parse_context {
@@ -35,6 +36,10 @@ struct x509_parse_context {
 	u16		o_offset;		/* Offset of organizationName (O) */
 	u16		cn_offset;		/* Offset of commonName (CN) */
 	u16		email_offset;		/* Offset of emailAddress */
+	unsigned	raw_akid_size;
+	const void	*raw_akid;		/* Raw authorityKeyId in ASN.1 */
+	const void	*akid_raw_issuer;	/* Raw directoryName in authorityKeyId */
+	unsigned	akid_raw_issuer_size;
 };
 
 /*
@@ -48,7 +53,8 @@ void x509_free_certificate(struct x509_certificate *cert)
 		kfree(cert->subject);
 		kfree(cert->id);
 		kfree(cert->skid);
-		kfree(cert->authority);
+		kfree(cert->auth_id);
+		kfree(cert->auth_skid);
 		kfree(cert->sig.digest);
 		mpi_free(cert->sig.rsa.s);
 		kfree(cert);
@@ -85,6 +91,18 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 	if (ret < 0)
 		goto error_decode;
 
+	/* Decode the AuthorityKeyIdentifier */
+	if (ctx->raw_akid) {
+		pr_devel("AKID: %u %*phN\n",
+			 ctx->raw_akid_size, ctx->raw_akid_size, ctx->raw_akid);
+		ret = asn1_ber_decoder(&x509_akid_decoder, ctx,
+				       ctx->raw_akid, ctx->raw_akid_size);
+		if (ret < 0) {
+			pr_warn("Couldn't decode AuthKeyIdentifier\n");
+			goto error_decode;
+		}
+	}
+
 	/* Decode the public key */
 	ret = asn1_ber_decoder(&x509_rsakey_decoder, ctx,
 			       ctx->key, ctx->key_size);
@@ -422,7 +440,6 @@ int x509_process_extension(void *context, size_t hdrlen,
 	struct x509_parse_context *ctx = context;
 	struct asymmetric_key_id *kid;
 	const unsigned char *v = value;
-	int i;
 
 	pr_debug("Extension: %u\n", ctx->last_oid);
 
@@ -449,57 +466,8 @@ int x509_process_extension(void *context, size_t hdrlen,
 
 	if (ctx->last_oid == OID_authorityKeyIdentifier) {
 		/* Get hold of the CA key fingerprint */
-		if (ctx->cert->authority || vlen < 5)
-			return -EBADMSG;
-
-		/* Authority Key Identifier must be a Constructed SEQUENCE */
-		if (v[0] != (ASN1_SEQ | (ASN1_CONS << 5)))
-			return -EBADMSG;
-
-		/* Authority Key Identifier is not indefinite length */
-		if (unlikely(vlen == ASN1_INDEFINITE_LENGTH))
-			return -EBADMSG;
-
-		if (vlen < ASN1_INDEFINITE_LENGTH) {
-			/* Short Form length */
-			if (v[1] != vlen - 2 ||
-			    v[2] != SEQ_TAG_KEYID ||
-			    v[3] > vlen - 4)
-				return -EBADMSG;
-
-			vlen = v[3];
-			v += 4;
-		} else {
-			/* Long Form length */
-			size_t seq_len = 0;
-			size_t sub = v[1] - ASN1_INDEFINITE_LENGTH;
-
-			if (sub > 2)
-				return -EBADMSG;
-
-			/* calculate the length from subsequent octets */
-			v += 2;
-			for (i = 0; i < sub; i++) {
-				seq_len <<= 8;
-				seq_len |= v[i];
-			}
-
-			if (seq_len != vlen - 2 - sub ||
-			    v[sub] != SEQ_TAG_KEYID ||
-			    v[sub + 1] > vlen - 4 - sub)
-				return -EBADMSG;
-
-			vlen = v[sub + 1];
-			v += (sub + 2);
-		}
-
-		kid = asymmetric_key_generate_id(ctx->cert->raw_issuer,
-						 ctx->cert->raw_issuer_size,
-						 v, vlen);
-		if (IS_ERR(kid))
-			return PTR_ERR(kid);
-		pr_debug("authkeyid %*phN\n", kid->len, kid->data);
-		ctx->cert->authority = kid;
+		ctx->raw_akid = v;
+		ctx->raw_akid_size = vlen;
 		return 0;
 	}
 
@@ -569,3 +537,71 @@ int x509_note_not_after(void *context, size_t hdrlen,
 	struct x509_parse_context *ctx = context;
 	return x509_note_time(&ctx->cert->valid_to, hdrlen, tag, value, vlen);
 }
+
+/*
+ * Note a key identifier-based AuthorityKeyIdentifier
+ */
+int x509_akid_note_kid(void *context, size_t hdrlen,
+		       unsigned char tag,
+		       const void *value, size_t vlen)
+{
+	struct x509_parse_context *ctx = context;
+	struct asymmetric_key_id *kid;
+
+	pr_debug("AKID: keyid: %*phN\n", (int)vlen, value);
+
+	if (ctx->cert->auth_skid)
+		return 0;
+
+	kid = asymmetric_key_generate_id(ctx->cert->raw_issuer,
+					 ctx->cert->raw_issuer_size,
+					 value, vlen);
+	if (IS_ERR(kid))
+		return PTR_ERR(kid);
+	pr_debug("authkeyid %*phN\n", kid->len, kid->data);
+	ctx->cert->auth_skid = kid;
+	return 0;
+}
+
+/*
+ * Note a directoryName in an AuthorityKeyIdentifier
+ */
+int x509_akid_note_name(void *context, size_t hdrlen,
+			unsigned char tag,
+			const void *value, size_t vlen)
+{
+	struct x509_parse_context *ctx = context;
+
+	pr_debug("AKID: name: %*phN\n", (int)vlen, value);
+
+	ctx->akid_raw_issuer = value;
+	ctx->akid_raw_issuer_size = vlen;
+	return 0;
+}
+
+/*
+ * Note a serial number in an AuthorityKeyIdentifier
+ */
+int x509_akid_note_serial(void *context, size_t hdrlen,
+			  unsigned char tag,
+			  const void *value, size_t vlen)
+{
+	struct x509_parse_context *ctx = context;
+	struct asymmetric_key_id *kid;
+
+	pr_debug("AKID: serial: %*phN\n", (int)vlen, value);
+
+	if (!ctx->akid_raw_issuer || ctx->cert->auth_id)
+		return 0;
+
+	kid = asymmetric_key_generate_id(value,
+					 vlen,
+					 ctx->akid_raw_issuer,
+					 ctx->akid_raw_issuer_size);
+	if (IS_ERR(kid))
+		return PTR_ERR(kid);
+
+	pr_debug("authkeyid %*phN\n", kid->len, kid->data);
+	ctx->cert->auth_id = kid;
+	return 0;
+}
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 3dfe6b5d6f0b..223b72344060 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -21,7 +21,8 @@ struct x509_certificate {
 	char		*subject;		/* Name of certificate subject */
 	struct asymmetric_key_id *id;		/* Serial number + issuer */
 	struct asymmetric_key_id *skid;		/* Subject + subjectKeyId (optional) */
-	struct asymmetric_key_id *authority;	/* Authority key identifier (optional) */
+	struct asymmetric_key_id *auth_id;	/* CA AuthKeyId matching ->id (optional) */
+	struct asymmetric_key_id *auth_skid;	/* CA AuthKeyId matching ->skid (optional) */
 	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 a6c42031628e..a3d9ba999da5 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -214,10 +214,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
 	if (!trust_keyring)
 		return -EOPNOTSUPP;
 
-	if (ca_keyid && !asymmetric_key_id_partial(cert->authority, ca_keyid))
+	if (ca_keyid && !asymmetric_key_id_partial(cert->auth_skid, ca_keyid))
 		return -EPERM;
 
-	key = x509_request_asymmetric_key(trust_keyring, cert->authority,
+	key = x509_request_asymmetric_key(trust_keyring, cert->auth_skid,
 					  false);
 	if (!IS_ERR(key))  {
 		if (!use_builtin_keys
@@ -274,8 +274,8 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	cert->pub->id_type = PKEY_ID_X509;
 
 	/* Check the signature on the key if it appears to be self-signed */
-	if (!cert->authority ||
-	    asymmetric_key_id_same(cert->skid, cert->authority)) {
+	if (!cert->auth_skid ||
+	    asymmetric_key_id_same(cert->skid, cert->auth_skid)) {
 		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;

Re: [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier

[Vivek Goyal]

On Thu, Nov 20, 2014 at 04:54:03PM +0000, David Howells wrote:

[..]
> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
> index 3dfe6b5d6f0b..223b72344060 100644
> --- a/crypto/asymmetric_keys/x509_parser.h
> +++ b/crypto/asymmetric_keys/x509_parser.h
> @@ -21,7 +21,8 @@ struct x509_certificate {
>  	char		*subject;		/* Name of certificate subject */
>  	struct asymmetric_key_id *id;		/* Serial number + issuer */
>  	struct asymmetric_key_id *skid;		/* Subject + subjectKeyId (optional) */
> -	struct asymmetric_key_id *authority;	/* Authority key identifier (optional) */
> +	struct asymmetric_key_id *auth_id;	/* CA AuthKeyId matching ->id (optional) */
> +	struct asymmetric_key_id *auth_skid;	/* CA AuthKeyId matching ->skid (optional) */

A very minor nit. It might help if we put additional comment to explain what
auth_id and auth_skid are composed of (like other key ids).

auth_id /* akid issuer + akid serial */
auth_skid /* issuer + akid keyid */

Thanks
Vivek

Re: [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier

[Dmitry Kasatkin]

On 21/11/14 16:42, Vivek Goyal wrote:
> On Thu, Nov 20, 2014 at 04:54:03PM +0000, David Howells wrote:
>
> [..]
>> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
>> index 3dfe6b5d6f0b..223b72344060 100644
>> --- a/crypto/asymmetric_keys/x509_parser.h
>> +++ b/crypto/asymmetric_keys/x509_parser.h
>> @@ -21,7 +21,8 @@ struct x509_certificate {
>>  	char		*subject;		/* Name of certificate subject */
>>  	struct asymmetric_key_id *id;		/* Serial number + issuer */
>>  	struct asymmetric_key_id *skid;		/* Subject + subjectKeyId (optional) */
>> -	struct asymmetric_key_id *authority;	/* Authority key identifier (optional) */
>> +	struct asymmetric_key_id *auth_id;	/* CA AuthKeyId matching ->id (optional) */
>> +	struct asymmetric_key_id *auth_skid;	/* CA AuthKeyId matching ->skid (optional) */
> A very minor nit. It might help if we put additional comment to explain what
> auth_id and auth_skid are composed of (like other key ids).
>
> auth_id /* akid issuer + akid serial */
> auth_skid /* issuer + akid keyid */
>
> Thanks
> Vivek
>

Right,

David did not address this in his v2 patchset...

- Dmitry

Re: [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier

[David Howells]

Dmitry Kasatkin <d.kasatkin@samsung.com> wrote:

> >> -	struct asymmetric_key_id *authority;	/* Authority key identifier (optional) */
> >> +	struct asymmetric_key_id *auth_id;	/* CA AuthKeyId matching ->id (optional) */
> >> +	struct asymmetric_key_id *auth_skid;	/* CA AuthKeyId matching ->skid (optional) */
> > A very minor nit. It might help if we put additional comment to explain what
> > auth_id and auth_skid are composed of (like other key ids).
> >
> > auth_id /* akid issuer + akid serial */
> > auth_skid /* issuer + akid keyid */
> >
> > Thanks
> > Vivek
> >
> 
> Right,
> 
> David did not address this in his v2 patchset...

I decided against changing them on the basis that I'd prefer to show what they
match over the way they are fabricated.  The id and skid members do show how
they are fabricated.  If you really want, I can show both - but my thought is
that if you look at how AuthorityKeyIdentifier is constructed, you can work it
out reasonably easily.

David

Re: [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier

[David Howells]

Vivek Goyal <vgoyal@redhat.com> wrote:

> A very minor nit. It might help if we put additional comment to explain what
> auth_id and auth_skid are composed of (like other key ids).

I thought it better to show what they match - ie. auth_id matches id and
auth_skid matches skid from the same structure.  The id and skid members show
their composition (and I should fix the comment on id so that the bits are the
right way round).

David

[PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[David Howells]

If an X.509 certificate has an AuthorityKeyIdentifier extension that provides
an issuer and serialNumber, then make it so that these are used in preference
to the keyIdentifier field also held therein for searching for the signing
certificate.

If both the issuer+serialNumber and the keyIdentifier are supplied, then the
certificate is looked up by the former but the latter is checked as well.  If
the latter doesn't match the subjectKeyIdentifier of the parent certificate,
EKEYREJECTED is returned.

This makes it possible to chain X.509 certificates based on the issuer and
serialNumber fields rather than on subjectKeyIdentifier.  This is necessary as
we are having to deal with keys that are represented by X.509 certificates
that lack a subjectKeyIdentifier.

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

 crypto/asymmetric_keys/pkcs7_trust.c     |   10 +++-
 crypto/asymmetric_keys/pkcs7_verify.c    |   47 +++++++++++++----
 crypto/asymmetric_keys/x509_public_key.c |   83 +++++++++++++++++++++---------
 include/crypto/public_key.h              |    3 +
 4 files changed, 102 insertions(+), 41 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index f802cf118053..11f8ca752e2a 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -54,7 +54,8 @@ 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->id,
+		key = x509_request_asymmetric_key(trust_keyring,
+						  x509->id, x509->skid,
 						  false);
 		if (!IS_ERR(key)) {
 			/* One of the X.509 certificates in the PKCS#7 message
@@ -85,8 +86,10 @@ 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->auth_skid) {
-		key = x509_request_asymmetric_key(trust_keyring, last->auth_skid,
+	if (last && (last->auth_id || last->auth_skid)) {
+		key = x509_request_asymmetric_key(trust_keyring,
+						  last->auth_id,
+						  last->auth_skid,
 						  false);
 		if (!IS_ERR(key)) {
 			x509 = last;
@@ -103,6 +106,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 	 */
 	key = x509_request_asymmetric_key(trust_keyring,
 					  sinfo->signing_cert_id,
+					  NULL,
 					  false);
 	if (!IS_ERR(key)) {
 		pr_devel("sinfo %u: Direct signer is key %x\n",
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 5e956c5b9071..667064daf66b 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -170,6 +170,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 				  struct pkcs7_signed_info *sinfo)
 {
 	struct x509_certificate *x509 = sinfo->signer, *p;
+	struct asymmetric_key_id *auth;
 	int ret;
 
 	kenter("");
@@ -187,11 +188,14 @@ 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->auth_id)
+			pr_debug("- authkeyid.id %*phN\n",
+				 x509->auth_id->len, x509->auth_id->data);
 		if (x509->auth_skid)
-			pr_debug("- authkeyid %*phN\n",
+			pr_debug("- authkeyid.skid %*phN\n",
 				 x509->auth_skid->len, x509->auth_skid->data);
 
-		if (!x509->auth_skid ||
+		if ((!x509->auth_id && !x509->auth_skid) ||
 		    strcmp(x509->subject, x509->issuer) == 0) {
 			/* If there's no authority certificate specified, then
 			 * the certificate must be self-signed and is the root
@@ -215,21 +219,42 @@ 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 %*phN\n",
-			 x509->auth_skid->len, x509->auth_skid->data);
-		for (p = pkcs7->certs; p; p = p->next) {
-			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->auth_skid))
-				goto found_issuer;
+		auth = x509->auth_id;
+		if (auth) {
+			pr_debug("- want %*phN\n", auth->len, auth->data);
+			for (p = pkcs7->certs; p; p = p->next) {
+				pr_debug("- cmp [%u] %*phN\n",
+					 p->index, p->id->len, p->id->data);
+				if (asymmetric_key_id_same(p->id, auth))
+					goto found_issuer_check_skid;
+			}
+		} else {
+			auth = x509->auth_skid;
+			pr_debug("- want %*phN\n", auth->len, auth->data);
+			for (p = pkcs7->certs; p; p = p->next) {
+				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, auth))
+					goto found_issuer;
+			}
 		}
 
 		/* We didn't find the root of this chain */
 		pr_debug("- top\n");
 		return 0;
 
+	found_issuer_check_skid:
+		/* We matched issuer + serialNumber, but if there's an
+		 * authKeyId.keyId, that must match the CA subjKeyId also.
+		 */
+		if (x509->auth_skid &&
+		    !asymmetric_key_id_same(p->skid, x509->auth_skid)) {
+			pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
+				sinfo->index, x509->index, p->index);
+			return -EKEYREJECTED;
+		}
 	found_issuer:
 		pr_debug("- subject %s\n", p->subject);
 		if (p->seen) {
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index a3d9ba999da5..9c1a3d60a42d 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -52,23 +52,36 @@ __setup("ca_keys=", ca_keys_setup);
 /**
  * x509_request_asymmetric_key - Request a key by X.509 certificate params.
  * @keyring: The keys to search.
- * @kid: The key ID.
+ * @id: The issuer & serialNumber to look for or NULL.
+ * @skid: The subjectKeyIdentifier to look for or NULL.
  * @partial: Use partial match if true, exact if false.
  *
- * 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.
+ * Find a key in the given keyring by identifier.  The preferred identifier is
+ * the issuer + serialNumber and the fallback identifier is the
+ * subjectKeyIdentifier.  If both are given, the lookup is by the former, but
+ * the latter must also match.
  */
 struct key *x509_request_asymmetric_key(struct key *keyring,
-					const struct asymmetric_key_id *kid,
+					const struct asymmetric_key_id *id,
+					const struct asymmetric_key_id *skid,
 					bool partial)
 {
-	key_ref_t key;
-	char *id, *p;
-
+	struct key *key;
+	key_ref_t ref;
+	const char *lookup;
+	char *req, *p;
+	int len;
+
+	lookup = id->data;
+	len = id->len;
+	if (!lookup) {
+		lookup = skid->data;
+		len = skid->len;
+	}
+	
 	/* Construct an identifier "id:<keyid>". */
-	p = id = kmalloc(2 + 1 + kid->len * 2 + 1, GFP_KERNEL);
-	if (!id)
+	p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
+	if (!req)
 		return ERR_PTR(-ENOMEM);
 
 	if (partial) {
@@ -79,32 +92,48 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
 		*p++ = 'x';
 	}
 	*p++ = ':';
-	p = bin2hex(p, kid->data, kid->len);
+	p = bin2hex(p, lookup, len);
 	*p = 0;
 
-	pr_debug("Look up: \"%s\"\n", id);
+	pr_debug("Look up: \"%s\"\n", req);
 
-	key = keyring_search(make_key_ref(keyring, 1),
-			     &key_type_asymmetric, id);
-	if (IS_ERR(key))
-		pr_debug("Request for key '%s' err %ld\n", id, PTR_ERR(key));
-	kfree(id);
+	ref = keyring_search(make_key_ref(keyring, 1),
+			     &key_type_asymmetric, req);
+	if (IS_ERR(ref))
+		pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
+	kfree(req);
 
-	if (IS_ERR(key)) {
-		switch (PTR_ERR(key)) {
+	if (IS_ERR(ref)) {
+		switch (PTR_ERR(ref)) {
 			/* Hide some search errors */
 		case -EACCES:
 		case -ENOTDIR:
 		case -EAGAIN:
 			return ERR_PTR(-ENOKEY);
 		default:
-			return ERR_CAST(key);
+			return ERR_CAST(ref);
+		}
+	}
+
+	key = key_ref_to_ptr(ref);
+	if (id && skid) {
+		const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
+		if (!kids->id[1]) {
+			pr_debug("issuer+serial match, but expected SKID missing\n");
+			goto reject;
+		}
+		if (!asymmetric_key_id_same(skid, kids->id[1])) {
+			pr_debug("issuer+serial match, but SKID does not\n");
+			goto reject;
 		}
 	}
+	
+	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key));
+	return key;
 
-	pr_devel("<==%s() = 0 [%x]\n", __func__,
-		 key_serial(key_ref_to_ptr(key)));
-	return key_ref_to_ptr(key);
+reject:
+	key_put(key);
+	return ERR_PTR(-EKEYREJECTED);
 }
 EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
 
@@ -217,7 +246,8 @@ static int x509_validate_trust(struct x509_certificate *cert,
 	if (ca_keyid && !asymmetric_key_id_partial(cert->auth_skid, ca_keyid))
 		return -EPERM;
 
-	key = x509_request_asymmetric_key(trust_keyring, cert->auth_skid,
+	key = x509_request_asymmetric_key(trust_keyring,
+					  cert->auth_id, cert->auth_skid,
 					  false);
 	if (!IS_ERR(key))  {
 		if (!use_builtin_keys
@@ -274,8 +304,9 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	cert->pub->id_type = PKEY_ID_X509;
 
 	/* Check the signature on the key if it appears to be self-signed */
-	if (!cert->auth_skid ||
-	    asymmetric_key_id_same(cert->skid, cert->auth_skid)) {
+	if ((!cert->auth_skid && !cert->auth_id) ||
+	    asymmetric_key_id_same(cert->skid, cert->auth_skid) ||
+	    asymmetric_key_id_same(cert->id, cert->auth_id)) {
 		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 54add2069901..b6f27a240856 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -101,7 +101,8 @@ extern int verify_signature(const struct key *key,
 
 struct asymmetric_key_id;
 extern struct key *x509_request_asymmetric_key(struct key *keyring,
-					       const struct asymmetric_key_id *kid,
+					       const struct asymmetric_key_id *id,
+					       const struct asymmetric_key_id *skid,
 					       bool partial);
 
 #endif /* _LINUX_PUBLIC_KEY_H */

Re: [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[Vivek Goyal]

On Thu, Nov 20, 2014 at 04:54:14PM +0000, David Howells wrote:

[..]
> @@ -215,21 +219,42 @@ 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 %*phN\n",
> -			 x509->auth_skid->len, x509->auth_skid->data);
> -		for (p = pkcs7->certs; p; p = p->next) {
> -			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->auth_skid))
> -				goto found_issuer;
> +		auth = x509->auth_id;
> +		if (auth) {
> +			pr_debug("- want %*phN\n", auth->len, auth->data);
> +			for (p = pkcs7->certs; p; p = p->next) {
> +				pr_debug("- cmp [%u] %*phN\n",
> +					 p->index, p->id->len, p->id->data);
> +				if (asymmetric_key_id_same(p->id, auth))
> +					goto found_issuer_check_skid;
> +			}
> +		} else {
> +			auth = x509->auth_skid;
> +			pr_debug("- want %*phN\n", auth->len, auth->data);
> +			for (p = pkcs7->certs; p; p = p->next) {
> +				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, auth))
> +					goto found_issuer;
> +			}
>  		}
>  
>  		/* We didn't find the root of this chain */
>  		pr_debug("- top\n");
>  		return 0;
>  
> +	found_issuer_check_skid:
> +		/* We matched issuer + serialNumber, but if there's an
> +		 * authKeyId.keyId, that must match the CA subjKeyId also.
> +		 */
> +		if (x509->auth_skid &&
> +		    !asymmetric_key_id_same(p->skid, x509->auth_skid)) {
> +			pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
> +				sinfo->index, x509->index, p->index);
> +			return -EKEYREJECTED;
> +		}

Hi David,

A minor nit.

pkcs7_verify_sig_chain() is getting big with multiple goto labels. Will
it make sense to introduce a helper function to see if cert B is authority
cert of cert A or not. And then we should be able to get rid of labels
like found_issuer_check_skid() and some of the inline code also go away.

Thanks
Vivek

Re: [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[Mimi Zohar]

Don't assume that the issuer & serialNumber are specified.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
crypto/asymmetric_keys/x509_public_key.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_public_key.c
b/crypto/asymmetric_keys/x509_public_key.c
index 9c1a3d6..510269f 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -9,6 +9,7 @@
  * 2 of the Licence, or (at your option) any later version.
  */

+#define DEBUG 1
#define pr_fmt(fmt) "X.509: "fmt
#include <linux/module.h>
#include <linux/kernel.h>
@@ -68,12 +69,14 @@ struct key *x509_request_asymmetric_key(struct key
*keyring,
{
struct key *key;
key_ref_t ref;
- const char *lookup;
+ const char *lookup = NULL;
char *req, *p;
- int len;
+ int len = 0;

- lookup = id->data;
- len = id->len;
+        if (id) {
+ lookup = id->data;
+ len = id->len;
+ }
if (!lookup) {
lookup = skid->data;
len = skid->len;
-- 
1.8.1.4

Re: [Keyrings] [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[Mimi Zohar]

Previous post was mangled.  Reposting ...
---

Don't assume that the issuer & serialNumber are specified.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 crypto/asymmetric_keys/x509_public_key.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 9c1a3d6..510269f 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -9,6 +9,7 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
+#define DEBUG 1
 #define pr_fmt(fmt) "X.509: "fmt
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -68,12 +69,14 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
 {
 	struct key *key;
 	key_ref_t ref;
-	const char *lookup;
+	const char *lookup = NULL;
 	char *req, *p;
-	int len;
+	int len = 0;
 
-	lookup = id->data;
-	len = id->len;
+        if (id) {
+		lookup = id->data;
+		len = id->len;
+	}
 	if (!lookup) {
 		lookup = skid->data;
 		len = skid->len;
-- 
1.8.1.4

Re: [Keyrings] [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[David Howells]

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

> Don't assume that the issuer & serialNumber are specified.

Bah...  I didn't assume, I just did it wrong.

> +#define DEBUG 1

Shouldn't be in the patch.

> +        if (id) {

Tab, not spaces.

I'll fix these up and merge your patch into mine to avoid the issues coming
out in bisection.

David

Re: [Keyrings] [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[Mimi Zohar]

On Mon, 2014-11-24 at 16:55 +0000, David Howells wrote: 
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > Don't assume that the issuer & serialNumber are specified.
> 
> Bah...  I didn't assume, I just did it wrong.
> 
> > +#define DEBUG 1
> 
> Shouldn't be in the patch.
> 
> > +        if (id) {
> 
> Tab, not spaces.
> 
> I'll fix these up and merge your patch into mine to avoid the issues coming
> out in bisection.

Thanks!

Mimi

Re: [Keyrings] [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[David Howells]

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

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

> +        if (id) {
> +		lookup = id->data;
> +		len = id->len;
> +	}
>  	if (!lookup) {

The last line here can then just be replaced by an else-clause.

David

Re: [Keyrings] [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[Mimi Zohar]

On Mon, 2014-11-24 at 16:58 +0000, David Howells wrote: 
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > +        if (id) {
> > +		lookup = id->data;
> > +		len = id->len;
> > +	}
> >  	if (!lookup) {
> 
> The last line here can then just be replaced by an else-clause.

I would assume so, but the original code looked like "id->data" could
have been null.

Mimi

Re: [Keyrings] [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier

[David Howells]

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

> I would assume so, but the original code looked like "id->data" could
> have been null.

It can't be - it's an embedded array.

David

[PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes

[David Howells]

It is possible for a PKCS#7 message to have detached data.  However, to verify
the signatures on a PKCS#7 message, we have to be able to digest the data.
Provide a function to supply that data.  An error is given if the PKCS#7
message included embedded data.

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

 crypto/asymmetric_keys/pkcs7_verify.c |   26 ++++++++++++++++++++++++++
 include/crypto/pkcs7.h                |    3 +++
 2 files changed, 29 insertions(+)

diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 667064daf66b..c6ee41bfda6e 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -382,3 +382,29 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
 	return enopkg;
 }
 EXPORT_SYMBOL_GPL(pkcs7_verify);
+
+/**
+ * pkcs7_supply_detached_data - Supply the data needed to verify a PKCS#7 message
+ * @pkcs7: The PKCS#7 message
+ * @data: The data to be verified
+ * @datalen: The amount of data
+ *
+ * Supply the detached data needed to verify a PKCS#7 message.  Note that no
+ * attempt to retain/pin the data is made.  That is left to the caller.  The
+ * data will not be modified by pkcs7_verify() and will not be freed when the
+ * PKCS#7 message is freed.
+ *
+ * Returns -EINVAL if data is already supplied in the message, 0 otherwise.
+ */
+int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
+			       const void *data, size_t datalen)
+{
+	if (pkcs7->data) {
+		pr_debug("Data already supplied\n");
+		return -EINVAL;
+	}
+	pkcs7->data = data;
+	pkcs7->data_len = datalen;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pkcs7_supply_detached_data);
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 691c79172a26..e235ab4957ee 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -34,3 +34,6 @@ extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
  * pkcs7_verify.c
  */
 extern int pkcs7_verify(struct pkcs7_message *pkcs7);
+
+extern int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
+				      const void *data, size_t datalen);

Re: [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes

[Mimi Zohar]

On Thu, 2014-11-20 at 16:54 +0000, David Howells wrote: 
> It is possible for a PKCS#7 message to have detached data.  However, to verify
> the signatures on a PKCS#7 message, we have to be able to digest the data.
> Provide a function to supply that data.  An error is given if the PKCS#7
> message included embedded data.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

Ok, PCKS#7 supports detached data.  I assume this is not needed for
kernel modules.  What is the motivation for adding this support to the
kernel?

Mimi

> ---
> 
>  crypto/asymmetric_keys/pkcs7_verify.c |   26 ++++++++++++++++++++++++++
>  include/crypto/pkcs7.h                |    3 +++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> index 667064daf66b..c6ee41bfda6e 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -382,3 +382,29 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
>  	return enopkg;
>  }
>  EXPORT_SYMBOL_GPL(pkcs7_verify);
> +
> +/**
> + * pkcs7_supply_detached_data - Supply the data needed to verify a PKCS#7 message
> + * @pkcs7: The PKCS#7 message
> + * @data: The data to be verified
> + * @datalen: The amount of data
> + *
> + * Supply the detached data needed to verify a PKCS#7 message.  Note that no
> + * attempt to retain/pin the data is made.  That is left to the caller.  The
> + * data will not be modified by pkcs7_verify() and will not be freed when the
> + * PKCS#7 message is freed.
> + *
> + * Returns -EINVAL if data is already supplied in the message, 0 otherwise.
> + */
> +int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
> +			       const void *data, size_t datalen)
> +{
> +	if (pkcs7->data) {
> +		pr_debug("Data already supplied\n");
> +		return -EINVAL;
> +	}
> +	pkcs7->data = data;
> +	pkcs7->data_len = datalen;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pkcs7_supply_detached_data);
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 691c79172a26..e235ab4957ee 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -34,3 +34,6 @@ extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
>   * pkcs7_verify.c
>   */
>  extern int pkcs7_verify(struct pkcs7_message *pkcs7);
> +
> +extern int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
> +				      const void *data, size_t datalen);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes

[David Howells]

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

> Ok, PCKS#7 supports detached data.  I assume this is not needed for
> kernel modules.  What is the motivation for adding this support to the
> kernel?

See patch #5.  I should probably note that in the commit message.

David

Re: [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes

[Mimi Zohar]

On Mon, 2014-11-24 at 12:48 +0000, David Howells wrote: 
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > Ok, PCKS#7 supports detached data.  I assume this is not needed for
> > kernel modules.  What is the motivation for adding this support to the
> > kernel?
> 
> See patch #5.  I should probably note that in the commit message.

This patch set does not change the syscall.  The signature is still
appended to the kernel module.  In fact, the call from
kernel/module_signing.c: mod_verify_pkcs7() calls
pkcs7_supply_detached_data() with a pointer to the module and the module
length to set the data field. pkcs7_supply_detached_data() would not be
defined or exported, unless it was going to be called elsewhere.  How
else are you planning on using pkcs7_supply_detached_data()? 

Mimi

Re: [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes

[David Howells]

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

> > > Ok, PCKS#7 supports detached data.  I assume this is not needed for
> > > kernel modules.  What is the motivation for adding this support to the
> > > kernel?
> > 
> > See patch #5.  I should probably note that in the commit message.
> 
> This patch set does not change the syscall.  The signature is still
> appended to the kernel module.

Ummm, yes, so?

> In fact, the call from kernel/module_signing.c: mod_verify_pkcs7() calls
> pkcs7_supply_detached_data() with a pointer to the module and the module
> length to set the data field. pkcs7_supply_detached_data() would not be
> defined or exported, unless it was going to be called elsewhere.  How else
> are you planning on using pkcs7_supply_detached_data()?

Is your point that I exported it unnecessarily?  That I have now fixed.

David

Re: [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes

[Mimi Zohar]

On Mon, 2014-11-24 at 14:41 +0000, David Howells wrote: 
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > > > Ok, PCKS#7 supports detached data.  I assume this is not needed for
> > > > kernel modules.  What is the motivation for adding this support to the
> > > > kernel?
> > > 
> > > See patch #5.  I should probably note that in the commit message.
> > 
> > This patch set does not change the syscall.  The signature is still
> > appended to the kernel module.
> 
> Ummm, yes, so?
> 
> > In fact, the call from kernel/module_signing.c: mod_verify_pkcs7() calls
> > pkcs7_supply_detached_data() with a pointer to the module and the module
> > length to set the data field. pkcs7_supply_detached_data() would not be
> > defined or exported, unless it was going to be called elsewhere.  How else
> > are you planning on using pkcs7_supply_detached_data()?
> 
> Is your point that I exported it unnecessarily?  That I have now fixed.

No, it was more a comment on the function name
pkcs7_supply_detached_data() being a bit weird, as the signature is
appended to the data.

Mimi

Re: [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes

[David Howells]

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

> No, it was more a comment on the function name
> pkcs7_supply_detached_data() being a bit weird, as the signature is
> appended to the data.

>From the point of view of the PKCS#7 message it's detached.

David

[PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module

[David Howells]

Provide a utility that:

 (1) Digests a module using the specified hash algorithm (typically sha256).

     [The digest can be dumped into a file by passing the '-d' flag]

 (2) Generates a PKCS#7 message that:

     (a) Has detached data (ie. the module content).

     (b) Is signed with the specified private key.

     (c) Refers to the specified X.509 certificate.

     (d) Has an empty X.509 certificate list.

     [The PKCS#7 message can be dumped into a file by passing the '-p' flag]

 (3) Generates a signed module by concatenating the old module, the PKCS#7
     message, a descriptor and a magic string.  The descriptor contains the
     size of the PKCS#7 message and indicates the id_type as PKEY_ID_PKCS7.

 (4) Either writes the signed module to the specified destination or renames
     it over the source module.

This allows module signing to reuse the PKCS#7 handling code that was added
for PE file parsing for signed kexec.

Note that the utility is written in C and must be linked against the OpenSSL
crypto library.

Note further that I have temporarily dropped support for handling externally
created signatures until we can work out the best way to do those.  Hopefully,
whoever creates the signature can give me a PKCS#7 certificate.

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

 include/crypto/public_key.h |    1 
 scripts/sign-file.c         |  189 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 190 insertions(+)
 create mode 100755 scripts/sign-file.c

diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index b6f27a240856..fda097e079a4 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -33,6 +33,7 @@ extern const struct public_key_algorithm *pkey_algo[PKEY_ALGO__LAST];
 enum pkey_id_type {
 	PKEY_ID_PGP,		/* OpenPGP generated key ID */
 	PKEY_ID_X509,		/* X.509 arbitrary subjectKeyIdentifier */
+	PKEY_ID_PKCS7,		/* Signature in PKCS#7 message */
 	PKEY_ID_TYPE__LAST
 };
 
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
new file mode 100755
index 000000000000..3f9bedbd185f
--- /dev/null
+++ b/scripts/sign-file.c
@@ -0,0 +1,189 @@
+/* Sign a module file using the given key.
+ *
+ * Copyright (C) 2014 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <string.h>
+#include <getopt.h>
+#include <err.h>
+#include <arpa/inet.h>
+#include <openssl/bio.h>
+#include <openssl/evp.h>
+#include <openssl/pem.h>
+#include <openssl/pkcs7.h>
+#include <openssl/err.h>
+
+struct module_signature {
+	uint8_t		algo;		/* Public-key crypto algorithm [0] */
+	uint8_t		hash;		/* Digest algorithm [0] */
+	uint8_t		id_type;	/* Key identifier type [PKEY_ID_PKCS7] */
+	uint8_t		signer_len;	/* Length of signer's name [0] */
+	uint8_t		key_id_len;	/* Length of key identifier [0] */
+	uint8_t		__pad[3];
+	uint32_t	sig_len;	/* Length of signature data */
+};
+
+#define PKEY_ID_PKCS7 2
+
+static char magic_number[] = "~Module signature appended~\n";
+
+static __attribute__((noreturn))
+void format(void)
+{
+	fprintf(stderr,
+		"Usage: scripts/sign-file [-dp] <hash algo> <key> <x509> <module> [<dest>]\n");
+	exit(2);
+}
+
+static void display_openssl_errors(int l)
+{
+	const char *file;
+	char buf[120];
+	int e, line;
+
+	if (ERR_peek_error() == 0)
+		return;
+	fprintf(stderr, "At main.c:%d:\n", l);
+
+	while ((e = ERR_get_error_line(&file, &line))) {
+		ERR_error_string(e, buf);
+		fprintf(stderr, "- SSL %s: %s:%d\n", buf, file, line);
+	}
+}
+
+
+#define ERR(cond, ...)				  \
+	do {					  \
+		bool __cond = (cond);		  \
+		display_openssl_errors(__LINE__); \
+		if (__cond) {			  \
+			err(1, ## __VA_ARGS__);	  \
+		}				  \
+	} while(0)
+
+int main(int argc, char **argv)
+{
+	struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
+	char *hash_algo = NULL;
+	char *private_key_name, *x509_name, *module_name, *dest_name;
+	bool save_pkcs7 = false, replace_orig;
+	unsigned char buf[4096];
+	unsigned long module_size, pkcs7_size;
+	const EVP_MD *digest_algo;
+	EVP_PKEY *private_key;
+	PKCS7 *pkcs7;
+	X509 *x509;
+	BIO *b, *bd, *bm;
+	int opt, n;
+
+	do {
+		opt = getopt(argc, argv, "dp");
+		switch (opt) {
+		case 'p': save_pkcs7 = true; break;
+		case -1: break;
+		default: format();
+		}
+	} while (opt != -1);
+
+	argc -= optind;
+	argv += optind;
+	if (argc < 4 || argc > 5)
+		format();
+
+	hash_algo = argv[0];
+	private_key_name = argv[1];
+	x509_name = argv[2];
+	module_name = argv[3];
+	if (argc == 5) {
+		dest_name = argv[4];
+		replace_orig = false;
+	} else {
+		ERR(asprintf(&dest_name, "%s.~signed~", module_name) < 0,
+		    "asprintf");
+		replace_orig = true;
+	}
+
+	ERR_load_crypto_strings();
+	ERR_clear_error();
+
+	/* Read the private key and the X.509 cert the PKCS#7 message
+	 * will point to.
+	 */
+	b = BIO_new_file(private_key_name, "rb");
+	ERR(!b, "%s", private_key_name);
+        private_key = PEM_read_bio_PrivateKey(b, NULL, NULL, NULL);
+	BIO_free(b);
+
+	b = BIO_new_file(x509_name, "rb");
+	ERR(!b, "%s", x509_name);
+        x509 = PEM_read_bio_X509(b, NULL, NULL, NULL);
+	BIO_free(b);
+
+	/* Open the destination file now so that we can shovel the module data
+	 * across as we read it.
+	 */
+	bd = BIO_new_file(dest_name, "wb");
+	ERR(!bd, dest_name);
+
+	/* Digest the module data. */
+	OpenSSL_add_all_digests();
+	display_openssl_errors(__LINE__);
+	digest_algo = EVP_get_digestbyname(hash_algo);
+	ERR(!digest_algo, "EVP_get_digestbyname");
+
+	bm = BIO_new_file(module_name, "rb");
+	ERR(!bm, "%s", module_name);
+
+	/* Load the PKCS#7 message from the digest buffer. */
+	pkcs7 = PKCS7_sign(NULL, NULL, NULL, NULL,
+			   PKCS7_NOCERTS | PKCS7_PARTIAL | PKCS7_BINARY | PKCS7_DETACHED | PKCS7_STREAM);
+	ERR(!pkcs7, "PKCS7_sign");
+
+	ERR(PKCS7_sign_add_signer(pkcs7, x509, private_key, digest_algo, PKCS7_NOCERTS | PKCS7_BINARY) < 0,
+	    "PKCS7_sign_add_signer");
+	ERR(PKCS7_final(pkcs7, bm, PKCS7_NOCERTS | PKCS7_BINARY) < 0,
+	    "PKCS7_final");
+
+	if (save_pkcs7) {
+		char *pkcs7_name;
+
+		ERR(asprintf(&pkcs7_name, "%s.pkcs7", module_name) < 0, "asprintf");
+		b = BIO_new_file(pkcs7_name, "wb");
+		ERR(!b, pkcs7_name);
+		ERR(i2d_PKCS7_bio_stream(b, pkcs7, NULL, 0) < 0, pkcs7_name);
+		BIO_free(b);
+	}
+
+	/* Append the marker and the PKCS#7 message to the destination file */
+	ERR(BIO_reset(bm) < 0, module_name);
+	while ((n = BIO_read(bm, buf, sizeof(buf))),
+	       n > 0) {
+		ERR(BIO_write(bd, buf, n) < 0, dest_name);
+	}
+	ERR(n < 0, module_name);
+	module_size = BIO_number_written(bd);
+
+	ERR(i2d_PKCS7_bio_stream(bd, pkcs7, NULL, 0) < 0, dest_name);
+	pkcs7_size = BIO_number_written(bd) - module_size;
+	sig_info.sig_len = htonl(pkcs7_size);
+	ERR(BIO_write(bd, &sig_info, sizeof(sig_info)) < 0, dest_name);
+	ERR(BIO_write(bd, magic_number, sizeof(magic_number) - 1) < 0, dest_name);
+
+	ERR(BIO_free(bd) < 0, dest_name);
+
+	/* Finally, if we're signing in place, replace the original. */
+	if (replace_orig)
+		ERR(rename(dest_name, module_name) < 0, dest_name);
+
+	return 0;
+}

[PATCH 5/5] MODSIGN: Use PKCS#7 messages as module signatures

[David Howells]

Move to using PKCS#7 messages as module signatures because:

 (1) We have to be able to support the use of X.509 certificates that don't
     have a subjKeyId set.  We're currently relying on this to look up the
     X.509 certificate in the trusted keyring list.

 (2) PKCS#7 message signed information blocks have a field that supplies the
     data required to match with the X.509 certificate that signed it.

 (3) The PKCS#7 certificate carries fields that specify the digest algorithm
     used to generate the signature in a standardised way and the X.509
     certificates specify the public key algorithm in a standardised way - so
     we don't need our own methods of specifying these.

 (4) We now have PKCS#7 message support in the kernel for signed kexec purposes
     and we can make use of this.

To make this work, the old sign-file script has been replaced with a program
that needs compiling in a previous patch.  The rules to build it are added
here.

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

 init/Kconfig            |    1 
 kernel/module_signing.c |  220 +++++--------------------
 scripts/Makefile        |    2 
 scripts/sign-file       |  421 -----------------------------------------------
 4 files changed, 47 insertions(+), 597 deletions(-)
 delete mode 100755 scripts/sign-file

diff --git a/init/Kconfig b/init/Kconfig
index 80a6907f91c5..e6f418b97bdd 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1840,6 +1840,7 @@ config MODULE_SIG
 	select ASN1
 	select OID_REGISTRY
 	select X509_CERTIFICATE_PARSER
+	select PKCS7_MESSAGE_PARSER
 	help
 	  Check modules for valid signatures upon load: the signature
 	  is simply appended to the module. For more information see
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index be5b8fac4bd0..8eb20cc66b39 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,10 +11,9 @@
 
 #include <linux/kernel.h>
 #include <linux/err.h>
-#include <crypto/public_key.h>
-#include <crypto/hash.h>
-#include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
+#include <crypto/public_key.h>
+#include <crypto/pkcs7.h>
 #include "module-internal.h"
 
 /*
@@ -28,157 +27,53 @@
  *	- Information block
  */
 struct module_signature {
-	u8	algo;		/* Public-key crypto algorithm [enum pkey_algo] */
-	u8	hash;		/* Digest algorithm [enum hash_algo] */
-	u8	id_type;	/* Key identifier type [enum pkey_id_type] */
-	u8	signer_len;	/* Length of signer's name */
-	u8	key_id_len;	/* Length of key identifier */
+	u8	algo;		/* Public-key crypto algorithm [0] */
+	u8	hash;		/* Digest algorithm [0] */
+	u8	id_type;	/* Key identifier type [PKEY_ID_PKCS7] */
+	u8	signer_len;	/* Length of signer's name [0] */
+	u8	key_id_len;	/* Length of key identifier [0] */
 	u8	__pad[3];
 	__be32	sig_len;	/* Length of signature data */
 };
 
 /*
- * Digest the module contents.
+ * Verify a PKCS#7-based signature on a module.
  */
-static struct public_key_signature *mod_make_digest(enum hash_algo hash,
-						    const void *mod,
-						    unsigned long modlen)
+static int mod_verify_pkcs7(const void *mod, unsigned long modlen,
+			    const void *raw_pkcs7, size_t pkcs7_len)
 {
-	struct public_key_signature *pks;
-	struct crypto_shash *tfm;
-	struct shash_desc *desc;
-	size_t digest_size, desc_size;
+	struct pkcs7_message *pkcs7;
+	bool trusted;
 	int ret;
 
-	pr_devel("==>%s()\n", __func__);
-	
-	/* Allocate the hashing algorithm we're going to need and find out how
-	 * big the hash operational data will be.
-	 */
-	tfm = crypto_alloc_shash(hash_algo_name[hash], 0, 0);
-	if (IS_ERR(tfm))
-		return (PTR_ERR(tfm) == -ENOENT) ? ERR_PTR(-ENOPKG) : ERR_CAST(tfm);
-
-	desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
-	digest_size = crypto_shash_digestsize(tfm);
-
-	/* We allocate the hash operational data storage on the end of our
-	 * context data and the digest output buffer on the end of that.
-	 */
-	ret = -ENOMEM;
-	pks = kzalloc(digest_size + sizeof(*pks) + desc_size, GFP_KERNEL);
-	if (!pks)
-		goto error_no_pks;
-
-	pks->pkey_hash_algo	= hash;
-	pks->digest		= (u8 *)pks + sizeof(*pks) + desc_size;
-	pks->digest_size	= digest_size;
-
-	desc = (void *)pks + sizeof(*pks);
-	desc->tfm   = tfm;
-	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
-
-	ret = crypto_shash_init(desc);
+	pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
+	if (IS_ERR(pkcs7))
+		return PTR_ERR(pkcs7);
+
+	/* The data should be detached - so we need to supply it. */
+	if (pkcs7_supply_detached_data(pkcs7, mod, modlen) < 0) {
+		pr_err("PKCS#7 signature with non-detached data\n");
+		ret = -EBADMSG;
+		goto error;
+	}
+
+	ret = pkcs7_verify(pkcs7);
 	if (ret < 0)
 		goto error;
 
-	ret = crypto_shash_finup(desc, mod, modlen, pks->digest);
+	ret = pkcs7_validate_trust(pkcs7, system_trusted_keyring, &trusted);
 	if (ret < 0)
 		goto error;
 
-	crypto_free_shash(tfm);
-	pr_devel("<==%s() = ok\n", __func__);
-	return pks;
+	if (!trusted) {
+		pr_err("PKCS#7 signature not signed with a trusted key\n");
+		ret = -ENOKEY;
+	}
 
 error:
-	kfree(pks);
-error_no_pks:
-	crypto_free_shash(tfm);
+	pkcs7_free_message(pkcs7);
 	pr_devel("<==%s() = %d\n", __func__, ret);
-	return ERR_PTR(ret);
-}
-
-/*
- * Extract an MPI array from the signature data.  This represents the actual
- * signature.  Each raw MPI is prefaced by a BE 2-byte value indicating the
- * size of the MPI in bytes.
- *
- * RSA signatures only have one MPI, so currently we only read one.
- */
-static int mod_extract_mpi_array(struct public_key_signature *pks,
-				 const void *data, size_t len)
-{
-	size_t nbytes;
-	MPI mpi;
-
-	if (len < 3)
-		return -EBADMSG;
-	nbytes = ((const u8 *)data)[0] << 8 | ((const u8 *)data)[1];
-	data += 2;
-	len -= 2;
-	if (len != nbytes)
-		return -EBADMSG;
-
-	mpi = mpi_read_raw_data(data, nbytes);
-	if (!mpi)
-		return -ENOMEM;
-	pks->mpi[0] = mpi;
-	pks->nr_mpi = 1;
-	return 0;
-}
-
-/*
- * Request an asymmetric key.
- */
-static struct key *request_asymmetric_key(const char *signer, size_t signer_len,
-					  const u8 *key_id, size_t key_id_len)
-{
-	key_ref_t key;
-	size_t i;
-	char *id, *q;
-
-	pr_devel("==>%s(,%zu,,%zu)\n", __func__, signer_len, key_id_len);
-
-	/* Construct an identifier. */
-	id = kmalloc(signer_len + 2 + key_id_len * 2 + 1, GFP_KERNEL);
-	if (!id)
-		return ERR_PTR(-ENOKEY);
-
-	memcpy(id, signer, signer_len);
-
-	q = id + signer_len;
-	*q++ = ':';
-	*q++ = ' ';
-	for (i = 0; i < key_id_len; i++) {
-		*q++ = hex_asc[*key_id >> 4];
-		*q++ = hex_asc[*key_id++ & 0x0f];
-	}
-
-	*q = 0;
-
-	pr_debug("Look up: \"%s\"\n", id);
-
-	key = keyring_search(make_key_ref(system_trusted_keyring, 1),
-			     &key_type_asymmetric, id);
-	if (IS_ERR(key))
-		pr_warn("Request for unknown module key '%s' err %ld\n",
-			id, PTR_ERR(key));
-	kfree(id);
-
-	if (IS_ERR(key)) {
-		switch (PTR_ERR(key)) {
-			/* Hide some search errors */
-		case -EACCES:
-		case -ENOTDIR:
-		case -EAGAIN:
-			return ERR_PTR(-ENOKEY);
-		default:
-			return ERR_CAST(key);
-		}
-	}
-
-	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
-	return key_ref_to_ptr(key);
+	return ret;
 }
 
 /*
@@ -186,12 +81,8 @@ static struct key *request_asymmetric_key(const char *signer, size_t signer_len,
  */
 int mod_verify_sig(const void *mod, unsigned long *_modlen)
 {
-	struct public_key_signature *pks;
 	struct module_signature ms;
-	struct key *key;
-	const void *sig;
 	size_t modlen = *_modlen, sig_len;
-	int ret;
 
 	pr_devel("==>%s(,%zu)\n", __func__, modlen);
 
@@ -205,46 +96,23 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
 	if (sig_len >= modlen)
 		return -EBADMSG;
 	modlen -= sig_len;
-	if ((size_t)ms.signer_len + ms.key_id_len >= modlen)
-		return -EBADMSG;
-	modlen -= (size_t)ms.signer_len + ms.key_id_len;
-
 	*_modlen = modlen;
-	sig = mod + modlen;
-
-	/* For the moment, only support RSA and X.509 identifiers */
-	if (ms.algo != PKEY_ALGO_RSA ||
-	    ms.id_type != PKEY_ID_X509)
-		return -ENOPKG;
 
-	if (ms.hash >= PKEY_HASH__LAST ||
-	    !hash_algo_name[ms.hash])
+	if (ms.id_type != PKEY_ID_PKCS7) {
+		pr_err("Module is not signed with expected PKCS#7 message\n");
 		return -ENOPKG;
-
-	key = request_asymmetric_key(sig, ms.signer_len,
-				     sig + ms.signer_len, ms.key_id_len);
-	if (IS_ERR(key))
-		return PTR_ERR(key);
-
-	pks = mod_make_digest(ms.hash, mod, modlen);
-	if (IS_ERR(pks)) {
-		ret = PTR_ERR(pks);
-		goto error_put_key;
 	}
 
-	ret = mod_extract_mpi_array(pks, sig + ms.signer_len + ms.key_id_len,
-				    sig_len);
-	if (ret < 0)
-		goto error_free_pks;
-
-	ret = verify_signature(key, pks);
-	pr_devel("verify_signature() = %d\n", ret);
+	if (ms.algo != 0 ||
+	    ms.hash != 0 ||
+	    ms.signer_len != 0 ||
+	    ms.key_id_len != 0 ||
+	    ms.__pad[0] != 0 ||
+	    ms.__pad[1] != 0 ||
+	    ms.__pad[2] != 0) {
+		pr_err("PKCS#7 signature info has unexpected non-zero params\n");
+		return -EBADMSG;
+	}
 
-error_free_pks:
-	mpi_free(pks->rsa.s);
-	kfree(pks);
-error_put_key:
-	key_put(key);
-	pr_devel("<==%s() = %d\n", __func__, ret);
-	return ret;	
+	return mod_verify_pkcs7(mod, modlen, mod + modlen, sig_len);
 }
diff --git a/scripts/Makefile b/scripts/Makefile
index 72902b5f2721..719311b7bd46 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -16,9 +16,11 @@ hostprogs-$(CONFIG_VT)           += conmakehash
 hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
 hostprogs-$(CONFIG_BUILDTIME_EXTABLE_SORT) += sortextable
 hostprogs-$(CONFIG_ASN1)	 += asn1_compiler
+hostprogs-$(CONFIG_MODULE_SIG)	 += sign-file
 
 HOSTCFLAGS_sortextable.o = -I$(srctree)/tools/include
 HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
+HOSTLOADLIBES_sign-file = -lcrypto
 
 always		:= $(hostprogs-y) $(hostprogs-m)
 
diff --git a/scripts/sign-file b/scripts/sign-file
deleted file mode 100755
index 2b7c4484d46c..000000000000
--- a/scripts/sign-file
+++ /dev/null
@@ -1,421 +0,0 @@
-#!/usr/bin/perl -w
-#
-# Sign a module file using the given key.
-#
-
-my $USAGE =
-"Usage: scripts/sign-file [-v] <hash algo> <key> <x509> <module> [<dest>]\n" .
-"       scripts/sign-file [-v] -s <raw sig> <hash algo> <x509> <module> [<dest>]\n";
-
-use strict;
-use FileHandle;
-use IPC::Open2;
-use Getopt::Std;
-
-my %opts;
-getopts('vs:', \%opts) or die $USAGE;
-my $verbose = $opts{'v'};
-my $signature_file = $opts{'s'};
-
-die $USAGE if ($#ARGV > 4);
-die $USAGE if (!$signature_file && $#ARGV < 3 || $signature_file && $#ARGV < 2);
-
-my $dgst = shift @ARGV;
-my $private_key;
-if (!$signature_file) {
-	$private_key = shift @ARGV;
-}
-my $x509 = shift @ARGV;
-my $module = shift @ARGV;
-my ($dest, $keep_orig);
-if (@ARGV) {
-	$dest = $ARGV[0];
-	$keep_orig = 1;
-} else {
-	$dest = $module . "~";
-}
-
-die "Can't read private key\n" if (!$signature_file && !-r $private_key);
-die "Can't read signature file\n" if ($signature_file && !-r $signature_file);
-die "Can't read X.509 certificate\n" unless (-r $x509);
-die "Can't read module\n" unless (-r $module);
-
-#
-# Function to read the contents of a file into a variable.
-#
-sub read_file($)
-{
-    my ($file) = @_;
-    my $contents;
-    my $len;
-
-    open(FD, "<$file") || die $file;
-    binmode FD;
-    my @st = stat(FD);
-    die $file if (!@st);
-    $len = read(FD, $contents, $st[7]) || die $file;
-    close(FD) || die $file;
-    die "$file: Wanted length ", $st[7], ", got ", $len, "\n"
-	if ($len != $st[7]);
-    return $contents;
-}
-
-###############################################################################
-#
-# First of all, we have to parse the X.509 certificate to find certain details
-# about it.
-#
-# We read the DER-encoded X509 certificate and parse it to extract the Subject
-# name and Subject Key Identifier.  Theis provides the data we need to build
-# the certificate identifier.
-#
-# The signer's name part of the identifier is fabricated from the commonName,
-# the organizationName or the emailAddress components of the X.509 subject
-# name.
-#
-# The subject key ID is used to select which of that signer's certificates
-# we're intending to use to sign the module.
-#
-###############################################################################
-my $x509_certificate = read_file($x509);
-
-my $UNIV = 0 << 6;
-my $APPL = 1 << 6;
-my $CONT = 2 << 6;
-my $PRIV = 3 << 6;
-
-my $CONS = 0x20;
-
-my $BOOLEAN	= 0x01;
-my $INTEGER	= 0x02;
-my $BIT_STRING	= 0x03;
-my $OCTET_STRING = 0x04;
-my $NULL	= 0x05;
-my $OBJ_ID	= 0x06;
-my $UTF8String	= 0x0c;
-my $SEQUENCE	= 0x10;
-my $SET		= 0x11;
-my $UTCTime	= 0x17;
-my $GeneralizedTime = 0x18;
-
-my %OIDs = (
-    pack("CCC", 85, 4, 3)	=> "commonName",
-    pack("CCC", 85, 4, 6)	=> "countryName",
-    pack("CCC", 85, 4, 10)	=> "organizationName",
-    pack("CCC", 85, 4, 11)	=> "organizationUnitName",
-    pack("CCCCCCCCC", 42, 134, 72, 134, 247, 13, 1, 1, 1) => "rsaEncryption",
-    pack("CCCCCCCCC", 42, 134, 72, 134, 247, 13, 1, 1, 5) => "sha1WithRSAEncryption",
-    pack("CCCCCCCCC", 42, 134, 72, 134, 247, 13, 1, 9, 1) => "emailAddress",
-    pack("CCC", 85, 29, 35)	=> "authorityKeyIdentifier",
-    pack("CCC", 85, 29, 14)	=> "subjectKeyIdentifier",
-    pack("CCC", 85, 29, 19)	=> "basicConstraints"
-);
-
-###############################################################################
-#
-# Extract an ASN.1 element from a string and return information about it.
-#
-###############################################################################
-sub asn1_extract($$@)
-{
-    my ($cursor, $expected_tag, $optional) = @_;
-
-    return [ -1 ]
-	if ($cursor->[1] == 0 && $optional);
-
-    die $x509, ": ", $cursor->[0], ": ASN.1 data underrun (elem ", $cursor->[1], ")\n"
-	if ($cursor->[1] < 2);
-
-    my ($tag, $len) = unpack("CC", substr(${$cursor->[2]}, $cursor->[0], 2));
-
-    if ($expected_tag != -1 && $tag != $expected_tag) {
-	return [ -1 ]
-	    if ($optional);
-	die $x509, ": ", $cursor->[0], ": ASN.1 unexpected tag (", $tag,
-	" not ", $expected_tag, ")\n";
-    }
-
-    $cursor->[0] += 2;
-    $cursor->[1] -= 2;
-
-    die $x509, ": ", $cursor->[0], ": ASN.1 long tag\n"
-	if (($tag & 0x1f) == 0x1f);
-    die $x509, ": ", $cursor->[0], ": ASN.1 indefinite length\n"
-	if ($len == 0x80);
-
-    if ($len > 0x80) {
-	my $l = $len - 0x80;
-	die $x509, ": ", $cursor->[0], ": ASN.1 data underrun (len len $l)\n"
-	    if ($cursor->[1] < $l);
-
-	if ($l == 0x1) {
-	    $len = unpack("C", substr(${$cursor->[2]}, $cursor->[0], 1));
-	} elsif ($l == 0x2) {
-	    $len = unpack("n", substr(${$cursor->[2]}, $cursor->[0], 2));
-	} elsif ($l == 0x3) {
-	    $len = unpack("C", substr(${$cursor->[2]}, $cursor->[0], 1)) << 16;
-	    $len = unpack("n", substr(${$cursor->[2]}, $cursor->[0] + 1, 2));
-	} elsif ($l == 0x4) {
-	    $len = unpack("N", substr(${$cursor->[2]}, $cursor->[0], 4));
-	} else {
-	    die $x509, ": ", $cursor->[0], ": ASN.1 element too long (", $l, ")\n";
-	}
-
-	$cursor->[0] += $l;
-	$cursor->[1] -= $l;
-    }
-
-    die $x509, ": ", $cursor->[0], ": ASN.1 data underrun (", $len, ")\n"
-	if ($cursor->[1] < $len);
-
-    my $ret = [ $tag, [ $cursor->[0], $len, $cursor->[2] ] ];
-    $cursor->[0] += $len;
-    $cursor->[1] -= $len;
-
-    return $ret;
-}
-
-###############################################################################
-#
-# Retrieve the data referred to by a cursor
-#
-###############################################################################
-sub asn1_retrieve($)
-{
-    my ($cursor) = @_;
-    my ($offset, $len, $data) = @$cursor;
-    return substr($$data, $offset, $len);
-}
-
-###############################################################################
-#
-# Roughly parse the X.509 certificate
-#
-###############################################################################
-my $cursor = [ 0, length($x509_certificate), \$x509_certificate ];
-
-my $cert = asn1_extract($cursor, $UNIV | $CONS | $SEQUENCE);
-my $tbs = asn1_extract($cert->[1], $UNIV | $CONS | $SEQUENCE);
-my $version = asn1_extract($tbs->[1], $CONT | $CONS | 0, 1);
-my $serial_number = asn1_extract($tbs->[1], $UNIV | $INTEGER);
-my $sig_type = asn1_extract($tbs->[1], $UNIV | $CONS | $SEQUENCE);
-my $issuer = asn1_extract($tbs->[1], $UNIV | $CONS | $SEQUENCE);
-my $validity = asn1_extract($tbs->[1], $UNIV | $CONS | $SEQUENCE);
-my $subject = asn1_extract($tbs->[1], $UNIV | $CONS | $SEQUENCE);
-my $key = asn1_extract($tbs->[1], $UNIV | $CONS | $SEQUENCE);
-my $issuer_uid = asn1_extract($tbs->[1], $CONT | $CONS | 1, 1);
-my $subject_uid = asn1_extract($tbs->[1], $CONT | $CONS | 2, 1);
-my $extension_list = asn1_extract($tbs->[1], $CONT | $CONS | 3, 1);
-
-my $subject_key_id = ();
-my $authority_key_id = ();
-
-#
-# Parse the extension list
-#
-if ($extension_list->[0] != -1) {
-    my $extensions = asn1_extract($extension_list->[1], $UNIV | $CONS | $SEQUENCE);
-
-    while ($extensions->[1]->[1] > 0) {
-	my $ext = asn1_extract($extensions->[1], $UNIV | $CONS | $SEQUENCE);
-	my $x_oid = asn1_extract($ext->[1], $UNIV | $OBJ_ID);
-	my $x_crit = asn1_extract($ext->[1], $UNIV | $BOOLEAN, 1);
-	my $x_val = asn1_extract($ext->[1], $UNIV | $OCTET_STRING);
-
-	my $raw_oid = asn1_retrieve($x_oid->[1]);
-	next if (!exists($OIDs{$raw_oid}));
-	my $x_type = $OIDs{$raw_oid};
-
-	my $raw_value = asn1_retrieve($x_val->[1]);
-
-	if ($x_type eq "subjectKeyIdentifier") {
-	    my $vcursor = [ 0, length($raw_value), \$raw_value ];
-
-	    $subject_key_id = asn1_extract($vcursor, $UNIV | $OCTET_STRING);
-	}
-    }
-}
-
-###############################################################################
-#
-# Determine what we're going to use as the signer's name.  In order of
-# preference, take one of: commonName, organizationName or emailAddress.
-#
-###############################################################################
-my $org = "";
-my $cn = "";
-my $email = "";
-
-while ($subject->[1]->[1] > 0) {
-    my $rdn = asn1_extract($subject->[1], $UNIV | $CONS | $SET);
-    my $attr = asn1_extract($rdn->[1], $UNIV | $CONS | $SEQUENCE);
-    my $n_oid = asn1_extract($attr->[1], $UNIV | $OBJ_ID);
-    my $n_val = asn1_extract($attr->[1], -1);
-
-    my $raw_oid = asn1_retrieve($n_oid->[1]);
-    next if (!exists($OIDs{$raw_oid}));
-    my $n_type = $OIDs{$raw_oid};
-
-    my $raw_value = asn1_retrieve($n_val->[1]);
-
-    if ($n_type eq "organizationName") {
-	$org = $raw_value;
-    } elsif ($n_type eq "commonName") {
-	$cn = $raw_value;
-    } elsif ($n_type eq "emailAddress") {
-	$email = $raw_value;
-    }
-}
-
-my $signers_name = $email;
-
-if ($org && $cn) {
-    # Don't use the organizationName if the commonName repeats it
-    if (length($org) <= length($cn) &&
-	substr($cn, 0, length($org)) eq $org) {
-	$signers_name = $cn;
-	goto got_id_name;
-    }
-
-    # Or a signifcant chunk of it
-    if (length($org) >= 7 &&
-	length($cn) >= 7 &&
-	substr($cn, 0, 7) eq substr($org, 0, 7)) {
-	$signers_name = $cn;
-	goto got_id_name;
-    }
-
-    $signers_name = $org . ": " . $cn;
-} elsif ($org) {
-    $signers_name = $org;
-} elsif ($cn) {
-    $signers_name = $cn;
-}
-
-got_id_name:
-
-die $x509, ": ", "X.509: Couldn't find the Subject Key Identifier extension\n"
-    if (!$subject_key_id);
-
-my $key_identifier = asn1_retrieve($subject_key_id->[1]);
-
-###############################################################################
-#
-# Create and attach the module signature
-#
-###############################################################################
-
-#
-# Signature parameters
-#
-my $algo = 1;		# Public-key crypto algorithm: RSA
-my $hash = 0;		# Digest algorithm
-my $id_type = 1;	# Identifier type: X.509
-
-#
-# Digest the data
-#
-my $prologue;
-if ($dgst eq "sha1") {
-    $prologue = pack("C*",
-		     0x30, 0x21, 0x30, 0x09, 0x06, 0x05,
-		     0x2B, 0x0E, 0x03, 0x02, 0x1A,
-		     0x05, 0x00, 0x04, 0x14);
-    $hash = 2;
-} elsif ($dgst eq "sha224") {
-    $prologue = pack("C*",
-		     0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09,
-		     0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04,
-		     0x05, 0x00, 0x04, 0x1C);
-    $hash = 7;
-} elsif ($dgst eq "sha256") {
-    $prologue = pack("C*",
-		     0x30, 0x31, 0x30, 0x0d, 0x06, 0x09,
-		     0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
-		     0x05, 0x00, 0x04, 0x20);
-    $hash = 4;
-} elsif ($dgst eq "sha384") {
-    $prologue = pack("C*",
-		     0x30, 0x41, 0x30, 0x0d, 0x06, 0x09,
-		     0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02,
-		     0x05, 0x00, 0x04, 0x30);
-    $hash = 5;
-} elsif ($dgst eq "sha512") {
-    $prologue = pack("C*",
-		     0x30, 0x51, 0x30, 0x0d, 0x06, 0x09,
-		     0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03,
-		     0x05, 0x00, 0x04, 0x40);
-    $hash = 6;
-} else {
-    die "Unknown hash algorithm: $dgst\n";
-}
-
-my $signature;
-if ($signature_file) {
-	$signature = read_file($signature_file);
-} else {
-	#
-	# Generate the digest and read from openssl's stdout
-	#
-	my $digest;
-	$digest = readpipe("openssl dgst -$dgst -binary $module") || die "openssl dgst";
-
-	#
-	# Generate the binary signature, which will be just the integer that
-	# comprises the signature with no metadata attached.
-	#
-	my $pid;
-	$pid = open2(*read_from, *write_to,
-		     "openssl rsautl -sign -inkey $private_key -keyform PEM") ||
-	    die "openssl rsautl";
-	binmode write_to;
-	print write_to $prologue . $digest || die "pipe to openssl rsautl";
-	close(write_to) || die "pipe to openssl rsautl";
-
-	binmode read_from;
-	read(read_from, $signature, 4096) || die "pipe from openssl rsautl";
-	close(read_from) || die "pipe from openssl rsautl";
-	waitpid($pid, 0) || die;
-	die "openssl rsautl died: $?" if ($? >> 8);
-}
-$signature = pack("n", length($signature)) . $signature,
-
-#
-# Build the signed binary
-#
-my $unsigned_module = read_file($module);
-
-my $magic_number = "~Module signature appended~\n";
-
-my $info = pack("CCCCCxxxN",
-		$algo, $hash, $id_type,
-		length($signers_name),
-		length($key_identifier),
-		length($signature));
-
-if ($verbose) {
-    print "Size of unsigned module: ", length($unsigned_module), "\n";
-    print "Size of signer's name  : ", length($signers_name), "\n";
-    print "Size of key identifier : ", length($key_identifier), "\n";
-    print "Size of signature      : ", length($signature), "\n";
-    print "Size of informaton     : ", length($info), "\n";
-    print "Size of magic number   : ", length($magic_number), "\n";
-    print "Signer's name          : '", $signers_name, "'\n";
-    print "Digest                 : $dgst\n";
-}
-
-open(FD, ">$dest") || die $dest;
-binmode FD;
-print FD
-    $unsigned_module,
-    $signers_name,
-    $key_identifier,
-    $signature,
-    $info,
-    $magic_number
-    ;
-close FD || die $dest;
-
-if (!$keep_orig) {
-    rename($dest, $module) || die $module;
-}

Re: [PATCH 5/5] MODSIGN: Use PKCS#7 messages as module signatures

[Mimi Zohar]

On Thu, 2014-11-20 at 16:54 +0000, David Howells wrote:

> 
>  /*
> @@ -186,12 +81,8 @@ static struct key *request_asymmetric_key(const char *signer, size_t signer_len,
>   */
>  int mod_verify_sig(const void *mod, unsigned long *_modlen)
>  {
> -	struct public_key_signature *pks;
>  	struct module_signature ms;
> -	struct key *key;
> -	const void *sig;
>  	size_t modlen = *_modlen, sig_len;
> -	int ret;
> 
>  	pr_devel("==>%s(,%zu)\n", __func__, modlen);
> 
> @@ -205,46 +96,23 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
>  	if (sig_len >= modlen)
>  		return -EBADMSG;
>  	modlen -= sig_len;
> -	if ((size_t)ms.signer_len + ms.key_id_len >= modlen)
> -		return -EBADMSG;
> -	modlen -= (size_t)ms.signer_len + ms.key_id_len;
> -
>  	*_modlen = modlen;
> -	sig = mod + modlen;
> -
> -	/* For the moment, only support RSA and X.509 identifiers */
> -	if (ms.algo != PKEY_ALGO_RSA ||
> -	    ms.id_type != PKEY_ID_X509)
> -		return -ENOPKG;
> 
> -	if (ms.hash >= PKEY_HASH__LAST ||
> -	    !hash_algo_name[ms.hash])
> +	if (ms.id_type != PKEY_ID_PKCS7) {
> +		pr_err("Module is not signed with expected PKCS#7 message\n");
>  		return -ENOPKG;

Perhaps because modules are resigned with each kernel build, it is
acceptable to totally replace one signature format with another like
this, and fail the old method.

Mimi

Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[Dmitry Kasatkin]

Hi David,

Before I go into reviewing the patches just want to let you know that
Integrity stuff seems to work fine with these changes.

Thanks,
Dmitry

On 20/11/14 18:53, David Howells wrote:
> Here's a set of patches that does the following:
>
>  (1) Extracts both parts of an X.509 AuthorityKeyIdentifier (AKID) extension.
>      We already extract the bit that can match the subjectKeyIdentifier (SKID)
>      of the parent X.509 cert, but we currently ignore the bits that can match
>      the issuer and serialNumber.
>
>      Looks up an X.509 cert by issuer and serialNumber if those are provided in
>      the AKID.  If the keyIdentifier is also provided, checks that the
>      subjectKeyIdentifier of the cert found matches that also.
>
>      If no issuer and serialNumber are provided in the AKID, looks up an X.509
>      cert by SKID using the AKID keyIdentifier.
>
>      This allows module signing to be done with certificates that don't have an
>      SKID by which they can be looked up.
>
>  (2) Makes use of the PKCS#7 facility to provide module signatures.
>
>      sign-file is replaced with a program that generates a PKCS#7 message that
>      has no X.509 certs embedded and that has detached data (the module
>      content) and adds it onto the message with magic string and descriptor.
>
>  (3) The PKCS#7 message (and matching X.509 cert) supply all the information
>      that is needed to select the X.509 cert to be used to verify the signature
>      by standard means (including selection of digest algorithm and public key
>      algorithm).  No kernel-specific magic values are required.
>
> Note that the revised sign-file program no longer supports the "-s <signature>"
> option as I'm not sure what the best way to deal with this is.  Do we generate
> a PKCS#7 cert from the signature given, or do we get given a PKCS#7 cert?  I
> lean towards the latter.
>
> They can be found here also:
>
> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7
>
> These patches are based on the security tree's next branch.
>
> David
> ---
> David Howells (5):
>       X.509: Extract both parts of the AuthorityKeyIdentifier
>       X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
>       PKCS#7: Allow detached data to be supplied for signature checking purposes
>       MODSIGN: Provide a utility to append a PKCS#7 signature to a module
>       MODSIGN: Use PKCS#7 messages as module signatures
>
>
>  crypto/asymmetric_keys/Makefile           |    8 -
>  crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
>  crypto/asymmetric_keys/pkcs7_verify.c     |   81 ++++--
>  crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
>  crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
>  crypto/asymmetric_keys/x509_parser.h      |    3 
>  crypto/asymmetric_keys/x509_public_key.c  |   85 ++++--
>  include/crypto/pkcs7.h                    |    3 
>  include/crypto/public_key.h               |    4 
>  init/Kconfig                              |    1 
>  kernel/module_signing.c                   |  220 +++------------
>  scripts/Makefile                          |    2 
>  scripts/sign-file                         |  421 -----------------------------
>  scripts/sign-file.c                       |  189 +++++++++++++
>  14 files changed, 505 insertions(+), 699 deletions(-)
>  create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
>  delete mode 100755 scripts/sign-file
>  create mode 100755 scripts/sign-file.c
>
>

Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[Dmitry Kasatkin]

On 21/11/14 14:59, Dmitry Kasatkin wrote:
> Hi David,
>
> Before I go into reviewing the patches just want to let you know that
> Integrity stuff seems to work fine with these changes.

Actually after cleaning the tree and re-signing the modules, I get following

Unrecognized character \x7F; marked by <-- HERE after <-- HERE near
column 1 at ./scripts/sign-file line 1.
make[1]: *** [arch/x86/crypto/aes-x86_64.ko] Error 255


- Dmitry

> Thanks,
> Dmitry
>
> On 20/11/14 18:53, David Howells wrote:
>> Here's a set of patches that does the following:
>>
>>  (1) Extracts both parts of an X.509 AuthorityKeyIdentifier (AKID) extension.
>>      We already extract the bit that can match the subjectKeyIdentifier (SKID)
>>      of the parent X.509 cert, but we currently ignore the bits that can match
>>      the issuer and serialNumber.
>>
>>      Looks up an X.509 cert by issuer and serialNumber if those are provided in
>>      the AKID.  If the keyIdentifier is also provided, checks that the
>>      subjectKeyIdentifier of the cert found matches that also.
>>
>>      If no issuer and serialNumber are provided in the AKID, looks up an X.509
>>      cert by SKID using the AKID keyIdentifier.
>>
>>      This allows module signing to be done with certificates that don't have an
>>      SKID by which they can be looked up.
>>
>>  (2) Makes use of the PKCS#7 facility to provide module signatures.
>>
>>      sign-file is replaced with a program that generates a PKCS#7 message that
>>      has no X.509 certs embedded and that has detached data (the module
>>      content) and adds it onto the message with magic string and descriptor.
>>
>>  (3) The PKCS#7 message (and matching X.509 cert) supply all the information
>>      that is needed to select the X.509 cert to be used to verify the signature
>>      by standard means (including selection of digest algorithm and public key
>>      algorithm).  No kernel-specific magic values are required.
>>
>> Note that the revised sign-file program no longer supports the "-s <signature>"
>> option as I'm not sure what the best way to deal with this is.  Do we generate
>> a PKCS#7 cert from the signature given, or do we get given a PKCS#7 cert?  I
>> lean towards the latter.
>>
>> They can be found here also:
>>
>> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7
>>
>> These patches are based on the security tree's next branch.
>>
>> David
>> ---
>> David Howells (5):
>>       X.509: Extract both parts of the AuthorityKeyIdentifier
>>       X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
>>       PKCS#7: Allow detached data to be supplied for signature checking purposes
>>       MODSIGN: Provide a utility to append a PKCS#7 signature to a module
>>       MODSIGN: Use PKCS#7 messages as module signatures
>>
>>
>>  crypto/asymmetric_keys/Makefile           |    8 -
>>  crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
>>  crypto/asymmetric_keys/pkcs7_verify.c     |   81 ++++--
>>  crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
>>  crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
>>  crypto/asymmetric_keys/x509_parser.h      |    3 
>>  crypto/asymmetric_keys/x509_public_key.c  |   85 ++++--
>>  include/crypto/pkcs7.h                    |    3 
>>  include/crypto/public_key.h               |    4 
>>  init/Kconfig                              |    1 
>>  kernel/module_signing.c                   |  220 +++------------
>>  scripts/Makefile                          |    2 
>>  scripts/sign-file                         |  421 -----------------------------
>>  scripts/sign-file.c                       |  189 +++++++++++++
>>  14 files changed, 505 insertions(+), 699 deletions(-)
>>  create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
>>  delete mode 100755 scripts/sign-file
>>  create mode 100755 scripts/sign-file.c
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

Dmitry Kasatkin <d.kasatkin@samsung.com> wrote:

> Actually after cleaning the tree and re-signing the modules, I get following
> 
> Unrecognized character \x7F; marked by <-- HERE after <-- HERE near
> column 1 at ./scripts/sign-file line 1.
> make[1]: *** [arch/x86/crypto/aes-x86_64.ko] Error 255

warthog>grep -r sign-file Makefile 
mod_sign_cmd = perl $(srctree)/scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)

Because of that.  I need to remove the 'perl' bit.

David

Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> > Actually after cleaning the tree and re-signing the modules, I get following
> > 
> > Unrecognized character \x7F; marked by <-- HERE after <-- HERE near
> > column 1 at ./scripts/sign-file line 1.
> > make[1]: *** [arch/x86/crypto/aes-x86_64.ko] Error 255
> 
> warthog>grep -r sign-file Makefile 
> mod_sign_cmd = perl $(srctree)/scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
> 
> Because of that.  I need to remove the 'perl' bit.

It's a little more involved than that.  The X.509 cert being passed to the
program is binary, whereas the one I've been testing with is PEM encoded - and
libssl has separate routines that don't work out for themselves which encoding
is in force.  Proposed changes below.

David
---
diff --git a/Makefile b/Makefile
index b77de27e58fc..8d5624bf96db 100644
--- a/Makefile
+++ b/Makefile
@@ -859,7 +859,7 @@ ifdef CONFIG_MODULE_SIG_ALL
 MODSECKEY = ./signing_key.priv
 MODPUBKEY = ./signing_key.x509
 export MODPUBKEY
-mod_sign_cmd = perl $(srctree)/scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
+mod_sign_cmd = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
 else
 mod_sign_cmd = true
 endif
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 3f9bedbd185f..ff5e78348de0 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -61,14 +61,24 @@ static void display_openssl_errors(int l)
 	}
 }
 
+static void drain_openssl_errors(void)
+{
+	const char *file;
+	int line;
+
+	if (ERR_peek_error() == 0)
+		return;
+	while (ERR_get_error_line(&file, &line)) {}
+}
 
-#define ERR(cond, ...)				  \
-	do {					  \
-		bool __cond = (cond);		  \
-		display_openssl_errors(__LINE__); \
-		if (__cond) {			  \
-			err(1, ## __VA_ARGS__);	  \
-		}				  \
+
+#define ERR(cond, ...)					\
+	do {						\
+		bool __cond = (cond);			\
+		display_openssl_errors(__LINE__);	\
+		if (__cond) {				\
+			err(1, ## __VA_ARGS__);		\
+		}					\
 	} while(0)
 
 int main(int argc, char **argv)
@@ -126,8 +136,15 @@ int main(int argc, char **argv)
 
 	b = BIO_new_file(x509_name, "rb");
 	ERR(!b, "%s", x509_name);
-        x509 = PEM_read_bio_X509(b, NULL, NULL, NULL);
+	x509 = d2i_X509_bio(b, NULL); /* Binary encoded X.509 */
+	if (!x509) {
+		BIO_reset(b);
+		x509 = PEM_read_bio_X509(b, NULL, NULL, NULL); /* PEM encoded X.509 */
+		if (x509)
+			drain_openssl_errors();
+	}
 	BIO_free(b);
+	ERR(!x509, "%s", x509_name);
 
 	/* Open the destination file now so that we can shovel the module data
 	 * across as we read it.

Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[Mimi Zohar]

On Mon, 2014-11-24 at 16:13 +0000, David Howells wrote: 
> David Howells <dhowells@redhat.com> wrote:
> 
> > > Actually after cleaning the tree and re-signing the modules, I get following
> > > 
> > > Unrecognized character \x7F; marked by <-- HERE after <-- HERE near
> > > column 1 at ./scripts/sign-file line 1.
> > > make[1]: *** [arch/x86/crypto/aes-x86_64.ko] Error 255
> > 
> > warthog>grep -r sign-file Makefile 
> > mod_sign_cmd = perl $(srctree)/scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
> > 
> > Because of that.  I need to remove the 'perl' bit.
> 
> It's a little more involved than that.  The X.509 cert being passed to the
> program is binary, whereas the one I've been testing with is PEM encoded - and
> libssl has separate routines that don't work out for themselves which encoding
> is in force.  Proposed changes below.

With this patch, I'm now able to install and boot the new kernel and
modules.

Mimi

Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[Dmitry Kasatkin]

On 20/11/14 18:53, David Howells wrote:
> Here's a set of patches that does the following:
>
>  (1) Extracts both parts of an X.509 AuthorityKeyIdentifier (AKID) extension.
>      We already extract the bit that can match the subjectKeyIdentifier (SKID)
>      of the parent X.509 cert, but we currently ignore the bits that can match
>      the issuer and serialNumber.
>
>      Looks up an X.509 cert by issuer and serialNumber if those are provided in
>      the AKID.  If the keyIdentifier is also provided, checks that the
>      subjectKeyIdentifier of the cert found matches that also.
>
>      If no issuer and serialNumber are provided in the AKID, looks up an X.509
>      cert by SKID using the AKID keyIdentifier.
>
>      This allows module signing to be done with certificates that don't have an
>      SKID by which they can be looked up.
>
>  (2) Makes use of the PKCS#7 facility to provide module signatures.
>
>      sign-file is replaced with a program that generates a PKCS#7 message that
>      has no X.509 certs embedded and that has detached data (the module
>      content) and adds it onto the message with magic string and descriptor.

Why do you highlight that X509 is not embedded?
Current module signing does not embed X509 also.

- Dmitry

>  (3) The PKCS#7 message (and matching X.509 cert) supply all the information
>      that is needed to select the X.509 cert to be used to verify the signature
>      by standard means (including selection of digest algorithm and public key
>      algorithm).  No kernel-specific magic values are required.
>
> Note that the revised sign-file program no longer supports the "-s <signature>"
> option as I'm not sure what the best way to deal with this is.  Do we generate
> a PKCS#7 cert from the signature given, or do we get given a PKCS#7 cert?  I
> lean towards the latter.
>
> They can be found here also:
>
> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7
>
> These patches are based on the security tree's next branch.
>
> David
> ---
> David Howells (5):
>       X.509: Extract both parts of the AuthorityKeyIdentifier
>       X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
>       PKCS#7: Allow detached data to be supplied for signature checking purposes
>       MODSIGN: Provide a utility to append a PKCS#7 signature to a module
>       MODSIGN: Use PKCS#7 messages as module signatures
>
>
>  crypto/asymmetric_keys/Makefile           |    8 -
>  crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
>  crypto/asymmetric_keys/pkcs7_verify.c     |   81 ++++--
>  crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
>  crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
>  crypto/asymmetric_keys/x509_parser.h      |    3 
>  crypto/asymmetric_keys/x509_public_key.c  |   85 ++++--
>  include/crypto/pkcs7.h                    |    3 
>  include/crypto/public_key.h               |    4 
>  init/Kconfig                              |    1 
>  kernel/module_signing.c                   |  220 +++------------
>  scripts/Makefile                          |    2 
>  scripts/sign-file                         |  421 -----------------------------
>  scripts/sign-file.c                       |  189 +++++++++++++
>  14 files changed, 505 insertions(+), 699 deletions(-)
>  create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
>  delete mode 100755 scripts/sign-file
>  create mode 100755 scripts/sign-file.c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

Dmitry Kasatkin <d.kasatkin@samsung.com> wrote:

> >  (2) Makes use of the PKCS#7 facility to provide module signatures.
> >
> >      sign-file is replaced with a program that generates a PKCS#7 message
> >      that has no X.509 certs embedded and that has detached data (the
> >      module content) and adds it onto the message with magic string and
> >      descriptor.
> 
> Why do you highlight that X509 is not embedded?
> Current module signing does not embed X509 also.

A PKCS#7 message can have X.509 certs embedded within it - but it's optional
within the spec.  Given that we expect to have the appropriate cert available
to verify the signature on the PKCS#7 message directly, there's no need to
actually embed the X.509 cert therein.

Unfortunately, it doesn't appear that you can do this with the openssl command
line utility - hence why I moved to C.

David