[libcamera-devel] [PATCH 02/11] libcamera: Add IPA module signing infrastructure

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Apr 8 02:15:31 CEST 2020


Hi Niklas,

On Wed, Apr 08, 2020 at 01:50:30AM +0300, Laurent Pinchart wrote:
> On Tue, Apr 07, 2020 at 09:48:17PM +0200, Niklas Söderlund wrote:
> > On 2020-04-04 04:56:15 +0300, Laurent Pinchart wrote:
> > > Add infrastructure to generate an RSA private key and sign IPA modules.
> > > The signatures are stored in separate files with a .sign suffix.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  src/ipa/gen-ipa-priv-key.sh |  9 +++++++++
> > >  src/ipa/ipa-sign.sh         | 10 ++++++++++
> > >  src/ipa/meson.build         |  2 ++
> > >  src/ipa/rkisp1/meson.build  | 25 +++++++++++++++++--------
> > >  src/ipa/vimc/meson.build    | 12 +++++++++++-
> > >  src/meson.build             |  5 +++++
> > >  6 files changed, 54 insertions(+), 9 deletions(-)
> > >  create mode 100755 src/ipa/gen-ipa-priv-key.sh
> > >  create mode 100755 src/ipa/ipa-sign.sh
> > > 
> > > diff --git a/src/ipa/gen-ipa-priv-key.sh b/src/ipa/gen-ipa-priv-key.sh
> > > new file mode 100755
> > > index 000000000000..2b19c001d6c5
> > > --- /dev/null
> > > +++ b/src/ipa/gen-ipa-priv-key.sh
> > > @@ -0,0 +1,9 @@
> > > +#!/bin/sh
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +# Copyright (C) 2020, Google Inc.
> > > +#
> > > +# Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > +#
> > > +# gen-ipa-priv-key.sh - Generate an RSA private key to sign IPA modules
> > > +
> > > +openssl genpkey -algorithm RSA -out "$1" -pkeyopt rsa_keygen_bits:2048
> > > diff --git a/src/ipa/ipa-sign.sh b/src/ipa/ipa-sign.sh
> > > new file mode 100755
> > > index 000000000000..d41e67e00ad0
> > > --- /dev/null
> > > +++ b/src/ipa/ipa-sign.sh
> > > @@ -0,0 +1,10 @@
> > > +#!/bin/sh
> > > +
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +# Generate a signature for an IPA module
> > 
> > The asymmetry between gen-ipa-priv-key.sh and ipa-sign.sh bugs me. One 
> > states copyright and author the other one do not :-)
> 
> Good point, I'll add a copyright notice here.
> 
> > > +
> > > +key="$1"
> > > +input="$2"
> > > +output="$3"
> > > +
> > > +openssl dgst -sha256 -sign "${key}" -out "${output}" "${input}"
> > > diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> > > index 73278a60a99f..cb4e3ab3388f 100644
> > > --- a/src/ipa/meson.build
> > > +++ b/src/ipa/meson.build
> > > @@ -10,6 +10,8 @@ config_h.set('IPA_MODULE_DIR',
> > >  
> > >  subdir('libipa')
> > >  
> > > +ipa_sign = find_program('ipa-sign.sh')
> > > +
> > >  ipas = ['rkisp1', 'vimc']
> > >  
> > >  foreach pipeline : get_option('pipelines')
> > > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build
> > > index 521518bd1237..6ccadcfbbe64 100644
> > > --- a/src/ipa/rkisp1/meson.build
> > > +++ b/src/ipa/rkisp1/meson.build
> > > @@ -1,8 +1,17 @@
> > > -rkisp1_ipa = shared_module('ipa_rkisp1',
> > > -                           'rkisp1.cpp',
> > > -                           name_prefix : '',
> > > -                           include_directories : [ipa_includes, libipa_includes],
> > > -                           dependencies : libcamera_dep,
> > > -                           link_with : libipa,
> > > -                           install : true,
> > > -                           install_dir : ipa_install_dir)
> > > +ipa_name = 'ipa_rkisp1'
> > > +
> > > +mod = shared_module(ipa_name,
> > > +                    'rkisp1.cpp',
> > > +                    name_prefix : '',
> > > +                    include_directories : [ipa_includes, libipa_includes],
> > > +                    dependencies : libcamera_dep,
> > > +                    link_with : libipa,
> > > +                    install : true,
> > > +                    install_dir : ipa_install_dir)
> > > +
> > > +custom_target(ipa_name + '.so.sign',
> > > +              input : mod,
> > > +              output : ipa_name + '.so.sign',
> > > +              command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],
> > > +              install : true,
> > > +              install_dir : ipa_install_dir)
> > 
> > The pattern used here seems to be repeating. Shall we instead add all 
> > IPAs to a list that is then looped over to generate .so.sign files for 
> > all of them to ensure they are all generated the same way?
> 
> I had troubles when I initially tried that, but I gave it another shot
> and it seems to work. I'll include that change in v2.

Backtracking a bit on this. If I move signature generation to a foreach
loop in src/ipa/meson.build, the .so.sign files will be created in
src/ipa/, not in the subdirectories. This isn't an issue at build time,
and all files will still be installed where they belong, but when
running tests from the build tree, IPA modules and their signatures will
all of a sudden be in different directories.

I can handle that at runtime by loading the signature from the parent
directory if libcamera isn't installed, but that adds more complexity to
the code. Do you think that would be better than duplicating the
custom_target until generators are fixed in meson ?

> Ideally we should use a generator for this purpose, but they don't
> support using an "object" object as the input argument as far as I can
> tell, unlike custom_target.
> 
> > > diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build
> > > index e827e75f9f91..3097a12f964a 100644
> > > --- a/src/ipa/vimc/meson.build
> > > +++ b/src/ipa/vimc/meson.build
> > > @@ -1,4 +1,7 @@
> > > -ipa = shared_module('ipa_vimc', 'vimc.cpp',
> > > +ipa_name = 'ipa_vimc'
> > > +
> > > +mod = shared_module(ipa_name,
> > > +                    'vimc.cpp',
> > >                      name_prefix : '',
> > >                      include_directories : [ipa_includes, libipa_includes],
> > >                      dependencies : libcamera_dep,
> > > @@ -6,3 +9,10 @@ ipa = shared_module('ipa_vimc', 'vimc.cpp',
> > >                      install : true,
> > >                      install_dir : ipa_install_dir,
> > >                      cpp_args : '-DLICENSE="LGPL-2.1-or-later"')
> > > +
> > > +custom_target(ipa_name + '.so.sign',
> > > +              input : mod,
> > > +              output : ipa_name + '.so.sign',
> > > +              command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],
> > > +              install : true,
> > > +              install_dir : ipa_install_dir)
> > > diff --git a/src/meson.build b/src/meson.build
> > > index d818d8b86d93..dc0e0c82b900 100644
> > > --- a/src/meson.build
> > > +++ b/src/meson.build
> > > @@ -2,6 +2,11 @@ if get_option('android')
> > >      subdir('android')
> > >  endif
> > >  
> > > +ipa_gen_priv_key = find_program('ipa/gen-ipa-priv-key.sh')
> > > +ipa_priv_key = custom_target('ipa-priv-key',
> > > +                             output : [ 'ipa-priv-key.pem' ],
> > > +                             command : [ ipa_gen_priv_key, '@OUTPUT@' ])
> > > +
> > >  subdir('libcamera')
> > >  subdir('ipa')
> > >  subdir('cam')

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list