LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2]
@ 2014-11-26 14:17 David Howells
  2014-11-26 14:17 ` [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: David Howells @ 2014-11-26 14:17 UTC (permalink / raw)
  To: rusty
  Cc: mmarek, keyrings, d.kasatkin, linux-kernel, dhowells,
	linux-security-module, zohar


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

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

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

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

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

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

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

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

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

They can be found here also:

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

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

Changes:

 (*) Fixed a comment on x509_certificate::id to show the order of construction
     correctly [thanks to Vivek Goyal].

 (*) The 'id' argument to x509_request_asymmetric_key() can be NULL so needs to
     be checked [thanks to Mimi Zohar].

 (*) pkcs7_supply_detached_data() doesn't need exporting [thanks to Mimi
     Zohar].

 (*) Fixed "make install_modules" to not try to run sign-file under perl
     [thanks to Dmitry Kasatkin].

 (*) Fixed sign-file to handle binary X.509 certs as well as PEM-encoded X.509
     certs.

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


 Makefile                                  |    2 
 crypto/asymmetric_keys/Makefile           |    8 -
 crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
 crypto/asymmetric_keys/pkcs7_verify.c     |   80 ++++--
 crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
 crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
 crypto/asymmetric_keys/x509_parser.h      |    5 
 crypto/asymmetric_keys/x509_public_key.c  |   86 ++++--
 include/crypto/pkcs7.h                    |    3 
 include/crypto/public_key.h               |    4 
 init/Kconfig                              |    1 
 kernel/module_signing.c                   |  220 +++------------
 scripts/Makefile                          |    2 
 scripts/sign-file                         |  421 -----------------------------
 scripts/sign-file.c                       |  206 ++++++++++++++
 15 files changed, 524 insertions(+), 701 deletions(-)
 create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
 delete mode 100755 scripts/sign-file
 create mode 100755 scripts/sign-file.c


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

* [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier [ver #2]
  2014-11-26 14:17 [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2] David Howells
@ 2014-11-26 14:17 ` David Howells
  2014-12-04 12:14   ` Dmitry Kasatkin
  2014-12-04 12:51   ` David Howells
  2014-11-26 14:17 ` [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form " David Howells
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: David Howells @ 2014-11-26 14:17 UTC (permalink / raw)
  To: rusty
  Cc: mmarek, keyrings, d.kasatkin, linux-kernel, dhowells,
	linux-security-module, zohar

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

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

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


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

* [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier [ver #2]
  2014-11-26 14:17 [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2] David Howells
  2014-11-26 14:17 ` [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
@ 2014-11-26 14:17 ` David Howells
  2014-11-26 14:17 ` [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes " David Howells
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: David Howells @ 2014-11-26 14:17 UTC (permalink / raw)
  To: rusty
  Cc: mmarek, keyrings, d.kasatkin, linux-kernel, dhowells,
	linux-security-module, zohar

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

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

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

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

 crypto/asymmetric_keys/pkcs7_trust.c     |   10 +++-
 crypto/asymmetric_keys/pkcs7_verify.c    |   47 +++++++++++++----
 crypto/asymmetric_keys/x509_public_key.c |   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 f802cf118053..11f8ca752e2a 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -54,7 +54,8 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 		/* Look to see if this certificate is present in the trusted
 		 * keys.
 		 */
-		key = x509_request_asymmetric_key(trust_keyring, x509->id,
+		key = x509_request_asymmetric_key(trust_keyring,
+						  x509->id, x509->skid,
 						  false);
 		if (!IS_ERR(key)) {
 			/* One of the X.509 certificates in the PKCS#7 message
@@ -85,8 +86,10 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 	/* No match - see if the root certificate has a signer amongst the
 	 * trusted keys.
 	 */
-	if (last && last->auth_skid) {
-		key = x509_request_asymmetric_key(trust_keyring, last->auth_skid,
+	if (last && (last->auth_id || last->auth_skid)) {
+		key = x509_request_asymmetric_key(trust_keyring,
+						  last->auth_id,
+						  last->auth_skid,
 						  false);
 		if (!IS_ERR(key)) {
 			x509 = last;
@@ -103,6 +106,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 	 */
 	key = x509_request_asymmetric_key(trust_keyring,
 					  sinfo->signing_cert_id,
+					  NULL,
 					  false);
 	if (!IS_ERR(key)) {
 		pr_devel("sinfo %u: Direct signer is key %x\n",
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 5e956c5b9071..667064daf66b 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -170,6 +170,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 				  struct pkcs7_signed_info *sinfo)
 {
 	struct x509_certificate *x509 = sinfo->signer, *p;
+	struct asymmetric_key_id *auth;
 	int ret;
 
 	kenter("");
@@ -187,11 +188,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 			goto maybe_missing_crypto_in_x509;
 
 		pr_debug("- issuer %s\n", x509->issuer);
+		if (x509->auth_id)
+			pr_debug("- authkeyid.id %*phN\n",
+				 x509->auth_id->len, x509->auth_id->data);
 		if (x509->auth_skid)
-			pr_debug("- authkeyid %*phN\n",
+			pr_debug("- authkeyid.skid %*phN\n",
 				 x509->auth_skid->len, x509->auth_skid->data);
 
-		if (!x509->auth_skid ||
+		if ((!x509->auth_id && !x509->auth_skid) ||
 		    strcmp(x509->subject, x509->issuer) == 0) {
 			/* If there's no authority certificate specified, then
 			 * the certificate must be self-signed and is the root
@@ -215,21 +219,42 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
 		/* Look through the X.509 certificates in the PKCS#7 message's
 		 * list to see if the next one is there.
 		 */
-		pr_debug("- want %*phN\n",
-			 x509->auth_skid->len, x509->auth_skid->data);
-		for (p = pkcs7->certs; p; p = p->next) {
-			if (!p->skid)
-				continue;
-			pr_debug("- cmp [%u] %*phN\n",
-				 p->index, p->skid->len, p->skid->data);
-			if (asymmetric_key_id_same(p->skid, x509->auth_skid))
-				goto found_issuer;
+		auth = x509->auth_id;
+		if (auth) {
+			pr_debug("- want %*phN\n", auth->len, auth->data);
+			for (p = pkcs7->certs; p; p = p->next) {
+				pr_debug("- cmp [%u] %*phN\n",
+					 p->index, p->id->len, p->id->data);
+				if (asymmetric_key_id_same(p->id, auth))
+					goto found_issuer_check_skid;
+			}
+		} else {
+			auth = x509->auth_skid;
+			pr_debug("- want %*phN\n", auth->len, auth->data);
+			for (p = pkcs7->certs; p; p = p->next) {
+				if (!p->skid)
+					continue;
+				pr_debug("- cmp [%u] %*phN\n",
+					 p->index, p->skid->len, p->skid->data);
+				if (asymmetric_key_id_same(p->skid, auth))
+					goto found_issuer;
+			}
 		}
 
 		/* We didn't find the root of this chain */
 		pr_debug("- top\n");
 		return 0;
 
+	found_issuer_check_skid:
+		/* We matched issuer + serialNumber, but if there's an
+		 * authKeyId.keyId, that must match the CA subjKeyId also.
+		 */
+		if (x509->auth_skid &&
+		    !asymmetric_key_id_same(p->skid, x509->auth_skid)) {
+			pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
+				sinfo->index, x509->index, p->index);
+			return -EKEYREJECTED;
+		}
 	found_issuer:
 		pr_debug("- subject %s\n", p->subject);
 		if (p->seen) {
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index a3d9ba999da5..840cbf21fe45 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->auth_skid, ca_keyid))
 		return -EPERM;
 
-	key = x509_request_asymmetric_key(trust_keyring, cert->auth_skid,
+	key = x509_request_asymmetric_key(trust_keyring,
+					  cert->auth_id, cert->auth_skid,
 					  false);
 	if (!IS_ERR(key))  {
 		if (!use_builtin_keys
@@ -274,8 +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->auth_skid ||
-	    asymmetric_key_id_same(cert->skid, cert->auth_skid)) {
+	if ((!cert->auth_skid && !cert->auth_id) ||
+	    asymmetric_key_id_same(cert->skid, cert->auth_skid) ||
+	    asymmetric_key_id_same(cert->id, cert->auth_id)) {
 		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 54add2069901..b6f27a240856 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -101,7 +101,8 @@ extern int verify_signature(const struct key *key,
 
 struct asymmetric_key_id;
 extern struct key *x509_request_asymmetric_key(struct key *keyring,
-					       const struct asymmetric_key_id *kid,
+					       const struct asymmetric_key_id *id,
+					       const struct asymmetric_key_id *skid,
 					       bool partial);
 
 #endif /* _LINUX_PUBLIC_KEY_H */


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

* [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes [ver #2]
  2014-11-26 14:17 [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2] David Howells
  2014-11-26 14:17 ` [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
  2014-11-26 14:17 ` [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form " David Howells
@ 2014-11-26 14:17 ` David Howells
  2014-11-26 14:17 ` [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: David Howells @ 2014-11-26 14:17 UTC (permalink / raw)
  To: rusty
  Cc: mmarek, keyrings, d.kasatkin, linux-kernel, dhowells,
	linux-security-module, zohar

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

 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 667064daf66b..ee506bdfea54 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	[flat|nested] 16+ messages in thread

* [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-11-26 14:17 [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2] David Howells
                   ` (2 preceding siblings ...)
  2014-11-26 14:17 ` [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes " David Howells
@ 2014-11-26 14:17 ` David Howells
  2014-12-05  8:54   ` Dmitry Kasatkin
                     ` (2 more replies)
  2014-11-26 14:18 ` [PATCH 5/5] MODSIGN: Use PKCS#7 messages as module signatures " David Howells
  2014-12-04 18:56 ` [PATCH 0/5] MODSIGN: Use PKCS#7 for " Vivek Goyal
  5 siblings, 3 replies; 16+ messages in thread
From: David Howells @ 2014-11-26 14:17 UTC (permalink / raw)
  To: rusty
  Cc: mmarek, keyrings, d.kasatkin, linux-kernel, dhowells,
	linux-security-module, zohar

Provide a utility that:

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

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

 (2) Generates a PKCS#7 message that:

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

     (b) Is signed with the specified private key.

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

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

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

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

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

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

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

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

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

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

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


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

* [PATCH 5/5] MODSIGN: Use PKCS#7 messages as module signatures [ver #2]
  2014-11-26 14:17 [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2] David Howells
                   ` (3 preceding siblings ...)
  2014-11-26 14:17 ` [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
@ 2014-11-26 14:18 ` David Howells
  2014-12-04 18:56 ` [PATCH 0/5] MODSIGN: Use PKCS#7 for " Vivek Goyal
  5 siblings, 0 replies; 16+ messages in thread
From: David Howells @ 2014-11-26 14:18 UTC (permalink / raw)
  To: rusty
  Cc: mmarek, keyrings, d.kasatkin, linux-kernel, dhowells,
	linux-security-module, zohar

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

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

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


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

* Re: [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier [ver #2]
  2014-11-26 14:17 ` [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
@ 2014-12-04 12:14   ` Dmitry Kasatkin
  2014-12-04 12:51   ` David Howells
  1 sibling, 0 replies; 16+ messages in thread
From: Dmitry Kasatkin @ 2014-12-04 12:14 UTC (permalink / raw)
  To: David Howells, rusty
  Cc: mmarek, keyrings, linux-kernel, linux-security-module, zohar

On 26/11/14 16:17, David Howells wrote:
> Extract both parts of the AuthorityKeyIdentifier, not just the keyIdentifier,
> as the second part can be used to match X.509 certificates by issuer and
> serialNumber.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
>  crypto/asymmetric_keys/Makefile           |    8 +-
>  crypto/asymmetric_keys/pkcs7_trust.c      |    4 -
>  crypto/asymmetric_keys/pkcs7_verify.c     |   12 +-
>  crypto/asymmetric_keys/x509_akid.asn1     |   35 +++++++
>  crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++++++++++++++-----------
>  crypto/asymmetric_keys/x509_parser.h      |    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..f802cf118053 100644
> --- a/crypto/asymmetric_keys/pkcs7_trust.c
> +++ b/crypto/asymmetric_keys/pkcs7_trust.c
> @@ -85,8 +85,8 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
>  	/* No match - see if the root certificate has a signer amongst the
>  	 * trusted keys.
>  	 */
> -	if (last && last->authority) {
> -		key = x509_request_asymmetric_key(trust_keyring, last->authority,
> +	if (last && last->auth_skid) {
> +		key = x509_request_asymmetric_key(trust_keyring, last->auth_skid,
>  						  false);
>  		if (!IS_ERR(key)) {
>  			x509 = last;
> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> index cd455450b069..5e956c5b9071 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -187,11 +187,11 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>  			goto maybe_missing_crypto_in_x509;
>  
>  		pr_debug("- issuer %s\n", x509->issuer);
> -		if (x509->authority)
> +		if (x509->auth_skid)
>  			pr_debug("- authkeyid %*phN\n",
> -				 x509->authority->len, x509->authority->data);
> +				 x509->auth_skid->len, x509->auth_skid->data);
>  
> -		if (!x509->authority ||
> +		if (!x509->auth_skid ||
>  		    strcmp(x509->subject, x509->issuer) == 0) {
>  			/* If there's no authority certificate specified, then
>  			 * the certificate must be self-signed and is the root
> @@ -216,13 +216,13 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>  		 * list to see if the next one is there.
>  		 */
>  		pr_debug("- want %*phN\n",
> -			 x509->authority->len, x509->authority->data);
> +			 x509->auth_skid->len, x509->auth_skid->data);
>  		for (p = pkcs7->certs; p; p = p->next) {
>  			if (!p->skid)
>  				continue;
>  			pr_debug("- cmp [%u] %*phN\n",
>  				 p->index, p->skid->len, p->skid->data);
> -			if (asymmetric_key_id_same(p->skid, x509->authority))
> +			if (asymmetric_key_id_same(p->skid, x509->auth_skid))
>  				goto found_issuer;
>  		}
>  
> @@ -338,8 +338,6 @@ int pkcs7_verify(struct pkcs7_message *pkcs7)
>  		ret = x509_get_sig_params(x509);
>  		if (ret < 0)
>  			return ret;
> -		pr_debug("X.509[%u] %*phN\n",
> -			 n, x509->authority->len, x509->authority->data);
>  	}
>  
>  	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> diff --git a/crypto/asymmetric_keys/x509_akid.asn1 b/crypto/asymmetric_keys/x509_akid.asn1
> new file mode 100644
> index 000000000000..1a33231a75a8
> --- /dev/null
> +++ b/crypto/asymmetric_keys/x509_akid.asn1
> @@ -0,0 +1,35 @@
> +-- X.509 AuthorityKeyIdentifier
> +-- rfc5280 section 4.2.1.1
> +
> +AuthorityKeyIdentifier ::= SEQUENCE {
> +	keyIdentifier			[0] IMPLICIT KeyIdentifier		OPTIONAL,
> +	authorityCertIssuer		[1] IMPLICIT GeneralNames		OPTIONAL,
> +	authorityCertSerialNumber	[2] IMPLICIT CertificateSerialNumber	OPTIONAL
> +	}
> +
> +KeyIdentifier ::= OCTET STRING ({ x509_akid_note_kid })
> +
> +CertificateSerialNumber ::= INTEGER ({ x509_akid_note_serial })
> +
> +GeneralNames ::= SEQUENCE OF GeneralName
> +
> +GeneralName ::= CHOICE {
> +	otherName			[0] ANY,
> +	rfc822Name			[1] IA5String,
> +	dNSName				[2] IA5String,
> +	x400Address			[3] ANY,
> +	directoryName			[4] Name ({ x509_akid_note_name }),
> +	ediPartyName			[5] ANY,
> +	uniformResourceIdentifier	[6] IA5String,
> +	iPAddress			[7] OCTET STRING,
> +	registeredID			[8] OBJECT IDENTIFIER
> +	}
> +
> +Name ::= SEQUENCE OF RelativeDistinguishedName
> +
> +RelativeDistinguishedName ::= SET OF AttributeValueAssertion
> +
> +AttributeValueAssertion ::= SEQUENCE {
> +	attributeType		OBJECT IDENTIFIER ({ x509_note_OID }),
> +	attributeValue		ANY ({ x509_extract_name_segment })
> +	}
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index a668d90302d3..e9d6586fdf89 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -18,6 +18,7 @@
>  #include "public_key.h"
>  #include "x509_parser.h"
>  #include "x509-asn1.h"
> +#include "x509_akid-asn1.h"
>  #include "x509_rsakey-asn1.h"
>  
>  struct x509_parse_context {
> @@ -35,6 +36,10 @@ struct x509_parse_context {
>  	u16		o_offset;		/* Offset of organizationName (O) */
>  	u16		cn_offset;		/* Offset of commonName (CN) */
>  	u16		email_offset;		/* Offset of emailAddress */
> +	unsigned	raw_akid_size;
> +	const void	*raw_akid;		/* Raw authorityKeyId in ASN.1 */
> +	const void	*akid_raw_issuer;	/* Raw directoryName in authorityKeyId */
> +	unsigned	akid_raw_issuer_size;
>  };
>  
>  /*
> @@ -48,7 +53,8 @@ void x509_free_certificate(struct x509_certificate *cert)
>  		kfree(cert->subject);
>  		kfree(cert->id);
>  		kfree(cert->skid);
> -		kfree(cert->authority);
> +		kfree(cert->auth_id);
> +		kfree(cert->auth_skid);
>  		kfree(cert->sig.digest);
>  		mpi_free(cert->sig.rsa.s);
>  		kfree(cert);
> @@ -85,6 +91,18 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>  	if (ret < 0)
>  		goto error_decode;
>  
> +	/* Decode the AuthorityKeyIdentifier */
> +	if (ctx->raw_akid) {
> +		pr_devel("AKID: %u %*phN\n",
> +			 ctx->raw_akid_size, ctx->raw_akid_size, ctx->raw_akid);
> +		ret = asn1_ber_decoder(&x509_akid_decoder, ctx,
> +				       ctx->raw_akid, ctx->raw_akid_size);
> +		if (ret < 0) {
> +			pr_warn("Couldn't decode AuthKeyIdentifier\n");
> +			goto error_decode;
> +		}
> +	}
> +
>  	/* Decode the public key */
>  	ret = asn1_ber_decoder(&x509_rsakey_decoder, ctx,
>  			       ctx->key, ctx->key_size);
> @@ -422,7 +440,6 @@ int x509_process_extension(void *context, size_t hdrlen,
>  	struct x509_parse_context *ctx = context;
>  	struct asymmetric_key_id *kid;
>  	const unsigned char *v = value;
> -	int i;
>  
>  	pr_debug("Extension: %u\n", ctx->last_oid);
>  
> @@ -449,57 +466,8 @@ int x509_process_extension(void *context, size_t hdrlen,
>  
>  	if (ctx->last_oid == OID_authorityKeyIdentifier) {
>  		/* Get hold of the CA key fingerprint */
> -		if (ctx->cert->authority || vlen < 5)
> -			return -EBADMSG;
> -
> -		/* Authority Key Identifier must be a Constructed SEQUENCE */
> -		if (v[0] != (ASN1_SEQ | (ASN1_CONS << 5)))
> -			return -EBADMSG;
> -
> -		/* Authority Key Identifier is not indefinite length */
> -		if (unlikely(vlen == ASN1_INDEFINITE_LENGTH))
> -			return -EBADMSG;
> -
> -		if (vlen < ASN1_INDEFINITE_LENGTH) {
> -			/* Short Form length */
> -			if (v[1] != vlen - 2 ||
> -			    v[2] != SEQ_TAG_KEYID ||
> -			    v[3] > vlen - 4)
> -				return -EBADMSG;
> -
> -			vlen = v[3];
> -			v += 4;
> -		} else {
> -			/* Long Form length */
> -			size_t seq_len = 0;
> -			size_t sub = v[1] - ASN1_INDEFINITE_LENGTH;
> -
> -			if (sub > 2)
> -				return -EBADMSG;
> -
> -			/* calculate the length from subsequent octets */
> -			v += 2;
> -			for (i = 0; i < sub; i++) {
> -				seq_len <<= 8;
> -				seq_len |= v[i];
> -			}
> -
> -			if (seq_len != vlen - 2 - sub ||
> -			    v[sub] != SEQ_TAG_KEYID ||
> -			    v[sub + 1] > vlen - 4 - sub)
> -				return -EBADMSG;
> -
> -			vlen = v[sub + 1];
> -			v += (sub + 2);
> -		}
> -
> -		kid = asymmetric_key_generate_id(ctx->cert->raw_issuer,
> -						 ctx->cert->raw_issuer_size,
> -						 v, vlen);
> -		if (IS_ERR(kid))
> -			return PTR_ERR(kid);
> -		pr_debug("authkeyid %*phN\n", kid->len, kid->data);
> -		ctx->cert->authority = kid;
> +		ctx->raw_akid = v;
> +		ctx->raw_akid_size = vlen;
>  		return 0;
>  	}
>  
> @@ -569,3 +537,71 @@ int x509_note_not_after(void *context, size_t hdrlen,
>  	struct x509_parse_context *ctx = context;
>  	return x509_note_time(&ctx->cert->valid_to, hdrlen, tag, value, vlen);
>  }
> +
> +/*
> + * Note a key identifier-based AuthorityKeyIdentifier
> + */
> +int x509_akid_note_kid(void *context, size_t hdrlen,
> +		       unsigned char tag,
> +		       const void *value, size_t vlen)
> +{
> +	struct x509_parse_context *ctx = context;
> +	struct asymmetric_key_id *kid;
> +
> +	pr_debug("AKID: keyid: %*phN\n", (int)vlen, value);
> +
> +	if (ctx->cert->auth_skid)
> +		return 0;
> +
> +	kid = asymmetric_key_generate_id(ctx->cert->raw_issuer,
> +					 ctx->cert->raw_issuer_size,
> +					 value, vlen);
> +	if (IS_ERR(kid))
> +		return PTR_ERR(kid);
> +	pr_debug("authkeyid %*phN\n", kid->len, kid->data);
> +	ctx->cert->auth_skid = kid;
> +	return 0;
> +}
> +
> +/*
> + * Note a directoryName in an AuthorityKeyIdentifier
> + */
> +int x509_akid_note_name(void *context, size_t hdrlen,
> +			unsigned char tag,
> +			const void *value, size_t vlen)
> +{
> +	struct x509_parse_context *ctx = context;
> +
> +	pr_debug("AKID: name: %*phN\n", (int)vlen, value);
> +
> +	ctx->akid_raw_issuer = value;
> +	ctx->akid_raw_issuer_size = vlen;
> +	return 0;
> +}
> +
> +/*
> + * Note a serial number in an AuthorityKeyIdentifier
> + */
> +int x509_akid_note_serial(void *context, size_t hdrlen,
> +			  unsigned char tag,
> +			  const void *value, size_t vlen)
> +{
> +	struct x509_parse_context *ctx = context;
> +	struct asymmetric_key_id *kid;
> +
> +	pr_debug("AKID: serial: %*phN\n", (int)vlen, value);
> +
> +	if (!ctx->akid_raw_issuer || ctx->cert->auth_id)
> +		return 0;
> +
> +	kid = asymmetric_key_generate_id(value,
> +					 vlen,
> +					 ctx->akid_raw_issuer,
> +					 ctx->akid_raw_issuer_size);
> +	if (IS_ERR(kid))
> +		return PTR_ERR(kid);
> +
> +	pr_debug("authkeyid %*phN\n", kid->len, kid->data);
> +	ctx->cert->auth_id = kid;
> +	return 0;
> +}
> diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
> index 3dfe6b5d6f0b..c4d16ddbc2cb 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 *auth_id;	/* CA AuthKeyId matching ->id (optional) */
> +	struct asymmetric_key_id *auth_skid;	/* CA AuthKeyId matching ->skid (optional) */

Hi David,

Why do you call it "auth_skid", not just akid in similar way as 'skid'?
Why it is "auth & skid"?

- Dmitry

>  	struct tm	valid_from;
>  	struct tm	valid_to;
>  	const void	*tbs;			/* Signed data */
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index a6c42031628e..a3d9ba999da5 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -214,10 +214,10 @@ static int x509_validate_trust(struct x509_certificate *cert,
>  	if (!trust_keyring)
>  		return -EOPNOTSUPP;
>  
> -	if (ca_keyid && !asymmetric_key_id_partial(cert->authority, ca_keyid))
> +	if (ca_keyid && !asymmetric_key_id_partial(cert->auth_skid, ca_keyid))
>  		return -EPERM;
>  
> -	key = x509_request_asymmetric_key(trust_keyring, cert->authority,
> +	key = x509_request_asymmetric_key(trust_keyring, cert->auth_skid,
>  					  false);
>  	if (!IS_ERR(key))  {
>  		if (!use_builtin_keys
> @@ -274,8 +274,8 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>  	cert->pub->id_type = PKEY_ID_X509;
>  
>  	/* Check the signature on the key if it appears to be self-signed */
> -	if (!cert->authority ||
> -	    asymmetric_key_id_same(cert->skid, cert->authority)) {
> +	if (!cert->auth_skid ||
> +	    asymmetric_key_id_same(cert->skid, cert->auth_skid)) {
>  		ret = x509_check_signature(cert->pub, cert); /* self-signed */
>  		if (ret < 0)
>  			goto error_free_cert;
>
>


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

* Re: [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier [ver #2]
  2014-11-26 14:17 ` [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
  2014-12-04 12:14   ` Dmitry Kasatkin
@ 2014-12-04 12:51   ` David Howells
  1 sibling, 0 replies; 16+ messages in thread
From: David Howells @ 2014-12-04 12:51 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: dhowells, rusty, mmarek, keyrings, linux-kernel,
	linux-security-module, zohar

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

> > +	struct asymmetric_key_id *auth_id;	/* CA AuthKeyId matching ->id (optional) */
> > +	struct asymmetric_key_id *auth_skid;	/* CA AuthKeyId matching ->skid (optional) */
> 
> Hi David,
> 
> Why do you call it "auth_skid", not just akid in similar way as 'skid'?
> Why it is "auth & skid"?

Because both auth_skid and auth_id derive from the akid.

David

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

* Re: [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2]
  2014-11-26 14:17 [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2] David Howells
                   ` (4 preceding siblings ...)
  2014-11-26 14:18 ` [PATCH 5/5] MODSIGN: Use PKCS#7 messages as module signatures " David Howells
@ 2014-12-04 18:56 ` Vivek Goyal
  5 siblings, 0 replies; 16+ messages in thread
From: Vivek Goyal @ 2014-12-04 18:56 UTC (permalink / raw)
  To: David Howells
  Cc: rusty, mmarek, keyrings, d.kasatkin, linux-kernel,
	linux-security-module, zohar

On Wed, Nov 26, 2014 at 02:17:09PM +0000, David Howells wrote:
> 
> Here's a set of patches that does the following:
> 

I compiled the kernel with these patches and booted into this kernel
without any issues.

FWIW,

Tested-by: Vivek Goyal <vgoyal@redhat.com>

Thanks
Vivek

>  (1) Extracts both parts of an X.509 AuthorityKeyIdentifier (AKID) extension.
>      We already extract the bit that can match the subjectKeyIdentifier (SKID)
>      of the parent X.509 cert, but we currently ignore the bits that can match
>      the issuer and serialNumber.
> 
>      Looks up an X.509 cert by issuer and serialNumber if those are provided in
>      the AKID.  If the keyIdentifier is also provided, checks that the
>      subjectKeyIdentifier of the cert found matches that also.
> 
>      If no issuer and serialNumber are provided in the AKID, looks up an X.509
>      cert by SKID using the AKID keyIdentifier.
> 
>      This allows module signing to be done with certificates that don't have an
>      SKID by which they can be looked up.
> 
>  (2) Makes use of the PKCS#7 facility to provide module signatures.
> 
>      sign-file is replaced with a program that generates a PKCS#7 message that
>      has no X.509 certs embedded and that has detached data (the module
>      content) and adds it onto the message with magic string and descriptor.
> 
>  (3) The PKCS#7 message (and matching X.509 cert) supply all the information
>      that is needed to select the X.509 cert to be used to verify the signature
>      by standard means (including selection of digest algorithm and public key
>      algorithm).  No kernel-specific magic values are required.
> 
> Note that the revised sign-file program no longer supports the "-s <signature>"
> option as I'm not sure what the best way to deal with this is.  Do we generate
> a PKCS#7 cert from the signature given, or do we get given a PKCS#7 cert?  I
> lean towards the latter.
> 
> They can be found here also:
> 
> 	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7
> 
> These patches are based on the security tree's next branch.
> 
> Changes:
> 
>  (*) Fixed a comment on x509_certificate::id to show the order of construction
>      correctly [thanks to Vivek Goyal].
> 
>  (*) The 'id' argument to x509_request_asymmetric_key() can be NULL so needs to
>      be checked [thanks to Mimi Zohar].
> 
>  (*) pkcs7_supply_detached_data() doesn't need exporting [thanks to Mimi
>      Zohar].
> 
>  (*) Fixed "make install_modules" to not try to run sign-file under perl
>      [thanks to Dmitry Kasatkin].
> 
>  (*) Fixed sign-file to handle binary X.509 certs as well as PEM-encoded X.509
>      certs.
> 
> David
> ---
> David Howells (5):
>       X.509: Extract both parts of the AuthorityKeyIdentifier
>       X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
>       PKCS#7: Allow detached data to be supplied for signature checking purposes
>       MODSIGN: Provide a utility to append a PKCS#7 signature to a module
>       MODSIGN: Use PKCS#7 messages as module signatures
> 
> 
>  Makefile                                  |    2 
>  crypto/asymmetric_keys/Makefile           |    8 -
>  crypto/asymmetric_keys/pkcs7_trust.c      |   10 -
>  crypto/asymmetric_keys/pkcs7_verify.c     |   80 ++++--
>  crypto/asymmetric_keys/x509_akid.asn1     |   35 ++
>  crypto/asymmetric_keys/x509_cert_parser.c |  142 ++++++----
>  crypto/asymmetric_keys/x509_parser.h      |    5 
>  crypto/asymmetric_keys/x509_public_key.c  |   86 ++++--
>  include/crypto/pkcs7.h                    |    3 
>  include/crypto/public_key.h               |    4 
>  init/Kconfig                              |    1 
>  kernel/module_signing.c                   |  220 +++------------
>  scripts/Makefile                          |    2 
>  scripts/sign-file                         |  421 -----------------------------
>  scripts/sign-file.c                       |  206 ++++++++++++++
>  15 files changed, 524 insertions(+), 701 deletions(-)
>  create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
>  delete mode 100755 scripts/sign-file
>  create mode 100755 scripts/sign-file.c
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-11-26 14:17 ` [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
@ 2014-12-05  8:54   ` Dmitry Kasatkin
  2014-12-05 10:23   ` David Howells
  2014-12-11 16:41   ` David Howells
  2 siblings, 0 replies; 16+ messages in thread
From: Dmitry Kasatkin @ 2014-12-05  8:54 UTC (permalink / raw)
  To: David Howells, rusty
  Cc: mmarek, keyrings, linux-kernel, linux-security-module, zohar

Hi David,

sign-file.c produce lots of annoying noise.

scripts/sign-file.c:153:2: warning: format not a string literal and no
format arguments [-Wformat-security]
  ERR(!bd, dest_name);
  ^
scripts/sign-file.c:179:3: warning: format not a string literal and no
format arguments [-Wformat-security]
   ERR(!b, pkcs7_name);
   ^
scripts/sign-file.c:180:3: warning: format not a string literal and no
format arguments [-Wformat-security]
   ERR(i2d_PKCS7_bio_stream(b, pkcs7, NULL, 0) < 0, pkcs7_name);
   ^
scripts/sign-file.c:185:2: warning: format not a string literal and no
format arguments [-Wformat-security]
  ERR(BIO_reset(bm) < 0, module_name);
  ^


Would be great to fix it.

Thanks,
Dmitry


On 26/11/14 16:17, David Howells wrote:
> Provide a utility that:
>
>  (1) Digests a module using the specified hash algorithm (typically sha256).
>
>      [The digest can be dumped into a file by passing the '-d' flag]
>
>  (2) Generates a PKCS#7 message that:
>
>      (a) Has detached data (ie. the module content).
>
>      (b) Is signed with the specified private key.
>
>      (c) Refers to the specified X.509 certificate.
>
>      (d) Has an empty X.509 certificate list.
>
>      [The PKCS#7 message can be dumped into a file by passing the '-p' flag]
>
>  (3) Generates a signed module by concatenating the old module, the PKCS#7
>      message, a descriptor and a magic string.  The descriptor contains the
>      size of the PKCS#7 message and indicates the id_type as PKEY_ID_PKCS7.
>
>  (4) Either writes the signed module to the specified destination or renames
>      it over the source module.
>
> This allows module signing to reuse the PKCS#7 handling code that was added
> for PE file parsing for signed kexec.
>
> Note that the utility is written in C and must be linked against the OpenSSL
> crypto library.
>
> Note further that I have temporarily dropped support for handling externally
> created signatures until we can work out the best way to do those.  Hopefully,
> whoever creates the signature can give me a PKCS#7 certificate.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
>  include/crypto/public_key.h |    1 
>  scripts/sign-file.c         |  189 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 190 insertions(+)
>  create mode 100755 scripts/sign-file.c
>
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index b6f27a240856..fda097e079a4 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -33,6 +33,7 @@ extern const struct public_key_algorithm *pkey_algo[PKEY_ALGO__LAST];
>  enum pkey_id_type {
>  	PKEY_ID_PGP,		/* OpenPGP generated key ID */
>  	PKEY_ID_X509,		/* X.509 arbitrary subjectKeyIdentifier */
> +	PKEY_ID_PKCS7,		/* Signature in PKCS#7 message */
>  	PKEY_ID_TYPE__LAST
>  };
>  
> diff --git a/scripts/sign-file.c b/scripts/sign-file.c
> new file mode 100755
> index 000000000000..3f9bedbd185f
> --- /dev/null
> +++ b/scripts/sign-file.c
> @@ -0,0 +1,189 @@
> +/* Sign a module file using the given key.
> + *
> + * Copyright (C) 2014 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <getopt.h>
> +#include <err.h>
> +#include <arpa/inet.h>
> +#include <openssl/bio.h>
> +#include <openssl/evp.h>
> +#include <openssl/pem.h>
> +#include <openssl/pkcs7.h>
> +#include <openssl/err.h>
> +
> +struct module_signature {
> +	uint8_t		algo;		/* Public-key crypto algorithm [0] */
> +	uint8_t		hash;		/* Digest algorithm [0] */
> +	uint8_t		id_type;	/* Key identifier type [PKEY_ID_PKCS7] */
> +	uint8_t		signer_len;	/* Length of signer's name [0] */
> +	uint8_t		key_id_len;	/* Length of key identifier [0] */
> +	uint8_t		__pad[3];
> +	uint32_t	sig_len;	/* Length of signature data */
> +};
> +
> +#define PKEY_ID_PKCS7 2
> +
> +static char magic_number[] = "~Module signature appended~\n";
> +
> +static __attribute__((noreturn))
> +void format(void)
> +{
> +	fprintf(stderr,
> +		"Usage: scripts/sign-file [-dp] <hash algo> <key> <x509> <module> [<dest>]\n");
> +	exit(2);
> +}
> +
> +static void display_openssl_errors(int l)
> +{
> +	const char *file;
> +	char buf[120];
> +	int e, line;
> +
> +	if (ERR_peek_error() == 0)
> +		return;
> +	fprintf(stderr, "At main.c:%d:\n", l);
> +
> +	while ((e = ERR_get_error_line(&file, &line))) {
> +		ERR_error_string(e, buf);
> +		fprintf(stderr, "- SSL %s: %s:%d\n", buf, file, line);
> +	}
> +}
> +
> +
> +#define ERR(cond, ...)				  \
> +	do {					  \
> +		bool __cond = (cond);		  \
> +		display_openssl_errors(__LINE__); \
> +		if (__cond) {			  \
> +			err(1, ## __VA_ARGS__);	  \
> +		}				  \
> +	} while(0)
> +
> +int main(int argc, char **argv)
> +{
> +	struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
> +	char *hash_algo = NULL;
> +	char *private_key_name, *x509_name, *module_name, *dest_name;
> +	bool save_pkcs7 = false, replace_orig;
> +	unsigned char buf[4096];
> +	unsigned long module_size, pkcs7_size;
> +	const EVP_MD *digest_algo;
> +	EVP_PKEY *private_key;
> +	PKCS7 *pkcs7;
> +	X509 *x509;
> +	BIO *b, *bd, *bm;
> +	int opt, n;
> +
> +	do {
> +		opt = getopt(argc, argv, "dp");
> +		switch (opt) {
> +		case 'p': save_pkcs7 = true; break;
> +		case -1: break;
> +		default: format();
> +		}
> +	} while (opt != -1);
> +
> +	argc -= optind;
> +	argv += optind;
> +	if (argc < 4 || argc > 5)
> +		format();
> +
> +	hash_algo = argv[0];
> +	private_key_name = argv[1];
> +	x509_name = argv[2];
> +	module_name = argv[3];
> +	if (argc == 5) {
> +		dest_name = argv[4];
> +		replace_orig = false;
> +	} else {
> +		ERR(asprintf(&dest_name, "%s.~signed~", module_name) < 0,
> +		    "asprintf");
> +		replace_orig = true;
> +	}
> +
> +	ERR_load_crypto_strings();
> +	ERR_clear_error();
> +
> +	/* Read the private key and the X.509 cert the PKCS#7 message
> +	 * will point to.
> +	 */
> +	b = BIO_new_file(private_key_name, "rb");
> +	ERR(!b, "%s", private_key_name);
> +        private_key = PEM_read_bio_PrivateKey(b, NULL, NULL, NULL);
> +	BIO_free(b);
> +
> +	b = BIO_new_file(x509_name, "rb");
> +	ERR(!b, "%s", x509_name);
> +        x509 = PEM_read_bio_X509(b, NULL, NULL, NULL);
> +	BIO_free(b);
> +
> +	/* Open the destination file now so that we can shovel the module data
> +	 * across as we read it.
> +	 */
> +	bd = BIO_new_file(dest_name, "wb");
> +	ERR(!bd, dest_name);
> +
> +	/* Digest the module data. */
> +	OpenSSL_add_all_digests();
> +	display_openssl_errors(__LINE__);
> +	digest_algo = EVP_get_digestbyname(hash_algo);
> +	ERR(!digest_algo, "EVP_get_digestbyname");
> +
> +	bm = BIO_new_file(module_name, "rb");
> +	ERR(!bm, "%s", module_name);
> +
> +	/* Load the PKCS#7 message from the digest buffer. */
> +	pkcs7 = PKCS7_sign(NULL, NULL, NULL, NULL,
> +			   PKCS7_NOCERTS | PKCS7_PARTIAL | PKCS7_BINARY | PKCS7_DETACHED | PKCS7_STREAM);
> +	ERR(!pkcs7, "PKCS7_sign");
> +
> +	ERR(PKCS7_sign_add_signer(pkcs7, x509, private_key, digest_algo, PKCS7_NOCERTS | PKCS7_BINARY) < 0,
> +	    "PKCS7_sign_add_signer");
> +	ERR(PKCS7_final(pkcs7, bm, PKCS7_NOCERTS | PKCS7_BINARY) < 0,
> +	    "PKCS7_final");
> +
> +	if (save_pkcs7) {
> +		char *pkcs7_name;
> +
> +		ERR(asprintf(&pkcs7_name, "%s.pkcs7", module_name) < 0, "asprintf");
> +		b = BIO_new_file(pkcs7_name, "wb");
> +		ERR(!b, pkcs7_name);
> +		ERR(i2d_PKCS7_bio_stream(b, pkcs7, NULL, 0) < 0, pkcs7_name);
> +		BIO_free(b);
> +	}
> +
> +	/* Append the marker and the PKCS#7 message to the destination file */
> +	ERR(BIO_reset(bm) < 0, module_name);
> +	while ((n = BIO_read(bm, buf, sizeof(buf))),
> +	       n > 0) {
> +		ERR(BIO_write(bd, buf, n) < 0, dest_name);
> +	}
> +	ERR(n < 0, module_name);
> +	module_size = BIO_number_written(bd);
> +
> +	ERR(i2d_PKCS7_bio_stream(bd, pkcs7, NULL, 0) < 0, dest_name);
> +	pkcs7_size = BIO_number_written(bd) - module_size;
> +	sig_info.sig_len = htonl(pkcs7_size);
> +	ERR(BIO_write(bd, &sig_info, sizeof(sig_info)) < 0, dest_name);
> +	ERR(BIO_write(bd, magic_number, sizeof(magic_number) - 1) < 0, dest_name);
> +
> +	ERR(BIO_free(bd) < 0, dest_name);
> +
> +	/* Finally, if we're signing in place, replace the original. */
> +	if (replace_orig)
> +		ERR(rename(dest_name, module_name) < 0, dest_name);
> +
> +	return 0;
> +}
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


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

* Re: [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-11-26 14:17 ` [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
  2014-12-05  8:54   ` Dmitry Kasatkin
@ 2014-12-05 10:23   ` David Howells
  2014-12-05 12:47     ` Dmitry Kasatkin
  2014-12-05 14:04     ` David Howells
  2014-12-11 16:41   ` David Howells
  2 siblings, 2 replies; 16+ messages in thread
From: David Howells @ 2014-12-05 10:23 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: dhowells, rusty, mmarek, keyrings, linux-kernel,
	linux-security-module, zohar

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

> sign-file.c produce lots of annoying noise.

How did you get it to produce that?

David

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

* Re: [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-12-05 10:23   ` David Howells
@ 2014-12-05 12:47     ` Dmitry Kasatkin
  2014-12-05 12:49       ` Dmitry Kasatkin
  2014-12-05 14:04     ` David Howells
  1 sibling, 1 reply; 16+ messages in thread
From: Dmitry Kasatkin @ 2014-12-05 12:47 UTC (permalink / raw)
  To: David Howells
  Cc: rusty, mmarek, keyrings, linux-kernel, linux-security-module, zohar

On 05/12/14 12:23, David Howells wrote:
> Dmitry Kasatkin <d.kasatkin@samsung.com> wrote:
>
>> sign-file.c produce lots of annoying noise.
> How did you get it to produce that?
>
> David
>

With just "make all" on Ubuntu.


- Dmitry


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

* Re: [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-12-05 12:47     ` Dmitry Kasatkin
@ 2014-12-05 12:49       ` Dmitry Kasatkin
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Kasatkin @ 2014-12-05 12:49 UTC (permalink / raw)
  To: David Howells
  Cc: rusty, mmarek, keyrings, linux-kernel, linux-security-module, zohar

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

On 05/12/14 14:47, Dmitry Kasatkin wrote:
> On 05/12/14 12:23, David Howells wrote:
>> Dmitry Kasatkin <d.kasatkin@samsung.com> wrote:
>>
>>> sign-file.c produce lots of annoying noise.
>> How did you get it to produce that?
>>
>> David
>>
> With just "make all" on Ubuntu.
>
>
> - Dmitry
>

See command line in attached file...

- Dmitry


[-- Attachment #2: .sign-file.cmd --]
[-- Type: text/plain, Size: 4515 bytes --]

cmd_scripts/sign-file := gcc -Wp,-MD,scripts/.sign-file.d -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 -fomit-frame-pointer   -I./tools/include  -o scripts/sign-file scripts/sign-file.c  -lcrypto

source_scripts/sign-file := scripts/sign-file.c

deps_scripts/sign-file := \
  /usr/include/stdc-predef.h \
  /usr/include/stdio.h \
  /usr/include/features.h \
  /usr/include/x86_64-linux-gnu/sys/cdefs.h \
  /usr/include/x86_64-linux-gnu/bits/wordsize.h \
  /usr/include/x86_64-linux-gnu/gnu/stubs.h \
  /usr/include/x86_64-linux-gnu/gnu/stubs-64.h \
  /usr/lib/gcc/x86_64-linux-gnu/4.8/include/stddef.h \
  /usr/include/x86_64-linux-gnu/bits/types.h \
  /usr/include/x86_64-linux-gnu/bits/typesizes.h \
  /usr/include/libio.h \
  /usr/include/_G_config.h \
  /usr/include/wchar.h \
  /usr/lib/gcc/x86_64-linux-gnu/4.8/include/stdarg.h \
  /usr/include/x86_64-linux-gnu/bits/stdio_lim.h \
  /usr/include/x86_64-linux-gnu/bits/sys_errlist.h \
  /usr/include/x86_64-linux-gnu/bits/stdio.h \
  /usr/include/x86_64-linux-gnu/bits/stdio2.h \
  /usr/include/stdlib.h \
  /usr/include/x86_64-linux-gnu/bits/waitflags.h \
  /usr/include/x86_64-linux-gnu/bits/waitstatus.h \
  /usr/include/endian.h \
  /usr/include/x86_64-linux-gnu/bits/endian.h \
  /usr/include/x86_64-linux-gnu/bits/byteswap.h \
  /usr/include/x86_64-linux-gnu/bits/byteswap-16.h \
  /usr/include/xlocale.h \
  /usr/include/x86_64-linux-gnu/sys/types.h \
  /usr/include/time.h \
  /usr/include/x86_64-linux-gnu/sys/select.h \
  /usr/include/x86_64-linux-gnu/bits/select.h \
  /usr/include/x86_64-linux-gnu/bits/sigset.h \
  /usr/include/x86_64-linux-gnu/bits/time.h \
  /usr/include/x86_64-linux-gnu/bits/select2.h \
  /usr/include/x86_64-linux-gnu/sys/sysmacros.h \
  /usr/include/x86_64-linux-gnu/bits/pthreadtypes.h \
  /usr/include/alloca.h \
  /usr/include/x86_64-linux-gnu/bits/stdlib-bsearch.h \
  /usr/include/x86_64-linux-gnu/bits/stdlib-float.h \
  /usr/include/x86_64-linux-gnu/bits/stdlib.h \
  /usr/lib/gcc/x86_64-linux-gnu/4.8/include/stdint.h \
  /usr/include/stdint.h \
  /usr/include/x86_64-linux-gnu/bits/wchar.h \
  /usr/lib/gcc/x86_64-linux-gnu/4.8/include/stdbool.h \
  /usr/include/string.h \
  /usr/include/x86_64-linux-gnu/bits/string.h \
  /usr/include/x86_64-linux-gnu/bits/string2.h \
  /usr/include/x86_64-linux-gnu/bits/string3.h \
  /usr/include/getopt.h \
  /usr/include/err.h \
  /usr/include/arpa/inet.h \
  /usr/include/netinet/in.h \
  /usr/include/x86_64-linux-gnu/sys/socket.h \
  /usr/include/x86_64-linux-gnu/sys/uio.h \
  /usr/include/x86_64-linux-gnu/bits/uio.h \
  /usr/include/x86_64-linux-gnu/bits/socket.h \
  /usr/include/x86_64-linux-gnu/bits/socket_type.h \
  /usr/include/x86_64-linux-gnu/bits/sockaddr.h \
  /usr/include/x86_64-linux-gnu/asm/socket.h \
  /usr/include/asm-generic/socket.h \
  /usr/include/x86_64-linux-gnu/asm/sockios.h \
  /usr/include/asm-generic/sockios.h \
  /usr/include/x86_64-linux-gnu/bits/socket2.h \
  /usr/include/x86_64-linux-gnu/bits/in.h \
  /usr/include/openssl/bio.h \
  /usr/include/openssl/e_os2.h \
  /usr/include/x86_64-linux-gnu/openssl/opensslconf.h \
    $(wildcard include/config/header/bn/h.h) \
    $(wildcard include/config/header/rc4/locl/h.h) \
    $(wildcard include/config/header/bf/locl/h.h) \
    $(wildcard include/config/header/des/locl/h.h) \
  /usr/include/openssl/crypto.h \
  /usr/include/openssl/stack.h \
  /usr/include/openssl/safestack.h \
  /usr/include/openssl/opensslv.h \
  /usr/include/openssl/ossl_typ.h \
  /usr/include/openssl/symhacks.h \
  /usr/include/openssl/evp.h \
  /usr/include/openssl/objects.h \
  /usr/include/openssl/obj_mac.h \
  /usr/include/openssl/asn1.h \
  /usr/include/x86_64-linux-gnu/bits/timex.h \
  /usr/include/openssl/bn.h \
  /usr/include/openssl/pem.h \
  /usr/include/openssl/x509.h \
  /usr/include/openssl/buffer.h \
  /usr/include/openssl/ec.h \
  /usr/include/openssl/ecdsa.h \
  /usr/include/openssl/ecdh.h \
  /usr/include/openssl/rsa.h \
  /usr/include/openssl/dsa.h \
  /usr/include/openssl/dh.h \
  /usr/include/openssl/sha.h \
  /usr/include/openssl/x509_vfy.h \
  /usr/include/openssl/lhash.h \
  /usr/include/openssl/pkcs7.h \
  /usr/include/openssl/pem2.h \
  /usr/include/openssl/err.h \
  /usr/include/errno.h \
  /usr/include/x86_64-linux-gnu/bits/errno.h \
  /usr/include/linux/errno.h \
  /usr/include/x86_64-linux-gnu/asm/errno.h \
  /usr/include/asm-generic/errno.h \
  /usr/include/asm-generic/errno-base.h \

scripts/sign-file: $(deps_scripts/sign-file)

$(deps_scripts/sign-file):

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

* Re: [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-12-05 10:23   ` David Howells
  2014-12-05 12:47     ` Dmitry Kasatkin
@ 2014-12-05 14:04     ` David Howells
  2014-12-05 14:37       ` Dmitry Kasatkin
  1 sibling, 1 reply; 16+ messages in thread
From: David Howells @ 2014-12-05 14:04 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: dhowells, rusty, mmarek, keyrings, linux-kernel,
	linux-security-module, zohar

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

> With just "make all" on Ubuntu.

What gcc?  I don't see any warnings.

David

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

* Re: [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-12-05 14:04     ` David Howells
@ 2014-12-05 14:37       ` Dmitry Kasatkin
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Kasatkin @ 2014-12-05 14:37 UTC (permalink / raw)
  To: David Howells
  Cc: rusty, mmarek, keyrings, linux-kernel, linux-security-module, zohar

On 05/12/14 16:04, David Howells wrote:
> Dmitry Kasatkin <d.kasatkin@samsung.com> wrote:
>
>> With just "make all" on Ubuntu.
> What gcc?  I don't see any warnings.
>
> David
>

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.8/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
4.8.2-19ubuntu1' --with-bugurl=file:///usr/share/doc/gcc-4.8/README.Bugs
--enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.8 --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.8 --libdir=/usr/lib
--enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--enable-gnu-unique-object --disable-libmudflap --enable-plugin
--with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
--enable-gtk-cairo
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.8-amd64/jre
--enable-java-home
--with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.8-amd64
--with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.8-amd64
--with-arch-directory=amd64
--with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
--enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64
--with-multilib-list=m32,m64,mx32 --with-tune=generic
--enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1)



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

* Re: [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module [ver #2]
  2014-11-26 14:17 ` [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
  2014-12-05  8:54   ` Dmitry Kasatkin
  2014-12-05 10:23   ` David Howells
@ 2014-12-11 16:41   ` David Howells
  2 siblings, 0 replies; 16+ messages in thread
From: David Howells @ 2014-12-11 16:41 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: dhowells, rusty, mmarek, keyrings, linux-kernel,
	linux-security-module, zohar

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

> sign-file.c produce lots of annoying noise.

Compiling it manually with -Wformat-security found those problems you listed
and add -W found yet another problem.  Differential patch attached.  I've
folded it into the patch that adds sign-file.c.

David
---
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 3f9bedbd185f..7941f499ddba 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -62,12 +62,12 @@ static void display_openssl_errors(int l)
 }
 
 
-#define ERR(cond, ...)				  \
+#define ERR(cond, fmt, ...)			  \
 	do {					  \
 		bool __cond = (cond);		  \
 		display_openssl_errors(__LINE__); \
 		if (__cond) {			  \
-			err(1, ## __VA_ARGS__);	  \
+			err(1, fmt, ## __VA_ARGS__);	\
 		}				  \
 	} while(0)
 
@@ -133,7 +133,7 @@ int main(int argc, char **argv)
 	 * across as we read it.
 	 */
 	bd = BIO_new_file(dest_name, "wb");
-	ERR(!bd, dest_name);
+	ERR(!bd, "%s", dest_name);
 
 	/* Digest the module data. */
 	OpenSSL_add_all_digests();
@@ -149,7 +149,7 @@ int main(int argc, char **argv)
 			   PKCS7_NOCERTS | PKCS7_PARTIAL | PKCS7_BINARY | PKCS7_DETACHED | PKCS7_STREAM);
 	ERR(!pkcs7, "PKCS7_sign");
 
-	ERR(PKCS7_sign_add_signer(pkcs7, x509, private_key, digest_algo, PKCS7_NOCERTS | PKCS7_BINARY) < 0,
+	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");
@@ -159,31 +159,31 @@ int main(int argc, char **argv)
 
 		ERR(asprintf(&pkcs7_name, "%s.pkcs7", module_name) < 0, "asprintf");
 		b = BIO_new_file(pkcs7_name, "wb");
-		ERR(!b, pkcs7_name);
-		ERR(i2d_PKCS7_bio_stream(b, pkcs7, NULL, 0) < 0, pkcs7_name);
+		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, module_name);
+	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, dest_name);
+		ERR(BIO_write(bd, buf, n) < 0, "%s", dest_name);
 	}
-	ERR(n < 0, module_name);
+	ERR(n < 0, "%s", module_name);
 	module_size = BIO_number_written(bd);
 
-	ERR(i2d_PKCS7_bio_stream(bd, pkcs7, NULL, 0) < 0, dest_name);
+	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, dest_name);
-	ERR(BIO_write(bd, magic_number, sizeof(magic_number) - 1) < 0, dest_name);
+	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, 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, dest_name);
+		ERR(rename(dest_name, module_name) < 0, "%s", dest_name);
 
 	return 0;
 }

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

end of thread, other threads:[~2014-12-11 16:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-26 14:17 [PATCH 0/5] MODSIGN: Use PKCS#7 for module signatures [ver #2] David Howells
2014-11-26 14:17 ` [PATCH 1/5] X.509: Extract both parts of the AuthorityKeyIdentifier " David Howells
2014-12-04 12:14   ` Dmitry Kasatkin
2014-12-04 12:51   ` David Howells
2014-11-26 14:17 ` [PATCH 2/5] X.509: Support X.509 lookup by Issuer+Serial form " David Howells
2014-11-26 14:17 ` [PATCH 3/5] PKCS#7: Allow detached data to be supplied for signature checking purposes " David Howells
2014-11-26 14:17 ` [PATCH 4/5] MODSIGN: Provide a utility to append a PKCS#7 signature to a module " David Howells
2014-12-05  8:54   ` Dmitry Kasatkin
2014-12-05 10:23   ` David Howells
2014-12-05 12:47     ` Dmitry Kasatkin
2014-12-05 12:49       ` Dmitry Kasatkin
2014-12-05 14:04     ` David Howells
2014-12-05 14:37       ` Dmitry Kasatkin
2014-12-11 16:41   ` David Howells
2014-11-26 14:18 ` [PATCH 5/5] MODSIGN: Use PKCS#7 messages as module signatures " David Howells
2014-12-04 18:56 ` [PATCH 0/5] MODSIGN: Use PKCS#7 for " Vivek Goyal

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