[{"id":27070,"web_url":"https://patchwork.libcamera.org/comment/27070/","msgid":"","date":"2023-05-08T19:20:30","subject":"Re: [libcamera-devel] [PATCH v1 2/2] apps: Add ipa-verify\n\tapplication","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the patch.\n\nOn 5/6/23 4:40 PM, Laurent Pinchart via libcamera-devel wrote:\n> When packaging libcamera, distributions may break IPA module signatures\n> if the packaging process strips binaries. This can be fixed by resigning\n> the modules, but the process is error-prone.\n>\n> Add a command line ipa-verify utility that tests the signature on an IPA\n> module to help packagers. The tool takes a single argument, the path to\n> an IPA module shared object, and expects the signature file (.sign) to\n> be in the same directory.\n>\n> In order to access the public key needed for signature verification, add\n> a static function to the IPAManager class. As the class is internal to\n> libcamera, this doesn't affect the public API.\n>\n> Signed-off-by: Laurent Pinchart \n\nOverall looks good to me. I can see packegers using this as part of \n\"postinst\" of the package itself.\n\nReviewed-by: Umang Jain \n\n> ---\n> include/libcamera/internal/ipa_manager.h | 7 +++\n> src/apps/ipa-verify/main.cpp | 64 ++++++++++++++++++++++++\n> src/apps/ipa-verify/meson.build | 15 ++++++\n> src/apps/meson.build | 2 +\n> src/libcamera/ipa_manager.cpp | 13 +++++\n> 5 files changed, 101 insertions(+)\n> create mode 100644 src/apps/ipa-verify/main.cpp\n> create mode 100644 src/apps/ipa-verify/meson.build\n>\n> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n> index 7f36e58e8bfa..bf823563c91c 100644\n> --- a/include/libcamera/internal/ipa_manager.h\n> +++ b/include/libcamera/internal/ipa_manager.h\n> @@ -47,6 +47,13 @@ public:\n> \t\treturn proxy;\n> \t}\n> \n> +#if HAVE_IPA_PUBKEY\n> +\tstatic const PubKey &pubKey()\n> +\t{\n> +\t\treturn pubKey_;\n> +\t}\n> +#endif\n> +\n> private:\n> \tstatic IPAManager *self_;\n> \n> diff --git a/src/apps/ipa-verify/main.cpp b/src/apps/ipa-verify/main.cpp\n> new file mode 100644\n> index 000000000000..76ba5073d25a\n> --- /dev/null\n> +++ b/src/apps/ipa-verify/main.cpp\n> @@ -0,0 +1,64 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board Oy\n> + *\n> + * ipa_verify.cpp - Verify signature on an IPA module\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/ipa_manager.h\"\n> +#include \"libcamera/internal/ipa_module.h\"\n> +\n> +using namespace libcamera;\n> +\n> +namespace {\n> +\n> +bool isSignatureValid(IPAModule *ipa)\n> +{\n> +\tFile file{ ipa->path() };\n> +\tif (!file.open(File::OpenModeFlag::ReadOnly))\n> +\t\treturn false;\n> +\n> +\tSpan data = file.map();\n> +\tif (data.empty())\n> +\t\treturn false;\n> +\n> +\treturn IPAManager::pubKey().verify(data, ipa->signature());\n> +}\n> +\n> +void usage(char *argv0)\n> +{\n> +\tstd::cout << \"Usage: \" << basename(argv0) << \" ipa_name.so\" << std::endl;\n> +\tstd::cout << std::endl;\n> +\tstd::cout << \"Verify the signature of an IPA module. The signature file ipa_name.so.sign is\" << std::endl;\n> +\tstd::cout << \"expected to be in the same directory as the IPA module.\" << std::endl;\n> +}\n> +\n> +} /* namespace */\n> +\n> +int main(int argc, char **argv)\n> +{\n> +\tif (argc != 2) {\n> +\t\tusage(argv[0]);\n> +\t\treturn EXIT_FAILURE;\n> +\t}\n> +\n> +\tIPAModule module{ argv[1] };\n> +\tif (!module.isValid()) {\n> +\t\tstd::cout << \"Invalid IPA module \" << argv[1] << std::endl;\n> +\t\treturn EXIT_FAILURE;\n> +\t}\n> +\n> +\tif (!isSignatureValid(&module)) {\n> +\t\tstd::cout << \"IPA module signature is invalid\" << std::endl;\n> +\t\treturn EXIT_FAILURE;\n> +\t}\n> +\n> +\tstd::cout << \"IPA module signature is valid\" << std::endl;\n> +\treturn 0;\n> +}\n> diff --git a/src/apps/ipa-verify/meson.build b/src/apps/ipa-verify/meson.build\n> new file mode 100644\n> index 000000000000..7fdda3b9af4b\n> --- /dev/null\n> +++ b/src/apps/ipa-verify/meson.build\n> @@ -0,0 +1,15 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +if not ipa_sign_module\n> + subdir_done()\n> +endif\n> +\n> +ipa_verify_sources = files([\n> + 'main.cpp',\n> +])\n> +\n> +ipa_verify = executable('ipa_verify', ipa_verify_sources,\n> + dependencies : [\n> + libcamera_private,\n> + ],\n> + install : false)\n> diff --git a/src/apps/meson.build b/src/apps/meson.build\n> index 099876356bd1..af632b9a7b0b 100644\n> --- a/src/apps/meson.build\n> +++ b/src/apps/meson.build\n> @@ -18,3 +18,5 @@ subdir('lc-compliance')\n> \n> subdir('cam')\n> subdir('qcam')\n> +\n> +subdir('ipa-verify')\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 030ef43fb994..6d18d09b019c 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -279,6 +279,19 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n> * found or if the IPA proxy fails to initialize\n> */\n> \n> +#if HAVE_IPA_PUBKEY\n> +/**\n> + * \\fn IPAManager::pubKey()\n> + * \\brief Retrieve the IPA module signing public key\n> + *\n> + * IPA module signature verification is normally handled internally by the\n> + * IPAManager class. This function is meant to be used by utilities that need to\n> + * verify signatures externally.\n> + *\n> + * \\return The IPA module signing public key\n> + */\n> +#endif\n> +\n> bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n> {\n> #if HAVE_IPA_PUBKEY","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4D49DBE08B\n\tfor ;\n\tMon, 8 May 2023 19:20:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA47360544;\n\tMon, 8 May 2023 21:20:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 309396053A\n\tfor ;\n\tMon, 8 May 2023 21:20:38 +0200 (CEST)","from [192.168.1.105] (unknown [103.251.226.10])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4FC1D755;\n\tMon, 8 May 2023 21:20:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683573639;\n\tbh=nSLL/mcyrEJr1GXmija0MebCwr3Euh+LERgpHzWdCA0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=r7DCBGNqkhaSBcGPGulbE6o+902JgHV0RpDOstZT6LhJiriCLN7Po5j9TIYGiy3T2\n\t1ql2sRVjoZ0Jcn2lOYCe4R/USwA3IGlPC87ssGTIdMLexWbX/fZD7SEMDg/vfCA9ZY\n\tZOStO9nNaTASzBaeAYXOo/mXzP5vKFtpnZE1TAKp+GPFcuBngfkd5di3k3bx1Mxiee\n\t+VLC5IXjFKHpSEKS7M5LhBUXmZmT7skQ9RQUoTZEP8mfDSClRJOToHTsm1dnV0lrD2\n\tk6MTpEhF6W0g2kprepCASLqR8OJUDqdhLi+HdvJ8b3qkljxWE5WaVa+sAWVAg3aMQN\n\t6OqIDJJcVBJjw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683573632;\n\tbh=nSLL/mcyrEJr1GXmija0MebCwr3Euh+LERgpHzWdCA0=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=t7+mcWPtHcFuD5QHADNqF1udTtVEx7p6TxpW2yJtIEFGXEIC1VwCjuOYoZGhqkHh1\n\tWdHtrV+S0OxmB1KEqh/tobIeqTp3YGLH+GSIf5XkZ4XIx399zMcOu+Dt2AUxyMszEp\n\tOjBtakCepidw84qGdp/r6CJf2e9XDoxKKl2Muroc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"t7+mcWPt\"; dkim-atps=neutral","Message-ID":"","Date":"Tue, 9 May 2023 00:50:30 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230506111025.18669-1-laurent.pinchart@ideasonboard.com>\n\t<20230506111025.18669-3-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20230506111025.18669-3-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v1 2/2] apps: Add ipa-verify\n\tapplication","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27072,"web_url":"https://patchwork.libcamera.org/comment/27072/","msgid":"<168359290297.2007737.16600970082299577439@Monstersaurus>","date":"2023-05-09T00:41:42","subject":"Re: [libcamera-devel] [PATCH v1 2/2] apps: Add ipa-verify\n\tapplication","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2023-05-06 12:10:25)\n> When packaging libcamera, distributions may break IPA module signatures\n> if the packaging process strips binaries. This can be fixed by resigning\n> the modules, but the process is error-prone.\n> \n> Add a command line ipa-verify utility that tests the signature on an IPA\n> module to help packagers. The tool takes a single argument, the path to\n> an IPA module shared object, and expects the signature file (.sign) to\n> be in the same directory.\n> \n> In order to access the public key needed for signature verification, add\n> a static function to the IPAManager class. As the class is internal to\n> libcamera, this doesn't affect the public API.\n\nBut requires this tool to be built with access to the private internal\nAPIs... I think that's ok in our build though, and this is a very\nspecific tool ... and certainly would warrant this being a 'utility'\nrather than an 'app' indeed.\n\n\n\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/ipa_manager.h | 7 +++\n> src/apps/ipa-verify/main.cpp | 64 ++++++++++++++++++++++++\n> src/apps/ipa-verify/meson.build | 15 ++++++\n> src/apps/meson.build | 2 +\n> src/libcamera/ipa_manager.cpp | 13 +++++\n> 5 files changed, 101 insertions(+)\n> create mode 100644 src/apps/ipa-verify/main.cpp\n> create mode 100644 src/apps/ipa-verify/meson.build\n> \n> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n> index 7f36e58e8bfa..bf823563c91c 100644\n> --- a/include/libcamera/internal/ipa_manager.h\n> +++ b/include/libcamera/internal/ipa_manager.h\n> @@ -47,6 +47,13 @@ public:\n> return proxy;\n> }\n> \n> +#if HAVE_IPA_PUBKEY\n> + static const PubKey &pubKey()\n> + {\n> + return pubKey_;\n> + }\n> +#endif\n> +\n> private:\n> static IPAManager *self_;\n> \n> diff --git a/src/apps/ipa-verify/main.cpp b/src/apps/ipa-verify/main.cpp\n> new file mode 100644\n> index 000000000000..76ba5073d25a\n> --- /dev/null\n> +++ b/src/apps/ipa-verify/main.cpp\n> @@ -0,0 +1,64 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board Oy\n> + *\n> + * ipa_verify.cpp - Verify signature on an IPA module\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/ipa_manager.h\"\n> +#include \"libcamera/internal/ipa_module.h\"\n> +\n> +using namespace libcamera;\n> +\n> +namespace {\n> +\n> +bool isSignatureValid(IPAModule *ipa)\n> +{\n> + File file{ ipa->path() };\n> + if (!file.open(File::OpenModeFlag::ReadOnly))\n> + return false;\n> +\n> + Span data = file.map();\n> + if (data.empty())\n> + return false;\n> +\n> + return IPAManager::pubKey().verify(data, ipa->signature());\n\nWhat happens if HAVE_IPA_PUBKEY is not defined?\n\nAdmitedly - there won't be much to check - but will this build? break?\nreport failure?\n\nI think I'd expect the tool to still exist but report something on the\noutput.\n\n\nAnyway, I'm actually very pleased to see such a tool. I'd carved up a\nscript to do the same but that relies on having the build tree, while\nthis could actually be installed to a target and validate a full install\nwhen needed. Much nicer.\n\n\n\n> +}\n> +\n> +void usage(char *argv0)\n> +{\n> + std::cout << \"Usage: \" << basename(argv0) << \" ipa_name.so\" << std::endl;\n> + std::cout << std::endl;\n> + std::cout << \"Verify the signature of an IPA module. The signature file ipa_name.so.sign is\" << std::endl;\n> + std::cout << \"expected to be in the same directory as the IPA module.\" << std::endl;\n> +}\n> +\n> +} /* namespace */\n> +\n> +int main(int argc, char **argv)\n> +{\n> + if (argc != 2) {\n> + usage(argv[0]);\n> + return EXIT_FAILURE;\n> + }\n> +\n> + IPAModule module{ argv[1] };\n> + if (!module.isValid()) {\n> + std::cout << \"Invalid IPA module \" << argv[1] << std::endl;\n> + return EXIT_FAILURE;\n> + }\n> +\n> + if (!isSignatureValid(&module)) {\n> + std::cout << \"IPA module signature is invalid\" << std::endl;\n> + return EXIT_FAILURE;\n> + }\n> +\n> + std::cout << \"IPA module signature is valid\" << std::endl;\n> + return 0;\n> +}\n> diff --git a/src/apps/ipa-verify/meson.build b/src/apps/ipa-verify/meson.build\n> new file mode 100644\n> index 000000000000..7fdda3b9af4b\n> --- /dev/null\n> +++ b/src/apps/ipa-verify/meson.build\n> @@ -0,0 +1,15 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +if not ipa_sign_module\n> + subdir_done()\n> +endif\n> +\n> +ipa_verify_sources = files([\n> + 'main.cpp',\n> +])\n> +\n> +ipa_verify = executable('ipa_verify', ipa_verify_sources,\n> + dependencies : [\n> + libcamera_private,\n> + ],\n> + install : false)\n> diff --git a/src/apps/meson.build b/src/apps/meson.build\n> index 099876356bd1..af632b9a7b0b 100644\n> --- a/src/apps/meson.build\n> +++ b/src/apps/meson.build\n> @@ -18,3 +18,5 @@ subdir('lc-compliance')\n> \n> subdir('cam')\n> subdir('qcam')\n> +\n> +subdir('ipa-verify')\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 030ef43fb994..6d18d09b019c 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -279,6 +279,19 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n> * found or if the IPA proxy fails to initialize\n> */\n> \n> +#if HAVE_IPA_PUBKEY\n> +/**\n> + * \\fn IPAManager::pubKey()\n> + * \\brief Retrieve the IPA module signing public key\n> + *\n> + * IPA module signature verification is normally handled internally by the\n> + * IPAManager class. This function is meant to be used by utilities that need to\n> + * verify signatures externally.\n> + *\n> + * \\return The IPA module signing public key\n> + */\n> +#endif\n> +\n> bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n> {\n> #if HAVE_IPA_PUBKEY\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7CE77BE08B\n\tfor ;\n\tTue, 9 May 2023 00:41:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0059561EAB;\n\tTue, 9 May 2023 02:41:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4013F60544\n\tfor ;\n\tTue, 9 May 2023 02:41:46 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BD2B24DB;\n\tTue, 9 May 2023 02:41:39 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683592908;\n\tbh=f19R8z3eEdgmeyiLMjJWvFfaCoRAcjzSn5wyTK/3MeE=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=ZSF26Q0Kp0//d1Ibojerk+P54QuiQZ7sFDF4tHuFCOYVk4BmBvNrG4jcLhYCxk1gB\n\tJqz3A7IT79egDX8361MdgEvElX5TSIYiJo49/ghLQxqJ/2Cq+kPHqzvpAH0k3OmQcL\n\tg1vyK7d8WrZlUbDBfi/uERsUof0m7tDoFXKL7544KyJ7VyIElkvGoIvIELx8xMvTQe\n\tLihyuUcqRg7enJnB6A8EtF8GA2rE2Wowtc5RnyEkozaMaHr/uBpMIdzznRoBMjZAks\n\t8bVWHp2zOFYpBrufP970V9FInxDsSa1KKFdKpOXKA9OLwB7a03PEQsTZ5ai9hXYjxa\n\toyPVWprUjplGw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683592899;\n\tbh=f19R8z3eEdgmeyiLMjJWvFfaCoRAcjzSn5wyTK/3MeE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=LFY9ujmxFjIXfnNKdN3SL698A4fm3SUy6U8yhEQB5oCr13BVrRl7nyUVrzZHDotLE\n\tfUJhegS/nwPMmz/3DVowVPae4TbO01Crn2zcInX023cIoZ/oPrPd+qB/osFdBZighR\n\trWdvNeMSJhwy5UFrDnyUuYpm/oQ9DAtrlzPuvlSc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LFY9ujmx\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230506111025.18669-3-laurent.pinchart@ideasonboard.com>","References":"<20230506111025.18669-1-laurent.pinchart@ideasonboard.com>\n\t<20230506111025.18669-3-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 09 May 2023 01:41:42 +0100","Message-ID":"<168359290297.2007737.16600970082299577439@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 2/2] apps: Add ipa-verify\n\tapplication","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27073,"web_url":"https://patchwork.libcamera.org/comment/27073/","msgid":"<20230509033407.GA30543@pendragon.ideasonboard.com>","date":"2023-05-09T03:34:07","subject":"Re: [libcamera-devel] [PATCH v1 2/2] apps: Add ipa-verify\n\tapplication","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, May 09, 2023 at 01:41:42AM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2023-05-06 12:10:25)\n> > When packaging libcamera, distributions may break IPA module signatures\n> > if the packaging process strips binaries. This can be fixed by resigning\n> > the modules, but the process is error-prone.\n> > \n> > Add a command line ipa-verify utility that tests the signature on an IPA\n> > module to help packagers. The tool takes a single argument, the path to\n> > an IPA module shared object, and expects the signature file (.sign) to\n> > be in the same directory.\n> > \n> > In order to access the public key needed for signature verification, add\n> > a static function to the IPAManager class. As the class is internal to\n> > libcamera, this doesn't affect the public API.\n> \n> But requires this tool to be built with access to the private internal\n> APIs... I think that's ok in our build though, and this is a very\n> specific tool ... and certainly would warrant this being a 'utility'\n> rather than an 'app' indeed.\n\nAny advice on how to make it so would be appreciated :-)\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/ipa_manager.h | 7 +++\n> > src/apps/ipa-verify/main.cpp | 64 ++++++++++++++++++++++++\n> > src/apps/ipa-verify/meson.build | 15 ++++++\n> > src/apps/meson.build | 2 +\n> > src/libcamera/ipa_manager.cpp | 13 +++++\n> > 5 files changed, 101 insertions(+)\n> > create mode 100644 src/apps/ipa-verify/main.cpp\n> > create mode 100644 src/apps/ipa-verify/meson.build\n> > \n> > diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n> > index 7f36e58e8bfa..bf823563c91c 100644\n> > --- a/include/libcamera/internal/ipa_manager.h\n> > +++ b/include/libcamera/internal/ipa_manager.h\n> > @@ -47,6 +47,13 @@ public:\n> > return proxy;\n> > }\n> > \n> > +#if HAVE_IPA_PUBKEY\n> > + static const PubKey &pubKey()\n> > + {\n> > + return pubKey_;\n> > + }\n> > +#endif\n> > +\n> > private:\n> > static IPAManager *self_;\n> > \n> > diff --git a/src/apps/ipa-verify/main.cpp b/src/apps/ipa-verify/main.cpp\n> > new file mode 100644\n> > index 000000000000..76ba5073d25a\n> > --- /dev/null\n> > +++ b/src/apps/ipa-verify/main.cpp\n> > @@ -0,0 +1,64 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Ideas on Board Oy\n> > + *\n> > + * ipa_verify.cpp - Verify signature on an IPA module\n> > + */\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/internal/ipa_manager.h\"\n> > +#include \"libcamera/internal/ipa_module.h\"\n> > +\n> > +using namespace libcamera;\n> > +\n> > +namespace {\n> > +\n> > +bool isSignatureValid(IPAModule *ipa)\n> > +{\n> > + File file{ ipa->path() };\n> > + if (!file.open(File::OpenModeFlag::ReadOnly))\n> > + return false;\n> > +\n> > + Span data = file.map();\n> > + if (data.empty())\n> > + return false;\n> > +\n> > + return IPAManager::pubKey().verify(data, ipa->signature());\n> \n> What happens if HAVE_IPA_PUBKEY is not defined?\n> \n> Admitedly - there won't be much to check - but will this build? break?\n> report failure?\n> \n> I think I'd expect the tool to still exist but report something on the\n> output.\n\nAt the moment it's simply not compiled. I can make it report that IPA\nsignatures are not available if you prefer so.\n\n> Anyway, I'm actually very pleased to see such a tool. I'd carved up a\n> script to do the same but that relies on having the build tree, while\n> this could actually be installed to a target and validate a full install\n> when needed. Much nicer.\n\nI envision this utility being used to validate the packaging process,\nnot being installed as part of the normal libcamera package. It's\nactually a good question as to whether it would be most useful to run it\non the host or the target. At the moment it's compiled as a binary for\nthe target, we could switch that with `native : true`.\n\n> > +}\n> > +\n> > +void usage(char *argv0)\n> > +{\n> > + std::cout << \"Usage: \" << basename(argv0) << \" ipa_name.so\" << std::endl;\n> > + std::cout << std::endl;\n> > + std::cout << \"Verify the signature of an IPA module. The signature file ipa_name.so.sign is\" << std::endl;\n> > + std::cout << \"expected to be in the same directory as the IPA module.\" << std::endl;\n> > +}\n> > +\n> > +} /* namespace */\n> > +\n> > +int main(int argc, char **argv)\n> > +{\n> > + if (argc != 2) {\n> > + usage(argv[0]);\n> > + return EXIT_FAILURE;\n> > + }\n> > +\n> > + IPAModule module{ argv[1] };\n> > + if (!module.isValid()) {\n> > + std::cout << \"Invalid IPA module \" << argv[1] << std::endl;\n> > + return EXIT_FAILURE;\n> > + }\n> > +\n> > + if (!isSignatureValid(&module)) {\n> > + std::cout << \"IPA module signature is invalid\" << std::endl;\n> > + return EXIT_FAILURE;\n> > + }\n> > +\n> > + std::cout << \"IPA module signature is valid\" << std::endl;\n> > + return 0;\n> > +}\n> > diff --git a/src/apps/ipa-verify/meson.build b/src/apps/ipa-verify/meson.build\n> > new file mode 100644\n> > index 000000000000..7fdda3b9af4b\n> > --- /dev/null\n> > +++ b/src/apps/ipa-verify/meson.build\n> > @@ -0,0 +1,15 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +if not ipa_sign_module\n> > + subdir_done()\n> > +endif\n> > +\n> > +ipa_verify_sources = files([\n> > + 'main.cpp',\n> > +])\n> > +\n> > +ipa_verify = executable('ipa_verify', ipa_verify_sources,\n> > + dependencies : [\n> > + libcamera_private,\n> > + ],\n> > + install : false)\n> > diff --git a/src/apps/meson.build b/src/apps/meson.build\n> > index 099876356bd1..af632b9a7b0b 100644\n> > --- a/src/apps/meson.build\n> > +++ b/src/apps/meson.build\n> > @@ -18,3 +18,5 @@ subdir('lc-compliance')\n> > \n> > subdir('cam')\n> > subdir('qcam')\n> > +\n> > +subdir('ipa-verify')\n> > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> > index 030ef43fb994..6d18d09b019c 100644\n> > --- a/src/libcamera/ipa_manager.cpp\n> > +++ b/src/libcamera/ipa_manager.cpp\n> > @@ -279,6 +279,19 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n> > * found or if the IPA proxy fails to initialize\n> > */\n> > \n> > +#if HAVE_IPA_PUBKEY\n> > +/**\n> > + * \\fn IPAManager::pubKey()\n> > + * \\brief Retrieve the IPA module signing public key\n> > + *\n> > + * IPA module signature verification is normally handled internally by the\n> > + * IPAManager class. This function is meant to be used by utilities that need to\n> > + * verify signatures externally.\n> > + *\n> > + * \\return The IPA module signing public key\n> > + */\n> > +#endif\n> > +\n> > bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const\n> > {\n> > #if HAVE_IPA_PUBKEY","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4DB6CBD16B\n\tfor ;\n\tTue, 9 May 2023 03:33:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C84F2627DC;\n\tTue, 9 May 2023 05:33:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A5F7560493\n\tfor ;\n\tTue, 9 May 2023 05:33:55 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126090219015.bbtec.net\n\t[126.90.219.15])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 33F447CE;\n\tTue, 9 May 2023 05:33:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683603237;\n\tbh=1dg0TTnBUSlF9Y+S/k/oOe6f/adkKGyznTfJU6Ai1mo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=xZ1gSMvHA6MBF5NDeX4M2jTv+ZTx5terDHXB+iOynZ1q4B5xx5C/VM4N13HROc0XD\n\tIm8uCm5Nrkhm7gUFwyD4onuDfVT/xhCJ49r0P0ozT13HwJYEbBjFopmWwWZ7PICICk\n\tBtEVLAE9MAUmHkis4WmDAgrlRUDQAkg9DxuVYiUy6Ot0Y23ug+NovnJZC85yMKQP7P\n\t7/Uf1r1zOehs69rRxY2214qxNaK/AO/ONNe4BvRWCvT8vpkgoxTdBZrb7gFk+IdXXS\n\tp5sAxR2YdY7Gw9KWZx+FTNIQ8mNHVXbGRFlBXmqzbB20/PlrOlwgHMpIWBTzzvK2T9\n\tgOmsdWwXyK1Kw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683603229;\n\tbh=1dg0TTnBUSlF9Y+S/k/oOe6f/adkKGyznTfJU6Ai1mo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Gr1jkmiiq3FlUPhV4mfQ2ExZHwRbgNzD1+XLRVL4g3v3x9aXSbQFenG7vEB2797Nt\n\tm3kSLKQ4FckF3TtGn451ldhTw1eg4vpWGOyY3O/CYG+efVbcC8plK7VdGZ1XOm5w9x\n\tZSvPCop5MV+24x6qO9vpL3vglXEh7Znr3zYkSojA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Gr1jkmii\"; dkim-atps=neutral","Date":"Tue, 9 May 2023 06:34:07 +0300","To":"Kieran Bingham ","Message-ID":"<20230509033407.GA30543@pendragon.ideasonboard.com>","References":"<20230506111025.18669-1-laurent.pinchart@ideasonboard.com>\n\t<20230506111025.18669-3-laurent.pinchart@ideasonboard.com>\n\t<168359290297.2007737.16600970082299577439@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<168359290297.2007737.16600970082299577439@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v1 2/2] apps: Add ipa-verify\n\tapplication","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]