mbox

[PULL,00/22] Crypto and more patches

Message ID 20210712130223.1825930-1-berrange@redhat.com
State New
Headers show

Pull-request

https://gitlab.com/berrange/qemu tags/crypto-and-more-pull-request

Message

Daniel P. Berrangé July 12, 2021, 1:02 p.m. UTC 
The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:

  Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)

are available in the Git repository at:

  https://gitlab.com/berrange/qemu tags/crypto-and-more-pull-request

for you to fetch changes up to 1fc9958410c8683950ea22084b133a755561398b:

  tests/migration: fix unix socket migration (2021-07-12 14:00:20 +0100)

----------------------------------------------------------------
Merge crypto updates and misc fixes

 * Introduce a GNUTLS backend for crypto algorithms
 * Change crypto library preference gnutls > gcrypt > nettle > built-in
 * Remove built-in DES impl
 * Remove XTS mode from built-in AES impl
 * Fix seccomp rules to allow resource info getters
 * Fix migration performance test
 * Use GDateTime in io/ and net/rocker/ code

----------------------------------------------------------------

Daniel P. Berrangé (21):
  crypto: remove conditional around 3DES crypto test cases
  crypto: remove obsolete crypto test condition
  crypto: skip essiv ivgen tests if AES+ECB isn't available
  crypto: use &error_fatal in crypto tests
  crypto: fix gcrypt min version 1.8 regression
  crypto: drop gcrypt thread initialization code
  crypto: drop custom XTS support in gcrypt driver
  crypto: add crypto tests for single block DES-ECB and DES-CBC
  crypto: delete built-in DES implementation
  crypto: delete built-in XTS cipher mode support
  crypto: replace 'des-rfb' cipher with 'des'
  crypto: flip priority of backends to prefer gcrypt
  crypto: introduce build system for gnutls crypto backend
  crypto: add gnutls cipher provider
  crypto: add gnutls hash provider
  crypto: add gnutls hmac provider
  crypto: add gnutls pbkdf provider
  crypto: prefer gnutls as the crypto backend if new enough
  net/rocker: use GDateTime for formatting timestamp in debug messages
  io: use GDateTime for formatting timestamp for websock headers
  seccomp: don't block getters for resource control syscalls

Hyman (1):
  tests/migration: fix unix socket migration

 crypto/cipher-builtin.c.inc         | 132 ---------
 crypto/cipher-gcrypt.c.inc          | 143 +---------
 crypto/cipher-gnutls.c.inc          | 324 ++++++++++++++++++++++
 crypto/cipher-nettle.c.inc          |  26 +-
 crypto/cipher.c                     |  30 +-
 crypto/desrfb.c                     | 416 ----------------------------
 crypto/hash-gnutls.c                | 104 +++++++
 crypto/hmac-gnutls.c                | 139 ++++++++++
 crypto/init.c                       |  62 -----
 crypto/meson.build                  |   9 +-
 crypto/pbkdf-gnutls.c               |  90 ++++++
 hw/net/rocker/rocker.h              |  11 +-
 io/channel-websock.c                |  10 +-
 meson.build                         | 102 ++++---
 qapi/crypto.json                    |   4 +-
 softmmu/qemu-seccomp.c              |   6 -
 tests/migration/guestperf/engine.py |   2 +-
 tests/unit/test-crypto-cipher.c     |  31 ++-
 tests/unit/test-crypto-hash.c       |  13 +-
 tests/unit/test-crypto-hmac.c       |  28 +-
 tests/unit/test-crypto-ivgen.c      |  14 +-
 tests/unit/test-crypto-pbkdf.c      |   5 +-
 ui/vnc.c                            |  20 +-
 23 files changed, 823 insertions(+), 898 deletions(-)
 create mode 100644 crypto/cipher-gnutls.c.inc
 delete mode 100644 crypto/desrfb.c
 create mode 100644 crypto/hash-gnutls.c
 create mode 100644 crypto/hmac-gnutls.c
 create mode 100644 crypto/pbkdf-gnutls.c

Comments

Peter Maydell July 13, 2021, 9:25 a.m. UTC | #1 
On Mon, 12 Jul 2021 at 14:23, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/berrange/qemu tags/crypto-and-more-pull-request
>
> for you to fetch changes up to 1fc9958410c8683950ea22084b133a755561398b:
>
>   tests/migration: fix unix socket migration (2021-07-12 14:00:20 +0100)
>
> ----------------------------------------------------------------
> Merge crypto updates and misc fixes
>
>  * Introduce a GNUTLS backend for crypto algorithms
>  * Change crypto library preference gnutls > gcrypt > nettle > built-in
>  * Remove built-in DES impl
>  * Remove XTS mode from built-in AES impl
>  * Fix seccomp rules to allow resource info getters
>  * Fix migration performance test
>  * Use GDateTime in io/ and net/rocker/ code
>
> ----------------------------------------------------------------

Hi; this failed 'make check' on ppc64be:
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
G_TEST_SRCDIR=/home/pm215/qemu/tests/unit
G_TEST_BUILDDIR=/home/pm215/qemu/build/all/tests/unit
tests/unit/test-crypto-cipher --tap -k
test-crypto-cipher: cbc.c:53: nettle_cbc_encrypt: Assertion `!(length
% block_size)' failed.
PASS 1 test-crypto-cipher /crypto/cipher/aes-ecb-128
PASS 2 test-crypto-cipher /crypto/cipher/aes-ecb-192
PASS 3 test-crypto-cipher /crypto/cipher/aes-ecb-256
PASS 4 test-crypto-cipher /crypto/cipher/aes-cbc-128
PASS 5 test-crypto-cipher /crypto/cipher/aes-cbc-192
PASS 6 test-crypto-cipher /crypto/cipher/aes-cbc-256
PASS 7 test-crypto-cipher /crypto/cipher/des-ecb-56-one-block
PASS 8 test-crypto-cipher /crypto/cipher/des-cbc-56-one-block
PASS 9 test-crypto-cipher /crypto/cipher/des-ecb-56
PASS 10 test-crypto-cipher /crypto/cipher/3des-cbc
PASS 11 test-crypto-cipher /crypto/cipher/3des-ecb
PASS 12 test-crypto-cipher /crypto/cipher/aes-xts-128-1
PASS 13 test-crypto-cipher /crypto/cipher/aes-xts-128-2
PASS 14 test-crypto-cipher /crypto/cipher/aes-xts-128-3
PASS 15 test-crypto-cipher /crypto/cipher/aes-xts-128-4
ERROR test-crypto-cipher - too few tests run (expected 17, got 15)

The failure is reproducible. Here's a backtrace from a debug
build:

test-crypto-cipher: cbc.c:53: nettle_cbc_encrypt: Assertion `!(length
% block_size)' failed.

Thread 1 "test-crypto-cip" received signal SIGABRT, Aborted.
0x00007ffff77b8460 in __libc_signal_restore_set (set=0x7fffffffe468)
at ../sysdeps/unix/sysv/linux/internal-signals.h:86
86      ../sysdeps/unix/sysv/linux/internal-signals.h: No such file or
directory.
(gdb) bt
#0  0x00007ffff77b8460 in __libc_signal_restore_set
(set=0x7fffffffe468) at
../sysdeps/unix/sysv/linux/internal-signals.h:86
#1  __GI_raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:48
#2  0x00007ffff779bd40 in __GI_abort () at abort.c:79
#3  0x00007ffff77ae490 in __assert_fail_base (fmt=<optimized out>,
    assertion=assertion@entry=0x7ffff72b6f38 "!(length % block_size)",
file=file@entry=0x7ffff72b6f30 "cbc.c", line=line@entry=53,
    function=function@entry=0x7ffff72b6f50 "nettle_cbc_encrypt") at assert.c:92
#4  0x00007ffff77ae528 in __GI___assert_fail (assertion=0x7ffff72b6f38
"!(length % block_size)", file=0x7ffff72b6f30 "cbc.c",
    line=<optimized out>, function=0x7ffff72b6f50
"nettle_cbc_encrypt") at assert.c:101
#5  0x00007ffff728c154 in nettle_cbc_encrypt () from
/usr/lib/powerpc64-linux-gnu/libnettle.so.8
#6  0x00007ffff7e6b894 in ?? () from
/usr/lib/powerpc64-linux-gnu/libgnutls.so.30
#7  0x00007ffff7e6c72c in ?? () from
/usr/lib/powerpc64-linux-gnu/libgnutls.so.30
#8  0x00007ffff7d6d794 in gnutls_cipher_encrypt2 () from
/usr/lib/powerpc64-linux-gnu/libgnutls.so.30
#9  0x000000010003c330 in qcrypto_gnutls_cipher_encrypt
(cipher=0x10016e550, in=0x7fffffffeca8, out=0x7fffffffecc8, len=32,
    errp=0x100122b48 <error_abort>) at ../../crypto/cipher-gnutls.c.inc:103
#10 0x000000010003cef0 in qcrypto_cipher_encrypt (cipher=0x10016e550,
in=0x7fffffffeca8, out=0x7fffffffecc8, len=32,
    errp=0x100122b48 <error_abort>) at ../../crypto/cipher.c:177
#11 0x000000010002e75c in test_cipher_null_iv () at
../../tests/unit/test-crypto-cipher.c:749
#12 0x00007ffff7bbed38 in ?? () from
/usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
#13 0x00007ffff7bbeabc in ?? () from
/usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
#14 0x00007ffff7bbeabc in ?? () from
/usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
#15 0x00007ffff7bbf364 in g_test_run_suite () from
/usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
#16 0x00007ffff7bbf3bc in g_test_run () from
/usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
#17 0x000000010002eb78 in main (argc=1, argv=0x7ffffffff8e8) at
../../tests/unit/test-crypto-cipher.c:821

In frame 9 len is 32 and ctx_>blocksize is 16, so ¯\_(ツ)_/¯

thanks
-- PMM
Daniel P. Berrangé July 13, 2021, 1:45 p.m. UTC | #2 
On Tue, Jul 13, 2021 at 10:25:44AM +0100, Peter Maydell wrote:
> On Mon, 12 Jul 2021 at 14:23, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
> >
> >   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/berrange/qemu tags/crypto-and-more-pull-request
> >
> > for you to fetch changes up to 1fc9958410c8683950ea22084b133a755561398b:
> >
> >   tests/migration: fix unix socket migration (2021-07-12 14:00:20 +0100)
> >
> > ----------------------------------------------------------------
> > Merge crypto updates and misc fixes
> >
> >  * Introduce a GNUTLS backend for crypto algorithms
> >  * Change crypto library preference gnutls > gcrypt > nettle > built-in
> >  * Remove built-in DES impl
> >  * Remove XTS mode from built-in AES impl
> >  * Fix seccomp rules to allow resource info getters
> >  * Fix migration performance test
> >  * Use GDateTime in io/ and net/rocker/ code
> >
> > ----------------------------------------------------------------
> 
> Hi; this failed 'make check' on ppc64be:

> The failure is reproducible. Here's a backtrace from a debug
> build:
> 
> test-crypto-cipher: cbc.c:53: nettle_cbc_encrypt: Assertion `!(length
> % block_size)' failed.
> 
> Thread 1 "test-crypto-cip" received signal SIGABRT, Aborted.
> 0x00007ffff77b8460 in __libc_signal_restore_set (set=0x7fffffffe468)
> at ../sysdeps/unix/sysv/linux/internal-signals.h:86
> 86      ../sysdeps/unix/sysv/linux/internal-signals.h: No such file or
> directory.
> (gdb) bt
> #0  0x00007ffff77b8460 in __libc_signal_restore_set
> (set=0x7fffffffe468) at
> ../sysdeps/unix/sysv/linux/internal-signals.h:86
> #1  __GI_raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:48
> #2  0x00007ffff779bd40 in __GI_abort () at abort.c:79
> #3  0x00007ffff77ae490 in __assert_fail_base (fmt=<optimized out>,
>     assertion=assertion@entry=0x7ffff72b6f38 "!(length % block_size)",
> file=file@entry=0x7ffff72b6f30 "cbc.c", line=line@entry=53,
>     function=function@entry=0x7ffff72b6f50 "nettle_cbc_encrypt") at assert.c:92
> #4  0x00007ffff77ae528 in __GI___assert_fail (assertion=0x7ffff72b6f38
> "!(length % block_size)", file=0x7ffff72b6f30 "cbc.c",
>     line=<optimized out>, function=0x7ffff72b6f50
> "nettle_cbc_encrypt") at assert.c:101
> #5  0x00007ffff728c154 in nettle_cbc_encrypt () from
> /usr/lib/powerpc64-linux-gnu/libnettle.so.8
> #6  0x00007ffff7e6b894 in ?? () from
> /usr/lib/powerpc64-linux-gnu/libgnutls.so.30
> #7  0x00007ffff7e6c72c in ?? () from
> /usr/lib/powerpc64-linux-gnu/libgnutls.so.30
> #8  0x00007ffff7d6d794 in gnutls_cipher_encrypt2 () from
> /usr/lib/powerpc64-linux-gnu/libgnutls.so.30
> #9  0x000000010003c330 in qcrypto_gnutls_cipher_encrypt
> (cipher=0x10016e550, in=0x7fffffffeca8, out=0x7fffffffecc8, len=32,
>     errp=0x100122b48 <error_abort>) at ../../crypto/cipher-gnutls.c.inc:103
> #10 0x000000010003cef0 in qcrypto_cipher_encrypt (cipher=0x10016e550,
> in=0x7fffffffeca8, out=0x7fffffffecc8, len=32,
>     errp=0x100122b48 <error_abort>) at ../../crypto/cipher.c:177
> #11 0x000000010002e75c in test_cipher_null_iv () at
> ../../tests/unit/test-crypto-cipher.c:749
> #12 0x00007ffff7bbed38 in ?? () from
> /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> #13 0x00007ffff7bbeabc in ?? () from
> /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> #14 0x00007ffff7bbeabc in ?? () from
> /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> #15 0x00007ffff7bbf364 in g_test_run_suite () from
> /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> #16 0x00007ffff7bbf3bc in g_test_run () from
> /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> #17 0x000000010002eb78 in main (argc=1, argv=0x7ffffffff8e8) at
> ../../tests/unit/test-crypto-cipher.c:821
> 
> In frame 9 len is 32 and ctx_>blocksize is 16, so ¯\_(ツ)_/¯

The len in frame 9 is the plain text len, but I think the assert is
complaining about the initialization vector len, which is likely
zero here. I think I know what to fix, but I'm surprised this would
be architecture specific though.

Can you confirm what version of gnutls and nettle you have installed
and what distro this is


Regards,
Daniel
Peter Maydell July 13, 2021, 1:51 p.m. UTC | #3 
On Tue, 13 Jul 2021 at 14:45, Daniel P. Berrangé <berrange@redhat.com> wrote:
> Can you confirm what version of gnutls and nettle you have installed
> and what distro this is

Debian GNU/Linux 11 (bullseye)
libgnutls28-dev:ppc64                  3.7.1-5
libgnutls30:ppc64                      3.7.1-5
nettle-dev:ppc64                       3.7.3-1
libnettle8:ppc64                       3.7.3-1

(If you happen to have an account on the gcc compilefarm, it's
machine gcc203.)

thanks
-- PMM
Daniel P. Berrangé July 13, 2021, 3:45 p.m. UTC | #4 
On Tue, Jul 13, 2021 at 02:45:28PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 13, 2021 at 10:25:44AM +0100, Peter Maydell wrote:
> > On Mon, 12 Jul 2021 at 14:23, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
> > >
> > >   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
> > >
> > > are available in the Git repository at:
> > >
> > >   https://gitlab.com/berrange/qemu tags/crypto-and-more-pull-request
> > >
> > > for you to fetch changes up to 1fc9958410c8683950ea22084b133a755561398b:
> > >
> > >   tests/migration: fix unix socket migration (2021-07-12 14:00:20 +0100)
> > >
> > > ----------------------------------------------------------------
> > > Merge crypto updates and misc fixes
> > >
> > >  * Introduce a GNUTLS backend for crypto algorithms
> > >  * Change crypto library preference gnutls > gcrypt > nettle > built-in
> > >  * Remove built-in DES impl
> > >  * Remove XTS mode from built-in AES impl
> > >  * Fix seccomp rules to allow resource info getters
> > >  * Fix migration performance test
> > >  * Use GDateTime in io/ and net/rocker/ code
> > >
> > > ----------------------------------------------------------------
> > 
> > Hi; this failed 'make check' on ppc64be:
> 
> > The failure is reproducible. Here's a backtrace from a debug
> > build:
> > 
> > test-crypto-cipher: cbc.c:53: nettle_cbc_encrypt: Assertion `!(length
> > % block_size)' failed.
> > 
> > Thread 1 "test-crypto-cip" received signal SIGABRT, Aborted.
> > 0x00007ffff77b8460 in __libc_signal_restore_set (set=0x7fffffffe468)
> > at ../sysdeps/unix/sysv/linux/internal-signals.h:86
> > 86      ../sysdeps/unix/sysv/linux/internal-signals.h: No such file or
> > directory.
> > (gdb) bt
> > #0  0x00007ffff77b8460 in __libc_signal_restore_set
> > (set=0x7fffffffe468) at
> > ../sysdeps/unix/sysv/linux/internal-signals.h:86
> > #1  __GI_raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:48
> > #2  0x00007ffff779bd40 in __GI_abort () at abort.c:79
> > #3  0x00007ffff77ae490 in __assert_fail_base (fmt=<optimized out>,
> >     assertion=assertion@entry=0x7ffff72b6f38 "!(length % block_size)",
> > file=file@entry=0x7ffff72b6f30 "cbc.c", line=line@entry=53,
> >     function=function@entry=0x7ffff72b6f50 "nettle_cbc_encrypt") at assert.c:92
> > #4  0x00007ffff77ae528 in __GI___assert_fail (assertion=0x7ffff72b6f38
> > "!(length % block_size)", file=0x7ffff72b6f30 "cbc.c",
> >     line=<optimized out>, function=0x7ffff72b6f50
> > "nettle_cbc_encrypt") at assert.c:101
> > #5  0x00007ffff728c154 in nettle_cbc_encrypt () from
> > /usr/lib/powerpc64-linux-gnu/libnettle.so.8
> > #6  0x00007ffff7e6b894 in ?? () from
> > /usr/lib/powerpc64-linux-gnu/libgnutls.so.30
> > #7  0x00007ffff7e6c72c in ?? () from
> > /usr/lib/powerpc64-linux-gnu/libgnutls.so.30
> > #8  0x00007ffff7d6d794 in gnutls_cipher_encrypt2 () from
> > /usr/lib/powerpc64-linux-gnu/libgnutls.so.30
> > #9  0x000000010003c330 in qcrypto_gnutls_cipher_encrypt
> > (cipher=0x10016e550, in=0x7fffffffeca8, out=0x7fffffffecc8, len=32,
> >     errp=0x100122b48 <error_abort>) at ../../crypto/cipher-gnutls.c.inc:103
> > #10 0x000000010003cef0 in qcrypto_cipher_encrypt (cipher=0x10016e550,
> > in=0x7fffffffeca8, out=0x7fffffffecc8, len=32,
> >     errp=0x100122b48 <error_abort>) at ../../crypto/cipher.c:177
> > #11 0x000000010002e75c in test_cipher_null_iv () at
> > ../../tests/unit/test-crypto-cipher.c:749
> > #12 0x00007ffff7bbed38 in ?? () from
> > /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> > #13 0x00007ffff7bbeabc in ?? () from
> > /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> > #14 0x00007ffff7bbeabc in ?? () from
> > /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> > #15 0x00007ffff7bbf364 in g_test_run_suite () from
> > /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> > #16 0x00007ffff7bbf3bc in g_test_run () from
> > /usr/lib/powerpc64-linux-gnu/libglib-2.0.so.0
> > #17 0x000000010002eb78 in main (argc=1, argv=0x7ffffffff8e8) at
> > ../../tests/unit/test-crypto-cipher.c:821
> > 
> > In frame 9 len is 32 and ctx_>blocksize is 16, so ¯\_(ツ)_/¯
> 
> The len in frame 9 is the plain text len, but I think the assert is
> complaining about the initialization vector len, which is likely
> zero here. I think I know what to fix, but I'm surprised this would
> be architecture specific though.

Turns out it is related to whether gnutls has hardware acceleration
for CBC mode for a given arch. After compiling gnutls without
acceleration for x86_64, I could reproduce it and figure out a
fix.


Regards,
Daniel