[libcamera-devel,v2,4/4] libcamera: Make IPA module signing recommended instead of mandatory
diff mbox series

Message ID 20220808230833.16275-5-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Support openssl as an alternative to gnutls
Related show

Commit Message

Laurent Pinchart Aug. 8, 2022, 11:08 p.m. UTC
Commit b382f67c833d ("libcamera: Make IPA module signing mandatory for
the meantime") made openssl and gnutls dependencies mandatory to work
around the lack of proper IPA module isolation support, which broke
operation without module signatures. This has now been fixed, so IPA
module isolation isn't strictly required anymore.

There are few use cases for disabling module signing completely, given
that the openssl or gnutls dependencies are available on the vast
majority of systems and the overheard introduced by isolating all IPA
modules when signatures are not available is better avoided.
Nonetheless, libcamera should operate properly with forced IPA module
isolation, so we can support those use cases.

Adopt a middle-ground approach to avoid unintentional isolation by
documenting the dependencies as recommended, and warn at meson setup
time if they are not found.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 README.rst                |  5 ++++-
 src/libcamera/meson.build | 10 ++++++++--
 src/meson.build           |  3 ++-
 3 files changed, 14 insertions(+), 4 deletions(-)

Comments

Eric Curtin Aug. 9, 2022, 10:46 a.m. UTC | #1
On Tue, 9 Aug 2022 at 00:08, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Commit b382f67c833d ("libcamera: Make IPA module signing mandatory for
> the meantime") made openssl and gnutls dependencies mandatory to work
> around the lack of proper IPA module isolation support, which broke
> operation without module signatures. This has now been fixed, so IPA
> module isolation isn't strictly required anymore.
>
> There are few use cases for disabling module signing completely, given
> that the openssl or gnutls dependencies are available on the vast
> majority of systems and the overheard introduced by isolating all IPA
> modules when signatures are not available is better avoided.
> Nonetheless, libcamera should operate properly with forced IPA module
> isolation, so we can support those use cases.
>
> Adopt a middle-ground approach to avoid unintentional isolation by
> documenting the dependencies as recommended, and warn at meson setup
> time if they are not found.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

LGTM.

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

> ---
>  README.rst                |  5 ++++-
>  src/libcamera/meson.build | 10 ++++++++--
>  src/meson.build           |  3 ++-
>  3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/README.rst b/README.rst
> index 3bf4685b0e15..e9dd4207ae55 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -60,9 +60,12 @@ Meson Build system: [required]
>  for the libcamera core: [required]
>          libyaml-dev python3-yaml python3-ply python3-jinja2
>
> -for IPA module signing: [required]
> +for IPA module signing: [recommended]
>          Either libgnutls28-dev or libssl-dev, openssl
>
> +        Without IPA module signing, all IPA modules will be isolated in a
> +        separate process. This adds an unnecessary extra overhead at runtime.
> +
>  for improved debugging: [optional]
>          libdw-dev libunwind-dev
>
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 401fc498cfbc..0efa8fd5df7f 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -73,8 +73,14 @@ libcrypto = dependency('gnutls2', required : false)
>  if libcrypto.found()
>      config_h.set('HAVE_GNUTLS', 1)
>  else
> -    libcrypto = dependency('libcrypto', required : true)
> -    config_h.set('HAVE_CRYPTO', 1)
> +    libcrypto = dependency('libcrypto', required : false)
> +    if libcrypto.found()
> +        config_h.set('HAVE_CRYPTO', 1)
> +    endif
> +endif
> +
> +if not libcrypto.found()
> +    warning('Neither gnutls nor libcrypto found, all IPA modules will be isolated')
>  endif
>
>  if liblttng.found()
> diff --git a/src/meson.build b/src/meson.build
> index 34663a6f134d..f37c44ca9f60 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -14,7 +14,7 @@ summary({
>           }, section : 'Paths')
>
>  # Module Signing
> -openssl = find_program('openssl', required : true)
> +openssl = find_program('openssl', required : false)
>  if openssl.found()
>      ipa_priv_key = custom_target('ipa-priv-key',
>                                   output : ['ipa-priv-key.pem'],
> @@ -22,6 +22,7 @@ if openssl.found()
>      config_h.set('HAVE_IPA_PUBKEY', 1)
>      ipa_sign_module = true
>  else
> +    warning('openssl not found, all IPA modules will be isolated')
>      ipa_sign_module = false
>  endif
>
> --
> Regards,
>
> Laurent Pinchart
>
Kieran Bingham Aug. 9, 2022, 12:26 p.m. UTC | #2
Quoting Laurent Pinchart via libcamera-devel (2022-08-09 00:08:33)
> Commit b382f67c833d ("libcamera: Make IPA module signing mandatory for
> the meantime") made openssl and gnutls dependencies mandatory to work
> around the lack of proper IPA module isolation support, which broke
> operation without module signatures. This has now been fixed, so IPA
> module isolation isn't strictly required anymore.
> 
> There are few use cases for disabling module signing completely, given
> that the openssl or gnutls dependencies are available on the vast
> majority of systems and the overheard introduced by isolating all IPA
> modules when signatures are not available is better avoided.
> Nonetheless, libcamera should operate properly with forced IPA module
> isolation, so we can support those use cases.
> 
> Adopt a middle-ground approach to avoid unintentional isolation by
> documenting the dependencies as recommended, and warn at meson setup
> time if they are not found.

Sounds fine to me. It might be worthwhile making sure we test IPA
isolation more (on all platforms), as opposed to just our virtual IPA...


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

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  README.rst                |  5 ++++-
>  src/libcamera/meson.build | 10 ++++++++--
>  src/meson.build           |  3 ++-
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/README.rst b/README.rst
> index 3bf4685b0e15..e9dd4207ae55 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -60,9 +60,12 @@ Meson Build system: [required]
>  for the libcamera core: [required]
>          libyaml-dev python3-yaml python3-ply python3-jinja2
>  
> -for IPA module signing: [required]
> +for IPA module signing: [recommended]
>          Either libgnutls28-dev or libssl-dev, openssl
>  
> +        Without IPA module signing, all IPA modules will be isolated in a
> +        separate process. This adds an unnecessary extra overhead at runtime.
> +
>  for improved debugging: [optional]
>          libdw-dev libunwind-dev
>  
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 401fc498cfbc..0efa8fd5df7f 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -73,8 +73,14 @@ libcrypto = dependency('gnutls2', required : false)
>  if libcrypto.found()
>      config_h.set('HAVE_GNUTLS', 1)
>  else
> -    libcrypto = dependency('libcrypto', required : true)
> -    config_h.set('HAVE_CRYPTO', 1)
> +    libcrypto = dependency('libcrypto', required : false)
> +    if libcrypto.found()
> +        config_h.set('HAVE_CRYPTO', 1)
> +    endif
> +endif
> +
> +if not libcrypto.found()
> +    warning('Neither gnutls nor libcrypto found, all IPA modules will be isolated')
>  endif
>  
>  if liblttng.found()
> diff --git a/src/meson.build b/src/meson.build
> index 34663a6f134d..f37c44ca9f60 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -14,7 +14,7 @@ summary({
>           }, section : 'Paths')
>  
>  # Module Signing
> -openssl = find_program('openssl', required : true)
> +openssl = find_program('openssl', required : false)
>  if openssl.found()
>      ipa_priv_key = custom_target('ipa-priv-key',
>                                   output : ['ipa-priv-key.pem'],
> @@ -22,6 +22,7 @@ if openssl.found()
>      config_h.set('HAVE_IPA_PUBKEY', 1)
>      ipa_sign_module = true
>  else
> +    warning('openssl not found, all IPA modules will be isolated')
>      ipa_sign_module = false
>  endif
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>

Patch
diff mbox series

diff --git a/README.rst b/README.rst
index 3bf4685b0e15..e9dd4207ae55 100644
--- a/README.rst
+++ b/README.rst
@@ -60,9 +60,12 @@  Meson Build system: [required]
 for the libcamera core: [required]
         libyaml-dev python3-yaml python3-ply python3-jinja2
 
-for IPA module signing: [required]
+for IPA module signing: [recommended]
         Either libgnutls28-dev or libssl-dev, openssl
 
+        Without IPA module signing, all IPA modules will be isolated in a
+        separate process. This adds an unnecessary extra overhead at runtime.
+
 for improved debugging: [optional]
         libdw-dev libunwind-dev
 
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index 401fc498cfbc..0efa8fd5df7f 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -73,8 +73,14 @@  libcrypto = dependency('gnutls2', required : false)
 if libcrypto.found()
     config_h.set('HAVE_GNUTLS', 1)
 else
-    libcrypto = dependency('libcrypto', required : true)
-    config_h.set('HAVE_CRYPTO', 1)
+    libcrypto = dependency('libcrypto', required : false)
+    if libcrypto.found()
+        config_h.set('HAVE_CRYPTO', 1)
+    endif
+endif
+
+if not libcrypto.found()
+    warning('Neither gnutls nor libcrypto found, all IPA modules will be isolated')
 endif
 
 if liblttng.found()
diff --git a/src/meson.build b/src/meson.build
index 34663a6f134d..f37c44ca9f60 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -14,7 +14,7 @@  summary({
          }, section : 'Paths')
 
 # Module Signing
-openssl = find_program('openssl', required : true)
+openssl = find_program('openssl', required : false)
 if openssl.found()
     ipa_priv_key = custom_target('ipa-priv-key',
                                  output : ['ipa-priv-key.pem'],
@@ -22,6 +22,7 @@  if openssl.found()
     config_h.set('HAVE_IPA_PUBKEY', 1)
     ipa_sign_module = true
 else
+    warning('openssl not found, all IPA modules will be isolated')
     ipa_sign_module = false
 endif