[libcamera-devel,2/2] media-libs/libcamera: Do not strip IPA binaries
diff mbox series

Message ID 20201109011656.2560957-3-niklas.soderlund@ragnatech.se
State Not Applicable
Delegated to: Niklas Söderlund
Headers show
Series
  • media-libs/libcamera: Small fixes for IPA builds
Related show

Commit Message

Niklas Söderlund Nov. 9, 2020, 1:16 a.m. UTC
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.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 media-libs/libcamera/libcamera-9999.ebuild | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Tomasz Figa Nov. 10, 2020, 10:08 a.m. UTC | #1
Hi Niklas,

On Mon, Nov 9, 2020 at 10:17 AM Niklas Söderlund
<niklas.soderlund@ragnatech.se> 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.

Best regards,
Tomasz

> Signed-off-by: Niklas Söderlund <niklas.soderlund@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/
>  }
> --
> 2.25.1
>
Laurent Pinchart Nov. 26, 2020, 4:33 p.m. UTC | #2
Hi Tomasz,

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 ?

> > Signed-off-by: Niklas Söderlund <niklas.soderlund@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/
> >  }
Tomasz Figa Nov. 26, 2020, 4:39 p.m. UTC | #3
On Fri, Nov 27, 2020 at 1:33 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Tomasz,
>
> 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.

>
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@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
Laurent Pinchart Dec. 4, 2020, 11:43 p.m. UTC | #4
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@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/
> > > >  }
Niklas Söderlund Dec. 5, 2020, 9:20 a.m. UTC | #5
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@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
Laurent Pinchart Dec. 5, 2020, 2:15 p.m. UTC | #6
Hi Niklas,

On Sat, Dec 05, 2020 at 10:20:19AM +0100, Niklas Söderlund wrote:
> On 2020-12-05 01:43:13 +0200, Laurent Pinchart wrote:
> > 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.

Shipping the private key on the target would indeed make this all a bit
useless :-) When I mentioned saving the private key, I meant saving it
in a secure location on the integrator's side, the same way a secureboot
or kernel module signing key would be saved.

If there's no hook we can use post-stripping and pre-packaging, one
option would be to strip the IPA modules manually in the src_install
hook, resign them, and mark them with dostrip -x.

> > 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@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/
> > > > > >  }
Tomasz Figa Dec. 8, 2020, 2:28 a.m. UTC | #7
Hi Laurent,

On Sat, Dec 5, 2020 at 8:43 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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.
>
> 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.
>

While I understand this mechanism for the general purpose usage, my
point is that the extra signing just adds build-time complexity (and
time spent on the extra steps), while not serving any purpose on
Chrome OS, because we enforce the integrity and authenticity of all
the files with a higher level mechanism (dm-verity).

Could we perhaps add a build option to just bypass that signing step
and always enable the isolation?

Best regards,
Tomasz

> > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@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
Laurent Pinchart Dec. 8, 2020, 2:32 a.m. UTC | #8
Hi Tomasz,

On Tue, Dec 08, 2020 at 11:28:20AM +0900, Tomasz Figa wrote:
> On Sat, Dec 5, 2020 at 8:43 AM Laurent Pinchart wrote:
> > 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.
> >
> 
> While I understand this mechanism for the general purpose usage, my
> point is that the extra signing just adds build-time complexity (and
> time spent on the extra steps), while not serving any purpose on
> Chrome OS, because we enforce the integrity and authenticity of all
> the files with a higher level mechanism (dm-verity).
> 
> Could we perhaps add a build option to just bypass that signing step
> and always enable the isolation?

Yes, but not yet, as the isolated code path is currently incomplete.
Paul's work will fix that, and we expect to merge it soon, so maybe we
can delay this change. On the other hand, this fixes operation with IPA
modules right now, which is needed for development, so we all have to
carry this patch in our local trees. Could this be merged as a temporary
workaround ?

> > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@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/
> > > > > >  }
Tomasz Figa Dec. 8, 2020, 2:40 a.m. UTC | #9
On Tue, Dec 8, 2020 at 11:32 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Tomasz,
>
> On Tue, Dec 08, 2020 at 11:28:20AM +0900, Tomasz Figa wrote:
> > On Sat, Dec 5, 2020 at 8:43 AM Laurent Pinchart wrote:
> > > 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.
> > >
> >
> > While I understand this mechanism for the general purpose usage, my
> > point is that the extra signing just adds build-time complexity (and
> > time spent on the extra steps), while not serving any purpose on
> > Chrome OS, because we enforce the integrity and authenticity of all
> > the files with a higher level mechanism (dm-verity).
> >
> > Could we perhaps add a build option to just bypass that signing step
> > and always enable the isolation?
>
> Yes, but not yet, as the isolated code path is currently incomplete.
> Paul's work will fix that, and we expect to merge it soon, so maybe we
> can delay this change. On the other hand, this fixes operation with IPA
> modules right now, which is needed for development, so we all have to
> carry this patch in our local trees. Could this be merged as a temporary
> workaround ?
>

I'm perfectly fine with this as a temporary workaround. Let's file a
bug to track the implementation of the bypass and have a TODO comment
added in the workaround, so that we don't forget about it.

> > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@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

Patch
diff mbox series

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