[PATCH v2 0/3] crypto: Allow building with GnuTLS but without Libtasn1

[PATCH v2 0/3] crypto: Allow building with GnuTLS but without Libtasn1

[Philippe Mathieu-Daudé]

Since v1:
- split in 3
- remove "crypto-tls-x509-helpers.h" (danpb)
- include pkix_asn1_tab.c.inc

Philippe Mathieu-Daudé (3):
  crypto: Remove 'crypto-tls-x509-helpers.h' from
    crypto-tls-psk-helpers.c
  crypto: Restrict pkix_asn1_tab[] to crypto-tls-x509-helpers.c
  crypto: Allow building with GnuTLS but without Libtasn1

 meson.build                                         | 1 +
 tests/unit/crypto-tls-x509-helpers.h                | 3 ---
 tests/unit/crypto-tls-psk-helpers.c                 | 1 -
 tests/unit/crypto-tls-x509-helpers.c                | 6 +++++-
 tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} | 5 +----
 tests/qtest/meson.build                             | 3 +--
 tests/unit/meson.build                              | 6 +++---
 7 files changed, 11 insertions(+), 14 deletions(-)
 rename tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} (99%)

-- 
2.41.0

[PATCH v2 1/3] crypto: Remove 'crypto-tls-x509-helpers.h' from crypto-tls-psk-helpers.c

[Philippe Mathieu-Daudé]

crypto-tls-psk-helpers.c doesn't access the declarations
of "crypto-tls-x509-helpers.h", remove the include line
to avoid when building with GNUTLS but without Libtasn1:

  In file included from tests/unit/crypto-tls-psk-helpers.c:23:
  tests/unit/crypto-tls-x509-helpers.h:26:10: fatal error:
  libtasn1.h: No such file or directory
     26 | #include <libtasn1.h>
        |          ^~~~~~~~~~~~
  compilation terminated.

Fixes: e1a6dc91dd ("crypto: Implement TLS Pre-Shared Keys (PSK).")
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/crypto-tls-psk-helpers.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/unit/crypto-tls-psk-helpers.c b/tests/unit/crypto-tls-psk-helpers.c
index c6cc740772..36527fd655 100644
--- a/tests/unit/crypto-tls-psk-helpers.c
+++ b/tests/unit/crypto-tls-psk-helpers.c
@@ -20,7 +20,6 @@
 
 #include "qemu/osdep.h"
 
-#include "crypto-tls-x509-helpers.h"
 #include "crypto-tls-psk-helpers.h"
 #include "qemu/sockets.h"
 
-- 
2.41.0

Re: [PATCH v2 1/3] crypto: Remove 'crypto-tls-x509-helpers.h' from crypto-tls-psk-helpers.c

[Daniel P. Berrangé]

On Thu, May 02, 2024 at 11:56:40AM +0200, Philippe Mathieu-Daudé wrote:
> crypto-tls-psk-helpers.c doesn't access the declarations
> of "crypto-tls-x509-helpers.h", remove the include line
> to avoid when building with GNUTLS but without Libtasn1:
> 
>   In file included from tests/unit/crypto-tls-psk-helpers.c:23:
>   tests/unit/crypto-tls-x509-helpers.h:26:10: fatal error:
>   libtasn1.h: No such file or directory
>      26 | #include <libtasn1.h>
>         |          ^~~~~~~~~~~~
>   compilation terminated.
> 
> Fixes: e1a6dc91dd ("crypto: Implement TLS Pre-Shared Keys (PSK).")
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/unit/crypto-tls-psk-helpers.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 2/3] crypto: Restrict pkix_asn1_tab[] to crypto-tls-x509-helpers.c

[Philippe Mathieu-Daudé]

pkix_asn1_tab[] is only accessed by crypto-tls-x509-helpers.c,
rename pkix_asn1_tab.c as pkix_asn1_tab.c.inc and include it once.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/crypto-tls-x509-helpers.h                | 3 ---
 tests/unit/crypto-tls-x509-helpers.c                | 6 +++++-
 tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} | 5 +----
 tests/qtest/meson.build                             | 3 +--
 tests/unit/meson.build                              | 6 +++---
 5 files changed, 10 insertions(+), 13 deletions(-)
 rename tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} (99%)

diff --git a/tests/unit/crypto-tls-x509-helpers.h b/tests/unit/crypto-tls-x509-helpers.h
index 247e7160eb..562c160653 100644
--- a/tests/unit/crypto-tls-x509-helpers.h
+++ b/tests/unit/crypto-tls-x509-helpers.h
@@ -23,7 +23,6 @@
 
 #include <gnutls/gnutls.h>
 #include <gnutls/x509.h>
-#include <libtasn1.h>
 
 
 #define QCRYPTO_TLS_TEST_CLIENT_NAME "ACME QEMU Client"
@@ -171,6 +170,4 @@ void test_tls_cleanup(const char *keyfile);
     };                                                                  \
     test_tls_generate_cert(&varname, cavarname.crt)
 
-extern const asn1_static_node pkix_asn1_tab[];
-
 #endif
diff --git a/tests/unit/crypto-tls-x509-helpers.c b/tests/unit/crypto-tls-x509-helpers.c
index e9937f60d8..3e74ec5b5d 100644
--- a/tests/unit/crypto-tls-x509-helpers.c
+++ b/tests/unit/crypto-tls-x509-helpers.c
@@ -20,15 +20,19 @@
 
 #include "qemu/osdep.h"
 
+#include <libtasn1.h>
+
 #include "crypto-tls-x509-helpers.h"
 #include "crypto/init.h"
 #include "qemu/sockets.h"
 
+#include "pkix_asn1_tab.c.inc"
+
 /*
  * This stores some static data that is needed when
  * encoding extensions in the x509 certs
  */
-asn1_node pkix_asn1;
+static asn1_node pkix_asn1;
 
 /*
  * To avoid consuming random entropy to generate keys,
diff --git a/tests/unit/pkix_asn1_tab.c b/tests/unit/pkix_asn1_tab.c.inc
similarity index 99%
rename from tests/unit/pkix_asn1_tab.c
rename to tests/unit/pkix_asn1_tab.c.inc
index 89521408a1..fe29c4102a 100644
--- a/tests/unit/pkix_asn1_tab.c
+++ b/tests/unit/pkix_asn1_tab.c.inc
@@ -3,10 +3,7 @@
  * and is under copyright of various GNUTLS contributors.
  */
 
-#include "qemu/osdep.h"
-#include "crypto-tls-x509-helpers.h"
-
-const asn1_static_node pkix_asn1_tab[] = {
+static const asn1_static_node pkix_asn1_tab[] = {
   {"PKIX1", 536875024, 0},
   {0, 1073741836, 0},
   {"id-ce", 1879048204, 0},
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index b128fa5a4b..f16931f82e 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -314,8 +314,7 @@ if gnutls.found()
   migration_files += [files('../unit/crypto-tls-psk-helpers.c'), gnutls]
 
   if tasn1.found()
-    migration_files += [files('../unit/crypto-tls-x509-helpers.c',
-                              '../unit/pkix_asn1_tab.c'), tasn1]
+    migration_files += [files('../unit/crypto-tls-x509-helpers.c'), tasn1]
   endif
 endif
 
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 26c109c968..490ab8182d 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -99,11 +99,11 @@ if have_block
      tasn1.found() and \
      host_os != 'windows'
     tests += {
-      'test-crypto-tlscredsx509': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
+      'test-crypto-tlscredsx509': ['crypto-tls-x509-helpers.c',
                                    tasn1, crypto, gnutls],
-      'test-crypto-tlssession': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c', 'crypto-tls-psk-helpers.c',
+      'test-crypto-tlssession': ['crypto-tls-x509-helpers.c', 'crypto-tls-psk-helpers.c',
                                  tasn1, crypto, gnutls],
-      'test-io-channel-tls': ['io-channel-helpers.c', 'crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
+      'test-io-channel-tls': ['io-channel-helpers.c', 'crypto-tls-x509-helpers.c',
                               tasn1, io, crypto, gnutls]}
   endif
   if pam.found()
-- 
2.41.0

Re: [PATCH v2 2/3] crypto: Restrict pkix_asn1_tab[] to crypto-tls-x509-helpers.c

[Daniel P. Berrangé]

On Thu, May 02, 2024 at 11:56:41AM +0200, Philippe Mathieu-Daudé wrote:
> pkix_asn1_tab[] is only accessed by crypto-tls-x509-helpers.c,
> rename pkix_asn1_tab.c as pkix_asn1_tab.c.inc and include it once.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/unit/crypto-tls-x509-helpers.h                | 3 ---
>  tests/unit/crypto-tls-x509-helpers.c                | 6 +++++-
>  tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} | 5 +----
>  tests/qtest/meson.build                             | 3 +--
>  tests/unit/meson.build                              | 6 +++---
>  5 files changed, 10 insertions(+), 13 deletions(-)
>  rename tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} (99%)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 3/3] crypto: Allow building with GnuTLS but without Libtasn1

[Philippe Mathieu-Daudé]

We only use Libtasn1 in unit tests. As noted in commit d47b83b118
("tests: add migration tests of TLS with x509 credentials"), having
GnuTLS without Libtasn1 is a valid configuration, so do not require
Libtasn1, to avoid:

  Dependency gnutls found: YES 3.7.1 (cached)
  Run-time dependency libtasn1 found: NO (tried pkgconfig)

  ../meson.build:1914:10: ERROR: Dependency "libtasn1" not found, tried pkgconfig

Fixes: ba7ed407e6 ("configure, meson: convert libtasn1 detection to meson")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index 5db2dbc12e..837a2bdb56 100644
--- a/meson.build
+++ b/meson.build
@@ -1912,6 +1912,7 @@ endif
 tasn1 = not_found
 if gnutls.found()
   tasn1 = dependency('libtasn1',
+                     required: false,
                      method: 'pkg-config')
 endif
 keyutils = not_found
-- 
2.41.0

Re: [PATCH v2 3/3] crypto: Allow building with GnuTLS but without Libtasn1

[Daniel P. Berrangé]

On Thu, May 02, 2024 at 11:56:42AM +0200, Philippe Mathieu-Daudé wrote:
> We only use Libtasn1 in unit tests. As noted in commit d47b83b118
> ("tests: add migration tests of TLS with x509 credentials"), having
> GnuTLS without Libtasn1 is a valid configuration, so do not require
> Libtasn1, to avoid:
> 
>   Dependency gnutls found: YES 3.7.1 (cached)
>   Run-time dependency libtasn1 found: NO (tried pkgconfig)
> 
>   ../meson.build:1914:10: ERROR: Dependency "libtasn1" not found, tried pkgconfig
> 
> Fixes: ba7ed407e6 ("configure, meson: convert libtasn1 detection to meson")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  meson.build | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 3/3] crypto: Allow building with GnuTLS but without Libtasn1

[Daniel P. Berrangé]

On Thu, May 02, 2024 at 11:56:42AM +0200, Philippe Mathieu-Daudé wrote:
> We only use Libtasn1 in unit tests. As noted in commit d47b83b118
> ("tests: add migration tests of TLS with x509 credentials"), having
> GnuTLS without Libtasn1 is a valid configuration, so do not require
> Libtasn1, to avoid:
> 
>   Dependency gnutls found: YES 3.7.1 (cached)
>   Run-time dependency libtasn1 found: NO (tried pkgconfig)
> 
>   ../meson.build:1914:10: ERROR: Dependency "libtasn1" not found, tried pkgconfig

Did you actually try to build without libtasn1 present ?

If I remove /usr/lib64/pkgconfig/libtasn1.pc, then the prior
check for 'gnutls' itself will fail, as libtasn1 is declared
to be a dep of gnutls in its pkg-config file, regardless of
what QEMU askes for:

$ pkg-config --cflags --libs gnutls
Package libtasn1 was not found in the pkg-config search path.
Perhaps you should add the directory containing `libtasn1.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libtasn1', required by 'gnutls', not found

I'm still willing to merge this, because from QEMU's POV,
libtasn1 isn't required.

> 
> Fixes: ba7ed407e6 ("configure, meson: convert libtasn1 detection to meson")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  meson.build | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/meson.build b/meson.build
> index 5db2dbc12e..837a2bdb56 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1912,6 +1912,7 @@ endif
>  tasn1 = not_found
>  if gnutls.found()
>    tasn1 = dependency('libtasn1',
> +                     required: false,
>                       method: 'pkg-config')
>  endif
>  keyutils = not_found
> -- 
> 2.41.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 0/3] crypto: Allow building with GnuTLS but without Libtasn1

[Philippe Mathieu-Daudé]

ping?

On 2/5/24 11:56, Philippe Mathieu-Daudé wrote:
> Since v1:
> - split in 3
> - remove "crypto-tls-x509-helpers.h" (danpb)
> - include pkix_asn1_tab.c.inc
> 
> Philippe Mathieu-Daudé (3):
>    crypto: Remove 'crypto-tls-x509-helpers.h' from
>      crypto-tls-psk-helpers.c
>    crypto: Restrict pkix_asn1_tab[] to crypto-tls-x509-helpers.c
>    crypto: Allow building with GnuTLS but without Libtasn1
> 
>   meson.build                                         | 1 +
>   tests/unit/crypto-tls-x509-helpers.h                | 3 ---
>   tests/unit/crypto-tls-psk-helpers.c                 | 1 -
>   tests/unit/crypto-tls-x509-helpers.c                | 6 +++++-
>   tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} | 5 +----
>   tests/qtest/meson.build                             | 3 +--
>   tests/unit/meson.build                              | 6 +++---
>   7 files changed, 11 insertions(+), 14 deletions(-)
>   rename tests/unit/{pkix_asn1_tab.c => pkix_asn1_tab.c.inc} (99%)
>