[Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Open-Source IPA shlibs need to be signed in order to be runnable within
the same process, otherwise they are deemed Closed-Source and run in
another process and communicate over IPC.

The shlib installed on the target should be the same as the one signed
by libcamera during package creation otherwise the signature won't match
the shlib.

Buildroot sanitizes RPATH in a post build process. meson gets rid of
rpath while installing so we don't need to do it manually.

Buildroot may strip symbols, so we need to do the same before signing.
Since meson install target is also signing the IPA shlibs, let's strip
them before this happens.

Cc: Quentin Schulz <foss+buildroot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---

v2:
 - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
 LIBCAMERA_INSTALL_TARGET_CMDS,
 - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
 shouldn't,
 - added --no-run-if-empty to xargs, in case no IPA is selected,
 - removed stderr redirect and pipe to true to not hide useful
 information or fail the build if strip does not work,

 package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
index 77381ab3ca..41d6a5abef 100644
--- a/package/libcamera/libcamera.mk
+++ b/package/libcamera/libcamera.mk
@@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
 LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
 endif
 
+# Open-Source IPA shlibs need to be signed in order to be runnable within the
+# same process, otherwise they are deemed Closed-Source and run in another
+# process and communicate over IPC.
+# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
+# while installing so we don't need to do it manually here.
+# Buildroot may strip symbols, so we need to do the same before signing
+# otherwise the signature won't match the shlib on the rootfs. Since meson
+# install target is signing the shlibs, we need to strip them before.
+LIBCAMERA_STRIP_FIND_CMD = \
+	find $(@D)/build/src/ipa \
+	$(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
+		-not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
+	-type f -name 'ipa_*.so' -print0
+
+define LIBCAMERA_BUILD_STRIP_IPA_SO
+	$(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
+endef
+
+LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
+
 $(eval $(meson-package))
-- 
2.35.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Quentin Schulz]

Hi all,

Any feedback to offer on this patch?

Cheers,
Quentin

On 5/6/22 12:46, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> Open-Source IPA shlibs need to be signed in order to be runnable within
> the same process, otherwise they are deemed Closed-Source and run in
> another process and communicate over IPC.
> 
> The shlib installed on the target should be the same as the one signed
> by libcamera during package creation otherwise the signature won't match
> the shlib.
> 
> Buildroot sanitizes RPATH in a post build process. meson gets rid of
> rpath while installing so we don't need to do it manually.
> 
> Buildroot may strip symbols, so we need to do the same before signing.
> Since meson install target is also signing the IPA shlibs, let's strip
> them before this happens.
> 
> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
> 
> v2:
>   - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
>   LIBCAMERA_INSTALL_TARGET_CMDS,
>   - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
>   shouldn't,
>   - added --no-run-if-empty to xargs, in case no IPA is selected,
>   - removed stderr redirect and pipe to true to not hide useful
>   information or fail the build if strip does not work,
> 
>   package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> index 77381ab3ca..41d6a5abef 100644
> --- a/package/libcamera/libcamera.mk
> +++ b/package/libcamera/libcamera.mk
> @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
>   LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
>   endif
>   
> +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> +# same process, otherwise they are deemed Closed-Source and run in another
> +# process and communicate over IPC.
> +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> +# while installing so we don't need to do it manually here.
> +# Buildroot may strip symbols, so we need to do the same before signing
> +# otherwise the signature won't match the shlib on the rootfs. Since meson
> +# install target is signing the shlibs, we need to strip them before.
> +LIBCAMERA_STRIP_FIND_CMD = \
> +	find $(@D)/build/src/ipa \
> +	$(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> +		-not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> +	-type f -name 'ipa_*.so' -print0
> +
> +define LIBCAMERA_BUILD_STRIP_IPA_SO
> +	$(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> +endef
> +
> +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> +
>   $(eval $(meson-package))
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Quentin Schulz]

Hi all,

Gentle ping.

Cheers,
Quentin

On 5/31/22 12:13, Quentin Schulz wrote:
> Hi all,
> 
> Any feedback to offer on this patch?
> 
> Cheers,
> Quentin
> 
> On 5/6/22 12:46, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>
>> Open-Source IPA shlibs need to be signed in order to be runnable within
>> the same process, otherwise they are deemed Closed-Source and run in
>> another process and communicate over IPC.
>>
>> The shlib installed on the target should be the same as the one signed
>> by libcamera during package creation otherwise the signature won't match
>> the shlib.
>>
>> Buildroot sanitizes RPATH in a post build process. meson gets rid of
>> rpath while installing so we don't need to do it manually.
>>
>> Buildroot may strip symbols, so we need to do the same before signing.
>> Since meson install target is also signing the IPA shlibs, let's strip
>> them before this happens.
>>
>> Cc: Quentin Schulz <foss+buildroot@0leil.net>
>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>> ---
>>
>> v2:
>>   - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
>>   LIBCAMERA_INSTALL_TARGET_CMDS,
>>   - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
>>   shouldn't,
>>   - added --no-run-if-empty to xargs, in case no IPA is selected,
>>   - removed stderr redirect and pipe to true to not hide useful
>>   information or fail the build if strip does not work,
>>
>>   package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/package/libcamera/libcamera.mk 
>> b/package/libcamera/libcamera.mk
>> index 77381ab3ca..41d6a5abef 100644
>> --- a/package/libcamera/libcamera.mk
>> +++ b/package/libcamera/libcamera.mk
>> @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
>>   LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
>>   endif
>> +# Open-Source IPA shlibs need to be signed in order to be runnable 
>> within the
>> +# same process, otherwise they are deemed Closed-Source and run in 
>> another
>> +# process and communicate over IPC.
>> +# Buildroot sanitizes RPATH in a post build process. meson gets rid 
>> of rpath
>> +# while installing so we don't need to do it manually here.
>> +# Buildroot may strip symbols, so we need to do the same before signing
>> +# otherwise the signature won't match the shlib on the rootfs. Since 
>> meson
>> +# install target is signing the shlibs, we need to strip them before.
>> +LIBCAMERA_STRIP_FIND_CMD = \
>> +    find $(@D)/build/src/ipa \
>> +    $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
>> +        -not \( $(call findfileclauses,$(call 
>> qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
>> +    -type f -name 'ipa_*.so' -print0
>> +
>> +define LIBCAMERA_BUILD_STRIP_IPA_SO
>> +    $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
>> +endef
>> +
>> +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
>> +
>>   $(eval $(meson-package))
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.buildroot.org_mailman_listinfo_buildroot&d=DwICAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=Bc5tHjXGFNa8qOOOlC25b9weaSHYyUnfmrLaXxDORd1Pl9TgVQE9dpqEINWBr5VB&s=4LBeZlFtIqNFHJDBZO9mwFGWA14jLAfjt-163yXgJec&e= 
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[James Hilliard]

On Fri, May 6, 2022 at 4:47 AM Quentin Schulz <foss+buildroot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> Open-Source IPA shlibs need to be signed in order to be runnable within
> the same process, otherwise they are deemed Closed-Source and run in
> another process and communicate over IPC.

Why not just add an option to libcamera to disable signature
validation entirely?

It seems kinda pointless if one is not using closed source shlibs.

>
> The shlib installed on the target should be the same as the one signed
> by libcamera during package creation otherwise the signature won't match
> the shlib.
>
> Buildroot sanitizes RPATH in a post build process. meson gets rid of
> rpath while installing so we don't need to do it manually.
>
> Buildroot may strip symbols, so we need to do the same before signing.
> Since meson install target is also signing the IPA shlibs, let's strip
> them before this happens.
>
> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>
> v2:
>  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
>  LIBCAMERA_INSTALL_TARGET_CMDS,
>  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
>  shouldn't,
>  - added --no-run-if-empty to xargs, in case no IPA is selected,
>  - removed stderr redirect and pipe to true to not hide useful
>  information or fail the build if strip does not work,
>
>  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> index 77381ab3ca..41d6a5abef 100644
> --- a/package/libcamera/libcamera.mk
> +++ b/package/libcamera/libcamera.mk
> @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
>  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
>  endif
>
> +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> +# same process, otherwise they are deemed Closed-Source and run in another
> +# process and communicate over IPC.
> +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> +# while installing so we don't need to do it manually here.
> +# Buildroot may strip symbols, so we need to do the same before signing
> +# otherwise the signature won't match the shlib on the rootfs. Since meson
> +# install target is signing the shlibs, we need to strip them before.
> +LIBCAMERA_STRIP_FIND_CMD = \
> +       find $(@D)/build/src/ipa \
> +       $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> +               -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> +       -type f -name 'ipa_*.so' -print0

Wouldn't this make it difficult to analyze core dumps since we need unstripped
binaries for that?

> +
> +define LIBCAMERA_BUILD_STRIP_IPA_SO
> +       $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> +endef
> +
> +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> +
>  $(eval $(meson-package))
> --
> 2.35.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Kieran Bingham]

Quoting James Hilliard (2022-07-04 18:29:25)
> On Fri, May 6, 2022 at 4:47 AM Quentin Schulz <foss+buildroot@0leil.net> wrote:
> >
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >
> > Open-Source IPA shlibs need to be signed in order to be runnable within
> > the same process, otherwise they are deemed Closed-Source and run in
> > another process and communicate over IPC.
> 
> Why not just add an option to libcamera to disable signature
> validation entirely?
> 
> It seems kinda pointless if one is not using closed source shlibs.

The IPA modules can be installed separately. We detect that by
validating the signature. If the signature fails, we determine the IPA
shlib module is 'proprietary' and it gets sandboxed in a separate
process.

Providing an option to disable this would defeat the core purpose of it
entirely.

--
Kieran



> > The shlib installed on the target should be the same as the one signed
> > by libcamera during package creation otherwise the signature won't match
> > the shlib.
> >
> > Buildroot sanitizes RPATH in a post build process. meson gets rid of
> > rpath while installing so we don't need to do it manually.
> >
> > Buildroot may strip symbols, so we need to do the same before signing.
> > Since meson install target is also signing the IPA shlibs, let's strip
> > them before this happens.
> >
> > Cc: Quentin Schulz <foss+buildroot@0leil.net>
> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > ---
> >
> > v2:
> >  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
> >  LIBCAMERA_INSTALL_TARGET_CMDS,
> >  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
> >  shouldn't,
> >  - added --no-run-if-empty to xargs, in case no IPA is selected,
> >  - removed stderr redirect and pipe to true to not hide useful
> >  information or fail the build if strip does not work,
> >
> >  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> > index 77381ab3ca..41d6a5abef 100644
> > --- a/package/libcamera/libcamera.mk
> > +++ b/package/libcamera/libcamera.mk
> > @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
> >  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
> >  endif
> >
> > +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> > +# same process, otherwise they are deemed Closed-Source and run in another
> > +# process and communicate over IPC.
> > +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> > +# while installing so we don't need to do it manually here.
> > +# Buildroot may strip symbols, so we need to do the same before signing
> > +# otherwise the signature won't match the shlib on the rootfs. Since meson
> > +# install target is signing the shlibs, we need to strip them before.
> > +LIBCAMERA_STRIP_FIND_CMD = \
> > +       find $(@D)/build/src/ipa \
> > +       $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > +               -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > +       -type f -name 'ipa_*.so' -print0
> 
> Wouldn't this make it difficult to analyze core dumps since we need unstripped
> binaries for that?
> 
> > +
> > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > +       $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > +endef
> > +
> > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > +
> >  $(eval $(meson-package))
> > --
> > 2.35.1
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[James Hilliard]

On Mon, Jul 4, 2022 at 1:13 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting James Hilliard (2022-07-04 18:29:25)
> > On Fri, May 6, 2022 at 4:47 AM Quentin Schulz <foss+buildroot@0leil.net> wrote:
> > >
> > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > >
> > > Open-Source IPA shlibs need to be signed in order to be runnable within
> > > the same process, otherwise they are deemed Closed-Source and run in
> > > another process and communicate over IPC.
> >
> > Why not just add an option to libcamera to disable signature
> > validation entirely?
> >
> > It seems kinda pointless if one is not using closed source shlibs.
>
> The IPA modules can be installed separately. We detect that by
> validating the signature. If the signature fails, we determine the IPA
> shlib module is 'proprietary' and it gets sandboxed in a separate
> process.

So sandboxing is generally undesirable and kind of a hack to deal with
security issues around proprietary modules from my understanding.

>
> Providing an option to disable this would defeat the core purpose of it
> entirely.

The signature scheme from my understanding only makes sense when one
intends to mix open source and proprietary modules in the same build.

So if one isn't mixing open source and proprietary IPA modules...then one
can just disable the validation entirely and choose the desired sandboxing
on/off behavior at build time.

Mixing open source/proprietary modules at runtime seems like it would be
a pretty uncommon use case for buildroot at least.

Or am I missing something here?

>
> --
> Kieran
>
>
>
> > > The shlib installed on the target should be the same as the one signed
> > > by libcamera during package creation otherwise the signature won't match
> > > the shlib.
> > >
> > > Buildroot sanitizes RPATH in a post build process. meson gets rid of
> > > rpath while installing so we don't need to do it manually.
> > >
> > > Buildroot may strip symbols, so we need to do the same before signing.
> > > Since meson install target is also signing the IPA shlibs, let's strip
> > > them before this happens.
> > >
> > > Cc: Quentin Schulz <foss+buildroot@0leil.net>
> > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > ---
> > >
> > > v2:
> > >  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
> > >  LIBCAMERA_INSTALL_TARGET_CMDS,
> > >  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
> > >  shouldn't,
> > >  - added --no-run-if-empty to xargs, in case no IPA is selected,
> > >  - removed stderr redirect and pipe to true to not hide useful
> > >  information or fail the build if strip does not work,
> > >
> > >  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > >
> > > diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> > > index 77381ab3ca..41d6a5abef 100644
> > > --- a/package/libcamera/libcamera.mk
> > > +++ b/package/libcamera/libcamera.mk
> > > @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
> > >  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
> > >  endif
> > >
> > > +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> > > +# same process, otherwise they are deemed Closed-Source and run in another
> > > +# process and communicate over IPC.
> > > +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> > > +# while installing so we don't need to do it manually here.
> > > +# Buildroot may strip symbols, so we need to do the same before signing
> > > +# otherwise the signature won't match the shlib on the rootfs. Since meson
> > > +# install target is signing the shlibs, we need to strip them before.
> > > +LIBCAMERA_STRIP_FIND_CMD = \
> > > +       find $(@D)/build/src/ipa \
> > > +       $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > > +               -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > > +       -type f -name 'ipa_*.so' -print0
> >
> > Wouldn't this make it difficult to analyze core dumps since we need unstripped
> > binaries for that?
> >
> > > +
> > > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > > +       $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > > +endef
> > > +
> > > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > > +
> > >  $(eval $(meson-package))
> > > --
> > > 2.35.1
> > >
> > > _______________________________________________
> > > buildroot mailing list
> > > buildroot@buildroot.org
> > > https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Kieran Bingham]

Hi James,

(Adding in the libcamera development mailinglist for this topic, as
we're heading into inner details of libcamera support, and even perhaps
the philosophy of it)


Quoting James Hilliard (2022-07-04 20:45:25)
> On Mon, Jul 4, 2022 at 1:13 PM Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
> >
> > Quoting James Hilliard (2022-07-04 18:29:25)
> > > On Fri, May 6, 2022 at 4:47 AM Quentin Schulz <foss+buildroot@0leil.net> wrote:
> > > >
> > > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > >
> > > > Open-Source IPA shlibs need to be signed in order to be runnable within
> > > > the same process, otherwise they are deemed Closed-Source and run in
> > > > another process and communicate over IPC.
> > >
> > > Why not just add an option to libcamera to disable signature
> > > validation entirely?

In fact, we do already have ways to enforce signature verification to be
off - but in that case, we treat all modules as if they were
'proprietary' and would isolate them.


> > > It seems kinda pointless if one is not using closed source shlibs.
> >
> > The IPA modules can be installed separately. We detect that by
> > validating the signature. If the signature fails, we determine the IPA
> > shlib module is 'proprietary' and it gets sandboxed in a separate
> > process.
> 
> So sandboxing is generally undesirable and kind of a hack to deal with
> security issues around proprietary modules from my understanding.

I'd love to hear of some alternative options too.

What libcamera provides here is a way to minimize the closed source
components as much as possible, along with providing a means to operate
devices with open alternatives.

> > Providing an option to disable this would defeat the core purpose of it
> > entirely.
> 
> The signature scheme from my understanding only makes sense when one
> intends to mix open source and proprietary modules in the same build.
> 
> So if one isn't mixing open source and proprietary IPA modules...then one
> can just disable the validation entirely and choose the desired sandboxing
> on/off behavior at build time.
> 
> Mixing open source/proprietary modules at runtime seems like it would be
> a pretty uncommon use case for buildroot at least.

It could be a scenario right now.

A Raspberry Pi build configuration, but with an Arducam Autofocus
camera.  Arducam only provide a 'binary only' IPA module for their
Autofocus implementation.

 - https://github.com/ArduCAM/Arducam-Pivariety-V4L2-Driver/releases/tag/ipa-v0.0.3

I hope that will soon be replaced by an open source version, - but this
is what there is currently. So I could envisage someone needing to add
this closed IPA component with a board overlay.

--
Kieran


> 
> Or am I missing something here?
> 
> >
> > --
> > Kieran
> >
> >
> >
> > > > The shlib installed on the target should be the same as the one signed
> > > > by libcamera during package creation otherwise the signature won't match
> > > > the shlib.
> > > >
> > > > Buildroot sanitizes RPATH in a post build process. meson gets rid of
> > > > rpath while installing so we don't need to do it manually.
> > > >
> > > > Buildroot may strip symbols, so we need to do the same before signing.
> > > > Since meson install target is also signing the IPA shlibs, let's strip
> > > > them before this happens.
> > > >
> > > > Cc: Quentin Schulz <foss+buildroot@0leil.net>
> > > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > > ---
> > > >
> > > > v2:
> > > >  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
> > > >  LIBCAMERA_INSTALL_TARGET_CMDS,
> > > >  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
> > > >  shouldn't,
> > > >  - added --no-run-if-empty to xargs, in case no IPA is selected,
> > > >  - removed stderr redirect and pipe to true to not hide useful
> > > >  information or fail the build if strip does not work,
> > > >
> > > >  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
> > > >  1 file changed, 20 insertions(+)
> > > >
> > > > diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> > > > index 77381ab3ca..41d6a5abef 100644
> > > > --- a/package/libcamera/libcamera.mk
> > > > +++ b/package/libcamera/libcamera.mk
> > > > @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
> > > >  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
> > > >  endif
> > > >
> > > > +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> > > > +# same process, otherwise they are deemed Closed-Source and run in another
> > > > +# process and communicate over IPC.
> > > > +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> > > > +# while installing so we don't need to do it manually here.
> > > > +# Buildroot may strip symbols, so we need to do the same before signing
> > > > +# otherwise the signature won't match the shlib on the rootfs. Since meson
> > > > +# install target is signing the shlibs, we need to strip them before.
> > > > +LIBCAMERA_STRIP_FIND_CMD = \
> > > > +       find $(@D)/build/src/ipa \
> > > > +       $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > > > +               -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > > > +       -type f -name 'ipa_*.so' -print0
> > >
> > > Wouldn't this make it difficult to analyze core dumps since we need unstripped
> > > binaries for that?
> > >
> > > > +
> > > > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > > > +       $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > > > +endef
> > > > +
> > > > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > > > +
> > > >  $(eval $(meson-package))
> > > > --
> > > > 2.35.1
> > > >
> > > > _______________________________________________
> > > > buildroot mailing list
> > > > buildroot@buildroot.org
> > > > https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[James Hilliard]

On Mon, Jul 4, 2022 at 3:49 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi James,
>
> (Adding in the libcamera development mailinglist for this topic, as
> we're heading into inner details of libcamera support, and even perhaps
> the philosophy of it)
>
>
> Quoting James Hilliard (2022-07-04 20:45:25)
> > On Mon, Jul 4, 2022 at 1:13 PM Kieran Bingham
> > <kieran.bingham@ideasonboard.com> wrote:
> > >
> > > Quoting James Hilliard (2022-07-04 18:29:25)
> > > > On Fri, May 6, 2022 at 4:47 AM Quentin Schulz <foss+buildroot@0leil.net> wrote:
> > > > >
> > > > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > > >
> > > > > Open-Source IPA shlibs need to be signed in order to be runnable within
> > > > > the same process, otherwise they are deemed Closed-Source and run in
> > > > > another process and communicate over IPC.
> > > >
> > > > Why not just add an option to libcamera to disable signature
> > > > validation entirely?
>
> In fact, we do already have ways to enforce signature verification to be
> off - but in that case, we treat all modules as if they were
> 'proprietary' and would isolate them.

Sorry, I meant off as in a signature unenforced sandbox off mode.

For anyone not using proprietary modules at all this would probably be the
preferred mode, at least for typical buildroot builds where one typically
does full rootfs swapping updates.

>
>
> > > > It seems kinda pointless if one is not using closed source shlibs.
> > >
> > > The IPA modules can be installed separately. We detect that by
> > > validating the signature. If the signature fails, we determine the IPA
> > > shlib module is 'proprietary' and it gets sandboxed in a separate
> > > process.
> >
> > So sandboxing is generally undesirable and kind of a hack to deal with
> > security issues around proprietary modules from my understanding.
>
> I'd love to hear of some alternative options too.
>
> What libcamera provides here is a way to minimize the closed source
> components as much as possible, along with providing a means to operate
> devices with open alternatives.

I mean the signature validation+sandboxing is for the most part a feature only
used for supporting closed source components, for those use cases that don't
need closed source component support I see no reason one would want to have
signature validation and sandboxing enabled as it just adds unnecessary
complexity as one has to deal with accidentally triggering sig validation issues
during development as opposed to just switching the closed source component
features(signature validation+sandboxing) off entirely.

>
> > > Providing an option to disable this would defeat the core purpose of it
> > > entirely.
> >
> > The signature scheme from my understanding only makes sense when one
> > intends to mix open source and proprietary modules in the same build.
> >
> > So if one isn't mixing open source and proprietary IPA modules...then one
> > can just disable the validation entirely and choose the desired sandboxing
> > on/off behavior at build time.
> >
> > Mixing open source/proprietary modules at runtime seems like it would be
> > a pretty uncommon use case for buildroot at least.
>
> It could be a scenario right now.
>
> A Raspberry Pi build configuration, but with an Arducam Autofocus
> camera.  Arducam only provide a 'binary only' IPA module for their
> Autofocus implementation.
>
>  - https://github.com/ArduCAM/Arducam-Pivariety-V4L2-Driver/releases/tag/ipa-v0.0.3
>
> I hope that will soon be replaced by an open source version, - but this
> is what there is currently. So I could envisage someone needing to add
> this closed IPA component with a board overlay.

I mean, one could still disable signature validation and force on sandboxing for
a mixed use case, but I think mixed is probably not the norm for
buildroot as most
configurations would be built for one camera and thus could just
disable sig validation
and set the sandbox unconditionally enabled at build time for closed source
components or unconditionally disabled for open source.

>
> --
> Kieran
>
>
> >
> > Or am I missing something here?
> >
> > >
> > > --
> > > Kieran
> > >
> > >
> > >
> > > > > The shlib installed on the target should be the same as the one signed
> > > > > by libcamera during package creation otherwise the signature won't match
> > > > > the shlib.
> > > > >
> > > > > Buildroot sanitizes RPATH in a post build process. meson gets rid of
> > > > > rpath while installing so we don't need to do it manually.
> > > > >
> > > > > Buildroot may strip symbols, so we need to do the same before signing.
> > > > > Since meson install target is also signing the IPA shlibs, let's strip
> > > > > them before this happens.
> > > > >
> > > > > Cc: Quentin Schulz <foss+buildroot@0leil.net>
> > > > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > > > > ---
> > > > >
> > > > > v2:
> > > > >  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
> > > > >  LIBCAMERA_INSTALL_TARGET_CMDS,
> > > > >  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
> > > > >  shouldn't,
> > > > >  - added --no-run-if-empty to xargs, in case no IPA is selected,
> > > > >  - removed stderr redirect and pipe to true to not hide useful
> > > > >  information or fail the build if strip does not work,
> > > > >
> > > > >  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
> > > > >  1 file changed, 20 insertions(+)
> > > > >
> > > > > diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> > > > > index 77381ab3ca..41d6a5abef 100644
> > > > > --- a/package/libcamera/libcamera.mk
> > > > > +++ b/package/libcamera/libcamera.mk
> > > > > @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
> > > > >  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
> > > > >  endif
> > > > >
> > > > > +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> > > > > +# same process, otherwise they are deemed Closed-Source and run in another
> > > > > +# process and communicate over IPC.
> > > > > +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> > > > > +# while installing so we don't need to do it manually here.
> > > > > +# Buildroot may strip symbols, so we need to do the same before signing
> > > > > +# otherwise the signature won't match the shlib on the rootfs. Since meson
> > > > > +# install target is signing the shlibs, we need to strip them before.
> > > > > +LIBCAMERA_STRIP_FIND_CMD = \
> > > > > +       find $(@D)/build/src/ipa \
> > > > > +       $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > > > > +               -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > > > > +       -type f -name 'ipa_*.so' -print0
> > > >
> > > > Wouldn't this make it difficult to analyze core dumps since we need unstripped
> > > > binaries for that?
> > > >
> > > > > +
> > > > > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > > > > +       $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > > > > +endef
> > > > > +
> > > > > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > > > > +
> > > > >  $(eval $(meson-package))
> > > > > --
> > > > > 2.35.1
> > > > >
> > > > > _______________________________________________
> > > > > buildroot mailing list
> > > > > buildroot@buildroot.org
> > > > > https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Yann E. MORIN]

James, All,

On 2022-07-04 11:29 -0600, James Hilliard spake thusly:
> On Fri, May 6, 2022 at 4:47 AM Quentin Schulz <foss+buildroot@0leil.net> wrote:
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
[--SNIP--]
> > Buildroot may strip symbols, so we need to do the same before signing.
> > Since meson install target is also signing the IPA shlibs, let's strip
> > them before this happens.
[--SNIP--]
> > +LIBCAMERA_STRIP_FIND_CMD = \
> > +       find $(@D)/build/src/ipa \
> > +       $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > +               -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > +       -type f -name 'ipa_*.so' -print0
> Wouldn't this make it difficult to analyze core dumps since we need unstripped
> binaries for that?

This still obeys the BR2_STRIP_EXCLUDE_FILES, so a user investigating a
coredump would be able to rebuild by adding the IPA libs to the list of
files to exclude from stripping.

It also uses STRIPCMD, which a noop when stripping is disabled (see
below).

So this leaves this package in about the same state as all other
packages, I believe.

Regards,
Yann E. MORIN.

> > +
> > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > +       $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > +endef
> > +
> > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > +
> >  $(eval $(meson-package))
> > --
> > 2.35.1
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[James Hilliard]

On Mon, Jul 4, 2022 at 2:09 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> James, All,
>
> On 2022-07-04 11:29 -0600, James Hilliard spake thusly:
> > On Fri, May 6, 2022 at 4:47 AM Quentin Schulz <foss+buildroot@0leil.net> wrote:
> > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> [--SNIP--]
> > > Buildroot may strip symbols, so we need to do the same before signing.
> > > Since meson install target is also signing the IPA shlibs, let's strip
> > > them before this happens.
> [--SNIP--]
> > > +LIBCAMERA_STRIP_FIND_CMD = \
> > > +       find $(@D)/build/src/ipa \
> > > +       $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > > +               -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > > +       -type f -name 'ipa_*.so' -print0
> > Wouldn't this make it difficult to analyze core dumps since we need unstripped
> > binaries for that?
>
> This still obeys the BR2_STRIP_EXCLUDE_FILES, so a user investigating a
> coredump would be able to rebuild by adding the IPA libs to the list of
> files to exclude from stripping.

I mean normally you would not have to rebuild since the files in the
build directory
would retain their debug symbols and only get stripped in the target directory.

>
> It also uses STRIPCMD, which a noop when stripping is disabled (see
> below).
>
> So this leaves this package in about the same state as all other
> packages, I believe.
>
> Regards,
> Yann E. MORIN.
>
> > > +
> > > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > > +       $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > > +endef
> > > +
> > > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > > +
> > >  $(eval $(meson-package))
> > > --
> > > 2.35.1
> > >
> > > _______________________________________________
> > > buildroot mailing list
> > > buildroot@buildroot.org
> > > https://lists.buildroot.org/mailman/listinfo/buildroot
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Yann E. MORIN]

Quentin, All,

Kieran, some question for you toward the end... ;-)

On 2022-05-06 12:46 +0200, Quentin Schulz spake thusly:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> Open-Source IPA shlibs need to be signed in order to be runnable within
> the same process, otherwise they are deemed Closed-Source and run in
> another process and communicate over IPC.
> 
> The shlib installed on the target should be the same as the one signed
> by libcamera during package creation otherwise the signature won't match
> the shlib.
> 
> Buildroot sanitizes RPATH in a post build process. meson gets rid of
> rpath while installing so we don't need to do it manually.
> 
> Buildroot may strip symbols, so we need to do the same before signing.
> Since meson install target is also signing the IPA shlibs, let's strip
> them before this happens.
> 
> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Applied to master, thanks.

However, this is a bit fragile, since libcamera may ultimately decide to
do the signing during the build phase (the install step is supposed to
be just about copying files around in theory).

So maybe:
 1. Buildroot needs to learn about FOO_STRIP_EXCLUDE_FILES/DIRS
 2. libcamera needs an option -Dstip-ipa=true/false
 3. libcamera.mk needs to set LIBCAMERA_STRIP_EXCLUDE_FILES/DIRS

Kieran, what do you think?

Regards,
Yann E. MORIN.

> ---
> 
> v2:
>  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
>  LIBCAMERA_INSTALL_TARGET_CMDS,
>  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
>  shouldn't,
>  - added --no-run-if-empty to xargs, in case no IPA is selected,
>  - removed stderr redirect and pipe to true to not hide useful
>  information or fail the build if strip does not work,
> 
>  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> index 77381ab3ca..41d6a5abef 100644
> --- a/package/libcamera/libcamera.mk
> +++ b/package/libcamera/libcamera.mk
> @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
>  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
>  endif
>  
> +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> +# same process, otherwise they are deemed Closed-Source and run in another
> +# process and communicate over IPC.
> +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> +# while installing so we don't need to do it manually here.
> +# Buildroot may strip symbols, so we need to do the same before signing
> +# otherwise the signature won't match the shlib on the rootfs. Since meson
> +# install target is signing the shlibs, we need to strip them before.
> +LIBCAMERA_STRIP_FIND_CMD = \
> +	find $(@D)/build/src/ipa \
> +	$(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> +		-not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> +	-type f -name 'ipa_*.so' -print0
> +
> +define LIBCAMERA_BUILD_STRIP_IPA_SO
> +	$(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> +endef
> +
> +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> +
>  $(eval $(meson-package))
> -- 
> 2.35.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Kieran Bingham]

Quoting Yann E. MORIN (2022-07-04 21:04:26)
> Quentin, All,
> 
> Kieran, some question for you toward the end... ;-)
> 
> On 2022-05-06 12:46 +0200, Quentin Schulz spake thusly:
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > 
> > Open-Source IPA shlibs need to be signed in order to be runnable within
> > the same process, otherwise they are deemed Closed-Source and run in
> > another process and communicate over IPC.
> > 
> > The shlib installed on the target should be the same as the one signed
> > by libcamera during package creation otherwise the signature won't match
> > the shlib.
> > 
> > Buildroot sanitizes RPATH in a post build process. meson gets rid of
> > rpath while installing so we don't need to do it manually.
> > 
> > Buildroot may strip symbols, so we need to do the same before signing.
> > Since meson install target is also signing the IPA shlibs, let's strip
> > them before this happens.
> > 
> > Cc: Quentin Schulz <foss+buildroot@0leil.net>
> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> Applied to master, thanks.
> 
> However, this is a bit fragile, since libcamera may ultimately decide to
> do the signing during the build phase (the install step is supposed to
> be just about copying files around in theory).
> 
> So maybe:
>  1. Buildroot needs to learn about FOO_STRIP_EXCLUDE_FILES/DIRS
>  2. libcamera needs an option -Dstip-ipa=true/false
>  3. libcamera.mk needs to set LIBCAMERA_STRIP_EXCLUDE_FILES/DIRS
> 
> Kieran, what do you think?

I think if we change how or when the signing happens on the libcamera
side - it would/should only be towards a path to simplify things so we
don't need to strip, so I don't think we need to do anything extra here
at the moment.

I think tracking the stripping of files is a pain, and we should find a
way to sign only the relevant sections which don't get stripped ;-) But
unfortunately - that's a bit more work, and not likely to happen in the
near-ish future, unless anyone thinks 'that sounds easy' and wants to try.

--
Kieran

> 
> Regards,
> Yann E. MORIN.
> 
> > ---
> > 
> > v2:
> >  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
> >  LIBCAMERA_INSTALL_TARGET_CMDS,
> >  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
> >  shouldn't,
> >  - added --no-run-if-empty to xargs, in case no IPA is selected,
> >  - removed stderr redirect and pipe to true to not hide useful
> >  information or fail the build if strip does not work,
> > 
> >  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> > index 77381ab3ca..41d6a5abef 100644
> > --- a/package/libcamera/libcamera.mk
> > +++ b/package/libcamera/libcamera.mk
> > @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
> >  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
> >  endif
> >  
> > +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> > +# same process, otherwise they are deemed Closed-Source and run in another
> > +# process and communicate over IPC.
> > +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> > +# while installing so we don't need to do it manually here.
> > +# Buildroot may strip symbols, so we need to do the same before signing
> > +# otherwise the signature won't match the shlib on the rootfs. Since meson
> > +# install target is signing the shlibs, we need to strip them before.
> > +LIBCAMERA_STRIP_FIND_CMD = \
> > +     find $(@D)/build/src/ipa \
> > +     $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > +             -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > +     -type f -name 'ipa_*.so' -print0
> > +
> > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > +     $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > +endef
> > +
> > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > +
> >  $(eval $(meson-package))
> > -- 
> > 2.35.1
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[James Hilliard]

On Mon, Jul 4, 2022 at 4:16 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Yann E. MORIN (2022-07-04 21:04:26)
> > Quentin, All,
> >
> > Kieran, some question for you toward the end... ;-)
> >
> > On 2022-05-06 12:46 +0200, Quentin Schulz spake thusly:
> > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > >
> > > Open-Source IPA shlibs need to be signed in order to be runnable within
> > > the same process, otherwise they are deemed Closed-Source and run in
> > > another process and communicate over IPC.
> > >
> > > The shlib installed on the target should be the same as the one signed
> > > by libcamera during package creation otherwise the signature won't match
> > > the shlib.
> > >
> > > Buildroot sanitizes RPATH in a post build process. meson gets rid of
> > > rpath while installing so we don't need to do it manually.
> > >
> > > Buildroot may strip symbols, so we need to do the same before signing.
> > > Since meson install target is also signing the IPA shlibs, let's strip
> > > them before this happens.
> > >
> > > Cc: Quentin Schulz <foss+buildroot@0leil.net>
> > > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >
> > Applied to master, thanks.
> >
> > However, this is a bit fragile, since libcamera may ultimately decide to
> > do the signing during the build phase (the install step is supposed to
> > be just about copying files around in theory).
> >
> > So maybe:
> >  1. Buildroot needs to learn about FOO_STRIP_EXCLUDE_FILES/DIRS
> >  2. libcamera needs an option -Dstip-ipa=true/false
> >  3. libcamera.mk needs to set LIBCAMERA_STRIP_EXCLUDE_FILES/DIRS
> >
> > Kieran, what do you think?
>
> I think if we change how or when the signing happens on the libcamera
> side - it would/should only be towards a path to simplify things so we
> don't need to strip, so I don't think we need to do anything extra here
> at the moment.
>
> I think tracking the stripping of files is a pain, and we should find a
> way to sign only the relevant sections which don't get stripped ;-) But
> unfortunately - that's a bit more work, and not likely to happen in the
> near-ish future, unless anyone thinks 'that sounds easy' and wants to try.

Easiest option for now IMO is probably a 3 state meson flag like:
-Dsandbox={on,off,signature}

That way one can default to -Dsandbox=off when not using closed source
components and only enable the signature validation when using them.

>
> --
> Kieran
>
> >
> > Regards,
> > Yann E. MORIN.
> >
> > > ---
> > >
> > > v2:
> > >  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
> > >  LIBCAMERA_INSTALL_TARGET_CMDS,
> > >  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
> > >  shouldn't,
> > >  - added --no-run-if-empty to xargs, in case no IPA is selected,
> > >  - removed stderr redirect and pipe to true to not hide useful
> > >  information or fail the build if strip does not work,
> > >
> > >  package/libcamera/libcamera.mk | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > >
> > > diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> > > index 77381ab3ca..41d6a5abef 100644
> > > --- a/package/libcamera/libcamera.mk
> > > +++ b/package/libcamera/libcamera.mk
> > > @@ -104,4 +104,24 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
> > >  LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
> > >  endif
> > >
> > > +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> > > +# same process, otherwise they are deemed Closed-Source and run in another
> > > +# process and communicate over IPC.
> > > +# Buildroot sanitizes RPATH in a post build process. meson gets rid of rpath
> > > +# while installing so we don't need to do it manually here.
> > > +# Buildroot may strip symbols, so we need to do the same before signing
> > > +# otherwise the signature won't match the shlib on the rootfs. Since meson
> > > +# install target is signing the shlibs, we need to strip them before.
> > > +LIBCAMERA_STRIP_FIND_CMD = \
> > > +     find $(@D)/build/src/ipa \
> > > +     $(if $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)), \
> > > +             -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) ) \
> > > +     -type f -name 'ipa_*.so' -print0
> > > +
> > > +define LIBCAMERA_BUILD_STRIP_IPA_SO
> > > +     $(LIBCAMERA_STRIP_FIND_CMD) | xargs --no-run-if-empty -0 $(STRIPCMD)
> > > +endef
> > > +
> > > +LIBCAMERA_POST_BUILD_HOOKS += LIBCAMERA_BUILD_STRIP_IPA_SO
> > > +
> > >  $(eval $(meson-package))
> > > --
> > > 2.35.1
> > >
> > > _______________________________________________
> > > buildroot mailing list
> > > buildroot@buildroot.org
> > > https://lists.buildroot.org/mailman/listinfo/buildroot
> >
> > --
> > .-----------------.--------------------.------------------.--------------------.
> > |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> > | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> > | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> > '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: strip symbols before signing IPA libs

[Peter Korsgaard]

>>>>> "Quentin" == Quentin Schulz <foss+buildroot@0leil.net> writes:

 > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
 > Open-Source IPA shlibs need to be signed in order to be runnable within
 > the same process, otherwise they are deemed Closed-Source and run in
 > another process and communicate over IPC.

 > The shlib installed on the target should be the same as the one signed
 > by libcamera during package creation otherwise the signature won't match
 > the shlib.

 > Buildroot sanitizes RPATH in a post build process. meson gets rid of
 > rpath while installing so we don't need to do it manually.

 > Buildroot may strip symbols, so we need to do the same before signing.
 > Since meson install target is also signing the IPA shlibs, let's strip
 > them before this happens.

 > Cc: Quentin Schulz <foss+buildroot@0leil.net>
 > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
 > ---

 > v2:
 >  - use LIBCAMERA_POST_BUILD_HOOKS instead of replacing
 >  LIBCAMERA_INSTALL_TARGET_CMDS,
 >  - add handling of BR2_STRIP_EXCLUDE_FILES to not strip files which
 >  shouldn't,
 >  - added --no-run-if-empty to xargs, in case no IPA is selected,
 >  - removed stderr redirect and pipe to true to not hide useful
 >  information or fail the build if strip does not work,

Committed to 2022.05.x and 2022.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot