[libcamera-devel] [PATCH v1 2/2] apps: Add ipa-verify application

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue May 9 05:34:07 CEST 2023


Hi Kieran,

On Tue, May 09, 2023 at 01:41:42AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2023-05-06 12:10:25)
> > When packaging libcamera, distributions may break IPA module signatures
> > if the packaging process strips binaries. This can be fixed by resigning
> > the modules, but the process is error-prone.
> > 
> > Add a command line ipa-verify utility that tests the signature on an IPA
> > module to help packagers. The tool takes a single argument, the path to
> > an IPA module shared object, and expects the signature file (.sign) to
> > be in the same directory.
> > 
> > In order to access the public key needed for signature verification, add
> > a static function to the IPAManager class. As the class is internal to
> > libcamera, this doesn't affect the public API.
> 
> But requires this tool to be built with access to the private internal
> APIs... I think that's ok in our build though, and this is a very
> specific tool ... and certainly would warrant this being a 'utility'
> rather than an 'app' indeed.

Any advice on how to make it so would be appreciated :-)

> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/internal/ipa_manager.h |  7 +++
> >  src/apps/ipa-verify/main.cpp             | 64 ++++++++++++++++++++++++
> >  src/apps/ipa-verify/meson.build          | 15 ++++++
> >  src/apps/meson.build                     |  2 +
> >  src/libcamera/ipa_manager.cpp            | 13 +++++
> >  5 files changed, 101 insertions(+)
> >  create mode 100644 src/apps/ipa-verify/main.cpp
> >  create mode 100644 src/apps/ipa-verify/meson.build
> > 
> > diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> > index 7f36e58e8bfa..bf823563c91c 100644
> > --- a/include/libcamera/internal/ipa_manager.h
> > +++ b/include/libcamera/internal/ipa_manager.h
> > @@ -47,6 +47,13 @@ public:
> >                 return proxy;
> >         }
> >  
> > +#if HAVE_IPA_PUBKEY
> > +       static const PubKey &pubKey()
> > +       {
> > +               return pubKey_;
> > +       }
> > +#endif
> > +
> >  private:
> >         static IPAManager *self_;
> >  
> > diff --git a/src/apps/ipa-verify/main.cpp b/src/apps/ipa-verify/main.cpp
> > new file mode 100644
> > index 000000000000..76ba5073d25a
> > --- /dev/null
> > +++ b/src/apps/ipa-verify/main.cpp
> > @@ -0,0 +1,64 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2023, Ideas on Board Oy
> > + *
> > + * ipa_verify.cpp - Verify signature on an IPA module
> > + */
> > +
> > +#include <iostream>
> > +#include <libgen.h>
> > +
> > +#include <libcamera/base/file.h>
> > +#include <libcamera/base/span.h>
> > +
> > +#include "libcamera/internal/ipa_manager.h"
> > +#include "libcamera/internal/ipa_module.h"
> > +
> > +using namespace libcamera;
> > +
> > +namespace {
> > +
> > +bool isSignatureValid(IPAModule *ipa)
> > +{
> > +       File file{ ipa->path() };
> > +       if (!file.open(File::OpenModeFlag::ReadOnly))
> > +               return false;
> > +
> > +       Span<uint8_t> data = file.map();
> > +       if (data.empty())
> > +               return false;
> > +
> > +       return IPAManager::pubKey().verify(data, ipa->signature());
> 
> What happens if HAVE_IPA_PUBKEY is not defined?
> 
> Admitedly - there won't be much to check - but will this build? break?
> report failure?
> 
> I think I'd expect the tool to still exist but report something on the
> output.

At the moment it's simply not compiled. I can make it report that IPA
signatures are not available if you prefer so.

> Anyway, I'm actually very pleased to see such a tool. I'd carved up a
> script to do the same but that relies on having the build tree, while
> this could actually be installed to a target and validate a full install
> when needed. Much nicer.

I envision this utility being used to validate the packaging process,
not being installed as part of the normal libcamera package. It's
actually a good question as to whether it would be most useful to run it
on the host or the target. At the moment it's compiled as a binary for
the target, we could switch that with `native : true`.

> > +}
> > +
> > +void usage(char *argv0)
> > +{
> > +       std::cout << "Usage: " << basename(argv0) << " ipa_name.so" << std::endl;
> > +       std::cout << std::endl;
> > +       std::cout << "Verify the signature of an IPA module. The signature file ipa_name.so.sign is" << std::endl;
> > +       std::cout << "expected to be in the same directory as the IPA module." << std::endl;
> > +}
> > +
> > +} /* namespace */
> > +
> > +int main(int argc, char **argv)
> > +{
> > +       if (argc != 2) {
> > +               usage(argv[0]);
> > +               return EXIT_FAILURE;
> > +       }
> > +
> > +       IPAModule module{ argv[1] };
> > +       if (!module.isValid()) {
> > +               std::cout << "Invalid IPA module " << argv[1] << std::endl;
> > +               return EXIT_FAILURE;
> > +       }
> > +
> > +       if (!isSignatureValid(&module)) {
> > +               std::cout << "IPA module signature is invalid" << std::endl;
> > +               return EXIT_FAILURE;
> > +       }
> > +
> > +       std::cout << "IPA module signature is valid" << std::endl;
> > +       return 0;
> > +}
> > diff --git a/src/apps/ipa-verify/meson.build b/src/apps/ipa-verify/meson.build
> > new file mode 100644
> > index 000000000000..7fdda3b9af4b
> > --- /dev/null
> > +++ b/src/apps/ipa-verify/meson.build
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +if not ipa_sign_module
> > +    subdir_done()
> > +endif
> > +
> > +ipa_verify_sources = files([
> > +    'main.cpp',
> > +])
> > +
> > +ipa_verify  = executable('ipa_verify', ipa_verify_sources,
> > +                         dependencies : [
> > +                             libcamera_private,
> > +                         ],
> > +                         install : false)
> > diff --git a/src/apps/meson.build b/src/apps/meson.build
> > index 099876356bd1..af632b9a7b0b 100644
> > --- a/src/apps/meson.build
> > +++ b/src/apps/meson.build
> > @@ -18,3 +18,5 @@ subdir('lc-compliance')
> >  
> >  subdir('cam')
> >  subdir('qcam')
> > +
> > +subdir('ipa-verify')
> > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> > index 030ef43fb994..6d18d09b019c 100644
> > --- a/src/libcamera/ipa_manager.cpp
> > +++ b/src/libcamera/ipa_manager.cpp
> > @@ -279,6 +279,19 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,
> >   * found or if the IPA proxy fails to initialize
> >   */
> >  
> > +#if HAVE_IPA_PUBKEY
> > +/**
> > + * \fn IPAManager::pubKey()
> > + * \brief Retrieve the IPA module signing public key
> > + *
> > + * IPA module signature verification is normally handled internally by the
> > + * IPAManager class. This function is meant to be used by utilities that need to
> > + * verify signatures externally.
> > + *
> > + * \return The IPA module signing public key
> > + */
> > +#endif
> > +
> >  bool IPAManager::isSignatureValid([[maybe_unused]] IPAModule *ipa) const
> >  {
> >  #if HAVE_IPA_PUBKEY

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list