Re: [PATCH v6 03/12] tools: mkeficapsule: add firmwware image signing

2021-11-07 Thread AKASHI Takahiro
On Sun, Nov 07, 2021 at 07:53:24AM +0100, Heinrich Schuchardt wrote:
> On 11/2/21 01:55, AKASHI Takahiro wrote:
> > With this enhancement, mkeficapsule will be able to sign a capsule
> > file when it is created. A signature added will be used later
> > in the verification at FMP's SetImage() call.
> > 
> > To do that, We need specify additional command parameters:
> >-monotonic-cout  : monotonic count
> >-private-key  : private key file
> >-certificate  : certificate file
> > Only when all of those parameters are given, a signature will be added
> > to a capsule file.
> > 
> > Users are expected to maintain and increment the monotonic count at
> > every time of the update for each firmware image.
> > 
> > Signed-off-by: AKASHI Takahiro 
> > Reviewed-by: Simon Glass 
> 
> You licensed tools/mkeficapsule.c under GPL-2.0 which is imcompatible
> with OpenSSL (https://www.gnu.org/licenses/license-list.html#OpenSSL).

In fact, this is not a this-tool-specific issue, but other tools,
including mkimage, have it and that is why CONFIG_TOOLS_LIBCRYPTO was
introduced.

# I think that I have mentioned the issue in my past comment.

-Takahiro Akashi


> Either agree with all contributors of the tool to use a more liberal
> license or use another library.
> 
> > ---
> >   tools/Kconfig|   8 +
> >   tools/Makefile   |   8 +-
> >   tools/mkeficapsule.c | 382 ---
> >   3 files changed, 376 insertions(+), 22 deletions(-)
> > 
> > diff --git a/tools/Kconfig b/tools/Kconfig
> > index 91ce8ae3e516..117c921da3fe 100644
> > --- a/tools/Kconfig
> > +++ b/tools/Kconfig
> > @@ -90,4 +90,12 @@ config TOOLS_SHA512
> > help
> >   Enable SHA512 support in the tools builds
> > 
> > +config TOOLS_MKEFICAPSULE
> > +   bool "Build efimkcapsule command"
> > +   default y if EFI_CAPSULE_ON_DISK
> > +   help
> > + This command allows users to create a UEFI capsule file and,
> > + optionally sign that file. If you want to enable UEFI capsule
> > + update feature on your target, you certainly need this.
> > +
> >   endmenu
> > diff --git a/tools/Makefile b/tools/Makefile
> > index b45219e2c30c..5a73cc4b363d 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -238,8 +238,12 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
> >   hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler
> >   HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
> > 
> > -mkeficapsule-objs  := mkeficapsule.o $(LIBFDT_OBJS)
> > -hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
> > +HOSTLDLIBS_mkeficapsule += -luuid
> > +ifeq ($(CONFIG_TOOLS_LIBCRYPTO),y)
> > +HOSTLDLIBS_mkeficapsule += \
> > +   $(shell pkg-config --libs libssl libcrypto 2> /dev/null || echo "-lssl 
> > -lcrypto")
> > +endif
> > +hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
> > 
> >   # We build some files with extra pedantic flags to try to minimize things
> >   # that won't build on some weird host compiler -- though there are lots of
> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > index 8427fedd941c..086757ee8ad7 100644
> > --- a/tools/mkeficapsule.c
> > +++ b/tools/mkeficapsule.c
> > @@ -15,6 +15,16 @@
> >   #include 
> >   #include 
> > 
> > +#include 
> > +#ifdef CONFIG_TOOLS_LIBCRYPTO
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#endif
> > +
> >   typedef __u8 u8;
> >   typedef __u16 u16;
> >   typedef __u32 u32;
> > @@ -38,12 +48,25 @@ efi_guid_t efi_guid_image_type_uboot_fit =
> > EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
> >   efi_guid_t efi_guid_image_type_uboot_raw =
> > EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
> > +efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> > +
> > +#ifdef CONFIG_TOOLS_LIBCRYPTO
> > +static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
> > +#else
> > +static const char *opts_short = "f:r:i:I:v:h";
> > +#endif
> > 
> >   static struct option options[] = {
> > {"fit", required_argument, NULL, 'f'},
> > {"raw", required_argument, NULL, 'r'},
> > {"index", required_argument, NULL, 'i'},
> > {"instance", required_argument, NULL, 'I'},
> > +#ifdef CONFIG_TOOLS_LIBCRYPTO
> > +   {"private-key", required_argument, NULL, 'p'},
> > +   {"certificate", required_argument, NULL, 'c'},
> > +   {"monotonic-count", required_argument, NULL, 'm'},
> > +   {"dump-sig", no_argument, NULL, 'd'},
> > +#endif
> > {"help", no_argument, NULL, 'h'},
> > {NULL, 0, NULL, 0},
> >   };
> > @@ -57,10 +80,252 @@ static void print_usage(void)
> >"\t-r, --rawnew raw image file\n"
> >"\t-i, --index  update image index\n"
> >"\t-I, --instanceupdate hardware instance\n"
> > +#ifdef CONFIG_TOOLS_LIBCRYPTO
> > +  "\t-p, --private-key   private key file\n"
> > +  "\t-c, --certificate  signer's certificate file\n"
> > +  "\t-m, --monotonic-count  monotonic count\n"
> > +  "

Re: [PATCH v6 03/12] tools: mkeficapsule: add firmwware image signing

2021-11-06 Thread Heinrich Schuchardt

On 11/2/21 01:55, AKASHI Takahiro wrote:

With this enhancement, mkeficapsule will be able to sign a capsule
file when it is created. A signature added will be used later
in the verification at FMP's SetImage() call.

To do that, We need specify additional command parameters:
   -monotonic-cout  : monotonic count
   -private-key  : private key file
   -certificate  : certificate file
Only when all of those parameters are given, a signature will be added
to a capsule file.

Users are expected to maintain and increment the monotonic count at
every time of the update for each firmware image.

Signed-off-by: AKASHI Takahiro 
Reviewed-by: Simon Glass 


You licensed tools/mkeficapsule.c under GPL-2.0 which is imcompatible
with OpenSSL (https://www.gnu.org/licenses/license-list.html#OpenSSL).

Either agree with all contributors of the tool to use a more liberal
license or use another library.


---
  tools/Kconfig|   8 +
  tools/Makefile   |   8 +-
  tools/mkeficapsule.c | 382 ---
  3 files changed, 376 insertions(+), 22 deletions(-)

diff --git a/tools/Kconfig b/tools/Kconfig
index 91ce8ae3e516..117c921da3fe 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -90,4 +90,12 @@ config TOOLS_SHA512
help
  Enable SHA512 support in the tools builds

+config TOOLS_MKEFICAPSULE
+   bool "Build efimkcapsule command"
+   default y if EFI_CAPSULE_ON_DISK
+   help
+ This command allows users to create a UEFI capsule file and,
+ optionally sign that file. If you want to enable UEFI capsule
+ update feature on your target, you certainly need this.
+
  endmenu
diff --git a/tools/Makefile b/tools/Makefile
index b45219e2c30c..5a73cc4b363d 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -238,8 +238,12 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
  hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler
  HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include

-mkeficapsule-objs  := mkeficapsule.o $(LIBFDT_OBJS)
-hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
+HOSTLDLIBS_mkeficapsule += -luuid
+ifeq ($(CONFIG_TOOLS_LIBCRYPTO),y)
+HOSTLDLIBS_mkeficapsule += \
+   $(shell pkg-config --libs libssl libcrypto 2> /dev/null || echo "-lssl 
-lcrypto")
+endif
+hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule

  # We build some files with extra pedantic flags to try to minimize things
  # that won't build on some weird host compiler -- though there are lots of
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index 8427fedd941c..086757ee8ad7 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -15,6 +15,16 @@
  #include 
  #include 

+#include 
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
+
  typedef __u8 u8;
  typedef __u16 u16;
  typedef __u32 u32;
@@ -38,12 +48,25 @@ efi_guid_t efi_guid_image_type_uboot_fit =
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
  efi_guid_t efi_guid_image_type_uboot_raw =
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
+efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
+
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
+#else
+static const char *opts_short = "f:r:i:I:v:h";
+#endif

  static struct option options[] = {
{"fit", required_argument, NULL, 'f'},
{"raw", required_argument, NULL, 'r'},
{"index", required_argument, NULL, 'i'},
{"instance", required_argument, NULL, 'I'},
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+   {"private-key", required_argument, NULL, 'p'},
+   {"certificate", required_argument, NULL, 'c'},
+   {"monotonic-count", required_argument, NULL, 'm'},
+   {"dump-sig", no_argument, NULL, 'd'},
+#endif
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0},
  };
@@ -57,10 +80,252 @@ static void print_usage(void)
   "\t-r, --rawnew raw image file\n"
   "\t-i, --index  update image index\n"
   "\t-I, --instanceupdate hardware instance\n"
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+  "\t-p, --private-key   private key file\n"
+  "\t-c, --certificate  signer's certificate file\n"
+  "\t-m, --monotonic-count  monotonic count\n"
+  "\t-d, --dump_sig  dump signature (*.p7)\n"
+#endif
   "\t-h, --help  print a help message\n",
   tool_name);
  }

+/**
+ * auth_context - authentication context
+ * @key_file:  Path to a private key file
+ * @cert_file: Path to a certificate file
+ * @image_data:Pointer to firmware data
+ * @image_size:Size of firmware data
+ * @auth:  Authentication header
+ * @sig_data:  Signature data
+ * @sig_size:  Size of signature data
+ *
+ * Data structure used in create_auth_data(). @key_file through
+ * @image_size are input parameters. @auth, @sig_data and @sig_size
+ * are fi

[PATCH v6 03/12] tools: mkeficapsule: add firmwware image signing

2021-11-01 Thread AKASHI Takahiro
With this enhancement, mkeficapsule will be able to sign a capsule
file when it is created. A signature added will be used later
in the verification at FMP's SetImage() call.

To do that, We need specify additional command parameters:
  -monotonic-cout  : monotonic count
  -private-key  : private key file
  -certificate  : certificate file
Only when all of those parameters are given, a signature will be added
to a capsule file.

Users are expected to maintain and increment the monotonic count at
every time of the update for each firmware image.

Signed-off-by: AKASHI Takahiro 
Reviewed-by: Simon Glass 
---
 tools/Kconfig|   8 +
 tools/Makefile   |   8 +-
 tools/mkeficapsule.c | 382 ---
 3 files changed, 376 insertions(+), 22 deletions(-)

diff --git a/tools/Kconfig b/tools/Kconfig
index 91ce8ae3e516..117c921da3fe 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -90,4 +90,12 @@ config TOOLS_SHA512
help
  Enable SHA512 support in the tools builds
 
+config TOOLS_MKEFICAPSULE
+   bool "Build efimkcapsule command"
+   default y if EFI_CAPSULE_ON_DISK
+   help
+ This command allows users to create a UEFI capsule file and,
+ optionally sign that file. If you want to enable UEFI capsule
+ update feature on your target, you certainly need this.
+
 endmenu
diff --git a/tools/Makefile b/tools/Makefile
index b45219e2c30c..5a73cc4b363d 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -238,8 +238,12 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
 hostprogs-$(CONFIG_ASN1_COMPILER)  += asn1_compiler
 HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
 
-mkeficapsule-objs  := mkeficapsule.o $(LIBFDT_OBJS)
-hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
+HOSTLDLIBS_mkeficapsule += -luuid
+ifeq ($(CONFIG_TOOLS_LIBCRYPTO),y)
+HOSTLDLIBS_mkeficapsule += \
+   $(shell pkg-config --libs libssl libcrypto 2> /dev/null || echo "-lssl 
-lcrypto")
+endif
+hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
 
 # We build some files with extra pedantic flags to try to minimize things
 # that won't build on some weird host compiler -- though there are lots of
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index 8427fedd941c..086757ee8ad7 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -15,6 +15,16 @@
 #include 
 #include 
 
+#include 
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#endif
+
 typedef __u8 u8;
 typedef __u16 u16;
 typedef __u32 u32;
@@ -38,12 +48,25 @@ efi_guid_t efi_guid_image_type_uboot_fit =
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
 efi_guid_t efi_guid_image_type_uboot_raw =
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
+efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
+
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
+#else
+static const char *opts_short = "f:r:i:I:v:h";
+#endif
 
 static struct option options[] = {
{"fit", required_argument, NULL, 'f'},
{"raw", required_argument, NULL, 'r'},
{"index", required_argument, NULL, 'i'},
{"instance", required_argument, NULL, 'I'},
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+   {"private-key", required_argument, NULL, 'p'},
+   {"certificate", required_argument, NULL, 'c'},
+   {"monotonic-count", required_argument, NULL, 'm'},
+   {"dump-sig", no_argument, NULL, 'd'},
+#endif
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0},
 };
@@ -57,10 +80,252 @@ static void print_usage(void)
   "\t-r, --rawnew raw image file\n"
   "\t-i, --index  update image index\n"
   "\t-I, --instanceupdate hardware instance\n"
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+  "\t-p, --private-key   private key file\n"
+  "\t-c, --certificate  signer's certificate file\n"
+  "\t-m, --monotonic-count  monotonic count\n"
+  "\t-d, --dump_sig  dump signature (*.p7)\n"
+#endif
   "\t-h, --help  print a help message\n",
   tool_name);
 }
 
+/**
+ * auth_context - authentication context
+ * @key_file:  Path to a private key file
+ * @cert_file: Path to a certificate file
+ * @image_data:Pointer to firmware data
+ * @image_size:Size of firmware data
+ * @auth:  Authentication header
+ * @sig_data:  Signature data
+ * @sig_size:  Size of signature data
+ *
+ * Data structure used in create_auth_data(). @key_file through
+ * @image_size are input parameters. @auth, @sig_data and @sig_size
+ * are filled in by create_auth_data().
+ */
+struct auth_context {
+   char *key_file;
+   char *cert_file;
+   u8 *image_data;
+   size_t image_size;
+   struct efi_firmware_image_authentication auth;
+   u8 *sig_data;
+   size_t sig_size;
+};
+
+static int dump_sig;
+
+#ifdef CONFI