[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 at 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 at 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 at 0leil.net> wrote:
> > > > >
> > > > > From: Quentin Schulz <quentin.schulz at 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 at 0leil.net>
> > > > > Signed-off-by: Quentin Schulz <quentin.schulz at 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 at buildroot.org
> > > > > https://lists.buildroot.org/mailman/listinfo/buildroot