[libcamera-devel] [PATCH 2/2] media-libs/libcamera: Do not strip IPA binaries

[Laurent Pinchart]

Hi Tomasz,

On Fri, Nov 27, 2020 at 01:39:40AM +0900, Tomasz Figa wrote:
> On Fri, Nov 27, 2020 at 1:33 AM Laurent Pinchart wrote:
> > On Tue, Nov 10, 2020 at 07:08:53PM +0900, Tomasz Figa wrote:
> > > On Mon, Nov 9, 2020 at 10:17 AM Niklas Söderlund wrote:
> > > >
> > > > Libcamera signs its IPA modules (.so files) after they are built. The
> > > > signature is later verified when loading the IPA modules and if they do
> > > > not match the IPA is treated as a untrusted module. The CrOS build
> > > > system by default strips all binaries after the build step and modify
> > > > the IPA .so files in so they fail the signature check.
> > > >
> > > > The build system inject hooks after the post_src_install hook that
> > > > strips binaries and creates the packet that is installed on target. It
> > > > is therefor not possible to to generate the IPA module signature for the
> > > > stripped modules without also packeting the private key and doing so in
> > > > pre_pkg_preinst. Stripping and generating signatures for the IPA .so
> > > > files in src_install is not possible as the exact method for stripping
> > > > them may differ between the ebuild and the build system hook.
> > > >
> > > > Safest route is to never stripp the IPA modules. Instead of restricting
> > > > stripping of all libcamera binaries use dostrip to only disable
> > > > stripping of the IPA modules. The EAPI needs to be increased to version
> > > > 7 to support dostrip.
> > >
> > > Could we just disable the extra signing and signature verification on
> > > Chrome OS? We have integrity enforced for the whole file system by
> > > dm-verity, so there is no need to verify anything in particular
> > > components of the stack anymore.
> >
> > The signature mechanism is how we decide if an IPA module has to be
> > isolated. Once Paul's IPA IPC series gets merged, we could disable it
> > indeed, which would force isolation of all IPA modules, even the
> > open-source ones. Is that desired though ?
> 
> Could you elaborate a bit more how we decide whether to isolate or not
> based on this? I'd assume there would be integrators willing to run
> out of tree IPAs (which could be still open source) without isolation.

At the moment, we isolate all IPA modules that don't provide a valid
signature. In-tree modules are signed during the build process, and are
thus run without isolation (but in a separate thread, to replicate the
asynchronous communication mechanism of the isolated case, in order to
avoid too many differences between the two cases). Out-of-tree modules
are not signed, and are thus isolated.

We expect this mechanism to be extended with some or all of the
following:

- A flag in the module information structure to force isolated
  execution. This would be set, for instance, by the wrapper module for
  the IPU3 that loads the Intel binaries, even if the module is in-tree
  (we haven't decided on whether that will be the case though) and thus
  gets signed.

- A similar mechanism that forces isolation of modules listed in a
  configuration file.

- A method to save the private key at build time, to sign modules built
  out of tree. This could be used by Linux distributions to update IPA
  modules without having to update libcamera itself. We may also update
  the build process to import a public/private key pair instead of
  generating one.

It will ultimately be an integrator decision, as integrators will in any
case have the option of carrying local modifications to libcamera that
changes the IPA module loading and isolation mechanism. Changes that
make sense upstream should of course be merged in our tree.

Note that we also foresee changes in the isolation mechanism, at least
for Chrome OS, but possibly globally, to use an algorithm daemon. I
would however prefer not implementing this right now as we have more
urgent tasks to focus on.

> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > > > ---
> > > >  media-libs/libcamera/libcamera-9999.ebuild | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/media-libs/libcamera/libcamera-9999.ebuild b/media-libs/libcamera/libcamera-9999.ebuild
> > > > index 57ff00337309f30c..ce4183a89ef095de 100644
> > > > --- a/media-libs/libcamera/libcamera-9999.ebuild
> > > > +++ b/media-libs/libcamera/libcamera-9999.ebuild
> > > > @@ -1,7 +1,7 @@
> > > >  # Copyright 2019 The Chromium OS Authors. All rights reserved.
> > > >  # Distributed under the terms of the GNU General Public License v2
> > > >
> > > > -EAPI=6
> > > > +EAPI=7
> > > >
> > > >  CROS_WORKON_PROJECT="chromiumos/third_party/libcamera"
> > > >  CROS_WORKON_INCREMENTAL_BUILD="1"
> > > > @@ -49,4 +49,6 @@ src_install() {
> > > >         meson_src_install
> > > >
> > > >         dosym ../libcamera.so "/usr/$(get_libdir)/camera_hal/libcamera.so"
> > > > +
> > > > +       dostrip -x /usr/$(get_libdir)/libcamera/
> > > >  }

-- 
Regards,

Laurent Pinchart