Grafting breaks libcamera signatures

  • Done
  • quality assurance status badge
Details
3 participants
  • Andrew Tropin
  • Jacopo Mondi
  • Ludovic Courtès
Owner
unassigned
Submitted by
Andrew Tropin
Severity
normal
A
A
Andrew Tropin wrote on 27 Aug 12:46 +0200
Grafting breaks libraries signatures
(address . bug-guix@gnu.org)
87h6b6b5v3.fsf@trop.in
TLDR: libcamera checks the signatures of its own libraries, when loading
them, grafts are enabled by default in Guix, grafted libraries fails to
pass the check.


The full story:

For the last a few days I was updating and fixing libcamera package.

The last problem I faced with it is invalid signatures:

[0:44:16.200646504] [17247] DEBUG IPAManager ipa_manager.cpp:316 IPA module /gnu/store/pfh7adzzy8akkqsjj4wlnmvmbzmrfbvk-libcamera-0.3.1/lib/libcamera/ipa_soft_simple.so signature is not valid

The full log:

I thought it was because of grafts and tried to build it with
--no-grafts, it didn't help. After that I realized that the signature
created during install phase and before strip phases. I added a phase
after 'shrink-runpath and it helped:


qcam now shows the image, after that I removed --no-grafts flag and
built the package again and the problem came back (now because of
grafts).

The issue with related discussion in libcamera backtracker:


How we can solve or workaround this problem?

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmbNrpAACgkQIgjSCVjB
3rDuxA/9F7TM1ThMILaYBaBKTrjXRQVBO6DMYnFHcCSYQCSTb/esqGohp6//TLPf
GVA2kjbvkS/xg/tv205FzZdLCU4/FFEsDphqRtk5xeA0t6cdSn6GW+juhR+Ltv+4
K2Zq8zSBcmTUZ10i5pBkNMzCKdZQPShejnRkAvOP/FVhe3oUTM46fJGI7/2FJf9O
9RzCUiYUYyg7xKPedxnV2szaUtW4BCdhO/jvBTwDObmO3Kpeiv1mKPdO5QPK2vi4
pPiKHFLX6LMrGU25Qmp+DqKoC2UNKeoig0udfkq+ILrIpb4FwfOOMXbXZvUlc9Du
wUMysqj93hHfNk7VvqmLhB5+cdqnO0Mawaz9sfeIdB4J5wnRHxhSFKyOQxNv+l5q
QhqiHYN9v9E9K1l9Eq+1sHy8J1mRzUnVHkpUdSoVe13pr6oPwK5v6QOoKR6lA25q
JEGaW4oFU/q02Cx8GcBSQRD4NEYp5XdYvRjsXQAcONVc4wrOv1GBLI5nAGl6NiL2
6FoL4QOpGKrwKjvExZWqyDKGD49+2XaMSWRbyBo87pmBKuTJAGgvPrzbxZA8ly7h
xeMoZZe7zSq5MKxGyHBeqKKer3wUd7KC6XMvMcux4zOeRAwnb7yeNP78R3a0yb0l
7TMyQEM3IaFApRKUp6cjB7CwESAgVKvHl1pux0oP8veKHovo3a0=
=wFVe
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 31 Aug 21:20 +0200
control message for bug #72828
(address . control@debbugs.gnu.org)
87cyloh531.fsf@gnu.org
retitle 72828 Grafting breaks libcamera signatures
quit
L
L
Ludovic Courtès wrote on 31 Aug 21:36 +0200
libcamera module signatures
(name . Andrew Tropin)(address . andrew@trop.in)(address . 72828@debbugs.gnu.org)
87r0a4fpri.fsf@gnu.org
Hi Andrew,

Andrew Tropin <andrew@trop.in> skribis:

Toggle quote (6 lines)
> For the last a few days I was updating and fixing libcamera package.
>
> The last problem I faced with it is invalid signatures:
>
> [0:44:16.200646504] [17247] DEBUG IPAManager ipa_manager.cpp:316 IPA module /gnu/store/pfh7adzzy8akkqsjj4wlnmvmbzmrfbvk-libcamera-0.3.1/lib/libcamera/ipa_soft_simple.so signature is not valid

I was curious about those signatures so I browsed ‘ipa_module.cpp’ and
‘ipa_manager.cpp’. I wondered: what is that supposed to protect against
in the first place? Bogus LD_LIBRARY_PATH that leads users to load
third-party code instead of the intended module?

Apparently those loadable modules can be isolated in separate processes
when they lack a valid signature, or when LIBCAMERA_IPA_FORCE_ISOLATION
is set. ‘ipa_manager.cpp’ sheds some light on the rationale for so much
sophistication:

* Module isolation is based on the module licence. Open-source modules are
* loaded without isolation, while closed-source module are forcefully isolated.
* The isolation mechanism ensures that no code from a closed-source module is
* ever run in the libcamera process.

This probably makes sense in the context that the copyright owner,
Google, envisioned: presumably Android programs loading random
proprietary modules coming from the app store. But I wonder what the
point is in the context of a free GNU/Linux distro.

In Meson there’s an ‘ipa_sign_module’ boolean variable and
‘src/meson.build’ says this:

Toggle snippet (12 lines)
if openssl.found()
ipa_priv_key = custom_target('ipa-priv-key',
output : ['ipa-priv-key.pem'],
command : [gen_ipa_priv_key, '@OUTPUT@'])
config_h.set('HAVE_IPA_PUBKEY', 1)
ipa_sign_module = true
else
warning('openssl not found, all IPA modules will be isolated')
ipa_sign_module = false
endif

Perhaps we should try removing ‘openssl’ from the inputs and thus have
all the modules isolated?

Ludo’.
A
A
Andrew Tropin wrote on 2 Sep 08:45 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 72828@debbugs.gnu.org)
87tteywo2n.fsf@trop.in
On 2024-08-31 21:36, Ludovic Courtès wrote:

Toggle quote (51 lines)
> Hi Andrew,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> For the last a few days I was updating and fixing libcamera package.
>>
>> The last problem I faced with it is invalid signatures:
>>
>> [0:44:16.200646504] [17247] DEBUG IPAManager ipa_manager.cpp:316 IPA module /gnu/store/pfh7adzzy8akkqsjj4wlnmvmbzmrfbvk-libcamera-0.3.1/lib/libcamera/ipa_soft_simple.so signature is not valid
>
> I was curious about those signatures so I browsed ‘ipa_module.cpp’ and
> ‘ipa_manager.cpp’. I wondered: what is that supposed to protect against
> in the first place? Bogus LD_LIBRARY_PATH that leads users to load
> third-party code instead of the intended module?
>
> Apparently those loadable modules can be isolated in separate processes
> when they lack a valid signature, or when LIBCAMERA_IPA_FORCE_ISOLATION
> is set. ‘ipa_manager.cpp’ sheds some light on the rationale for so much
> sophistication:
>
> * Module isolation is based on the module licence. Open-source modules are
> * loaded without isolation, while closed-source module are forcefully isolated.
> * The isolation mechanism ensures that no code from a closed-source module is
> * ever run in the libcamera process.
>
> This probably makes sense in the context that the copyright owner,
> Google, envisioned: presumably Android programs loading random
> proprietary modules coming from the app store. But I wonder what the
> point is in the context of a free GNU/Linux distro.
>
> In Meson there’s an ‘ipa_sign_module’ boolean variable and
> ‘src/meson.build’ says this:
>
> --8<---------------cut here---------------start------------->8---
> if openssl.found()
> ipa_priv_key = custom_target('ipa-priv-key',
> output : ['ipa-priv-key.pem'],
> command : [gen_ipa_priv_key, '@OUTPUT@'])
> config_h.set('HAVE_IPA_PUBKEY', 1)
> ipa_sign_module = true
> else
> warning('openssl not found, all IPA modules will be isolated')
> ipa_sign_module = false
> endif
> --8<---------------cut here---------------end--------------->8---
>
> Perhaps we should try removing ‘openssl’ from the inputs and thus have
> all the modules isolated?
>
> Ludo’.

It seems by default libcamera fallbacks to loading the module with
invalid signature in a separate process, however in my case it
segfaulted and killed pipewire in addition to that. Not sure that all
the modules can be properly loaded in isolation, will clarify it with
libcamera devs.

Anyway, I think the current most reasonable solution is to remove
signing step at all, because the signaturs will be invalidated by
grafting anyway and make it work somehow (either by loading in
isolation if it's possible or by loading unsigned libraries without
signature check directly).

WDYT?

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmbVXyAACgkQIgjSCVjB
3rDkZw/+PGCWd9zrYEc1ZW77Nyd4V1l2qsYhXyVRNQEddq8eR2N0ho/ZbAsrHUFg
zIAO782FVpK/tfliccgguqnQYXZsFG+u0vmra675r89fmRgn03Dlq6OlMainEp8A
3nvDerTjSqnqSe6WpcZiQYDFAOYkuppYBjvGtmKGz6FK5OyfH+SbOeEqdwLeV0YJ
Bx8Z1ke465IfamCuZtdLS2NNfBQhhc8Z/0CbbS5wsa+3t9aQcjnrWokeh8kkpY6C
Qxbt1pr7D5/BM0bw+sQJP0iMr7exEkpe+FTbWXY/ZD+7gvkn+oeB88nyuF7LhGt3
ewfKxwVYwSkQ9s+L1h1o+GXX2rrFqy/z5QFezruSQJJdQV3qR61jtXNmNHJfSMa9
1MiCDdBmPqASDuKWVdUJYFCq/K6XdPuVacxjgIRGR8SDtTO7PiYYbiSQWscN6aOU
lzLwWkVaU37inprGAWPDYONDjE3EAHRJGqysIeJUIUR27ngx9Ew2czAk/qZbu6X6
GigS0RH419/kdDvFsKNnLP6R6fnim/r1rL11w94UNbD0cqA+Jga3Z0JRrkecYvB2
hpU7Ogvvl1ujTu7SwlqUqeBesfoginLAkS2Nh51cjajVSgSF6mwgrerNhO2CPjvW
Jlqqe39QjM4d9LW6N1omQrO+/3l44CunqHy6rMzZYv1B1gcuDk4=
=E+xZ
-----END PGP SIGNATURE-----

J
J
Jacopo Mondi wrote on 2 Sep 10:37 +0200
Grafting breaks libcamera signatures
(address . 72828@debbugs.gnu.org)
2zsqyfesu5qldhngmls7owv4aweuc5gjr5ugyurxco5bmtw3nc@vli7jiwfqf5g
Hi, I hope this mail reaches the issue tracker.

I'm one of the libcamera developer, and while I cant share any useful
opinions on the guix build issue, I would like to clarify some points
from the discussion above in order to help better understand the
context on why we sign libraries and load unsigned modules in a separate
context (as Ludo put it: why all this sophistication)

Quiting again Ludo

Toggle quote (5 lines)
> This probably makes sense in the context that the copyright owner,
> Google, envisioned: presumably Android programs loading random
> proprietary modules coming from the app store. But I wonder what the
> point is in the context of a free GNU/Linux distro.

Not exactly. In libcamera, apart from creating a library to ease all
the camera stack plumbing, we're creating an ecosystem of open-source
3A algorithms (what we call the IPA modules).

Camera vendors and ODMs which invested in products with specific
camera features, consider 3A algorithms and their tuning their secret
sauce and are usually not willing to consider releasing them as open
source with, fortunately, notable exceptions such as RaspberryPi

Please note that all the platforms libcamera supports have an
open-source 3A algorithm module available part of the main code base,
and we consider open source 3A modules our 'first class citizens' and
we're willing to develop and maintain them in libcamera mainline
branch as free software, but at this point we have to provide a way for
third-parties to load binary modules if they want to.

The alternative is to have them continue developing camera stacks
fully behind closed doors as it has been done so far.

As said, modules not built against the libcamera sources will not be
signed, as they are distributed by other means by a vendor in binary
form. To establish if a module has been built with the libcamera
sources or not, we sign it during the build with a volatile key and
validate the signature at run-time, when the IPA module is loaded.

IPA modules for which the signature is not valid (either because they
are distributed as binaries or, as in this case, because the build
system strips symbols before installing the objects) are loaded in an
isolated process and instead of being operated with direct function
calls, we have implemented an IPC mechanism to communicate with them.
This path is way less tested by our regular users and in our daily
work on libcamera. Vendors that are running their binaries as isolated
might have fixed issues here and there but maybe they have not
reported the issue and the associated fix upstream (we have no control
over this).

For this reason I don't suggest running modules as isolated, even more
if you have no reasons to do so. If all it takes is re-signing IPA modules
after stripping them as Andrew did I would really consider doing that.
L
L
Ludovic Courtès wrote on 4 Sep 18:42 +0200
(name . Jacopo Mondi)(address . jacopo.mondi@ideasonboard.com)(address . 72828@debbugs.gnu.org)
877cbrcqv6.fsf@gnu.org
Hi Jacopo,

Jacopo Mondi <jacopo.mondi@ideasonboard.com> skribis:

Toggle quote (19 lines)
> Not exactly. In libcamera, apart from creating a library to ease all
> the camera stack plumbing, we're creating an ecosystem of open-source
> 3A algorithms (what we call the IPA modules).
>
> Camera vendors and ODMs which invested in products with specific
> camera features, consider 3A algorithms and their tuning their secret
> sauce and are usually not willing to consider releasing them as open
> source with, fortunately, notable exceptions such as RaspberryPi
>
> Please note that all the platforms libcamera supports have an
> open-source 3A algorithm module available part of the main code base,
> and we consider open source 3A modules our 'first class citizens' and
> we're willing to develop and maintain them in libcamera mainline
> branch as free software, but at this point we have to provide a way for
> third-parties to load binary modules if they want to.
>
> The alternative is to have them continue developing camera stacks
> fully behind closed doors as it has been done so far.

OK, I see, thanks for explaining the context.

Toggle quote (21 lines)
> As said, modules not built against the libcamera sources will not be
> signed, as they are distributed by other means by a vendor in binary
> form. To establish if a module has been built with the libcamera
> sources or not, we sign it during the build with a volatile key and
> validate the signature at run-time, when the IPA module is loaded.
>
> IPA modules for which the signature is not valid (either because they
> are distributed as binaries or, as in this case, because the build
> system strips symbols before installing the objects) are loaded in an
> isolated process and instead of being operated with direct function
> calls, we have implemented an IPC mechanism to communicate with them.
> This path is way less tested by our regular users and in our daily
> work on libcamera. Vendors that are running their binaries as isolated
> might have fixed issues here and there but maybe they have not
> reported the issue and the associated fix upstream (we have no control
> over this).
>
> For this reason I don't suggest running modules as isolated, even more
> if you have no reasons to do so. If all it takes is re-signing IPA modules
> after stripping them as Andrew did I would really consider doing that.

Yeah, got it. The other option, with the understanding that IPA modules
are all going to be free software here, would be to dismiss both the
authentication and the isolation mechanism, possibly with a custom
patch. It seems like the change wouldn’t be too intrusive and it would
solve the problem for “grafts” as well (grafts modify files in a
non-functional way).

Thanks for chiming in!

Ludo’.
A
A
Andrew Tropin wrote on 4 Sep 19:42 +0200
(address . 72828@debbugs.gnu.org)
87ed5zwc1y.fsf@trop.in
On 2024-09-04 18:42, Ludovic Courtès wrote:

Toggle quote (53 lines)
> Hi Jacopo,
>
> Jacopo Mondi <jacopo.mondi@ideasonboard.com> skribis:
>
>> Not exactly. In libcamera, apart from creating a library to ease all
>> the camera stack plumbing, we're creating an ecosystem of open-source
>> 3A algorithms (what we call the IPA modules).
>>
>> Camera vendors and ODMs which invested in products with specific
>> camera features, consider 3A algorithms and their tuning their secret
>> sauce and are usually not willing to consider releasing them as open
>> source with, fortunately, notable exceptions such as RaspberryPi
>>
>> Please note that all the platforms libcamera supports have an
>> open-source 3A algorithm module available part of the main code base,
>> and we consider open source 3A modules our 'first class citizens' and
>> we're willing to develop and maintain them in libcamera mainline
>> branch as free software, but at this point we have to provide a way for
>> third-parties to load binary modules if they want to.
>>
>> The alternative is to have them continue developing camera stacks
>> fully behind closed doors as it has been done so far.
>
> OK, I see, thanks for explaining the context.
>
>> As said, modules not built against the libcamera sources will not be
>> signed, as they are distributed by other means by a vendor in binary
>> form. To establish if a module has been built with the libcamera
>> sources or not, we sign it during the build with a volatile key and
>> validate the signature at run-time, when the IPA module is loaded.
>>
>> IPA modules for which the signature is not valid (either because they
>> are distributed as binaries or, as in this case, because the build
>> system strips symbols before installing the objects) are loaded in an
>> isolated process and instead of being operated with direct function
>> calls, we have implemented an IPC mechanism to communicate with them.
>> This path is way less tested by our regular users and in our daily
>> work on libcamera. Vendors that are running their binaries as isolated
>> might have fixed issues here and there but maybe they have not
>> reported the issue and the associated fix upstream (we have no control
>> over this).
>>
>> For this reason I don't suggest running modules as isolated, even more
>> if you have no reasons to do so. If all it takes is re-signing IPA modules
>> after stripping them as Andrew did I would really consider doing that.
>
> Yeah, got it. The other option, with the understanding that IPA modules
> are all going to be free software here, would be to dismiss both the
> authentication and the isolation mechanism, possibly with a custom
> patch. It seems like the change wouldn’t be too intrusive and it would
> solve the problem for “grafts” as well (grafts modify files in a
> non-functional way).

On 2024-09-02 10:45, Andrew Tropin via Bug reports for GNU Guix wrote:
Toggle quote (6 lines)
> Anyway, I think the current most reasonable solution is to remove
> signing step at all, because the signaturs will be invalidated by
> grafting anyway and make it work somehow (either by loading in
> isolation if it's possible or by loading unsigned libraries without
> signature check directly).

Everything indicates that we need to disable module authentication.

Jacopo, I think I'll patch IPAManager::isSignatureValid to always return
true.


Like that:
From c99706475cde3d963a17f4f8871149711ce6c467 Mon Sep 17 00:00:00 2001
From: Andrew Tropin <andrew@trop.in>
Date: Wed, 4 Sep 2024 21:36:16 +0400
Subject: [PATCH] libcamera: ipa_manager: Disable signature verification

---
src/libcamera/ipa_manager.cpp | 28 +++++-----------------------
1 file changed, 5 insertions(+), 23 deletions(-)

Toggle diff (45 lines)
diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index cfc24d38..4fd3cf3e 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -284,33 +284,15 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,
bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const
{
-#if HAVE_IPA_PUBKEY
- char *force = utils::secure_getenv("LIBCAMERA_IPA_FORCE_ISOLATION");
- if (force && force[0] != '\0') {
- LOG(IPAManager, Debug)
- << "Isolation of IPA module " << ipa->path()
- << " forced through environment variable";
- return false;
- }
-
- File file{ ipa->path() };
- if (!file.open(File::OpenModeFlag::ReadOnly))
- return false;
-
- Span<uint8_t> data = file.map();
- if (data.empty())
- return false;
-
- bool valid = pubKey_.verify(data, ipa->signature());
+ LOG(IPAManager, Debug)
+ << "Signature verification is disabled by Guix. "
+ << "See https://issues.guix.gnu.org/72828 for more details.";
LOG(IPAManager, Debug)
<< "IPA module " << ipa->path() << " signature is "
- << (valid ? "valid" : "not valid");
+ << "not verified (verification skipped).";
- return valid;
-#else
- return false;
-#endif
+ return true;
}
} /* namespace libcamera */
--
2.45.2
Everyone is ok with it?

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmbYm/kACgkQIgjSCVjB
3rAppQ//RZelL/x7Ob/73heJW/DWoUdM1fQSy/gZujwIW2RewFcwFYb0BiScMOWt
hgombiYt2PxJdyaYEx+U7jf4mTNys/UgDKBJ3pbLDcuo9AAc9DbqBRCYrOFQmWHN
k/f9gMDxFIxn7x83qLZ3igPzhpP49x9PjX9OTdpKj9gv9fpd0CckyiQTQgTtVgt6
nq1j1yqhNIBrHJvAIgaHtqIgAiQM7UrWY/Y+CciwtkXhe7NGM+88Ikv2UBaCUvXA
9G5Yw2PM/z+/08tZrRwlIoEctSkVApF6Q7KdDryi2yT6Ok3JGie5GDncBdl9y8EF
Pi1UHqQuJ1JogGkNtbVceePQVsO8Sjle7JkQSl2jJdSNbMgRMIzEFrUMEIkJNYBS
C0pgoG8wGc+0jtaNiM8DCoifCDgW9o8EHjbE5E0li0NsIgki3kHcf4wXjCififIK
oychNTUFZZ0kvIy2RjOHy3U+cvFee5EWEr0Grx6tFM5po+C6LVXZ9ckG9r+7PW90
sIrGsbjBQqU8vt6nCt3p+n0hj/X/EujQnr94hSP142TQsy6fE1fswBfF7ltHutAv
0J2y/q4D6seMmP+DRkgh1doblBkOt0eXCdf9/OOkcnHZDTx9oUnSk6MmhgiOQ3OV
rHF+X+zDiSyTFBjmQOuemnA066Upe6zhfvkI0XQE6g4vq8xqnww=
=0yMM
-----END PGP SIGNATURE-----

A
A
Andrew Tropin wrote on 5 Sep 08:46 +0200
(address . 72828-done@debbugs.gnu.org)
87mskm8uok.fsf@trop.in
On 2024-09-04 21:42, Andrew Tropin wrote:

Toggle quote (129 lines)
> On 2024-09-04 18:42, Ludovic Courtès wrote:
>
>> Hi Jacopo,
>>
>> Jacopo Mondi <jacopo.mondi@ideasonboard.com> skribis:
>>
>>> Not exactly. In libcamera, apart from creating a library to ease all
>>> the camera stack plumbing, we're creating an ecosystem of open-source
>>> 3A algorithms (what we call the IPA modules).
>>>
>>> Camera vendors and ODMs which invested in products with specific
>>> camera features, consider 3A algorithms and their tuning their secret
>>> sauce and are usually not willing to consider releasing them as open
>>> source with, fortunately, notable exceptions such as RaspberryPi
>>>
>>> Please note that all the platforms libcamera supports have an
>>> open-source 3A algorithm module available part of the main code base,
>>> and we consider open source 3A modules our 'first class citizens' and
>>> we're willing to develop and maintain them in libcamera mainline
>>> branch as free software, but at this point we have to provide a way for
>>> third-parties to load binary modules if they want to.
>>>
>>> The alternative is to have them continue developing camera stacks
>>> fully behind closed doors as it has been done so far.
>>
>> OK, I see, thanks for explaining the context.
>>
>>> As said, modules not built against the libcamera sources will not be
>>> signed, as they are distributed by other means by a vendor in binary
>>> form. To establish if a module has been built with the libcamera
>>> sources or not, we sign it during the build with a volatile key and
>>> validate the signature at run-time, when the IPA module is loaded.
>>>
>>> IPA modules for which the signature is not valid (either because they
>>> are distributed as binaries or, as in this case, because the build
>>> system strips symbols before installing the objects) are loaded in an
>>> isolated process and instead of being operated with direct function
>>> calls, we have implemented an IPC mechanism to communicate with them.
>>> This path is way less tested by our regular users and in our daily
>>> work on libcamera. Vendors that are running their binaries as isolated
>>> might have fixed issues here and there but maybe they have not
>>> reported the issue and the associated fix upstream (we have no control
>>> over this).
>>>
>>> For this reason I don't suggest running modules as isolated, even more
>>> if you have no reasons to do so. If all it takes is re-signing IPA modules
>>> after stripping them as Andrew did I would really consider doing that.
>>
>> Yeah, got it. The other option, with the understanding that IPA modules
>> are all going to be free software here, would be to dismiss both the
>> authentication and the isolation mechanism, possibly with a custom
>> patch. It seems like the change wouldn’t be too intrusive and it would
>> solve the problem for “grafts” as well (grafts modify files in a
>> non-functional way).
>
> On 2024-09-02 10:45, Andrew Tropin via Bug reports for GNU Guix wrote:
>> Anyway, I think the current most reasonable solution is to remove
>> signing step at all, because the signaturs will be invalidated by
>> grafting anyway and make it work somehow (either by loading in
>> isolation if it's possible or by loading unsigned libraries without
>> signature check directly).
>
> Everything indicates that we need to disable module authentication.
>
> Jacopo, I think I'll patch IPAManager::isSignatureValid to always return
> true.
>
> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/ipa_manager.cpp#n285
>
> Like that:
>
> From c99706475cde3d963a17f4f8871149711ce6c467 Mon Sep 17 00:00:00 2001
> From: Andrew Tropin <andrew@trop.in>
> Date: Wed, 4 Sep 2024 21:36:16 +0400
> Subject: [PATCH] libcamera: ipa_manager: Disable signature verification
>
> ---
> src/libcamera/ipa_manager.cpp | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index cfc24d38..4fd3cf3e 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -284,33 +284,15 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,
>
> bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const
> {
> -#if HAVE_IPA_PUBKEY
> - char *force = utils::secure_getenv("LIBCAMERA_IPA_FORCE_ISOLATION");
> - if (force && force[0] != '\0') {
> - LOG(IPAManager, Debug)
> - << "Isolation of IPA module " << ipa->path()
> - << " forced through environment variable";
> - return false;
> - }
> -
> - File file{ ipa->path() };
> - if (!file.open(File::OpenModeFlag::ReadOnly))
> - return false;
> -
> - Span<uint8_t> data = file.map();
> - if (data.empty())
> - return false;
> -
> - bool valid = pubKey_.verify(data, ipa->signature());
> + LOG(IPAManager, Debug)
> + << "Signature verification is disabled by Guix. "
> + << "See https://issues.guix.gnu.org/72828 for more details.";
>
> LOG(IPAManager, Debug)
> << "IPA module " << ipa->path() << " signature is "
> - << (valid ? "valid" : "not valid");
> + << "not verified (verification skipped).";
>
> - return valid;
> -#else
> - return false;
> -#endif
> + return true;
> }
>
> } /* namespace libcamera */
> --
> 2.45.2
>
>
> Everyone is ok with it?

I've disabled signature verification, tested with cam -l and qcam, it
works good so far, will check browsers and pipewire after ci finish
building them.


Thank you everyone for participating, it helped a lot in narrowing down
and fixing the problem!

Marking issue as DONE.

I'll update corresponding ticket on libcamera bugtracker after I test
the rest of applications.

--
Best regards,
Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEKEGaxlA4dEDH6S/6IgjSCVjB3rAFAmbZU6sACgkQIgjSCVjB
3rDnfg/9FfyHceo379wYYMBwkpA40ORu9zFL54n5EJokbbccGvmG/ncKy3AawOG+
jOcAFKSzswclQHFvTFc12PYSJw2W8z+QTkiPVsmAROgrmNzxdHTyz4DvZFz5YQtj
JvX6UsKz0NELFqiYQv3hDkZjLkB20AogNHyQyJr/Zs2FpdBG2t98DME8SjEpdUr6
MEcHtB6M1tbpnWPBFG5d8oWT+oCwcLY/VjQ01Rl1TPoyzwRv8Y7QLraVtiwan3Oq
En/Yi36c3A+xeB87DVMRDGHlIpqqEIxNOnT0TdJU74c6Iz/K5G50pA8kEdSbjAE5
I83uNsnro32PCDL83TbKSxOiKRn9NSooYhPJiHu/w+lWQvbhL+srPmUdqvTW5nT9
sRucFcyNbO9o03J+zREtN0mNll0/V9rH95VU9pcw4QBGZL22w3PMEA9xIMVb3XrA
6D6wP8qK/YElh7n8eyfWcUPavlrzXAxKBv1grWq6runHRj6WfmYL6QfQgRYmGgQ5
iBlHJWXXPx0Z4vKPCFnp0wllGGv8YuL4yKcrV8Jz4osO25cItmUnW2kkSgSEBhRa
VeSXjOQ0glTcNNaWaDiKvEJa2+3noBSGwD/p2cTbYGuxYgUHas1uv3ePdMEdkyHn
D1ub2VVN1MYe8GkNrzFHNeUrg3rNPtWlQ8fgi22LY/s6p1/bG0o=
=7j2H
-----END PGP SIGNATURE-----

Closed
J
J
Jacopo Mondi wrote on 5 Sep 08:56 +0200
(name . Andrew Tropin)(address . andrew@trop.in)
nux26yyisniy2blhue6nadbgeufbnqmzm5gf7pnf3jefy275dm@nulvs2l46fwy
Hi Andrew, Ludovic,

On Wed, Sep 04, 2024 at 09:42:17PM GMT, Andrew Tropin wrote:
Toggle quote (76 lines)
> On 2024-09-04 18:42, Ludovic Courtès wrote:
>
> > Hi Jacopo,
> >
> > Jacopo Mondi <jacopo.mondi@ideasonboard.com> skribis:
> >
> >> Not exactly. In libcamera, apart from creating a library to ease all
> >> the camera stack plumbing, we're creating an ecosystem of open-source
> >> 3A algorithms (what we call the IPA modules).
> >>
> >> Camera vendors and ODMs which invested in products with specific
> >> camera features, consider 3A algorithms and their tuning their secret
> >> sauce and are usually not willing to consider releasing them as open
> >> source with, fortunately, notable exceptions such as RaspberryPi
> >>
> >> Please note that all the platforms libcamera supports have an
> >> open-source 3A algorithm module available part of the main code base,
> >> and we consider open source 3A modules our 'first class citizens' and
> >> we're willing to develop and maintain them in libcamera mainline
> >> branch as free software, but at this point we have to provide a way for
> >> third-parties to load binary modules if they want to.
> >>
> >> The alternative is to have them continue developing camera stacks
> >> fully behind closed doors as it has been done so far.
> >
> > OK, I see, thanks for explaining the context.
> >
> >> As said, modules not built against the libcamera sources will not be
> >> signed, as they are distributed by other means by a vendor in binary
> >> form. To establish if a module has been built with the libcamera
> >> sources or not, we sign it during the build with a volatile key and
> >> validate the signature at run-time, when the IPA module is loaded.
> >>
> >> IPA modules for which the signature is not valid (either because they
> >> are distributed as binaries or, as in this case, because the build
> >> system strips symbols before installing the objects) are loaded in an
> >> isolated process and instead of being operated with direct function
> >> calls, we have implemented an IPC mechanism to communicate with them.
> >> This path is way less tested by our regular users and in our daily
> >> work on libcamera. Vendors that are running their binaries as isolated
> >> might have fixed issues here and there but maybe they have not
> >> reported the issue and the associated fix upstream (we have no control
> >> over this).
> >>
> >> For this reason I don't suggest running modules as isolated, even more
> >> if you have no reasons to do so. If all it takes is re-signing IPA modules
> >> after stripping them as Andrew did I would really consider doing that.
> >
> > Yeah, got it. The other option, with the understanding that IPA modules
> > are all going to be free software here, would be to dismiss both the
> > authentication and the isolation mechanism, possibly with a custom
> > patch. It seems like the change wouldn’t be too intrusive and it would
> > solve the problem for “grafts” as well (grafts modify files in a
> > non-functional way).
>
> On 2024-09-02 10:45, Andrew Tropin via Bug reports for GNU Guix wrote:
> > Anyway, I think the current most reasonable solution is to remove
> > signing step at all, because the signaturs will be invalidated by
> > grafting anyway and make it work somehow (either by loading in
> > isolation if it's possible or by loading unsigned libraries without
> > signature check directly).
>
> Everything indicates that we need to disable module authentication.
>
> Jacopo, I think I'll patch IPAManager::isSignatureValid to always return
> true.
>
> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/ipa_manager.cpp#n285
>
> Like that:
>


>
> Everyone is ok with it?

At this point is a distro decision, either if you prefer to carry an
out-of-tree patch in your tree or tweak the build system.

Be aware that, sooner or later, the signature mechanism will be reworked and
your custom patch might not apply anymore.

Up to you :)

Toggle quote (4 lines)
>
> --
> Best regards,
> Andrew Tropin
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEtcQ9SICaIIqPWDjAcjQGjxahVjwFAmbZVjYACgkQcjQGjxah
VjwMNQ/9EZZx6uUAHkRKYUWfF9CQ0RUuzRKUjeo7z1Ab4AWu9CXyqpe9y5Ha1AU6
GgA5zY5jP5OqQ+X8Qu0yN/Dl8sdFphri0vZ75KrDshKfjHF5CUmd0f1pGDanPMmi
ASLsiQwZLUCo/a5HREU88UwvyRDI1fAU9WxGMFnLeo/7775DBq4l6csYzx2IeOis
eCq1AU98mAIqa4GHnpnu4o8q+1muedkbN0FP7Lwau47MzTD8ullAXlpdtKWtJqpz
hrak0e/J/uMPzHVJtlTo/rQVrcBQULH9W8USon+Zf1n663byGNwB7/SfEtBTEbi2
iewJvvAtWKtqwHocS4SbpurmsWG8L62GTR7TXUH7Hujc2CI4yb3urjhStPnjoRks
CmxAB+QMqKfm5iKR8PYyvAf4R4vOdfJw3SaJ+tKr6ij7B/Xa+XtRiUcbbWiAU/uN
eR/ykIAkGcR8DyAtH2Q7CS5oghnu3UGCvcRcpFVgXZebWwbYsDOw5KlQyPBXUEVJ
ols2cm7WDmYbdm0wv5nNX5ooMKRd2x8eJWNOpJnJbfaaBj8eltcad7sidpKe415L
3T7LiLYE5uUbK+MuB1ZmHrcd5piannjR9LWj39R/LMrc5PucBdOwkHi4PSLr8IaI
ARXM+hbt49oqyhsoKL/k+PwHf86sekB+GTEitFyA4UQtN8YD0fA=
=XveB
-----END PGP SIGNATURE-----


?
Your comment

This issue is archived.

To comment on this conversation send an email to 72828@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 72828
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch