Re: [Qemu-devel] [PATCH v3 04/10] cryptodev: introduce gcrypt lib as a new cryptodev backend

[Daniel P. Berrange]

On Mon, Sep 19, 2016 at 04:16:16PM +0800, Gonglei wrote:
> Signed-off-by: Gonglei <address@hidden>
> ---
>  crypto/Makefile.objs      |   1 +
>  crypto/cryptodev-gcrypt.c | 329 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx           |  18 +++
>  3 files changed, 348 insertions(+)
>  create mode 100644 crypto/cryptodev-gcrypt.c
> 
> diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
> index f7f3c4f..bd8aea7 100644
> --- a/crypto/Makefile.objs
> +++ b/crypto/Makefile.objs
> @@ -27,6 +27,7 @@ crypto-obj-y += block.o
>  crypto-obj-y += block-qcow.o
>  crypto-obj-y += block-luks.o
>  crypto-obj-y += cryptodev.o
> +crypto-obj-$(CONFIG_GCRYPT) += cryptodev-gcrypt.o

This can be just crypto-obj-y +=

>  # Let the userspace emulators avoid linking gnutls/etc
>  crypto-aes-obj-y = aes.o
> diff --git a/crypto/cryptodev-gcrypt.c b/crypto/cryptodev-gcrypt.c
> new file mode 100644
> index 0000000..66a0e5e
> --- /dev/null
> +++ b/crypto/cryptodev-gcrypt.c
> +/**
> + * @TYPE_QCRYPTO_CRYPTODEV_BACKEND_GCRYPT:
> + * name of backend that uses gcrypt library
> + */
> +#define TYPE_QCRYPTO_CRYPTODEV_BACKEND_GCRYPT "cryptodev-backend-gcrypt"

I'd suggest we just call this backend "builtin", so do a
replace of "gcrypt" with "builtin" throughout.

> +static void qcrypto_cryptodev_backend_gcrypt_init(
> +             QCryptoCryptoDevBackend *backend, Error **errp)
> +{
> +    /* Only support one queue */
> +    int queues = MAX(backend->conf.peers.queues, 1);
> +    int i;

Nitpick, I prefer to see 'size_t' for list iterators
that are always positive. Similar comment in other
places in this series using int i

> +    QCryptoCryptoDevBackendClientState *cc;
> +
> +    for (i = 0; i < queues; i++) {
> +        cc = qcrypto_cryptodev_backend_new_client(
> +                  "cryptodev-gcrypt", NULL);
> +        snprintf(cc->info_str, sizeof(cc->info_str),
> +                 "cryptodev-gcrypt%d", i);
> +        cc->queue_index = i;
> +
> +        backend->conf.peers.ccs[i] = cc;
> +    }
> +
> +    backend->conf.crypto_services =
> +                         1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
> +                         1u << VIRTIO_CRYPTO_SERVICE_HASH |
> +                         1u << VIRTIO_CRYPTO_SERVICE_MAC;
> +    backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
> +    backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
> +}
> +
> +static int
> +qcrypto_cryptodev_backend_gcrypt_get_unused_session_index(
> +      QCryptoCryptoDevBackendGcrypt *gcrypt)
> +{
> +    int i;
> +
> +    for (i = 0; i < MAX_NUM_SESSIONS; i++) {
> +        if (gcrypt->sessions[i] == NULL) {
> +            return i;
> +        }
> +    }
> +
> +    return -1;
> +}
> +
> +static int qcrypto_cryptodev_backend_gcrypt_create_cipher_session(
> +                    QCryptoCryptoDevBackendGcrypt *gcrypt,
> +                    QCryptoCryptoDevBackendSymSessionInfo *sess_info,
> +                    Error **errp)
> +{
> +    int algo;
> +    int mode;
> +    QCryptoCipher *cipher;
> +    int index;
> +    QCryptoCryptoDevBackendGcryptSession *sess;
> +
> +    if (sess_info->op_type != VIRTIO_CRYPTO_SYM_OP_CIPHER) {
> +        error_setg(errp, "unsupported optype :%u", sess_info->op_type);
> +        return -1;
> +    }
> +
> +    index = 
> qcrypto_cryptodev_backend_gcrypt_get_unused_session_index(gcrypt);
> +    if (index < 0) {
> +        error_setg(errp, "the total number of created session exceed %u",
> +                  MAX_NUM_SESSIONS);
> +        return -1;
> +    }
> +
> +    switch (sess_info->cipher_alg) {
> +    case VIRTIO_CRYPTO_CIPHER_AES_ECB:
> +        if (sess_info->key_len == 128 / 8) {
> +            algo = QCRYPTO_CIPHER_ALG_AES_128;
> +        } else if (sess_info->key_len == 192 / 8) {
> +            algo = QCRYPTO_CIPHER_ALG_AES_192;
> +        } else if (sess_info->key_len == 256 / 8) {
> +            algo = QCRYPTO_CIPHER_ALG_AES_256;
> +        } else {
> +            error_setg(errp, "unsupported key length :%u",
> +                       sess_info->key_len);
> +            return -1;
> +        }
> +        mode = QCRYPTO_CIPHER_MODE_ECB;
> +        break;
> +    case VIRTIO_CRYPTO_CIPHER_AES_CBC:
> +        if (sess_info->key_len == 128 / 8) {
> +            algo = QCRYPTO_CIPHER_ALG_AES_128;
> +        } else if (sess_info->key_len == 192 / 8) {
> +            algo = QCRYPTO_CIPHER_ALG_AES_192;
> +        } else if (sess_info->key_len == 256 / 8) {
> +            algo = QCRYPTO_CIPHER_ALG_AES_256;
> +        } else {
> +            error_setg(errp, "unsupported key length :%u",
> +                       sess_info->key_len);
> +            return -1;
> +        }
> +        mode = QCRYPTO_CIPHER_MODE_CBC;
> +        break;
> +    case VIRTIO_CRYPTO_CIPHER_AES_CTR:

Although the QEMU cipher.h API does not export CTR mode currently
it should be trivial to add it. So feel free to add a patch at
the start of the series implementing CTR mode in the cipher API.
Both gcrypt and nettle have support for it which is all we need.

> +    default:
> +        error_setg(errp, "unsupported cipher alg :%u",
> +                   sess_info->cipher_alg);
> +        return -1;
> +    }
> +
> +    cipher = qcrypto_cipher_new(algo, mode,
> +                               sess_info->cipher_key,
> +                               sess_info->key_len,
> +                               errp);
> +    if (!cipher) {
> +        return -1;
> +    }
> +
> +    sess = g_new0(QCryptoCryptoDevBackendGcryptSession, 1);
> +    sess->cipher = cipher;
> +    sess->direction = sess_info->direction;
> +    sess->type = sess_info->op_type;
> +
> +    gcrypt->sessions[index] = sess;
> +
> +    return index;
> +}

> +
> +static int64_t qcrypto_cryptodev_backend_gcrypt_sym_create_session(
> +           QCryptoCryptoDevBackend *backend,
> +           QCryptoCryptoDevBackendSymSessionInfo *sess_info,
> +           uint32_t queue_index, Error **errp)
> +{
> +    QCryptoCryptoDevBackendGcrypt *gcrypt =
> +                      QCRYPTO_CRYPTODEV_BACKEND_GCRYPT(backend);
> +    int64_t session_id = -1;
> +    int ret;
> +
> +    switch (sess_info->op_code) {
> +    case VIRTIO_CRYPTO_CIPHER_CREATE_SESSION:
> +        ret = qcrypto_cryptodev_backend_gcrypt_create_cipher_session(
> +                           gcrypt, sess_info, errp);
> +        if (ret < 0) {
> +            return ret;
> +        } else {
> +            session_id = ret;
> +        }
> +        break;
> +    case VIRTIO_CRYPTO_HASH_CREATE_SESSION:
> +    case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
> +    default:
> +        error_setg(errp, "unsupported opcode :%" PRIu32 "",
> +                   sess_info->op_code);
> +        return -1;
> +    }
> +
> +    return session_id;
> +}
> +
> +static int qcrypto_cryptodev_backend_gcrypt_sym_close_session(
> +           QCryptoCryptoDevBackend *backend,
> +           uint64_t session_id, Error **errp)
> +{
> +    QCryptoCryptoDevBackendGcrypt *gcrypt =
> +                      QCRYPTO_CRYPTODEV_BACKEND_GCRYPT(backend);
> +
> +    if (session_id >= MAX_NUM_SESSIONS ||
> +              gcrypt->sessions[session_id] == NULL) {
> +        error_setg(errp, "Cannot find a valid session id: %" PRIu64 "",
> +                      session_id);
> +        return -1;
> +    }
> +
> +    qcrypto_cipher_free(gcrypt->sessions[session_id]->cipher);
> +    g_free(gcrypt->sessions[session_id]);
> +    gcrypt->sessions[session_id] = NULL;
> +    return 0;
> +}
> +
> +static int qcrypto_cryptodev_backend_gcrypt_sym_operation(
> +                 QCryptoCryptoDevBackend *backend,
> +                 QCryptoCryptoDevBackendSymOpInfo *op_info,
> +                 uint32_t queue_index, Error **errp)
> +{
> +    QCryptoCryptoDevBackendGcrypt *gcrypt =
> +                      QCRYPTO_CRYPTODEV_BACKEND_GCRYPT(backend);
> +    QCryptoCryptoDevBackendGcryptSession *sess;
> +    int ret;
> +
> +    if (op_info->session_id >= MAX_NUM_SESSIONS ||
> +              gcrypt->sessions[op_info->session_id] == NULL) {
> +        error_setg(errp, "Cannot find a valid session id: %" PRIu64 "",
> +                   op_info->session_id);
> +        return -VIRTIO_CRYPTO_OP_INVSESS;
> +    }
> +
> +    sess = gcrypt->sessions[op_info->session_id];
> +
> +    ret = qcrypto_cipher_setiv(sess->cipher, op_info->iv,
> +                               op_info->iv_len, errp);
> +    if (ret < 0) {
> +        return -VIRTIO_CRYPTO_OP_ERR;
> +    }
> +
> +    if (sess->direction == VIRTIO_CRYPTO_OP_ENCRYPT) {
> +        ret = qcrypto_cipher_encrypt(sess->cipher, op_info->src,
> +                                     op_info->dst, op_info->src_len, errp);
> +        if (ret < 0) {
> +            return -VIRTIO_CRYPTO_OP_ERR;
> +        }
> +    } else {
> +        ret = qcrypto_cipher_decrypt(sess->cipher, op_info->src,
> +                                     op_info->dst, op_info->src_len, errp);
> +        if (ret < 0) {
> +            return -VIRTIO_CRYPTO_OP_ERR;
> +        }
> +    }
> +    return 0;
> +}


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 :|