[{"id":24981,"web_url":"https://patchwork.libcamera.org/comment/24981/","msgid":"<166314325272.3298829.8543568893170435258@Monstersaurus>","date":"2022-09-14T08:14:12","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa_manager: Fix build\n\twithout openssl","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Matthias Fend via libcamera-devel (2022-09-14 08:00:25)\n> Since commit bedef55d9500 (\"libcamera: pub_key: Gracefully handle failures\n> to load public key'\") the build will fail if openssl is not found on the\n> host system.\n> Use the existing HAVE_IPA_PUBKEY define to avoid accessing pubKey_ which\n> does not exist when building without openssl.\n> \n> Signed-off-by: Matthias Fend \n> ---\n> src/libcamera/ipa_manager.cpp | 2 ++\n> 1 file changed, 2 insertions(+)\n> \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 2f96a207..030ef43f 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -109,8 +109,10 @@ IPAManager::IPAManager()\n> LOG(IPAManager, Fatal)\n> << \"Multiple IPAManager objects are not allowed\";\n> \n> +#if HAVE_IPA_PUBKEY\n\nIf we have to add a #if here, it feels to me like the implementation\nfor handling the key itself isn't right.\n\nThe alternative is to construct an invalid public key at \nsrc/libcamera/ipa_pub_key.cpp.in to ensure we always have a key.. but\nI'm not sure that sounds a lot better either...\n\nThis is limited to the IPA manager which knows about the keys anyway, so\n... worst case I could see this as a solution ... but I think I would\nalways want to push against ifdeffery spreading\n\nCould you check if it's reasonable to instantiate a pubKey_ which isn't\nvalid please?\n\nBut given here in ipa_manager - we already use #if HAVE_IPA_PUBKEY in\nIPAManager::isSignatureValid() I think I'd let this through.\n\n\nReviewed-by: Kieran Bingham \n\n> if (!pubKey_.isValid())\n> LOG(IPAManager, Warning) << \"Public key not valid\";\n> +#endif\n> \n> unsigned int ipaCount = 0;\n> \n> -- \n> 2.25.1\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 DA69AC3272\n\tfor ;\n\tWed, 14 Sep 2022 08:14:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 131DC61FA1;\n\tWed, 14 Sep 2022 10:14:18 +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 A2AE26099B\n\tfor ;\n\tWed, 14 Sep 2022 10:14:15 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C57F305;\n\tWed, 14 Sep 2022 10:14:15 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663143258;\n\tbh=XV2w+TaS61d4AYwxbUH/V0jREyvVlERHST7UmR7yWwg=;\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=IdFVu/jmnOfoCs2GgQvowcIctzlHBQezc1Q71toZMmfeyybjAqX83ACY/ZNVl7hl0\n\tutJ2sacgK0dVTUbpUxGdr1aeFL4NaLBpi8P+i26D9QcWWaohneGAxIeOnX5EtOoPJ4\n\tbN7pMmZBb1XGjdZFRqMlQeuE8V6MoEa3bZ4kn+uuvRiDPMEu1gDv/2Zw+DVLXeXIWx\n\tb823Ol5WT9C6hNXkRNXEjqDVVvkeErHkkkFsfGfrNHCo+JvaTtCmPUqbyoKYxGbkBj\n\tiFhZrmsKdoaT+mn7ya3qqHRiFYKnPUU4VjkCnWqLIgWfAJLBxl9k3Wz6uBOCUwc1xv\n\tWIEcMc90OBA3w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663143255;\n\tbh=XV2w+TaS61d4AYwxbUH/V0jREyvVlERHST7UmR7yWwg=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=KaV3zieRAt2uS+cMFXs2igY/Ih1pyGdHhHx+kOmGBYZ/9wgSMbS2JNJQA8i7OmjMi\n\tDXnIqHBMAQJrY+ymNrCmlP7WkTgFvRqwxwu83Askp8UWhqM4jQwNSUTYWq8F/sdzYD\n\tsFvbij5SN42J6Q7tyL9RofZdAia8vSUzSqfmilEw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"KaV3zieR\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220914070025.27540-1-matthias.fend@emfend.at>","References":"<20220914070025.27540-1-matthias.fend@emfend.at>","To":"Matthias Fend ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 14 Sep 2022 09:14:12 +0100","Message-ID":"<166314325272.3298829.8543568893170435258@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa_manager: Fix build\n\twithout openssl","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":24984,"web_url":"https://patchwork.libcamera.org/comment/24984/","msgid":"<408c9b17-3cc5-71fd-0b26-00fb52c2e7e4@ideasonboard.com>","date":"2022-09-14T09:29:18","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa_manager: Fix build\n\twithout openssl","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi  Matthias, Kieran\n\nOn 9/14/22 1:44 PM, Kieran Bingham via libcamera-devel wrote:\n> Quoting Matthias Fend via libcamera-devel (2022-09-14 08:00:25)\n>> Since commit bedef55d9500 (\"libcamera: pub_key: Gracefully handle failures\n>> to load public key'\") the build will fail if openssl is not found on the\n>> host system.\n>> Use the existing HAVE_IPA_PUBKEY define to avoid accessing pubKey_ which\n>> does not exist when building without openssl.\n>>\n>> Signed-off-by: Matthias Fend \n>> ---\n>> src/libcamera/ipa_manager.cpp | 2 ++\n>> 1 file changed, 2 insertions(+)\n>>\n>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>> index 2f96a207..030ef43f 100644\n>> --- a/src/libcamera/ipa_manager.cpp\n>> +++ b/src/libcamera/ipa_manager.cpp\n>> @@ -109,8 +109,10 @@ IPAManager::IPAManager()\n>> LOG(IPAManager, Fatal)\n>> << \"Multiple IPAManager objects are not allowed\";\n>> \n>> +#if HAVE_IPA_PUBKEY\n> If we have to add a #if here, it feels to me like the implementation\n> for handling the key itself isn't right.\n>\n> The alternative is to construct an invalid public key at\n> src/libcamera/ipa_pub_key.cpp.in to ensure we always have a key.. but\n> I'm not sure that sounds a lot better either...\n>\n> This is limited to the IPA manager which knows about the keys anyway, so\n> ... worst case I could see this as a solution ... but I think I would\n> always want to push against ifdeffery spreading\n>\n> Could you check if it's reasonable to instantiate a pubKey_ which isn't\n> valid please?\n>\n> But given here in ipa_manager - we already use #if HAVE_IPA_PUBKEY in\n> IPAManager::isSignatureValid() I think I'd let this through.\n\nWe also have HAVE_IPA_PUBKEY #ifdeffery in the ipa_manager.h header \nwhich makes conditional existence for pubKey_ member\n\nclass IPAManager\n{\n...\nprivate:\n     #if HAVE_IPA_PUBKEY\n             static const uint8_t publicKeyData_[];\n             static const PubKey pubKey_;\n     #endif\n}\n\n>\n> Reviewed-by: Kieran Bingham \n\nWith the above header hunk in mind, the patch looks fine to me for now,\n\nReviewed-by: Umang Jain \n\n>\n>> if (!pubKey_.isValid())\n>> LOG(IPAManager, Warning) << \"Public key not valid\";\n>> +#endif\n>> \n>> unsigned int ipaCount = 0;\n>> \n>> -- \n>> 2.25.1\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 3A4BFC0DA4\n\tfor ;\n\tWed, 14 Sep 2022 09:29:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 49CAB61FA5;\n\tWed, 14 Sep 2022 11:29:29 +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 CE20C6099B\n\tfor ;\n\tWed, 14 Sep 2022 11:29:27 +0200 (CEST)","from [192.168.1.102] (unknown [103.251.226.115])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D9322B3;\n\tWed, 14 Sep 2022 11:29:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663147769;\n\tbh=92JgIGh3Z+WkOXFlahKe3mA6a+tvHICNN9hIpU1QVKg=;\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=MzJeigFoaDTRSuMjA17wEI5RIZYgsEOEeZVYtN6krC3bJW00S2JEO9UZ6s4nO2wIf\n\tY8lppqAuuUgu4odXV+Y/IrfbdtztsrSVWqzQdvAVIB6C53hsVeFHRtIUUzJhN5Gh70\n\thQKwHDVv1L8YAg2lV7YmZY4XNFp0t4btU38dBEUNlbH7GVeFCOyPqqgJDf1oDszfYg\n\tVBW/vrb/yFNQF1sYajb2vHcqEe96YaDedhtl1r5eWplNh7cepgcVJtQeXMSHxOEwWl\n\tTevNUwiNOpfMJ6Kz3ttFanLw7F7jKNrsIIrsCtRO7XHMzeyu9QnWphhd9VGlr24Ez7\n\t7fMZKBeoxWx5Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663147767;\n\tbh=92JgIGh3Z+WkOXFlahKe3mA6a+tvHICNN9hIpU1QVKg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=m4Qp4rLeJsPCCThKZhcC+Ty1JYaXbJsCIj1E2ewKtWXXOhagk+F7ed6jbP+RmlESa\n\tYGGkCIikCrae/1Gvmx0Jnq+ZqyFGpHbzisKWpCpg1GSF7HkuwN/tcvvEVonczpbgGM\n\t+R0ZljelHJuNxdDGoynnHWlY6TPEZP8+hV3dJ0Gg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"m4Qp4rLe\"; dkim-atps=neutral","Message-ID":"<408c9b17-3cc5-71fd-0b26-00fb52c2e7e4@ideasonboard.com>","Date":"Wed, 14 Sep 2022 14:59:18 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.2.1","To":"Kieran Bingham ,\n\tMatthias Fend ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220914070025.27540-1-matthias.fend@emfend.at>\n\t<166314325272.3298829.8543568893170435258@Monstersaurus>","Content-Language":"en-US","In-Reply-To":"<166314325272.3298829.8543568893170435258@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa_manager: Fix build\n\twithout openssl","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\" "}}]