[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/10] crypto: introduce generic cipher API & bu
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH 04/10] crypto: introduce generic cipher API & built-in implementation |
Date: |
Thu, 21 May 2015 12:52:43 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 05/21/2015 03:56 AM, Daniel P. Berrange wrote:
> +QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
> + QCryptoCipherMode mode,
> + const uint8_t *key, size_t nkey,
> + Error **errp)
> +{
> + QCryptoCipher *cipher;
> +
> + cipher = g_new0(QCryptoCipher, 1);
> + cipher->alg = alg;
> + cipher->mode = mode;
> +
> + switch (cipher->alg) {
> + case QCRYPTO_CIPHER_ALG_DES_RFB:
> + if (qcrypto_cipher_init_des_rfb(cipher, key, nkey, errp) < 0) {
> + goto error;
> + }
> + break;
> + case QCRYPTO_CIPHER_ALG_AES:
> + if (qcrypto_cipher_init_aes(cipher, key, nkey, errp) < 0) {
> + goto error;
> + }
> + break;
> + default:
> + error_setg(errp,
> + _("Unsupported cipher algorithm %d"), cipher->alg);
> + goto error;
> + }
> +
> + return cipher;
> +
> + error:
> + g_free(cipher);
> + return NULL;
> +}
Is it really that helpful to have all of these switches, as opposed to having
one function per cipher and calling it directly? Similarly for the hashing.
The uses I pulled out of the later patches are like
+ if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
+ qiov->iov, qiov->niov,
+ &data, &len,
+ NULL) < 0) {
+ return -EINVAL;
+ if (qcrypto_hash_base64(QCRYPTO_HASH_ALG_SHA1,
+ combined_key,
+ WS_CLIENT_KEY_LEN + WS_GUID_LEN,
+ &accept,
+ &err) < 0) {
+ cipher = qcrypto_cipher_new(
+ QCRYPTO_CIPHER_ALG_DES_RFB,
+ QCRYPTO_CIPHER_MODE_ECB,
+ key, G_N_ELEMENTS(key),
+ &err);
+ s->cipher = qcrypto_cipher_new(
+ QCRYPTO_CIPHER_ALG_AES,
+ QCRYPTO_CIPHER_MODE_CBC,
+ keybuf, G_N_ELEMENTS(keybuf),
+ &err);
This one could have explicitly specified AES128, but you hid that behind
G_N_ELEMENTS. Which seems like obfuscation to me, but...
r~
- [Qemu-devel] [PATCH 00/10] Consolidate crypto APIs & implementations, Daniel P. Berrange, 2015/05/21
- [Qemu-devel] [PATCH 07/10] block: convert quorum blockdrv to use crypto APIs, Daniel P. Berrange, 2015/05/21
- [Qemu-devel] [PATCH 05/10] crypto: add a gcrypt cipher implementation, Daniel P. Berrange, 2015/05/21
- [Qemu-devel] [PATCH 08/10] ui: convert VNC websockets to use crypto APIs, Daniel P. Berrange, 2015/05/21
- [Qemu-devel] [PATCH 06/10] crypto: add a nettle cipher implementation, Daniel P. Berrange, 2015/05/21