All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5]
@ 2015-05-28 15:46 David Howells
  2015-05-28 15:46 ` [PATCH 01/20] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
                   ` (21 more replies)
  0 siblings, 22 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:46 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2


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 supplies 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) Extracts 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.

 (6) Provides support for providing a password/pin for an encrypted private
     key to sign-file.

 (7) Makes it possible to use PKCS#11 with sign-file, thus allowing the use of
     cryptographic hardware.

 (8) Overhauls the way the module signing key is handled.  If the name in
     CONFIG_MODULE_SIG_KEY is "signing_key.priv" then a key will be
     automatically generated and placed in the build directory.  If the name
     is different, autogeneration is suppressed and the file is presumed to be
     a PEM file containing both the private key and X.509 certificate.

 (9) Overhauls the way auxiliary trusted keys are added to the kernel.  Files
     matching the pattern "*.x509" are no longer just gathered up and cat'd
     together.  Now CONFIG_SYSTEM_TRUSTED_KEYS must be set to point to a
     single PEM file containing a set of X.509 certs cat'd together if this
     facility is desired.

Note that the revised sign-file program no longer supports the "-s <signature>"
option to add an externally generated signature.  This is deprecated in favour
of using PKCS#11.  Note also that the format of the signature file that would
be passed to -s has changed.

David Woodhouse also has stated an intention to overhaul the makefile magic he
added to deal with quotes and quoting encountered when using CONFIG_* option
strings in the makefile.

The patches 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-20150526


Additionally, the last four patches are provisionally added to support firmware
signing, but will need further modification (ie. registration of OIDs) before
they can be committed, but are included for comment:

(10) Add a PKCS#7 authenticated attribute to hold the name of the firmware blob
     passed to request_key() so that this can be checked prior to permitting
     the load.

(11) Add usage restriction markers to the extendedKeyUsage field of an X.509
     certificate to indicate what may be checked with it.

(12) Parse the keyUsage extension of an X.509 certificate to detect CA keys
     that are used for key signing.

(13) Implement a key usage restrictions.  For instance, a keys specifically
     restricted to module signature checking may not be used to verify firmware
     blobs and a key specifically restricted to kexec image signature checking
     may not be used for checking the signature on an X.509 certificate.

     I have allowed that keys that have no restrictions noted can be used for
     anything other than firmware, but possibly this should be restricted
     further.

They can be found here also:

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

David
---
David Howells (11):
      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
      PKCS#7: Add an optional authenticated attribute to hold firmware name
      X.509: Restrict the usage of a key based on information in X.509 certificate
      X.509: Parse the keyUsage extension to detect key-signing keys
      KEYS: Restrict signature verification to keys appropriate to the purpose

David Woodhouse (7):
      modsign: Abort modules_install when signing fails
      modsign: Allow password to be specified for signing key
      modsign: Allow signing key to be PKCS#11
      modsign: Allow external signing key to be specified
      modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed
      modsign: Use single PEM file for autogenerated key
      modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option

Luis R. Rodriguez (2):
      sign-file: Add option to only create signature file
      sign-file: use .p7s instead of .pkcs7 file extension


 .gitignore                                |    1 
 Documentation/kbuild/kbuild.txt           |    5 
 Documentation/module-signing.txt          |   51 +++-
 Makefile                                  |    8 -
 crypto/asymmetric_keys/Makefile           |   12 +
 crypto/asymmetric_keys/asymmetric_type.c  |   12 +
 crypto/asymmetric_keys/pkcs7_key_type.c   |   19 +
 crypto/asymmetric_keys/pkcs7_parser.c     |   77 +++++
 crypto/asymmetric_keys/pkcs7_parser.h     |    1 
 crypto/asymmetric_keys/pkcs7_trust.c      |   24 +-
 crypto/asymmetric_keys/pkcs7_verify.c     |  101 +++++--
 crypto/asymmetric_keys/public_key.c       |   78 +++++
 crypto/asymmetric_keys/public_key.h       |    3 
 crypto/asymmetric_keys/signature.c        |    6 
 crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
 crypto/asymmetric_keys/x509_cert_parser.c |  255 ++++++++++++++----
 crypto/asymmetric_keys/x509_extusage.asn1 |    3 
 crypto/asymmetric_keys/x509_parser.h      |    8 -
 crypto/asymmetric_keys/x509_public_key.c  |  101 +++++--
 include/crypto/pkcs7.h                    |    8 -
 include/crypto/public_key.h               |   19 +
 include/keys/asymmetric-subtype.h         |    6 
 include/keys/asymmetric-type.h            |   13 +
 include/keys/system_keyring.h             |    8 +
 include/linux/oid_registry.h              |    6 
 init/Kconfig                              |   55 +++-
 kernel/Makefile                           |  110 +++++---
 kernel/module_signing.c                   |  213 +--------------
 kernel/system_keyring.c                   |   78 +++++
 lib/oid_registry.c                        |    1 
 scripts/Makefile                          |    3 
 scripts/Makefile.modinst                  |    2 
 scripts/extract-cert.c                    |  132 +++++++++
 scripts/sign-file                         |  421 -----------------------------
 scripts/sign-file.c                       |  284 ++++++++++++++++++++
 35 files changed, 1356 insertions(+), 803 deletions(-)
 create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
 create mode 100644 crypto/asymmetric_keys/x509_extusage.asn1
 create mode 100644 scripts/extract-cert.c
 delete mode 100755 scripts/sign-file
 create mode 100755 scripts/sign-file.c


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

* [PATCH 01/20] X.509: Extract both parts of the AuthorityKeyIdentifier [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
@ 2015-05-28 15:46 ` David Howells
  2015-05-28 15:46 ` [PATCH 02/20] X.509: Support X.509 lookup by Issuer+Serial form " David Howells
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:46 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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;


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

* [PATCH 02/20] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
  2015-05-28 15:46 ` [PATCH 01/20] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
@ 2015-05-28 15:46 ` David Howells
  2015-05-28 15:46 ` [PATCH 03/20] PKCS#7: Allow detached data to be supplied for signature checking purposes " David Howells
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:46 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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 */


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

* [PATCH 03/20] PKCS#7: Allow detached data to be supplied for signature checking purposes [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
  2015-05-28 15:46 ` [PATCH 01/20] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
  2015-05-28 15:46 ` [PATCH 02/20] X.509: Support X.509 lookup by Issuer+Serial form " David Howells
@ 2015-05-28 15:46 ` David Howells
  2015-05-28 15:46 ` [PATCH 04/20] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:46 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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);


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

* [PATCH 04/20] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (2 preceding siblings ...)
  2015-05-28 15:46 ` [PATCH 03/20] PKCS#7: Allow detached data to be supplied for signature checking purposes " David Howells
@ 2015-05-28 15:46 ` David Howells
  2015-05-28 15:46 ` [PATCH 05/20] MODSIGN: Use PKCS#7 messages as module signatures " David Howells
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:46 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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;
+}


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

* [PATCH 05/20] MODSIGN: Use PKCS#7 messages as module signatures [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (3 preceding siblings ...)
  2015-05-28 15:46 ` [PATCH 04/20] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
@ 2015-05-28 15:46 ` David Howells
  2015-05-28 15:47 ` [PATCH 06/20] sign-file: Add option to only create signature file " David Howells
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:46 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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;
-}


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

* [PATCH 06/20] sign-file: Add option to only create signature file [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (4 preceding siblings ...)
  2015-05-28 15:46 ` [PATCH 05/20] MODSIGN: Use PKCS#7 messages as module signatures " David Howells
@ 2015-05-28 15:47 ` David Howells
  2015-05-28 15:47 ` [PATCH 07/20] system_keyring.c doesn't need to #include module-internal.h " David Howells
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:47 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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))),


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

* [PATCH 07/20] system_keyring.c doesn't need to #include module-internal.h [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (5 preceding siblings ...)
  2015-05-28 15:47 ` [PATCH 06/20] sign-file: Add option to only create signature file " David Howells
@ 2015-05-28 15:47 ` David Howells
  2015-05-28 15:47 ` [PATCH 08/20] MODSIGN: Extract the blob PKCS#7 signature verifier from module signing " David Howells
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:47 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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);


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

* [PATCH 08/20] MODSIGN: Extract the blob PKCS#7 signature verifier from module signing [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (6 preceding siblings ...)
  2015-05-28 15:47 ` [PATCH 07/20] system_keyring.c doesn't need to #include module-internal.h " David Howells
@ 2015-05-28 15:47 ` David Howells
  2015-05-28 15:47 ` [PATCH 09/20] modsign: Abort modules_install when signing fails " David Howells
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:47 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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 */


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

* [PATCH 09/20] modsign: Abort modules_install when signing fails [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (7 preceding siblings ...)
  2015-05-28 15:47 ` [PATCH 08/20] MODSIGN: Extract the blob PKCS#7 signature verifier from module signing " David Howells
@ 2015-05-28 15:47 ` David Howells
  2015-05-28 15:47 ` [PATCH 10/20] modsign: Allow password to be specified for signing key " David Howells
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:47 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

From: David Woodhouse <David.Woodhouse@intel.com>

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

 scripts/Makefile.modinst |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index e48a4e9d8868..07650eeaaf06 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


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

* [PATCH 10/20] modsign: Allow password to be specified for signing key [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (8 preceding siblings ...)
  2015-05-28 15:47 ` [PATCH 09/20] modsign: Abort modules_install when signing fails " David Howells
@ 2015-05-28 15:47 ` David Howells
  2015-05-28 15:47 ` [PATCH 11/20] modsign: Allow signing key to be PKCS#11 " David Howells
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:47 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

From: David Woodhouse <David.Woodhouse@intel.com>

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>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.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 6466704d47b5..0ff6a466a05b 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 c72702ec1ded..faaa6ea002f7 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 39aaabe89388..720b9bc933ae 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");


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

* [PATCH 11/20] modsign: Allow signing key to be PKCS#11 [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (9 preceding siblings ...)
  2015-05-28 15:47 ` [PATCH 10/20] modsign: Allow password to be specified for signing key " David Howells
@ 2015-05-28 15:47 ` David Howells
  2015-05-28 15:47 ` [PATCH 12/20] modsign: Allow external signing key to be specified " David Howells
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:47 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

From: David Woodhouse <David.Woodhouse@intel.com>

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>
Signed-off-by: David Howells <dhowells@redhat.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 720b9bc933ae..ad0aa21bd3ac 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);


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

* [PATCH 12/20] modsign: Allow external signing key to be specified [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (10 preceding siblings ...)
  2015-05-28 15:47 ` [PATCH 11/20] modsign: Allow signing key to be PKCS#11 " David Howells
@ 2015-05-28 15:47 ` David Howells
  2015-05-28 15:48 ` [PATCH 13/20] modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed " David Howells
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:47 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

From: David Woodhouse <David.Woodhouse@intel.com>

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: David Howells <dhowells@redhat.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 faaa6ea002f7..84597c7ea175 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 ad27fc9fa02b..9590e67c2094 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 c05afd6594f2..385181546f6a 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 60c302cfb4d3..050a55eca566 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


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

* [PATCH 13/20] modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (11 preceding siblings ...)
  2015-05-28 15:47 ` [PATCH 12/20] modsign: Allow external signing key to be specified " David Howells
@ 2015-05-28 15:48 ` David Howells
  2015-05-28 15:48 ` [PATCH 14/20] modsign: Use single PEM file for autogenerated key " David Howells
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:48 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

From: David Woodhouse <David.Woodhouse@intel.com>

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>
Signed-off-by: David Howells <dhowells@redhat.com>
---

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

diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index 84597c7ea175..693001920890 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 385181546f6a..a4b4f390d8f2 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 050a55eca566..9fb259d0383c 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 b12fe020664d..236f683510bd 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 000000000000..4fd5b2f07b45
--- /dev/null
+++ b/scripts/extract-cert.c
@@ -0,0 +1,132 @@
+/* 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 const char *key_pass;
+
+int main(int argc, char **argv)
+{
+	char *cert_src, *cert_dst;
+	X509 *x509;
+	BIO *b;
+
+	OpenSSL_add_all_algorithms();
+	ERR_load_crypto_strings();
+	ERR_clear_error();
+
+        key_pass = getenv("KBUILD_SIGN_PIN");
+
+	if (argc != 3)
+		format();
+
+	cert_src = argv[1];
+	cert_dst = argv[2];
+
+	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;
+}


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

* [PATCH 14/20] modsign: Use single PEM file for autogenerated key [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (12 preceding siblings ...)
  2015-05-28 15:48 ` [PATCH 13/20] modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed " David Howells
@ 2015-05-28 15:48 ` David Howells
  2015-05-28 15:48 ` [PATCH 15/20] modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option " David Howells
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:48 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

From: David Woodhouse <David.Woodhouse@intel.com>

The current rule for generating signing_key.priv and signing_key.x509 is
a classic example of a bad rule which has a tendency to break parallel
make. When invoked to create *either* target, it generates the other
target as a side-effect that make didn't predict.

So let's switch to using a single file signing_key.pem which contains
both key and certificate. That matches what we do in the case of an
external key specified by CONFIG_MODULE_SIG_KEY anyway, so it's also
slightly cleaner.

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

 .gitignore                       |    1 +
 Documentation/module-signing.txt |    9 ++++-----
 Makefile                         |    4 ++--
 init/Kconfig                     |    4 ++--
 kernel/Makefile                  |   15 +++++++--------
 5 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/.gitignore b/.gitignore
index 4ad4a98b884b..17fa24dd7e46 100644
--- a/.gitignore
+++ b/.gitignore
@@ -97,6 +97,7 @@ GTAGS
 # Leavings from module signing
 #
 extra_certificates
+signing_key.pem
 signing_key.priv
 signing_key.x509
 x509.genkey
diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index 693001920890..5d5e4e32dc26 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -91,7 +91,7 @@ This has a number of options available:
  (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
+     "signing_key.pem" 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 both a private
      key and its corresponding X.509 certificate in PEM form, or — on
@@ -116,11 +116,10 @@ kernel so that it can be used to check the signatures as the modules are
 loaded.
 
 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:
+default, the kernel build will automatically generate a new keypair using
+openssl if one does not exist in the file:
 
-	signing_key.priv
-	signing_key.x509
+	signing_key.pem
 
 during the building of vmlinux (the public part of the key needs to be built
 into vmlinux) using parameters in the:
diff --git a/Makefile b/Makefile
index 9590e67c2094..ce2f4682a37a 100644
--- a/Makefile
+++ b/Makefile
@@ -1175,8 +1175,8 @@ MRPROPER_DIRS  += include/config usr/include include/generated          \
 		  arch/*/include/generated .tmp_objdiff
 MRPROPER_FILES += .config .config.old .version .old_version \
 		  Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \
-		  signing_key.priv signing_key.x509 x509.genkey		\
-		  extra_certificates signing_key.x509.keyid		\
+		  signing_key.pem signing_key.priv signing_key.x509	\
+		  x509.genkey extra_certificates signing_key.x509.keyid	\
 		  signing_key.x509.signer vmlinux-gdb.py
 
 # clean - Delete most, but leave enough to build external modules
diff --git a/init/Kconfig b/init/Kconfig
index a4b4f390d8f2..a52935338419 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1956,7 +1956,7 @@ config MODULE_SIG_HASH
 
 config MODULE_SIG_KEY
 	string "File name or PKCS#11 URI of module signing key"
-	default "signing_key.priv"
+	default "signing_key.pem"
 	depends on MODULE_SIG
 	help
          Provide the file name of a private key/certificate in PEM format,
@@ -1964,7 +1964,7 @@ config MODULE_SIG_KEY
          the URI should identify, both the certificate and its corresponding
          private key.
 
-         If this option is unchanged from its default "signing_key.priv",
+         If this option is unchanged from its default "signing_key.pem",
          then the kernel will automatically generate the private key and
          certificate as described in Documentation/module-signing.txt
 
diff --git a/kernel/Makefile b/kernel/Makefile
index 9fb259d0383c..541e1e89e90c 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -173,8 +173,8 @@ 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
+ifeq ($(CONFIG_MODULE_SIG_KEY),"signing_key.pem")
+signing_key.pem: x509.genkey
 	@echo "###"
 	@echo "### Now generating an X.509 key pair to be used for signing modules."
 	@echo "###"
@@ -185,8 +185,8 @@ signing_key.priv signing_key.x509: x509.genkey
 	@echo "###"
 	openssl req -new -nodes -utf8 -$(CONFIG_MODULE_SIG_HASH) -days 36500 \
 		-batch -x509 -config x509.genkey \
-		-outform DER -out signing_key.x509 \
-		-keyout signing_key.priv 2>&1
+		-outform PEM -out signing_key.pem \
+		-keyout signing_key.pem 2>&1
 	@echo "###"
 	@echo "### Key pair generated."
 	@echo "###"
@@ -210,9 +210,9 @@ 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.
+endif
+
+# We need to obtain the certificate from CONFIG_MODULE_SIG_KEY.
 quiet_cmd_extract_der = CERT_DER $(2)
       cmd_extract_der = scripts/extract-cert "$(2)" signing_key.x509
 
@@ -249,4 +249,3 @@ endif
 signing_key.x509: scripts/extract-cert include/config/module/sig/key.h $(X509_DEP)
 	$(call cmd,extract_der,$(X509_SOURCE))
 endif
-endif


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

* [PATCH 15/20] modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (13 preceding siblings ...)
  2015-05-28 15:48 ` [PATCH 14/20] modsign: Use single PEM file for autogenerated key " David Howells
@ 2015-05-28 15:48 ` David Howells
  2015-05-28 15:48 ` [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name " David Howells
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:48 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

From: David Woodhouse <David.Woodhouse@intel.com>

Let the user explicitly provide a file containing trusted keys, instead of
just automatically finding files matching *.x509 in the build tree and
trusting whatever we find. This really ought to be an *explicit*
configuration, and the build rules for dealing with the files were
fairly painful too.

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

 Documentation/module-signing.txt |   15 +++--
 init/Kconfig                     |   13 ++++
 kernel/Makefile                  |  125 ++++++++++++++++++++------------------
 3 files changed, 89 insertions(+), 64 deletions(-)

diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index 5d5e4e32dc26..4e62bc29666e 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -88,6 +88,7 @@ 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
@@ -104,6 +105,13 @@ This has a number of options available:
      means of the KBUILD_SIGN_PIN variable.
 
 
+ (5) "Additional X.509 keys for default system keyring" (CONFIG_SYSTEM_TRUSTED_KEYS)
+
+     This option can be set to the filename of a PEM-encoded file containing
+     additional certificates which will be included in the system keyring by
+     default.
+
+
 =======================
 GENERATING SIGNING KEYS
 =======================
@@ -171,10 +179,9 @@ in a keyring called ".system_keyring" that can be seen by:
 	302d2d52 I------     1 perm 1f010000     0     0 asymmetri Fedora kernel signing key: d69a84e6bce3d216b979e9505b3e3ef9a7118079: X509.RSA a7118079 []
 	...
 
-Beyond the public key generated specifically for module signing, any file
-placed in the kernel source root directory or the kernel build root directory
-whose name is suffixed with ".x509" will be assumed to be an X.509 public key
-and will be added to the keyring.
+Beyond the public key generated specifically for module signing, additional
+trusted certificates can be provided in a PEM-encoded file referenced by the
+CONFIG_SYSTEM_TRUSTED_KEYS configuration option.
 
 Further, the architecture code may take public keys from a hardware store and
 add those in also (e.g. from the UEFI key database).
diff --git a/init/Kconfig b/init/Kconfig
index a52935338419..b46c19525074 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1758,6 +1758,19 @@ config SYSTEM_TRUSTED_KEYRING
 
 	  Keys in this keyring are used by module signature checking.
 
+config SYSTEM_TRUSTED_KEYS
+	string "Additional X.509 keys for default system keyring"
+	depends on SYSTEM_TRUSTED_KEYRING
+	help
+	  If set, this option should be the filename of a PEM-formatted file
+	  containing trusted X.509 certificates to be included in the default
+	  system keyring. Any certificate used for module signing is implicitly
+	  also trusted.
+
+	  NOTE: If you previously provided keys for the system keyring in the
+	  form of DER-encoded *.x509 files in the top-level build directory,
+	  those are no longer used. You will need to set this option instead.
+
 config SYSTEM_DATA_VERIFICATION
 	def_bool n
 	select SYSTEM_TRUSTED_KEYRING
diff --git a/kernel/Makefile b/kernel/Makefile
index 541e1e89e90c..2d03e870ba8d 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -114,46 +114,75 @@ $(obj)/config_data.h: $(obj)/config_data.gz FORCE
 
 ###############################################################################
 #
-# Roll all the X.509 certificates that we can find together and pull them into
-# the kernel so that they get loaded into the system trusted keyring during
-# boot.
+# When a Kconfig string contains a filename, it is suitable for
+# passing to shell commands. It is surrounded by double-quotes, and
+# any double-quotes or backslashes within it are escaped by
+# backslashes.
 #
-# We look in the source root and the build root for all files whose name ends
-# in ".x509".  Unfortunately, this will generate duplicate filenames, so we
-# have make canonicalise the pathnames and then sort them to discard the
-# duplicates.
+# This is no use for dependencies or $(wildcard). We need to strip the
+# surrounding quotes and the escaping from quotes and backslashes, and
+# we *do* need to escape any spaces in the string. So, for example:
+#
+# Usage: $(eval $(call config_filename,FOO))
+#
+# Defines FOO_FILENAME based on the contents of the CONFIG_FOO option,
+# transformed as described above to be suitable for use within the
+# makefile.
+#
+# Also, if the filename is a relative filename and exists in the source
+# tree but not the build tree, define FOO_SRCPREFIX as $(srctree)/ to
+# be prefixed to *both* command invocation and dependencies.
+#
+# Note: We also print the filenames in the quiet_cmd_foo text, and
+# perhaps ought to have a version specially escaped for that purpose.
+# But it's only cosmetic, and $(patsubst "%",%,$(CONFIG_FOO)) is good
+# enough.  It'll strip the quotes in the common case where there's no
+# space and it's a simple filename, and it'll retain the quotes when
+# there's a space. There are some esoteric cases in which it'll print
+# the wrong thing, but we don't really care. The actual dependencies
+# and commands *do* get it right, with various combinations of single
+# and double quotes, backslashes and spaces in the filenames.
 #
 ###############################################################################
-ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
-X509_CERTIFICATES-y := $(wildcard *.x509) $(wildcard $(srctree)/*.x509)
-X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(objtree)/signing_key.x509
-X509_CERTIFICATES-raw := $(sort $(foreach CERT,$(X509_CERTIFICATES-y), \
-				$(or $(realpath $(CERT)),$(CERT))))
-X509_CERTIFICATES := $(subst $(realpath $(objtree))/,,$(X509_CERTIFICATES-raw))
-
-ifeq ($(X509_CERTIFICATES),)
-$(warning *** No X.509 certificates found ***)
+#
+quote := $(firstword " ")
+space :=
+space +=
+space_escape := %%%SPACE%%%
+#
+define config_filename =
+ifneq ($$(CONFIG_$(1)),"")
+$(1)_FILENAME := $$(subst \\,\,$$(subst \$$(quote),$$(quote),$$(subst $$(space_escape),\$$(space),$$(patsubst "%",%,$$(subst $$(space),$$(space_escape),$$(CONFIG_$(1)))))))
+ifneq ($$(patsubst /%,%,$$(firstword $$($(1)_FILENAME))),$$(firstword $$($(1)_FILENAME)))
+else
+ifeq ($$(wildcard $$($(1)_FILENAME)),)
+ifneq ($$(wildcard $$(srctree)/$$($(1)_FILENAME)),)
+$(1)_SRCPREFIX := $(srctree)/
+endif
 endif
-
-ifneq ($(wildcard $(obj)/.x509.list),)
-ifneq ($(shell cat $(obj)/.x509.list),$(X509_CERTIFICATES))
-$(info X.509 certificate list changed)
-$(shell rm $(obj)/.x509.list)
 endif
 endif
+endef
+#
+###############################################################################
+
+
+ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
+
+$(eval $(call config_filename,SYSTEM_TRUSTED_KEYS))
+
+SIGNING_X509-$(CONFIG_MODULE_SIG) += signing_key.x509
 
 kernel/system_certificates.o: $(obj)/x509_certificate_list
 
-quiet_cmd_x509certs  = CERTS   $@
-      cmd_x509certs  = cat $(X509_CERTIFICATES) /dev/null >$@ $(foreach X509,$(X509_CERTIFICATES),; $(kecho) "  - Including cert $(X509)")
+quiet_cmd_x509certs  = CERTS   $(SIGNING_X509-y) $(patsubst "%",%,$(2))
+      cmd_x509certs  = ( cat $(SIGNING_X509-y) /dev/null; \
+			 awk '/-----BEGIN CERTIFICATE-----/{flag=1;next}/-----END CERTIFICATE-----/{flag=0}flag' $(2) /dev/null | base64 -d ) > $@ || ( rm $@; exit 1)
 
 targets += $(obj)/x509_certificate_list
-$(obj)/x509_certificate_list: $(X509_CERTIFICATES) $(obj)/.x509.list
-	$(call if_changed,x509certs)
+$(obj)/x509_certificate_list: $(SIGNING_X509-y) include/config/system/trusted/keys.h include/config/module/sig.h $(SYSTEM_TRUSTED_KEYS_SRCPREFIX)$(SYSTEM_TRUSTED_KEYS_FILENAME)
+	$(call if_changed,x509certs,$(SYSTEM_TRUSTED_KEYS_SRCPREFIX)$(CONFIG_SYSTEM_TRUSTED_KEYS))
 
-targets += $(obj)/.x509.list
-$(obj)/.x509.list:
-	@echo $(X509_CERTIFICATES) >$@
 endif
 
 clean-files := x509_certificate_list .x509.list
@@ -212,40 +241,16 @@ x509.genkey:
 	@echo >>x509.genkey "authorityKeyIdentifier=keyid"
 endif
 
-# We need to obtain the certificate from CONFIG_MODULE_SIG_KEY.
-quiet_cmd_extract_der = CERT_DER $(2)
-      cmd_extract_der = scripts/extract-cert "$(2)" signing_key.x509
+$(eval $(call config_filename,MODULE_SIG_KEY))
 
-# 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
+# If CONFIG_MODULE_SIG_KEY isn't a PKCS#11 URI, depend on it
+ifeq ($(patsubst pkcs11:%,%,$(firstword $(MODULE_SIG_KEY_FILENAME))),$(firstword $(MODULE_SIG_KEY_FILENAME)))
+X509_DEP := $(MODULE_SIG_KEY_SRCPREFIX)$(MODULE_SIG_KEY_FILENAME)
 endif
 
+quiet_cmd_extract_der = SIGNING_CERT $(patsubst "%",%,$(2))
+      cmd_extract_der = scripts/extract-cert $(2) signing_key.x509
+
 signing_key.x509: scripts/extract-cert include/config/module/sig/key.h $(X509_DEP)
-	$(call cmd,extract_der,$(X509_SOURCE))
+	$(call cmd,extract_der,$(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY))
 endif


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

* [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (14 preceding siblings ...)
  2015-05-28 15:48 ` [PATCH 15/20] modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option " David Howells
@ 2015-05-28 15:48 ` David Howells
  2015-05-29  1:30   ` Andy Lutomirski
  2015-05-29 12:40   ` David Howells
  2015-05-28 15:48 ` [PATCH 17/20] X.509: Restrict the usage of a key based on information in X.509 certificate " David Howells
                   ` (5 subsequent siblings)
  21 siblings, 2 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:48 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

Modify the sign-file program to take a "-F <firmware name>" parameter.  The
name is a utf8 string that, if given, is inserted in a PKCS#7 authenticated
attribute from where it can be extracted by the kernel.  Authenticated
attributes are added to the signature digest.

If the attribute is present, the signature would be assumed to be for
firmware and would not be permitted with module signing or kexec.  The name
associated with the attribute would be compared to the name passed to
request_firmware() and the load request would be denied if they didn't
match.

If not present, the signature would be rejected if used for firmware.

One oddity is that the attribute is per-signature, so if a second signature
was added (which PKCS#7 supports), it would have to have the attribute added
separately to that signature also.

Note that the OID I have selected is an unregistered element of the Red Hat
OID space to serve as an example.  An OID would need to be allocated for
this purpose.

The kernel then parses this out, saves the string and makes sure the same
string (or lack thereof) is present from all signers.  Then when
system_verify_data() is called, it is passed a NULL if the attribute is
expected not to be present and the name from request_firmware() if it is
expected to be present.  Verification is rejected if there's a mismatch.


========================
MARKING KEYS FOR PURPOSE
========================

To mark a key as being for firmware signing only, for example, the openssl
req command can be given an extension specifier to mark the X.509
certificate.  Assuming a config script is used, this could be done in one of
a couple of ways:

 (1) Add an OID indicating this to the list in the x509v3 extendedKeyUsage
     extension, eg:

	extendedKeyUsage=critical,1.3.6.1.4.1.2312.99.2

 (2) Add our own extension type with our own OID, eg:

	1.3.6.1.4.1.2312.99.2=critical,ASN1:NULL

Option (2) is easier to deal with since we examine all the extensions
anyway, but option (1) is probably the correct way.

Note I'm using an unregistered Red Hat space OID here as an example.

We would need to decide how we want to break down the usage space and
register appropriate OIDs.  If we went with option (2), we could
parameterise the thing and use that to select either driver class or driver.

It's possible that just declaring that a key is used for firmware signing or
anything but and then including the firmware name in the signature is
sufficient for our purposes, rather than breaking it down per driver class
or per driver.

Not-yet-signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/pkcs7_parser.c |   77 +++++++++++++++++++++++++++++++++
 crypto/asymmetric_keys/pkcs7_parser.h |    1 
 include/crypto/pkcs7.h                |    1 
 include/keys/system_keyring.h         |    3 +
 include/linux/oid_registry.h          |    3 +
 kernel/module_signing.c               |    2 -
 kernel/system_keyring.c               |   26 +++++++++++
 scripts/sign-file.c                   |   36 ++++++++++++++-
 8 files changed, 142 insertions(+), 7 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 3bd5a1e4c493..381e3d8bcc8d 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/oid_registry.h>
+#include <linux/ctype.h>
 #include "public_key.h"
 #include "pkcs7_parser.h"
 #include "pkcs7-asn1.h"
@@ -29,6 +30,13 @@ struct pkcs7_parse_context {
 	enum OID	last_oid;		/* Last OID encountered */
 	unsigned	x509_index;
 	unsigned	sinfo_index;
+	unsigned	firmware_name_len;
+	enum {
+		maybe_firmware_sig,
+		is_firmware_sig,
+		isnt_firmware_sig
+	} firmware_sig_state : 8;
+	bool		signer_has_firmware_name;
 	const void	*raw_serial;
 	unsigned	raw_serial_size;
 	unsigned	raw_issuer_size;
@@ -73,6 +81,7 @@ void pkcs7_free_message(struct pkcs7_message *pkcs7)
 			pkcs7->signed_infos = sinfo->next;
 			pkcs7_free_signed_info(sinfo);
 		}
+		kfree(pkcs7->firmware_name);
 		kfree(pkcs7);
 	}
 }
@@ -310,6 +319,8 @@ int pkcs7_sig_note_authenticated_attr(void *context, size_t hdrlen,
 				      const void *value, size_t vlen)
 {
 	struct pkcs7_parse_context *ctx = context;
+	char *p;
+	int i;
 
 	pr_devel("AuthAttr: %02x %zu [%*ph]\n", tag, vlen, (unsigned)vlen, value);
 
@@ -320,6 +331,45 @@ int pkcs7_sig_note_authenticated_attr(void *context, size_t hdrlen,
 		ctx->sinfo->msgdigest = value;
 		ctx->sinfo->msgdigest_len = vlen;
 		return 0;
+
+	case OID_firmwareName:
+		if (tag != ASN1_UTF8STR || vlen == 0)
+			return -EBADMSG;
+
+		/* If there's more than one signature, they must have the same
+		 * firmware name.
+		 */
+		switch (ctx->firmware_sig_state) {
+		case maybe_firmware_sig:
+			for (i = 0; i < vlen; i++)
+				if (!isprint(((const char *)value)[i]))
+					return -EBADMSG;
+			p = kmalloc(vlen + 1, GFP_KERNEL);
+			if (!p)
+				return -ENOMEM;
+			memcpy(p, value, vlen);
+			p[vlen] = 0;
+			ctx->msg->firmware_name = p;
+			ctx->firmware_name_len = vlen;
+			ctx->firmware_sig_state = is_firmware_sig;
+			pr_debug("Found firmware name '%s'\n", p);
+			ctx->signer_has_firmware_name = true;
+			return 0;
+
+		case is_firmware_sig:
+			if (ctx->firmware_name_len != vlen ||
+			    memcmp(ctx->msg->firmware_name, value, vlen) != 0) {
+				pr_warn("Mismatched firmware names\n");
+				return -EBADMSG;
+			}
+			ctx->signer_has_firmware_name = true;
+			return 0;
+
+		case isnt_firmware_sig:
+			pr_warn("Mismatched presence of firmware name\n");
+			return -EBADMSG;
+		}
+
 	default:
 		return 0;
 	}
@@ -334,12 +384,39 @@ int pkcs7_sig_note_set_of_authattrs(void *context, size_t hdrlen,
 {
 	struct pkcs7_parse_context *ctx = context;
 
+	/* Make sure we either have all or no firmware names */
+	switch (ctx->firmware_sig_state) {
+	case maybe_firmware_sig:
+		ctx->firmware_sig_state = isnt_firmware_sig;
+	case isnt_firmware_sig:
+		break;
+	case is_firmware_sig:
+		if (!ctx->signer_has_firmware_name) {
+			pr_warn("Mismatched presence of firmware name\n");
+			return -EBADMSG;
+		}
+		ctx->signer_has_firmware_name = false;
+		break;
+	}
+
 	/* We need to switch the 'CONT 0' to a 'SET OF' when we digest */
 	ctx->sinfo->authattrs = value - (hdrlen - 1);
 	ctx->sinfo->authattrs_len = vlen + (hdrlen - 1);
 	return 0;
 }
 
+/**
+ * pkcs7_get_firmware_name - Get firmware name extension value
+ * @pkcs7: The preparsed PKCS#7 message to access
+ *
+ * Get the value of the firmware name extension if there was one or NULL
+ * otherwise.
+ */
+const char *pkcs7_get_firmware_name(const struct pkcs7_message *pkcs7)
+{
+	return pkcs7->firmware_name;
+}
+
 /*
  * Note the issuing certificate serial number
  */
diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
index efc7dc9b8f9c..03a30f8578e2 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -50,6 +50,7 @@ struct pkcs7_message {
 	struct x509_certificate *certs;	/* Certificate list */
 	struct x509_certificate *crl;	/* Revocation list */
 	struct pkcs7_signed_info *signed_infos;
+	char *firmware_name;		/* Firmware name if present */
 
 	/* Content Data (or NULL) */
 	enum OID	data_type;	/* Type of Data */
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index e235ab4957ee..0999eac6313f 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -22,6 +22,7 @@ extern void pkcs7_free_message(struct pkcs7_message *pkcs7);
 extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
 				  const void **_data, size_t *_datalen,
 				  bool want_wrapper);
+extern const char *pkcs7_get_firmware_name(const struct pkcs7_message *pkcs7);
 
 /*
  * pkcs7_trust.c
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 9791c907cdb7..30303745f845 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -30,7 +30,8 @@ static inline struct key *get_system_trusted_keyring(void)
 
 #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
 extern int system_verify_data(const void *data, unsigned long len,
-			      const void *raw_pkcs7, size_t pkcs7_len);
+			      const void *raw_pkcs7, size_t pkcs7_len,
+			      const char *firmware_name);
 #endif
 
 #endif /* _KEYS_SYSTEM_KEYRING_H */
diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
index c2bbf672b84e..db8d382699b5 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -88,6 +88,9 @@ enum OID {
 	OID_authorityKeyIdentifier,	/* 2.5.29.35 */
 	OID_extKeyUsage,		/* 2.5.29.37 */
 
+	/* Signing */
+	OID_firmwareName,		/* 1.3.6.1.4.1.2312.99.1 */
+
 	OID__NR
 };
 
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 70ad463f6df0..9361711897ce 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -72,5 +72,5 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
 		return -EBADMSG;
 	}
 
-	return system_verify_data(mod, modlen, mod + modlen, sig_len);
+	return system_verify_data(mod, modlen, mod + modlen, sig_len, NULL);
 }
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index 95f2dcbc7616..ccb2814f89c1 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -113,11 +113,14 @@ late_initcall(load_system_certificate_list);
  * @len: Size of @data.
  * @raw_pkcs7: The PKCS#7 message that is the signature.
  * @pkcs7_len: The size of @raw_pkcs7.
+ * @firmware_name: The required firmware name or NULL.
  */
 int system_verify_data(const void *data, unsigned long len,
-		       const void *raw_pkcs7, size_t pkcs7_len)
+		       const void *raw_pkcs7, size_t pkcs7_len,
+		       const char *firmware_name)
 {
 	struct pkcs7_message *pkcs7;
+	const char *p7_firmware_name;
 	bool trusted;
 	int ret;
 
@@ -125,6 +128,27 @@ int system_verify_data(const void *data, unsigned long len,
 	if (IS_ERR(pkcs7))
 		return PTR_ERR(pkcs7);
 
+	ret = -EINVAL;
+	p7_firmware_name = pkcs7_get_firmware_name(pkcs7);
+	if (firmware_name) {
+		if (!p7_firmware_name) {
+			pr_err("Expected name '%s' in firmware signature but not present\n",
+			       firmware_name);
+			goto error;
+		}
+		if (strcmp(p7_firmware_name, firmware_name) != 0) {
+			pr_err("Expected name '%s' in firmware signature but got '%s'\n",
+			       firmware_name, p7_firmware_name);
+			goto error;
+		}
+	} else {
+		if (p7_firmware_name) {
+			pr_err("Unexpected firmware name in signature '%s'\n",
+			       p7_firmware_name);
+			goto error;
+		}
+	}
+
 	/* 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");
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index ad0aa21bd3ac..94109c9e8ce6 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -43,6 +43,8 @@ void format(void)
 {
 	fprintf(stderr,
 		"Usage: scripts/sign-file [-dp] <hash algo> <key> <x509> <module> [<dest>]\n");
+	fprintf(stderr,
+		"       scripts/sign-file [-dp] -F <firmware name> <hash algo> <key> <x509> <firmware> [<dest>]\n");
 	exit(2);
 }
 
@@ -105,12 +107,14 @@ static int pem_pw_cb(char *buf, int len, int w, void *v)
 int main(int argc, char **argv)
 {
 	struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
+	const char *firmware_name = NULL;
 	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;
+	PKCS7_SIGNER_INFO *signer;
 	const EVP_MD *digest_algo;
 	EVP_PKEY *private_key;
 	PKCS7 *pkcs7;
@@ -125,10 +129,11 @@ int main(int argc, char **argv)
 	key_pass = getenv("KBUILD_SIGN_PIN");
 
 	do {
-		opt = getopt(argc, argv, "dp");
+		opt = getopt(argc, argv, "dpF:");
 		switch (opt) {
 		case 'p': save_pkcs7 = true; break;
 		case 'd': sign_only = true; save_pkcs7 = true; break;
+		case 'F': firmware_name = optarg; break;
 		case -1: break;
 		default: format();
 		}
@@ -210,11 +215,34 @@ int main(int argc, char **argv)
 
 	/* 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);
+			   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");
+	signer = PKCS7_sign_add_signer(pkcs7, x509, private_key, digest_algo,
+				       PKCS7_NOCERTS | PKCS7_BINARY);
+	ERR(!signer, "PKCS7_sign_add_signer");
+
+	if (firmware_name) {
+		/* Add an entry into the authenticated attributes to note the
+		 * firmware name if this is a firmware signature.
+		 */
+		ASN1_UTF8STRING *str;
+		int nid;
+
+		// As an example, use an OID of redhat.99.1 - which is not assigned
+		nid = OBJ_create("1.3.6.1.4.1.2312.99.1", NULL, NULL);
+		ERR(!nid, "OBJ_create");
+
+		str = M_ASN1_UTF8STRING_new();
+		ERR(!str, "M_ASN1_UTF8STRING_new");
+		ERR(!ASN1_STRING_set(str, firmware_name, strlen(firmware_name)),
+		    "ASN1_STRING_set");
+		ERR(!PKCS7_add_signed_attribute(signer, nid,
+						V_ASN1_UTF8STRING, str),
+		    "PKCS7_add_signed_attribute");
+	}
+
 	ERR(PKCS7_final(pkcs7, bm, PKCS7_NOCERTS | PKCS7_BINARY) < 0,
 	    "PKCS7_final");
 


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

* [PATCH 17/20] X.509: Restrict the usage of a key based on information in X.509 certificate [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (15 preceding siblings ...)
  2015-05-28 15:48 ` [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name " David Howells
@ 2015-05-28 15:48 ` David Howells
  2015-05-28 15:48 ` [PATCH 18/20] X.509: Parse the keyUsage extension to detect key-signing keys " David Howells
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:48 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

Use X.509 extendedKeyUsage extension [RFC5280 4.2.1.12] to hold restriction
information as to the purpose of the key.  The following changes are made:

 (1) The kernel's X.509 parser is modified to extract this information and
     stash it in the public_key struct.

 (2) The kernel indicates in /proc/keys the restriction if one is found.

 (3) Autogenerated module signing key certificates are marked with a module
     signing only restriction.

The extendedKeyUsage extension takes a sequence of OIDs to indicate the set of
restricted cases.  To this end, I have temporarily used three unassigned OIDs
from RH OID space.  These will need to be replaced with real assigned OIDs:

	1.3.6.1.4.1.2312.99.2	Key is restricted to firmware signing
	1.3.6.1.4.1.2312.99.3	Key is restricted to module signing
	1.3.6.1.4.1.2312.99.4	Key is restricted to kexecable image signing

I would propose a fourth, key signing, but that should perhaps be handled
through the keyUsage extension [RFC5280 4.2.1.3] setting keyCertSign.

I am treating these as mutually exclusive.  A key with a restriction is
rejected if it also gives a second restriction.


To mark a key as being for firmware signing only, for example, the "openssl
req" command can be given an extension specifier to mark the X.509
certificate.  Assuming a config script is used, this would be done by
including the following in the extension list:

	extendedKeyUsage=critical,1.3.6.1.4.1.2312.99.2

This adds it to the extendedKeyUsage extension.  Another, perhaps more
convenient way to do it would be to add our own extension type, eg:

	1.3.6.1.4.1.2312.99.2=critical,ASN1:NULL

This would easier to deal with since we examine all the extensions anyway, and
we could parameterise it, but the first option is probably the correct way.

Also, do we need to break the firmware restriction space down by class or
manufacturer?  Or will one restriction do?

Not-yet-signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/Makefile           |    4 ++
 crypto/asymmetric_keys/asymmetric_type.c  |    3 +
 crypto/asymmetric_keys/public_key.c       |   23 +++++++++
 crypto/asymmetric_keys/x509_cert_parser.c |   72 +++++++++++++++++++++++++++++
 crypto/asymmetric_keys/x509_extusage.asn1 |    3 +
 include/crypto/public_key.h               |   12 +++++
 include/keys/asymmetric-subtype.h         |    3 +
 include/linux/oid_registry.h              |    3 +
 kernel/Makefile                           |    1 
 lib/oid_registry.c                        |    1 
 10 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 crypto/asymmetric_keys/x509_extusage.asn1

diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index cd1406f9b14a..ac18b7f64a77 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o
 x509_key_parser-y := \
 	x509-asn1.o \
 	x509_akid-asn1.o \
+	x509_extusage-asn1.o \
 	x509_rsakey-asn1.o \
 	x509_cert_parser.o \
 	x509_public_key.o
@@ -23,13 +24,16 @@ x509_key_parser-y := \
 $(obj)/x509_cert_parser.o: \
 	$(obj)/x509-asn1.h \
 	$(obj)/x509_akid-asn1.h \
+	$(obj)/x509_extusage-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_extusage-asn1.o: $(obj)/x509_extusage-asn1.c $(obj)/x509_extusage-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_extusage-asn1.c x509_extusage-asn1.h
 clean-files	+= x509_rsakey-asn1.c x509_rsakey-asn1.h
 
 #
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index bcbbbd794e1d..9047175fe818 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -246,7 +246,8 @@ static void asymmetric_key_describe(const struct key *key, struct seq_file *m)
 		}
 
 		seq_puts(m, " [");
-		/* put something here to indicate the key's capabilities */
+		if (subtype->describe_caps)
+			subtype->describe_caps(key, m);
 		seq_putc(m, ']');
 	}
 }
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 2f6e4fb1a1ea..7790f58d00bf 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -42,6 +42,16 @@ const char *const pkey_id_type_name[PKEY_ID_TYPE__LAST] = {
 };
 EXPORT_SYMBOL_GPL(pkey_id_type_name);
 
+static const char *const public_key_restrictions[NR__PKEY_USAGE_RESTRICTION] = {
+	[PKEY_USAGE_NOT_SPECIFIED]		= "unrestricted",
+	[PKEY_RESTRICTED_USAGE]			= "unspecified",
+	[PKEY_RESTRICTED_TO_OTHER]		= "other use",
+	[PKEY_RESTRICTED_TO_MODULE_SIGNING]	= "module sig",
+	[PKEY_RESTRICTED_TO_FIRMWARE_SIGNING]	= "firmware sig",
+	[PKEY_RESTRICTED_TO_KEXEC_SIGNING]	= "kexec sig",
+	[PKEY_RESTRICTED_TO_KEY_SIGNING]	= "key sig",
+};
+
 /*
  * Provide a part of a description of the key for /proc/keys.
  */
@@ -56,6 +66,18 @@ static void public_key_describe(const struct key *asymmetric_key,
 }
 
 /*
+ * Describe capabilities/restrictions of the key for /proc/keys.
+ */
+static void public_key_describe_caps(const struct key *asymmetric_key,
+				     struct seq_file *m)
+{
+	struct public_key *key = asymmetric_key->payload.data;
+
+	if (key)
+		seq_puts(m, public_key_restrictions[key->usage_restriction]);
+}
+
+/*
  * Destroy a public key algorithm key.
  */
 void public_key_destroy(void *payload)
@@ -123,6 +145,7 @@ struct asymmetric_key_subtype public_key_subtype = {
 	.name			= "public_key",
 	.name_len		= sizeof("public_key") - 1,
 	.describe		= public_key_describe,
+	.describe_caps		= public_key_describe_caps,
 	.destroy		= public_key_destroy,
 	.verify_signature	= public_key_verify_signature_2,
 };
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 6c130dd56f35..4d9e9a7f8fd2 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -19,6 +19,7 @@
 #include "x509_parser.h"
 #include "x509-asn1.h"
 #include "x509_akid-asn1.h"
+#include "x509_extusage-asn1.h"
 #include "x509_rsakey-asn1.h"
 
 struct x509_parse_context {
@@ -40,6 +41,8 @@ struct x509_parse_context {
 	const void	*raw_akid;		/* Raw authorityKeyId in ASN.1 */
 	const void	*akid_raw_issuer;	/* Raw directoryName in authorityKeyId */
 	unsigned	akid_raw_issuer_size;
+	unsigned	raw_extusage_size;
+	const void	*raw_extusage;		/* Raw extKeyUsage in ASN.1 */
 };
 
 /*
@@ -91,6 +94,20 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 	if (ret < 0)
 		goto error_decode;
 
+	/* Decode the extended key usage information */
+	if (ctx->raw_extusage) {
+		pr_devel("EXTUSAGE: %u %*phN\n",
+			 ctx->raw_extusage_size, ctx->raw_extusage_size,
+			 ctx->raw_extusage);
+		ret = asn1_ber_decoder(&x509_extusage_decoder, ctx,
+				       ctx->raw_extusage,
+				       ctx->raw_extusage_size);
+		if (ret < 0) {
+			pr_warn("Couldn't decode extKeyUsage\n");
+			goto error_decode;
+		}
+	}
+
 	/* Decode the AuthorityKeyIdentifier */
 	if (ctx->raw_akid) {
 		pr_devel("AKID: %u %*phN\n",
@@ -471,6 +488,14 @@ int x509_process_extension(void *context, size_t hdrlen,
 		return 0;
 	}
 
+	if (ctx->last_oid == OID_extKeyUsage) {
+		/* Get hold of the extended key usage information */
+		ctx->raw_extusage = v;
+		ctx->raw_extusage_size = vlen;
+		ctx->cert->pub->usage_restriction = PKEY_RESTRICTED_USAGE;
+		return 0;
+	}
+
 	return 0;
 }
 
@@ -605,3 +630,50 @@ int x509_akid_note_serial(void *context, size_t hdrlen,
 	ctx->cert->akid_id = kid;
 	return 0;
 }
+
+/*
+ * Note restriction to a purpose
+ */
+int x509_extusage_note_purpose(void *context, size_t hdrlen,
+			       unsigned char tag,
+			       const void *value, size_t vlen)
+{
+	struct x509_parse_context *ctx = context;
+	enum pkey_usage_restriction restriction;
+	char buffer[50];
+	enum OID oid;
+
+	sprint_oid(value, vlen, buffer, sizeof(buffer));
+	pr_debug("ExtUsage: %s\n", buffer);
+
+	oid = look_up_OID(value, vlen);
+	if (oid == OID__NR) {
+		pr_debug("Unknown extension: [%lu] %s\n",
+			 (unsigned long)value - ctx->data, buffer);
+		return 0;
+	}
+
+	switch (oid) {
+	case OID_firmwareSigningOnlyKey:
+		restriction = PKEY_RESTRICTED_TO_FIRMWARE_SIGNING;
+		break;
+	case OID_moduleSigningOnlyKey:
+		restriction = PKEY_RESTRICTED_TO_MODULE_SIGNING;
+		break;
+	case OID_kexecSigningOnlyKey:
+		restriction = PKEY_RESTRICTED_TO_KEXEC_SIGNING;
+		break;
+	default:
+		restriction = PKEY_RESTRICTED_TO_OTHER;
+		break;
+	}
+
+	if (ctx->cert->pub->usage_restriction != PKEY_RESTRICTED_USAGE) {
+		pr_warn("Rejecting certificate with multiple restrictions\n");
+		return -EKEYREJECTED;
+	}
+
+	ctx->cert->pub->usage_restriction = restriction;
+	pr_debug("usage restriction %u\n", restriction);
+	return 0;
+}
diff --git a/crypto/asymmetric_keys/x509_extusage.asn1 b/crypto/asymmetric_keys/x509_extusage.asn1
new file mode 100644
index 000000000000..ffe12ae644b5
--- /dev/null
+++ b/crypto/asymmetric_keys/x509_extusage.asn1
@@ -0,0 +1,3 @@
+ExtKeyUsageSyntax ::= SEQUENCE OF KeyPurposeId
+
+KeyPurposeId ::= OBJECT IDENTIFIER ({ x509_extusage_note_purpose })
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index fda097e079a4..d15c74cb1f98 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -37,6 +37,17 @@ enum pkey_id_type {
 	PKEY_ID_TYPE__LAST
 };
 
+enum pkey_usage_restriction {
+	PKEY_USAGE_NOT_SPECIFIED,
+	PKEY_RESTRICTED_USAGE,
+	PKEY_RESTRICTED_TO_OTHER,
+	PKEY_RESTRICTED_TO_MODULE_SIGNING,
+	PKEY_RESTRICTED_TO_FIRMWARE_SIGNING,
+	PKEY_RESTRICTED_TO_KEXEC_SIGNING,
+	PKEY_RESTRICTED_TO_KEY_SIGNING,
+	NR__PKEY_USAGE_RESTRICTION
+};
+
 extern const char *const pkey_id_type_name[PKEY_ID_TYPE__LAST];
 
 /*
@@ -52,6 +63,7 @@ struct public_key {
 #define PKEY_CAN_DECRYPT	0x02
 #define PKEY_CAN_SIGN		0x04
 #define PKEY_CAN_VERIFY		0x08
+	enum pkey_usage_restriction usage_restriction : 8;
 	enum pkey_algo pkey_algo : 8;
 	enum pkey_id_type id_type : 8;
 	union {
diff --git a/include/keys/asymmetric-subtype.h b/include/keys/asymmetric-subtype.h
index 4b840e822209..b6a47f09ef2b 100644
--- a/include/keys/asymmetric-subtype.h
+++ b/include/keys/asymmetric-subtype.h
@@ -31,6 +31,9 @@ struct asymmetric_key_subtype {
 	/* Describe a key of this subtype for /proc/keys */
 	void (*describe)(const struct key *key, struct seq_file *m);
 
+	/* Describe capabilities/restrictions of a key of this subtype */
+	void (*describe_caps)(const struct key *key, struct seq_file *m);
+
 	/* Destroy a key of this subtype */
 	void (*destroy)(void *payload);
 
diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
index db8d382699b5..46b3b10e2dc1 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -90,6 +90,9 @@ enum OID {
 
 	/* Signing */
 	OID_firmwareName,		/* 1.3.6.1.4.1.2312.99.1 */
+	OID_firmwareSigningOnlyKey,	/* 1.3.6.1.4.1.2312.99.2 */
+	OID_moduleSigningOnlyKey,	/* 1.3.6.1.4.1.2312.99.3 */
+	OID_kexecSigningOnlyKey,	/* 1.3.6.1.4.1.2312.99.4 */
 
 	OID__NR
 };
diff --git a/kernel/Makefile b/kernel/Makefile
index 2d03e870ba8d..1b1aea6fe655 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -239,6 +239,7 @@ x509.genkey:
 	@echo >>x509.genkey "keyUsage=digitalSignature"
 	@echo >>x509.genkey "subjectKeyIdentifier=hash"
 	@echo >>x509.genkey "authorityKeyIdentifier=keyid"
+	@echo >>x509.genkey "extendedKeyUsage=critical,1.3.6.1.4.1.2312.99.3"
 endif
 
 $(eval $(call config_filename,MODULE_SIG_KEY))
diff --git a/lib/oid_registry.c b/lib/oid_registry.c
index 318f382a010d..41df9b4a5fe7 100644
--- a/lib/oid_registry.c
+++ b/lib/oid_registry.c
@@ -115,6 +115,7 @@ int sprint_oid(const void *data, size_t datasize, char *buffer, size_t bufsize)
 	size_t ret;
 	int count;
 
+	buffer[0] = 0;
 	if (v >= end)
 		return -EBADMSG;
 


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

* [PATCH 18/20] X.509: Parse the keyUsage extension to detect key-signing keys [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (16 preceding siblings ...)
  2015-05-28 15:48 ` [PATCH 17/20] X.509: Restrict the usage of a key based on information in X.509 certificate " David Howells
@ 2015-05-28 15:48 ` David Howells
  2015-05-28 15:49 ` [PATCH 19/20] KEYS: Restrict signature verification to keys appropriate to the purpose " David Howells
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:48 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

Parse the keyUsage extension to detect keys for which the purpose is key
signing and to restrict their use only to the verification of signatures on
keys.

Not-yet-signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/x509_cert_parser.c |   45 ++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 4d9e9a7f8fd2..82b869de1759 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -457,9 +457,51 @@ 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;
+	u32 shift, tmp;
 
 	pr_debug("Extension: %u\n", ctx->last_oid);
 
+	if (ctx->last_oid == OID_keyUsage) {
+		/* There's a bitstring inside the content that we want to get
+		 * at.  This bitstring is shuffled up so that the most
+		 * significant set bit is the MSB of the second byte.  The
+		 * first byte tells us how far we have to shift the string back
+		 * to put bit string bit 0 on a byte bit 0.
+		 */
+		if (vlen < 3)
+			return -EBADMSG;
+		if (v[0] != ASN1_BTS || v[1] != vlen - 2)
+			return -EBADMSG;
+		shift = v[2];
+		if (shift > 7)
+			return -EBADMSG;
+		v += 3;
+		vlen -= 3;
+		if (vlen > 2) {
+			/* We only care about the least significant 9 bits, but
+			 * they will be spread over up to two bytes (leading 0s
+			 * aren't stored).
+			 */
+			v += vlen - 2;
+			vlen = 2;
+		}
+
+		tmp = 0;
+		if (vlen == 2) {
+			tmp = (u32)*v << 8;
+			v++;
+		}
+		if (vlen >= 1)
+			tmp |= *v;
+		tmp >>= shift;
+		pr_debug("keyUsage %x\n", tmp);
+		/* check for keyCertSign */
+		if (tmp & (1 << 5))
+			ctx->cert->pub->usage_restriction =
+				PKEY_RESTRICTED_TO_KEY_SIGNING;
+		return 0;
+	}
+
 	if (ctx->last_oid == OID_subjectKeyIdentifier) {
 		/* Get hold of the key fingerprint */
 		if (ctx->cert->skid || vlen < 3)
@@ -492,7 +534,8 @@ int x509_process_extension(void *context, size_t hdrlen,
 		/* Get hold of the extended key usage information */
 		ctx->raw_extusage = v;
 		ctx->raw_extusage_size = vlen;
-		ctx->cert->pub->usage_restriction = PKEY_RESTRICTED_USAGE;
+		if (ctx->cert->pub->usage_restriction == PKEY_USAGE_NOT_SPECIFIED)
+			ctx->cert->pub->usage_restriction = PKEY_RESTRICTED_USAGE;
 		return 0;
 	}
 


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

* [PATCH 19/20] KEYS: Restrict signature verification to keys appropriate to the purpose [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (17 preceding siblings ...)
  2015-05-28 15:48 ` [PATCH 18/20] X.509: Parse the keyUsage extension to detect key-signing keys " David Howells
@ 2015-05-28 15:49 ` David Howells
  2015-05-28 15:49 ` [PATCH 20/20] sign-file: use .p7s instead of .pkcs7 file extension " David Howells
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:49 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

Restrict the verification of X.509 certificates such that a certificate can
only be verified if either:

 (1) A certificate is signed with the key it holds.

 (2) A certificate is signed with a key that has keyCertSign set in its
     keyUsage extension and has no purpose restriction set.

Restrict the verification of PKCS#7 messages such that a signature can only be
verified by a matching key if the key does not have keyCertSign set and either
of the following is true:

 (1) The key has no purpose restriction and the PKCS#7 is not a firmware
     signature.

 (2) The key has a recognised purpose restriction that matches the use to
     which the PKCS#7 signature is being put.

In the event that a restriction mismatch occurs, EKEYREJECTED will be returned
and an error similar to one of the following will be logged to dmesg:

	PKEY: Firmware signed with non-firmware key (module sig)
	PKEY: Restricted usage key (module sig) used for wrong purpose (kexec sig)

The PKCS#7 test key type is given the usage to specify in a module parameter.
For example:

	echo 1 >/sys/module/pkcs7_test_key/parameters/usage
	keyctl padd pkcs7_test foo @s </tmp/stuff.pkcs7

will attempt to check the signature on stuff.pkcs7 as if it contains a
firmware blob (1 being KEY_VERIFYING_FIRMWARE_SIGNATURE).

Not-yet-signed-off-by: David Howells <dhowells@redhat.com>
---

 crypto/asymmetric_keys/asymmetric_type.c |    9 +++++
 crypto/asymmetric_keys/pkcs7_key_type.c  |   19 +++++++++-
 crypto/asymmetric_keys/pkcs7_trust.c     |   14 +++++---
 crypto/asymmetric_keys/pkcs7_verify.c    |   21 ++++++++---
 crypto/asymmetric_keys/public_key.c      |   55 ++++++++++++++++++++++++++++--
 crypto/asymmetric_keys/public_key.h      |    3 +-
 crypto/asymmetric_keys/signature.c       |    6 ++-
 crypto/asymmetric_keys/x509_parser.h     |    3 +-
 crypto/asymmetric_keys/x509_public_key.c |   15 +++++---
 include/crypto/pkcs7.h                   |    4 ++
 include/crypto/public_key.h              |    3 +-
 include/keys/asymmetric-subtype.h        |    3 +-
 include/keys/asymmetric-type.h           |   13 +++++++
 include/keys/system_keyring.h            |    2 +
 kernel/module_signing.c                  |    3 +-
 kernel/system_keyring.c                  |    7 +++-
 16 files changed, 148 insertions(+), 32 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 9047175fe818..6367d3734533 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -20,6 +20,15 @@
 
 MODULE_LICENSE("GPL");
 
+const char *const asymmetric_key_verification_names[NR__ASYMMETRIC_KEY_VERIFICATION] = {
+	[KEY_VERIFYING_MODULE_SIGNATURE]	= "mod sig",
+	[KEY_VERIFYING_FIRMWARE_SIGNATURE]	= "firmware sig",
+	[KEY_VERIFYING_KEXEC_SIGNATURE]		= "kexec sig",
+	[KEY_VERIFYING_KEY_SIGNATURE]		= "key sig",
+	[KEY_VERIFYING_KEY_SELF_SIGNATURE]	= "key self sig",
+};
+EXPORT_SYMBOL_GPL(asymmetric_key_verification_names);
+
 static LIST_HEAD(asymmetric_key_parsers);
 static DECLARE_RWSEM(asymmetric_key_parsers_sem);
 
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
index 751f8fd7335d..ce5eb0346d83 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -12,17 +12,24 @@
 #define pr_fmt(fmt) "PKCS7key: "fmt
 #include <linux/key.h>
 #include <linux/err.h>
-#include <linux/key-type.h>
+#include <linux/module.h>
+#include <keys/asymmetric-type.h>
 #include <crypto/pkcs7.h>
 #include <keys/user-type.h>
 #include <keys/system_keyring.h>
 #include "pkcs7_parser.h"
 
+unsigned pkcs7_usage;
+module_param_named(usage, pkcs7_usage, uint, S_IWUSR | S_IRUGO);
+MODULE_PARM_DESC(pkcs7_usage,
+		 "Usage to specify when verifying the PKCS#7 message");
+
 /*
  * Preparse a PKCS#7 wrapped and validated data blob.
  */
 static int pkcs7_preparse(struct key_preparsed_payload *prep)
 {
+	enum asymmetric_key_verification usage = pkcs7_usage;
 	struct pkcs7_message *pkcs7;
 	const void *data, *saved_prep_data;
 	size_t datalen, saved_prep_datalen;
@@ -31,6 +38,11 @@ static int pkcs7_preparse(struct key_preparsed_payload *prep)
 
 	kenter("");
 
+	if (usage >= NR__ASYMMETRIC_KEY_VERIFICATION) {
+		pr_err("Invalid usage type %d\n", usage);
+		return -EINVAL;
+	}
+
 	saved_prep_data = prep->data;
 	saved_prep_datalen = prep->datalen;
 	pkcs7 = pkcs7_parse_message(saved_prep_data, saved_prep_datalen);
@@ -39,11 +51,12 @@ static int pkcs7_preparse(struct key_preparsed_payload *prep)
 		goto error;
 	}
 
-	ret = pkcs7_verify(pkcs7);
+	ret = pkcs7_verify(pkcs7, usage);
 	if (ret < 0)
 		goto error_free;
 
-	ret = pkcs7_validate_trust(pkcs7, system_trusted_keyring, &trusted);
+	ret = pkcs7_validate_trust(pkcs7, system_trusted_keyring, usage,
+				   &trusted);
 	if (ret < 0)
 		goto error_free;
 	if (!trusted)
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 90d6d47965b0..bbe3dc9755e4 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -25,7 +25,8 @@
  */
 static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 				    struct pkcs7_signed_info *sinfo,
-				    struct key *trust_keyring)
+				    struct key *trust_keyring,
+				    enum asymmetric_key_verification usage)
 {
 	struct public_key_signature *sig = &sinfo->sig;
 	struct x509_certificate *x509, *last = NULL, *p;
@@ -65,6 +66,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 			 */
 			pr_devel("sinfo %u: Cert %u as key %x\n",
 				 sinfo->index, x509->index, key_serial(key));
+			usage = KEY_VERIFYING_KEY_SIGNATURE;
 			goto matched;
 		}
 		if (key == ERR_PTR(-ENOMEM))
@@ -95,6 +97,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 			x509 = last;
 			pr_devel("sinfo %u: Root cert %u signer is key %x\n",
 				 sinfo->index, x509->index, key_serial(key));
+			usage = KEY_VERIFYING_KEY_SIGNATURE;
 			goto matched;
 		}
 		if (PTR_ERR(key) != -ENOKEY)
@@ -121,7 +124,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 	return -ENOKEY;
 
 matched:
-	ret = verify_signature(key, sig);
+	ret = verify_signature(key, sig, usage);
 	trusted = test_bit(KEY_FLAG_TRUSTED, &key->flags);
 	key_put(key);
 	if (ret < 0) {
@@ -148,7 +151,8 @@ verified:
  * pkcs7_validate_trust - Validate PKCS#7 trust chain
  * @pkcs7: The PKCS#7 certificate to validate
  * @trust_keyring: Signing certificates to use as starting points
- * @_trusted: Set to true if trustworth, false otherwise
+ * @usage: The use to which the key is being put.
+ * @_trusted: Set to true if trustworthy, false otherwise
  *
  * Validate that the certificate chain inside the PKCS#7 message intersects
  * keys we already know and trust.
@@ -171,6 +175,7 @@ verified:
  */
 int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
 			 struct key *trust_keyring,
+			 enum asymmetric_key_verification usage,
 			 bool *_trusted)
 {
 	struct pkcs7_signed_info *sinfo;
@@ -182,7 +187,8 @@ int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
 		p->seen = false;
 
 	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
-		ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring);
+		ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring,
+					       usage);
 		switch (ret) {
 		case -ENOKEY:
 			continue;
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 404f89a0f852..74edabd3b36a 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -208,7 +208,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 				   x509->raw_issuer_size) != 0)
 				return 0;
 
-			ret = x509_check_signature(x509->pub, x509);
+			ret = x509_check_signature(x509->pub, x509,
+						   KEY_VERIFYING_KEY_SELF_SIGNATURE);
 			if (ret < 0)
 				goto maybe_missing_crypto_in_x509;
 			x509->signer = x509;
@@ -262,7 +263,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 				sinfo->index);
 			return 0;
 		}
-		ret = x509_check_signature(p->pub, x509);
+		ret = x509_check_signature(p->pub, x509,
+					   KEY_VERIFYING_KEY_SIGNATURE);
 		if (ret < 0)
 			return ret;
 		x509->signer = p;
@@ -290,7 +292,8 @@ maybe_missing_crypto_in_x509:
  * Verify one signed information block from a PKCS#7 message.
  */
 static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
-			    struct pkcs7_signed_info *sinfo)
+			    struct pkcs7_signed_info *sinfo,
+			    enum asymmetric_key_verification usage)
 {
 	int ret;
 
@@ -315,7 +318,8 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
 		 sinfo->signer->index, sinfo->index);
 
 	/* Verify the PKCS#7 binary against the key */
-	ret = public_key_verify_signature(sinfo->signer->pub, &sinfo->sig);
+	ret = public_key_verify_signature(sinfo->signer->pub, &sinfo->sig,
+					  usage);
 	if (ret < 0)
 		return ret;
 
@@ -328,6 +332,7 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
 /**
  * pkcs7_verify - Verify a PKCS#7 message
  * @pkcs7: The PKCS#7 message to be verified
+ * @usage: The use to which the key is being put
  *
  * Verify a PKCS#7 message is internally consistent - that is, the data digest
  * matches the digest in the AuthAttrs and any signature in the message or one
@@ -339,6 +344,9 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
  *
  * Returns, in order of descending priority:
  *
+ *  (*) -EKEYREJECTED if a key was selected that had a usage restriction at
+ *      odds with the specified usage, or:
+ *
  *  (*) -EKEYREJECTED if a signature failed to match for which we found an
  *	appropriate X.509 certificate, or:
  *
@@ -350,7 +358,8 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
  *  (*) 0 if all the signature chains that don't incur -ENOPKG can be verified
  *	(note that a signature chain may be of zero length), or:
  */
-int pkcs7_verify(struct pkcs7_message *pkcs7)
+int pkcs7_verify(struct pkcs7_message *pkcs7,
+		 enum asymmetric_key_verification usage)
 {
 	struct pkcs7_signed_info *sinfo;
 	struct x509_certificate *x509;
@@ -366,7 +375,7 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
 	}
 
 	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
-		ret = pkcs7_verify_one(pkcs7, sinfo);
+		ret = pkcs7_verify_one(pkcs7, sinfo, usage);
 		if (ret < 0) {
 			if (ret == -ENOPKG) {
 				sinfo->unsupported_crypto = true;
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 7790f58d00bf..9e354d8e86bf 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -94,12 +94,56 @@ void public_key_destroy(void *payload)
 EXPORT_SYMBOL_GPL(public_key_destroy);
 
 /*
+ * Apply key usage policy.
+ */
+static int public_key_usage_policy(enum asymmetric_key_verification usage,
+				   enum pkey_usage_restriction restriction)
+{
+	switch (usage) {
+	case KEY_VERIFYING_MODULE_SIGNATURE:
+		if (restriction != PKEY_RESTRICTED_TO_MODULE_SIGNING &&
+		    restriction != PKEY_USAGE_NOT_SPECIFIED)
+			goto wrong_purpose;
+		return 0;
+	case KEY_VERIFYING_FIRMWARE_SIGNATURE:
+		if (restriction != PKEY_RESTRICTED_TO_FIRMWARE_SIGNING) {
+			pr_warn("Firmware signed with non-firmware key (%s)\n",
+				public_key_restrictions[restriction]);
+			return -EKEYREJECTED;
+		}
+		return 0;
+	case KEY_VERIFYING_KEXEC_SIGNATURE:
+		if (restriction != PKEY_RESTRICTED_TO_KEXEC_SIGNING &&
+		    restriction != PKEY_USAGE_NOT_SPECIFIED)
+			goto wrong_purpose;
+		return 0;
+	case KEY_VERIFYING_KEY_SIGNATURE:
+		if (restriction != PKEY_RESTRICTED_TO_KEY_SIGNING &&
+		    restriction != PKEY_USAGE_NOT_SPECIFIED)
+			goto wrong_purpose;
+		return 0;
+	case KEY_VERIFYING_KEY_SELF_SIGNATURE:
+		return 0;
+	default:
+		BUG();
+	}
+
+wrong_purpose:
+	pr_warn("Restricted usage key (%s) used for wrong purpose (%s)\n",
+		public_key_restrictions[restriction],
+		asymmetric_key_verification_names[usage]);
+	return -EKEYREJECTED;
+}
+
+/*
  * Verify a signature using a public key.
  */
 int public_key_verify_signature(const struct public_key *pk,
-				const struct public_key_signature *sig)
+				const struct public_key_signature *sig,
+				enum asymmetric_key_verification usage)
 {
 	const struct public_key_algorithm *algo;
+	int ret;
 
 	BUG_ON(!pk);
 	BUG_ON(!pk->mpi[0]);
@@ -126,15 +170,20 @@ int public_key_verify_signature(const struct public_key *pk,
 		return -EINVAL;
 	}
 
+	ret = public_key_usage_policy(usage, pk->usage_restriction);
+	if (ret < 0)
+		return ret;
+
 	return algo->verify_signature(pk, sig);
 }
 EXPORT_SYMBOL_GPL(public_key_verify_signature);
 
 static int public_key_verify_signature_2(const struct key *key,
-					 const struct public_key_signature *sig)
+					 const struct public_key_signature *sig,
+					 enum asymmetric_key_verification usage)
 {
 	const struct public_key *pk = key->payload.data;
-	return public_key_verify_signature(pk, sig);
+	return public_key_verify_signature(pk, sig, usage);
 }
 
 /*
diff --git a/crypto/asymmetric_keys/public_key.h b/crypto/asymmetric_keys/public_key.h
index 5c37a22a0637..d8585ac3a8e9 100644
--- a/crypto/asymmetric_keys/public_key.h
+++ b/crypto/asymmetric_keys/public_key.h
@@ -33,4 +33,5 @@ extern const struct public_key_algorithm RSA_public_key_algorithm;
  * public_key.c
  */
 extern int public_key_verify_signature(const struct public_key *pk,
-				       const struct public_key_signature *sig);
+				       const struct public_key_signature *sig,
+				       enum asymmetric_key_verification usage);
diff --git a/crypto/asymmetric_keys/signature.c b/crypto/asymmetric_keys/signature.c
index 7525fd183574..8b69cbff1dbc 100644
--- a/crypto/asymmetric_keys/signature.c
+++ b/crypto/asymmetric_keys/signature.c
@@ -22,11 +22,13 @@
  * verify_signature - Initiate the use of an asymmetric key to verify a signature
  * @key: The asymmetric key to verify against
  * @sig: The signature to check
+ * @usage: The use to which the key is being put.
  *
  * Returns 0 if successful or else an error.
  */
 int verify_signature(const struct key *key,
-		     const struct public_key_signature *sig)
+		     const struct public_key_signature *sig,
+		     enum asymmetric_key_verification usage)
 {
 	const struct asymmetric_key_subtype *subtype;
 	int ret;
@@ -42,7 +44,7 @@ int verify_signature(const struct key *key,
 	if (!subtype->verify_signature)
 		return -ENOTSUPP;
 
-	ret = subtype->verify_signature(key, sig);
+	ret = subtype->verify_signature(key, sig, usage);
 
 	pr_devel("<==%s() = %d\n", __func__, ret);
 	return ret;
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index dcdb5c94f514..e4dc470c8fbd 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -55,4 +55,5 @@ extern struct x509_certificate *x509_cert_parse(const void *data, size_t datalen
  */
 extern int x509_get_sig_params(struct x509_certificate *cert);
 extern int x509_check_signature(const struct public_key *pub,
-				struct x509_certificate *cert);
+				struct x509_certificate *cert,
+				enum asymmetric_key_verification usage);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 8a4e3a29ec31..aaaa6bf8a0f6 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -208,7 +208,8 @@ EXPORT_SYMBOL_GPL(x509_get_sig_params);
  * Check the signature on a certificate using the provided public key
  */
 int x509_check_signature(const struct public_key *pub,
-			 struct x509_certificate *cert)
+			 struct x509_certificate *cert,
+			 enum asymmetric_key_verification usage)
 {
 	int ret;
 
@@ -218,7 +219,7 @@ int x509_check_signature(const struct public_key *pub,
 	if (ret < 0)
 		return ret;
 
-	ret = public_key_verify_signature(pub, &cert->sig);
+	ret = public_key_verify_signature(pub, &cert->sig, usage);
 	if (ret == -ENOPKG)
 		cert->unsupported_crypto = true;
 	pr_debug("Cert Verification: %d\n", ret);
@@ -251,9 +252,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
 					  cert->akid_id, cert->akid_skid,
 					  false);
 	if (!IS_ERR(key))  {
-		if (!use_builtin_keys
-		    || test_bit(KEY_FLAG_BUILTIN, &key->flags))
-			ret = x509_check_signature(key->payload.data, cert);
+		if (!use_builtin_keys ||
+		    test_bit(KEY_FLAG_BUILTIN, &key->flags))
+			ret = x509_check_signature(key->payload.data, cert,
+						   KEY_VERIFYING_KEY_SIGNATURE);
 		key_put(key);
 	}
 	return ret;
@@ -308,7 +310,8 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	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 */
+		ret = x509_check_signature(cert->pub, cert,
+					   KEY_VERIFYING_KEY_SELF_SIGNATURE);
 		if (ret < 0)
 			goto error_free_cert;
 	} else if (!prep->trusted) {
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 0999eac6313f..56388a6d8505 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -29,12 +29,14 @@ extern const char *pkcs7_get_firmware_name(const struct pkcs7_message *pkcs7);
  */
 extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
 				struct key *trust_keyring,
+				enum asymmetric_key_verification usage,
 				bool *_trusted);
 
 /*
  * pkcs7_verify.c
  */
-extern int pkcs7_verify(struct pkcs7_message *pkcs7);
+extern int pkcs7_verify(struct pkcs7_message *pkcs7,
+			enum asymmetric_key_verification usage);
 
 extern int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
 				      const void *data, size_t datalen);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index d15c74cb1f98..c2f51bd7aef5 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -110,7 +110,8 @@ struct public_key_signature {
 
 struct key;
 extern int verify_signature(const struct key *key,
-			    const struct public_key_signature *sig);
+			    const struct public_key_signature *sig,
+			    enum asymmetric_key_verification usage);
 
 struct asymmetric_key_id;
 extern struct key *x509_request_asymmetric_key(struct key *keyring,
diff --git a/include/keys/asymmetric-subtype.h b/include/keys/asymmetric-subtype.h
index b6a47f09ef2b..bc83a0a2d10d 100644
--- a/include/keys/asymmetric-subtype.h
+++ b/include/keys/asymmetric-subtype.h
@@ -39,7 +39,8 @@ struct asymmetric_key_subtype {
 
 	/* Verify the signature on a key of this subtype (optional) */
 	int (*verify_signature)(const struct key *key,
-				const struct public_key_signature *sig);
+				const struct public_key_signature *sig,
+				enum asymmetric_key_verification usage);
 };
 
 /**
diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h
index c0754abb2f56..329e1f1b185b 100644
--- a/include/keys/asymmetric-type.h
+++ b/include/keys/asymmetric-type.h
@@ -60,6 +60,19 @@ extern struct asymmetric_key_id *asymmetric_key_generate_id(const void *val_1,
 							    size_t len_2);
 
 /*
+ * The use to which an asymmetric is being put when verifying a signature.
+ */
+enum asymmetric_key_verification {
+	KEY_VERIFYING_MODULE_SIGNATURE,
+	KEY_VERIFYING_FIRMWARE_SIGNATURE,
+	KEY_VERIFYING_KEXEC_SIGNATURE,
+	KEY_VERIFYING_KEY_SIGNATURE,
+	KEY_VERIFYING_KEY_SELF_SIGNATURE,
+	NR__ASYMMETRIC_KEY_VERIFICATION
+};
+extern const char *const asymmetric_key_verification_names[NR__ASYMMETRIC_KEY_VERIFICATION];
+
+/*
  * The payload is at the discretion of the subtype.
  */
 
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 30303745f845..3f5fee29ce8a 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -29,8 +29,10 @@ static inline struct key *get_system_trusted_keyring(void)
 #endif
 
 #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+enum asymmetric_key_verification;
 extern int system_verify_data(const void *data, unsigned long len,
 			      const void *raw_pkcs7, size_t pkcs7_len,
+			      enum asymmetric_key_verification usage,
 			      const char *firmware_name);
 #endif
 
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 9361711897ce..2ac070a89496 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -72,5 +72,6 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
 		return -EBADMSG;
 	}
 
-	return system_verify_data(mod, modlen, mod + modlen, sig_len, NULL);
+	return system_verify_data(mod, modlen, mod + modlen, sig_len,
+				  KEY_VERIFYING_MODULE_SIGNATURE, NULL);
 }
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index ccb2814f89c1..1ae70cbcb377 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -113,10 +113,12 @@ late_initcall(load_system_certificate_list);
  * @len: Size of @data.
  * @raw_pkcs7: The PKCS#7 message that is the signature.
  * @pkcs7_len: The size of @raw_pkcs7.
+ * @usage: The use to which the key is being put.
  * @firmware_name: The required firmware name or NULL.
  */
 int system_verify_data(const void *data, unsigned long len,
 		       const void *raw_pkcs7, size_t pkcs7_len,
+		       enum asymmetric_key_verification usage,
 		       const char *firmware_name)
 {
 	struct pkcs7_message *pkcs7;
@@ -156,11 +158,12 @@ int system_verify_data(const void *data, unsigned long len,
 		goto error;
 	}
 
-	ret = pkcs7_verify(pkcs7);
+	ret = pkcs7_verify(pkcs7, usage);
 	if (ret < 0)
 		goto error;
 
-	ret = pkcs7_validate_trust(pkcs7, system_trusted_keyring, &trusted);
+	ret = pkcs7_validate_trust(pkcs7, system_trusted_keyring, usage,
+				   &trusted);
 	if (ret < 0)
 		goto error;
 


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

* [PATCH 20/20] sign-file: use .p7s instead of .pkcs7 file extension [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (18 preceding siblings ...)
  2015-05-28 15:49 ` [PATCH 19/20] KEYS: Restrict signature verification to keys appropriate to the purpose " David Howells
@ 2015-05-28 15:49 ` David Howells
  2015-05-28 15:52 ` [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures " David Woodhouse
  2015-05-28 15:57 ` David Howells
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:49 UTC (permalink / raw)
  To: mcgrof
  Cc: mjg59, keyrings, gregkh, kyle, linux-wireless, linux-kernel,
	seth.forshee, linux-security-module, zohar, dwmw2

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

The file extension .p7s is much more common, use that.

Cc: Kyle McMartin <kyle@kernel.org>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 scripts/sign-file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 94109c9e8ce6..cf9eb7f79868 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -249,7 +249,7 @@ int main(int argc, char **argv)
 	if (save_pkcs7) {
 		char *pkcs7_name;
 
-		ERR(asprintf(&pkcs7_name, "%s.pkcs7", module_name) < 0, "asprintf");
+		ERR(asprintf(&pkcs7_name, "%s.p7s", 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);


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

* Re: [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (19 preceding siblings ...)
  2015-05-28 15:49 ` [PATCH 20/20] sign-file: use .p7s instead of .pkcs7 file extension " David Howells
@ 2015-05-28 15:52 ` David Woodhouse
  2015-05-28 15:57 ` David Howells
  21 siblings, 0 replies; 28+ messages in thread
From: David Woodhouse @ 2015-05-28 15:52 UTC (permalink / raw)
  To: David Howells
  Cc: mcgrof, mjg59, keyrings, gregkh, kyle, linux-wireless,
	linux-kernel, seth.forshee, linux-security-module, zohar

On Thu, 2015-05-28 at 16:46 +0100, David Howells wrote:
> 
> Additionally, the last four patches are provisionally added to support firmware
> signing, but will need further modification (ie. registration of OIDs) before
> they can be committed, but are included for comment:

I'd quite like to see a way for a given driver to specify the key with
which its firmware needs to be signed. Perhaps an extra argument to the
request_firmware() call giving the X509v3 Subject Key Identifier of the
cert to be trusted?

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


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

* Re: [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5]
  2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
                   ` (20 preceding siblings ...)
  2015-05-28 15:52 ` [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures " David Woodhouse
@ 2015-05-28 15:57 ` David Howells
  21 siblings, 0 replies; 28+ messages in thread
From: David Howells @ 2015-05-28 15:57 UTC (permalink / raw)
  Cc: dhowells, mcgrof, mjg59, keyrings, gregkh, kyle, linux-wireless,
	linux-kernel, seth.forshee, linux-security-module, zohar, dwmw2

David Howells <dhowells@redhat.com> wrote:

> Additionally, the last four patches are provisionally added to support firmware

The last five, that is.

David

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

* Re: [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name [ver #5]
  2015-05-28 15:48 ` [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name " David Howells
@ 2015-05-29  1:30   ` Andy Lutomirski
  2015-05-29 12:40   ` David Howells
  1 sibling, 0 replies; 28+ messages in thread
From: Andy Lutomirski @ 2015-05-29  1:30 UTC (permalink / raw)
  To: David Howells, Luis Rodriguez
  Cc: Matthew Garrett, keyrings, Greg Kroah-Hartman, Kyle McMartin,
	linux-wireless, Linux Kernel Mailing List, Seth Forshee,
	LSM List, Mimi Zohar, David Woodhouse

[resending with further gmane screwups fixed]

On 05/28/2015 08:48 AM, David Howells wrote:
> Modify the sign-file program to take a "-F <firmware name>" parameter.  The
> name is a utf8 string that, if given, is inserted in a PKCS#7 authenticated
> attribute from where it can be extracted by the kernel.  Authenticated
> attributes are added to the signature digest.
>
> If the attribute is present, the signature would be assumed to be for
> firmware and would not be permitted with module signing or kexec.  The name
> associated with the attribute would be compared to the name passed to
> request_firmware() and the load request would be denied if they didn't
> match.

This is insecure because PKCS#7 authenticated attributes are broken (see 
RFC2315 section 9.4 note 4).  You need to either require that everything 
have authenticated attributes or require that nothing have authenticated 
attributes.   Maybe this insecurity doesn't matter in practice, but I 
don't wouldn't want to bet on it.

On top of that, this is a ton of code to support something trivial.  And 
it requires an OID to be registered (ick).

Earlier you suggested just appending the signature purpose to the thing 
being signed.  What's wrong with that?  It's probably much less code, it 
doesn't require reviewing details of the godawful PKCS#7 spec, and it 
will continue working if and when someone adds a more sensible signature 
format.  And you could tear out PKCS#7 authenticated attribute support 
on top of it.

P.S.  Or you could stop using PKCS#7 if possible.  Holy crap, maybe it's 
a standard, but it's a standard that we don't actually have to follow 
and it *has trivial collisions by construction*.

For Pete's sake, there are already 1262 lines of code just implementing 
PKCS#7.  In contrast, the *entire* tweetnacl library, which uses better 
crypto and is actually correct (no built-in collisions) fits a complete 
signature implementation and the underlying crypto in barely half that 
many lines.  This is actually unfair to tweetnacl, as tweetnacl also 
includes an AEAD, which could be removed to shorten it by a decent amount.

--Andy

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

* Re: [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name [ver #5]
  2015-05-28 15:48 ` [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name " David Howells
  2015-05-29  1:30   ` Andy Lutomirski
@ 2015-05-29 12:40   ` David Howells
  2015-05-29 18:38     ` Andy Lutomirski
  2015-06-01 15:50     ` David Howells
  1 sibling, 2 replies; 28+ messages in thread
From: David Howells @ 2015-05-29 12:40 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: dhowells, Luis Rodriguez, Matthew Garrett, keyrings,
	Greg Kroah-Hartman, Kyle McMartin, linux-wireless,
	Linux Kernel Mailing List, Seth Forshee, LSM List, Mimi Zohar,
	David Woodhouse

Andy Lutomirski <luto@kernel.org> wrote:

> This is insecure because PKCS#7 authenticated attributes are broken (see
> RFC2315 section 9.4 note 4).  You need to either require that everything have
> authenticated attributes or require that nothing have authenticated
> attributes.   Maybe this insecurity doesn't matter in practice, but I don't
> wouldn't want to bet on it.

You can also fudge the signature (or a hash) by adding extra data to or
modifying the data blob and by switching signature values between signature
blobs.

PKCS#7 authenticated attributes aren't as broken as you make out.  They are
added to the signature hash - so an attacker *would* have to fudge things to
make it work.  Further, we can easily make it so that auth attrs are
*required*.

> On top of that, this is a ton of code to support something trivial.

I don't think it's as bad as you're making it out to be.

> And it requires an OID to be registered (ick).

That shouldn't be too hard to achieve - at least if we don't mind having RH
space OIDs.

> Earlier you suggested just appending the signature purpose to the thing being
> signed.  What's wrong with that?

You can't tell the difference between a corrupted key/signature and a firmware
blob being loaded for the wrong request.  Firstly, I want to be able to detect
the difference and secondly, it makes it easier to debug it if something does
go wrong.

> P.S.  Or you could stop using PKCS#7 if possible.

We've discussed this before.  We have to have a PKCS#7 parser in the kernel
anyway if we're going to support signed PE files for kexec.

David

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

* Re: [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name [ver #5]
  2015-05-29 12:40   ` David Howells
@ 2015-05-29 18:38     ` Andy Lutomirski
  2015-06-01 15:50     ` David Howells
  1 sibling, 0 replies; 28+ messages in thread
From: Andy Lutomirski @ 2015-05-29 18:38 UTC (permalink / raw)
  To: David Howells
  Cc: Andy Lutomirski, Luis Rodriguez, Matthew Garrett, keyrings,
	Greg Kroah-Hartman, Kyle McMartin, Linux Wireless List,
	Linux Kernel Mailing List, Seth Forshee, LSM List, Mimi Zohar,
	David Woodhouse

On Fri, May 29, 2015 at 5:40 AM, David Howells <dhowells@redhat.com> wrote:
> Andy Lutomirski <luto@kernel.org> wrote:
>
>> This is insecure because PKCS#7 authenticated attributes are broken (see
>> RFC2315 section 9.4 note 4).  You need to either require that everything have
>> authenticated attributes or require that nothing have authenticated
>> attributes.   Maybe this insecurity doesn't matter in practice, but I don't
>> wouldn't want to bet on it.
>
> You can also fudge the signature (or a hash) by adding extra data to or
> modifying the data blob and by switching signature values between signature
> blobs.

So there's another design error in PKCS#7?  Great!

>
> PKCS#7 authenticated attributes aren't as broken as you make out.  They are
> added to the signature hash - so an attacker *would* have to fudge things to
> make it work.  Further, we can easily make it so that auth attrs are
> *required*.
>
>> On top of that, this is a ton of code to support something trivial.
>
> I don't think it's as bad as you're making it out to be.
>
>> And it requires an OID to be registered (ick).
>
> That shouldn't be too hard to achieve - at least if we don't mind having RH
> space OIDs.
>
>> Earlier you suggested just appending the signature purpose to the thing being
>> signed.  What's wrong with that?
>
> You can't tell the difference between a corrupted key/signature and a firmware
> blob being loaded for the wrong request.  Firstly, I want to be able to detect
> the difference and secondly, it makes it easier to debug it if something does
> go wrong.
>

This seems only barely useful.  In any event, you could still do it by
appending the purpose as long as the purpose was literally appended to
the payload as opposed to being implicit and only covered by the
signature.

>> P.S.  Or you could stop using PKCS#7 if possible.
>
> We've discussed this before.  We have to have a PKCS#7 parser in the kernel
> anyway if we're going to support signed PE files for kexec.

If you neuter the PKCS#7 parser to requre authenticated attributes,
will this use case still work?

Honestly, it still might be less code in the end if you used PKCS#7
solely for kexec.

--Andy

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

* Re: [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name [ver #5]
  2015-05-29 12:40   ` David Howells
  2015-05-29 18:38     ` Andy Lutomirski
@ 2015-06-01 15:50     ` David Howells
  2015-06-01 17:06       ` Andy Lutomirski
  1 sibling, 1 reply; 28+ messages in thread
From: David Howells @ 2015-06-01 15:50 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: dhowells, Andy Lutomirski, Luis Rodriguez, Matthew Garrett,
	keyrings, Greg Kroah-Hartman, Kyle McMartin, Linux Wireless List,
	Linux Kernel Mailing List, Seth Forshee, LSM List, Mimi Zohar,
	David Woodhouse

Andy Lutomirski <luto@amacapital.net> wrote:

> > You can also fudge the signature (or a hash) by adding extra data to or
> > modifying the data blob and by switching signature values between signature
> > blobs.
> 
> So there's another design error in PKCS#7?  Great!

No.  This applies to *all* signatures where you're signing a hash.

David

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

* Re: [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name [ver #5]
  2015-06-01 15:50     ` David Howells
@ 2015-06-01 17:06       ` Andy Lutomirski
  0 siblings, 0 replies; 28+ messages in thread
From: Andy Lutomirski @ 2015-06-01 17:06 UTC (permalink / raw)
  To: David Howells
  Cc: Andy Lutomirski, Luis Rodriguez, Matthew Garrett, keyrings,
	Greg Kroah-Hartman, Kyle McMartin, Linux Wireless List,
	Linux Kernel Mailing List, Seth Forshee, LSM List, Mimi Zohar,
	David Woodhouse

On Mon, Jun 1, 2015 at 8:50 AM, David Howells <dhowells@redhat.com> wrote:
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> > You can also fudge the signature (or a hash) by adding extra data to or
>> > modifying the data blob and by switching signature values between signature
>> > blobs.
>>
>> So there's another design error in PKCS#7?  Great!
>
> No.  This applies to *all* signatures where you're signing a hash.

What kind of fudging are you talking about here?  I don't see what
not-intentionally-signed message can be generically fudged to look
like it's signed.

--Andy

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

end of thread, other threads:[~2015-06-01 17:07 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-28 15:46 [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures [ver #5] David Howells
2015-05-28 15:46 ` [PATCH 01/20] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
2015-05-28 15:46 ` [PATCH 02/20] X.509: Support X.509 lookup by Issuer+Serial form " David Howells
2015-05-28 15:46 ` [PATCH 03/20] PKCS#7: Allow detached data to be supplied for signature checking purposes " David Howells
2015-05-28 15:46 ` [PATCH 04/20] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
2015-05-28 15:46 ` [PATCH 05/20] MODSIGN: Use PKCS#7 messages as module signatures " David Howells
2015-05-28 15:47 ` [PATCH 06/20] sign-file: Add option to only create signature file " David Howells
2015-05-28 15:47 ` [PATCH 07/20] system_keyring.c doesn't need to #include module-internal.h " David Howells
2015-05-28 15:47 ` [PATCH 08/20] MODSIGN: Extract the blob PKCS#7 signature verifier from module signing " David Howells
2015-05-28 15:47 ` [PATCH 09/20] modsign: Abort modules_install when signing fails " David Howells
2015-05-28 15:47 ` [PATCH 10/20] modsign: Allow password to be specified for signing key " David Howells
2015-05-28 15:47 ` [PATCH 11/20] modsign: Allow signing key to be PKCS#11 " David Howells
2015-05-28 15:47 ` [PATCH 12/20] modsign: Allow external signing key to be specified " David Howells
2015-05-28 15:48 ` [PATCH 13/20] modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed " David Howells
2015-05-28 15:48 ` [PATCH 14/20] modsign: Use single PEM file for autogenerated key " David Howells
2015-05-28 15:48 ` [PATCH 15/20] modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option " David Howells
2015-05-28 15:48 ` [PATCH 16/20] PKCS#7: Add an optional authenticated attribute to hold firmware name " David Howells
2015-05-29  1:30   ` Andy Lutomirski
2015-05-29 12:40   ` David Howells
2015-05-29 18:38     ` Andy Lutomirski
2015-06-01 15:50     ` David Howells
2015-06-01 17:06       ` Andy Lutomirski
2015-05-28 15:48 ` [PATCH 17/20] X.509: Restrict the usage of a key based on information in X.509 certificate " David Howells
2015-05-28 15:48 ` [PATCH 18/20] X.509: Parse the keyUsage extension to detect key-signing keys " David Howells
2015-05-28 15:49 ` [PATCH 19/20] KEYS: Restrict signature verification to keys appropriate to the purpose " David Howells
2015-05-28 15:49 ` [PATCH 20/20] sign-file: use .p7s instead of .pkcs7 file extension " David Howells
2015-05-28 15:52 ` [PATCH 00/20] MODSIGN: Use PKCS#7 for module signatures " David Woodhouse
2015-05-28 15:57 ` David Howells

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.