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

[Andy Lutomirski <luto@kernel.org>]

On 05/15/2015 05:35 AM, David Howells wrote:
> Provide a utility that:
>
>   (1) Digests a module using the specified hash algorithm (typically sha256).
>
>       [The digest can be dumped into a file by passing the '-d' flag]
>
>   (2) Generates a PKCS#7 message that:
>
>       (a) Has detached data (ie. the module content).
>
>       (b) Is signed with the specified private key.
>
>       (c) Refers to the specified X.509 certificate.
>
>       (d) Has an empty X.509 certificate list.
>
>       [The PKCS#7 message can be dumped into a file by passing the '-p' flag]
>
>   (3) Generates a signed module by concatenating the old module, the PKCS#7
>       message, a descriptor and a magic string.  The descriptor contains the
>       size of the PKCS#7 message and indicates the id_type as PKEY_ID_PKCS7.
>
>   (4) Either writes the signed module to the specified destination or renames
>       it over the source module.
>
> This allows module signing to reuse the PKCS#7 handling code that was added
> for PE file parsing for signed kexec.
>
> Note that the utility is written in C and must be linked against the OpenSSL
> crypto library.
>
> Note further that I have temporarily dropped support for handling externally
> created signatures until we can work out the best way to do those.  Hopefully,
> whoever creates the signature can give me a PKCS#7 certificate.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Tested-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>
>   include/crypto/public_key.h |    1
>   scripts/sign-file.c         |  205 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 206 insertions(+)
>   create mode 100755 scripts/sign-file.c
>
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index b6f27a240856..fda097e079a4 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -33,6 +33,7 @@ extern const struct public_key_algorithm *pkey_algo[PKEY_ALGO__LAST];
>   enum pkey_id_type {
>   	PKEY_ID_PGP,		/* OpenPGP generated key ID */
>   	PKEY_ID_X509,		/* X.509 arbitrary subjectKeyIdentifier */
> +	PKEY_ID_PKCS7,		/* Signature in PKCS#7 message */
>   	PKEY_ID_TYPE__LAST
>   };
>

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

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

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

It could be as simple as:

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

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

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

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

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

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

--Andy