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

[Laurent Pinchart]

Hi Niklas,

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.

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