Re: [Qemu-devel] [PATCH] crypto: add CTR mode support

2016-09-22 Thread Gonglei (Arei)

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, September 22, 2016 4:17 PM
> To: qemu-devel@nongnu.org
> Cc: Gonglei (Arei); f...@redhat.com; Wubin (H)
> Subject: Re: [Qemu-devel] [PATCH] crypto: add CTR mode support
> 
> On Thu, Sep 22, 2016 at 12:17:43AM -0700,
> no-re...@ec2-52-6-146-230.compute-1.amazonaws.com wrote:
> > Hi,
> >
> > Your series failed automatic build test. Please find the testing commands 
> > and
> > their output below. If you have docker installed, you can probably 
> > reproduce it
> > locally.
> 
> > GTESTER tests/test-crypto-cipher
> > **
> > ERROR:/tmp/qemu-test/src/tests/test-crypto-cipher.c:528:test_cipher:
> assertion failed: (err == NULL)
> > GTESTER tests/test-crypto-secret
> > GTESTER tests/test-qga
> > GTESTER tests/test-timed-average
> > GTESTER tests/test-io-task
> > GTester: last random seed: R02S94c4e01f686829a0cec78fda5b9551b8
> > **
> > ERROR:/tmp/qemu-test/src/tests/test-crypto-cipher.c:528:test_cipher:
> assertion failed: (err == NULL)
> > GTESTER tests/test-io-channel-socket
> > GTester: last random seed: R02S2749a35e038d902a633846f61116c95d
> > GTESTER tests/test-io-channel-file
> > **
> > ERROR:/tmp/qemu-test/src/tests/test-crypto-cipher.c:528:test_cipher:
> assertion failed: (err == NULL)
> 
> It seems you exposed a pre-existing bug in the test suite. It is using
> qcrypto_cipher_supports() to check if the cipher algorithm is supported
> before running each test case. Unfortunately this assumes that if the
> algorithm is supported, then all cipher modes are also supported, which
> is not true with --disable-gcrypt --disable-nettle configure flags.
> 
Yes, that's right.

> So we need to extend qcrypto_cipher_supports() to take both the algorithm
> and mode as parameters.
> 
Make sense. Let me add another patch to fix it.

Regards,
-Gonglei

> Regards,
> Daniel
> --
> |: http://berrange.com  -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o-
> http://virt-manager.org :|
> |: http://autobuild.org   -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org   -o-
> http://live.gnome.org/gtk-vnc :|


Re: [Qemu-devel] [PATCH] crypto: add CTR mode support

2016-09-22 Thread Daniel P. Berrange
On Thu, Sep 22, 2016 at 12:17:43AM -0700, 
no-re...@ec2-52-6-146-230.compute-1.amazonaws.com wrote:
> Hi,
> 
> Your series failed automatic build test. Please find the testing commands and
> their output below. If you have docker installed, you can probably reproduce 
> it
> locally.

> GTESTER tests/test-crypto-cipher
> **
> ERROR:/tmp/qemu-test/src/tests/test-crypto-cipher.c:528:test_cipher: 
> assertion failed: (err == NULL)
> GTESTER tests/test-crypto-secret
> GTESTER tests/test-qga
> GTESTER tests/test-timed-average
> GTESTER tests/test-io-task
> GTester: last random seed: R02S94c4e01f686829a0cec78fda5b9551b8
> **
> ERROR:/tmp/qemu-test/src/tests/test-crypto-cipher.c:528:test_cipher: 
> assertion failed: (err == NULL)
> GTESTER tests/test-io-channel-socket
> GTester: last random seed: R02S2749a35e038d902a633846f61116c95d
> GTESTER tests/test-io-channel-file
> **
> ERROR:/tmp/qemu-test/src/tests/test-crypto-cipher.c:528:test_cipher: 
> assertion failed: (err == NULL)

It seems you exposed a pre-existing bug in the test suite. It is using
qcrypto_cipher_supports() to check if the cipher algorithm is supported
before running each test case. Unfortunately this assumes that if the
algorithm is supported, then all cipher modes are also supported, which
is not true with --disable-gcrypt --disable-nettle configure flags.

So we need to extend qcrypto_cipher_supports() to take both the algorithm
and mode as parameters.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH] crypto: add CTR mode support

2016-09-22 Thread no-reply
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 1474525997-318296-1-git-send-email-arei.gong...@huawei.com
Subject: [Qemu-devel] [PATCH] crypto: add CTR mode support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7bbb09d crypto: add CTR mode support

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/src/tests/docker/install
BIOS directory/tmp/qemu-test/src/tests/docker/install/share/qemu
binary directory  /tmp/qemu-test/src/tests/docker/install/bin
library directory /tmp/qemu-test/src/tests/docker/install/lib
module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
include directory /tmp/qemu-test/src/tests/docker/install/include
config directory  /tmp/qemu-test/src/tests/docker/install/etc
local state directory   /tmp/qemu-test/src/tests/docker/install/var
Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
uuid support  no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backendslog
spice support no 
rbd support   no
xfsctl supportno
smartcard support no
libusbno
usb net redir no
OpenGL supportno
OpenGL dmabufsno
libiscsi support  no
libnfs supportno
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine poolyes
GlusterFS support no
Archipelago support no
gcov  gcov
gcov enabled  no
TPM support   yes
libssh2 support   no
TPM passthrough   yes
QOM debugging yes
vhdx  no
lzo support   no
snappy supportno
bzip2 support no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
replication support yes
  GEN   x86_64-softmmu/config-devices.mak.tmp
  GEN   aarch64-softmmu/config-devices.mak.tmp
  GEN   config-host.h
  GEN   qemu-options.def
  GEN   qmp-commands.h
  GEN   qapi-types.h
  GEN   qapi-visit.h
  GEN   qapi-event.h
  GEN   x86_64-softmmu/config-devices.mak
  GEN   aarch64-softmmu/config-devices.

[Qemu-devel] [PATCH] crypto: add CTR mode support

2016-09-21 Thread Gonglei
Introduce CTR mode support for the cipher APIs.
CTR mode uses a counter rather than a traditional IV.
The counter has additional properties, including a nonce
and initial counter block. We reuse the ctx->iv as
the counter for conveniences.

Both libgcrypt and nettle support CTR mode, the
cipher-builtin doesn't support yet.

Signed-off-by: Gonglei 
---
 crypto/cipher-gcrypt.c | 25 -
 crypto/cipher-nettle.c | 15 -
 crypto/cipher.c|  1 +
 include/crypto/cipher.h|  6 ++---
 qapi/crypto.json   |  3 ++-
 tests/test-crypto-cipher.c | 55 ++
 6 files changed, 94 insertions(+), 11 deletions(-)

diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index da3f4c7..97b015a 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -48,6 +48,7 @@ struct QCryptoCipherGcrypt {
 gcry_cipher_hd_t handle;
 gcry_cipher_hd_t tweakhandle;
 size_t blocksize;
+/* Initialization vector or Counter */
 uint8_t *iv;
 };
 
@@ -69,6 +70,9 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
 case QCRYPTO_CIPHER_MODE_CBC:
 gcrymode = GCRY_CIPHER_MODE_CBC;
 break;
+case QCRYPTO_CIPHER_MODE_CTR:
+gcrymode = GCRY_CIPHER_MODE_CTR;
+break;
 default:
 error_setg(errp, "Unsupported cipher mode %s",
QCryptoCipherMode_lookup[mode]);
@@ -339,12 +343,21 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
 if (ctx->iv) {
 memcpy(ctx->iv, iv, niv);
 } else {
-gcry_cipher_reset(ctx->handle);
-err = gcry_cipher_setiv(ctx->handle, iv, niv);
-if (err != 0) {
-error_setg(errp, "Cannot set IV: %s",
-   gcry_strerror(err));
-return -1;
+if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) {
+err = gcry_cipher_setctr(ctx->handle, iv, niv);
+if (err != 0) {
+error_setg(errp, "Cannot set Counter: %s",
+   gcry_strerror(err));
+return -1;
+}
+} else {
+gcry_cipher_reset(ctx->handle);
+err = gcry_cipher_setiv(ctx->handle, iv, niv);
+if (err != 0) {
+error_setg(errp, "Cannot set IV: %s",
+   gcry_strerror(err));
+return -1;
+}
 }
 }
 
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index 879d831..4b673aa 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 typedef void (*QCryptoCipherNettleFuncWrapper)(const void *ctx,
size_t length,
@@ -186,7 +187,7 @@ struct QCryptoCipherNettle {
 QCryptoCipherNettleFuncNative alg_decrypt_native;
 QCryptoCipherNettleFuncWrapper alg_encrypt_wrapper;
 QCryptoCipherNettleFuncWrapper alg_decrypt_wrapper;
-
+/* Initialization vector or Counter */
 uint8_t *iv;
 size_t blocksize;
 };
@@ -225,6 +226,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
 case QCRYPTO_CIPHER_MODE_ECB:
 case QCRYPTO_CIPHER_MODE_CBC:
 case QCRYPTO_CIPHER_MODE_XTS:
+case QCRYPTO_CIPHER_MODE_CTR:
 break;
 default:
 error_setg(errp, "Unsupported cipher mode %s",
@@ -430,6 +432,12 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
 ctx->iv, len, out, in);
 break;
 
+case QCRYPTO_CIPHER_MODE_CTR:
+ctr_crypt(ctx->ctx, ctx->alg_encrypt_native,
+ctx->blocksize, ctx->iv,
+len, out, in);
+break;
+
 default:
 error_setg(errp, "Unsupported cipher mode %s",
QCryptoCipherMode_lookup[cipher->mode]);
@@ -469,6 +477,11 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
 ctx->alg_encrypt_wrapper, ctx->alg_decrypt_wrapper,
 ctx->iv, len, out, in);
 break;
+case QCRYPTO_CIPHER_MODE_CTR:
+ctr_crypt(ctx->ctx, ctx->alg_encrypt_native,
+ctx->blocksize, ctx->iv,
+len, out, in);
+break;
 
 default:
 error_setg(errp, "Unsupported cipher mode %s",
diff --git a/crypto/cipher.c b/crypto/cipher.c
index cafb454..a9bca41 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -55,6 +55,7 @@ static bool mode_need_iv[QCRYPTO_CIPHER_MODE__MAX] = {
 [QCRYPTO_CIPHER_MODE_ECB] = false,
 [QCRYPTO_CIPHER_MODE_CBC] = true,
 [QCRYPTO_CIPHER_MODE_XTS] = true,
+[QCRYPTO_CIPHER_MODE_CTR] = true,
 };
 
 
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index 376654d..f9015e1 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -213,16 +213,16 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
 /**
  * qcrypto_cipher_setiv:
  * @cipher: the cipher object
- * @iv: the initialization vector bytes
+ * @iv: