Hi Takahiro,
On Thu, 28 Oct 2021 at 22:56, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
On Thu, Oct 28, 2021 at 09:17:45PM -0600, Simon Glass wrote:
Hi Takahiro,
On Thu, 28 Oct 2021 at 00:25, AKASHI Takahiro takahiro.akashi@linaro.org 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 <count> : monotonic count -private-key <private key file> : private key file -certificate <certificate file> : 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 takahiro.akashi@linaro.org
tools/Kconfig | 8 + tools/Makefile | 8 +- tools/mkeficapsule.c | 435 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 417 insertions(+), 34 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Thank you for your reviewing.
This looks OK but I have some suggestions
- I don't think you should return -1 from main
exit(EXIT_FAILURE)? Yeah, but when I first wrote this tool (without authentication support), 'return -1' was used everywhere. So I didn't want to have mixed styles in this patch. I will make a change with the tweak below.
OK. I just mean that I think the return code is supposed to be 1 or 2 or maybe 3 on error, not 255.
- could you split up your create_fwbin() to return the number of gotos?
Yeah, lots of gotos are messy.
- could we have a man page for the tool?
Patch#3
OK
- should the files be opened in binary mode?
Well, the man page of fopen() says, This is strictly for compatibility with C89 and has no effect; the 'b' is ignored on all POSIX conforming sys- tems, including Linux.
U-Boot now requires C11, and so no need?
Ah OK. I suppose no one builds this on Windows.
- can we just build the tool always?
This is one of my questions. Why do you want to do so while there are bunch of tools that are not always built.
Because I think all tools should be built always. It is fine if that happens due to CONFIG options but we should try to avoid making it complicated.
# I saw some discussion in another topic thread, and some distro guy said # that they used sandbox_defconfig for tool packaging.
What about tools-only ?
So long as the options are enabled it is fine to have options for the tools. But I think we should try to build all the tools.
Regards, Simon