[Qemu-devel] [PATCH for-2.9 0/3] crypto: add standard des support

[Qemu-devel] [PATCH for-2.9 0/3] crypto: add standard des support

[Longpeng(Mike)]

This patchset add standard DES support when using gcrypt/nettle.

Qemu only support DES-RFB which is used by vnc-auth currently,
however gcrypt/nettle library support standard DES in actually,
what's more, due to the virtio-crypto has been supportted, the
guest can use various cipher-algo including standard DES.

Longpeng(Mike) (3):
  crypto: add standard des support
  cryptodev: switch to standard des
  tests: crypto: add testcase for standard des(ecb)

 backends/cryptodev-builtin.c |  2 +-
 crypto/cipher-gcrypt.c       |  3 +++
 crypto/cipher-nettle.c       | 13 ++++++++++---
 crypto/cipher.c              |  5 ++++-
 qapi/crypto.json             |  2 +-
 tests/test-crypto-cipher.c   | 11 +++++++++++
 6 files changed, 30 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Longpeng(Mike)]

This patch add standart DES support.

Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
---
 crypto/cipher-gcrypt.c |  3 +++
 crypto/cipher-nettle.c | 13 ++++++++++---
 crypto/cipher.c        |  5 ++++-
 qapi/crypto.json       |  2 +-
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index c550db9..7ca049c 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -28,6 +28,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
 {
     switch (alg) {
+    case QCRYPTO_CIPHER_ALG_DES:
     case QCRYPTO_CIPHER_ALG_DES_RFB:
     case QCRYPTO_CIPHER_ALG_AES_128:
     case QCRYPTO_CIPHER_ALG_AES_192:
@@ -95,6 +96,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
     }
 
     switch (alg) {
+    case QCRYPTO_CIPHER_ALG_DES:
     case QCRYPTO_CIPHER_ALG_DES_RFB:
         gcryalg = GCRY_CIPHER_DES;
         break;
@@ -200,6 +202,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         case QCRYPTO_CIPHER_ALG_TWOFISH_256:
             ctx->blocksize = 16;
             break;
+        case QCRYPTO_CIPHER_ALG_DES:
         case QCRYPTO_CIPHER_ALG_CAST5_128:
             ctx->blocksize = 8;
             break;
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index cd094cd..81cc634 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -196,6 +196,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
 {
     switch (alg) {
+    case QCRYPTO_CIPHER_ALG_DES:
     case QCRYPTO_CIPHER_ALG_DES_RFB:
     case QCRYPTO_CIPHER_ALG_AES_128:
     case QCRYPTO_CIPHER_ALG_AES_192:
@@ -256,11 +257,17 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
     ctx = g_new0(QCryptoCipherNettle, 1);
 
     switch (alg) {
+    case QCRYPTO_CIPHER_ALG_DES:
     case QCRYPTO_CIPHER_ALG_DES_RFB:
         ctx->ctx = g_new0(struct des_ctx, 1);
-        rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
-        des_set_key(ctx->ctx, rfbkey);
-        g_free(rfbkey);
+
+        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
+            rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
+            des_set_key(ctx->ctx, rfbkey);
+            g_free(rfbkey);
+        } else {
+            des_set_key(ctx->ctx, key);
+        }
 
         ctx->alg_encrypt_native = des_encrypt_native;
         ctx->alg_decrypt_native = des_decrypt_native;
diff --git a/crypto/cipher.c b/crypto/cipher.c
index a9bca41..00d9682 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -27,6 +27,7 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_AES_128] = 16,
     [QCRYPTO_CIPHER_ALG_AES_192] = 24,
     [QCRYPTO_CIPHER_ALG_AES_256] = 32,
+    [QCRYPTO_CIPHER_ALG_DES] = 8,
     [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
     [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
     [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
@@ -41,6 +42,7 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_AES_128] = 16,
     [QCRYPTO_CIPHER_ALG_AES_192] = 16,
     [QCRYPTO_CIPHER_ALG_AES_256] = 16,
+    [QCRYPTO_CIPHER_ALG_DES] = 8,
     [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
     [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
     [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
@@ -107,7 +109,8 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
     }
 
     if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
+        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
+            || alg == QCRYPTO_CIPHER_ALG_DES) {
             error_setg(errp, "XTS mode not compatible with DES-RFB");
             return false;
         }
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 5c9d7d4..d403ab9 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -75,7 +75,7 @@
 { 'enum': 'QCryptoCipherAlgorithm',
   'prefix': 'QCRYPTO_CIPHER_ALG',
   'data': ['aes-128', 'aes-192', 'aes-256',
-           'des-rfb',
+           'des-rfb', 'des',
            'cast5-128',
            'serpent-128', 'serpent-192', 'serpent-256',
            'twofish-128', 'twofish-192', 'twofish-256']}
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.9 2/3] cryptodev: switch to standard des

[Longpeng(Mike)]

The cryptodev use DES-RFB for ecb(des) currently, we should use
standard DES for cryptodev instead.

Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
---
 backends/cryptodev-builtin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index ecad565..f1b5b1b 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -207,7 +207,7 @@ static int cryptodev_builtin_create_cipher_session(
         break;
     case VIRTIO_CRYPTO_CIPHER_DES_ECB:
         mode = QCRYPTO_CIPHER_MODE_ECB;
-        algo = QCRYPTO_CIPHER_ALG_DES_RFB;
+        algo = QCRYPTO_CIPHER_ALG_DES;
         break;
     default:
         error_setg(errp, "Unsupported cipher alg :%u",
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.9 3/3] tests: crypto: add testcase for standard des(ecb)

[Longpeng(Mike)]

As we have added standart DES support when using gcrypt/nettle,
so this patch add a testcase for standard ecb(des).

Note: the data is copied from linux-kernel's tcrypt module.

Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
---
 tests/test-crypto-cipher.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index 5d9e535..382d77e 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -150,6 +150,17 @@ static QCryptoCipherTestData test_data[] = {
             "b2eb05e2c39be9fcda6c19078c6a9d1b",
     },
     {
+        .path = "/crypto/cipher/des-ecb",
+        .alg = QCRYPTO_CIPHER_ALG_DES,
+        .mode = QCRYPTO_CIPHER_MODE_ECB,
+        .key =
+            "0123456789abcdef",
+        .plaintext =
+            "0123456789abcde7",
+        .ciphertext =
+            "c95744256a5ed31d",
+    },
+    {
         .path = "/crypto/cipher/des-rfb-ecb-56",
         .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
         .mode = QCRYPTO_CIPHER_MODE_ECB,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Daniel P. Berrange]

On Mon, Dec 05, 2016 at 04:59:38PM +0800, Longpeng(Mike) wrote:
> This patch add standart DES support.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---
>  crypto/cipher-gcrypt.c |  3 +++
>  crypto/cipher-nettle.c | 13 ++++++++++---
>  crypto/cipher.c        |  5 ++++-
>  qapi/crypto.json       |  2 +-
>  4 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
> index c550db9..7ca049c 100644
> --- a/crypto/cipher-gcrypt.c
> +++ b/crypto/cipher-gcrypt.c
> @@ -28,6 +28,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
>                               QCryptoCipherMode mode)
>  {
>      switch (alg) {
> +    case QCRYPTO_CIPHER_ALG_DES:
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
>      case QCRYPTO_CIPHER_ALG_AES_128:
>      case QCRYPTO_CIPHER_ALG_AES_192:
> @@ -95,6 +96,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
>      }
>  
>      switch (alg) {
> +    case QCRYPTO_CIPHER_ALG_DES:
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
>          gcryalg = GCRY_CIPHER_DES;
>          break;
> @@ -200,6 +202,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
>          case QCRYPTO_CIPHER_ALG_TWOFISH_256:
>              ctx->blocksize = 16;
>              break;
> +        case QCRYPTO_CIPHER_ALG_DES:
>          case QCRYPTO_CIPHER_ALG_CAST5_128:
>              ctx->blocksize = 8;
>              break;
> diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
> index cd094cd..81cc634 100644
> --- a/crypto/cipher-nettle.c
> +++ b/crypto/cipher-nettle.c
> @@ -196,6 +196,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
>                               QCryptoCipherMode mode)
>  {
>      switch (alg) {
> +    case QCRYPTO_CIPHER_ALG_DES:
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
>      case QCRYPTO_CIPHER_ALG_AES_128:
>      case QCRYPTO_CIPHER_ALG_AES_192:
> @@ -256,11 +257,17 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
>      ctx = g_new0(QCryptoCipherNettle, 1);
>  
>      switch (alg) {
> +    case QCRYPTO_CIPHER_ALG_DES:
>      case QCRYPTO_CIPHER_ALG_DES_RFB:
>          ctx->ctx = g_new0(struct des_ctx, 1);
> -        rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> -        des_set_key(ctx->ctx, rfbkey);
> -        g_free(rfbkey);
> +
> +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> +            rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> +            des_set_key(ctx->ctx, rfbkey);
> +            g_free(rfbkey);
> +        } else {
> +            des_set_key(ctx->ctx, key);
> +        }
>  
>          ctx->alg_encrypt_native = des_encrypt_native;
>          ctx->alg_decrypt_native = des_decrypt_native;
> diff --git a/crypto/cipher.c b/crypto/cipher.c
> index a9bca41..00d9682 100644
> --- a/crypto/cipher.c
> +++ b/crypto/cipher.c
> @@ -27,6 +27,7 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
>      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
>      [QCRYPTO_CIPHER_ALG_AES_192] = 24,
>      [QCRYPTO_CIPHER_ALG_AES_256] = 32,
> +    [QCRYPTO_CIPHER_ALG_DES] = 8,
>      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
>      [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
>      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> @@ -41,6 +42,7 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
>      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
>      [QCRYPTO_CIPHER_ALG_AES_192] = 16,
>      [QCRYPTO_CIPHER_ALG_AES_256] = 16,
> +    [QCRYPTO_CIPHER_ALG_DES] = 8,
>      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
>      [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
>      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> @@ -107,7 +109,8 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
>      }
>  
>      if (mode == QCRYPTO_CIPHER_MODE_XTS) {
> -        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
> +            || alg == QCRYPTO_CIPHER_ALG_DES) {
>              error_setg(errp, "XTS mode not compatible with DES-RFB");
>              return false;
>          }
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 5c9d7d4..d403ab9 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -75,7 +75,7 @@
>  { 'enum': 'QCryptoCipherAlgorithm',
>    'prefix': 'QCRYPTO_CIPHER_ALG',
>    'data': ['aes-128', 'aes-192', 'aes-256',
> -           'des-rfb',
> +           'des-rfb', 'des',

Can we call this '3des' to make it clear that this is Triple-DES and not
the single-DES (which des-rfb is)

>             'cast5-128',
>             'serpent-128', 'serpent-192', 'serpent-256',
>             'twofish-128', 'twofish-192', 'twofish-256']}

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

Re: [Qemu-devel] [PATCH for-2.9 3/3] tests: crypto: add testcase for standard des(ecb)

[Daniel P. Berrange]

On Mon, Dec 05, 2016 at 04:59:40PM +0800, Longpeng(Mike) wrote:
> As we have added standart DES support when using gcrypt/nettle,
> so this patch add a testcase for standard ecb(des).
> 
> Note: the data is copied from linux-kernel's tcrypt module.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---
>  tests/test-crypto-cipher.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
> index 5d9e535..382d77e 100644
> --- a/tests/test-crypto-cipher.c
> +++ b/tests/test-crypto-cipher.c
> @@ -150,6 +150,17 @@ static QCryptoCipherTestData test_data[] = {
>              "b2eb05e2c39be9fcda6c19078c6a9d1b",
>      },
>      {
> +        .path = "/crypto/cipher/des-ecb",
> +        .alg = QCRYPTO_CIPHER_ALG_DES,
> +        .mode = QCRYPTO_CIPHER_MODE_ECB,
> +        .key =
> +            "0123456789abcdef",
> +        .plaintext =
> +            "0123456789abcde7",
> +        .ciphertext =
> +            "c95744256a5ed31d",
> +    },
> +    {
>          .path = "/crypto/cipher/des-rfb-ecb-56",
>          .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
>          .mode = QCRYPTO_CIPHER_MODE_ECB,

This should be included as part of the patch which adds the cipher
in the first place. Ideally there should be a CBC mode test as well.

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

Re: [Qemu-devel] [PATCH for-2.9 2/3] cryptodev: switch to standard des

[Daniel P. Berrange]

On Mon, Dec 05, 2016 at 04:59:39PM +0800, Longpeng(Mike) wrote:
> The cryptodev use DES-RFB for ecb(des) currently, we should use
> standard DES for cryptodev instead.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---
>  backends/cryptodev-builtin.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> index ecad565..f1b5b1b 100644
> --- a/backends/cryptodev-builtin.c
> +++ b/backends/cryptodev-builtin.c
> @@ -207,7 +207,7 @@ static int cryptodev_builtin_create_cipher_session(
>          break;
>      case VIRTIO_CRYPTO_CIPHER_DES_ECB:
>          mode = QCRYPTO_CIPHER_MODE_ECB;
> -        algo = QCRYPTO_CIPHER_ALG_DES_RFB;
> +        algo = QCRYPTO_CIPHER_ALG_DES;
>          break;
>      default:
>          error_setg(errp, "Unsupported cipher alg :%u",

This will need updating due to my suggestion to change the enum name, but
regardless this fix is clearly needed - we don't want cryptodev using the
VNC custom bit-reversed single-DES

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

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

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Gonglei (Arei)]

>
> >      switch (alg) {
> > +    case QCRYPTO_CIPHER_ALG_DES:
> >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> >      case QCRYPTO_CIPHER_ALG_AES_128:
> >      case QCRYPTO_CIPHER_ALG_AES_192:
> > @@ -256,11 +257,17 @@ QCryptoCipher
> *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
> >      ctx = g_new0(QCryptoCipherNettle, 1);
> >
> >      switch (alg) {
> > +    case QCRYPTO_CIPHER_ALG_DES:
> >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> >          ctx->ctx = g_new0(struct des_ctx, 1);
> > -        rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > -        des_set_key(ctx->ctx, rfbkey);
> > -        g_free(rfbkey);
> > +
> > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > +            rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > +            des_set_key(ctx->ctx, rfbkey);
> > +            g_free(rfbkey);
> > +        } else {
> > +            des_set_key(ctx->ctx, key);
> > +        }
> >
> >          ctx->alg_encrypt_native = des_encrypt_native;
> >          ctx->alg_decrypt_native = des_decrypt_native;
> > diff --git a/crypto/cipher.c b/crypto/cipher.c
> > index a9bca41..00d9682 100644
> > --- a/crypto/cipher.c
> > +++ b/crypto/cipher.c
> > @@ -27,6 +27,7 @@ static size_t
> alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
> >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> >      [QCRYPTO_CIPHER_ALG_AES_192] = 24,
> >      [QCRYPTO_CIPHER_ALG_AES_256] = 32,
> > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
> >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > @@ -41,6 +42,7 @@ static size_t
> alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
> >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> >      [QCRYPTO_CIPHER_ALG_AES_192] = 16,
> >      [QCRYPTO_CIPHER_ALG_AES_256] = 16,
> > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
> >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > @@ -107,7 +109,8 @@
> qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
> >      }
> >
> >      if (mode == QCRYPTO_CIPHER_MODE_XTS) {
> > -        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
> > +            || alg == QCRYPTO_CIPHER_ALG_DES) {
> >              error_setg(errp, "XTS mode not compatible with DES-RFB");
> >              return false;
> >          }
> > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > index 5c9d7d4..d403ab9 100644
> > --- a/qapi/crypto.json
> > +++ b/qapi/crypto.json
> > @@ -75,7 +75,7 @@
> >  { 'enum': 'QCryptoCipherAlgorithm',
> >    'prefix': 'QCRYPTO_CIPHER_ALG',
> >    'data': ['aes-128', 'aes-192', 'aes-256',
> > -           'des-rfb',
> > +           'des-rfb', 'des',
> 
> Can we call this '3des' to make it clear that this is Triple-DES and not
> the single-DES (which des-rfb is)
> 
Actually the current des is not triple-DES, just the single-DES, and des-rfb in QEMU is just a variant of
single DES, which change the standard key by calling qcrypto_cipher_munge_des_rfb_key().

I think we can add the 3des support as well in the next step.

The current single-DES in the patch set is ok to me. :)

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH for-2.9 3/3] tests: crypto: add testcase for standard des(ecb)

[Longpeng (Mike)]

Hi Daniel,

On 2016/12/5 17:24, Daniel P. Berrange wrote:

> On Mon, Dec 05, 2016 at 04:59:40PM +0800, Longpeng(Mike) wrote:
......
>>      {
>> +        .path = "/crypto/cipher/des-ecb",
>> +        .alg = QCRYPTO_CIPHER_ALG_DES,
>> +        .mode = QCRYPTO_CIPHER_MODE_ECB,
>> +        .key =
>> +            "0123456789abcdef",
>> +        .plaintext =
>> +            "0123456789abcde7",
>> +        .ciphertext =
>> +            "c95744256a5ed31d",
>> +    },
>> +    {
>>          .path = "/crypto/cipher/des-rfb-ecb-56",
>>          .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
>>          .mode = QCRYPTO_CIPHER_MODE_ECB,
> 
> This should be included as part of the patch which adds the cipher
> in the first place. Ideally there should be a CBC mode test as well.
> 

Okay.
I will put this in the first patch(which adds the cipher) and add a cbc(aes)
testcase in v2. :)

> Regards,
> Daniel


-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Longpeng (Mike)]

Hi Daniel,

On 2016/12/5 17:18, Daniel P. Berrange wrote:

> On Mon, Dec 05, 2016 at 04:59:38PM +0800, Longpeng(Mike) wrote:
......
>> diff --git a/qapi/crypto.json b/qapi/crypto.json
>> index 5c9d7d4..d403ab9 100644
>> --- a/qapi/crypto.json
>> +++ b/qapi/crypto.json
>> @@ -75,7 +75,7 @@
>>  { 'enum': 'QCryptoCipherAlgorithm',
>>    'prefix': 'QCRYPTO_CIPHER_ALG',
>>    'data': ['aes-128', 'aes-192', 'aes-256',
>> -           'des-rfb',
>> +           'des-rfb', 'des',
> 
> Can we call this '3des' to make it clear that this is Triple-DES and not
> the single-DES (which des-rfb is)
> 

As the comment in qapi/crypto.json said:
@des-rfb: RFB specific variant of single DES.

This patch just add the standard single-DES support, not the triple-DES, so I
think maybe "des" is suitable.

And we will add "3des" in the near-future.

>>             'cast5-128',
>>             'serpent-128', 'serpent-192', 'serpent-256',
>>             'twofish-128', 'twofish-192', 'twofish-256']}
> 
> Regards,
> Daniel


-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Daniel P. Berrange]

On Mon, Dec 05, 2016 at 07:11:37PM +0800, Longpeng (Mike) wrote:
> Hi Daniel,
> 
> On 2016/12/5 17:18, Daniel P. Berrange wrote:
> 
> > On Mon, Dec 05, 2016 at 04:59:38PM +0800, Longpeng(Mike) wrote:
> ......
> >> diff --git a/qapi/crypto.json b/qapi/crypto.json
> >> index 5c9d7d4..d403ab9 100644
> >> --- a/qapi/crypto.json
> >> +++ b/qapi/crypto.json
> >> @@ -75,7 +75,7 @@
> >>  { 'enum': 'QCryptoCipherAlgorithm',
> >>    'prefix': 'QCRYPTO_CIPHER_ALG',
> >>    'data': ['aes-128', 'aes-192', 'aes-256',
> >> -           'des-rfb',
> >> +           'des-rfb', 'des',
> > 
> > Can we call this '3des' to make it clear that this is Triple-DES and not
> > the single-DES (which des-rfb is)
> > 
> 
> As the comment in qapi/crypto.json said:
> @des-rfb: RFB specific variant of single DES.
> 
> This patch just add the standard single-DES support, not the triple-DES, so I
> think maybe "des" is suitable.

Oh I missed that - QEMU should not support single-DES at all for
cryptodev IMHO. Single DES has been cryptographically broken/useless
for *decades* - way back in 1999, the EFF built a machine that could
brute force single-DES in a mere 56 hours.

Triple-DES is the bare minimum that's acceptable and even that
should only be for legacy usage which can't use a more modern
cipher like AES

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

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Daniel P. Berrange]

On Mon, Dec 05, 2016 at 09:29:59AM +0000, Gonglei (Arei) wrote:
> >
> > >      switch (alg) {
> > > +    case QCRYPTO_CIPHER_ALG_DES:
> > >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> > >      case QCRYPTO_CIPHER_ALG_AES_128:
> > >      case QCRYPTO_CIPHER_ALG_AES_192:
> > > @@ -256,11 +257,17 @@ QCryptoCipher
> > *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
> > >      ctx = g_new0(QCryptoCipherNettle, 1);
> > >
> > >      switch (alg) {
> > > +    case QCRYPTO_CIPHER_ALG_DES:
> > >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> > >          ctx->ctx = g_new0(struct des_ctx, 1);
> > > -        rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > -        des_set_key(ctx->ctx, rfbkey);
> > > -        g_free(rfbkey);
> > > +
> > > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > +            rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > +            des_set_key(ctx->ctx, rfbkey);
> > > +            g_free(rfbkey);
> > > +        } else {
> > > +            des_set_key(ctx->ctx, key);
> > > +        }
> > >
> > >          ctx->alg_encrypt_native = des_encrypt_native;
> > >          ctx->alg_decrypt_native = des_decrypt_native;
> > > diff --git a/crypto/cipher.c b/crypto/cipher.c
> > > index a9bca41..00d9682 100644
> > > --- a/crypto/cipher.c
> > > +++ b/crypto/cipher.c
> > > @@ -27,6 +27,7 @@ static size_t
> > alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > >      [QCRYPTO_CIPHER_ALG_AES_192] = 24,
> > >      [QCRYPTO_CIPHER_ALG_AES_256] = 32,
> > > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> > >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
> > >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > @@ -41,6 +42,7 @@ static size_t
> > alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > >      [QCRYPTO_CIPHER_ALG_AES_192] = 16,
> > >      [QCRYPTO_CIPHER_ALG_AES_256] = 16,
> > > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> > >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
> > >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > @@ -107,7 +109,8 @@
> > qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
> > >      }
> > >
> > >      if (mode == QCRYPTO_CIPHER_MODE_XTS) {
> > > -        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
> > > +            || alg == QCRYPTO_CIPHER_ALG_DES) {
> > >              error_setg(errp, "XTS mode not compatible with DES-RFB");
> > >              return false;
> > >          }
> > > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > > index 5c9d7d4..d403ab9 100644
> > > --- a/qapi/crypto.json
> > > +++ b/qapi/crypto.json
> > > @@ -75,7 +75,7 @@
> > >  { 'enum': 'QCryptoCipherAlgorithm',
> > >    'prefix': 'QCRYPTO_CIPHER_ALG',
> > >    'data': ['aes-128', 'aes-192', 'aes-256',
> > > -           'des-rfb',
> > > +           'des-rfb', 'des',
> > 
> > Can we call this '3des' to make it clear that this is Triple-DES and not
> > the single-DES (which des-rfb is)
> > 
> Actually the current des is not triple-DES, just the single-DES, and des-rfb in QEMU is just a variant of
> single DES, which change the standard key by calling qcrypto_cipher_munge_des_rfb_key().
> 
> I think we can add the 3des support as well in the next step.
> 
> The current single-DES in the patch set is ok to me. :)

Per my othre reply in this thread, I don't think we should be supporting
single-DES at all in QEMU / cryptodev. So IMHO, the correct fix is to
remove the single-DES support from cryptodev entirely

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

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On 12/05/2016 02:59 AM, Longpeng(Mike) wrote:
> This patch add standart DES support.

s/standart/standard/

> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---

> +++ b/qapi/crypto.json
> @@ -75,7 +75,7 @@
>  { 'enum': 'QCryptoCipherAlgorithm',
>    'prefix': 'QCRYPTO_CIPHER_ALG',
>    'data': ['aes-128', 'aes-192', 'aes-256',
> -           'des-rfb',
> +           'des-rfb', 'des',

Missing documentation that includes a (since 2.9) blurb

>             'cast5-128',
>             'serpent-128', 'serpent-192', 'serpent-256',
>             'twofish-128', 'twofish-192', 'twofish-256']}
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Gonglei (Arei)]

>
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, December 06, 2016 12:59 AM
> To: Gonglei (Arei)
> Cc: longpeng; eblake@redhat.com; armbru@redhat.com;
> qemu-devel@nongnu.org; Wubin (H); Zhoujian (jay, Euler)
> Subject: Re: [PATCH for-2.9 1/3] crypto: add standard des support
> 
> On Mon, Dec 05, 2016 at 09:29:59AM +0000, Gonglei (Arei) wrote:
> > >
> > > >      switch (alg) {
> > > > +    case QCRYPTO_CIPHER_ALG_DES:
> > > >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> > > >      case QCRYPTO_CIPHER_ALG_AES_128:
> > > >      case QCRYPTO_CIPHER_ALG_AES_192:
> > > > @@ -256,11 +257,17 @@ QCryptoCipher
> > > *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
> > > >      ctx = g_new0(QCryptoCipherNettle, 1);
> > > >
> > > >      switch (alg) {
> > > > +    case QCRYPTO_CIPHER_ALG_DES:
> > > >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> > > >          ctx->ctx = g_new0(struct des_ctx, 1);
> > > > -        rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > > -        des_set_key(ctx->ctx, rfbkey);
> > > > -        g_free(rfbkey);
> > > > +
> > > > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > > +            rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > > +            des_set_key(ctx->ctx, rfbkey);
> > > > +            g_free(rfbkey);
> > > > +        } else {
> > > > +            des_set_key(ctx->ctx, key);
> > > > +        }
> > > >
> > > >          ctx->alg_encrypt_native = des_encrypt_native;
> > > >          ctx->alg_decrypt_native = des_decrypt_native;
> > > > diff --git a/crypto/cipher.c b/crypto/cipher.c
> > > > index a9bca41..00d9682 100644
> > > > --- a/crypto/cipher.c
> > > > +++ b/crypto/cipher.c
> > > > @@ -27,6 +27,7 @@ static size_t
> > > alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > > >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > > >      [QCRYPTO_CIPHER_ALG_AES_192] = 24,
> > > >      [QCRYPTO_CIPHER_ALG_AES_256] = 32,
> > > > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> > > >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > > >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
> > > >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > > @@ -41,6 +42,7 @@ static size_t
> > > alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > > >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > > >      [QCRYPTO_CIPHER_ALG_AES_192] = 16,
> > > >      [QCRYPTO_CIPHER_ALG_AES_256] = 16,
> > > > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> > > >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > > >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
> > > >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > > @@ -107,7 +109,8 @@
> > > qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
> > > >      }
> > > >
> > > >      if (mode == QCRYPTO_CIPHER_MODE_XTS) {
> > > > -        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
> > > > +            || alg == QCRYPTO_CIPHER_ALG_DES) {
> > > >              error_setg(errp, "XTS mode not compatible with
> DES-RFB");
> > > >              return false;
> > > >          }
> > > > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > > > index 5c9d7d4..d403ab9 100644
> > > > --- a/qapi/crypto.json
> > > > +++ b/qapi/crypto.json
> > > > @@ -75,7 +75,7 @@
> > > >  { 'enum': 'QCryptoCipherAlgorithm',
> > > >    'prefix': 'QCRYPTO_CIPHER_ALG',
> > > >    'data': ['aes-128', 'aes-192', 'aes-256',
> > > > -           'des-rfb',
> > > > +           'des-rfb', 'des',
> > >
> > > Can we call this '3des' to make it clear that this is Triple-DES and not
> > > the single-DES (which des-rfb is)
> > >
> > Actually the current des is not triple-DES, just the single-DES, and des-rfb in
> QEMU is just a variant of
> > single DES, which change the standard key by calling
> qcrypto_cipher_munge_des_rfb_key().
> >
> > I think we can add the 3des support as well in the next step.
> >
> > The current single-DES in the patch set is ok to me. :)
> 
> Per my othre reply in this thread,

I saw that, thanks for your information, Daniel.

> I don't think we should be supporting
> single-DES at all in QEMU / cryptodev. So IMHO, the correct fix is to
> remove the single-DES support from cryptodev entirely
> 
The cryptodev-builtin is one kind of cryptodev backends. It provides the
real crypto capability for virtio crypto device.
 
I don't think we should artificially remove one algorithm support if
the frontend driver (users) wants to use it, though the algorithm is
unsafe.

Of course, if the QEMU crypto API doesn't provide single-DES support,
we should remove it from the cryptodev-builtin backend as well. ;)

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Daniel P. Berrange]

On Tue, Dec 06, 2016 at 01:23:31AM +0000, Gonglei (Arei) wrote:
> >
> > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > Sent: Tuesday, December 06, 2016 12:59 AM
> > To: Gonglei (Arei)
> > Cc: longpeng; eblake@redhat.com; armbru@redhat.com;
> > qemu-devel@nongnu.org; Wubin (H); Zhoujian (jay, Euler)
> > Subject: Re: [PATCH for-2.9 1/3] crypto: add standard des support
> > 
> > On Mon, Dec 05, 2016 at 09:29:59AM +0000, Gonglei (Arei) wrote:
> > > >
> > > > >      switch (alg) {
> > > > > +    case QCRYPTO_CIPHER_ALG_DES:
> > > > >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> > > > >      case QCRYPTO_CIPHER_ALG_AES_128:
> > > > >      case QCRYPTO_CIPHER_ALG_AES_192:
> > > > > @@ -256,11 +257,17 @@ QCryptoCipher
> > > > *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
> > > > >      ctx = g_new0(QCryptoCipherNettle, 1);
> > > > >
> > > > >      switch (alg) {
> > > > > +    case QCRYPTO_CIPHER_ALG_DES:
> > > > >      case QCRYPTO_CIPHER_ALG_DES_RFB:
> > > > >          ctx->ctx = g_new0(struct des_ctx, 1);
> > > > > -        rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > > > -        des_set_key(ctx->ctx, rfbkey);
> > > > > -        g_free(rfbkey);
> > > > > +
> > > > > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > > > +            rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > > > +            des_set_key(ctx->ctx, rfbkey);
> > > > > +            g_free(rfbkey);
> > > > > +        } else {
> > > > > +            des_set_key(ctx->ctx, key);
> > > > > +        }
> > > > >
> > > > >          ctx->alg_encrypt_native = des_encrypt_native;
> > > > >          ctx->alg_decrypt_native = des_decrypt_native;
> > > > > diff --git a/crypto/cipher.c b/crypto/cipher.c
> > > > > index a9bca41..00d9682 100644
> > > > > --- a/crypto/cipher.c
> > > > > +++ b/crypto/cipher.c
> > > > > @@ -27,6 +27,7 @@ static size_t
> > > > alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > > > >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > > > >      [QCRYPTO_CIPHER_ALG_AES_192] = 24,
> > > > >      [QCRYPTO_CIPHER_ALG_AES_256] = 32,
> > > > > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> > > > >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > > > >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
> > > > >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > > > @@ -41,6 +42,7 @@ static size_t
> > > > alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > > > >      [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > > > >      [QCRYPTO_CIPHER_ALG_AES_192] = 16,
> > > > >      [QCRYPTO_CIPHER_ALG_AES_256] = 16,
> > > > > +    [QCRYPTO_CIPHER_ALG_DES] = 8,
> > > > >      [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > > > >      [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
> > > > >      [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > > > @@ -107,7 +109,8 @@
> > > > qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
> > > > >      }
> > > > >
> > > > >      if (mode == QCRYPTO_CIPHER_MODE_XTS) {
> > > > > -        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > > > +        if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
> > > > > +            || alg == QCRYPTO_CIPHER_ALG_DES) {
> > > > >              error_setg(errp, "XTS mode not compatible with
> > DES-RFB");
> > > > >              return false;
> > > > >          }
> > > > > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > > > > index 5c9d7d4..d403ab9 100644
> > > > > --- a/qapi/crypto.json
> > > > > +++ b/qapi/crypto.json
> > > > > @@ -75,7 +75,7 @@
> > > > >  { 'enum': 'QCryptoCipherAlgorithm',
> > > > >    'prefix': 'QCRYPTO_CIPHER_ALG',
> > > > >    'data': ['aes-128', 'aes-192', 'aes-256',
> > > > > -           'des-rfb',
> > > > > +           'des-rfb', 'des',
> > > >
> > > > Can we call this '3des' to make it clear that this is Triple-DES and not
> > > > the single-DES (which des-rfb is)
> > > >
> > > Actually the current des is not triple-DES, just the single-DES, and des-rfb in
> > QEMU is just a variant of
> > > single DES, which change the standard key by calling
> > qcrypto_cipher_munge_des_rfb_key().
> > >
> > > I think we can add the 3des support as well in the next step.
> > >
> > > The current single-DES in the patch set is ok to me. :)
> > 
> > Per my othre reply in this thread,
> 
> I saw that, thanks for your information, Daniel.
> 
> > I don't think we should be supporting
> > single-DES at all in QEMU / cryptodev. So IMHO, the correct fix is to
> > remove the single-DES support from cryptodev entirely
> > 
> The cryptodev-builtin is one kind of cryptodev backends. It provides the
> real crypto capability for virtio crypto device.
>  
> I don't think we should artificially remove one algorithm support if
> the frontend driver (users) wants to use it, though the algorithm is
> unsafe.

IIUC the cryptodev hardware is ultimately about allowing the guest
to offload crypto operations to the host, potentialy using hardware
acceleration. If the cryptodev backend doesn't support a particular
algorithm, the guest is still capable of using its own built-in
support for that algorithm. I see no compelling reason to provide
host offload / acceleration for single-DES. Just kill this obsolete
algorithm from cryptodev and in the unlikely event that a guest
really does want single-DES it can use its built-in impl instead.

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

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Gonglei (Arei)]

>
> > > > > >          }
> > > > > > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > > > > > index 5c9d7d4..d403ab9 100644
> > > > > > --- a/qapi/crypto.json
> > > > > > +++ b/qapi/crypto.json
> > > > > > @@ -75,7 +75,7 @@
> > > > > >  { 'enum': 'QCryptoCipherAlgorithm',
> > > > > >    'prefix': 'QCRYPTO_CIPHER_ALG',
> > > > > >    'data': ['aes-128', 'aes-192', 'aes-256',
> > > > > > -           'des-rfb',
> > > > > > +           'des-rfb', 'des',
> > > > >
> > > > > Can we call this '3des' to make it clear that this is Triple-DES and not
> > > > > the single-DES (which des-rfb is)
> > > > >
> > > > Actually the current des is not triple-DES, just the single-DES, and des-rfb
> in
> > > QEMU is just a variant of
> > > > single DES, which change the standard key by calling
> > > qcrypto_cipher_munge_des_rfb_key().
> > > >
> > > > I think we can add the 3des support as well in the next step.
> > > >
> > > > The current single-DES in the patch set is ok to me. :)
> > >
> > > Per my othre reply in this thread,
> >
> > I saw that, thanks for your information, Daniel.
> >
> > > I don't think we should be supporting
> > > single-DES at all in QEMU / cryptodev. So IMHO, the correct fix is to
> > > remove the single-DES support from cryptodev entirely
> > >
> > The cryptodev-builtin is one kind of cryptodev backends. It provides the
> > real crypto capability for virtio crypto device.
> >
> > I don't think we should artificially remove one algorithm support if
> > the frontend driver (users) wants to use it, though the algorithm is
> > unsafe.
> 
> IIUC the cryptodev hardware is ultimately about allowing the guest
> to offload crypto operations to the host, potentialy using hardware
> acceleration. If the cryptodev backend doesn't support a particular
> algorithm, the guest is still capable of using its own built-in
> support for that algorithm. I see no compelling reason to provide
> host offload / acceleration for single-DES. Just kill this obsolete
> algorithm from cryptodev and in the unlikely event that a guest
> really does want single-DES it can use its built-in impl instead.
> 
Make sense. And I don't want to support single-DES in the virtio-crypto
frontend driver as well. The guest will use the software realization.

Thanks,
-Gonglei

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Longpeng (Mike)]

Hi Eric,

On 2016/12/6 3:15, Eric Blake wrote:

> On 12/05/2016 02:59 AM, Longpeng(Mike) wrote:
>> This patch add standart DES support.
> 
> s/standart/standard/
> 
>>
>> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
>> ---
> 
>> +++ b/qapi/crypto.json
>> @@ -75,7 +75,7 @@
>>  { 'enum': 'QCryptoCipherAlgorithm',
>>    'prefix': 'QCRYPTO_CIPHER_ALG',
>>    'data': ['aes-128', 'aes-192', 'aes-256',
>> -           'des-rfb',
>> +           'des-rfb', 'des',
> 
> Missing documentation that includes a (since 2.9) blurb
> 

Thanks, but I decide to disable standard single-DES in cryptodev, as suggested
by Daniel.

>>             'cast5-128',
>>             'serpent-128', 'serpent-192', 'serpent-256',
>>             'twofish-128', 'twofish-192', 'twofish-256']}
>>
> 


-- 
Regards,
Longpeng(Mike)