[GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

Hi James,

Can you pull this into security/next please?  Its aim is twofold: firstly,
make the module signatures of PKCS#7/CMS format rather than a home-brewed
format and secondly to pave the way for use of the signing code for
firmware signatures (to follow later).

To this end, the patchset effects the following changes:

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

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

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

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

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

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

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

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

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

 (6) Adds support for CMS messages in place of PKCS#7 (they're very similar
     ASN.1) and makes sign-file create CMS messages instead of PKCS#7.
     This allows signatures to refer to the verifying key by X.509 cert
     SKID instead of X.509 cert issuer and serial number.

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

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

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

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

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

I can added the following to the patch set:

Tested-by: Luis R. Rodriguez <mcgrof@suse.com>

David
---
The following changes since commit fe6c59dc17908effd4e2caa666795b9ad984005b:

  Merge tag 'seccomp-next' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux into next (2015-07-20 17:19:19 +1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/modsign-pkcs7-20150720

for you to fetch changes up to e8e9007e54dbaa1919fb3e0fad7e83782e0a4cd6:

  modsign: Use extract-cert to process CONFIG_SYSTEM_TRUSTED_KEYS (2015-07-20 21:16:34 +0100)

----------------------------------------------------------------
Module signing with PKCS#7

----------------------------------------------------------------
David Howells (13):
      X.509: Extract both parts of the AuthorityKeyIdentifier
      X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
      PKCS#7: Allow detached data to be supplied for signature checking purposes
      MODSIGN: Provide a utility to append a PKCS#7 signature to a module
      MODSIGN: Use PKCS#7 messages as module signatures
      system_keyring.c doesn't need to #include module-internal.h
      MODSIGN: Extract the blob PKCS#7 signature verifier from module signing
      PKCS#7: Check content type and versions
      ASN.1: Add an ASN.1 compiler option to dump the element tree
      ASN.1: Fix handling of CHOICE in ASN.1 compiler
      X.509: Change recorded SKID & AKID to not include Subject or Issuer
      PKCS#7: Support CMS messages also [RFC5652]
      sign-file: Generate CMS message as signature instead of PKCS#7

David Woodhouse (9):
      modsign: Abort modules_install when signing fails
      modsign: Allow password to be specified for signing key
      modsign: Allow signing key to be PKCS#11
      modsign: Allow external signing key to be specified
      modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed
      modsign: Use single PEM file for autogenerated key
      modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option
      extract-cert: Cope with multiple X.509 certificates in a single file
      modsign: Use extract-cert to process CONFIG_SYSTEM_TRUSTED_KEYS

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

 .gitignore                                |   1 +
 Documentation/kbuild/kbuild.txt           |   5 +
 Documentation/module-signing.txt          |  51 +++-
 Makefile                                  |   8 +-
 crypto/asymmetric_keys/Makefile           |   8 +-
 crypto/asymmetric_keys/pkcs7.asn1         |  16 +-
 crypto/asymmetric_keys/pkcs7_parser.c     | 131 +++++++++-
 crypto/asymmetric_keys/pkcs7_parser.h     |   5 +-
 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 | 144 ++++++----
 crypto/asymmetric_keys/x509_parser.h      |   5 +-
 crypto/asymmetric_keys/x509_public_key.c  |  86 ++++--
 include/crypto/pkcs7.h                    |   3 +
 include/crypto/public_key.h               |   4 +-
 include/keys/system_keyring.h             |   5 +
 init/Kconfig                              |  55 +++-
 kernel/Makefile                           | 112 +++++---
 kernel/module_signing.c                   | 212 ++-------------
 kernel/system_certificates.S              |   3 +
 kernel/system_keyring.c                   |  51 +++-
 scripts/Makefile                          |   4 +
 scripts/Makefile.modinst                  |   2 +-
 scripts/asn1_compiler.c                   | 104 ++++++--
 scripts/extract-cert.c                    | 166 ++++++++++++
 scripts/sign-file                         | 421 ------------------------------
 scripts/sign-file.c                       | 259 ++++++++++++++++++
 28 files changed, 1178 insertions(+), 808 deletions(-)
 create mode 100644 crypto/asymmetric_keys/x509_akid.asn1
 create mode 100644 scripts/extract-cert.c
 delete mode 100755 scripts/sign-file
 create mode 100755 scripts/sign-file.c

Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[Andy Lutomirski]

On 07/27/2015 12:33 PM, David Howells wrote:
> Hi James,
>
> Can you pull this into security/next please?  Its aim is twofold: firstly,
> make the module signatures of PKCS#7/CMS format rather than a home-brewed
> format and secondly to pave the way for use of the signing code for
> firmware signatures (to follow later).

With all this stuff applied, will the kernel accept PKCS#7 signatures 
that *don't* have authenticated attributes or that are otherwise 
cryptographically insecure in that they fail to provide the property 
that an attacker can't manipulate a valid signature on one message to 
look like a valid signature on a different message?

It looks like fixing that might actually be important if anyone ever 
wants to use this for firmware signing.

At least there's no issue with newer kernels needing to accept module 
signautures generated by old tools, since the newer kernels won't accept 
the underlying modules anyway.

--Andy

Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

Andy Lutomirski <luto@kernel.org> wrote:

> With all this stuff applied, will the kernel accept PKCS#7 signatures that
> *don't* have authenticated attributes or that are otherwise cryptographically
> insecure in that they fail to provide the property that an attacker can't
> manipulate a valid signature on one message to look like a valid signature on
> a different message?

Hmmm...  That's easy enough to fix (see below).  However, will that cause
kexec problems, I wonder?  Does mscode require authattrs?

David
---
commit 44460686dfb0a4cca06f20e27988965e327e0f93
Author: David Howells <dhowells@redhat.com>
Date:   Mon Jul 27 23:32:03 2015 +0100

    PKCS#7: Require authenticated attributes
    
    Require there to be authenticated attributes in the PKCS#7/CMS message so
    that an attacker can't drop them to provide greater opportunity for
    manipulating the message.
    
    Suggested-by: Andy Lutomirski <luto@kernel.org>
    Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 404f89a0f852..be0fc3b49b43 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -30,6 +30,7 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
 	size_t digest_size, desc_size;
 	void *digest;
 	int ret;
+	u8 tag;
 
 	kenter(",%u,%u", sinfo->index, sinfo->sig.pkey_hash_algo);
 
@@ -70,43 +71,45 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
 	 * message digest attribute amongst them which corresponds to the
 	 * digest we just calculated.
 	 */
-	if (sinfo->msgdigest) {
-		u8 tag;
-
-		if (sinfo->msgdigest_len != sinfo->sig.digest_size) {
-			pr_debug("Sig %u: Invalid digest size (%u)\n",
-				 sinfo->index, sinfo->msgdigest_len);
-			ret = -EBADMSG;
-			goto error;
-		}
+	if (!sinfo->authattrs || !sinfo->msgdigest) {
+		pr_warn("Sig %u: No authenticatedAttrs\n", sinfo->index);
+		ret = -EKEYREJECTED;
+		goto error;
+	}
+	
+	if (sinfo->msgdigest_len != sinfo->sig.digest_size) {
+		pr_debug("Sig %u: Invalid digest size (%u)\n",
+			 sinfo->index, sinfo->msgdigest_len);
+		ret = -EBADMSG;
+		goto error;
+	}
 
-		if (memcmp(digest, sinfo->msgdigest, sinfo->msgdigest_len) != 0) {
-			pr_debug("Sig %u: Message digest doesn't match\n",
-				 sinfo->index);
-			ret = -EKEYREJECTED;
-			goto error;
-		}
+	if (memcmp(digest, sinfo->msgdigest, sinfo->msgdigest_len) != 0) {
+		pr_debug("Sig %u: Message digest doesn't match\n",
+			 sinfo->index);
+		ret = -EKEYREJECTED;
+		goto error;
+	}
 
-		/* We then calculate anew, using the authenticated attributes
-		 * as the contents of the digest instead.  Note that we need to
-		 * convert the attributes from a CONT.0 into a SET before we
-		 * hash it.
-		 */
-		memset(digest, 0, sinfo->sig.digest_size);
+	/* We then calculate anew, using the authenticated attributes
+	 * as the contents of the digest instead.  Note that we need to
+	 * convert the attributes from a CONT.0 into a SET before we
+	 * hash it.
+	 */
+	memset(digest, 0, sinfo->sig.digest_size);
 
-		ret = crypto_shash_init(desc);
-		if (ret < 0)
-			goto error;
-		tag = ASN1_CONS_BIT | ASN1_SET;
-		ret = crypto_shash_update(desc, &tag, 1);
-		if (ret < 0)
-			goto error;
-		ret = crypto_shash_finup(desc, sinfo->authattrs,
-					 sinfo->authattrs_len, digest);
-		if (ret < 0)
-			goto error;
-		pr_devel("AADigest = [%*ph]\n", 8, digest);
-	}
+	ret = crypto_shash_init(desc);
+	if (ret < 0)
+		goto error;
+	tag = ASN1_CONS_BIT | ASN1_SET;
+	ret = crypto_shash_update(desc, &tag, 1);
+	if (ret < 0)
+		goto error;
+	ret = crypto_shash_finup(desc, sinfo->authattrs,
+				 sinfo->authattrs_len, digest);
+	if (ret < 0)
+		goto error;
+	pr_devel("AADigest = [%*ph]\n", 8, digest);
 
 	sinfo->sig.digest = digest;
 	digest = NULL;

Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[Andy Lutomirski]

On Mon, Jul 27, 2015 at 3:43 PM, David Howells <dhowells@redhat.com> wrote:
> Andy Lutomirski <luto@kernel.org> wrote:
>
>> With all this stuff applied, will the kernel accept PKCS#7 signatures that
>> *don't* have authenticated attributes or that are otherwise cryptographically
>> insecure in that they fail to provide the property that an attacker can't
>> manipulate a valid signature on one message to look like a valid signature on
>> a different message?
>
> Hmmm...  That's easy enough to fix (see below).  However, will that cause
> kexec problems, I wonder?  Does mscode require authattrs?
>

Seems sensible.

How would it cause kexec problems?  I can only see it being a problem
if Authenticode can't handle authattrs, right?  There shouldn't be any
legacy PKCS7 kexec images whatsoever, because no existing kernel will
boot them or generate them.

--Andy

Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

Andy Lutomirski <luto@amacapital.net> wrote:

> How would it cause kexec problems?  I can only see it being a problem
> if Authenticode can't handle authattrs, right?  There shouldn't be any
> legacy PKCS7 kexec images whatsoever, because no existing kernel will
> boot them or generate them.

I was wondering if it is possible to get an mscode message that doesn't have
authattrs in it.  I guess we can just ignore the possibility until the matter
arises, if it does, and deal with it then as appropriate.

David

Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[David Woodhouse]

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

On Mon, 2015-07-27 at 23:43 +0100, David Howells wrote:
> 
>     PKCS#7: Require authenticated attributes
>     
>     Require there to be authenticated attributes in the PKCS#7/CMS message so
>     that an attacker can't drop them to provide greater opportunity for
>     manipulating the message.

There doesn't seem to be a lot of point in this part. If the
authenticated attribute isn't being *checked*, then the attacker
doesn't need to drop it at all. There's no point in merely requiring
its *existence*.

As part of the firmware signatures, if we are asked to check the
filename then yes we should require it to be present *and* match. But
if we aren't checking (which we can't for modules since we don't know
what's being loaded), why require it to be present at all?

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

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

Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

[David Howells]

David Woodhouse <dwmw2@infradead.org> wrote:

> As part of the firmware signatures, if we are asked to check the
> filename then yes we should require it to be present *and* match. But
> if we aren't checking (which we can't for modules since we don't know
> what's being loaded), why require it to be present at all?

For firmware, that's in the next set of patches, at the tag
fwsign-pkcs7-20150720.

For modules we could require it not to be present since, as you say, there's
no way generally for the kernel check the module name requested.  The only
thing it could really do is to extract the expected name from the PKCS#7 and
compare it against the name in the modinfo structure *after* checking the
signature.  This would require passing the module name to sign-file too.

David