On Mon, Dec 12, 2016 at 04:08:09PM +0800, Longpeng(Mike) wrote:
> This patch add HMAC algorithms based on libgcrypt support
>
> Signed-off-by: Longpeng(Mike) <longpe...@huawei.com>
> ---
> crypto/hmac-gcrypt.c | 138
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 138 insertions(+)
>
> diff --git a/crypto/hmac-gcrypt.c b/crypto/hmac-gcrypt.c
> index 26f42bc..6cf3046 100644
> --- a/crypto/hmac-gcrypt.c
> +++ b/crypto/hmac-gcrypt.c
> @@ -15,6 +15,142 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "crypto/hmac.h"
> +#include <gcrypt.h>
> +
> +#ifdef CONFIG_GCRYPT_SUPPORT_HMAC
You shouldn't use this conditional in the code. Instead, you should
use it in crypto/Makefile.objs, so that this file is only ever
compiled when CONFIG_GCRYPTO_HMAC is set - see the way we deal
with CONFIG_NETTLE_KDF for example.
> +static int qcrypto_hmac_alg_map[QCRYPTO_HMAC_ALG__MAX] = {
> + [QCRYPTO_HMAC_ALG_MD5] = GCRY_MAC_HMAC_MD5,
> + [QCRYPTO_HMAC_ALG_SHA1] = GCRY_MAC_HMAC_SHA1,
> + [QCRYPTO_HMAC_ALG_SHA256] = GCRY_MAC_HMAC_SHA256,
> + [QCRYPTO_HMAC_ALG_SHA512] = GCRY_MAC_HMAC_SHA512,
> +};
Since I requested use of existing QCRYPTO_HASH_ALG_* constants,
can you expand this to handle all 7 hash algoriths we have defined
for qemu
> +QCryptoHmac *qcrypto_hmac_new(QCryptoHmacAlgorithm alg,
> + const uint8_t *key, size_t nkey,
> + Error **errp)
Nitpick, indentation wrt '('
> +{
> + QCryptoHmac *hmac;
> + QCryptoHmacGcrypt *ctx;
> + gcry_error_t err;
> +
> + if (!qcrypto_hmac_supports(alg)) {
> + error_setg(errp, "Unsupported hmac algorithm %s",
> + QCryptoHmacAlgorithm_lookup[alg]);
Again, nitpick on indentation wrt '('
> + return NULL;
> + }
> +
> + hmac = g_new0(QCryptoHmac, 1);
> + hmac->alg = alg;
> +
> + ctx = g_new0(QCryptoHmacGcrypt, 1);
> +
> + err = gcry_mac_open(&ctx->handle, qcrypto_hmac_alg_map[alg],
> + GCRY_MAC_FLAG_SECURE, NULL);
Again, nitpick on indentation
> + if (err != 0) {
> + error_setg(errp, "Cannot initialize hmac: %s",
> + gcry_strerror(err));
Again
> + goto error;
> + }
> +
> + err = gcry_mac_setkey(ctx->handle, (const void *)key, nkey);
> + if (err != 0) {
> + error_setg(errp, "Cannot set key: %s",
> + gcry_strerror(err));
Again. I'll stop repeating this now - just fix up all indentation
through this file and all remaining patches in the series.
> + goto error;
> + }
> +
> + hmac->opaque = ctx;
> + return hmac;
> +
> +error:
> + g_free(ctx);
> + g_free(hmac);
> + return NULL;
> +}
> +
> +void qcrypto_hmac_free(QCryptoHmac *hmac)
> +{
> + QCryptoHmacGcrypt *ctx;
> +
> + if (!hmac) {
> + return;
> + }
> +
> + ctx = hmac->opaque;
> + gcry_mac_close(ctx->handle);
> +
> + g_free(ctx);
> + g_free(hmac);
> +}
> +
> +int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
> + const struct iovec *iov,
> + size_t niov,
> + uint8_t **result,
> + size_t *resultlen,
> + Error **errp)
> +{
> + QCryptoHmacGcrypt *ctx;
> + gcry_error_t err;
> + uint32_t ret;
> + int i;
> +
> + ctx = hmac->opaque;
> +
> + for (i = 0; i < niov; i++) {
> + gcry_mac_write(ctx->handle, iov[i].iov_base, iov[i].iov_len);
> + }
> +
> + ret = gcry_mac_get_algo_maclen(qcrypto_hmac_alg_map[hmac->alg]);
> + if (ret <= 0) {
> + error_setg(errp, "Unable to get hmac length: %s",
> + gcry_strerror(ret));
> + return -1;
> + }
> +
> + if (*resultlen == 0) {
> + *resultlen = ret;
> + *result = g_new0(uint8_t, *resultlen);
> + } else if (*resultlen != ret) {
> + error_setg(errp, "Result buffer size %zu is smaller than hmac %d",
> + *resultlen, ret);
> + return -1;
> + }
> +
> + err = gcry_mac_read(ctx->handle, *result, resultlen);
> + if (err != 0) {
> + error_setg(errp, "Cannot get result: %s",
> + gcry_strerror(err));
> + return -1;
> + }
> +
> + err = gcry_mac_reset(ctx->handle);
> + if (err != 0) {
> + error_setg(errp, "Cannot reset hmac context: %s",
> + gcry_strerror(err));
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +#else
>
> bool qcrypto_hmac_supports(QCryptoHmacAlgorithm alg)
> {
> @@ -42,3 +178,5 @@ int qcrypto_hmac_bytesv(QCryptoHmac *hmac,
> {
> return -1;
> }
> +
> +#endif
You shouldn't need this else block either, since we should never
compile any no-op code - we want to compile hmac-glib instead
if gcrypt doesn't do hmac
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/ :|
