Hi Daniel,
Thanks for your review, and I fix them as your suggestion in V3,
please review V3 when you're free. :)
Regards,
On 2016/12/12 18:31, Daniel P. Berrange wrote:
> On Mon, Dec 12, 2016 at 04:08:12PM +0800, Longpeng(Mike) wrote:
>> This patch add HMAC algorithms testcases
>>
>> Signed-off-by: Longpeng(Mike) <longpe...@huawei.com>
>> ---
>> tests/Makefile.include | 2 +
>> tests/test-crypto-hmac.c | 166
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 168 insertions(+)
>> create mode 100644 tests/test-crypto-hmac.c
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index e98d3b6..4841d58 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -91,6 +91,7 @@ gcov-files-test-qemu-opts-y = qom/test-qemu-opts.c
>> check-unit-y += tests/test-write-threshold$(EXESUF)
>> gcov-files-test-write-threshold-y = block/write-threshold.c
>> check-unit-y += tests/test-crypto-hash$(EXESUF)
>> +check-unit-y += tests/test-crypto-hmac$(EXESUF)
>> check-unit-y += tests/test-crypto-cipher$(EXESUF)
>> check-unit-y += tests/test-crypto-secret$(EXESUF)
>> check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
>> @@ -571,6 +572,7 @@ tests/test-opts-visitor$(EXESUF):
>> tests/test-opts-visitor.o $(test-qapi-obj-y)
>> tests/test-mul64$(EXESUF): tests/test-mul64.o $(test-util-obj-y)
>> tests/test-bitops$(EXESUF): tests/test-bitops.o $(test-util-obj-y)
>> tests/test-crypto-hash$(EXESUF): tests/test-crypto-hash.o
>> $(test-crypto-obj-y)
>> +tests/test-crypto-hmac$(EXESUF): tests/test-crypto-hmac.o
>> $(test-crypto-obj-y)
>> tests/test-crypto-cipher$(EXESUF): tests/test-crypto-cipher.o
>> $(test-crypto-obj-y)
>> tests/test-crypto-secret$(EXESUF): tests/test-crypto-secret.o
>> $(test-crypto-obj-y)
>> tests/test-crypto-xts$(EXESUF): tests/test-crypto-xts.o $(test-crypto-obj-y)
>> diff --git a/tests/test-crypto-hmac.c b/tests/test-crypto-hmac.c
>> new file mode 100644
>> index 0000000..678df52
>> --- /dev/null
>> +++ b/tests/test-crypto-hmac.c
>> @@ -0,0 +1,166 @@
>> +/*
>> + * QEMU Crypto hmac algorithms tests
>> + *
>> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
>> + *
>> + * Authors:
>> + * Longpeng(Mike) <longpe...@huawei.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> + * (at your option) any later version. See the COPYING file in the
>> + * top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "crypto/init.h"
>> +#include "crypto/hmac.h"
>> +
>> +typedef struct QCryptoHmacTestData QCryptoHmacTestData;
>> +struct QCryptoHmacTestData {
>> + const char *path;
>> + QCryptoHmacAlgorithm alg;
>> + const char *key;
>> + const char *message;
>> + const char *digest;
>> +};
>> +
>> +static QCryptoHmacTestData test_data[] = {
>> + {
>> + .path = "/crypto/hmac/hmac-md5",
>> + .alg = QCRYPTO_HMAC_ALG_MD5,
>> + .key =
>> + "0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b",
>> + .message =
>> + "4869205468657265",
>> + .digest =
>> + "9294727a3638bb1c13f48ef8158bfc9d",
>> + },
>> + {
>> + .path = "/crypto/hmac/hmac-sha1",
>> + .alg = QCRYPTO_HMAC_ALG_SHA1,
>> + .key =
>> + "0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b"
>> + "0b0b0b0b",
>> + .message =
>> + "4869205468657265",
>> + .digest =
>> + "b617318655057264e28bc0b6fb378c8e"
>> + "f146be00",
>> + },
>> +};
>
> This is quite weak test coverage - it needs to cover all 7 hash algorithms
>
>
>> +
>> +static inline int unhex(char c)
>> +{
>> + if (c >= 'a' && c <= 'f') {
>> + return 10 + (c - 'a');
>> + }
>> + if (c >= 'A' && c <= 'F') {
>> + return 10 + (c - 'A');
>> + }
>> + return c - '0';
>> +}
>> +
>> +static inline char hex(int i)
>> +{
>> + if (i < 10) {
>> + return '0' + i;
>> + }
>> + return 'a' + (i - 10);
>> +}
>> +
>> +static size_t unhex_string(const char *hexstr,
>> + uint8_t **data)
>> +{
>> + size_t len;
>> + size_t i;
>> +
>> + if (!hexstr) {
>> + *data = NULL;
>> + return 0;
>> + }
>> +
>> + len = strlen(hexstr);
>> + *data = g_new0(uint8_t, len / 2);
>> +
>> + for (i = 0; i < len; i += 2) {
>> + (*data)[i / 2] = (unhex(hexstr[i]) << 4) | unhex(hexstr[i + 1]);
>> + }
>> + return len / 2;
>> +}
>> +
>> +static char *hex_string(const uint8_t *bytes,
>> + size_t len)
>> +{
>> + char *hexstr = g_new0(char, len * 2 + 1);
>> + size_t i;
>> +
>> + for (i = 0; i < len; i++) {
>> + hexstr[i * 2] = hex((bytes[i] >> 4) & 0xf);
>> + hexstr[i * 2 + 1] = hex(bytes[i] & 0xf);
>> + }
>> + hexstr[len * 2] = '\0';
>> +
>> + return hexstr;
>> +}
>> +
>> +static void test_hmac(const void *opaque)
>> +{
>> + const QCryptoHmacTestData *data = opaque;
>> + size_t nkey, digest_len, msg_len;
>> + uint8_t *key = NULL;
>> + uint8_t *message = NULL;
>> + uint8_t *digest = NULL;
>> + uint8_t *output = NULL;
>> + char *outputhex = NULL;
>> + QCryptoHmac *hmac = NULL;
>> + Error *err = NULL;
>> + int ret;
>> +
>> + if (!qcrypto_hmac_supports(data->alg)) {
>> + return;
>> + }
>> +
>> + nkey = unhex_string(data->key, &key);
>> + digest_len = unhex_string(data->digest, &digest);
>> + msg_len = unhex_string(data->message, &message);
>> +
>> + output = g_new0(uint8_t, digest_len);
>> +
>> + hmac = qcrypto_hmac_new(data->alg, key, nkey, &err);
>> + g_assert(err == NULL);
>> + g_assert(hmac != NULL);
>> +
>> + ret = qcrypto_hmac_bytes(hmac, (const char *)message,
>> + msg_len, &output, &digest_len, &err);
>> +
>> + g_assert(ret == 0);
>> +
>> + outputhex = hex_string(output, digest_len);
>> +
>> + g_assert_cmpstr(outputhex, ==, data->digest);
>> +
>> + qcrypto_hmac_free(hmac);
>> +
>> + g_free(outputhex);
>> + g_free(output);
>> + g_free(message);
>> + g_free(digest);
>> + g_free(key);
>> +}
>
>
> We need to cover qcrypto_hmac_bytesv and qcrypto_hmac_digest
> methods too.
>
> IOW, can you simply copy the test-crypto-hash.c test suite entirely
> but remove the base64 part of it.
>
> Regards,
> Daniel
--
Regards,
Longpeng(Mike)
