[libvirt] [PATCH 0/5] Rely on GnuTLS for md5/sha256 functions
Ján Tomko (5): vircrypto: provide constants for hash sizes Introduce virCryptoHashBuf esx: use virCryptoHashBuf esx: Use VIR_CRYPTO_HASH_SIZE_MD5 vircrypto: Rely on GnuTLS for hash functions bootstrap.conf | 2 -- src/esx/esx_network_driver.c | 22 ++++++++------ src/esx/esx_storage_backend_iscsi.c | 46 +++++++++++++++++------------- src/esx/esx_storage_backend_vmfs.c | 20 ++++++------- src/util/vircrypto.c | 57 ++++++++++++++++++++++++++----------- src/util/vircrypto.h | 10 +++++++ 6 files changed, 100 insertions(+), 57 deletions(-) -- 2.16.1
The callers needing to know the size of the resulting digest rely on _DIGEST_SIZE constants from gnulib. Introduce VIR_CRYPTO_HASH_SIZE_ constants to remove the dependency. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/vircrypto.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h index 068602f5df..81743d2f74 100644 --- a/src/util/vircrypto.h +++ b/src/util/vircrypto.h @@ -23,6 +23,9 @@ # include "internal.h" +# define VIR_CRYPTO_HASH_SIZE_MD5 16 +# define VIR_CRYPTO_HASH_SIZE_SHA256 32 + typedef enum { VIR_CRYPTO_HASH_MD5, /* Don't use this except for historic compat */ VIR_CRYPTO_HASH_SHA256, -- 2.16.1
A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/vircrypto.c | 31 +++++++++++++++++++++---------- src/util/vircrypto.h | 7 +++++++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); int -virCryptoHashString(virCryptoHash hash, - const char *input, - char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { - unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; - size_t hashstrlen; - size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; } - hashstrlen = (hashinfo[hash].hashlen * 2) + 1; - - if (!(hashinfo[hash].func(input, strlen(input), buf))) { + if (!(hashinfo[hash].func(input, strlen(input), output))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to compute hash of data")); return -1; } + return 0; +} + +int +virCryptoHashString(virCryptoHash hash, + const char *input, + char **output) +{ + unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; + size_t hashstrlen; + size_t i; + + if (virCryptoHashBuf(hash, input, buf) < 0) + return -1; + + hashstrlen = (hashinfo[hash].hashlen * 2) + 1; + if (VIR_ALLOC_N(*output, hashstrlen) < 0) return -1; diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h index 81743d2f74..64984006be 100644 --- a/src/util/vircrypto.h +++ b/src/util/vircrypto.h @@ -41,6 +41,13 @@ typedef enum { VIR_CRYPTO_CIPHER_LAST } virCryptoCipher; +int +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + int virCryptoHashString(virCryptoHash hash, const char *input, -- 2.16.1
On 05/11/2018 05:50 PM, Ján Tomko wrote:
A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/vircrypto.c | 31 +++++++++++++++++++++---------- src/util/vircrypto.h | 7 +++++++ 2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST);
int -virCryptoHashString(virCryptoHash hash, - const char *input, - char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { - unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; - size_t hashstrlen; - size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; }
This check feels useless. It's like if we were checking a value before passing it to vir*TypeToString(). But it's pre-existing, so you have my ACK. But a follow up patch removing it (=trivial) would be nice. Also, don't forget to export the symbol in libvirt_private.syms ;-) I'm wondering how linker succeeds in 3/5 where you use the function without it being exported. Maybe my compiler decided to inline this function? Michal
On Mon, May 14, 2018 at 11:17:51AM +0200, Michal Privoznik wrote:
On 05/11/2018 05:50 PM, Ján Tomko wrote:
A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/vircrypto.c | 31 +++++++++++++++++++++---------- src/util/vircrypto.h | 7 +++++++ 2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST);
int -virCryptoHashString(virCryptoHash hash, - const char *input, - char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { - unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; - size_t hashstrlen; - size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; }
This check feels useless. It's like if we were checking a value before passing it to vir*TypeToString(). But it's pre-existing, so you have my ACK. But a follow up patch removing it (=trivial) would be nice.
On one hand, this check should be pointless if the rest of our code is correct. On the other hand, our functions in src/util usually do perform some validation of the parameters. Although it could be transformed to use virReportEnumRangeError.
Also, don't forget to export the symbol in libvirt_private.syms ;-) I'm wondering how linker succeeds in 3/5 where you use the function without it being exported. Maybe my compiler decided to inline this function?
Thanks, fixed. Jano
Michal
On 05/11/2018 11:50 AM, Ján Tomko wrote:
A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/vircrypto.c | 31 +++++++++++++++++++++---------- src/util/vircrypto.h | 7 +++++++ 2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST);
int -virCryptoHashString(virCryptoHash hash, - const char *input, - char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { - unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; - size_t hashstrlen; - size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; }
- hashstrlen = (hashinfo[hash].hashlen * 2) + 1; - - if (!(hashinfo[hash].func(input, strlen(input), buf))) { + if (!(hashinfo[hash].func(input, strlen(input), output))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to compute hash of data")); return -1; }
+ return 0; +} + +int +virCryptoHashString(virCryptoHash hash, + const char *input, + char **output) +{ + unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; + size_t hashstrlen; + size_t i; + + if (virCryptoHashBuf(hash, input, buf) < 0) + return -1; + + hashstrlen = (hashinfo[hash].hashlen * 2) + 1; +
How about virCryptoHashBuf returning the number of raw bytes it produced so that not all callers need to look it up? Stefan
if (VIR_ALLOC_N(*output, hashstrlen) < 0) return -1;
diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h index 81743d2f74..64984006be 100644 --- a/src/util/vircrypto.h +++ b/src/util/vircrypto.h @@ -41,6 +41,13 @@ typedef enum { VIR_CRYPTO_CIPHER_LAST } virCryptoCipher;
+int +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + int virCryptoHashString(virCryptoHash hash, const char *input,
Instead of using md5_buffer from gnulib directly. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/esx/esx_network_driver.c | 13 +++++++++---- src/esx/esx_storage_backend_iscsi.c | 19 +++++++++++++------ src/esx/esx_storage_backend_vmfs.c | 9 +++++---- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index b19c06a4cb..7386efb0f5 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -33,6 +33,7 @@ #include "esx_vi.h" #include "esx_vi_methods.h" #include "esx_util.h" +#include "vircrypto.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_ESX @@ -152,7 +153,8 @@ esxNetworkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch; hostVirtualSwitch = hostVirtualSwitch->_next) { - md5_buffer(hostVirtualSwitch->key, strlen(hostVirtualSwitch->key), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostVirtualSwitch->key, md5) < 0) + goto cleanup; if (memcmp(uuid, md5, VIR_UUID_BUFLEN) == 0) break; @@ -201,7 +203,8 @@ esxNetworkLookupByName(virConnectPtr conn, const char *name) * The MD5 sum of the key can be used as UUID, assuming MD5 is considered * to be collision-free enough for this use case. */ - md5_buffer(hostVirtualSwitch->key, strlen(hostVirtualSwitch->key), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostVirtualSwitch->key, md5) < 0) + return NULL; network = virGetNetwork(conn, hostVirtualSwitch->name, md5); @@ -464,7 +467,8 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } - md5_buffer(hostVirtualSwitch->key, strlen(hostVirtualSwitch->key), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostVirtualSwitch->key, md5) < 0) + goto cleanup; network = virGetNetwork(conn, hostVirtualSwitch->name, md5); @@ -655,7 +659,8 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) goto cleanup; } - md5_buffer(hostVirtualSwitch->key, strlen(hostVirtualSwitch->key), def->uuid); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostVirtualSwitch->key, def->uuid) < 0) + goto cleanup; if (VIR_STRDUP(def->name, hostVirtualSwitch->name) < 0) goto cleanup; diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index b106c517e8..42b52214d1 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -37,6 +37,7 @@ #include "esx_vi.h" #include "esx_vi_methods.h" #include "esx_util.h" +#include "vircrypto.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_ESX @@ -180,7 +181,8 @@ esxStoragePoolLookupByName(virConnectPtr conn, * but iScsiName (or widely known as IQN) is unique across the multiple * hosts, using it to compute key */ - md5_buffer(target->iScsiName, strlen(target->iScsiName), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0) + goto cleanup; pool = virGetStoragePool(conn, name, md5, &esxStorageBackendISCSI, NULL); @@ -218,7 +220,8 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, for (target = hostInternetScsiHba->configuredStaticTarget; target; target = target->_next) { - md5_buffer(target->iScsiName, strlen(target->iScsiName), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0) + goto cleanup; if (memcmp(uuid, md5, VIR_UUID_BUFLEN) == 0) break; @@ -456,7 +459,8 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, * compute MD5 hash to transform it to an acceptable * libvirt format */ - md5_buffer(scsiLun->uuid, strlen(scsiLun->uuid), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0) + goto cleanup; virUUIDFormat(md5, uuid_string); /* @@ -507,7 +511,8 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) goto cleanup; } - md5_buffer(scsiLun->uuid, strlen(scsiLun->uuid), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0) + goto cleanup; virUUIDFormat(md5, uuid_string); volume = virGetStorageVol(conn, poolName, path, uuid_string, @@ -549,7 +554,8 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) memset(uuid_string, '\0', sizeof(uuid_string)); memset(md5, '\0', sizeof(md5)); - md5_buffer(scsiLun->uuid, strlen(scsiLun->uuid), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0) + goto cleanup; virUUIDFormat(md5, uuid_string); if (STREQ(key, uuid_string)) { @@ -697,7 +703,8 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, def.name = volume->name; - md5_buffer(scsiLun->uuid, strlen(scsiLun->uuid), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0) + goto cleanup; virUUIDFormat(md5, uuid_string); diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 940e5d1f06..bf5093cf45 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -41,6 +41,7 @@ #include "esx_vi.h" #include "esx_vi_methods.h" #include "esx_util.h" +#include "vircrypto.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_ESX @@ -236,8 +237,8 @@ esxStoragePoolLookupByName(virConnectPtr conn, goto cleanup; } - md5_buffer(hostMount->mountInfo->path, - strlen(hostMount->mountInfo->path), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostMount->mountInfo->path, md5) < 0) + goto cleanup; pool = virGetStoragePool(conn, name, md5, &esxStorageBackendVMFS, NULL); @@ -289,8 +290,8 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, goto cleanup; } - md5_buffer(hostMount->mountInfo->path, - strlen(hostMount->mountInfo->path), md5); + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostMount->mountInfo->path, md5) < 0) + goto cleanup; if (memcmp(uuid, md5, VIR_UUID_BUFLEN) == 0) break; -- 2.16.1
Do not rely on gnulib's MD5_DIGEST_SIZE from md5.h. Include vircrypto.h and use VIR_CRYPTO_HASH_SIZE_MD5. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/esx/esx_network_driver.c | 9 ++++----- src/esx/esx_storage_backend_iscsi.c | 27 +++++++++++++-------------- src/esx/esx_storage_backend_vmfs.c | 11 +++++------ 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 7386efb0f5..b4f7f006d0 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -23,7 +23,6 @@ #include <config.h> -#include "md5.h" #include "internal.h" #include "viralloc.h" #include "viruuid.h" @@ -42,7 +41,7 @@ * The UUID of a network is the MD5 sum of its key. Therefore, verify that * UUID and MD5 sum match in size, because we rely on that. */ -verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN); +verify(VIR_CRYPTO_HASH_SIZE_MD5 == VIR_UUID_BUFLEN); static int @@ -142,7 +141,7 @@ esxNetworkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) esxPrivate *priv = conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; - unsigned char md5[MD5_DIGEST_SIZE]; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; if (esxVI_EnsureSession(priv->primary) < 0 || @@ -186,7 +185,7 @@ esxNetworkLookupByName(virConnectPtr conn, const char *name) virNetworkPtr network = NULL; esxPrivate *priv = conn->privateData; esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; - unsigned char md5[MD5_DIGEST_SIZE]; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ if (esxVI_EnsureSession(priv->primary) < 0 || esxVI_LookupHostVirtualSwitchByName(priv->primary, name, @@ -296,7 +295,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) esxVI_HostPortGroupSpec *hostPortGroupSpec = NULL; size_t i; - unsigned char md5[MD5_DIGEST_SIZE]; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ if (esxVI_EnsureSession(priv->primary) < 0) return NULL; diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 42b52214d1..fd0ace6fcb 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -27,7 +27,6 @@ #include <unistd.h> #include "internal.h" -#include "md5.h" #include "viralloc.h" #include "viruuid.h" #include "storage_conf.h" @@ -46,7 +45,7 @@ * The UUID of a storage pool is the MD5 sum of its mount path. Therefore, * verify that UUID and MD5 sum match in size, because we rely on that. */ -verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN); +verify(VIR_CRYPTO_HASH_SIZE_MD5 == VIR_UUID_BUFLEN); @@ -157,8 +156,8 @@ esxStoragePoolLookupByName(virConnectPtr conn, { esxPrivate *priv = conn->privateData; esxVI_HostInternetScsiHbaStaticTarget *target = NULL; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; virStoragePoolPtr pool = NULL; /* @@ -202,8 +201,8 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, esxPrivate *priv = conn->privateData; esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; esxVI_HostInternetScsiHbaStaticTarget *target; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; if (esxVI_LookupHostInternetScsiHba(priv->primary, &hostInternetScsiHba) < 0) { @@ -443,8 +442,8 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, esxPrivate *priv = pool->conn->privateData; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0) @@ -491,8 +490,8 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) esxVI_ScsiLun *scsiLun; esxVI_HostScsiDisk *hostScsiDisk = NULL; char *poolName = NULL; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0) @@ -538,8 +537,8 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char *key) char *poolName = NULL; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; /* key may be LUN device path */ @@ -671,8 +670,8 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume, esxVI_ScsiLun *scsiLun; esxVI_HostScsiDisk *hostScsiDisk = NULL; virStorageVolDef def; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; virCheckFlags(0, NULL); diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index bf5093cf45..630a6aa8c9 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -29,7 +29,6 @@ #include <unistd.h> #include "internal.h" -#include "md5.h" #include "viralloc.h" #include "virfile.h" #include "virlog.h" @@ -52,7 +51,7 @@ VIR_LOG_INIT("esx.esx_storage_backend_vmfs"); * The UUID of a storage pool is the MD5 sum of its mount path. Therefore, * verify that UUID and MD5 sum match in size, because we rely on that. */ -verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN); +verify(VIR_CRYPTO_HASH_SIZE_MD5 == VIR_UUID_BUFLEN); @@ -205,8 +204,8 @@ esxStoragePoolLookupByName(virConnectPtr conn, esxPrivate *priv = conn->privateData; esxVI_ObjectContent *datastore = NULL; esxVI_DatastoreHostMount *hostMount = NULL; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; virStoragePoolPtr pool = NULL; if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, &datastore, @@ -260,8 +259,8 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, esxVI_ObjectContent *datastoreList = NULL; esxVI_ObjectContent *datastore = NULL; esxVI_DatastoreHostMount *hostMount = NULL; - /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[MD5_DIGEST_SIZE]; + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; char *name = NULL; virStoragePoolPtr pool = NULL; -- 2.16.1
Ditch the use of gnulib's digest functions in favor of GnuTLS, which might be more likely to get FIPS-certified. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- bootstrap.conf | 2 -- src/util/vircrypto.c | 32 +++++++++++++++++++++++--------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 9559922fce..c4ef54ff13 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -37,8 +37,6 @@ connect configmake count-leading-zeros count-one-bits -crypto/md5 -crypto/sha256 dirname-lgpl environ execinfo diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 1a2dcc28b7..62a027353b 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -26,8 +26,6 @@ #include "viralloc.h" #include "virrandom.h" -#include "md5.h" -#include "sha256.h" #ifdef WITH_GNUTLS # include <gnutls/gnutls.h> # if HAVE_GNUTLS_CRYPTO_H @@ -41,15 +39,18 @@ VIR_LOG_INIT("util.crypto"); static const char hex[] = "0123456789abcdef"; +#define VIR_CRYPTO_LARGEST_DIGEST_SIZE VIR_CRYPTO_HASH_SIZE_SHA256 + +#if WITH_GNUTLS + struct virHashInfo { - void *(*func)(const char *buf, size_t len, void *res); + gnutls_digest_algorithm_t algorithm; size_t hashlen; } hashinfo[] = { - { md5_buffer, MD5_DIGEST_SIZE }, - { sha256_buffer, SHA256_DIGEST_SIZE }, + { GNUTLS_DIG_MD5, VIR_CRYPTO_HASH_SIZE_MD5 }, + { GNUTLS_DIG_SHA256, VIR_CRYPTO_HASH_SIZE_SHA256 }, }; -#define VIR_CRYPTO_LARGEST_DIGEST_SIZE SHA256_DIGEST_SIZE verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); @@ -58,20 +59,33 @@ virCryptoHashBuf(virCryptoHash hash, const char *input, unsigned char *output) { + int rc; if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; } - if (!(hashinfo[hash].func(input, strlen(input), output))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to compute hash of data")); + rc = gnutls_hash_fast(hashinfo[hash].algorithm, input, strlen(input), output); + if (rc < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to compute hash of data: %s"), + gnutls_strerror(rc)); return -1; } return 0; } +#else +int +virCryptoHashBuf(virCryptoHash hash, + const char *input ATTRIBUTE_UNUSED, + unsigned char *output ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_INVALID_ARG, + _("algorithm=%d is not supported"), hash); +} +#endif int virCryptoHashString(virCryptoHash hash, -- 2.16.1
On 05/11/2018 05:50 PM, Ján Tomko wrote:
Ján Tomko (5): vircrypto: provide constants for hash sizes Introduce virCryptoHashBuf esx: use virCryptoHashBuf esx: Use VIR_CRYPTO_HASH_SIZE_MD5 vircrypto: Rely on GnuTLS for hash functions
bootstrap.conf | 2 -- src/esx/esx_network_driver.c | 22 ++++++++------ src/esx/esx_storage_backend_iscsi.c | 46 +++++++++++++++++------------- src/esx/esx_storage_backend_vmfs.c | 20 ++++++------- src/util/vircrypto.c | 57 ++++++++++++++++++++++++++----------- src/util/vircrypto.h | 10 +++++++ 6 files changed, 100 insertions(+), 57 deletions(-)
ACK series, but please see my comment in 2/5 before pushing. Michal
participants (3)
-
Ján Tomko -
Michal Privoznik -
Stefan Berger