[{"id":11608,"web_url":"https://patchwork.libcamera.org/comment/11608/","msgid":"<20200726234211.GL28704@pendragon.ideasonboard.com>","date":"2020-07-26T23:42:11","subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi You-Sheng,\n\nThank you for the patch.\n\nOn Sat, Jul 25, 2020 at 08:24:41PM +0800, You-Sheng Yang wrote:\n> While these tests may be executed as normal user at build time,\n> unshare() call will fail and so are tests log_process and process_test.\n> This change checks if one is granted with necessary capabilities so that\n> we don't fail the build unexpectedly.\n> \n> Signed-off-by: You-Sheng Yang \n> ---\n> test/log/log_process.cpp | 20 ++++++++++++++++++++\n> test/log/meson.build | 2 +-\n> test/meson.build | 2 ++\n> test/process/meson.build | 2 +-\n> test/process/process_test.cpp | 23 +++++++++++++++++++++++\n> 5 files changed, 47 insertions(+), 2 deletions(-)\n> \n> diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> index d46d5e3..876da22 100644\n> --- a/test/log/log_process.cpp\n> +++ b/test/log/log_process.cpp\n> @@ -9,6 +9,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -55,6 +56,25 @@ class LogProcessTest : public Test\n> protected:\n> \tint init()\n> \t{\n> +\t\tint ret = TestPass;\n> +\n> +\t\tcap_t caps = cap_get_proc();\n> +\t\tif (caps == NULL) {\n> +\t\t\tcerr << \"failed to check process capabilities\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Check required permissions: CAP_SYS_ADMIN: unshare */\n> +\t\tcap_flag_value_t fv;\n> +\t\tif ((cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, &fv) < 0) || (fv != CAP_SET)) {\n> +\t\t\tcerr << \"skip due to insufficient capability\" << endl;\n> +\t\t\tret = TestSkip;\n> +\t\t}\n\nWould it make sense to add this as a helper function to the base Test\nclass ?\n\n> +\n> +\t\tcap_free(caps);\n> +\t\tif (ret != TestPass)\n> +\t\t\treturn ret;\n> +\n> \t\trandom_device random;\n> \t\tnum_ = random();\n> \t\tlogPath_ = \"/tmp/libcamera.worker.test.\" +\n> diff --git a/test/log/meson.build b/test/log/meson.build\n> index 8cd664e..000f980 100644\n> --- a/test/log/meson.build\n> +++ b/test/log/meson.build\n> @@ -7,7 +7,7 @@ log_test = [\n> \n> foreach t : log_test\n> exe = executable(t[0], t[1],\n> - dependencies : libcamera_dep,\n> + dependencies : [libcamera_dep, libcap],\n> link_with : test_libraries,\n> include_directories : test_includes_internal)\n> \n> diff --git a/test/meson.build b/test/meson.build\n> index f41d6e7..b4db328 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -1,5 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> +libcap = dependency('libcap', required : true)\n\n'true' is the default value for 'required', you can omit it. However,\nI'd like to keep the dependency optional, as we try to also support\nresource-constrainted embedded systems (based on musl or uclibc for\ninstance, and/or without udev).\n\nI have an idea how to do that, I'll try to submit a patch shortly.\n\n> +\n> subdir('libtest')\n> \n> subdir('camera')\n> diff --git a/test/process/meson.build b/test/process/meson.build\n> index c215fa7..828c17b 100644\n> --- a/test/process/meson.build\n> +++ b/test/process/meson.build\n> @@ -6,7 +6,7 @@ process_tests = [\n> \n> foreach t : process_tests\n> exe = executable(t[0], t[1],\n> - dependencies : libcamera_dep,\n> + dependencies : [libcamera_dep, libcap],\n> link_with : test_libraries,\n> include_directories : test_includes_internal)\n> \n> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> index ce0cc7c..ffa2143 100644\n> --- a/test/process/process_test.cpp\n> +++ b/test/process/process_test.cpp\n> @@ -5,6 +5,8 @@\n> * process_test.cpp - Process test\n> */\n> \n> +#include \n> +\n> #include \n> #include \n> #include \n> @@ -41,6 +43,27 @@ public:\n> \t}\n> \n> protected:\n> +\tint init()\n> +\t{\n> +\t\tint ret = TestPass;\n> +\n> +\t\tcap_t caps = cap_get_proc();\n> +\t\tif (caps == NULL) {\n> +\t\t\tcerr << \"failed to check process capabilities\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Check required permissions: CAP_SYS_ADMIN: unshare */\n> +\t\tcap_flag_value_t fv;\n> +\t\tif ((cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, &fv) < 0) || (fv != CAP_SET)) {\n> +\t\t\tcerr << \"skip due to insufficient capability\" << endl;\n> +\t\t\tret = TestSkip;\n> +\t\t}\n> +\n> +\t\tcap_free(caps);\n> +\t\treturn ret;\n> +\t}\n> +\n> \tint run()\n> \t{\n> \t\tEventDispatcher *dispatcher = Thread::current()->eventDispatcher();","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 F057DBD86F\n\tfor ;\n\tSun, 26 Jul 2020 23:42:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6168161223;\n\tMon, 27 Jul 2020 01:42:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE34E60399\n\tfor ;\n\tMon, 27 Jul 2020 01:42:19 +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 2C278304;\n\tMon, 27 Jul 2020 01:42:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oLgWoOgp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595806939;\n\tbh=qEBiyOSbyHjyfnZudbvxNW/qGIz2QfoieT0WTCp8z8s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oLgWoOgppoDeXtnC/HLVhf7GLts/nbJ02SG60OKHgyP/4ANI9PtGnE0mEORtRV9+g\n\t8Yd2ZMHVQsU9S4YLnECGZejIr2621UN+HyFGu5vAX+DzS421Bj+2grh6e6I+789uM6\n\tgaCfR+8Lq3R1NCr0pZ8q+kl+F8ngWWv3bKGYe41o=","Date":"Mon, 27 Jul 2020 02:42:11 +0300","From":"Laurent Pinchart ","To":"You-Sheng Yang ","Message-ID":"<20200726234211.GL28704@pendragon.ideasonboard.com>","References":"<20200725122442.1679820-1-vicamo.yang@canonical.com>\n\t<20200725122442.1679820-3-vicamo.yang@canonical.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200725122442.1679820-3-vicamo.yang@canonical.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11609,"web_url":"https://patchwork.libcamera.org/comment/11609/","msgid":"<20200726234712.GM28704@pendragon.ideasonboard.com>","date":"2020-07-26T23:47:12","subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi again,\n\nOn Mon, Jul 27, 2020 at 02:42:11AM +0300, Laurent Pinchart wrote:\n> Hi You-Sheng,\n> \n> Thank you for the patch.\n> \n> On Sat, Jul 25, 2020 at 08:24:41PM +0800, You-Sheng Yang wrote:\n> > While these tests may be executed as normal user at build time,\n> > unshare() call will fail and so are tests log_process and process_test.\n> > This change checks if one is granted with necessary capabilities so that\n> > we don't fail the build unexpectedly.\n> > \n> > Signed-off-by: You-Sheng Yang \n> > ---\n> > test/log/log_process.cpp | 20 ++++++++++++++++++++\n> > test/log/meson.build | 2 +-\n> > test/meson.build | 2 ++\n> > test/process/meson.build | 2 +-\n> > test/process/process_test.cpp | 23 +++++++++++++++++++++++\n> > 5 files changed, 47 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> > index d46d5e3..876da22 100644\n> > --- a/test/log/log_process.cpp\n> > +++ b/test/log/log_process.cpp\n> > @@ -9,6 +9,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -55,6 +56,25 @@ class LogProcessTest : public Test\n> > protected:\n> > \tint init()\n> > \t{\n> > +\t\tint ret = TestPass;\n> > +\n> > +\t\tcap_t caps = cap_get_proc();\n> > +\t\tif (caps == NULL) {\n> > +\t\t\tcerr << \"failed to check process capabilities\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Check required permissions: CAP_SYS_ADMIN: unshare */\n> > +\t\tcap_flag_value_t fv;\n> > +\t\tif ((cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, &fv) < 0) || (fv != CAP_SET)) {\n> > +\t\t\tcerr << \"skip due to insufficient capability\" << endl;\n> > +\t\t\tret = TestSkip;\n> > +\t\t}\n> \n> Would it make sense to add this as a helper function to the base Test\n> class ?\n> \n> > +\n> > +\t\tcap_free(caps);\n> > +\t\tif (ret != TestPass)\n> > +\t\t\treturn ret;\n> > +\n> > \t\trandom_device random;\n> > \t\tnum_ = random();\n> > \t\tlogPath_ = \"/tmp/libcamera.worker.test.\" +\n> > diff --git a/test/log/meson.build b/test/log/meson.build\n> > index 8cd664e..000f980 100644\n> > --- a/test/log/meson.build\n> > +++ b/test/log/meson.build\n> > @@ -7,7 +7,7 @@ log_test = [\n> > \n> > foreach t : log_test\n> > exe = executable(t[0], t[1],\n> > - dependencies : libcamera_dep,\n> > + dependencies : [libcamera_dep, libcap],\n> > link_with : test_libraries,\n> > include_directories : test_includes_internal)\n> > \n> > diff --git a/test/meson.build b/test/meson.build\n> > index f41d6e7..b4db328 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -1,5 +1,7 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> > \n> > +libcap = dependency('libcap', required : true)\n> \n> 'true' is the default value for 'required', you can omit it. However,\n> I'd like to keep the dependency optional, as we try to also support\n> resource-constrainted embedded systems (based on musl or uclibc for\n> instance, and/or without udev).\n> \n> I have an idea how to do that, I'll try to submit a patch shortly.\n\nActually, thinking about it some more, would it make sense to instead\ncondition the call to unshare() to CAP_SYS_ADMIN in the\nProcess:isolate() class ? Or turn it into a non-fatal error ?\n\nCould you maybe elaborate a little bit on the failure this patch is\ntrying to solve ? I haven't seen any such failure, how can they be\nreproduced ?\n\n> > +\n> > subdir('libtest')\n> > \n> > subdir('camera')\n> > diff --git a/test/process/meson.build b/test/process/meson.build\n> > index c215fa7..828c17b 100644\n> > --- a/test/process/meson.build\n> > +++ b/test/process/meson.build\n> > @@ -6,7 +6,7 @@ process_tests = [\n> > \n> > foreach t : process_tests\n> > exe = executable(t[0], t[1],\n> > - dependencies : libcamera_dep,\n> > + dependencies : [libcamera_dep, libcap],\n> > link_with : test_libraries,\n> > include_directories : test_includes_internal)\n> > \n> > diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> > index ce0cc7c..ffa2143 100644\n> > --- a/test/process/process_test.cpp\n> > +++ b/test/process/process_test.cpp\n> > @@ -5,6 +5,8 @@\n> > * process_test.cpp - Process test\n> > */\n> > \n> > +#include \n> > +\n> > #include \n> > #include \n> > #include \n> > @@ -41,6 +43,27 @@ public:\n> > \t}\n> > \n> > protected:\n> > +\tint init()\n> > +\t{\n> > +\t\tint ret = TestPass;\n> > +\n> > +\t\tcap_t caps = cap_get_proc();\n> > +\t\tif (caps == NULL) {\n> > +\t\t\tcerr << \"failed to check process capabilities\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Check required permissions: CAP_SYS_ADMIN: unshare */\n> > +\t\tcap_flag_value_t fv;\n> > +\t\tif ((cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, &fv) < 0) || (fv != CAP_SET)) {\n> > +\t\t\tcerr << \"skip due to insufficient capability\" << endl;\n> > +\t\t\tret = TestSkip;\n> > +\t\t}\n> > +\n> > +\t\tcap_free(caps);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > \tint run()\n> > \t{\n> > \t\tEventDispatcher *dispatcher = Thread::current()->eventDispatcher();","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 89AD6BD878\n\tfor ;\n\tSun, 26 Jul 2020 23:47:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 20D5461223;\n\tMon, 27 Jul 2020 01:47:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D182A60399\n\tfor ;\n\tMon, 27 Jul 2020 01:47:20 +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 4C35C304;\n\tMon, 27 Jul 2020 01:47:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L0zC+Agi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595807240;\n\tbh=DIXp3zj9KFg5gJAHA7x09XbqwB0svaF7uI2AeXyOhbk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L0zC+AgimGioSIDL0mC71Z1+PFZP10uAHY/eeFckIkRQgYR+2i6lRZ4SSnJQV2EO5\n\tXshoNYbBLpVoBwn/oxDk7d/BRbMLkGzv1pVP3Asof0X50hV3cjpvEZCKOtbron4sLm\n\tlA75f8y0i9L7XVTb13H9IAq5xgNdIRlPFgVXxlO0=","Date":"Mon, 27 Jul 2020 02:47:12 +0300","From":"Laurent Pinchart ","To":"You-Sheng Yang ","Message-ID":"<20200726234712.GM28704@pendragon.ideasonboard.com>","References":"<20200725122442.1679820-1-vicamo.yang@canonical.com>\n\t<20200725122442.1679820-3-vicamo.yang@canonical.com>\n\t<20200726234211.GL28704@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200726234211.GL28704@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11650,"web_url":"https://patchwork.libcamera.org/comment/11650/","msgid":"","date":"2020-07-27T15:50:39","subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","submitter":{"id":61,"url":"https://patchwork.libcamera.org/api/people/61/","name":"You-Sheng Yang","email":"vicamo.yang@canonical.com"},"content":"On 2020-07-27 07:47, Laurent Pinchart wrote:\n> Hi again,\n> \n> On Mon, Jul 27, 2020 at 02:42:11AM +0300, Laurent Pinchart wrote:\n>> Hi You-Sheng,\n>>\n>> Thank you for the patch.\n>>\n>> On Sat, Jul 25, 2020 at 08:24:41PM +0800, You-Sheng Yang wrote:\n>>> While these tests may be executed as normal user at build time,\n>>> unshare() call will fail and so are tests log_process and process_test.\n>>> This change checks if one is granted with necessary capabilities so that\n>>> we don't fail the build unexpectedly.\n>>>\n>>> Signed-off-by: You-Sheng Yang \n>>> ---\n>>> test/log/log_process.cpp | 20 ++++++++++++++++++++\n>>> test/log/meson.build | 2 +-\n>>> test/meson.build | 2 ++\n>>> test/process/meson.build | 2 +-\n>>> test/process/process_test.cpp | 23 +++++++++++++++++++++++\n>>> 5 files changed, 47 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n>>> index d46d5e3..876da22 100644\n>>> --- a/test/log/log_process.cpp\n>>> +++ b/test/log/log_process.cpp\n>>> @@ -9,6 +9,7 @@\n>>> #include \n>>> #include \n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>> #include \n>>> @@ -55,6 +56,25 @@ class LogProcessTest : public Test\n>>> protected:\n>>> \tint init()\n>>> \t{\n>>> +\t\tint ret = TestPass;\n>>> +\n>>> +\t\tcap_t caps = cap_get_proc();\n>>> +\t\tif (caps == NULL) {\n>>> +\t\t\tcerr << \"failed to check process capabilities\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\t/* Check required permissions: CAP_SYS_ADMIN: unshare */\n>>> +\t\tcap_flag_value_t fv;\n>>> +\t\tif ((cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, &fv) < 0) || (fv != CAP_SET)) {\n>>> +\t\t\tcerr << \"skip due to insufficient capability\" << endl;\n>>> +\t\t\tret = TestSkip;\n>>> +\t\t}\n>>\n>> Would it make sense to add this as a helper function to the base Test\n>> class ?\n\nWill do. But probably after having a conclusion below.\n\n>>> +\n>>> +\t\tcap_free(caps);\n>>> +\t\tif (ret != TestPass)\n>>> +\t\t\treturn ret;\n>>> +\n>>> \t\trandom_device random;\n>>> \t\tnum_ = random();\n>>> \t\tlogPath_ = \"/tmp/libcamera.worker.test.\" +\n>>> diff --git a/test/log/meson.build b/test/log/meson.build\n>>> index 8cd664e..000f980 100644\n>>> --- a/test/log/meson.build\n>>> +++ b/test/log/meson.build\n>>> @@ -7,7 +7,7 @@ log_test = [\n>>> \n>>> foreach t : log_test\n>>> exe = executable(t[0], t[1],\n>>> - dependencies : libcamera_dep,\n>>> + dependencies : [libcamera_dep, libcap],\n>>> link_with : test_libraries,\n>>> include_directories : test_includes_internal)\n>>> \n>>> diff --git a/test/meson.build b/test/meson.build\n>>> index f41d6e7..b4db328 100644\n>>> --- a/test/meson.build\n>>> +++ b/test/meson.build\n>>> @@ -1,5 +1,7 @@\n>>> # SPDX-License-Identifier: CC0-1.0\n>>> \n>>> +libcap = dependency('libcap', required : true)\n>>\n>> 'true' is the default value for 'required', you can omit it. However,\n>> I'd like to keep the dependency optional, as we try to also support\n>> resource-constrainted embedded systems (based on musl or uclibc for\n>> instance, and/or without udev).\n>>\n>> I have an idea how to do that, I'll try to submit a patch shortly.\n> \n> Actually, thinking about it some more, would it make sense to instead\n> condition the call to unshare() to CAP_SYS_ADMIN in the\n> Process:isolate() class ? Or turn it into a non-fatal error ?\n\nIt's about API design, so your opinions matter most.\n\nI didn't have much idea about the rational behind the unshare() call\ninside libcamera::Process, but I'm really suspect the necessity of it as\npart of a, at least looks like, generic API. It implicitly adds a\nconstrain that any process tries to create a subprocess in libcamera\nusing libcamera::Process, its child process must be either executed by\nroot or have CAP_SYS_ADMIN. This doesn't really sound a good idea for\nme, especially when I believe one should really build a multimedia\nlibrary to run as a normal user as possible.\n\nAnyway, the only user of this API in libcamera is ipa_proxy_linux, you\ncould have put unshare() into ipa_proxy_linux itself. This way you could\ninstall some selinux/apparmor rules to grant such permission to this\nexecutable explicitly. But again, is that really necessary? Is\nipa_proxy_linux really has to own its own network and uid namespace?\n\n> Could you maybe elaborate a little bit on the failure this patch is\n> trying to solve ? I haven't seen any such failure, how can they be\n> reproduced ?\n\nPlease see https://gitlab.com/vicamo/libcamera/-/jobs/650449281\n\n21/55 libcamera:process / process_test FAIL\n0.01s (exit status 255 or signal 127 SIGinvalid)\n--- command ---\n10:26:15\n/builds/vicamo/libcamera/debian/output/libcamera-0~git20200722+d929555/obj-x86_64-linux-gnu/test/process/process_test\n--- stderr ---\n\nI was trying to fix debian packaging and to have a daily build based on\nmaster tip.\n\nYou-Sheng Yang","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 E625EBD878\n\tfor ;\n\tMon, 27 Jul 2020 15:56:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C1FE161253;\n\tMon, 27 Jul 2020 17:56:51 +0200 (CEST)","from mail-pj1-f67.google.com (mail-pj1-f67.google.com\n\t[209.85.216.67])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C18E60536\n\tfor ;\n\tMon, 27 Jul 2020 17:50:55 +0200 (CEST)","by mail-pj1-f67.google.com with SMTP id k71so9664581pje.0\n\tfor ;\n\tMon, 27 Jul 2020 08:50:55 -0700 (PDT)","from ?IPv6:2001:b400:e26f:eb3a:89c1:fae8:da9b:92c3?\n\t(2001-b400-e26f-eb3a-89c1-fae8-da9b-92c3.emome-ip6.hinet.net.\n\t[2001:b400:e26f:eb3a:89c1:fae8:da9b:92c3])\n\tby smtp.gmail.com with ESMTPSA id\n\td25sm15346611pgn.2.2020.07.27.08.50.50\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 27 Jul 2020 08:50:51 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:autocrypt\n\t:message-id:date:user-agent:mime-version:in-reply-to;\n\tbh=B8BOm2lO3OT1msAA9TFappgCNQsH9jGAOG7/vvKB4WI=;\n\tb=fEQc55eKqc/qGVteA6To1M8XZJS3qCJRgGIGR6eT3O5QcvsIGB4unrQjuoYAyN4Eml\n\tGDUjt3A+P7Ng6RjrWyZ5+0YHdu6aMUklhnfV4hVrSXMEP1IxB1poLJ3wt20/HejJPqj1\n\t09GVzGgRUElWe4G1o5v6VSs/u5TGakVZQyAvbFfkjKpU/7NQ2Mbv5ci20VvYAsc2IDUa\n\t0QN/u6MHObzT9bHN//CM1uZmMXBncJMxsY7platluvtYAi36XxiTIIOh9IGFhSkRKMAA\n\t+41ISiYULBGExLV3F2nTq4IaMK3q85b6P4IAe2JAp14MPQBwkCB1n5Gvxe87EX8edp0c\n\tRAHg==","X-Gm-Message-State":"AOAM5302rZTKUUHo4ZTi2eluzwoUGLFE0JftSs7zmpNJ0lg6xzq5DI3W\n\tddG7ynKjvVeC+zmAV4vsKU8vYx76MsY=","X-Google-Smtp-Source":"ABdhPJwBrzVuNzKu9k4p4V8qpHBFh3ocmfqNLhNSTQ+MKNGk5efXt6gAp5rMoFllXY+8tbZj/1LEBw==","X-Received":"by 2002:a17:902:d715:: with SMTP id\n\tw21mr18823925ply.140.1595865053051; \n\tMon, 27 Jul 2020 08:50:53 -0700 (PDT)","To":"Laurent Pinchart ","References":"<20200725122442.1679820-1-vicamo.yang@canonical.com>\n\t<20200725122442.1679820-3-vicamo.yang@canonical.com>\n\t<20200726234211.GL28704@pendragon.ideasonboard.com>\n\t<20200726234712.GM28704@pendragon.ideasonboard.com>","From":"You-Sheng Yang ","Autocrypt":"addr=vicamo.yang@canonical.com; keydata=\n\tmQINBFxnlfIBEAC2RZLjA5pfvBm/uOPB++2AC5Z+hie/zQnaiwoS+4p1pVeZ80lTPdS57b89\n\tH0k3mD6cwF7lLPmUeL6Gi4vriRsiZNiU9ZWS3AVol1YsAQhidJ5aSGOLn1Vhari9NQYwPYjM\n\t+MzbzBtjdaUolvBAGqmWFNUtJ2+C43CSKUykDFxHz5NeYE78z3g/2R4MdIvlTO0vQRQM0eNf\n\tprpdriEUjHBbMGZFkHNA0cO9WqyT/hztlwEZkP+nGje+oBeNKNlxCy1zXtQPBrFwlisWLycj\n\tDF4St3YzMm6Yv7l4Jz+dO7EUkJcKTlhA6QimF4o0u61ebZ9szemrMHkcK+inRwNVlfILZvIO\n\tLOUUks7ExzvtxD66mIrjgqcGcKAU9plc7lSqUWvfKHgiWwU/56Sb8y4BprsWKiGEUWytUGu1\n\tSZclJIibcyG0Ookxx43y00YvCCJAy7svkfJJMu7W6+9vpaTAdvUz5GOr9qncxrHXNR2JD9uy\n\tf0S7DXVKDBDhgmrNt2bg1FeP/Y9Nz2U/9SMeV6zNwZBwHos5AxAlY3x0IAAk+GZ6gpjdUXY2\n\tGTb1Y1l9RUp/untzo76ytRs6m8BAdwRjWdBAgQ7xMZFpWTD2Unhi45QAXtHd+WgSi0Nwin/W\n\tyzVOoWffgS0Z8+xgOBVOs4HKsb1rr0CwcfJa+bsD4JwxRnAkFwARAQABtCpZb3UtU2hlbmcg\n\tWWFuZyA8dmljYW1vLnlhbmdAY2Fub25pY2FsLmNvbT6JAlQEEwEKAD4WIQSf4T7aw75OM7ft\n\t1VTU3r32YVqihAUCXG3YPgIbAQUJA8JnAAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAKCRDU\n\t3r32YVqihLkZD/9/BSCD2cYtBap+UqoZMXRU1GkzT6upy+/HmTBEza+RDDoGWtWbHt7hgUyg\n\tKEL2Sl4E1Bkurm9OQg1Zc8gU3dcpIzyWuXLBXNlORtbqiApob+6JwTFC7mareCIeK42QOPcV\n\tOK+wZZQTHjIhqR/FyycFzvNGiKlzBHHRzlSrKSV/vm7grwui04OqddOdlWDtVfO4fQMYTpWC\n\tjsOKkgFJWtf2uMzXwH/vPmk3P9XvTT6N+U2l01KiSMv3rRQw6VeLXK10Gg+q4PbdPZP4gNUu\n\ty2u/KECWNw18L+Y3N004wsNC68W073w9bbTh0GbpAxHpqIAGbk5s7aOOhl2MO9PxSvP7bVju\n\t7msN7fowXU8dqFQ6noOkGPoN75osTWHrdHeWjw5It9qyXm0/TAlbsRTrMUbg3mCUJQuRHDv5\n\tLVOdCvAUSyobAQq/583GP4S08jRr51AOelcsMq+bVZdHb7gIdE3LDNlfqlbu/NfihJdcDTpo\n\tDTRg1XO7xXZc2Sud4QSQCF6RSkUFbXR6IncLLmVMmU45mQQGqMqnk3jJFqkz+mapxe7kYvd6\n\tVHB42vpdK+l30eODzU65owqvH36W+5cvHp+raj89+z8KysNJksVAeuZeqydXN15/x3xuFRlJ\n\txas+mLayG02U0uSqvjaIuLJXqKD8GvB9BONZufyMecQL13+iI7QzWW91LVNoZW5nIFlhbmcg\n\tKE1vYmlsZSkgPHZpY2Ftby55YW5nQGNhbm9uaWNhbC5jb20+iQJUBBMBCgA+FiEEn+E+2sO+\n\tTjO37dVU1N699mFaooQFAlx1UYwCGwEFCQPCZwAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AA\n\tCgkQ1N699mFaooQ/0g/9FrRRrl+P7orbxYuQmjF/65VHn3H99di5TzkEmobhrFIX5c/5VEF8\n\t6pwxtCnYnUyf+0on8HyvBtfiZfcA8bvUoqrPiu5Xr+46BvDU6DSq62QjDXv0brSLvPOdZmsy\n\tcrxNFhuODvYFsUxZSLxsVljhcbOIRv0ISguyHIqiuxjYlkIQ/QJ3r6ZBFL44lDm9RfuxcHWk\n\tyMljUVj3JVhh15Nu0rQnyMcTObVinZMqbWPf9G8lPYdRH7nI9XL1f8odsTDPn8MshORnmOmS\n\taESf+6NQZtR6pF2p2l9IWQc1ABBkIAjRrfen3SFylItm8b4vosbeNS4vltSl1pli2U1RzMJ4\n\tZgeOQJO7pd8MzTRY+RCQ1CqN9PEhtxoDnLdyhAubRTotQ+YZOcMOUJ+uHM1d/yvRe6sp04gS\n\tOw17s52fX3U8kiBbLQp0QRzv5gUX46Y3vDdkd5a6lbLQFgYNtosFvrwrdRwMOfKYw4Or7xcj\n\tYUhHsC5CaihUjp7d7nt2YGIXsDjnAUvILU4cA967bWfknEJaK0NY3BYN6Vxf6GL7g8pXug3l\n\tPd3yVkoSEP+pTu+EZtymI6SHcIJZLNqxNoKneDIYLebHkMsNq+6NdF2KZ8M1amD5nbY3kUdq\n\t/EJKItxjgnuMYm/eGPq6byZQVirZIA58AvFS5PMHpvytHvYhBflMLB+5AQ0EXGeWyQEIALMb\n\tD2wCNDvLCJD79AYjIX9mDpHzJtkKX8Uh6MtAybfUzZP7R4qKOFBRZOH94e59Jx7D1O3eD0KZ\n\tW8CXqdx5pqBtssTOA1We4zfOe7f1XLDaDvl62TXQYqufGllOuIIZ49IgtEYAbSrFtyC/qbRk\n\tt58ophBlJoDRkBln/Uo0l5RtCkNucKXtEoy+N8unJzHEEdi9BxOW4DxqiTPhRKso8BekAeZO\n\tT/RF5ka3JXaJlyFBk08XLTtk8Fw2RnHvi7zVdx45GuvLxT0tVwkjZfklOiOoBLbWuNr+ghv9\n\tXG0Qq4pG0xexKPMQN2l+1ap9oeiH/CAPaK/o0XrwVwPWOQTIZiMAEQEAAYkCNgQYAQoAIBYh\n\tBJ/hPtrDvk4zt+3VVNTevfZhWqKEBQJcZ5bJAhsMAAoJENTevfZhWqKEZxMP/2WqtBXPWPPi\n\t/pcRkrYQkkVZL3yzHB1hKeGbtwvaABRD7KUg5Mm3Z8VIINK6pet9qXpXEaX4g1Ch7Arb8kzY\n\tIH535jdwcfE2eEbWg55HQUqu1G/OQ4E3bmrXNe8WBQXrKlJjqK4Xo02tUjbSBobRE++6O8Yb\n\tHig84jZlBpYBDNqixvaaASM1/NA7pvasuMFpGjw+ULvWbRTR2euTsACUIZCcmpBytrX6Q1lx\n\tWwIyPvVO1Ns0PW7F832xMkKS1Y3Ntha5bi9j+Inh0NV2Q59gen6Oo8GQJsmjA10L2/QFeIsM\n\teT+w6WIrFJt19yY/OLtVg5dFv7mAeCx1KefpdGjRDx4MH01uqypG/+UKf8bmkF0TYGd8/iXp\n\t2w7En8D9HIM+/Rm+KmNjQ7QgaTxvYEqC8R0y2yIfHiHwyp3SQw1COKT9jIMdmCbrUV99OFcu\n\tqifhMOJJ3hFFpEtNzGKL7yoKVop7PWMufwgzB6aALqxtZah+ibrKyaKce1p/sbxxp/ekUpwa\n\tgyJn0L3coWrgOCMsifiL1sifJ2cK9Z4NCRzCMsJdLtHSrIbAG2Hxm8vaLOLLSaeK/1tVY/Qi\n\try5WlCi6uVuNbwuAfMiK4jOnBPDYWTPFQtpg59XLXTq1xGPhA4RD5XjMmuvp7mJXFsvvlda/\n\tpsgobKXZGwvpcJsTTesykaeYuQGNBFx1T6UBDADqO+s9eLWQ3fr4njPoLQ8ff4pGoXgZqu0O\n\tCcn0LoqVnaLZzIfsUZ4ONp+y2S81sJL82AKAOuJ5Kq5REg+xntPBLSs326JzfhuoTOmP4m2h\n\tXhyoem3BPPqJnFcJdr6/HE7QuH0Whdv+PVe55S/iXwHPQddpz9fEcHy3SleHGljPINCn1G4F\n\t5CNV07kS7MS6Zx2HeofHcvUECunARrwuFqMlFAn5u580ORhmCZ+ha0+B4stL+ZUDNAX7ADjb\n\tcvtxUS0vdbRRrZVc/mK4Weqsb8vNSgRbKdLZlwDvEhWHWIIG4lfLXGmbvLsUFMa3cU9rl2oH\n\tWeh+GUIMfuUJfOryzl5UO1hFAn31zs9GAC0/RtTOotOEm/t3zWbvFai5zmGeWU2ZAQb+sRMX\n\tuZLSjxJklcSCCJsG9k+PaBOyzjdj3U1XWp/aUb+bfGiN4VijBVozWkLndMcNt3IL6YRR+uX/\n\tvP8XgEL0kEvx4a7qtBUZNxLF00Hy5q3FRWPnt3A7RU2TD7MAEQEAAYkD7AQYAQoAIBYhBJ/h\n\tPtrDvk4zt+3VVNTevfZhWqKEBQJcdU+lAhsCAcAJENTevfZhWqKEwPQgBBkBCgAdFiEES1bV\n\ta9nnnyj3TuTG4eTfmHHSmlMFAlx1T6UACgkQ4eTfmHHSmlO+PAwAthzvSuazTk4oFYRFDj1Q\n\tzQSwcTUVFw5jW4i4gNrbb5066UDdVmoTsTeY8OpBLGqBPVKUWhFhMxvF2uxmYTAjZFCvfabS\n\ts+PW+cbb9NfRZMKD8KUj2SRWZY2zcRXTwYtnIj3+SEDk+AB5NQuBG63zDecV2Af1+n9HXD+X\n\tsckKCNUHVYH1L2Bps5wnhzwbIboMSOjY6P3n+8ztuL6De4kzLqpJFq9b/5IB7bffns7WCdkZ\n\tkbET9d0uufKMQR2z/WJJYC/oVSUg445lhqU4SVXAwZjSG5nQsPRreuwjuFT78ExRjxtzohk3\n\tobLh+v0NhXK1QH+88ypBFVjB7IdnUHY4itJBQGJhSWTwXta2uYzxMzsMj8P+o1wN79DfG2gy\n\tuDSIwecGB6HtyDmsL5rtfKU5KhrklaYdX1bgPBS46IfpCDt3QfNKFy7icmZm1U4+xEnOkjxo\n\taJ7tUVDfC5YVtAX1B6HVczR2Up6iaWjml+yfLZSBLKbuC8/O0FfLZIs4iVaOP9YP/AqaSq7K\n\tHBEf4sY4RT1ivhVUl1nIAc7RiCHFZYPeFmygQUZ6raIyhySCNetzx+am3EGr7QIm2414IC0B\n\tciC9GAYwDR/5cca7hP8wowYWvrB+76vejXJ/g3TRxE+CnNAg6YjRsxPvhKqTwtPDjYeAbZM1\n\t9HkPK2TqogoH1BDenMfzRp7Niv5wS/nEHaLLRvViKr9k8j8alycLlFs1aDT8BJF29aRp1Mbc\n\tW8vVHCD7Ks3TYz6rf+saoA7BVDZetTE3qigbeZHtpMrWGPk7y4pidrcV/OwOhotUvKm2wHuD\n\tjU33fE+d5lJY8NZBX7cSbbFj8q6yd4jdAnCEITfuG4rfblGJMpEMbU0mrsfan05zbjchPuho\n\t6xMjG/p58xZnMtRmMy+JPG/nA2piiveObircDqeiNvSpZankQ9MggsdCFyh54ocRt+lTAeSw\n\tHUWvbN7OWSkbuwS6DWMWUEnVFhXIvRv0wn4ZM/Xc68h4IJ+lxwViCNZSuzMovJNH8sbbTtq9\n\teGCQoHAmaHhiefRstYMqpZyCTUtALQgqnRZLl83YN1U3xlzs65CfHfB0psYRiDi68HeniqSa\n\t3QoiE+kUr7jrh1xSanUdyl/g82JL570qPrCBvgE3PT8Na0xvLfImmK7dWOmDCXZetgronuP3\n\tsuzL+d2CSm1cCUYQeOxX/7MpmAIm","Message-ID":"","Date":"Mon, 27 Jul 2020 23:50:39 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200726234712.GM28704@pendragon.ideasonboard.com>","X-Mailman-Approved-At":"Mon, 27 Jul 2020 17:56:51 +0200","Subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============5967908146425114731==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11659,"web_url":"https://patchwork.libcamera.org/comment/11659/","msgid":"<20200727235850.GF15448@pendragon.ideasonboard.com>","date":"2020-07-27T23:58:50","subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi You-Sheng,\n\nOn Mon, Jul 27, 2020 at 11:50:39PM +0800, You-Sheng Yang wrote:\n> On 2020-07-27 07:47, Laurent Pinchart wrote:\n> > On Mon, Jul 27, 2020 at 02:42:11AM +0300, Laurent Pinchart wrote:\n> >> On Sat, Jul 25, 2020 at 08:24:41PM +0800, You-Sheng Yang wrote:\n> >>> While these tests may be executed as normal user at build time,\n> >>> unshare() call will fail and so are tests log_process and process_test.\n> >>> This change checks if one is granted with necessary capabilities so that\n> >>> we don't fail the build unexpectedly.\n> >>>\n> >>> Signed-off-by: You-Sheng Yang \n> >>> ---\n> >>> test/log/log_process.cpp | 20 ++++++++++++++++++++\n> >>> test/log/meson.build | 2 +-\n> >>> test/meson.build | 2 ++\n> >>> test/process/meson.build | 2 +-\n> >>> test/process/process_test.cpp | 23 +++++++++++++++++++++++\n> >>> 5 files changed, 47 insertions(+), 2 deletions(-)\n> >>>\n> >>> diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> >>> index d46d5e3..876da22 100644\n> >>> --- a/test/log/log_process.cpp\n> >>> +++ b/test/log/log_process.cpp\n> >>> @@ -9,6 +9,7 @@\n> >>> #include \n> >>> #include \n> >>> #include \n> >>> +#include \n> >>> #include \n> >>> #include \n> >>> #include \n> >>> @@ -55,6 +56,25 @@ class LogProcessTest : public Test\n> >>> protected:\n> >>> \tint init()\n> >>> \t{\n> >>> +\t\tint ret = TestPass;\n> >>> +\n> >>> +\t\tcap_t caps = cap_get_proc();\n> >>> +\t\tif (caps == NULL) {\n> >>> +\t\t\tcerr << \"failed to check process capabilities\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\t/* Check required permissions: CAP_SYS_ADMIN: unshare */\n> >>> +\t\tcap_flag_value_t fv;\n> >>> +\t\tif ((cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, &fv) < 0) || (fv != CAP_SET)) {\n> >>> +\t\t\tcerr << \"skip due to insufficient capability\" << endl;\n> >>> +\t\t\tret = TestSkip;\n> >>> +\t\t}\n> >>\n> >> Would it make sense to add this as a helper function to the base Test\n> >> class ?\n> \n> Will do. But probably after having a conclusion below.\n> \n> >>> +\n> >>> +\t\tcap_free(caps);\n> >>> +\t\tif (ret != TestPass)\n> >>> +\t\t\treturn ret;\n> >>> +\n> >>> \t\trandom_device random;\n> >>> \t\tnum_ = random();\n> >>> \t\tlogPath_ = \"/tmp/libcamera.worker.test.\" +\n> >>> diff --git a/test/log/meson.build b/test/log/meson.build\n> >>> index 8cd664e..000f980 100644\n> >>> --- a/test/log/meson.build\n> >>> +++ b/test/log/meson.build\n> >>> @@ -7,7 +7,7 @@ log_test = [\n> >>> \n> >>> foreach t : log_test\n> >>> exe = executable(t[0], t[1],\n> >>> - dependencies : libcamera_dep,\n> >>> + dependencies : [libcamera_dep, libcap],\n> >>> link_with : test_libraries,\n> >>> include_directories : test_includes_internal)\n> >>> \n> >>> diff --git a/test/meson.build b/test/meson.build\n> >>> index f41d6e7..b4db328 100644\n> >>> --- a/test/meson.build\n> >>> +++ b/test/meson.build\n> >>> @@ -1,5 +1,7 @@\n> >>> # SPDX-License-Identifier: CC0-1.0\n> >>> \n> >>> +libcap = dependency('libcap', required : true)\n> >>\n> >> 'true' is the default value for 'required', you can omit it. However,\n> >> I'd like to keep the dependency optional, as we try to also support\n> >> resource-constrainted embedded systems (based on musl or uclibc for\n> >> instance, and/or without udev).\n> >>\n> >> I have an idea how to do that, I'll try to submit a patch shortly.\n> > \n> > Actually, thinking about it some more, would it make sense to instead\n> > condition the call to unshare() to CAP_SYS_ADMIN in the\n> > Process:isolate() class ? Or turn it into a non-fatal error ?\n> \n> It's about API design, so your opinions matter most.\n> \n> I didn't have much idea about the rational behind the unshare() call\n> inside libcamera::Process, but I'm really suspect the necessity of it as\n> part of a, at least looks like, generic API. It implicitly adds a\n> constrain that any process tries to create a subprocess in libcamera\n> using libcamera::Process, its child process must be either executed by\n> root or have CAP_SYS_ADMIN. This doesn't really sound a good idea for\n> me, especially when I believe one should really build a multimedia\n> library to run as a normal user as possible.\n\nThe Process class is meant to run closed-source image processing\nalgorithm (IPA) modules in a separate, isolated process. The unshare()\ncall is a very first (mockup) step in that direction, and we know more\nwork is needed to achieve a real sandboxing.\n\nNow that I think about it, it may be better to instead rely on minijail\nor firejail instead of reinventing the wheel.\n\n> Anyway, the only user of this API in libcamera is ipa_proxy_linux, you\n> could have put unshare() into ipa_proxy_linux itself. This way you could\n> install some selinux/apparmor rules to grant such permission to this\n> executable explicitly. But again, is that really necessary? Is\n> ipa_proxy_linux really has to own its own network and uid namespace?\n\nWe want to isolate the IPA modules, limiting their access to the system\nas much as possible. They should only be able to access specific file\nsystem directories (in order to load configuration data and write logs),\nand nothing else (no device access, no network access, ...).\nClosed-source IPA modules are considered to be untrusted binaries.\n\n> > Could you maybe elaborate a little bit on the failure this patch is\n> > trying to solve ? I haven't seen any such failure, how can they be\n> > reproduced ?\n> \n> Please see https://gitlab.com/vicamo/libcamera/-/jobs/650449281\n\nThat's lots of failures :-S\n\nThe process test has been part of our test suite for a long time, and\nit's not run as root or with CAP_SYS_ADMIN. As far as I can tell, we've\nnever noticed any issue with unshare() failing. I'm not sure what's\ndifferent in your environment.\n\nWe can also consider dropping the unshare() call for now, as it's only a\npartial implementation of process isolation. We would need to implement\nthat feature down the line though. Wrapping the ipa_proxy_worker with\nminijail or firejail, or implementing isolation in the worker itself,\nare two possible candidates. Another option would be to run the proxy\nworker as a system daemon, but at this point we would like to avoid\ngoing down that route if possible.\n\nDo you have any recommendation ?\n\n> 21/55 libcamera:process / process_test FAIL\n> 0.01s (exit status 255 or signal 127 SIGinvalid)\n> --- command ---\n> 10:26:15\n> /builds/vicamo/libcamera/debian/output/libcamera-0~git20200722+d929555/obj-x86_64-linux-gnu/test/process/process_test\n> --- stderr ---\n> \n> I was trying to fix debian packaging and to have a daily build based on\n> master tip.","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 D1965BD878\n\tfor ;\n\tMon, 27 Jul 2020 23:59:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DD8D613C6;\n\tTue, 28 Jul 2020 01:59:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D4BD96053C\n\tfor ;\n\tTue, 28 Jul 2020 01:58:58 +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 4A890556;\n\tTue, 28 Jul 2020 01:58:58 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"A6F5lJ75\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595894338;\n\tbh=7Ndq/SKSS/9o+FfeoyvSlrFPlUKWruAePgGy34xhkNA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=A6F5lJ755ZC53dKSxcWxOv012qhYQQhqOsLfq/pillGgMs5JVRw/Y5Xwe7qSc8pPn\n\tPU8XgE2YYZU9dVHdmDX1QNLLrTDZMBW8FqW6iOipekvq5ClL1BQi0L8cPY9jjJfkcL\n\tFh8296T8grtnGNm+iDobmxNl8znkoZUSn83rWC/g=","Date":"Tue, 28 Jul 2020 02:58:50 +0300","From":"Laurent Pinchart ","To":"You-Sheng Yang ","Message-ID":"<20200727235850.GF15448@pendragon.ideasonboard.com>","References":"<20200725122442.1679820-1-vicamo.yang@canonical.com>\n\t<20200725122442.1679820-3-vicamo.yang@canonical.com>\n\t<20200726234211.GL28704@pendragon.ideasonboard.com>\n\t<20200726234712.GM28704@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11661,"web_url":"https://patchwork.libcamera.org/comment/11661/","msgid":"","date":"2020-07-28T03:39:38","subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","submitter":{"id":61,"url":"https://patchwork.libcamera.org/api/people/61/","name":"You-Sheng Yang","email":"vicamo.yang@canonical.com"},"content":"Hi Laurent,\n\nOn 2020-07-28 07:58, Laurent Pinchart wrote:\n> Hi You-Sheng,\n> \n> On Mon, Jul 27, 2020 at 11:50:39PM +0800, You-Sheng Yang wrote:\n>> On 2020-07-27 07:47, Laurent Pinchart wrote:\n>>> On Mon, Jul 27, 2020 at 02:42:11AM +0300, Laurent Pinchart wrote:\n>>>> On Sat, Jul 25, 2020 at 08:24:41PM +0800, You-Sheng Yang wrote:\n>>>>> diff --git a/test/meson.build b/test/meson.build\n>>>>> index f41d6e7..b4db328 100644\n>>>>> --- a/test/meson.build\n>>>>> +++ b/test/meson.build\n>>>>> @@ -1,5 +1,7 @@\n>>>>> # SPDX-License-Identifier: CC0-1.0\n>>>>> \n>>>>> +libcap = dependency('libcap', required : true)\n>>>>\n>>>> 'true' is the default value for 'required', you can omit it. However,\n>>>> I'd like to keep the dependency optional, as we try to also support\n>>>> resource-constrainted embedded systems (based on musl or uclibc for\n>>>> instance, and/or without udev).\n>>>>\n>>>> I have an idea how to do that, I'll try to submit a patch shortly.\n>>>\n>>> Actually, thinking about it some more, would it make sense to instead\n>>> condition the call to unshare() to CAP_SYS_ADMIN in the\n>>> Process:isolate() class ? Or turn it into a non-fatal error ?\n>>\n>> It's about API design, so your opinions matter most.\n>>\n>> I didn't have much idea about the rational behind the unshare() call\n>> inside libcamera::Process, but I'm really suspect the necessity of it as\n>> part of a, at least looks like, generic API. It implicitly adds a\n>> constrain that any process tries to create a subprocess in libcamera\n>> using libcamera::Process, its child process must be either executed by\n>> root or have CAP_SYS_ADMIN. This doesn't really sound a good idea for\n>> me, especially when I believe one should really build a multimedia\n>> library to run as a normal user as possible.\n> \n> The Process class is meant to run closed-source image processing\n> algorithm (IPA) modules in a separate, isolated process. The unshare()\n> call is a very first (mockup) step in that direction, and we know more\n> work is needed to achieve a real sandboxing.\n> \n> Now that I think about it, it may be better to instead rely on minijail\n> or firejail instead of reinventing the wheel.\n\n\n>> Anyway, the only user of this API in libcamera is ipa_proxy_linux, you\n>> could have put unshare() into ipa_proxy_linux itself. This way you could\n>> install some selinux/apparmor rules to grant such permission to this\n>> executable explicitly. But again, is that really necessary? Is\n>> ipa_proxy_linux really has to own its own network and uid namespace?\n> \n> We want to isolate the IPA modules, limiting their access to the system\n> as much as possible. They should only be able to access specific file\n> system directories (in order to load configuration data and write logs),\n> and nothing else (no device access, no network access, ...).\n> Closed-source IPA modules are considered to be untrusted binaries.\n\nI understand. But while ipa_linux_proxy is currently integrated into\nlibcamera source, you know and can setup constrains for it correctly.\nWhen some other vendor adopts libcamera and creates similar plugin by\ntheir own, that may become something blocking their normal function.\n\n>>> Could you maybe elaborate a little bit on the failure this patch is\n>>> trying to solve ? I haven't seen any such failure, how can they be\n>>> reproduced ?\n>>\n>> Please see https://gitlab.com/vicamo/libcamera/-/jobs/650449281\n> \n> That's lots of failures :-S\n> \n> The process test has been part of our test suite for a long time, and\n> it's not run as root or with CAP_SYS_ADMIN. As far as I can tell, we've\n> never noticed any issue with unshare() failing. I'm not sure what's\n> different in your environment.\n\nThis is executed in a unprivileged docker container. And since unshare()\ntakes CAP_SYS_ADMIN, if that doesn't fail in your setup, it follows\neither that test has never been enrolled or its executed with\nCAP_SYS_ADMIN somehow.\n\n> We can also consider dropping the unshare() call for now, as it's only a\n> partial implementation of process isolation. We would need to implement\n> that feature down the line though. Wrapping the ipa_proxy_worker with\n> minijail or firejail, or implementing isolation in the worker itself,\n> are two possible candidates. Another option would be to run the proxy\n> worker as a system daemon, but at this point we would like to avoid\n> going down that route if possible.\n\nminijail/firejail seem sufficient to me. It's already a separate\nprocess. There are many other ways to contain it without touching the\nsource code.\n\n> Do you have any recommendation ?\n\nNot really. But as an end user, I would really love to see you drop all\nthose incomplete/error-prone EventBlahBlah/Signal/Timer stuff with Boost\nio_service/process/... since you're already using boost.\n\nYou-Sheng Yang","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 910B7BD86F\n\tfor ;\n\tTue, 28 Jul 2020 03:39:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E66C36114F;\n\tTue, 28 Jul 2020 05:39:52 +0200 (CEST)","from mail-pg1-f195.google.com (mail-pg1-f195.google.com\n\t[209.85.215.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4EEE36039F\n\tfor ;\n\tTue, 28 Jul 2020 05:39:52 +0200 (CEST)","by mail-pg1-f195.google.com with SMTP id o13so11138990pgf.0\n\tfor ;\n\tMon, 27 Jul 2020 20:39:52 -0700 (PDT)","from [10.101.46.193] (61-220-137-37.HINET-IP.hinet.net.\n\t[61.220.137.37]) by smtp.gmail.com with ESMTPSA id\n\tj94sm1076013pje.44.2020.07.27.20.39.47\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 27 Jul 2020 20:39:48 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:autocrypt\n\t:message-id:date:user-agent:mime-version:in-reply-to;\n\tbh=VO76C7zniM0lgFVfu07s6VAZBH+QSwWspJnW6AD2WKE=;\n\tb=j5Er8Rvuoxwu5NYaDlNNPgmQJC4kJEVLmAQ0aso8qoCl6aGQhovpICyOMM3Irf9cQw\n\t/uyS6SwyQY9Qbt0lW3D+ZnhhRbxYsJLwvwDyiCRU7xdPPYnaNxezFz7iwzAcYjkTNyMj\n\tQS3cFYszukQ2aGOjdnyrpM2CLXXqkBbP+BjXOgdFvFyw2Ep5Pz6TXnrIK0TwHAHslP6+\n\tIVEeWTOeex2iGtMr0cy90c8h5E5+UhMH3DqtrhhOdDRtgXFI7Vy7wG2KZnBwHFKK7rc3\n\teDBtCiQuZERyTfkeSX8r7cdNPqC1xy/OEDHWSvVt2cB8J5iFvRJgcQF7WGi0XffTHmDz\n\tiV7g==","X-Gm-Message-State":"AOAM530i6Rzw81NIfdZywmpfWH0zMktet0nzh4gXyufcOD69nfnZVsZq\n\tITrPTXLj7sBe4a+08GlVnfblhuFBv+M=","X-Google-Smtp-Source":"ABdhPJy+IXTR0SLQSLS05zblN4dQD9wzaEO96VH5ZC7U5syOevrOfCfrrX/B46watuaBJ+rYQwymJw==","X-Received":"by 2002:a05:6a00:14c7:: with SMTP id\n\tw7mr1241228pfu.243.1595907589700; \n\tMon, 27 Jul 2020 20:39:49 -0700 (PDT)","To":"Laurent Pinchart ","References":"<20200725122442.1679820-1-vicamo.yang@canonical.com>\n\t<20200725122442.1679820-3-vicamo.yang@canonical.com>\n\t<20200726234211.GL28704@pendragon.ideasonboard.com>\n\t<20200726234712.GM28704@pendragon.ideasonboard.com>\n\t\n\t<20200727235850.GF15448@pendragon.ideasonboard.com>","From":"You-Sheng Yang ","Autocrypt":"addr=vicamo.yang@canonical.com; keydata=\n\tmQINBFxnlfIBEAC2RZLjA5pfvBm/uOPB++2AC5Z+hie/zQnaiwoS+4p1pVeZ80lTPdS57b89\n\tH0k3mD6cwF7lLPmUeL6Gi4vriRsiZNiU9ZWS3AVol1YsAQhidJ5aSGOLn1Vhari9NQYwPYjM\n\t+MzbzBtjdaUolvBAGqmWFNUtJ2+C43CSKUykDFxHz5NeYE78z3g/2R4MdIvlTO0vQRQM0eNf\n\tprpdriEUjHBbMGZFkHNA0cO9WqyT/hztlwEZkP+nGje+oBeNKNlxCy1zXtQPBrFwlisWLycj\n\tDF4St3YzMm6Yv7l4Jz+dO7EUkJcKTlhA6QimF4o0u61ebZ9szemrMHkcK+inRwNVlfILZvIO\n\tLOUUks7ExzvtxD66mIrjgqcGcKAU9plc7lSqUWvfKHgiWwU/56Sb8y4BprsWKiGEUWytUGu1\n\tSZclJIibcyG0Ookxx43y00YvCCJAy7svkfJJMu7W6+9vpaTAdvUz5GOr9qncxrHXNR2JD9uy\n\tf0S7DXVKDBDhgmrNt2bg1FeP/Y9Nz2U/9SMeV6zNwZBwHos5AxAlY3x0IAAk+GZ6gpjdUXY2\n\tGTb1Y1l9RUp/untzo76ytRs6m8BAdwRjWdBAgQ7xMZFpWTD2Unhi45QAXtHd+WgSi0Nwin/W\n\tyzVOoWffgS0Z8+xgOBVOs4HKsb1rr0CwcfJa+bsD4JwxRnAkFwARAQABtCpZb3UtU2hlbmcg\n\tWWFuZyA8dmljYW1vLnlhbmdAY2Fub25pY2FsLmNvbT6JAlQEEwEKAD4WIQSf4T7aw75OM7ft\n\t1VTU3r32YVqihAUCXG3YPgIbAQUJA8JnAAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAKCRDU\n\t3r32YVqihLkZD/9/BSCD2cYtBap+UqoZMXRU1GkzT6upy+/HmTBEza+RDDoGWtWbHt7hgUyg\n\tKEL2Sl4E1Bkurm9OQg1Zc8gU3dcpIzyWuXLBXNlORtbqiApob+6JwTFC7mareCIeK42QOPcV\n\tOK+wZZQTHjIhqR/FyycFzvNGiKlzBHHRzlSrKSV/vm7grwui04OqddOdlWDtVfO4fQMYTpWC\n\tjsOKkgFJWtf2uMzXwH/vPmk3P9XvTT6N+U2l01KiSMv3rRQw6VeLXK10Gg+q4PbdPZP4gNUu\n\ty2u/KECWNw18L+Y3N004wsNC68W073w9bbTh0GbpAxHpqIAGbk5s7aOOhl2MO9PxSvP7bVju\n\t7msN7fowXU8dqFQ6noOkGPoN75osTWHrdHeWjw5It9qyXm0/TAlbsRTrMUbg3mCUJQuRHDv5\n\tLVOdCvAUSyobAQq/583GP4S08jRr51AOelcsMq+bVZdHb7gIdE3LDNlfqlbu/NfihJdcDTpo\n\tDTRg1XO7xXZc2Sud4QSQCF6RSkUFbXR6IncLLmVMmU45mQQGqMqnk3jJFqkz+mapxe7kYvd6\n\tVHB42vpdK+l30eODzU65owqvH36W+5cvHp+raj89+z8KysNJksVAeuZeqydXN15/x3xuFRlJ\n\txas+mLayG02U0uSqvjaIuLJXqKD8GvB9BONZufyMecQL13+iI7QzWW91LVNoZW5nIFlhbmcg\n\tKE1vYmlsZSkgPHZpY2Ftby55YW5nQGNhbm9uaWNhbC5jb20+iQJUBBMBCgA+FiEEn+E+2sO+\n\tTjO37dVU1N699mFaooQFAlx1UYwCGwEFCQPCZwAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AA\n\tCgkQ1N699mFaooQ/0g/9FrRRrl+P7orbxYuQmjF/65VHn3H99di5TzkEmobhrFIX5c/5VEF8\n\t6pwxtCnYnUyf+0on8HyvBtfiZfcA8bvUoqrPiu5Xr+46BvDU6DSq62QjDXv0brSLvPOdZmsy\n\tcrxNFhuODvYFsUxZSLxsVljhcbOIRv0ISguyHIqiuxjYlkIQ/QJ3r6ZBFL44lDm9RfuxcHWk\n\tyMljUVj3JVhh15Nu0rQnyMcTObVinZMqbWPf9G8lPYdRH7nI9XL1f8odsTDPn8MshORnmOmS\n\taESf+6NQZtR6pF2p2l9IWQc1ABBkIAjRrfen3SFylItm8b4vosbeNS4vltSl1pli2U1RzMJ4\n\tZgeOQJO7pd8MzTRY+RCQ1CqN9PEhtxoDnLdyhAubRTotQ+YZOcMOUJ+uHM1d/yvRe6sp04gS\n\tOw17s52fX3U8kiBbLQp0QRzv5gUX46Y3vDdkd5a6lbLQFgYNtosFvrwrdRwMOfKYw4Or7xcj\n\tYUhHsC5CaihUjp7d7nt2YGIXsDjnAUvILU4cA967bWfknEJaK0NY3BYN6Vxf6GL7g8pXug3l\n\tPd3yVkoSEP+pTu+EZtymI6SHcIJZLNqxNoKneDIYLebHkMsNq+6NdF2KZ8M1amD5nbY3kUdq\n\t/EJKItxjgnuMYm/eGPq6byZQVirZIA58AvFS5PMHpvytHvYhBflMLB+5AQ0EXGeWyQEIALMb\n\tD2wCNDvLCJD79AYjIX9mDpHzJtkKX8Uh6MtAybfUzZP7R4qKOFBRZOH94e59Jx7D1O3eD0KZ\n\tW8CXqdx5pqBtssTOA1We4zfOe7f1XLDaDvl62TXQYqufGllOuIIZ49IgtEYAbSrFtyC/qbRk\n\tt58ophBlJoDRkBln/Uo0l5RtCkNucKXtEoy+N8unJzHEEdi9BxOW4DxqiTPhRKso8BekAeZO\n\tT/RF5ka3JXaJlyFBk08XLTtk8Fw2RnHvi7zVdx45GuvLxT0tVwkjZfklOiOoBLbWuNr+ghv9\n\tXG0Qq4pG0xexKPMQN2l+1ap9oeiH/CAPaK/o0XrwVwPWOQTIZiMAEQEAAYkCNgQYAQoAIBYh\n\tBJ/hPtrDvk4zt+3VVNTevfZhWqKEBQJcZ5bJAhsMAAoJENTevfZhWqKEZxMP/2WqtBXPWPPi\n\t/pcRkrYQkkVZL3yzHB1hKeGbtwvaABRD7KUg5Mm3Z8VIINK6pet9qXpXEaX4g1Ch7Arb8kzY\n\tIH535jdwcfE2eEbWg55HQUqu1G/OQ4E3bmrXNe8WBQXrKlJjqK4Xo02tUjbSBobRE++6O8Yb\n\tHig84jZlBpYBDNqixvaaASM1/NA7pvasuMFpGjw+ULvWbRTR2euTsACUIZCcmpBytrX6Q1lx\n\tWwIyPvVO1Ns0PW7F832xMkKS1Y3Ntha5bi9j+Inh0NV2Q59gen6Oo8GQJsmjA10L2/QFeIsM\n\teT+w6WIrFJt19yY/OLtVg5dFv7mAeCx1KefpdGjRDx4MH01uqypG/+UKf8bmkF0TYGd8/iXp\n\t2w7En8D9HIM+/Rm+KmNjQ7QgaTxvYEqC8R0y2yIfHiHwyp3SQw1COKT9jIMdmCbrUV99OFcu\n\tqifhMOJJ3hFFpEtNzGKL7yoKVop7PWMufwgzB6aALqxtZah+ibrKyaKce1p/sbxxp/ekUpwa\n\tgyJn0L3coWrgOCMsifiL1sifJ2cK9Z4NCRzCMsJdLtHSrIbAG2Hxm8vaLOLLSaeK/1tVY/Qi\n\try5WlCi6uVuNbwuAfMiK4jOnBPDYWTPFQtpg59XLXTq1xGPhA4RD5XjMmuvp7mJXFsvvlda/\n\tpsgobKXZGwvpcJsTTesykaeYuQGNBFx1T6UBDADqO+s9eLWQ3fr4njPoLQ8ff4pGoXgZqu0O\n\tCcn0LoqVnaLZzIfsUZ4ONp+y2S81sJL82AKAOuJ5Kq5REg+xntPBLSs326JzfhuoTOmP4m2h\n\tXhyoem3BPPqJnFcJdr6/HE7QuH0Whdv+PVe55S/iXwHPQddpz9fEcHy3SleHGljPINCn1G4F\n\t5CNV07kS7MS6Zx2HeofHcvUECunARrwuFqMlFAn5u580ORhmCZ+ha0+B4stL+ZUDNAX7ADjb\n\tcvtxUS0vdbRRrZVc/mK4Weqsb8vNSgRbKdLZlwDvEhWHWIIG4lfLXGmbvLsUFMa3cU9rl2oH\n\tWeh+GUIMfuUJfOryzl5UO1hFAn31zs9GAC0/RtTOotOEm/t3zWbvFai5zmGeWU2ZAQb+sRMX\n\tuZLSjxJklcSCCJsG9k+PaBOyzjdj3U1XWp/aUb+bfGiN4VijBVozWkLndMcNt3IL6YRR+uX/\n\tvP8XgEL0kEvx4a7qtBUZNxLF00Hy5q3FRWPnt3A7RU2TD7MAEQEAAYkD7AQYAQoAIBYhBJ/h\n\tPtrDvk4zt+3VVNTevfZhWqKEBQJcdU+lAhsCAcAJENTevfZhWqKEwPQgBBkBCgAdFiEES1bV\n\ta9nnnyj3TuTG4eTfmHHSmlMFAlx1T6UACgkQ4eTfmHHSmlO+PAwAthzvSuazTk4oFYRFDj1Q\n\tzQSwcTUVFw5jW4i4gNrbb5066UDdVmoTsTeY8OpBLGqBPVKUWhFhMxvF2uxmYTAjZFCvfabS\n\ts+PW+cbb9NfRZMKD8KUj2SRWZY2zcRXTwYtnIj3+SEDk+AB5NQuBG63zDecV2Af1+n9HXD+X\n\tsckKCNUHVYH1L2Bps5wnhzwbIboMSOjY6P3n+8ztuL6De4kzLqpJFq9b/5IB7bffns7WCdkZ\n\tkbET9d0uufKMQR2z/WJJYC/oVSUg445lhqU4SVXAwZjSG5nQsPRreuwjuFT78ExRjxtzohk3\n\tobLh+v0NhXK1QH+88ypBFVjB7IdnUHY4itJBQGJhSWTwXta2uYzxMzsMj8P+o1wN79DfG2gy\n\tuDSIwecGB6HtyDmsL5rtfKU5KhrklaYdX1bgPBS46IfpCDt3QfNKFy7icmZm1U4+xEnOkjxo\n\taJ7tUVDfC5YVtAX1B6HVczR2Up6iaWjml+yfLZSBLKbuC8/O0FfLZIs4iVaOP9YP/AqaSq7K\n\tHBEf4sY4RT1ivhVUl1nIAc7RiCHFZYPeFmygQUZ6raIyhySCNetzx+am3EGr7QIm2414IC0B\n\tciC9GAYwDR/5cca7hP8wowYWvrB+76vejXJ/g3TRxE+CnNAg6YjRsxPvhKqTwtPDjYeAbZM1\n\t9HkPK2TqogoH1BDenMfzRp7Niv5wS/nEHaLLRvViKr9k8j8alycLlFs1aDT8BJF29aRp1Mbc\n\tW8vVHCD7Ks3TYz6rf+saoA7BVDZetTE3qigbeZHtpMrWGPk7y4pidrcV/OwOhotUvKm2wHuD\n\tjU33fE+d5lJY8NZBX7cSbbFj8q6yd4jdAnCEITfuG4rfblGJMpEMbU0mrsfan05zbjchPuho\n\t6xMjG/p58xZnMtRmMy+JPG/nA2piiveObircDqeiNvSpZankQ9MggsdCFyh54ocRt+lTAeSw\n\tHUWvbN7OWSkbuwS6DWMWUEnVFhXIvRv0wn4ZM/Xc68h4IJ+lxwViCNZSuzMovJNH8sbbTtq9\n\teGCQoHAmaHhiefRstYMqpZyCTUtALQgqnRZLl83YN1U3xlzs65CfHfB0psYRiDi68HeniqSa\n\t3QoiE+kUr7jrh1xSanUdyl/g82JL570qPrCBvgE3PT8Na0xvLfImmK7dWOmDCXZetgronuP3\n\tsuzL+d2CSm1cCUYQeOxX/7MpmAIm","Message-ID":"","Date":"Tue, 28 Jul 2020 11:39:38 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200727235850.GF15448@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============1934733732570386551==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11679,"web_url":"https://patchwork.libcamera.org/comment/11679/","msgid":"<20200728193934.GJ13753@pendragon.ideasonboard.com>","date":"2020-07-28T19:39:34","subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi You-Sheng,\n\n(CC'ing Paul Elder)\n\nOn Tue, Jul 28, 2020 at 11:39:38AM +0800, You-Sheng Yang wrote:\n> On 2020-07-28 07:58, Laurent Pinchart wrote:\n> > On Mon, Jul 27, 2020 at 11:50:39PM +0800, You-Sheng Yang wrote:\n> >> On 2020-07-27 07:47, Laurent Pinchart wrote:\n> >>> On Mon, Jul 27, 2020 at 02:42:11AM +0300, Laurent Pinchart wrote:\n> >>>> On Sat, Jul 25, 2020 at 08:24:41PM +0800, You-Sheng Yang wrote:\n> >>>>> diff --git a/test/meson.build b/test/meson.build\n> >>>>> index f41d6e7..b4db328 100644\n> >>>>> --- a/test/meson.build\n> >>>>> +++ b/test/meson.build\n> >>>>> @@ -1,5 +1,7 @@\n> >>>>> # SPDX-License-Identifier: CC0-1.0\n> >>>>> \n> >>>>> +libcap = dependency('libcap', required : true)\n> >>>>\n> >>>> 'true' is the default value for 'required', you can omit it. However,\n> >>>> I'd like to keep the dependency optional, as we try to also support\n> >>>> resource-constrainted embedded systems (based on musl or uclibc for\n> >>>> instance, and/or without udev).\n> >>>>\n> >>>> I have an idea how to do that, I'll try to submit a patch shortly.\n> >>>\n> >>> Actually, thinking about it some more, would it make sense to instead\n> >>> condition the call to unshare() to CAP_SYS_ADMIN in the\n> >>> Process:isolate() class ? Or turn it into a non-fatal error ?\n> >>\n> >> It's about API design, so your opinions matter most.\n> >>\n> >> I didn't have much idea about the rational behind the unshare() call\n> >> inside libcamera::Process, but I'm really suspect the necessity of it as\n> >> part of a, at least looks like, generic API. It implicitly adds a\n> >> constrain that any process tries to create a subprocess in libcamera\n> >> using libcamera::Process, its child process must be either executed by\n> >> root or have CAP_SYS_ADMIN. This doesn't really sound a good idea for\n> >> me, especially when I believe one should really build a multimedia\n> >> library to run as a normal user as possible.\n> > \n> > The Process class is meant to run closed-source image processing\n> > algorithm (IPA) modules in a separate, isolated process. The unshare()\n> > call is a very first (mockup) step in that direction, and we know more\n> > work is needed to achieve a real sandboxing.\n> > \n> > Now that I think about it, it may be better to instead rely on minijail\n> > or firejail instead of reinventing the wheel.\n> >\n> >> Anyway, the only user of this API in libcamera is ipa_proxy_linux, you\n> >> could have put unshare() into ipa_proxy_linux itself. This way you could\n> >> install some selinux/apparmor rules to grant such permission to this\n> >> executable explicitly. But again, is that really necessary? Is\n> >> ipa_proxy_linux really has to own its own network and uid namespace?\n> > \n> > We want to isolate the IPA modules, limiting their access to the system\n> > as much as possible. They should only be able to access specific file\n> > system directories (in order to load configuration data and write logs),\n> > and nothing else (no device access, no network access, ...).\n> > Closed-source IPA modules are considered to be untrusted binaries.\n> \n> I understand. But while ipa_linux_proxy is currently integrated into\n> libcamera source, you know and can setup constrains for it correctly.\n> When some other vendor adopts libcamera and creates similar plugin by\n> their own, that may become something blocking their normal function.\n> \n> >>> Could you maybe elaborate a little bit on the failure this patch is\n> >>> trying to solve ? I haven't seen any such failure, how can they be\n> >>> reproduced ?\n> >>\n> >> Please see https://gitlab.com/vicamo/libcamera/-/jobs/650449281\n> > \n> > That's lots of failures :-S\n> > \n> > The process test has been part of our test suite for a long time, and\n> > it's not run as root or with CAP_SYS_ADMIN. As far as I can tell, we've\n> > never noticed any issue with unshare() failing. I'm not sure what's\n> > different in your environment.\n> \n> This is executed in a unprivileged docker container. And since unshare()\n> takes CAP_SYS_ADMIN, if that doesn't fail in your setup, it follows\n> either that test has never been enrolled or its executed with\n> CAP_SYS_ADMIN somehow.\n\nI've double-checked, and I can run the test successfully, without\nCAP_SYS_ADMIN. I expect the problem to be caused by calling\nunshared(CLONE_NEWUSER) within a docker container, which is something I\nhaven't tried.\n\nNonetheless, we need to address this issue. Paul, you're the author of\nthat code, do you think we could just drop the skeleton isolation for\nnow, and bring it back in another form later ?\n\n> > We can also consider dropping the unshare() call for now, as it's only a\n> > partial implementation of process isolation. We would need to implement\n> > that feature down the line though. Wrapping the ipa_proxy_worker with\n> > minijail or firejail, or implementing isolation in the worker itself,\n> > are two possible candidates. Another option would be to run the proxy\n> > worker as a system daemon, but at this point we would like to avoid\n> > going down that route if possible.\n> \n> minijail/firejail seem sufficient to me. It's already a separate\n> process. There are many other ways to contain it without touching the\n> source code.\n\nI think that's the direction we'll take.\n\n> > Do you have any recommendation ?\n> \n> Not really. But as an end user, I would really love to see you drop all\n> those incomplete/error-prone EventBlahBlah/Signal/Timer stuff with Boost\n> io_service/process/... since you're already using boost.\n\nBoost is only needed for the Raspberry Pi IPA module, for the JSON\nparser, and it's something we may consider. libcamera aims at supporting\nresource-constrained embedded systems (as much as we can reasonably do),\nhence the attempt to limit the number of dependencies. That being said,\nthis decision could be reconsidered later.\n\nRegarding timers and event notifiers, I think we'll simply drop them\nfrom the public API, and make them internal only, as libcamera's goal\nisn't to provide a generic implementation of those concepts.","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 98CAABD86F\n\tfor ;\n\tTue, 28 Jul 2020 19:39:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B2D4613C6;\n\tTue, 28 Jul 2020 21:39:46 +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 5771760923\n\tfor ;\n\tTue, 28 Jul 2020 21:39:44 +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 B5518563;\n\tTue, 28 Jul 2020 21:39:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L2NdHBbD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595965183;\n\tbh=buKlTDR0b7qBdID8d/UwxxQ3r8X462Fh8osKxXIYg/E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L2NdHBbDX5iBEMTQO+fnLKV96YUkx2vJr1LAavt8b9176GGRPW0comrMGjS90f+/F\n\tlXL2zoQi4XfU32Hv/PjjRyZWJL8UmEMaPlxExx5EbzTAt5KYTOf7pjrJMSNcRgYxQE\n\trTjvUN1rOZI0tspMHTLeksiZypcQAtrt1wS/j03Y=","Date":"Tue, 28 Jul 2020 22:39:34 +0300","From":"Laurent Pinchart ","To":"You-Sheng Yang ","Message-ID":"<20200728193934.GJ13753@pendragon.ideasonboard.com>","References":"<20200725122442.1679820-1-vicamo.yang@canonical.com>\n\t<20200725122442.1679820-3-vicamo.yang@canonical.com>\n\t<20200726234211.GL28704@pendragon.ideasonboard.com>\n\t<20200726234712.GM28704@pendragon.ideasonboard.com>\n\t\n\t<20200727235850.GF15448@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12103,"web_url":"https://patchwork.libcamera.org/comment/12103/","msgid":"<20200824091602.GA1982@pyrite.rasen.tech>","date":"2020-08-24T09:16:02","subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hello,\n\nSorry for the delay.\n\nOn Tue, Jul 28, 2020 at 10:39:34PM +0300, Laurent Pinchart wrote:\n> Hi You-Sheng,\n> \n> (CC'ing Paul Elder)\n> \n> On Tue, Jul 28, 2020 at 11:39:38AM +0800, You-Sheng Yang wrote:\n> > On 2020-07-28 07:58, Laurent Pinchart wrote:\n> > > On Mon, Jul 27, 2020 at 11:50:39PM +0800, You-Sheng Yang wrote:\n> > >> On 2020-07-27 07:47, Laurent Pinchart wrote:\n> > >>> On Mon, Jul 27, 2020 at 02:42:11AM +0300, Laurent Pinchart wrote:\n> > >>>> On Sat, Jul 25, 2020 at 08:24:41PM +0800, You-Sheng Yang wrote:\n> > >>>>> diff --git a/test/meson.build b/test/meson.build\n> > >>>>> index f41d6e7..b4db328 100644\n> > >>>>> --- a/test/meson.build\n> > >>>>> +++ b/test/meson.build\n> > >>>>> @@ -1,5 +1,7 @@\n> > >>>>> # SPDX-License-Identifier: CC0-1.0\n> > >>>>> \n> > >>>>> +libcap = dependency('libcap', required : true)\n> > >>>>\n> > >>>> 'true' is the default value for 'required', you can omit it. However,\n> > >>>> I'd like to keep the dependency optional, as we try to also support\n> > >>>> resource-constrainted embedded systems (based on musl or uclibc for\n> > >>>> instance, and/or without udev).\n> > >>>>\n> > >>>> I have an idea how to do that, I'll try to submit a patch shortly.\n> > >>>\n> > >>> Actually, thinking about it some more, would it make sense to instead\n> > >>> condition the call to unshare() to CAP_SYS_ADMIN in the\n> > >>> Process:isolate() class ? Or turn it into a non-fatal error ?\n> > >>\n> > >> It's about API design, so your opinions matter most.\n> > >>\n> > >> I didn't have much idea about the rational behind the unshare() call\n> > >> inside libcamera::Process, but I'm really suspect the necessity of it as\n> > >> part of a, at least looks like, generic API. It implicitly adds a\n> > >> constrain that any process tries to create a subprocess in libcamera\n> > >> using libcamera::Process, its child process must be either executed by\n> > >> root or have CAP_SYS_ADMIN. This doesn't really sound a good idea for\n> > >> me, especially when I believe one should really build a multimedia\n> > >> library to run as a normal user as possible.\n> > > \n> > > The Process class is meant to run closed-source image processing\n> > > algorithm (IPA) modules in a separate, isolated process. The unshare()\n> > > call is a very first (mockup) step in that direction, and we know more\n> > > work is needed to achieve a real sandboxing.\n> > > \n> > > Now that I think about it, it may be better to instead rely on minijail\n> > > or firejail instead of reinventing the wheel.\n> > >\n> > >> Anyway, the only user of this API in libcamera is ipa_proxy_linux, you\n> > >> could have put unshare() into ipa_proxy_linux itself. This way you could\n> > >> install some selinux/apparmor rules to grant such permission to this\n> > >> executable explicitly. But again, is that really necessary? Is\n> > >> ipa_proxy_linux really has to own its own network and uid namespace?\n> > > \n> > > We want to isolate the IPA modules, limiting their access to the system\n> > > as much as possible. They should only be able to access specific file\n> > > system directories (in order to load configuration data and write logs),\n> > > and nothing else (no device access, no network access, ...).\n> > > Closed-source IPA modules are considered to be untrusted binaries.\n> > \n> > I understand. But while ipa_linux_proxy is currently integrated into\n> > libcamera source, you know and can setup constrains for it correctly.\n> > When some other vendor adopts libcamera and creates similar plugin by\n> > their own, that may become something blocking their normal function.\n> > \n> > >>> Could you maybe elaborate a little bit on the failure this patch is\n> > >>> trying to solve ? I haven't seen any such failure, how can they be\n> > >>> reproduced ?\n> > >>\n> > >> Please see https://gitlab.com/vicamo/libcamera/-/jobs/650449281\n> > > \n> > > That's lots of failures :-S\n> > > \n> > > The process test has been part of our test suite for a long time, and\n> > > it's not run as root or with CAP_SYS_ADMIN. As far as I can tell, we've\n> > > never noticed any issue with unshare() failing. I'm not sure what's\n> > > different in your environment.\n> > \n> > This is executed in a unprivileged docker container. And since unshare()\n> > takes CAP_SYS_ADMIN, if that doesn't fail in your setup, it follows\n> > either that test has never been enrolled or its executed with\n> > CAP_SYS_ADMIN somehow.\n> \n> I've double-checked, and I can run the test successfully, without\n> CAP_SYS_ADMIN. I expect the problem to be caused by calling\n> unshared(CLONE_NEWUSER) within a docker container, which is something I\n> haven't tried.\n> \n> Nonetheless, we need to address this issue. Paul, you're the author of\n> that code, do you think we could just drop the skeleton isolation for\n> now, and bring it back in another form later ?\n\nYeah, I think we could just drop the skeleton isolation for now. It's\nprobably better to replace it with minijail (or something similar)\ninstead of reimplementing our own jail. I think I used unshare() just to\nshow how isolation would work/fit in.\n\n\nPaul","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 521F9BD87C\n\tfor ;\n\tMon, 24 Aug 2020 09:16:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C92F162818;\n\tMon, 24 Aug 2020 11:16:13 +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 1FDBC60387\n\tfor ;\n\tMon, 24 Aug 2020 11:16:12 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CEF94279;\n\tMon, 24 Aug 2020 11:16:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YbYd6aCV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598260570;\n\tbh=t3r1jyLpZi9PTGBgDVX6Tlke+tepBq4jukSzHUEag04=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YbYd6aCVAVqcuXXcR47LuA0DSYo+VhXOQtQVKizoJHsFdWUYbvLs2wl4H6RO/R4j8\n\tF16ONQVElMjPMXqhNlBY3KcXMY4GAaPORQu0btDN9lIvqW8J1p+vqiRi/FUHQj0DNf\n\t9csS27rGPf51pOHXNtOQxUHyHwMyv+pA/zbk9dk8=","Date":"Mon, 24 Aug 2020 18:16:02 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20200824091602.GA1982@pyrite.rasen.tech>","References":"<20200725122442.1679820-1-vicamo.yang@canonical.com>\n\t<20200725122442.1679820-3-vicamo.yang@canonical.com>\n\t<20200726234211.GL28704@pendragon.ideasonboard.com>\n\t<20200726234712.GM28704@pendragon.ideasonboard.com>\n\t\n\t<20200727235850.GF15448@pendragon.ideasonboard.com>\n\t\n\t<20200728193934.GJ13753@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200728193934.GJ13753@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] test: log/process: check\n\tCAP_SYS_ADMIN in test init","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","Cc":"You-Sheng Yang ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]