unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#72828: Grafting breaks libraries signatures
@ 2024-08-27 10:46 Andrew Tropin via Bug reports for GNU Guix
  2024-08-31 19:36 ` bug#72828: libcamera module signatures Ludovic Courtès
  2024-09-02  8:37 ` bug#72828: Grafting breaks libcamera signatures Jacopo Mondi
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Tropin via Bug reports for GNU Guix @ 2024-08-27 10:46 UTC (permalink / raw)
  To: 72828

[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]

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:
https://paste.sr.ht/~abcdw/09e6d4aa66c8fb70a1e8e7e91bfef4cae5aa4e24#cam2.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:

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=4a19fe41c3

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:
https://bugs.libcamera.org/show_bug.cgi?id=231


How we can solve or workaround this problem?

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72828: libcamera module signatures
  2024-08-27 10:46 bug#72828: Grafting breaks libraries signatures Andrew Tropin via Bug reports for GNU Guix
@ 2024-08-31 19:36 ` Ludovic Courtès
  2024-09-02  6:45   ` Andrew Tropin via Bug reports for GNU Guix
  2024-09-02  8:37 ` bug#72828: Grafting breaks libcamera signatures Jacopo Mondi
  1 sibling, 1 reply; 8+ messages in thread
From: Ludovic Courtès @ 2024-08-31 19:36 UTC (permalink / raw)
  To: Andrew Tropin; +Cc: 72828

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’.




^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72828: libcamera module signatures
  2024-08-31 19:36 ` bug#72828: libcamera module signatures Ludovic Courtès
@ 2024-09-02  6:45   ` Andrew Tropin via Bug reports for GNU Guix
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Tropin via Bug reports for GNU Guix @ 2024-09-02  6:45 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 72828

[-- Attachment #1: Type: text/plain, Size: 2959 bytes --]

On 2024-08-31 21:36, Ludovic Courtès wrote:

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72828: Grafting breaks libcamera signatures
  2024-08-27 10:46 bug#72828: Grafting breaks libraries signatures Andrew Tropin via Bug reports for GNU Guix
  2024-08-31 19:36 ` bug#72828: libcamera module signatures Ludovic Courtès
@ 2024-09-02  8:37 ` Jacopo Mondi
  2024-09-04 16:42   ` Ludovic Courtès
  1 sibling, 1 reply; 8+ messages in thread
From: Jacopo Mondi @ 2024-09-02  8:37 UTC (permalink / raw)
  To: 72828

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

> 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.




^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72828: Grafting breaks libcamera signatures
  2024-09-02  8:37 ` bug#72828: Grafting breaks libcamera signatures Jacopo Mondi
@ 2024-09-04 16:42   ` Ludovic Courtès
  2024-09-04 17:42     ` Andrew Tropin via Bug reports for GNU Guix
  0 siblings, 1 reply; 8+ messages in thread
From: Ludovic Courtès @ 2024-09-04 16:42 UTC (permalink / raw)
  To: Jacopo Mondi; +Cc: 72828

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).

Thanks for chiming in!

Ludo’.




^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72828: Grafting breaks libcamera signatures
  2024-09-04 16:42   ` Ludovic Courtès
@ 2024-09-04 17:42     ` Andrew Tropin via Bug reports for GNU Guix
  2024-09-05  6:46       ` Andrew Tropin via Bug reports for GNU Guix
  2024-09-05  6:56       ` Jacopo Mondi
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Tropin via Bug reports for GNU Guix @ 2024-09-04 17:42 UTC (permalink / raw)
  To: Ludovic Courtès, Jacopo Mondi; +Cc: 72828


[-- Attachment #1.1: Type: text/plain, Size: 3519 bytes --]

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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-libcamera-ipa_manager-Disable-signature-verification.patch --]
[-- Type: text/x-patch, Size: 1663 bytes --]

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


[-- Attachment #1.3: Type: text/plain, Size: 64 bytes --]


Everyone is ok with it?

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#72828: Grafting breaks libcamera signatures
  2024-09-04 17:42     ` Andrew Tropin via Bug reports for GNU Guix
@ 2024-09-05  6:46       ` Andrew Tropin via Bug reports for GNU Guix
  2024-09-05  6:56       ` Jacopo Mondi
  1 sibling, 0 replies; 8+ messages in thread
From: Andrew Tropin via Bug reports for GNU Guix @ 2024-09-05  6:46 UTC (permalink / raw)
  To: Ludovic Courtès, Jacopo Mondi; +Cc: 72828-done

[-- Attachment #1: Type: text/plain, Size: 5933 bytes --]

On 2024-09-04 21:42, Andrew Tropin wrote:

> 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.

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b0e224566f

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72828: Grafting breaks libcamera signatures
  2024-09-04 17:42     ` Andrew Tropin via Bug reports for GNU Guix
  2024-09-05  6:46       ` Andrew Tropin via Bug reports for GNU Guix
@ 2024-09-05  6:56       ` Jacopo Mondi
  1 sibling, 0 replies; 8+ messages in thread
From: Jacopo Mondi @ 2024-09-05  6:56 UTC (permalink / raw)
  To: Andrew Tropin; +Cc: Jacopo Mondi, 72828, Ludovic Courtès

[-- Attachment #1: Type: text/plain, Size: 4095 bytes --]

Hi Andrew, Ludovic,

On Wed, Sep 04, 2024 at 09:42:17PM GMT, Andrew Tropin wrote:
> 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 :)

>
> --
> Best regards,
> Andrew Tropin




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-09-05  7:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-27 10:46 bug#72828: Grafting breaks libraries signatures Andrew Tropin via Bug reports for GNU Guix
2024-08-31 19:36 ` bug#72828: libcamera module signatures Ludovic Courtès
2024-09-02  6:45   ` Andrew Tropin via Bug reports for GNU Guix
2024-09-02  8:37 ` bug#72828: Grafting breaks libcamera signatures Jacopo Mondi
2024-09-04 16:42   ` Ludovic Courtès
2024-09-04 17:42     ` Andrew Tropin via Bug reports for GNU Guix
2024-09-05  6:46       ` Andrew Tropin via Bug reports for GNU Guix
2024-09-05  6:56       ` Jacopo Mondi

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).