[PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[David Howells]

Hi Rusty,

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.

 (4) Makes it possible to get sign-file to just write out a file containing the
     PKCS#7 signature blob.  This can be used for debugging and potentially for
     firmware signing.

 (5) Extract the function that does PKCS#7 signature verification on a blob
     from the module signing code and put it somewhere more general so that
     other things, such as firmware signing, can make use of it without
     depending on module config options.

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.  Note that David Woodhouse is looking at making
sign-file work with PKCS#11, so bringing back -s might not be necessary.

They can be found here also:

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

and are tagged with:

	modsign-pkcs7-20150515

Should these go via the security tree or your tree?

David
---
David Howells (7):
      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
      system_keyring.c doesn't need to #include module-internal.h
      MODSIGN: Extract the blob PKCS#7 signature verifier from module signing

Luis R. Rodriguez (1):
      sign-file: Add option to only create signature file


 Makefile                                  |    2 
 crypto/asymmetric_keys/Makefile           |    8 -
 crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
 crypto/asymmetric_keys/pkcs7_verify.c     |   80 ++++--
 crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
 crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
 crypto/asymmetric_keys/x509_parser.h      |    5 
 crypto/asymmetric_keys/x509_public_key.c  |   86 ++++--
 include/crypto/pkcs7.h                    |    3 
 include/crypto/public_key.h               |    4 
 include/keys/system_keyring.h             |    5 
 init/Kconfig                              |   28 +-
 kernel/module_signing.c                   |  212 +--------------
 kernel/system_keyring.c                   |   51 +++-
 scripts/Makefile                          |    2 
 scripts/sign-file                         |  421 -----------------------------
 scripts/sign-file.c                       |  212 +++++++++++++++
 17 files changed, 578 insertions(+), 728 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/8] X.509: Extract both parts of the AuthorityKeyIdentifier [ver #4]

[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>
Tested-by: Vivek Goyal <vgoyal@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      |    5 +
 crypto/asymmetric_keys/x509_public_key.c  |    8 +-
 7 files changed, 145 insertions(+), 69 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..0f6463b6692b 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->akid_skid) {
+		key = x509_request_asymmetric_key(trust_keyring, last->akid_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..a4d083f7e9e1 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->akid_skid)
 			pr_debug("- authkeyid %*phN\n",
-				 x509->authority->len, x509->authority->data);
+				 x509->akid_skid->len, x509->akid_skid->data);
 
-		if (!x509->authority ||
+		if (!x509->akid_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->akid_skid->len, x509->akid_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->akid_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..6c130dd56f35 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->akid_id);
+		kfree(cert->akid_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->akid_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->akid_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->akid_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->akid_id = kid;
+	return 0;
+}
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 3dfe6b5d6f0b..dcdb5c94f514 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -19,9 +19,10 @@ struct x509_certificate {
 	struct public_key_signature sig;	/* Signature parameters */
 	char		*issuer;		/* Name of certificate issuer */
 	char		*subject;		/* Name of certificate subject */
-	struct asymmetric_key_id *id;		/* Serial number + issuer */
+	struct asymmetric_key_id *id;		/* Issuer + Serial number */
 	struct asymmetric_key_id *skid;		/* Subject + subjectKeyId (optional) */
-	struct asymmetric_key_id *authority;	/* Authority key identifier (optional) */
+	struct asymmetric_key_id *akid_id;	/* CA AuthKeyId matching ->id (optional) */
+	struct asymmetric_key_id *akid_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..49b875b709d5 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->akid_skid, ca_keyid))
 		return -EPERM;
 
-	key = x509_request_asymmetric_key(trust_keyring, cert->authority,
+	key = x509_request_asymmetric_key(trust_keyring, cert->akid_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->akid_skid ||
+	    asymmetric_key_id_same(cert->skid, cert->akid_skid)) {
 		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;

[PATCH 2/8] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier [ver #4]

[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>
Tested-by: Vivek Goyal <vgoyal@redhat.com>
---

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

diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 0f6463b6692b..90d6d47965b0 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->akid_skid) {
-		key = x509_request_asymmetric_key(trust_keyring, last->akid_skid,
+	if (last && (last->akid_id || last->akid_skid)) {
+		key = x509_request_asymmetric_key(trust_keyring,
+						  last->akid_id,
+						  last->akid_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 a4d083f7e9e1..42bfc9de0d79 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->akid_id)
+			pr_debug("- authkeyid.id %*phN\n",
+				 x509->akid_id->len, x509->akid_id->data);
 		if (x509->akid_skid)
-			pr_debug("- authkeyid %*phN\n",
+			pr_debug("- authkeyid.skid %*phN\n",
 				 x509->akid_skid->len, x509->akid_skid->data);
 
-		if (!x509->akid_skid ||
+		if ((!x509->akid_id && !x509->akid_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->akid_skid->len, x509->akid_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->akid_skid))
-				goto found_issuer;
+		auth = x509->akid_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->akid_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->akid_skid &&
+		    !asymmetric_key_id_same(p->skid, x509->akid_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 49b875b709d5..8a4e3a29ec31 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -52,23 +52,37 @@ __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;
+
+	if (id) {
+		lookup = id->data;
+		len = id->len;
+	} else {
+		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 +93,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 +247,8 @@ static int x509_validate_trust(struct x509_certificate *cert,
 	if (ca_keyid && !asymmetric_key_id_partial(cert->akid_skid, ca_keyid))
 		return -EPERM;
 
-	key = x509_request_asymmetric_key(trust_keyring, cert->akid_skid,
+	key = x509_request_asymmetric_key(trust_keyring,
+					  cert->akid_id, cert->akid_skid,
 					  false);
 	if (!IS_ERR(key))  {
 		if (!use_builtin_keys
@@ -274,8 +305,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->akid_skid ||
-	    asymmetric_key_id_same(cert->skid, cert->akid_skid)) {
+	if ((!cert->akid_skid && !cert->akid_id) ||
+	    asymmetric_key_id_same(cert->skid, cert->akid_skid) ||
+	    asymmetric_key_id_same(cert->id, cert->akid_id)) {
 		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 */

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

[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.

This is used in a subsequent patch to supply the data to module signing where
the signature is in the form of a PKCS#7 message with detached data, whereby
the detached data is the module content that is signed.

Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Vivek Goyal <vgoyal@redhat.com>
---

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

diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 42bfc9de0d79..404f89a0f852 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -382,3 +382,28 @@ 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;
+}
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);

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

[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>
Tested-by: Vivek Goyal <vgoyal@redhat.com>
---

 include/crypto/public_key.h |    1 
 scripts/sign-file.c         |  205 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 206 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..5b8a6dda3235
--- /dev/null
+++ b/scripts/sign-file.c
@@ -0,0 +1,205 @@
+/* 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);
+	}
+}
+
+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, fmt, ...)				\
+	do {						\
+		bool __cond = (cond);			\
+		display_openssl_errors(__LINE__);	\
+		if (__cond) {				\
+			err(1, fmt, ## __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;
+
+	ERR_load_crypto_strings();
+	ERR_clear_error();
+
+	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;
+	}
+
+	/* 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 = 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.
+	 */
+	bd = BIO_new_file(dest_name, "wb");
+	ERR(!bd, "%s", 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),
+	    "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, "%s", pkcs7_name);
+		ERR(i2d_PKCS7_bio_stream(b, pkcs7, NULL, 0) < 0, "%s", pkcs7_name);
+		BIO_free(b);
+	}
+
+	/* Append the marker and the PKCS#7 message to the destination file */
+	ERR(BIO_reset(bm) < 0, "%s", module_name);
+	while ((n = BIO_read(bm, buf, sizeof(buf))),
+	       n > 0) {
+		ERR(BIO_write(bd, buf, n) < 0, "%s", dest_name);
+	}
+	ERR(n < 0, "%s", module_name);
+	module_size = BIO_number_written(bd);
+
+	ERR(i2d_PKCS7_bio_stream(bd, pkcs7, NULL, 0) < 0, "%s", 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, "%s", dest_name);
+	ERR(BIO_write(bd, magic_number, sizeof(magic_number) - 1) < 0, "%s", dest_name);
+
+	ERR(BIO_free(bd) < 0, "%s", dest_name);
+
+	/* Finally, if we're signing in place, replace the original. */
+	if (replace_orig)
+		ERR(rename(dest_name, module_name) < 0, "%s", dest_name);
+
+	return 0;
+}

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

[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>
Tested-by: Vivek Goyal <vgoyal@redhat.com>
---

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

diff --git a/Makefile b/Makefile
index eae539d69bf3..ad27fc9fa02b 100644
--- a/Makefile
+++ b/Makefile
@@ -875,7 +875,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/init/Kconfig b/init/Kconfig
index dc24dec60232..fb98cba069c1 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1875,6 +1875,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 2016a64497ab..b12fe020664d 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 3906ee1e2f76..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 information    : ", 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;
-}

[PATCH 6/8] sign-file: Add option to only create signature file [ver #4]

[David Howells]

From: Luis R. Rodriguez <mcgrof@suse.com>

Make the -d option (which currently isn't actually wired to anything) write
out the PKCS#7 message as per the -p option and then exit without either
modifying the source or writing out a compound file of the source, signature
and metadata.

This will be useful when firmware signature support is added
upstream as firmware will be left intact, and we'll only require
the signature file. The descriptor is implicit by file extension
and the file's own size.

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 scripts/sign-file.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 5b8a6dda3235..39aaabe89388 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -86,13 +86,14 @@ int main(int argc, char **argv)
 	char *hash_algo = NULL;
 	char *private_key_name, *x509_name, *module_name, *dest_name;
 	bool save_pkcs7 = false, replace_orig;
+	bool sign_only = false;
 	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;
+	BIO *b, *bd = NULL, *bm;
 	int opt, n;
 
 	ERR_load_crypto_strings();
@@ -102,6 +103,7 @@ int main(int argc, char **argv)
 		opt = getopt(argc, argv, "dp");
 		switch (opt) {
 		case 'p': save_pkcs7 = true; break;
+		case 'd': sign_only = true; save_pkcs7 = true; break;
 		case -1: break;
 		default: format();
 		}
@@ -148,8 +150,10 @@ int main(int argc, char **argv)
 	/* 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, "%s", dest_name);
+	if (!sign_only) {
+		bd = BIO_new_file(dest_name, "wb");
+		ERR(!bd, "%s", dest_name);
+	}
 
 	/* Digest the module data. */
 	OpenSSL_add_all_digests();
@@ -180,6 +184,9 @@ int main(int argc, char **argv)
 		BIO_free(b);
 	}
 
+	if (sign_only)
+		return 0;
+
 	/* Append the marker and the PKCS#7 message to the destination file */
 	ERR(BIO_reset(bm) < 0, "%s", module_name);
 	while ((n = BIO_read(bm, buf, sizeof(buf))),

[PATCH 7/8] system_keyring.c doesn't need to #include module-internal.h [ver #4]

[David Howells]

system_keyring.c doesn't need to #include module-internal.h as it doesn't use
the one thing that exports.  Remove the inclusion.

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

 kernel/system_keyring.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index 875f64e8935b..4cda71ee51c7 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -16,7 +16,6 @@
 #include <linux/err.h>
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
-#include "module-internal.h"
 
 struct key *system_trusted_keyring;
 EXPORT_SYMBOL_GPL(system_trusted_keyring);

[PATCH 8/8] MODSIGN: Extract the blob PKCS#7 signature verifier from module signing [ver #4]

[David Howells]

Extract the function that drives the PKCS#7 signature verification given a
data blob and a PKCS#7 blob out from the module signing code and lump it with
the system keyring code as it's generic.  This makes it independent of module
config options and opens it to use by the firmware loader.

Signed-off-by: David Howells <dhowells@redhat.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Kyle McMartin <kyle@kernel.org>
---

 include/keys/system_keyring.h |    5 ++++
 init/Kconfig                  |   29 ++++++++++++++++--------
 kernel/module_signing.c       |   44 +-----------------------------------
 kernel/system_keyring.c       |   50 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 53 deletions(-)

diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 72665eb80692..9791c907cdb7 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -28,4 +28,9 @@ static inline struct key *get_system_trusted_keyring(void)
 }
 #endif
 
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+extern int system_verify_data(const void *data, unsigned long len,
+			      const void *raw_pkcs7, size_t pkcs7_len);
+#endif
+
 #endif /* _KEYS_SYSTEM_KEYRING_H */
diff --git a/init/Kconfig b/init/Kconfig
index fb98cba069c1..c05afd6594f2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1758,6 +1758,24 @@ config SYSTEM_TRUSTED_KEYRING
 
 	  Keys in this keyring are used by module signature checking.
 
+config SYSTEM_DATA_VERIFICATION
+	def_bool n
+	select SYSTEM_TRUSTED_KEYRING
+	select KEYS
+	select CRYPTO
+	select ASYMMETRIC_KEY_TYPE
+	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	select PUBLIC_KEY_ALGO_RSA
+	select ASN1
+	select OID_REGISTRY
+	select X509_CERTIFICATE_PARSER
+	select PKCS7_MESSAGE_PARSER
+	help
+	  Provide PKCS#7 message verification using the contents of the system
+	  trusted keyring to provide public keys.  This then can be used for
+	  module verification, kexec image verification and firmware blob
+	  verification.
+
 config PROFILING
 	bool "Profiling support"
 	help
@@ -1866,16 +1884,7 @@ config MODULE_SRCVERSION_ALL
 config MODULE_SIG
 	bool "Module signature verification"
 	depends on MODULES
-	select SYSTEM_TRUSTED_KEYRING
-	select KEYS
-	select CRYPTO
-	select ASYMMETRIC_KEY_TYPE
-	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
-	select PUBLIC_KEY_ALGO_RSA
-	select ASN1
-	select OID_REGISTRY
-	select X509_CERTIFICATE_PARSER
-	select PKCS7_MESSAGE_PARSER
+	select SYSTEM_DATA_VERIFICATION
 	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 8eb20cc66b39..70ad463f6df0 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -10,10 +10,8 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/err.h>
 #include <keys/system_keyring.h>
 #include <crypto/public_key.h>
-#include <crypto/pkcs7.h>
 #include "module-internal.h"
 
 /*
@@ -37,46 +35,6 @@ struct module_signature {
 };
 
 /*
- * Verify a PKCS#7-based signature on a module.
- */
-static int mod_verify_pkcs7(const void *mod, unsigned long modlen,
-			    const void *raw_pkcs7, size_t pkcs7_len)
-{
-	struct pkcs7_message *pkcs7;
-	bool trusted;
-	int ret;
-
-	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 = pkcs7_validate_trust(pkcs7, system_trusted_keyring, &trusted);
-	if (ret < 0)
-		goto error;
-
-	if (!trusted) {
-		pr_err("PKCS#7 signature not signed with a trusted key\n");
-		ret = -ENOKEY;
-	}
-
-error:
-	pkcs7_free_message(pkcs7);
-	pr_devel("<==%s() = %d\n", __func__, ret);
-	return ret;
-}
-
-/*
  * Verify the signature on a module.
  */
 int mod_verify_sig(const void *mod, unsigned long *_modlen)
@@ -114,5 +72,5 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
 		return -EBADMSG;
 	}
 
-	return mod_verify_pkcs7(mod, modlen, mod + modlen, sig_len);
+	return system_verify_data(mod, modlen, mod + modlen, sig_len);
 }
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index 4cda71ee51c7..95f2dcbc7616 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -16,6 +16,7 @@
 #include <linux/err.h>
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
+#include <crypto/pkcs7.h>
 
 struct key *system_trusted_keyring;
 EXPORT_SYMBOL_GPL(system_trusted_keyring);
@@ -103,3 +104,52 @@ dodgy_cert:
 	return 0;
 }
 late_initcall(load_system_certificate_list);
+
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+
+/**
+ * Verify a PKCS#7-based signature on system data.
+ * @data: The data to be verified.
+ * @len: Size of @data.
+ * @raw_pkcs7: The PKCS#7 message that is the signature.
+ * @pkcs7_len: The size of @raw_pkcs7.
+ */
+int system_verify_data(const void *data, unsigned long len,
+		       const void *raw_pkcs7, size_t pkcs7_len)
+{
+	struct pkcs7_message *pkcs7;
+	bool trusted;
+	int ret;
+
+	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, data, len) < 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 = pkcs7_validate_trust(pkcs7, system_trusted_keyring, &trusted);
+	if (ret < 0)
+		goto error;
+
+	if (!trusted) {
+		pr_err("PKCS#7 signature not signed with a trusted key\n");
+		ret = -ENOKEY;
+	}
+
+error:
+	pkcs7_free_message(pkcs7);
+	pr_devel("<==%s() = %d\n", __func__, ret);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(system_verify_data);
+
+#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

On Fri, 2015-05-15 at 13:35 +0100, David Howells wrote:
> Note that David Woodhouse is looking at making
> sign-file work with PKCS#11, so bringing back -s might not be 
> necessary.

I actually already *had* it working with PKCS#11, at 
http://git.infradead.org/users/dwmw2/modsign-pkcs11.git

Then you went and rewrote it in C, so I'm still refactoring it. WIP at 
http://git.infradead.org/users/dwmw2/modsign-pkcs11-c.git just needs
me to add the ENGINE_by_id("pkcs11")... bits to scripts/sign-file.c.

I'm also vacillating about whether to allow an external *cert* to be
specified separately from the key. Do we...

 1. Just require the X.509 DER cert in $(topdir)/signing_key.x509,

 2. Automatically extract it from $CONFIG_MODULE_SIG_EXTERNAL_KEY
    which shall be a file (or PKCS#11 URI) containing *both* key
    and cert, or

 3. Add a separate CONFIG_MODULE_SIG_EXTERNAL_CERT option.

I'm probably inclined towards #2. I'll need to script something to
automatically extract the key from a PEM file or PKCS#11 and drop it
in DER form in $(topdir)/signing_key.x509 where needed. Using
basically the same make rules we already *have* for creating a new
key+cert on demand anyway.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

[PATCH 1/4] modsign: Abort modules_install when signing fails

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 scripts/Makefile.modinst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index e48a4e9..07650ee 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -22,7 +22,7 @@ quiet_cmd_modules_install = INSTALL $@
     mkdir -p $(2) ; \
     cp $@ $(2) ; \
     $(mod_strip_cmd) $(2)/$(notdir $@) ; \
-    $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
+    $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) && \
     $(mod_compress_cmd) $(2)/$(notdir $@)
 
 # Modules built outside the kernel source tree go into extra by default
-- 
2.4.0



-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

[PATCH 2/4] modsign: Allow external signing key to be specified

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2620 bytes --]

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 Makefile        |  2 +-
 init/Kconfig    | 13 +++++++++++++
 kernel/Makefile |  6 ++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index ad27fc9..9590e67 100644
--- a/Makefile
+++ b/Makefile
@@ -872,7 +872,7 @@ INITRD_COMPRESS-$(CONFIG_RD_LZ4)   := lz4
 # export INITRD_COMPRESS := $(INITRD_COMPRESS-y)
 
 ifdef CONFIG_MODULE_SIG_ALL
-MODSECKEY = ./signing_key.priv
+MODSECKEY = $(CONFIG_MODULE_SIG_KEY)
 MODPUBKEY = ./signing_key.x509
 export MODPUBKEY
 mod_sign_cmd = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
diff --git a/init/Kconfig b/init/Kconfig
index c05afd6..1ca075a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1954,6 +1954,19 @@ config MODULE_SIG_HASH
        default "sha384" if MODULE_SIG_SHA384
        default "sha512" if MODULE_SIG_SHA512
 
+config MODULE_SIG_EXTERNAL_KEY
+       bool "Use external key for module signing"
+       depends on MODULE_SIG_ALL
+       help
+         Use an external private key for module signing.
+
+config MODULE_SIG_KEY
+       string "File name or PKCS#11 URI of module signing key" if MODULE_SIG_EXTERNAL_KEY
+       default "signing_key.priv"
+       help
+         Provide the file name of a private key in PEM format, or a PKCS#11
+         URI according to RFC7512 to specify the key.
+
 config MODULE_COMPRESS
        bool "Compress modules on installation"
        depends on MODULES
diff --git a/kernel/Makefile b/kernel/Makefile
index 60c302c..b1a718c 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -170,6 +170,11 @@ ifndef CONFIG_MODULE_SIG_HASH
 $(error Could not determine digest type to use from kernel config)
 endif
 
+# 'make randconfig' might enable CONFIG_MODULE_SIG_EXTERNAL_KEY but will
+# leave any strings at their default settings. We want to let the automatic
+# key generation work in that case, which is why we check the string here
+# instead of just using 'ifndef CONFIG_MODULE_SIG_EXTERNAL_KEY'.
+ifeq ($(CONFIG_MODULE_SIG_KEY),"signing_key.priv")
 signing_key.priv signing_key.x509: x509.genkey
        @echo "###"
        @echo "### Now generating an X.509 key pair to be used for signing modules."
@@ -207,3 +212,4 @@ x509.genkey:
        @echo >>x509.genkey "subjectKeyIdentifier=hash"
        @echo >>x509.genkey "authorityKeyIdentifier=keyid"
 endif
+endif
-- 
2.4.0

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

[PATCH 3/4] modsign: Allow password to be specified for signing key

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 4361 bytes --]

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 Documentation/module-signing.txt |  2 ++
 Makefile                         |  1 +
 init/Kconfig                     |  6 ++++++
 scripts/sign-file.c              | 39 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index c72702e..b0ed080 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -194,6 +194,8 @@ The hash algorithm used does not have to match the one configured, but if it
 doesn't, you should make sure that hash algorithm is either built into the
 kernel or can be loaded without requiring itself.
 
+If the private key requires a passphrase or PIN, it can be provided in the
+$CONFIG_MODULE_SIG_KEY_PASSWORD environment variable.
 
 ============================
 SIGNED MODULES AND STRIPPING
diff --git a/Makefile b/Makefile
index 9590e67..70c066c 100644
--- a/Makefile
+++ b/Makefile
@@ -875,6 +875,7 @@ ifdef CONFIG_MODULE_SIG_ALL
 MODSECKEY = $(CONFIG_MODULE_SIG_KEY)
 MODPUBKEY = ./signing_key.x509
 export MODPUBKEY
+export CONFIG_MODULE_SIG_KEY_PASSWORD
 mod_sign_cmd = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
 else
 mod_sign_cmd = true
diff --git a/init/Kconfig b/init/Kconfig
index 1ca075a..7bbc857 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1967,6 +1967,12 @@ config MODULE_SIG_KEY
          Provide the file name of a private key in PEM format, or a PKCS#11
          URI according to RFC7512 to specify the key.
 
+config MODULE_SIG_KEY_PASSWORD
+       string "Passphrase or PIN for module signing key if needed" if MODULE_SIG_EXTERNAL_KEY
+       help
+         If a passphrase or PIN is required for the private key, provide
+         it here.
+
 config MODULE_COMPRESS
        bool "Compress modules on installation"
        depends on MODULES
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 39aaabe..9a54acc 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -80,9 +80,32 @@ static void drain_openssl_errors(void)
                }                                       \
        } while(0)
 
+static char *key_pass;
+
+static int pem_pw_cb(char *buf, int len, int w, void *v)
+{
+       int pwlen;
+
+       if (!key_pass)
+               return -1;
+
+       pwlen = strlen(key_pass);
+       if (pwlen >= len)
+               return -1;
+
+       strcpy(buf, key_pass);
+
+       /* If it's wrong, don't keep trying it. */
+       free(key_pass);
+       key_pass = NULL;
+
+       return pwlen;
+}
+
 int main(int argc, char **argv)
 {
        struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
+       const char *pass_env;
        char *hash_algo = NULL;
        char *private_key_name, *x509_name, *module_name, *dest_name;
        bool save_pkcs7 = false, replace_orig;
@@ -96,6 +119,7 @@ int main(int argc, char **argv)
        BIO *b, *bd = NULL, *bm;
        int opt, n;
 
+       OpenSSL_add_all_algorithms();
        ERR_load_crypto_strings();
        ERR_clear_error();
 
@@ -127,12 +151,25 @@ int main(int argc, char **argv)
                replace_orig = true;
        }
 
+       pass_env = getenv("CONFIG_MODULE_SIG_KEY_PASSWORD");
+       if (pass_env) {
+               int pwlen = strlen(pass_env);
+
+               if (pass_env[0] == '\"' && pass_env[pwlen - 1] == '\"') {
+                       pass_env++;
+                       pwlen -= 2;
+               }
+               if (pwlen)
+                       key_pass = strndup(pass_env, pwlen);
+       }
+
        /* 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);
+       private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb, NULL);
+       ERR(!private_key, "%s", private_key_name);
        BIO_free(b);
 
        b = BIO_new_file(x509_name, "rb");
-- 
2.4.0

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

[PATCH 4/4] modsign: Allow signing key to be PKCS#11

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2382 bytes --]

This is only the key; the corresponding *cert* still needs to be in
$(topdir)/signing_key.x509

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 scripts/sign-file.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 9a54acc..3a923e4 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -22,6 +22,7 @@
 #include <openssl/pem.h>
 #include <openssl/pkcs7.h>
 #include <openssl/err.h>
+#include <openssl/engine.h>
 
 struct module_signature {
        uint8_t         algo;           /* Public-key crypto algorithm [0] */
@@ -166,11 +167,29 @@ int main(int argc, char **argv)
        /* 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, pem_pw_cb, NULL);
-       ERR(!private_key, "%s", private_key_name);
-       BIO_free(b);
+       if (!strncmp(private_key_name, "pkcs11:", 7)) {
+               ENGINE *e;
+
+               ENGINE_load_builtin_engines();
+               drain_openssl_errors();
+               e = ENGINE_by_id("pkcs11");
+               ERR(!e, "Load PKCS#11 ENGINE");
+               if (ENGINE_init(e))
+                       drain_openssl_errors();
+               else
+                       ERR(1, "ENGINE_init");
+               if (key_pass)
+                       ERR(!ENGINE_ctrl_cmd_string(e, "PIN", key_pass, 0), "Set PKCS#11 PIN");
+               private_key = ENGINE_load_private_key(e, private_key_name, NULL,
+                                                     NULL);
+               ERR(!private_key, "%s", private_key_name);
+       } else {
+               b = BIO_new_file(private_key_name, "rb");
+               ERR(!b, "%s", private_key_name);
+               private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb, NULL);
+               ERR(!private_key, "%s", private_key_name);
+               BIO_free(b);
+       }
 
        b = BIO_new_file(x509_name, "rb");
        ERR(!b, "%s", x509_name);
-- 
2.4.0

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

sign-file and detached PKCS#7 firmware signatures

[David Howells]

Hi Luis,

As David Woodhouse pointed out to me, you don't need sign-file if you're just
going to create a detached PKCS#7 message as your signature.  You can just use
"openssl smime" directly.

The reason that sign-file is needed for module signing is that the signature
is added to the module with a little bit of metadata to indicate its presence
- but if you're having detached signatures, that isn't relevant.

You can do this with two steps:

 (1) Require that an X.509 certificate is made available to the kernel to
     provide the public key.  One way to do this is to convert it to DER form
     and place it in the source directory as <name>.x509 when you build the
     kernel.

 (2) Document that to produce a signature for a firmware blob, you just run
     the following command:

		openssl smime -sign \
		 -in $FIRMWARE_BLOB_NAME \
		 -outform DER \
		 -inkey $PRIVATE_KEY_FILE_IN_PEM_FORM \
		 -signer $X509_CERT_FILE_IN_PEM_FORM \
		 -nocerts \
		 -md $DIGEST_ALGORITHM \
		 >$PKCS7_MESSAGE_FILE_IN_DER_FORM

     Note that if you have crypto hardware available that openssl can use, you
     can do that in this command.


To summarise, what you have to present to the kernel is the following:

 (A) A DER-encoded X.509 certificate containing the public key.

 (B) A DER-encoded PKCS#7 message containing the signatures.

 (C) A binary blob that is the detached data for the PKCS#7 message.

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Rusty Russell]

David Howells <dhowells@redhat.com> writes:
> Hi Rusty,

Hi David,

        I try to stick my nose into patches which touch module.c/h: this
doesn't, so am happy for this via another tree (AFAICT doesn't even need
my ack).

Thanks,
Rusty.

> 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.
>
>  (4) Makes it possible to get sign-file to just write out a file containing the
>      PKCS#7 signature blob.  This can be used for debugging and potentially for
>      firmware signing.
>
>  (5) Extract the function that does PKCS#7 signature verification on a blob
>      from the module signing code and put it somewhere more general so that
>      other things, such as firmware signing, can make use of it without
>      depending on module config options.
>
> 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.  Note that David Woodhouse is looking at making
> sign-file work with PKCS#11, so bringing back -s might not be necessary.
>
> They can be found here also:
>
> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7
>
> and are tagged with:
>
> 	modsign-pkcs7-20150515
>
> Should these go via the security tree or your tree?
>
> David
> ---
> David Howells (7):
>       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
>       system_keyring.c doesn't need to #include module-internal.h
>       MODSIGN: Extract the blob PKCS#7 signature verifier from module signing
>
> Luis R. Rodriguez (1):
>       sign-file: Add option to only create signature file
>
>
>  Makefile                                  |    2 
>  crypto/asymmetric_keys/Makefile           |    8 -
>  crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
>  crypto/asymmetric_keys/pkcs7_verify.c     |   80 ++++--
>  crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
>  crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
>  crypto/asymmetric_keys/x509_parser.h      |    5 
>  crypto/asymmetric_keys/x509_public_key.c  |   86 ++++--
>  include/crypto/pkcs7.h                    |    3 
>  include/crypto/public_key.h               |    4 
>  include/keys/system_keyring.h             |    5 
>  init/Kconfig                              |   28 +-
>  kernel/module_signing.c                   |  212 +--------------
>  kernel/system_keyring.c                   |   51 +++-
>  scripts/Makefile                          |    2 
>  scripts/sign-file                         |  421 -----------------------------
>  scripts/sign-file.c                       |  212 +++++++++++++++
>  17 files changed, 578 insertions(+), 728 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 4/4] modsign: Allow signing key to be PKCS#11

[David Howells]

You also need to update Documentation/module-signing.txt

David

Re: sign-file and detached PKCS#7 firmware signatures

[Luis R. Rodriguez]

On Fri, May 15, 2015 at 08:07:55PM +0100, David Howells wrote:
> Hi Luis,
> 
> As David Woodhouse pointed out to me, you don't need sign-file if you're just
> going to create a detached PKCS#7 message as your signature.  You can just use
> "openssl smime" directly.
> 
> The reason that sign-file is needed for module signing is that the signature
> is added to the module with a little bit of metadata to indicate its presence
> - but if you're having detached signatures, that isn't relevant.
> 
> You can do this with two steps:
> 
>  (1) Require that an X.509 certificate is made available to the kernel to
>      provide the public key.  One way to do this is to convert it to DER form
>      and place it in the source directory as <name>.x509 when you build the
>      kernel.

OK.

>  (2) Document that to produce a signature for a firmware blob, you just run
>      the following command:
> 
> 		openssl smime -sign \
> 		 -in $FIRMWARE_BLOB_NAME \
> 		 -outform DER \
> 		 -inkey $PRIVATE_KEY_FILE_IN_PEM_FORM \
> 		 -signer $X509_CERT_FILE_IN_PEM_FORM \
> 		 -nocerts \
> 		 -md $DIGEST_ALGORITHM \

There's a missing -binary argument here, other than that this works fine.

> 		 >$PKCS7_MESSAGE_FILE_IN_DER_FORM

I however cannot figure out how to use openssl to verify this signature.

>      Note that if you have crypto hardware available that openssl can use, you
>      can do that in this command.
> 
> 
> To summarise, what you have to present to the kernel is the following:
> 
>  (A) A DER-encoded X.509 certificate containing the public key.
> 
>  (B) A DER-encoded PKCS#7 message containing the signatures.
> 
>  (C) A binary blob that is the detached data for the PKCS#7 message.

Will respin.

 Lui

Re: [PATCH 1/4] modsign: Abort modules_install when signing fails

[Mimi Zohar]

On Fri, 2015-05-15 at 17:52 +0100, David Woodhouse wrote:
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

I assume the patch descriptions will be added before being upstreamed.  

> ---
>  scripts/Makefile.modinst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index e48a4e9..07650ee 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -22,7 +22,7 @@ quiet_cmd_modules_install = INSTALL $@
>      mkdir -p $(2) ; \
>      cp $@ $(2) ; \
>      $(mod_strip_cmd) $(2)/$(notdir $@) ; \
> -    $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
> +    $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) && \
>      $(mod_compress_cmd) $(2)/$(notdir $@)

With this patch, as expected the modules_install aborted on failure.  Is
there any way to capture the reason for the failure?   In my case,
dropping the '-j <num>' option resolved the problem.

Mimi

>  # Modules built outside the kernel source tree go into extra by default

Re: [PATCH 3/4] modsign: Allow password to be specified for signing key

[Mimi Zohar]

On Fri, 2015-05-15 at 17:53 +0100, David Woodhouse wrote:
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
>  Documentation/module-signing.txt |  2 ++
>  Makefile                         |  1 +
>  init/Kconfig                     |  6 ++++++
>  scripts/sign-file.c              | 39 ++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
> index c72702e..b0ed080 100644
> --- a/Documentation/module-signing.txt
> +++ b/Documentation/module-signing.txt
> @@ -194,6 +194,8 @@ The hash algorithm used does not have to match the one configured, but if it
>  doesn't, you should make sure that hash algorithm is either built into the
>  kernel or can be loaded without requiring itself.
>  
> +If the private key requires a passphrase or PIN, it can be provided in the
> +$CONFIG_MODULE_SIG_KEY_PASSWORD environment variable.

This works, but probably is not a good idea.  For one, if IKCONFIG is
enabled, the pin is readily visible via /proc/config.gz. 

Mimi

>  ============================
>  SIGNED MODULES AND STRIPPING
> diff --git a/Makefile b/Makefile
> index 9590e67..70c066c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -875,6 +875,7 @@ ifdef CONFIG_MODULE_SIG_ALL
>  MODSECKEY = $(CONFIG_MODULE_SIG_KEY)
>  MODPUBKEY = ./signing_key.x509
>  export MODPUBKEY
> +export CONFIG_MODULE_SIG_KEY_PASSWORD
>  mod_sign_cmd = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
>  else
>  mod_sign_cmd = true
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ca075a..7bbc857 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1967,6 +1967,12 @@ config MODULE_SIG_KEY
>           Provide the file name of a private key in PEM format, or a PKCS#11
>           URI according to RFC7512 to specify the key.
>  
> +config MODULE_SIG_KEY_PASSWORD
> +       string "Passphrase or PIN for module signing key if needed" if MODULE_SIG_EXTERNAL_KEY
> +       help
> +         If a passphrase or PIN is required for the private key, provide
> +         it here.
> +
>  config MODULE_COMPRESS
>         bool "Compress modules on installation"
>         depends on MODULES
> diff --git a/scripts/sign-file.c b/scripts/sign-file.c
> index 39aaabe..9a54acc 100755
> --- a/scripts/sign-file.c
> +++ b/scripts/sign-file.c
> @@ -80,9 +80,32 @@ static void drain_openssl_errors(void)
>                 }                                       \
>         } while(0)
>  
> +static char *key_pass;
> +
> +static int pem_pw_cb(char *buf, int len, int w, void *v)
> +{
> +       int pwlen;
> +
> +       if (!key_pass)
> +               return -1;
> +
> +       pwlen = strlen(key_pass);
> +       if (pwlen >= len)
> +               return -1;
> +
> +       strcpy(buf, key_pass);
> +
> +       /* If it's wrong, don't keep trying it. */
> +       free(key_pass);
> +       key_pass = NULL;
> +
> +       return pwlen;
> +}
> +
>  int main(int argc, char **argv)
>  {
>         struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
> +       const char *pass_env;
>         char *hash_algo = NULL;
>         char *private_key_name, *x509_name, *module_name, *dest_name;
>         bool save_pkcs7 = false, replace_orig;
> @@ -96,6 +119,7 @@ int main(int argc, char **argv)
>         BIO *b, *bd = NULL, *bm;
>         int opt, n;
>  
> +       OpenSSL_add_all_algorithms();
>         ERR_load_crypto_strings();
>         ERR_clear_error();
>  
> @@ -127,12 +151,25 @@ int main(int argc, char **argv)
>                 replace_orig = true;
>         }
>  
> +       pass_env = getenv("CONFIG_MODULE_SIG_KEY_PASSWORD");
> +       if (pass_env) {
> +               int pwlen = strlen(pass_env);
> +
> +               if (pass_env[0] == '\"' && pass_env[pwlen - 1] == '\"') {
> +                       pass_env++;
> +                       pwlen -= 2;
> +               }
> +               if (pwlen)
> +                       key_pass = strndup(pass_env, pwlen);
> +       }
> +
>         /* 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);
> +       private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb, NULL);
> +       ERR(!private_key, "%s", private_key_name);
>         BIO_free(b);
>  
>         b = BIO_new_file(x509_name, "rb");
> -- 
> 2.4.0
> 

Re: [PATCH 1/4] modsign: Abort modules_install when signing fails

[Woodhouse, David]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

On Mon, 2015-05-18 at 21:29 -0400, Mimi Zohar wrote:
> On Fri, 2015-05-15 at 17:52 +0100, David Woodhouse wrote:
> > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> 
> I assume the patch descriptions will be added before being upstreamed.  

This patch aborts modules_install when signing fails :)

> With this patch, as expected the modules_install aborted on failure.  Is
> there any way to capture the reason for the failure?   In my case,
> dropping the '-j <num>' option resolved the problem.

Hm, was there no output from sign-file when this happened? Remember that
with a parallel make the error which stops the build might not be the
last thing it printed. Can you show the full output?

It's possible that there's a limit on the number of sessions you can
have open to the hardware token, and we are exceeding it with a parallel
build. I thought that pcscd was going to serialize the access and it
should work properly though. I can certainly do 'make -j
modules_install' with a Yubikey NEO here (although my test build only
has about 20 modules).

Any better ideas on how to specify the key passphrase/PIN? Just put it
in a file in the top-level directory? 

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3437 bytes --]

Re: sign-file and detached PKCS#7 firmware signatures

[David Howells]

Luis R. Rodriguez <mcgrof@suse.com> wrote:

> There's a missing -binary argument here, other than that this works fine.

Good point.

> > 		 >$PKCS7_MESSAGE_FILE_IN_DER_FORM
> 
> I however cannot figure out how to use openssl to verify this signature.

Something like:

	openssl smime -verify \
		-in $PKCS7_MESSAGE_FILE_IN_DER_FORM \
		-inform DER \
		-content $FIRMWARE_BLOB_NAME \
		-inkey $PRIVATE_KEY_FILE_IN_PEM_FORM \
		-signer $X509_CERT_FILE_IN_PEM_FORM

I would guess.

David

Re: [PATCH 1/4] modsign: Abort modules_install when signing fails

[Mimi Zohar]

On Tue, 2015-05-19 at 06:40 +0000, Woodhouse, David wrote:
> On Mon, 2015-05-18 at 21:29 -0400, Mimi Zohar wrote:
> > On Fri, 2015-05-15 at 17:52 +0100, David Woodhouse wrote:
> > > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>


> > With this patch, as expected the modules_install aborted on failure.  Is
> > there any way to capture the reason for the failure?   In my case,
> > dropping the '-j <num>' option resolved the problem.

My mistake the failure was there. 

> Hm, was there no output from sign-file when this happened? Remember that
> with a parallel make the error which stops the build might not be the
> last thing it printed. Can you show the full output?

/bin/sh: line 1: 22771 Segmentation fault      (core dumped) scripts/sign-file "sha256" "pkcs11:manufacturer=piv_II;id=%01" ./signing_key.x509 /lib/modules/4.1.0-rc1-test+/kernel/net/ipv6/netfilter/ip6table_filter.ko
/home/zohar/src/kernel/linux-integrity/scripts/Makefile.modinst:35: recipe for target 'net/ipv6/netfilter/ip6table_filter.ko' failed
make[2]: *** [net/ipv6/netfilter/ip6table_filter.ko] Error 139
make[2]: *** Waiting for unfinished jobs....
/bin/sh: line 1: 22842 Segmentation fault      (core dumped) scripts/sign-file "sha256" "pkcs11:manufacturer=piv_II;id=%01" ./signing_key.x509 /lib/modules/4.1.0-rc1-test+/kernel/net/netfilter/nf_nat.ko
/home/zohar/src/kernel/linux-integrity/scripts/Makefile.modinst:35: recipe for target 'net/netfilter/nf_nat.ko' failed
make[2]: *** [net/netfilter/nf_nat.ko] Error 139
/home/zohar/src/kernel/linux-integrity/Makefile:1123: recipe for target '_modinst_' failed
make[1]: *** [_modinst_] Error 2
make[1]: Leaving directory '/home/zohar/src/kernel/build/linux-test'
Makefile:146: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2

> It's possible that there's a limit on the number of sessions you can
> have open to the hardware token, and we are exceeding it with a parallel
> build. I thought that pcscd was going to serialize the access and it
> should work properly though. I can certainly do 'make -j
> modules_install' with a Yubikey NEO here (although my test build only
> has about 20 modules).
> 
> Any better ideas on how to specify the key passphrase/PIN? Just put it
> in a file in the top-level directory? 

Define a kbuild command parameter?

Mimi

Re: [PATCH 1/4] modsign: Abort modules_install when signing fails

[Woodhouse, David]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

On Tue, 2015-05-19 at 07:45 -0400, Mimi Zohar wrote:
> 
> /bin/sh: line 1: 22771 Segmentation fault      (core dumped) 
> scripts/sign-file "sha256" "pkcs11:manufacturer=piv_II;id=%01" 
> ./signing_key.x509 /lib/modules/4.1.0-rc1
> -test+/kernel/net/ipv6/netfilter/ip6table_filter.ko
> /home/zohar/src/kernel/linux-integrity/scripts/Makefile.modinst:35: 
> recipe for target 'net/ipv6/netfilter/ip6table_filter.ko' failed
> make[2]: *** [net/ipv6/netfilter/ip6table_filter.ko] Error 139

Hm, can you get a backtrace of that? It would be impressive if that
were a bug in the very few lines of code we have in sign-file.c to
handle the engine; I suspect it's more likely to be elsewhere in the
OpenSSL/p11-kit-proxy/OpenSC/pcsc stack.

It only happens with 'make -j', yes?


-- 
                  Sent with Evolution's ActiveSync support.

David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3437 bytes --]

Re: [PATCH 1/4] modsign: Abort modules_install when signing fails

[Mimi Zohar]

On Tue, 2015-05-19 at 12:57 +0000, Woodhouse, David wrote:
> On Tue, 2015-05-19 at 07:45 -0400, Mimi Zohar wrote:
> > 
> > /bin/sh: line 1: 22771 Segmentation fault      (core dumped) 
> > scripts/sign-file "sha256" "pkcs11:manufacturer=piv_II;id=%01" 
> > ./signing_key.x509 /lib/modules/4.1.0-rc1
> > -test+/kernel/net/ipv6/netfilter/ip6table_filter.ko
> > /home/zohar/src/kernel/linux-integrity/scripts/Makefile.modinst:35: 
> > recipe for target 'net/ipv6/netfilter/ip6table_filter.ko' failed
> > make[2]: *** [net/ipv6/netfilter/ip6table_filter.ko] Error 139
> 
> Hm, can you get a backtrace of that? It would be impressive if that
> were a bug in the very few lines of code we have in sign-file.c to
> handle the engine; I suspect it's more likely to be elsewhere in the
> OpenSSL/p11-kit-proxy/OpenSC/pcsc stack.
> 
> It only happens with 'make -j', yes?

To be exact, "-j 10".

Mimi

[PATCH 9/8] modsign: Abort modules_install when signing fails

[David Woodhouse]

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 scripts/Makefile.modinst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index e48a4e9..07650ee 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -22,7 +22,7 @@ quiet_cmd_modules_install = INSTALL $@
     mkdir -p $(2) ; \
     cp $@ $(2) ; \
     $(mod_strip_cmd) $(2)/$(notdir $@) ; \
-    $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
+    $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) && \
     $(mod_compress_cmd) $(2)/$(notdir $@)
 
 # Modules built outside the kernel source tree go into extra by default
-- 
2.4.0


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[PATCH 10/8] modsign: Allow password to be specified for signing key

[David Woodhouse]

We don't want this in the Kconfig since it might then get exposed in
/proc/config.gz. So make it a parameter to Kbuild instead. This also
means we don't have to jump through hoops to strip quotes from it, as
we would if it was a config option.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 Documentation/kbuild/kbuild.txt  |  5 +++++
 Documentation/module-signing.txt |  3 +++
 scripts/sign-file.c              | 27 ++++++++++++++++++++++++++-
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
index 6466704..0ff6a46 100644
--- a/Documentation/kbuild/kbuild.txt
+++ b/Documentation/kbuild/kbuild.txt
@@ -174,6 +174,11 @@ The output directory is often set using "O=..." on the commandline.
 
 The value can be overridden in which case the default value is ignored.
 
+KBUILD_SIGN_PIN
+--------------------------------------------------
+This variable allows a passphrase or PIN to be passed to the sign-file
+utility when signing kernel modules, if the private key requires such.
+
 KBUILD_MODPOST_WARN
 --------------------------------------------------
 KBUILD_MODPOST_WARN can be set to avoid errors in case of undefined
diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index c72702e..faaa6ea 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -194,6 +194,9 @@ The hash algorithm used does not have to match the one configured, but if it
 doesn't, you should make sure that hash algorithm is either built into the
 kernel or can be loaded without requiring itself.
 
+If the private key requires a passphrase or PIN, it can be provided in the
+$KBUILD_SIGN_PIN environment variable.
+
 
 ============================
 SIGNED MODULES AND STRIPPING
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 39aaabe..720b9bc 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -80,6 +80,27 @@ static void drain_openssl_errors(void)
 		}					\
 	} while(0)
 
+static const char *key_pass;
+
+static int pem_pw_cb(char *buf, int len, int w, void *v)
+{
+	int pwlen;
+
+	if (!key_pass)
+		return -1;
+
+	pwlen = strlen(key_pass);
+	if (pwlen >= len)
+		return -1;
+
+	strcpy(buf, key_pass);
+
+	/* If it's wrong, don't keep trying it. */
+	key_pass = NULL;
+
+	return pwlen;
+}
+
 int main(int argc, char **argv)
 {
 	struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
@@ -96,9 +117,12 @@ int main(int argc, char **argv)
 	BIO *b, *bd = NULL, *bm;
 	int opt, n;
 
+	OpenSSL_add_all_algorithms();
 	ERR_load_crypto_strings();
 	ERR_clear_error();
 
+	key_pass = getenv("KBUILD_SIGN_PIN");
+
 	do {
 		opt = getopt(argc, argv, "dp");
 		switch (opt) {
@@ -132,7 +156,8 @@ int main(int argc, char **argv)
 	 */
 	b = BIO_new_file(private_key_name, "rb");
 	ERR(!b, "%s", private_key_name);
-        private_key = PEM_read_bio_PrivateKey(b, NULL, NULL, NULL);
+	private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb, NULL);
+	ERR(!private_key, "%s", private_key_name);
 	BIO_free(b);
 
 	b = BIO_new_file(x509_name, "rb");
-- 
2.4.0


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[PATCH 11/8] modsign: Allow signing key to be PKCS#11

[David Woodhouse]

This is only the key; the corresponding *cert* still needs to be in
$(topdir)/signing_key.x509. And there's no way to actually use this
from the build system yet.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 scripts/sign-file.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 720b9bc..ad0aa21 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -22,6 +22,7 @@
 #include <openssl/pem.h>
 #include <openssl/pkcs7.h>
 #include <openssl/err.h>
+#include <openssl/engine.h>
 
 struct module_signature {
 	uint8_t		algo;		/* Public-key crypto algorithm [0] */
@@ -154,11 +155,29 @@ int main(int argc, char **argv)
 	/* 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, pem_pw_cb, NULL);
-	ERR(!private_key, "%s", private_key_name);
-	BIO_free(b);
+	if (!strncmp(private_key_name, "pkcs11:", 7)) {
+		ENGINE *e;
+
+		ENGINE_load_builtin_engines();
+		drain_openssl_errors();
+		e = ENGINE_by_id("pkcs11");
+		ERR(!e, "Load PKCS#11 ENGINE");
+		if (ENGINE_init(e))
+			drain_openssl_errors();
+		else
+			ERR(1, "ENGINE_init");
+		if (key_pass)
+			ERR(!ENGINE_ctrl_cmd_string(e, "PIN", key_pass, 0), "Set PKCS#11 PIN");
+		private_key = ENGINE_load_private_key(e, private_key_name, NULL,
+						      NULL);
+		ERR(!private_key, "%s", private_key_name);
+	} else {
+		b = BIO_new_file(private_key_name, "rb");
+		ERR(!b, "%s", private_key_name);
+		private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb, NULL);
+		ERR(!private_key, "%s", private_key_name);
+		BIO_free(b);
+	}
 
 	b = BIO_new_file(x509_name, "rb");
 	ERR(!b, "%s", x509_name);
-- 
2.4.0


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[PATCH 12/8] modsign: Allow external signing key to be specified

[David Woodhouse]

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 Documentation/module-signing.txt | 31 ++++++++++++++++++++++++++-----
 Makefile                         |  2 +-
 init/Kconfig                     | 14 ++++++++++++++
 kernel/Makefile                  |  5 +++++
 4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index faaa6ea..84597c7 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -88,6 +88,22 @@ This has a number of options available:
      than being a module) so that modules signed with that algorithm can have
      their signatures checked without causing a dependency loop.
 
+ (4) "File name or PKCS#11 URI of module signing key" (CONFIG_MODULE_SIG_KEY)
+
+     Setting this option to something other than its default of
+     "signing_key.priv" will disable the autogeneration of signing keys and
+     allow the kernel modules to be signed with a key of your choosing.
+     The string provided should identify a file containing a private key
+     in PEM form, or — on systems where the OpenSSL ENGINE_pkcs11 is
+     appropriately installed — a PKCS#11 URI as defined by RFC7512.
+
+     If the PEM file containing the private key is encrypted, or if the
+     PKCS#11 token requries a PIN, this can be provided at build time by
+     means of the KBUILD_SIGN_PIN variable.
+
+     The corresponding X.509 certificate in DER form should still be placed
+     in a file named signing_key.x509 in the top-level build directory.
+
 
 =======================
 GENERATING SIGNING KEYS
@@ -100,8 +116,9 @@ it can be deleted or stored securely.  The public key gets built into the
 kernel so that it can be used to check the signatures as the modules are
 loaded.
 
-Under normal conditions, the kernel build will automatically generate a new
-keypair using openssl if one does not exist in the files:
+Under normal conditions, when CONFIG_MODULE_SIG_KEY is unchanged from its
+default of "signing_key.priv", the kernel build will automatically generate
+a new keypair using openssl if one does not exist in the files:
 
 	signing_key.priv
 	signing_key.x509
@@ -135,8 +152,12 @@ kernel sources tree and the openssl command.  The following is an example to
 generate the public/private key files:
 
 	openssl req -new -nodes -utf8 -sha256 -days 36500 -batch -x509 \
-	   -config x509.genkey -outform DER -out signing_key.x509 \
-	   -keyout signing_key.priv
+	   -config x509.genkey -outform PEM -out kernel_key.pem \
+	   -keyout kernel_key.pem
+
+The full pathname for the resulting kernel_key.pem file can then be specified
+in the CONFIG_MODULE_SIG_KEY option, and the certificate and key therein will
+be used instead of an autogenerated keypair.
 

 =========================
@@ -181,7 +202,7 @@ To manually sign a module, use the scripts/sign-file tool available in
 the Linux kernel source tree.  The script requires 4 arguments:
 
 	1.  The hash algorithm (e.g., sha256)
-	2.  The private key filename
+	2.  The private key filename or PKCS#11 URI
 	3.  The public key filename
 	4.  The kernel module to be signed
 
diff --git a/Makefile b/Makefile
index ad27fc9..9590e67 100644
--- a/Makefile
+++ b/Makefile
@@ -872,7 +872,7 @@ INITRD_COMPRESS-$(CONFIG_RD_LZ4)   := lz4
 # export INITRD_COMPRESS := $(INITRD_COMPRESS-y)
 
 ifdef CONFIG_MODULE_SIG_ALL
-MODSECKEY = ./signing_key.priv
+MODSECKEY = $(CONFIG_MODULE_SIG_KEY)
 MODPUBKEY = ./signing_key.x509
 export MODPUBKEY
 mod_sign_cmd = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODSECKEY) $(MODPUBKEY)
diff --git a/init/Kconfig b/init/Kconfig
index c05afd6..3851815 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1954,6 +1954,20 @@ config MODULE_SIG_HASH
 	default "sha384" if MODULE_SIG_SHA384
 	default "sha512" if MODULE_SIG_SHA512
 
+config MODULE_SIG_KEY
+	string "File name or PKCS#11 URI of module signing key"
+	default "signing_key.priv"
+	depends on MODULE_SIG
+	help
+         Provide the file name of a private key in PKCS#8 PEM format, or
+         a PKCS#11 URI according to RFC7512. The corresponding X.509
+         certificate in DER form should be present in signing_key.x509
+         in the top-level build directory.
+
+         If this option is unchanged from its default "signing_key.priv",
+         then the kernel will automatically generate the private key and
+         certificate as described in Documentation/module-signing.txt
+
 config MODULE_COMPRESS
 	bool "Compress modules on installation"
 	depends on MODULES
diff --git a/kernel/Makefile b/kernel/Makefile
index 60c302c..050a55e 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -170,6 +170,10 @@ ifndef CONFIG_MODULE_SIG_HASH
 $(error Could not determine digest type to use from kernel config)
 endif
 
+# We do it this way rather than having a boolean option for enabling an
+# external private key, because 'make randconfig' might enable such a
+# boolean option and we unfortunately can't make it depend on !RANDCONFIG.
+ifeq ($(CONFIG_MODULE_SIG_KEY),"signing_key.priv")
 signing_key.priv signing_key.x509: x509.genkey
 	@echo "###"
 	@echo "### Now generating an X.509 key pair to be used for signing modules."
@@ -207,3 +211,4 @@ x509.genkey:
 	@echo >>x509.genkey "subjectKeyIdentifier=hash"
 	@echo >>x509.genkey "authorityKeyIdentifier=keyid"
 endif
+endif
-- 
2.4.0


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[PATCH 13/8] modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed

[David Woodhouse]

Where an external PEM file or PKCS#11 URI is given, we can get the cert
from it for ourselves instead of making the user drop signing_key.x509
in place for us.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
The make rules are perhaps a little more baroque than I first intended.
I wonder if I really need to cope with filenames with spaces in. I note
that the *.x509 handling doesn't — if I create a file named 'foo bar.x509'
the build just craps out...

 Documentation/module-signing.txt |  11 ++-
 init/Kconfig                     |   8 +--
 kernel/Makefile                  |  38 +++++++++++
 scripts/Makefile                 |   3 +-
 scripts/extract-cert.c           | 144 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 193 insertions(+), 11 deletions(-)
 create mode 100644 scripts/extract-cert.c

diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index 84597c7..6930019 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -93,17 +93,16 @@ This has a number of options available:
      Setting this option to something other than its default of
      "signing_key.priv" will disable the autogeneration of signing keys and
      allow the kernel modules to be signed with a key of your choosing.
-     The string provided should identify a file containing a private key
-     in PEM form, or — on systems where the OpenSSL ENGINE_pkcs11 is
-     appropriately installed — a PKCS#11 URI as defined by RFC7512.
+     The string provided should identify a file containing both a private
+     key and its corresponding X.509 certificate in PEM form, or — on
+     systems where the OpenSSL ENGINE_pkcs11 is functional — a PKCS#11 URI
+     as defined by RFC7512. In the latter case, the PKCS#11 URI should
+     reference both a certificate and a private key.
 
      If the PEM file containing the private key is encrypted, or if the
      PKCS#11 token requries a PIN, this can be provided at build time by
      means of the KBUILD_SIGN_PIN variable.
 
-     The corresponding X.509 certificate in DER form should still be placed
-     in a file named signing_key.x509 in the top-level build directory.
-
 
 =======================
 GENERATING SIGNING KEYS
diff --git a/init/Kconfig b/init/Kconfig
index 3851815..a4b4f39 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1959,10 +1959,10 @@ config MODULE_SIG_KEY
 	default "signing_key.priv"
 	depends on MODULE_SIG
 	help
-         Provide the file name of a private key in PKCS#8 PEM format, or
-         a PKCS#11 URI according to RFC7512. The corresponding X.509
-         certificate in DER form should be present in signing_key.x509
-         in the top-level build directory.
+         Provide the file name of a private key/certificate in PEM format,
+         or a PKCS#11 URI according to RFC7512. The file should contain, or
+         the URI should identify, both the certificate and its corresponding
+         private key.
 
          If this option is unchanged from its default "signing_key.priv",
          then the kernel will automatically generate the private key and
diff --git a/kernel/Makefile b/kernel/Makefile
index 050a55e..9fb259d 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -210,5 +210,43 @@ x509.genkey:
 	@echo >>x509.genkey "keyUsage=digitalSignature"
 	@echo >>x509.genkey "subjectKeyIdentifier=hash"
 	@echo >>x509.genkey "authorityKeyIdentifier=keyid"
+else
+# For external (PKCS#11 or PEM) key, we need to obtain the certificate from
+# CONFIG_MODULE_SIG_KEY automatically.
+quiet_cmd_extract_der = CERT_DER $(2)
+      cmd_extract_der = scripts/extract-cert "$(2)" signing_key.x509
+
+# CONFIG_MODULE_SIG_KEY is either a PKCS#11 URI or a filename. It is
+# surrounded by quotes, and may contain spaces. To strip the quotes
+# with $(patsubst) we need to turn the spaces into something else.
+# And if it's a filename, those spaces need to be escaped as '\ ' in
+# order to use it in dependencies or $(wildcard).
+space :=
+space +=
+space_escape := %%%SPACE%%%
+X509_SOURCE_temp := $(subst $(space),$(space_escape),$(CONFIG_MODULE_SIG_KEY))
+# We need this to check for absolute paths or PKCS#11 URIs.
+X509_SOURCE_ONEWORD := $(patsubst "%",%,$(X509_SOURCE_temp))
+# This is the actual source filename/URI without the quotes
+X509_SOURCE := $(subst $(space_escape),$(space),$(X509_SOURCE_ONEWORD))
+# This\ version\ with\ spaces\ escaped\ for\ $(wildcard)\ and\ dependencies
+X509_SOURCE_ESCAPED := $(subst $(space_escape),\$(space),$(X509_SOURCE_ONEWORD))
+
+ifeq ($(patsubst pkcs11:%,%,$(X509_SOURCE_ONEWORD)),$(X509_SOURCE_ONEWORD))
+# If it's a filename, depend on it.
+X509_DEP := $(X509_SOURCE_ESCAPED)
+ifeq ($(patsubst /%,%,$(X509_SOURCE_ONEWORD)),$(X509_SOURCE_ONEWORD))
+ifeq ($(wildcard $(X509_SOURCE_ESCAPED)),)
+ifneq ($(wildcard $(srctree)/$(X509_SOURCE_ESCAPED)),)
+# Non-absolute filename, found in source tree and not build tree
+X509_SOURCE := $(srctree)/$(X509_SOURCE)
+X509_DEP := $(srctree)/$(X509_SOURCE_ESCAPED)
+endif
+endif
+endif
+endif
+
+signing_key.x509: scripts/extract-cert include/config/module/sig/key.h $(X509_DEP)
+	$(call cmd,extract_der,$(X509_SOURCE))
 endif
 endif
diff --git a/scripts/Makefile b/scripts/Makefile
index b12fe02..236f683 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -16,11 +16,12 @@ 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
+hostprogs-$(CONFIG_MODULE_SIG)	 += sign-file extract-cert
 
 HOSTCFLAGS_sortextable.o = -I$(srctree)/tools/include
 HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
 HOSTLOADLIBES_sign-file = -lcrypto
+HOSTLOADLIBES_extract-cert = -lcrypto
 
 always		:= $(hostprogs-y) $(hostprogs-m)
 
diff --git a/scripts/extract-cert.c b/scripts/extract-cert.c
new file mode 100644
index 0000000..3faeff1
--- /dev/null
+++ b/scripts/extract-cert.c
@@ -0,0 +1,144 @@
+/* Extract X.509 certificate in DER form from PKCS#11 or PEM.
+ *
+ * Copyright © 2014 Red Hat, Inc. All Rights Reserved.
+ * Copyright © 2015 Intel Corporation.
+ *
+ * Authors: David Howells <dhowells@redhat.com>
+ *          David Woodhouse <dwmw2@infradead.org>
+ *
+ * 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>
+#include <openssl/engine.h>
+
+#define PKEY_ID_PKCS7 2
+
+static __attribute__((noreturn))
+void format(void)
+{
+	fprintf(stderr,
+		"Usage: scripts/extract-cert <source> <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);
+	}
+}
+
+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, fmt, ...)				\
+	do {						\
+		bool __cond = (cond);			\
+		display_openssl_errors(__LINE__);	\
+		if (__cond) {				\
+			err(1, fmt, ## __VA_ARGS__);	\
+		}					\
+	} while(0)
+
+static char *key_pass;
+
+int main(int argc, char **argv)
+{
+	const char *pass_env;
+	char *cert_src, *cert_dst;
+	X509 *x509;
+	BIO *b;
+
+	OpenSSL_add_all_algorithms();
+	ERR_load_crypto_strings();
+	ERR_clear_error();
+
+	if (argc != 3)
+		format();
+
+	cert_src = argv[1];
+	cert_dst = argv[2];
+
+	/* If we add PKCS#12 support we'll need this... */
+	pass_env = getenv("CONFIG_MODULE_SIG_KEY_PASSWORD");
+	if (pass_env) {
+		int pwlen = strlen(pass_env);
+
+		if (pass_env[0] == '\"' && pass_env[pwlen - 1] == '\"') {
+			pass_env++;
+			pwlen -= 2;
+		}
+		if (pwlen)
+			key_pass = strndup(pass_env, pwlen);
+	}
+
+	if (!strncmp(cert_src, "pkcs11:", 7)) {
+		ENGINE *e;
+		struct {
+			const char *cert_id;
+			X509 *cert;
+		} parms;
+
+		parms.cert_id = cert_src;
+		parms.cert = NULL;
+
+		ENGINE_load_builtin_engines();
+		drain_openssl_errors();
+		e = ENGINE_by_id("pkcs11");
+		ERR(!e, "Load PKCS#11 ENGINE");
+		if (ENGINE_init(e))
+			drain_openssl_errors();
+		else
+			ERR(1, "ENGINE_init");
+		if (key_pass)
+			ERR(!ENGINE_ctrl_cmd_string(e, "PIN", key_pass, 0), "Set PKCS#11 PIN");
+		ENGINE_ctrl_cmd(e, "LOAD_CERT_CTRL", 0, &parms, NULL, 1);
+		ERR(!parms.cert, "Get X.509 from PKCS#11");
+		x509 = parms.cert;
+	} else {
+		b = BIO_new_file(cert_src, "rb");
+		ERR(!b, "%s", cert_src);
+		x509 = PEM_read_bio_X509(b, NULL, NULL, NULL);
+		ERR(!x509, "%s", cert_src);
+		BIO_free(b);
+	}
+
+	b = BIO_new_file(cert_dst, "wb");
+	ERR(!b, "%s", cert_dst);
+	ERR(!i2d_X509_bio(b, x509), cert_dst);
+	BIO_free(b);
+
+	return 0;
+}
-- 
2.4.0


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[David Howells]

David Woodhouse <dwmw2@infradead.org> wrote:

> We don't want this in the Kconfig since it might then get exposed in
> /proc/config.gz. So make it a parameter to Kbuild instead. This also
> means we don't have to jump through hoops to strip quotes from it, as
> we would if it was a config option.

Hi Mimi,

Are you okay with this?

David

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[Petko Manolov]

On 15-05-19 15:45:58, David Woodhouse wrote:
> We don't want this in the Kconfig since it might then get exposed in
> /proc/config.gz. So make it a parameter to Kbuild instead. This also
> means we don't have to jump through hoops to strip quotes from it, as
> we would if it was a config option.

If it were on a network-less, secure sign/build server i'd say it is OK.  

However, exposing your private key's password in an environment variable on a 
regular Linux box is a bit fishy.


cheers,
Petko

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[David Woodhouse]

On Tue, 2015-05-19 at 18:50 +0300, Petko Manolov wrote:
> On 15-05-19 15:45:58, David Woodhouse wrote:
> > We don't want this in the Kconfig since it might then get exposed in
> > /proc/config.gz. So make it a parameter to Kbuild instead. This also
> > means we don't have to jump through hoops to strip quotes from it, as
> > we would if it was a config option.
> 
> If it were on a network-less, secure sign/build server i'd say it is OK.  
> 
> However, exposing your private key's password in an environment variable on a 
> regular Linux box is a bit fishy.

I don't quite understand the objection.

If you want the modules to be signed with an external key of your
choice, then for the duration of the 'make modules_sign' run (or 'make
modules_install if CONFIG_MODULE_SIG_ALL=y) surely the password has to
be available *somehow*? 

You are, of course, free to sign the modules by invoking sign-file
directly. In which case you *still* need to provide it with the password
for the key somehow, if there is one.

Mimi quite rightly pointed out that my original mechanism for this, a
CONFIG_MODULE_SIG_KEY_PASSWORD option, was inadvertently exposing it
more than was necessary.

As it is now, you *only* need it in the environment for the duration of
the operations that actually *use* it.

Do you have a better suggestion?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: sign-file and detached PKCS#7 firmware signatures

[Luis R. Rodriguez]

On Tue, May 19, 2015 at 10:25:24AM +0100, David Howells wrote:
> Luis R. Rodriguez <mcgrof@suse.com> wrote:
> 
> > There's a missing -binary argument here, other than that this works fine.
> 
> Good point.
> 
> > > 		 >$PKCS7_MESSAGE_FILE_IN_DER_FORM
> > 
> > I however cannot figure out how to use openssl to verify this signature.
> 
> Something like:
> 
> 	openssl smime -verify \
> 		-in $PKCS7_MESSAGE_FILE_IN_DER_FORM \
> 		-inform DER \
> 		-content $FIRMWARE_BLOB_NAME \
> 		-inkey $PRIVATE_KEY_FILE_IN_PEM_FORM \
> 		-signer $X509_CERT_FILE_IN_PEM_FORM
> 
> I would guess.

I tried a few things and no luck with that.

  Luis

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[Petko Manolov]

On 15-05-19 17:15:12, David Woodhouse wrote:
> On Tue, 2015-05-19 at 18:50 +0300, Petko Manolov wrote:
> > On 15-05-19 15:45:58, David Woodhouse wrote:
> > > We don't want this in the Kconfig since it might then get exposed in
> > > /proc/config.gz. So make it a parameter to Kbuild instead. This also
> > > means we don't have to jump through hoops to strip quotes from it, as
> > > we would if it was a config option.
> > 
> > If it were on a network-less, secure sign/build server i'd say it is OK.  
> > 
> > However, exposing your private key's password in an environment variable on a 
> > regular Linux box is a bit fishy.
> 
> I don't quite understand the objection.
> 
> If you want the modules to be signed with an external key of your
> choice, then for the duration of the 'make modules_sign' run (or 'make
> modules_install if CONFIG_MODULE_SIG_ALL=y) surely the password has to
> be available *somehow*? 
> 
> You are, of course, free to sign the modules by invoking sign-file
> directly. In which case you *still* need to provide it with the password
> for the key somehow, if there is one.
> 
> Mimi quite rightly pointed out that my original mechanism for this, a
> CONFIG_MODULE_SIG_KEY_PASSWORD option, was inadvertently exposing it
> more than was necessary.
> 
> As it is now, you *only* need it in the environment for the duration of
> the operations that actually *use* it.

As with everything there is bad and good side to your proposal.

bad:
  - password in environment variable _could_ be very dangerous;
  - someone is bound to misuse this feature sooner or later;

good:
  - the actual risk is mitigated as the key is very short-lived;
  - the feature is going to be used by a small number of people;
  - does not break automated builds, maybe;
  - there is an alternative for those who want more secure approach;

> Do you have a better suggestion?

*better* is a matter of prospective.  Security and convenience are at the wrong 
side of the spectrum relative to each other. :)

Don't get me wrong, your patch is perhaps the lesser evil.  I just wanted to 
bring up my concerns.


cheers,
Petko

Re: sign-file and detached PKCS#7 firmware signatures

[David Howells]

Luis R. Rodriguez <mcgrof@suse.com> wrote:

> > Something like:
> > 
> > 	openssl smime -verify \
> > 		-in $PKCS7_MESSAGE_FILE_IN_DER_FORM \
> > 		-inform DER \
> > 		-content $FIRMWARE_BLOB_NAME \
> > 		-inkey $PRIVATE_KEY_FILE_IN_PEM_FORM \
> > 		-signer $X509_CERT_FILE_IN_PEM_FORM
> > 
> > I would guess.
> 
> I tried a few things and no luck with that.

Try this:

	openssl smime -verify \
		-in $PKCS7_MESSAGE_FILE_IN_DER_FORM \
		-inform DER \
		-certfile $X509_CERT_FILE_IN_PEM_FORM \
 		-content $FIRMWARE_BLOB_NAME \
		-binary \
		-noverify \
		>/dev/null

Seems using -signer with -verify isn't a good idea as -signer refers to an
output file during verification just for the surprise factor.

David

Re: sign-file and detached PKCS#7 firmware signatures

[Luis R. Rodriguez]

On Tue, May 19, 2015 at 05:48:57PM +0100, David Howells wrote:
> Luis R. Rodriguez <mcgrof@suse.com> wrote:
> 
> > > Something like:
> > > 
> > > 	openssl smime -verify \
> > > 		-in $PKCS7_MESSAGE_FILE_IN_DER_FORM \
> > > 		-inform DER \
> > > 		-content $FIRMWARE_BLOB_NAME \
> > > 		-inkey $PRIVATE_KEY_FILE_IN_PEM_FORM \
> > > 		-signer $X509_CERT_FILE_IN_PEM_FORM
> > > 
> > > I would guess.
> > 
> > I tried a few things and no luck with that.
> 
> Try this:
> 
> 	openssl smime -verify \
> 		-in $PKCS7_MESSAGE_FILE_IN_DER_FORM \
> 		-inform DER \
> 		-certfile $X509_CERT_FILE_IN_PEM_FORM \
>  		-content $FIRMWARE_BLOB_NAME \
> 		-binary \
> 		-noverify \
> 		>/dev/null

Neat ok yeah this works thanks, I'll throw this in the docs too
with the change to  use -out /dev/null.

  Luis

Re: sign-file and detached PKCS#7 firmware signatures

[Luis R. Rodriguez]

I'll also mention:

---
The $DIGEST_ALGORITHM needs to be supported on the running kernel and           
can differ from CONFIG_MODULE_SIG_HASH.
---

As I do no think that is quite obvious to a system integrator at first.

 Luis

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[Mimi Zohar]

On Tue, 2015-05-19 at 15:45 +0100, David Woodhouse wrote:
> We don't want this in the Kconfig since it might then get exposed in
> /proc/config.gz. So make it a parameter to Kbuild instead. This also
> means we don't have to jump through hoops to strip quotes from it, as
> we would if it was a config option.

Definitely better.  (FYI, Dmitry's modsig patches from 2012 used the
keyring for safely storing a password. )

Mimi

> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
>  Documentation/kbuild/kbuild.txt  |  5 +++++
>  Documentation/module-signing.txt |  3 +++
>  scripts/sign-file.c              | 27 ++++++++++++++++++++++++++-
>  3 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
> index 6466704..0ff6a46 100644
> --- a/Documentation/kbuild/kbuild.txt
> +++ b/Documentation/kbuild/kbuild.txt
> @@ -174,6 +174,11 @@ The output directory is often set using "O=..." on the commandline.
> 
>  The value can be overridden in which case the default value is ignored.
> 
> +KBUILD_SIGN_PIN
> +--------------------------------------------------
> +This variable allows a passphrase or PIN to be passed to the sign-file
> +utility when signing kernel modules, if the private key requires such.
> +
>  KBUILD_MODPOST_WARN
>  --------------------------------------------------
>  KBUILD_MODPOST_WARN can be set to avoid errors in case of undefined
> diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
> index c72702e..faaa6ea 100644
> --- a/Documentation/module-signing.txt
> +++ b/Documentation/module-signing.txt
> @@ -194,6 +194,9 @@ The hash algorithm used does not have to match the one configured, but if it
>  doesn't, you should make sure that hash algorithm is either built into the
>  kernel or can be loaded without requiring itself.
> 
> +If the private key requires a passphrase or PIN, it can be provided in the
> +$KBUILD_SIGN_PIN environment variable.
> +
> 
>  ============================
>  SIGNED MODULES AND STRIPPING
> diff --git a/scripts/sign-file.c b/scripts/sign-file.c
> index 39aaabe..720b9bc 100755
> --- a/scripts/sign-file.c
> +++ b/scripts/sign-file.c
> @@ -80,6 +80,27 @@ static void drain_openssl_errors(void)
>  		}					\
>  	} while(0)
> 
> +static const char *key_pass;
> +
> +static int pem_pw_cb(char *buf, int len, int w, void *v)
> +{
> +	int pwlen;
> +
> +	if (!key_pass)
> +		return -1;
> +
> +	pwlen = strlen(key_pass);
> +	if (pwlen >= len)
> +		return -1;
> +
> +	strcpy(buf, key_pass);
> +
> +	/* If it's wrong, don't keep trying it. */
> +	key_pass = NULL;
> +
> +	return pwlen;
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
> @@ -96,9 +117,12 @@ int main(int argc, char **argv)
>  	BIO *b, *bd = NULL, *bm;
>  	int opt, n;
> 
> +	OpenSSL_add_all_algorithms();
>  	ERR_load_crypto_strings();
>  	ERR_clear_error();
> 
> +	key_pass = getenv("KBUILD_SIGN_PIN");
> +
>  	do {
>  		opt = getopt(argc, argv, "dp");
>  		switch (opt) {
> @@ -132,7 +156,8 @@ int main(int argc, char **argv)
>  	 */
>  	b = BIO_new_file(private_key_name, "rb");
>  	ERR(!b, "%s", private_key_name);
> -        private_key = PEM_read_bio_PrivateKey(b, NULL, NULL, NULL);
> +	private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb, NULL);
> +	ERR(!private_key, "%s", private_key_name);
>  	BIO_free(b);
> 
>  	b = BIO_new_file(x509_name, "rb");
> -- 
> 2.4.0
> 
> 

Re: sign-file and detached PKCS#7 firmware signatures

[David Howells]

Luis R. Rodriguez <mcgrof@suse.com> wrote:

> I'll also mention:
> 
> ---
> The $DIGEST_ALGORITHM needs to be supported on the running kernel and           
> can differ from CONFIG_MODULE_SIG_HASH.
> ---
> 
> As I do no think that is quite obvious to a system integrator at first.

Actually, this isn't necessarily so for the firmware.

It *is* for the module signing, but you can always load the module to give you
the digest algorithm (or public key algorithm) for the firmware.

Though you would still have to be careful with firmware loaded during the
initramfs phase.

David

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[David Howells]

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

> Definitely better.  (FYI, Dmitry's modsig patches from 2012 used the
> keyring for safely storing a password. )

Can I add that as a reviewed-by for Dave?

David

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[Mimi Zohar]

On Tue, 2015-05-19 at 19:48 +0100, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > Definitely better.  (FYI, Dmitry's modsig patches from 2012 used the
> > keyring for safely storing a password. )

Without the environment variable set, there's a pop up prompt to enter
the pin.  A pain to have to enter for each and every kernel module, but
definitely a nice option.

> Can I add that as a reviewed-by for Dave?

Sure

Mimi

Re: [PATCH 10/8] modsign: Allow password to be specified for signing key

[David Woodhouse]

On Tue, 2015-05-19 at 15:14 -0400, Mimi Zohar wrote:
> On Tue, 2015-05-19 at 19:48 +0100, David Howells wrote:
> > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > 
> > > Definitely better.  (FYI, Dmitry's modsig patches from 2012 used the
> > > keyring for safely storing a password. )
> 
> Without the environment variable set, there's a pop up prompt to enter
> the pin.  A pain to have to enter for each and every kernel module, but
> definitely a nice option.

Right. In fact now that sign-file is written in C and not having to call
out to /usr/bin/openssl for each signature, we *could* authenticate to
the PKCS#11 token (or load the private key from the file) just once and
sign all the modules in a *single* invocation.

So you'd only be asked for the password *once*.

The make rules to achieve that are somewhat non-trivial, but it was an
idea we had in our minds when we settled on doing it in C rather than
scripting it.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: sign-file and detached PKCS#7 firmware signatures

[Luis R. Rodriguez]

On Tue, May 19, 2015 at 11:47 AM, David Howells <dhowells@redhat.com> wrote:
> Luis R. Rodriguez <mcgrof@suse.com> wrote:
>
>> I'll also mention:
>>
>> ---
>> The $DIGEST_ALGORITHM needs to be supported on the running kernel and
>> can differ from CONFIG_MODULE_SIG_HASH.
>> ---
>>
>> As I do no think that is quite obvious to a system integrator at first.
>
> Actually, this isn't necessarily so for the firmware.

Sorry by "needs to be supported on the running kernel" I meant "=y" or "=m".

> It *is* for the module signing, but you can always load the module to give you
> the digest algorithm (or public key algorithm) for the firmware.

Sure.

> Though you would still have to be careful with firmware loaded during the
> initramfs phase.

Make sense, how about:

---
The $DIGEST_ALGORITHM needs to be enabled as built-in (=y) or modular
(=m) in the running kernel and can differ from CONFIG_MODULE_SIG_HASH.
If you are enabling the $DIGEST_ALGORITHM as a module take care to
ensure that this module will also be present on the initramfs used as
some modules within the initramfs may need it if using the
firmware_class APIs and firmware signing has been enabled.
---

  Luis

Re: sign-file and detached PKCS#7 firmware signatures

[David Howells]

Luis R. Rodriguez <mcgrof@suse.com> wrote:

> > Though you would still have to be careful with firmware loaded during the
> > initramfs phase.
> 
> Make sense, how about:
> 
> ---
> The $DIGEST_ALGORITHM needs to be enabled as built-in (=y) or modular
> (=m) in the running kernel and can differ from CONFIG_MODULE_SIG_HASH.
> If you are enabling the $DIGEST_ALGORITHM as a module take care to
> ensure that this module will also be present on the initramfs used as
> some modules within the initramfs may need it if using the

Some firmware signatures, not modules.

> firmware_class APIs and firmware signing has been enabled.
> ---

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

I'm not sure I want to get involved here, but...

On 05/15/2015 05:35 AM, David Howells wrote:
>
> Hi Rusty,
>
> 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.

I think this is way more complicated than it has to be.  Can't we look 
up certificates by their subjectPublicKeyInfo?  Every public key has a 
subjectPublicKeyInfo, and even key types that aren't X.509 at all have 
something equivalent to that.

>
>   (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 is PKCS#7 better than whatever we're using now?

>
>   (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.

I would take kernel-specific over PKCS#7 any day.  PKCS#7 is severely 
overcomplicated for what we're doing here.

--Andy

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

[Andy Lutomirski]

On 05/15/2015 05:35 AM, David Howells wrote:
> 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>
> Tested-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>
>   include/crypto/public_key.h |    1
>   scripts/sign-file.c         |  205 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 206 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
>   };
>

I don't understand these comments.  "OpenPGP generated key ID" refers to 
the name of a key.  "X.509 arbitrary subjectKeyIdentifier" also refers 
to a name of a key.  "Signature in PKCS#7 message" refers to a signature 
style.  This seems inconsistent.

Also, I think we're really going to want signatures that specify their 
purpose, e.g. "module named xyz" or "firmware file named abc" or "kexec 
image".  Let's get this right the first time rather than needing yet 
another type in the very near future.

Finally, why are we using PKCS#7 for this?  Can't everything except 
kexec images use raw signatures in some trivial non-ASN.1-ified format? 
  A raw signature can reference a UEFI-sourced key just fine.

It could be as simple as:

4 bytes of signature type
(length of pubkey identifier, pubkey identifier)
4 bytes of purpose
(length of purpose-specific data, purpose-specific data)

The signature would be over the purpose, length of purpose-specific 
data, purpose-specific data, and the hash of the module.

Purpose could be PURPOSE_MODULE, PURPOSE_FIRMWARE, etc. 
PURPOSE_FIRMWARE's purpose-specific data could be the firmware file name.

One of the signature types could be SIGTYPE_RSA_SHA256.  For that 
sigtype, the pubkey identifier would be the subjectPublicKeyInfo and the 
signature would be whatever the simplest standard encoding of an RSA 
signature is (probably just the raw data).

Generating and parsing this would be dead simple, and it solves a 
problem that needs solving (the purpose field).

kexec images probably need PKCS#7 support for Authenticode.  Ick.  That 
could be completely separate, though.

--Andy

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

[David Howells]

Andy Lutomirski <luto@kernel.org> wrote:

> >   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
> >   };
> >
> 
> I don't understand these comments.  "OpenPGP generated key ID" refers to the
> name of a key.  "X.509 arbitrary subjectKeyIdentifier" also refers to a name
> of a key.

OpenPGP was how we did things originally.  We then switched to X.509 because
we had to take account of UEFI.  These values are implicit parts of the kernel
ABI.

> "Signature in PKCS#7 message" refers to a signature style.  This seems
> inconsistent.

Not precisely.  The format of the descriptor is immutable given the particular
magic number.  You set the ID type to that and all the other fields bar one to
zero and you put the signature and all the metadata in the PKCS#7 blob which
you place directly prior to the descriptor (the length of the blob is the one
thing you do need to specify).  Effectively, it's an override.

> Also, I think we're really going to want signatures that specify their
> purpose, e.g. "module named xyz" or "firmware file named abc" or "kexec
> image".  Let's get this right the first time rather than needing yet another
> type in the very near future.

If this is so, then this _must_ also apply to your hash list idea.

> Finally, why are we using PKCS#7 for this?  Can't everything except kexec
> images use raw signatures in some trivial non-ASN.1-ified format? A raw
> signature can reference a UEFI-sourced key just fine.

We have PKCS#7 already in the kernel.  It's a standard.  We can add attributes
of our own devising to extend it if necessary (say your typing idea referenced
above).

> It could be as simple as:
> 
> 4 bytes of signature type
> (length of pubkey identifier, pubkey identifier)
> 4 bytes of purpose
> (length of purpose-specific data, purpose-specific data)

Let's not create yet another unextendable non-standard standard.

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[David Howells]

Andy Lutomirski <luto@kernel.org> wrote:

> I think this is way more complicated than it has to be.  Can't we look up
> certificates by their subjectPublicKeyInfo?

I want to be able to handle an X.509 chain to a root key that we have in the
kernel.  X.509 certs don't chain on subjectPublicKeyInfo unless that happens
to be what's in the SKID (which is a pretty indefinite standard in its own
right:-( ).  So we need to be able to match on the two things I made available
anyway.  PKCS#7 matches on one of them too, so that then just works.

> Why is PKCS#7 better than whatever we're using now?

It has a standard form[*].  It has standard ways to specify things such as the
key to use and the digest to use.  It can carry multiple signatures from
different keys and can carry key chains (something that's more likely to be
useful for kexec or firmware, admittedly).  It can be generated by extant
tools (though adding it onto a module needs a special tool).

 [*] We can agree it's a somewhat, um, ropy standard, but it's still a
     standard.

What we're using now isn't very extensible without changing the magic string
or putting in an override in one of the fields.

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

On Wed, May 20, 2015 at 6:36 AM, David Howells <dhowells@redhat.com> wrote:
> Andy Lutomirski <luto@kernel.org> wrote:
>
>> I think this is way more complicated than it has to be.  Can't we look up
>> certificates by their subjectPublicKeyInfo?
>
> I want to be able to handle an X.509 chain to a root key that we have in the
> kernel.  X.509 certs don't chain on subjectPublicKeyInfo unless that happens
> to be what's in the SKID (which is a pretty indefinite standard in its own
> right:-( ).  So we need to be able to match on the two things I made available
> anyway.  PKCS#7 matches on one of them too, so that then just works.

I see.  PKCS#7 (and PKCS#6 and X.509 which it inherits from) is too
dumb to identify the public key that signed a certificate, so you
can't match on it.  Sigh.

That being said, are you actually planning on implementing X.509 chain
validation correctly?  ISTM you can't really do it usefully, as we
don't even know what time it is when we run this code.  What's the
point of accepting certificate chains if there's no meaningful
validation we can do?

>
>> Why is PKCS#7 better than whatever we're using now?
>
> It has a standard form[*].  It has standard ways to specify things such as the
> key to use and the digest to use.  It can carry multiple signatures from
> different keys and can carry key chains (something that's more likely to be
> useful for kexec or firmware, admittedly).  It can be generated by extant
> tools (though adding it onto a module needs a special tool).

Given how much special stuff is needed (special tool to append it,
probably weird or absent certificate chain validation, etc), this
seems to me to be of questionable value.

Would it make more sense to permit X.509 chains to be loaded into the
keyring instead if we actually need that feature?  IOW, let userspace
(or early initramfs stuff) extend our keyring trust to intermediate
certs that validly chain to already-trusted things?  I think that a
reasonable design goal would be that everything overcomplicated that's
involved should be optional, and moving toward embedding PKCS#7
signatures in the modules themselves does the other direction?

--Andy

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

[Andy Lutomirski]

On Wed, May 20, 2015 at 6:14 AM, David Howells <dhowells@redhat.com> wrote:
> Andy Lutomirski <luto@kernel.org> wrote:
>
>> >   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
>> >   };
>> >
>>
>> I don't understand these comments.  "OpenPGP generated key ID" refers to the
>> name of a key.  "X.509 arbitrary subjectKeyIdentifier" also refers to a name
>> of a key.
>
> OpenPGP was how we did things originally.  We then switched to X.509 because
> we had to take account of UEFI.  These values are implicit parts of the kernel
> ABI.
>
>> "Signature in PKCS#7 message" refers to a signature style.  This seems
>> inconsistent.
>
> Not precisely.  The format of the descriptor is immutable given the particular
> magic number.  You set the ID type to that and all the other fields bar one to
> zero and you put the signature and all the metadata in the PKCS#7 blob which
> you place directly prior to the descriptor (the length of the blob is the one
> thing you do need to specify).  Effectively, it's an override.

Is there a document anywhere in the kernel tree that defines the
actual format?  I suspect that this will confuse most people who read
the code right now.

>
>> Also, I think we're really going to want signatures that specify their
>> purpose, e.g. "module named xyz" or "firmware file named abc" or "kexec
>> image".  Let's get this right the first time rather than needing yet another
>> type in the very near future.
>
> If this is so, then this _must_ also apply to your hash list idea.

Definitely.

>
>> Finally, why are we using PKCS#7 for this?  Can't everything except kexec
>> images use raw signatures in some trivial non-ASN.1-ified format? A raw
>> signature can reference a UEFI-sourced key just fine.
>
> We have PKCS#7 already in the kernel.  It's a standard.  We can add attributes
> of our own devising to extend it if necessary (say your typing idea referenced
> above).
>
>> It could be as simple as:
>>
>> 4 bytes of signature type
>> (length of pubkey identifier, pubkey identifier)
>> 4 bytes of purpose
>> (length of purpose-specific data, purpose-specific data)
>
> Let's not create yet another unextendable non-standard standard.

It doesn't really have to be a standard at all.

Actually, I don't see why we are even trying to make the module
signature format compatible across kernel versions.  The module
payload is completely incompatible across kernel versions already.
Firmware is a different story, of course.

Also, I'll personally take some simple ad-hoc thing over PKCS#7 any
day.  I've tried reading the PKCS stuff.  90% is completely
inapplicable to anything the kernel (or the Internet in general, for
that matter) will ever do, and the other 10% is very poorly designed.

Heck, moving to NaCl format might be a good idea except for the
NIST/FIPS compliance part.

--Andy

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Petko Manolov]

On 15-05-20 08:56:21, Andy Lutomirski wrote:
> 
> Would it make more sense to permit X.509 chains to be loaded into the keyring 
> instead if we actually need that feature?  IOW, let userspace (or early 
> initramfs stuff) extend our keyring trust to intermediate certs that validly 
> chain to already-trusted things?  I think that a reasonable design goal would 
> be that everything overcomplicated that's involved should be optional, and 
> moving toward embedding PKCS#7 signatures in the modules themselves does the 
> other direction?

This is similar to what i am doing right now - create CA hierarchy so we can 
have something like:

                               +-> KeyB
                               |
RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
                    |
                    +-> CertA' ---> KeyA"

The RootCA may be the one whose private key was used to sign the modules and all 
downstream certificates are either directly signed by it or one of the others.  
Not all of the infrastructure is in the mainline kernel, but this can easily be 
rectified.

Now, as Mimi pointed out this scheme is flawed and should be used with care if 
at all.  Revoking certificates is always a PITA.  Being valid for one year only 
adds to the fun.


		Petko

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

On Wed, May 20, 2015 at 9:21 AM, Petko Manolov <petkan@mip-labs.com> wrote:
> On 15-05-20 08:56:21, Andy Lutomirski wrote:
>>
>> Would it make more sense to permit X.509 chains to be loaded into the keyring
>> instead if we actually need that feature?  IOW, let userspace (or early
>> initramfs stuff) extend our keyring trust to intermediate certs that validly
>> chain to already-trusted things?  I think that a reasonable design goal would
>> be that everything overcomplicated that's involved should be optional, and
>> moving toward embedding PKCS#7 signatures in the modules themselves does the
>> other direction?
>
> This is similar to what i am doing right now - create CA hierarchy so we can
> have something like:
>
>                                +-> KeyB
>                                |
> RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
>                     |
>                     +-> CertA' ---> KeyA"
>
> The RootCA may be the one whose private key was used to sign the modules and all
> downstream certificates are either directly signed by it or one of the others.
> Not all of the infrastructure is in the mainline kernel, but this can easily be
> rectified.

Right.  I guess that I can imagine some uses for this, but I don't see
why those intermediate certs would be embedded with the signatures
being verified as opposed to being loaded beforehand.

>
> Now, as Mimi pointed out this scheme is flawed and should be used with care if
> at all.  Revoking certificates is always a PITA.  Being valid for one year only
> adds to the fun.
>

Valid for only one year is worse than that.  We might be verifying the
signature on our clock driver :)  I think that, at best, we could
reject certificates that expired before the running kernel was built.

>
>                 Petko



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Petko Manolov]

On 15-05-20 09:41:21, Andy Lutomirski wrote:
> On Wed, May 20, 2015 at 9:21 AM, Petko Manolov <petkan@mip-labs.com> wrote:
> > On 15-05-20 08:56:21, Andy Lutomirski wrote:
> >>
> >> Would it make more sense to permit X.509 chains to be loaded into the keyring
> >> instead if we actually need that feature?  IOW, let userspace (or early
> >> initramfs stuff) extend our keyring trust to intermediate certs that validly
> >> chain to already-trusted things?  I think that a reasonable design goal would
> >> be that everything overcomplicated that's involved should be optional, and
> >> moving toward embedding PKCS#7 signatures in the modules themselves does the
> >> other direction?
> >
> > This is similar to what i am doing right now - create CA hierarchy so we can
> > have something like:
> >
> >                                +-> KeyB
> >                                |
> > RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
> >                     |
> >                     +-> CertA' ---> KeyA"
> >
> > The RootCA may be the one whose private key was used to sign the modules and all
> > downstream certificates are either directly signed by it or one of the others.
> > Not all of the infrastructure is in the mainline kernel, but this can easily be
> > rectified.
> 
> Right.  I guess that I can imagine some uses for this, but I don't see
> why those intermediate certs would be embedded with the signatures
> being verified as opposed to being loaded beforehand.

They aren't.  They shouldn't.

I think module signing/verification should be simple but cryptographically sound 
process.  Over-designing it is dumb, but i find it useful when there is 
compatibility (at least at key format level) with other systems, namely IMA and 
EVM.

> > Now, as Mimi pointed out this scheme is flawed and should be used with care 
> > if at all.  Revoking certificates is always a PITA.  Being valid for one 
> > year only adds to the fun.
> >
> 
> Valid for only one year is worse than that.  We might be verifying the 
> signature on our clock driver :) I think that, at best, we could reject 
> certificates that expired before the running kernel was built.

Nah, nothing as complex as that.  It is so easy to play with the system clock.  
In the general case, and if needed, the user would either use self signed 
certificate or one signed by whoever the user trusts - Fedora, Debian, LF or 
even NSA.  :)


		Petko

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[David Howells]

Andy Lutomirski <luto@amacapital.net> wrote:

> That being said, are you actually planning on implementing X.509 chain
> validation correctly?  ISTM you can't really do it usefully, as we
> don't even know what time it is when we run this code.

We can't validate certificates based on time.  We've been there, tried that
and patched it out again.  The problem is that we can't trust the system clock
until we've done NTP - and possibly not even then.  A dodgy or unset system
clock can lead to the system not booting, even for installation.

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Luis R. Rodriguez]

On Wed, May 20, 2015 at 07:21:00PM +0300, Petko Manolov wrote:
> On 15-05-20 08:56:21, Andy Lutomirski wrote:
> > 
> > Would it make more sense to permit X.509 chains to be loaded into the keyring 
> > instead if we actually need that feature?  IOW, let userspace (or early 
> > initramfs stuff) extend our keyring trust to intermediate certs that validly 
> > chain to already-trusted things?  I think that a reasonable design goal would 
> > be that everything overcomplicated that's involved should be optional, and 
> > moving toward embedding PKCS#7 signatures in the modules themselves does the 
> > other direction?
> 
> This is similar to what i am doing right now - create CA hierarchy so we can 
> have something like:
> 
>                                +-> KeyB
>                                |
> RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
>                     |
>                     +-> CertA' ---> KeyA"

How exactly do you go about uploading CertB to the kernel BTW? And could fw
signing replace that functionality? Keep in mind "fw uploading" should be
rebranded as "system data upload", which is one of the goals I have when
extending the firware_class module.

> The RootCA may be the one whose private key was used to sign the modules and all 
> downstream certificates are either directly signed by it or one of the others.  
> Not all of the infrastructure is in the mainline kernel, but this can easily be 
> rectified.
> 
> Now, as Mimi pointed out this scheme is flawed and should be used with care if 
> at all.  Revoking certificates is always a PITA.  Being valid for one year only 
> adds to the fun.

Freedom of stupidity comes with a cost :)

  Luis

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

On Thu, May 21, 2015 at 2:38 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Wed, May 20, 2015 at 07:21:00PM +0300, Petko Manolov wrote:
>> On 15-05-20 08:56:21, Andy Lutomirski wrote:
>> >
>> > Would it make more sense to permit X.509 chains to be loaded into the keyring
>> > instead if we actually need that feature?  IOW, let userspace (or early
>> > initramfs stuff) extend our keyring trust to intermediate certs that validly
>> > chain to already-trusted things?  I think that a reasonable design goal would
>> > be that everything overcomplicated that's involved should be optional, and
>> > moving toward embedding PKCS#7 signatures in the modules themselves does the
>> > other direction?
>>
>> This is similar to what i am doing right now - create CA hierarchy so we can
>> have something like:
>>
>>                                +-> KeyB
>>                                |
>> RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
>>                     |
>>                     +-> CertA' ---> KeyA"
>
> How exactly do you go about uploading CertB to the kernel BTW? And could fw
> signing replace that functionality? Keep in mind "fw uploading" should be
> rebranded as "system data upload", which is one of the goals I have when
> extending the firware_class module.

In PKCS#7 land, you don't.  Instead you stick CertB and CertC into the
PKCS#7 signature on the module signed by KeyC.  Then the kernel
verifies them (in theory) using the X.509/PKIX godawful verification
algorithm.  For fun, go search for the large number of errors in the
implementation of this awful algorithm in basically every TLS
implementation out there.

One option would be to add another type of verifiable thing.  We can
verify modules, and we should add firmware to the types of things that
can be signed.  We could add signing keys, too.  IOW, you could ask
the kernel to load a signing key with certain rights, and, if they key
is validly signed by some other key that has the same rights and has a
bit set saying that it can delegate those rights, then the kernel will
add that signing key to the keyring.

If the general infrastructure were there, this would be very little
additional code.

--Andy

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Luis R. Rodriguez]

On Thu, May 21, 2015 at 2:44 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> One option would be to add another type of verifiable thing.  We can
> verify modules, and we should add firmware to the types of things that
> can be signed.  We could add signing keys, too.  IOW, you could ask
> the kernel to load a signing key with certain rights, and, if they key
> is validly signed by some other key that has the same rights and has a
> bit set saying that it can delegate those rights, then the kernel will
> add that signing key to the keyring.
>
> If the general infrastructure were there, this would be very little
> additional code.

I really like this idea, but I've heard of many great ideas before
followed by nothing but vaporware. So is it a direct requirement to
implicate blocking a change for current module signature checking
strategy to a new one given the concerns you raise, or can we enable
those who wish to want additional better solutions as the one you
propose to opt-in to develop those solutions? I like the idea of the
later given that it seems those using the current module signing
infrastructure would prefer the change and enabling what you say does
not seem to be a not possible based on allowing that to be advanced.

 Luis

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

On Thu, May 21, 2015 at 2:59 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, May 21, 2015 at 2:44 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> One option would be to add another type of verifiable thing.  We can
>> verify modules, and we should add firmware to the types of things that
>> can be signed.  We could add signing keys, too.  IOW, you could ask
>> the kernel to load a signing key with certain rights, and, if they key
>> is validly signed by some other key that has the same rights and has a
>> bit set saying that it can delegate those rights, then the kernel will
>> add that signing key to the keyring.
>>
>> If the general infrastructure were there, this would be very little
>> additional code.
>
> I really like this idea, but I've heard of many great ideas before
> followed by nothing but vaporware. So is it a direct requirement to
> implicate blocking a change for current module signature checking
> strategy to a new one given the concerns you raise, or can we enable
> those who wish to want additional better solutions as the one you
> propose to opt-in to develop those solutions? I like the idea of the
> later given that it seems those using the current module signing
> infrastructure would prefer the change and enabling what you say does
> not seem to be a not possible based on allowing that to be advanced.
>

>From my POV (and keep in mind that I'm not really involved in this
stuff and my POV shouldn't be treated as gospel), a firmware signature
verification should have verification that the signature was intended
to apply to a firmware file with the name being requested as a
requirement.  Everything else is nice-to-have.

Given that, I would say that merely shoving firmware files through the
module verifier as-is would not be okay.  There's plenty of
flexibility in how you fix it, though.  Doing it with PKCS#7
authenticated attributes *gag* would work, but my off-the-cuff guess
is that making that work is actually harder, even on top of David's
patches, than doing it from scratch.  PKCS#7 is not easy to work with.

FWIW, openssl rsautl can generate raw PKCS#1 v1.5 signatures (use
-pkcs, not -raw).  openssl pkeyutl can do PKCS#1 v2.0 (i.e. PSS)
signatures, but you'd have to write the verifier yourself.  The kernel
already has a v1.5 verifier that even follows the best practices that
I remember.  (For v2.0, there's a security proof, so an implementation
of the spec is actually secure and there are no "best practices" to
worry about.  v1.5 is known insecure if you implement it naively.)

--Andy

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Luis R. Rodriguez]

On Thu, May 21, 2015 at 3:06 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> Given that, I would say that merely shoving firmware files through the
> module verifier as-is would not be okay.

Replacing one dog and pony show for another is what is going on, what
you describe and suggest seems best, and I welcome patches, it seems
you know what you are talking about :)

 Luis

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

On Thu, May 21, 2015 at 3:16 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, May 21, 2015 at 3:06 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> Given that, I would say that merely shoving firmware files through the
>> module verifier as-is would not be okay.
>
> Replacing one dog and pony show for another is what is going on, what
> you describe and suggest seems best, and I welcome patches, it seems
> you know what you are talking about :)
>

Don't hold your breath.  My plate is over-full.  I'm probably a decent
reviewer of crypto, though.

--Andy

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Luis R. Rodriguez]

On Thu, May 21, 2015 at 3:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, May 21, 2015 at 3:16 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> On Thu, May 21, 2015 at 3:06 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> Given that, I would say that merely shoving firmware files through the
>>> module verifier as-is would not be okay.
>>
>> Replacing one dog and pony show for another is what is going on, what
>> you describe and suggest seems best, and I welcome patches, it seems
>> you know what you are talking about :)
>>
>
> Don't hold your breath.  My plate is over-full.  I'm probably a decent
> reviewer of crypto, though.

Well as good as you are in 10 years we'll have better ones. So when
module signature went into the kernel the real expectation should have
been:

This code looks good now but is going to be complete shit and
breakable a few years from now.

Hence my first implicit and now explicit claims on dog and pony shows.
Best thing we can do IMHO is to just allow us to replace stupid human
code with better human code later, and eventually hopefully better AI
code, and so on. Since you don't have time for a real replacement
maybe what we can do is at least document / target / agree for what
pipe dream we want and shoot for it with time. Hopefully folks will
find time to implement it.

In the meantime should that block current dog and pony show trading? I
don't think so.

  Luis

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

On Thu, May 21, 2015 at 3:31 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, May 21, 2015 at 3:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Thu, May 21, 2015 at 3:16 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>>> On Thu, May 21, 2015 at 3:06 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> Given that, I would say that merely shoving firmware files through the
>>>> module verifier as-is would not be okay.
>>>
>>> Replacing one dog and pony show for another is what is going on, what
>>> you describe and suggest seems best, and I welcome patches, it seems
>>> you know what you are talking about :)
>>>
>>
>> Don't hold your breath.  My plate is over-full.  I'm probably a decent
>> reviewer of crypto, though.
>
> Well as good as you are in 10 years we'll have better ones. So when
> module signature went into the kernel the real expectation should have
> been:
>
> This code looks good now but is going to be complete shit and
> breakable a few years from now.
>
> Hence my first implicit and now explicit claims on dog and pony shows.
> Best thing we can do IMHO is to just allow us to replace stupid human
> code with better human code later, and eventually hopefully better AI
> code, and so on. Since you don't have time for a real replacement
> maybe what we can do is at least document / target / agree for what
> pipe dream we want and shoot for it with time. Hopefully folks will
> find time to implement it.

I disagree.  I'm a firm believer in security proofs.  While I'm not
trained in formal crypto proofs, I can sketch out a proof of why a
system that properly tags its signatures is secure against a
reasonable threat model.  I can also show why that proof wouldn't work
for a scheme without tags, and I can demonstrate the actual weakness
in a scheme without tags.

In ten years, the only reason a scheme that I say looks good would be
because (a) I screwed up, (b) an underlying assumption is wrong, or
(c) the implementation is subtly wrong.  In particular, it won't fail
because I'm insufficiently clever.

A real professional expert would be less likely to screw up.

(For reference, I wrote an actual doctoral thesis involving crypto.)

>
> In the meantime should that block current dog and pony show trading? I
> don't think so.

Yes, since I can demonstrate the actual weakness without tags, and
crypto is notoriously hard to fix once done poorly and there's a great
history of obviously-theoretically-weak systems being meaningfully
attacked in the real world.  See, for example, every single old
SSL/TLS cipher.  (And yes, the crypto community knew what was wrong in
theory and how to fix it when the protocol was designed.  People just
didn't pay attention.)

--Andy

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Luis R. Rodriguez]

On Thu, May 21, 2015 at 03:47:49PM -0700, Andy Lutomirski wrote:
> On Thu, May 21, 2015 at 3:31 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> >
> > Well as good as you are in 10 years we'll have better ones. So when
> > module signature went into the kernel the real expectation should have
> > been:
> >
> > This code looks good now but is going to be complete shit and
> > breakable a few years from now.
> >
> > Hence my first implicit and now explicit claims on dog and pony shows.
> > Best thing we can do IMHO is to just allow us to replace stupid human
> > code with better human code later, and eventually hopefully better AI
> > code, and so on. Since you don't have time for a real replacement
> > maybe what we can do is at least document / target / agree for what
> > pipe dream we want and shoot for it with time. Hopefully folks will
> > find time to implement it.
> 
> I disagree.  I'm a firm believer in security proofs.  While I'm not
> trained in formal crypto proofs, I can sketch out a proof of why a
> system that properly tags its signatures is secure against a
> reasonable threat model.  I can also show why that proof wouldn't work
> for a scheme without tags, and I can demonstrate the actual weakness
> in a scheme without tags.
> 
> In ten years, the only reason a scheme that I say looks good would be
> because (a) I screwed up, (b) an underlying assumption is wrong, or
> (c) the implementation is subtly wrong.  In particular, it won't fail
> because I'm insufficiently clever.
> 
> A real professional expert would be less likely to screw up.
> 
> (For reference, I wrote an actual doctoral thesis involving crypto.)

OK, I think what I mentioned still holds:

these premises must hold true for a period of time, and provided
you have all information. You cannot have all the information, so
the "threat model" depends on the reviewer, and the information they
have access to. So, still its still a dog and pony show, at least
a temporal one or one with a set of clauses.

> > In the meantime should that block current dog and pony show trading? I
> > don't think so.
> 
> Yes, since I can demonstrate the actual weakness without tags,

But you don't want to do the work to provide a better replacement?

> and
> crypto is notoriously hard to fix once done poorly and there's a great
> history of obviously-theoretically-weak systems being meaningfully
> attacked in the real world.  See, for example, every single old
> SSL/TLS cipher.  (And yes, the crypto community knew what was wrong in
> theory and how to fix it when the protocol was designed.  People just
> didn't pay attention.)

Its a fair argument, but still -- we have the vaporware problem.

  Luis

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Andy Lutomirski]

On Thu, May 21, 2015 at 4:01 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, May 21, 2015 at 03:47:49PM -0700, Andy Lutomirski wrote:
>> On Thu, May 21, 2015 at 3:31 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> >
>> > Well as good as you are in 10 years we'll have better ones. So when
>> > module signature went into the kernel the real expectation should have
>> > been:
>> >
>> > This code looks good now but is going to be complete shit and
>> > breakable a few years from now.
>> >
>> > Hence my first implicit and now explicit claims on dog and pony shows.
>> > Best thing we can do IMHO is to just allow us to replace stupid human
>> > code with better human code later, and eventually hopefully better AI
>> > code, and so on. Since you don't have time for a real replacement
>> > maybe what we can do is at least document / target / agree for what
>> > pipe dream we want and shoot for it with time. Hopefully folks will
>> > find time to implement it.
>>
>> I disagree.  I'm a firm believer in security proofs.  While I'm not
>> trained in formal crypto proofs, I can sketch out a proof of why a
>> system that properly tags its signatures is secure against a
>> reasonable threat model.  I can also show why that proof wouldn't work
>> for a scheme without tags, and I can demonstrate the actual weakness
>> in a scheme without tags.
>>
>> In ten years, the only reason a scheme that I say looks good would be
>> because (a) I screwed up, (b) an underlying assumption is wrong, or
>> (c) the implementation is subtly wrong.  In particular, it won't fail
>> because I'm insufficiently clever.
>>
>> A real professional expert would be less likely to screw up.
>>
>> (For reference, I wrote an actual doctoral thesis involving crypto.)
>
> OK, I think what I mentioned still holds:
>
> these premises must hold true for a period of time, and provided
> you have all information. You cannot have all the information, so
> the "threat model" depends on the reviewer, and the information they
> have access to. So, still its still a dog and pony show, at least
> a temporal one or one with a set of clauses.
>
>> > In the meantime should that block current dog and pony show trading? I
>> > don't think so.
>>
>> Yes, since I can demonstrate the actual weakness without tags,
>
> But you don't want to do the work to provide a better replacement?

Hell no.  At least not until someone convinces me that I want any of
this on any system I build.  Thus far I'm unconvinced.

Meanwhile, the threat model seems to be that you don't want an
attacker not in possession of a distro-trusted or UEFI-trusted private
key to cause the kernel to load incorrect firmware into a device or
incorrect regulatory data.

Without tagging the purpose of the signed file, you simply don't have
a cryptographic guarantee of that.  The bad guy can load something
else that was signed for an entirely different purpose into the wrong
device, possibly crashing it, causing buffer overflows because the
format is wrong, or doing any number of other bad things.

>
>> and
>> crypto is notoriously hard to fix once done poorly and there's a great
>> history of obviously-theoretically-weak systems being meaningfully
>> attacked in the real world.  See, for example, every single old
>> SSL/TLS cipher.  (And yes, the crypto community knew what was wrong in
>> theory and how to fix it when the protocol was designed.  People just
>> didn't pay attention.)
>
> Its a fair argument, but still -- we have the vaporware problem.

If you write crypto verification code for firmware and I review it,
and your code doesn't cryptographically protect it adequately, I'll
say that in my review.  That's all.

--Andy

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[David Howells]

Luis R. Rodriguez <mcgrof@suse.com> wrote:

> > This is similar to what i am doing right now - create CA hierarchy so we can 
> > have something like:
> > 
> >                                +-> KeyB
> >                                |
> > RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
> >                     |
> >                     +-> CertA' ---> KeyA"
> 
> How exactly do you go about uploading CertB to the kernel BTW?

Assuming RootCA or CertA is present in the kernel, the idea would be to use
the add_key() system call or the request_key() mechanism to add the key to the
system keyring.  The key in the cert would only be added to the keyring if it
is trusted by a key already there.

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[David Howells]

Andy Lutomirski <luto@amacapital.net> wrote:

> In PKCS#7 land, you don't.  Instead you stick CertB and CertC into the
> PKCS#7 signature on the module signed by KeyC.

Yes, that works too.

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[David Howells]

Andy Lutomirski <luto@amacapital.net> wrote:

> Without tagging the purpose of the signed file, you simply don't have
> a cryptographic guarantee of that.  The bad guy can load something
> else that was signed for an entirely different purpose into the wrong
> device, possibly crashing it, causing buffer overflows because the
> format is wrong, or doing any number of other bad things.

One suggestion David Woodhouse made with regard to tagging is that the tag
could just be added into the digest before it is signed/verified and not
actually stored in the signature.

This means that if you try loading the firmware for the wrong request, the
signature verification will fail.

It's an interesting approach that's simple to achieve, but it has the downside
that the signature will be invalid in the mismatch situation and you can't
tell whether it's because the module is being misused or the signature is just
wrong.  However, that might be livable with.

David

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Mimi Zohar]

On Fri, 2015-05-22 at 08:48 +0100, David Howells wrote:
> Luis R. Rodriguez <mcgrof@suse.com> wrote:
> 
> > > This is similar to what i am doing right now - create CA hierarchy so we can 
> > > have something like:
> > > 
> > >                                +-> KeyB
> > >                                |
> > > RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
> > >                     |
> > >                     +-> CertA' ---> KeyA"
> > 
> > How exactly do you go about uploading CertB to the kernel BTW?
> 
> Assuming RootCA or CertA is present in the kernel, the idea would be to use
> the add_key() system call or the request_key() mechanism to add the key to the
> system keyring.  The key in the cert would only be added to the keyring if it
> is trusted by a key already there.

>From Petko's description, the RootCA is on the system keyring, but CertA
is on a new IMA trusted CA keyring.   So everything you said is true,
but on this new, yet to be upstreamed, IMA trusted CA keyring.

Mimi

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Mimi Zohar]

On Fri, 2015-05-22 at 08:56 +0100, David Howells wrote:
> Andy Lutomirski <luto@amacapital.net> wrote:
> 
> > Without tagging the purpose of the signed file, you simply don't have
> > a cryptographic guarantee of that.  The bad guy can load something
> > else that was signed for an entirely different purpose into the wrong
> > device, possibly crashing it, causing buffer overflows because the
> > format is wrong, or doing any number of other bad things.
> 
> One suggestion David Woodhouse made with regard to tagging is that the tag
> could just be added into the digest before it is signed/verified and not
> actually stored in the signature.
> 
> This means that if you try loading the firmware for the wrong request, the
> signature verification will fail.
> 
> It's an interesting approach that's simple to achieve, but it has the downside
> that the signature will be invalid in the mismatch situation and you can't
> tell whether it's because the module is being misused or the signature is just
> wrong.  However, that might be livable with.

With transitive trust, any key on the system keyring would be able to
add keys with any tag, whether the tag is in the cert or the digest.  If
I trust cert A to sign keys that add kernel modules or other certs that
add kernel modules, it doesn't mean that I trust that cert to also sign
keys that add firmware, for example, or other keys that add firmware.

Mimi

Re: [PATCH 0/8] MODSIGN: Use PKCS#7 for module signatures [ver #4]

[Petko Manolov]

On 15-05-22 08:28:17, Mimi Zohar wrote:
> On Fri, 2015-05-22 at 08:48 +0100, David Howells wrote:
> > Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > 
> > > > This is similar to what i am doing right now - create CA hierarchy so we can 
> > > > have something like:
> > > > 
> > > >                                +-> KeyB
> > > >                                |
> > > > RootCA --->  CertA ---> CertB ---> CertC ---> KeyC
> > > >                     |
> > > >                     +-> CertA' ---> KeyA"
> > > 
> > > How exactly do you go about uploading CertB to the kernel BTW?
> > 
> > Assuming RootCA or CertA is present in the kernel, the idea would be to use 
> > the add_key() system call or the request_key() mechanism to add the key to 
> > the system keyring.  The key in the cert would only be added to the keyring 
> > if it is trusted by a key already there.
> 
> From Petko's description, the RootCA is on the system keyring, but CertA is on 
> a new IMA trusted CA keyring.  So everything you said is true, but on this 
> new, yet to be upstreamed, IMA trusted CA keyring.

I only named this intermediate keyring .ima_root_ca because this is how i use 
it.  However, it is system-wide trusted keyring, which accepts keys that are 
either trusted by CAs in the .system_keyring or higher level CA is already in 
.ima_root_ca.  For example CertC will be accepted in .ima_root_ca if CertB is 
already there.

The name (.ima_root_ca) is misleading and should be replaced with something that 
better describes it's functionality.  As far as i see there is no reason this 
keyring not to hold the CA that verifies module's signature.


		Petko