[{"id":4457,"web_url":"https://patchwork.libcamera.org/comment/4457/","msgid":"<6bb37a25-c07e-06d5-5b2a-41933c6bd4ea@ideasonboard.com>","date":"2020-04-16T12:16:01","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Make IPA module\n\tsigning optional","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 15/04/2020 20:37, Laurent Pinchart wrote:\n> The IPA module signing mechanism relies on openssl to generate keys and\n> sign the module. If openssl is not found on the system, the build will\n> fail. Make the dependency optional by detecting openssl, and skip\n> generation of signatures if openssl isn't found.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nIf I had more time I might play around with different options here - but\ngiven this works ... lets get it in and move on.\n\nSome small notes and possible options below but:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> ---\n>  src/ipa/meson.build                 |  4 +++-\n>  src/ipa/rkisp1/meson.build          | 14 ++++++++------\n>  src/ipa/vimc/meson.build            | 14 ++++++++------\n>  src/libcamera/include/ipa_manager.h |  2 ++\n>  src/libcamera/ipa_manager.cpp       |  4 ++++\n>  src/libcamera/ipa_pub_key.cpp.in    |  4 +++-\n>  src/libcamera/meson.build           | 16 +++++++++-------\n>  src/meson.build                     | 15 +++++++++++----\n>  8 files changed, 48 insertions(+), 25 deletions(-)\n> \n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index cb4e3ab3388f..96d3ce79ffe9 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -10,7 +10,9 @@ config_h.set('IPA_MODULE_DIR',\n>  \n>  subdir('libipa')\n>  \n> -ipa_sign = find_program('ipa-sign.sh')\n> +if ipa_sign_module\n> +    ipa_sign = find_program('ipa-sign.sh')\n> +endif\n\nThis file will exist regardless, so making this conditional on\nipa_sign_module feels like clutter.\n\nSure it might not be used though ... Does meson complain if a variable\nis defined but not used?\n\n\n>  ipas = ['rkisp1', 'vimc']\n>  \n> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n> index 6ccadcfbbe64..247d0429b49c 100644\n> --- a/src/ipa/rkisp1/meson.build\n> +++ b/src/ipa/rkisp1/meson.build\n> @@ -9,9 +9,11 @@ mod = shared_module(ipa_name,\n>                      install : true,\n>                      install_dir : ipa_install_dir)\n>  \n> -custom_target(ipa_name + '.so.sign',\n> -              input : mod,\n> -              output : ipa_name + '.so.sign',\n> -              command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> -              install : true,\n> -              install_dir : ipa_install_dir)\n> +if ipa_sign_module\n> +    custom_target(ipa_name + '.so.sign',\n> +                  input : mod,\n> +                  output : ipa_name + '.so.sign',\n> +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> +                  install : true,\n> +                  install_dir : ipa_install_dir)\n> +endif\n> diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build\n> index 3c932aa7aaad..a354096d8496 100644\n> --- a/src/ipa/vimc/meson.build\n> +++ b/src/ipa/vimc/meson.build\n> @@ -9,9 +9,11 @@ mod = shared_module(ipa_name,\n>                      install : true,\n>                      install_dir : ipa_install_dir)\n>  \n> -custom_target(ipa_name + '.so.sign',\n> -              input : mod,\n> -              output : ipa_name + '.so.sign',\n> -              command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> -              install : true,\n> -              install_dir : ipa_install_dir)\n> +if ipa_sign_module\n> +    custom_target(ipa_name + '.so.sign',\n> +                  input : mod,\n> +                  output : ipa_name + '.so.sign',\n> +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> +                  install : true,\n> +                  install_dir : ipa_install_dir)\n> +endif\n> diff --git a/src/libcamera/include/ipa_manager.h b/src/libcamera/include/ipa_manager.h\n> index 0b5fd2ac1f12..6165efb8b145 100644\n> --- a/src/libcamera/include/ipa_manager.h\n> +++ b/src/libcamera/include/ipa_manager.h\n> @@ -40,8 +40,10 @@ private:\n>  \n>  \tbool isSignatureValid(IPAModule *ipa) const;\n>  \n> +#if HAVE_IPA_PUBKEY\n\nI wonder if this could be 'cleaner' using the linker and weak symbols or\nsuch ... but that would take time we don't have to play around with...\nso this should be ok.\n\nIt's all internal, so it's not going to affect any ABI I don't believe.\n\n\n>  \tstatic const uint8_t publicKeyData_[];\n>  \tstatic const PubKey pubKey_;\n> +#endif\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 7de1404eebdd..50b6792d6cce 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -304,6 +304,7 @@ std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,\n>  \n>  bool IPAManager::isSignatureValid(IPAModule *ipa) const\n>  {\n> +#if HAVE_IPA_PUBKEY\n>  \tFile file{ ipa->path() };\n>  \tif (!file.open(File::ReadOnly))\n>  \t\treturn false;\n> @@ -319,6 +320,9 @@ bool IPAManager::isSignatureValid(IPAModule *ipa) const\n>  \t\t<< (valid ? \"valid\" : \"not valid\");\n>  \n>  \treturn valid;\n> +#else\n> +\treturn false;\n> +#endif\n>  }\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_pub_key.cpp.in b/src/libcamera/ipa_pub_key.cpp.in\n> index e1fe287c160e..7ffc1e24d67b 100644\n> --- a/src/libcamera/ipa_pub_key.cpp.in\n> +++ b/src/libcamera/ipa_pub_key.cpp.in\n> @@ -2,7 +2,7 @@\n>  /*\n>   * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>   *\n> - * ipa_key.cpp - IPA module signing public key\n> + * ipa_pub_key.cpp - IPA module signing public key\n\nUnrelated change, but keep it.\n\n\n>   *\n>   * This file is auto-generated. Do not edit.\n>   */\n> @@ -11,10 +11,12 @@\n>  \n>  namespace libcamera {\n>  \n> +#if HAVE_IPA_PUBKEY\n>  const uint8_t IPAManager::publicKeyData_[] = {\n>  \t${ipa_key}\n>  };\n>  \n>  const PubKey IPAManager::pubKey_{ { IPAManager::publicKeyData_ } };\n> +#endif\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index c502450c4b2d..dcd2fb4900e6 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -101,13 +101,15 @@ version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n>  \n>  libcamera_sources += version_cpp\n>  \n> -gen_ipa_pub_key = files('gen-ipa-pub-key.py')\n> -ipa_pub_key_cpp = custom_target('ipa_pub_key_cpp',\n> -                                input : [ ipa_priv_key, 'ipa_pub_key.cpp.in' ],\n> -                                output : 'ipa_pub_key.cpp',\n> -                                command : [ gen_ipa_pub_key, '@INPUT@', '@OUTPUT@' ])\n> -\n> -libcamera_sources += ipa_pub_key_cpp\n> +if ipa_sign_module\n> +    gen_ipa_pub_key = files('gen-ipa-pub-key.py')\n> +    ipa_pub_key_cpp = custom_target('ipa_pub_key_cpp',\n> +                                    input : [ ipa_priv_key, 'ipa_pub_key.cpp.in' ],\n> +                                    output : 'ipa_pub_key.cpp',\n> +                                    command : [ gen_ipa_pub_key, '@INPUT@', '@OUTPUT@' ])\n> +\n> +    libcamera_sources += ipa_pub_key_cpp\n> +endif\n>  \n>  libcamera_deps = [\n>      libatomic,\n> diff --git a/src/meson.build b/src/meson.build\n> index dc0e0c82b900..296682758613 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -2,10 +2,17 @@ if get_option('android')\n>      subdir('android')\n>  endif\n>  \n> -ipa_gen_priv_key = find_program('ipa/gen-ipa-priv-key.sh')\n> -ipa_priv_key = custom_target('ipa-priv-key',\n> -                             output : [ 'ipa-priv-key.pem' ],\n> -                             command : [ ipa_gen_priv_key, '@OUTPUT@' ])\n> +openssl = find_program('openssl', required : false)\n\n\n> +if openssl.found()\n\nI'd be tempted to make this also dependant upon whether the gnutls was\nfound or not, as if we can't validate the key - there's not much point\nin signing it ...\n\n> +    ipa_gen_priv_key = find_program('ipa/gen-ipa-priv-key.sh')\n> +    ipa_priv_key = custom_target('ipa-priv-key',\n> +                                 output : [ 'ipa-priv-key.pem' ],\n> +                                 command : [ ipa_gen_priv_key, '@OUTPUT@' ])\n> +    config_h.set('HAVE_IPA_PUBKEY', 1)\n> +    ipa_sign_module = true\n> +else\n> +    ipa_sign_module = false\n> +endif\n>  \n>  subdir('libcamera')\n>  subdir('ipa')\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["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 5D53060403\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Apr 2020 14:16:05 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E0E9B97D;\n\tThu, 16 Apr 2020 14:16:04 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nVrTKTNz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587039365;\n\tbh=Rq1XQAAmcB/VYupNWz4baftDkDSJ1JB+TFRVdfH0KfY=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=nVrTKTNzR+byO5Uy2wzprIcwseEUojzT58V6jxwE8nDwdoaCbvUXwTl9op6bgjfa4\n\tfKkoc5KeobdR7K7E9fv+3hLmEXvhROxuvR5E+OO6MGWqYAlDZYm9p7qrDaDHu1snpv\n\tgxkjJzu1nrUXnpbgStT3DLyd3JOmNyPUSI0nLpSY=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200415193719.783-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<6bb37a25-c07e-06d5-5b2a-41933c6bd4ea@ideasonboard.com>","Date":"Thu, 16 Apr 2020 13:16:01 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200415193719.783-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Make IPA module\n\tsigning optional","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 16 Apr 2020 12:16:05 -0000"}},{"id":4459,"web_url":"https://patchwork.libcamera.org/comment/4459/","msgid":"<20200416143635.GB4796@pendragon.ideasonboard.com>","date":"2020-04-16T14:36:35","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Make IPA module\n\tsigning optional","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Thu, Apr 16, 2020 at 01:16:01PM +0100, Kieran Bingham wrote:\n> On 15/04/2020 20:37, Laurent Pinchart wrote:\n> > The IPA module signing mechanism relies on openssl to generate keys and\n> > sign the module. If openssl is not found on the system, the build will\n> > fail. Make the dependency optional by detecting openssl, and skip\n> > generation of signatures if openssl isn't found.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> If I had more time I might play around with different options here - but\n> given this works ... lets get it in and move on.\n> \n> Some small notes and possible options below but:\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > ---\n> >  src/ipa/meson.build                 |  4 +++-\n> >  src/ipa/rkisp1/meson.build          | 14 ++++++++------\n> >  src/ipa/vimc/meson.build            | 14 ++++++++------\n> >  src/libcamera/include/ipa_manager.h |  2 ++\n> >  src/libcamera/ipa_manager.cpp       |  4 ++++\n> >  src/libcamera/ipa_pub_key.cpp.in    |  4 +++-\n> >  src/libcamera/meson.build           | 16 +++++++++-------\n> >  src/meson.build                     | 15 +++++++++++----\n> >  8 files changed, 48 insertions(+), 25 deletions(-)\n> > \n> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> > index cb4e3ab3388f..96d3ce79ffe9 100644\n> > --- a/src/ipa/meson.build\n> > +++ b/src/ipa/meson.build\n> > @@ -10,7 +10,9 @@ config_h.set('IPA_MODULE_DIR',\n> >  \n> >  subdir('libipa')\n> >  \n> > -ipa_sign = find_program('ipa-sign.sh')\n> > +if ipa_sign_module\n> > +    ipa_sign = find_program('ipa-sign.sh')\n> > +endif\n> \n> This file will exist regardless, so making this conditional on\n> ipa_sign_module feels like clutter.\n> \n> Sure it might not be used though ... Does meson complain if a variable\n> is defined but not used?\n\nIt doesn't, so I'll fix this.\n\n> >  ipas = ['rkisp1', 'vimc']\n> >  \n> > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n> > index 6ccadcfbbe64..247d0429b49c 100644\n> > --- a/src/ipa/rkisp1/meson.build\n> > +++ b/src/ipa/rkisp1/meson.build\n> > @@ -9,9 +9,11 @@ mod = shared_module(ipa_name,\n> >                      install : true,\n> >                      install_dir : ipa_install_dir)\n> >  \n> > -custom_target(ipa_name + '.so.sign',\n> > -              input : mod,\n> > -              output : ipa_name + '.so.sign',\n> > -              command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> > -              install : true,\n> > -              install_dir : ipa_install_dir)\n> > +if ipa_sign_module\n> > +    custom_target(ipa_name + '.so.sign',\n> > +                  input : mod,\n> > +                  output : ipa_name + '.so.sign',\n> > +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> > +                  install : true,\n> > +                  install_dir : ipa_install_dir)\n> > +endif\n> > diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build\n> > index 3c932aa7aaad..a354096d8496 100644\n> > --- a/src/ipa/vimc/meson.build\n> > +++ b/src/ipa/vimc/meson.build\n> > @@ -9,9 +9,11 @@ mod = shared_module(ipa_name,\n> >                      install : true,\n> >                      install_dir : ipa_install_dir)\n> >  \n> > -custom_target(ipa_name + '.so.sign',\n> > -              input : mod,\n> > -              output : ipa_name + '.so.sign',\n> > -              command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> > -              install : true,\n> > -              install_dir : ipa_install_dir)\n> > +if ipa_sign_module\n> > +    custom_target(ipa_name + '.so.sign',\n> > +                  input : mod,\n> > +                  output : ipa_name + '.so.sign',\n> > +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> > +                  install : true,\n> > +                  install_dir : ipa_install_dir)\n> > +endif\n> > diff --git a/src/libcamera/include/ipa_manager.h b/src/libcamera/include/ipa_manager.h\n> > index 0b5fd2ac1f12..6165efb8b145 100644\n> > --- a/src/libcamera/include/ipa_manager.h\n> > +++ b/src/libcamera/include/ipa_manager.h\n> > @@ -40,8 +40,10 @@ private:\n> >  \n> >  \tbool isSignatureValid(IPAModule *ipa) const;\n> >  \n> > +#if HAVE_IPA_PUBKEY\n> \n> I wonder if this could be 'cleaner' using the linker and weak symbols or\n> such ... but that would take time we don't have to play around with...\n> so this should be ok.\n> \n> It's all internal, so it's not going to affect any ABI I don't believe.\n\nWeak symbols could be a good idea, yes. Let's explore it on top.\n\n> >  \tstatic const uint8_t publicKeyData_[];\n> >  \tstatic const PubKey pubKey_;\n> > +#endif\n> >  };\n> >  \n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> > index 7de1404eebdd..50b6792d6cce 100644\n> > --- a/src/libcamera/ipa_manager.cpp\n> > +++ b/src/libcamera/ipa_manager.cpp\n> > @@ -304,6 +304,7 @@ std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,\n> >  \n> >  bool IPAManager::isSignatureValid(IPAModule *ipa) const\n> >  {\n> > +#if HAVE_IPA_PUBKEY\n> >  \tFile file{ ipa->path() };\n> >  \tif (!file.open(File::ReadOnly))\n> >  \t\treturn false;\n> > @@ -319,6 +320,9 @@ bool IPAManager::isSignatureValid(IPAModule *ipa) const\n> >  \t\t<< (valid ? \"valid\" : \"not valid\");\n> >  \n> >  \treturn valid;\n> > +#else\n> > +\treturn false;\n> > +#endif\n> >  }\n> >  \n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/ipa_pub_key.cpp.in b/src/libcamera/ipa_pub_key.cpp.in\n> > index e1fe287c160e..7ffc1e24d67b 100644\n> > --- a/src/libcamera/ipa_pub_key.cpp.in\n> > +++ b/src/libcamera/ipa_pub_key.cpp.in\n> > @@ -2,7 +2,7 @@\n> >  /*\n> >   * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >   *\n> > - * ipa_key.cpp - IPA module signing public key\n> > + * ipa_pub_key.cpp - IPA module signing public key\n> \n> Unrelated change, but keep it.\n\nI forgot to mention it in the commit message, sorry.\n\n> >   *\n> >   * This file is auto-generated. Do not edit.\n> >   */\n> > @@ -11,10 +11,12 @@\n> >  \n> >  namespace libcamera {\n> >  \n> > +#if HAVE_IPA_PUBKEY\n> >  const uint8_t IPAManager::publicKeyData_[] = {\n> >  \t${ipa_key}\n> >  };\n> >  \n> >  const PubKey IPAManager::pubKey_{ { IPAManager::publicKeyData_ } };\n> > +#endif\n> >  \n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index c502450c4b2d..dcd2fb4900e6 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -101,13 +101,15 @@ version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n> >  \n> >  libcamera_sources += version_cpp\n> >  \n> > -gen_ipa_pub_key = files('gen-ipa-pub-key.py')\n> > -ipa_pub_key_cpp = custom_target('ipa_pub_key_cpp',\n> > -                                input : [ ipa_priv_key, 'ipa_pub_key.cpp.in' ],\n> > -                                output : 'ipa_pub_key.cpp',\n> > -                                command : [ gen_ipa_pub_key, '@INPUT@', '@OUTPUT@' ])\n> > -\n> > -libcamera_sources += ipa_pub_key_cpp\n> > +if ipa_sign_module\n> > +    gen_ipa_pub_key = files('gen-ipa-pub-key.py')\n> > +    ipa_pub_key_cpp = custom_target('ipa_pub_key_cpp',\n> > +                                    input : [ ipa_priv_key, 'ipa_pub_key.cpp.in' ],\n> > +                                    output : 'ipa_pub_key.cpp',\n> > +                                    command : [ gen_ipa_pub_key, '@INPUT@', '@OUTPUT@' ])\n> > +\n> > +    libcamera_sources += ipa_pub_key_cpp\n> > +endif\n> >  \n> >  libcamera_deps = [\n> >      libatomic,\n> > diff --git a/src/meson.build b/src/meson.build\n> > index dc0e0c82b900..296682758613 100644\n> > --- a/src/meson.build\n> > +++ b/src/meson.build\n> > @@ -2,10 +2,17 @@ if get_option('android')\n> >      subdir('android')\n> >  endif\n> >  \n> > -ipa_gen_priv_key = find_program('ipa/gen-ipa-priv-key.sh')\n> > -ipa_priv_key = custom_target('ipa-priv-key',\n> > -                             output : [ 'ipa-priv-key.pem' ],\n> > -                             command : [ ipa_gen_priv_key, '@OUTPUT@' ])\n> > +openssl = find_program('openssl', required : false)\n> \n> \n> > +if openssl.found()\n> \n> I'd be tempted to make this also dependant upon whether the gnutls was\n> found or not, as if we can't validate the key - there's not much point\n> in signing it ...\n\nI agree they should be bundled. However, there are more options than\nopenssl and gnutls that we probably want to look into. Or maybe look\ninto how to use certtool from gnutls on the host side, and openssl on\nthe target side, to have more options for users. I thus expect this to\nbe reworked at that time.\n\n> > +    ipa_gen_priv_key = find_program('ipa/gen-ipa-priv-key.sh')\n> > +    ipa_priv_key = custom_target('ipa-priv-key',\n> > +                                 output : [ 'ipa-priv-key.pem' ],\n> > +                                 command : [ ipa_gen_priv_key, '@OUTPUT@' ])\n> > +    config_h.set('HAVE_IPA_PUBKEY', 1)\n> > +    ipa_sign_module = true\n> > +else\n> > +    ipa_sign_module = false\n> > +endif\n> >  \n> >  subdir('libcamera')\n> >  subdir('ipa')","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 67E9560403\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Apr 2020 16:36:48 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C55A797D;\n\tThu, 16 Apr 2020 16:36:47 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vfmYjwEu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587047808;\n\tbh=mfgj8J2P2moR+dHzgbvXrOE51B1RQ02cBrPAUbqdvtQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vfmYjwEuhuYArEvaWo/5BD1OAPW6KzZ/2y4BVlX8iwtu87aS6ubyV/fBVk4RNNqLO\n\tLNKirpdFm8SKWoybBIYgWeDgjXn2uw54CZ1i4EsSVnzx2F9ne5QLuuyEDkY8wlNSD7\n\tD2Nf1cvih3AVWiPQE0F4+wZGZvX0jKwYtMv3lLws=","Date":"Thu, 16 Apr 2020 17:36:35 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200416143635.GB4796@pendragon.ideasonboard.com>","References":"<20200415193719.783-1-laurent.pinchart@ideasonboard.com>\n\t<6bb37a25-c07e-06d5-5b2a-41933c6bd4ea@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6bb37a25-c07e-06d5-5b2a-41933c6bd4ea@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: Make IPA module\n\tsigning optional","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 16 Apr 2020 14:36:48 -0000"}}]