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

[Niklas Söderlund]

Hello Laurent and Tomasz,

On 2020-12-05 01:43:13 +0200, Laurent Pinchart wrote:
> 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.

Quick note: Storing the generated key and signing modules at packet 
install time is one of the possible solutions for CrOS that could be 
added with little effort. I was reluctant to it as it feels a bit 
redundant to ship the things that should be signed together with the 
signing key ;-) It would however allow us to sign the IPA's after the 
CrOS striping of the binaries at packet creation time as we could resign 
them when they are installed on target.

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

-- 
Regards,
Niklas Söderlund