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

18608 diff mbox series

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.

Signed-off-by: Laurent Pinchart <laurent.pinchart@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

Hi Laurent,

Thank you for the patch.

On 5/6/23 4:40 PM, Laurent Pinchart via libcamera-devel wrote:
> 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.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Overall looks good to me. I can see packegers using this as part of 
"postinst" of the package itself.

Reviewed-by: Umang Jain <umang.jain@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());
> +}
> +
> +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

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.



> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@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.


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.



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

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