[PATCH 0/2] crypto: fix build with gcrypt

[PATCH 0/2] crypto: fix build with gcrypt

[Daniel P. Berrangé]

The build system failed to add gcrypt flags and also didn't link to
gnutls in all scenarios.  This was missed because of the lack of CI
coverage for various build scenarios

Daniel P. Berrangé (2):
  crypto: fix build without gcrypt and gnutls
  gitlab: expand test coverage for crypto builds

 .gitlab-ci.yml                          | 66 +++++++++++++++++++++++++
 configure                               |  2 +
 crypto/meson.build                      | 24 ++++-----
 meson.build                             |  5 ++
 tests/docker/dockerfiles/centos7.docker |  2 +
 tests/docker/dockerfiles/centos8.docker |  1 +
 6 files changed, 88 insertions(+), 12 deletions(-)

-- 
2.26.2

[PATCH 1/2] crypto: fix build without gcrypt and gnutls

[Daniel P. Berrangé]

If nettle is disabled and gcrypt enabled, the compiler and linker flags
needed for gcrypt are not passed.

Gnutls was also not added as a dependancy wen gcrypt is enabled.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 configure          |  2 ++
 crypto/meson.build | 24 ++++++++++++------------
 meson.build        |  5 +++++
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index 6ecaff429b..4effe769c9 100755
--- a/configure
+++ b/configure
@@ -6973,6 +6973,8 @@ if test "$gcrypt" = "yes" ; then
   if test "$gcrypt_hmac" = "yes" ; then
     echo "CONFIG_GCRYPT_HMAC=y" >> $config_host_mak
   fi
+  echo "GCRYPT_CFLAGS=$gcrypt_cflags" >> $config_host_mak
+  echo "GCRYPT_LIBS=$gcrypt_libs" >> $config_host_mak
 fi
 if test "$nettle" = "yes" ; then
   echo "CONFIG_NETTLE=y" >> $config_host_mak
diff --git a/crypto/meson.build b/crypto/meson.build
index 18da7c8541..af12b85aae 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -23,23 +23,23 @@ crypto_ss.add(files(
   'tlssession.c',
 ))
 
-if 'CONFIG_GCRYPT' in config_host
-  wo_nettle = files('hash-gcrypt.c', 'pbkdf-gcrypt.c')
+if 'CONFIG_NETTLE' in config_host
+  crypto_ss.add(files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'), nettle)
+elif 'CONFIG_GCRYPT' in config_host
+  crypto_ss.add(files('hash-gcrypt.c', 'pbkdf-gcrypt.c'), gcrypt)
+  if 'CONFIG_GCRYPT_HMAC' in config_host
+    crypto_ss.add(files('hmac-gcrypt.c'))
+  else
+    crypto_ss.add(files('hmac-glib.c'))
+  endif
 else
-  wo_nettle = files('hash-glib.c', 'pbkdf-stub.c')
-endif
-if 'CONFIG_GCRYPT_HMAC' not in config_host
-  wo_nettle += files('hmac-glib.c')
+  crypto_ss.add(files('hash-glib.c', 'hmac-glib.c', 'pbkdf-stub.c'))
 endif
-crypto_ss.add(when: [nettle, 'CONFIG_NETTLE'],
-             if_true: files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'),
-             if_false: wo_nettle)
 
 crypto_ss.add(when: 'CONFIG_SECRET_KEYRING', if_true: files('secret_keyring.c'))
 crypto_ss.add(when: 'CONFIG_QEMU_PRIVATE_XTS', if_true: files('xts.c'))
-crypto_ss.add(when: 'CONFIG_GCRYPT_HMAC', if_true: files('hmac-gcrypt.c'))
 crypto_ss.add(when: 'CONFIG_AF_ALG', if_true: files('afalg.c', 'cipher-afalg.c', 'hash-afalg.c'))
-crypto_ss.add(when: 'CONFIG_GNUTLS', if_true: files('tls-cipher-suites.c'))
+crypto_ss.add(when: [gnutls, 'CONFIG_GNUTLS'], if_true: files('tls-cipher-suites.c'))
 
 crypto_ss = crypto_ss.apply(config_host, strict: false)
 libcrypto = static_library('crypto', crypto_ss.sources() + genh,
@@ -53,7 +53,7 @@ crypto = declare_dependency(link_whole: libcrypto,
 util_ss.add(files('aes.c'))
 util_ss.add(files('init.c'))
 if 'CONFIG_GCRYPT' in config_host
-  util_ss.add(files('random-gcrypt.c'))
+  util_ss.add(files('random-gcrypt.c'), gcrypt)
 elif 'CONFIG_GNUTLS' in config_host
   util_ss.add(files('random-gnutls.c'), gnutls)
 elif 'CONFIG_RNG_NONE' in config_host
diff --git a/meson.build b/meson.build
index 74f8ea0c2e..c5f672028f 100644
--- a/meson.build
+++ b/meson.build
@@ -114,6 +114,11 @@ urcubp = not_found
 if 'CONFIG_TRACE_UST' in config_host
   urcubp = declare_dependency(link_args: config_host['URCU_BP_LIBS'].split())
 endif
+gcrypt = not_found
+if 'CONFIG_GCRYPT' in config_host
+  gcrypt = declare_dependency(compile_args: config_host['GCRYPT_CFLAGS'].split(),
+                              link_args: config_host['GCRYPT_LIBS'].split())
+endif
 nettle = not_found
 if 'CONFIG_NETTLE' in config_host
   nettle = declare_dependency(compile_args: config_host['NETTLE_CFLAGS'].split(),
-- 
2.26.2

[PATCH 2/2] gitlab: expand test coverage for crypto builds

[Daniel P. Berrangé]

Most jobs test the latest nettle library. This adds explicit coverage
for latest gcrypt using Fedora, and old gcrypt and nettle using
CentOS-7. The latter does a minimal tools-only build, as we only need to
validate that the crypto code builds and unit tests pass. Finally a job
disabling both nettle and gcrypt is provided to validate that gnutls
still works.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 .gitlab-ci.yml                          | 66 +++++++++++++++++++++++++
 tests/docker/dockerfiles/centos7.docker |  2 +
 tests/docker/dockerfiles/centos8.docker |  1 +
 3 files changed, 69 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b7967b9a13..85cf1f6cfd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -130,6 +130,7 @@ build-system-fedora:
   <<: *native_build_job_definition
   variables:
     IMAGE: fedora
+    CONFIGURE_ARGS: --disable-gcrypt --enable-nettle
     TARGETS: tricore-softmmu unicore32-softmmu microblaze-softmmu mips-softmmu
       xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
     MAKE_CHECK_ARGS: check-build
@@ -160,6 +161,7 @@ build-system-centos:
   <<: *native_build_job_definition
   variables:
     IMAGE: centos8
+    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt
     TARGETS: ppc64-softmmu lm32-softmmu or1k-softmmu s390x-softmmu
       x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
     MAKE_CHECK_ARGS: check-build
@@ -196,6 +198,7 @@ build-disabled:
       --disable-guest-agent --disable-curses --disable-libxml2 --disable-tpm
       --disable-qom-cast-debug --disable-spice --disable-vhost-vsock
       --disable-vhost-net --disable-vhost-crypto --disable-vhost-user
+      --disable-nettle --disable-gcrypt --disable-gnutls
     TARGETS: i386-softmmu ppc64-softmmu mips64-softmmu i386-linux-user
     MAKE_CHECK_ARGS: check-qtest SPEED=slow
 
@@ -271,3 +274,66 @@ build-tci:
       done
     - QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
     - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
+
+# Most jobs test latest gcrypto or nettle builds
+#
+# These jobs test old gcrypt and nettle from RHEL7
+# which had some API differences.
+build-crypto-old-nettle:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: centos7
+    CONFIGURE_ARGS: --disable-system --disable-user --enable-tools --disable-gcrypt --enable-nettle
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    paths:
+      - build
+
+check-crypto-old-nettle:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-crypto-old-nettle
+      artifacts: true
+  variables:
+    IMAGE: centos7
+    MAKE_CHECK_ARGS: check
+
+
+build-crypto-old-gcrypt:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: centos7
+    CONFIGURE_ARGS: --disable-system --disable-user --enable-tools --disable-nettle --enable-gcrypt
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    paths:
+      - build
+
+check-crypto-old-gcrypt:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-crypto-old-gcrypt
+      artifacts: true
+  variables:
+    IMAGE: centos7
+    MAKE_CHECK_ARGS: check
+
+
+build-crypto-only-gnutls:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: centos7
+    CONFIGURE_ARGS: --disable-system --disable-user --enable-tools --disable-nettle --disable-gcrypt --enable-gnutls
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    paths:
+      - build
+
+check-crypto-only-gnutls:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-crypto-only-gnutls
+      artifacts: true
+  variables:
+    IMAGE: centos7
+    MAKE_CHECK_ARGS: check
diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
index e197acdc3c..46277773bf 100644
--- a/tests/docker/dockerfiles/centos7.docker
+++ b/tests/docker/dockerfiles/centos7.docker
@@ -15,9 +15,11 @@ ENV PACKAGES \
     gettext \
     git \
     glib2-devel \
+    gnutls-devel \
     libaio-devel \
     libepoxy-devel \
     libfdt-devel \
+    libgcrypt-devel \
     librdmacm-devel \
     libzstd-devel \
     lzo-devel \
diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker
index 9852c5b9ee..f435616d6a 100644
--- a/tests/docker/dockerfiles/centos8.docker
+++ b/tests/docker/dockerfiles/centos8.docker
@@ -13,6 +13,7 @@ ENV PACKAGES \
     glib2-devel \
     libaio-devel \
     libepoxy-devel \
+    libgcrypt-devel \
     lzo-devel \
     make \
     mesa-libEGL-devel \
-- 
2.26.2

Re: [PATCH 1/2] crypto: fix build without gcrypt and gnutls

[Richard Henderson]

On 8/28/20 6:27 AM, Daniel P. Berrangé wrote:
> If nettle is disabled and gcrypt enabled, the compiler and linker flags
> needed for gcrypt are not passed.
> 
> Gnutls was also not added as a dependancy wen gcrypt is enabled.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

This still needs something else.  I get

Linking target qemu-x86_64
/usr/bin/ld: libqemuutil.a(crypto_init.c.o): in function `qcrypto_init':
/home/rth/qemu/bld-g/../qemu/crypto/init.c:111: undefined reference to
`gnutls_global_init'
/usr/bin/ld: /home/rth/qemu/bld-g/../qemu/crypto/init.c:113: undefined
reference to `gnutls_strerror'
collect2: error: ld returned 1 exit status
make: *** [Makefile.ninja:1570: qemu-x86_64] Error 1


r~

Re: [PATCH 1/2] crypto: fix build without gcrypt and gnutls

[Daniel P. Berrangé]

On Fri, Aug 28, 2020 at 08:13:26AM -0700, Richard Henderson wrote:
> On 8/28/20 6:27 AM, Daniel P. Berrangé wrote:
> > If nettle is disabled and gcrypt enabled, the compiler and linker flags
> > needed for gcrypt are not passed.
> > 
> > Gnutls was also not added as a dependancy wen gcrypt is enabled.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> This still needs something else.  I get
> 
> Linking target qemu-x86_64
> /usr/bin/ld: libqemuutil.a(crypto_init.c.o): in function `qcrypto_init':
> /home/rth/qemu/bld-g/../qemu/crypto/init.c:111: undefined reference to
> `gnutls_global_init'
> /usr/bin/ld: /home/rth/qemu/bld-g/../qemu/crypto/init.c:113: undefined
> reference to `gnutls_strerror'
> collect2: error: ld returned 1 exit status
> make: *** [Makefile.ninja:1570: qemu-x86_64] Error 1

Can you tell me what  configure args you're using and host OS distro

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 1/2] crypto: fix build without gcrypt and gnutls

[Richard Henderson]

On 8/28/20 8:23 AM, Daniel P. Berrangé wrote:
> On Fri, Aug 28, 2020 at 08:13:26AM -0700, Richard Henderson wrote:
>> On 8/28/20 6:27 AM, Daniel P. Berrangé wrote:
>>> If nettle is disabled and gcrypt enabled, the compiler and linker flags
>>> needed for gcrypt are not passed.
>>>
>>> Gnutls was also not added as a dependancy wen gcrypt is enabled.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>
>> This still needs something else.  I get
>>
>> Linking target qemu-x86_64
>> /usr/bin/ld: libqemuutil.a(crypto_init.c.o): in function `qcrypto_init':
>> /home/rth/qemu/bld-g/../qemu/crypto/init.c:111: undefined reference to
>> `gnutls_global_init'
>> /usr/bin/ld: /home/rth/qemu/bld-g/../qemu/crypto/init.c:113: undefined
>> reference to `gnutls_strerror'
>> collect2: error: ld returned 1 exit status
>> make: *** [Makefile.ninja:1570: qemu-x86_64] Error 1
> 
> Can you tell me what  configure args you're using and host OS distro

../qemu/configure --enable-gcrypt \
  --target-list=x86_64-softmmu,x86_64-linux-user

On ubuntu 20 (focal).


r~

Re: [PATCH 2/2] gitlab: expand test coverage for crypto builds

[Thomas Huth]

On 28/08/2020 15.27, Daniel P. Berrangé wrote:
> Most jobs test the latest nettle library. This adds explicit coverage
> for latest gcrypt using Fedora, and old gcrypt and nettle using
> CentOS-7. The latter does a minimal tools-only build, as we only need to
> validate that the crypto code builds and unit tests pass. Finally a job
> disabling both nettle and gcrypt is provided to validate that gnutls
> still works.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  .gitlab-ci.yml                          | 66 +++++++++++++++++++++++++
>  tests/docker/dockerfiles/centos7.docker |  2 +
>  tests/docker/dockerfiles/centos8.docker |  1 +
>  3 files changed, 69 insertions(+)

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 1/2] crypto: fix build without gcrypt and gnutls

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> If nettle is disabled and gcrypt enabled, the compiler and linker flags
> needed for gcrypt are not passed.
>
> Gnutls was also not added as a dependancy wen gcrypt is enabled.

nit: when

-- 
Alex Bennée