[libcamera-devel,v2,2/4] libcamera: pub_key: Gracefully handle failures to load public key

17033 diff mbox series

If the public key fails to load, PubKey::isValid() function returns
false. The only user of the PubKey class, the IPAManager class, doesn't
check that condition, and still calls the PubKey::verify() function,
which leads to a crash.

Fix this by returning false from PubKey::verify() if the key isn't
valid, and log a warning in the IPAManager constructor to report the
issue.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/ipa_manager.cpp | 3 +++
 src/libcamera/pub_key.cpp     | 3 +++
 2 files changed, 6 insertions(+)

On Tue, 9 Aug 2022 at 00:08, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> If the public key fails to load, PubKey::isValid() function returns
> false. The only user of the PubKey class, the IPAManager class, doesn't
> check that condition, and still calls the PubKey::verify() function,
> which leads to a crash.
>
> Fix this by returning false from PubKey::verify() if the key isn't
> valid, and log a warning in the IPAManager constructor to report the
> issue.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

LGTM.

Reviewed-by: Eric Curtin <ecurtin@redhat.com>

> ---
>  src/libcamera/ipa_manager.cpp | 3 +++
>  src/libcamera/pub_key.cpp     | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index ec9660456960..2f96a2072fd6 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -109,6 +109,9 @@ IPAManager::IPAManager()
>                 LOG(IPAManager, Fatal)
>                         << "Multiple IPAManager objects are not allowed";
>
> +       if (!pubKey_.isValid())
> +               LOG(IPAManager, Warning) << "Public key not valid";
> +
>         unsigned int ipaCount = 0;
>
>         /* User-specified paths take precedence. */
> diff --git a/src/libcamera/pub_key.cpp b/src/libcamera/pub_key.cpp
> index 9bb08fda34af..b2045a103bc0 100644
> --- a/src/libcamera/pub_key.cpp
> +++ b/src/libcamera/pub_key.cpp
> @@ -76,6 +76,9 @@ PubKey::~PubKey()
>  bool PubKey::verify([[maybe_unused]] Span<const uint8_t> data,
>                     [[maybe_unused]] Span<const uint8_t> sig) const
>  {
> +       if (!valid_)
> +               return false;
> +
>  #if HAVE_GNUTLS
>         const gnutls_datum_t gnuTlsData{
>                 const_cast<unsigned char *>(data.data()),
> --
> Regards,
>
> Laurent Pinchart
>

Quoting Laurent Pinchart via libcamera-devel (2022-08-09 00:08:31)
> If the public key fails to load, PubKey::isValid() function returns
> false. The only user of the PubKey class, the IPAManager class, doesn't
> check that condition, and still calls the PubKey::verify() function,
> which leads to a crash.
> 
> Fix this by returning false from PubKey::verify() if the key isn't
> valid, and log a warning in the IPAManager constructor to report the
> issue.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/ipa_manager.cpp | 3 +++
>  src/libcamera/pub_key.cpp     | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index ec9660456960..2f96a2072fd6 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -109,6 +109,9 @@ IPAManager::IPAManager()
>                 LOG(IPAManager, Fatal)
>                         << "Multiple IPAManager objects are not allowed";
>  
> +       if (!pubKey_.isValid())
> +               LOG(IPAManager, Warning) << "Public key not valid";
> +
>         unsigned int ipaCount = 0;
>  
>         /* User-specified paths take precedence. */
> diff --git a/src/libcamera/pub_key.cpp b/src/libcamera/pub_key.cpp
> index 9bb08fda34af..b2045a103bc0 100644
> --- a/src/libcamera/pub_key.cpp
> +++ b/src/libcamera/pub_key.cpp
> @@ -76,6 +76,9 @@ PubKey::~PubKey()
>  bool PubKey::verify([[maybe_unused]] Span<const uint8_t> data,
>                     [[maybe_unused]] Span<const uint8_t> sig) const
>  {
> +       if (!valid_)
> +               return false;
> +
>  #if HAVE_GNUTLS
>         const gnutls_datum_t gnuTlsData{
>                 const_cast<unsigned char *>(data.data()),
> -- 
> Regards,
> 
> Laurent Pinchart
>