From: Oliver Kurth Date: Tue, 24 Apr 2018 00:08:16 +0000 (-0700) Subject: lib/hashMap/hashMap.c: X-Git-Tag: stable-10.3.0~49 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b814f9b4dcbf54893078a4ba187d8d0cbf23e31;p=thirdparty%2Fopen-vm-tools.git lib/hashMap/hashMap.c: Add new HashMap_Get() which uses a constant time memory comparison function. lib/misc/utilMem.c: Constant time memory and string comparison functions. --- diff --git a/open-vm-tools/lib/hashMap/hashMap.c b/open-vm-tools/lib/hashMap/hashMap.c index 0c13d9556..59560407d 100644 --- a/open-vm-tools/lib/hashMap/hashMap.c +++ b/open-vm-tools/lib/hashMap/hashMap.c @@ -1,5 +1,5 @@ /********************************************************* - * Copyright (C) 2009-2016 VMware, Inc. All rights reserved. + * Copyright (C) 2009-2018 VMware, Inc. All rights reserved. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -38,6 +38,7 @@ #include "hashMap.h" #include "clamped.h" +#include "util.h" #ifdef VMX86_SERVER #include "aioMgr.h" #include "iovector.h" @@ -139,8 +140,9 @@ static Bool InitMap(struct HashMap *map, uint32 numEntries, uint32 alpha, static void CalculateEntrySize(struct HashMap *map); static void GetEntry(struct HashMap *map, uint32 index, HashMapEntryHeader **header, void **key, void **data); static uint32 ComputeHash(struct HashMap *map, const void *key); -static Bool LookupKey(struct HashMap* map, const void *key, HashMapEntryHeader **header, void **data, uint32 *freeIndex); +static Bool LookupKey(struct HashMap* map, const void *key, Bool constTimeLookup, HashMapEntryHeader **header, void **data, uint32 *freeIndex); static Bool CompareKeys(struct HashMap *map, const void *key, const void *compare); +static Bool ConstTimeCompareKeys(struct HashMap *map, const void *key, const void *compare); static Bool NeedsResize(struct HashMap *map); static void Resize(struct HashMap *map); INLINE void EnsureSanity(HashMap *map); @@ -370,13 +372,13 @@ HashMap_Put(struct HashMap *map, // IN HashMapEntryHeader *header; void *tableData; - if (!LookupKey(map, key, &header, &tableData, &freeIndex)) { + if (!LookupKey(map, key, FALSE, &header, &tableData, &freeIndex)) { uint32 hash = ComputeHash(map, key); void *tableKey; if (NeedsResize(map)) { Resize(map); - if (LookupKey(map, key, &header, &tableData, &freeIndex)) { + if (LookupKey(map, key, FALSE, &header, &tableData, &freeIndex)) { /* * Somehow our key appeared after resizing the table. */ @@ -432,7 +434,44 @@ HashMap_Get(struct HashMap *map, // IN uint32 freeIndex; HashMapEntryHeader *header; - if (LookupKey(map, key, &header, &data, &freeIndex)) { + if (LookupKey(map, key, FALSE, &header, &data, &freeIndex)) { + return data; + } + + return NULL; +} + + +/* + * ---------------------------------------------------------------------------- + * + * HashMap_ConstTimeGet -- + * + * Timing attack safe version of HashMap_Get. This will call LookupKey with + * the constTime flag set to 1 which will do the memory comparison with + * Util_ConstTimeMemDiff instead of memcmp. Note that there is a bit of a + * time penalty associated with this so only use this if you are looking up + * sensitive information. + * + * Results: + * Returns a pointer to the data that was previously stored by HashMap_Put or + * NULL if the key wasn't found. + * + * Side Effects: + * None. + * + * ---------------------------------------------------------------------------- + */ + +void * +HashMap_ConstTimeGet(struct HashMap *map, // IN + const void *key) // IN +{ + void *data; + uint32 freeIndex; + HashMapEntryHeader *header; + + if (LookupKey(map, key, TRUE, &header, &data, &freeIndex)) { return data; } @@ -499,7 +538,7 @@ HashMap_Remove(struct HashMap *map, // IN HashMapEntryHeader *header; void *tableData; - if (!LookupKey(map, key, &header, &tableData, &freeIndex)) { + if (!LookupKey(map, key, FALSE, &header, &tableData, &freeIndex)) { return FALSE; } @@ -583,6 +622,9 @@ CalculateEntrySize(struct HashMap *map) // IN * Use linear probing to find a free space in the table or the data that * we're interested in. * + * Call this function with constTimeLookup = TRUE to use a timing attack + * safe version of memcmp while comparing keys. + * * Returns: * - TRUE if the key was found in the table, FALSE otherwise. * - Returns the entry header on header, data pointer on data and the first @@ -599,6 +641,7 @@ CalculateEntrySize(struct HashMap *map) // IN Bool LookupKey(struct HashMap* map, // IN const void *key, // IN + Bool constTimeLookup, // IN HashMapEntryHeader **header, // OUT void **data, // OUT uint32 *freeIndex) // OUT @@ -639,7 +682,16 @@ LookupKey(struct HashMap* map, // IN break; case HashMapState_FILLED: if ((*header)->hash == hash) { - found = CompareKeys(map, key, tableKey); + /* + * There is some performance penalty to doing a constant time + * comparison, so only use that version if it's been explicitly + * asked for. + */ + if (constTimeLookup) { + found = ConstTimeCompareKeys(map, key, tableKey); + } else { + found = CompareKeys(map, key, tableKey); + } if (found) { done = TRUE; } @@ -765,6 +817,34 @@ CompareKeys(struct HashMap *map, // IN } +/* + * ---------------------------------------------------------------------------- + * + * ConstTimeCompareKeys -- + * + * Timing attack safe version of CompareKeys. Instead of calling memcmp, + * which will return after the first character that doesn't match, this + * calls a constant time memory comparison function. + * + * Results: + * Returns TRUE if the two keys are binary equal over the length specified + * in the map. FALSE if the two keys are different. + * + * Side Effects: + * None. + * + * ---------------------------------------------------------------------------- + */ + +Bool +ConstTimeCompareKeys(struct HashMap *map, // IN + const void *key, // IN + const void *compare) // IN +{ + return Util_ConstTimeMemDiff(key, compare, map->keySize) == 0; +} + + /* * ---------------------------------------------------------------------------- * @@ -871,7 +951,7 @@ Resize(struct HashMap *map) // IN case HashMapState_DELETED: continue; case HashMapState_FILLED: - if (!LookupKey(map, oldKey, &newHeader, &newData, &freeIndex)) { + if (!LookupKey(map, oldKey, FALSE, &newHeader, &newData, &freeIndex)) { GetEntry(map, freeIndex, &newHeader, &newKey, &newData); newHeader->hash = oldHeader->hash; diff --git a/open-vm-tools/lib/include/hashMap.h b/open-vm-tools/lib/include/hashMap.h index 00af41b0c..bb2bc4b59 100644 --- a/open-vm-tools/lib/include/hashMap.h +++ b/open-vm-tools/lib/include/hashMap.h @@ -1,5 +1,5 @@ /********************************************************* - * Copyright (C) 2009-2017 VMware, Inc. All rights reserved. + * Copyright (C) 2009-2018 VMware, Inc. All rights reserved. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -71,6 +71,7 @@ HashMap *HashMap_AllocMapAlpha(uint32 numEntries, uint32 alpha, size_t keySize, void HashMap_DestroyMap(HashMap *map); Bool HashMap_Put(HashMap *map, const void *key, const void *data); void *HashMap_Get(HashMap *map, const void *key); +void *HashMap_ConstTimeGet(struct HashMap *map, const void *key); void HashMap_Clear(HashMap *map); Bool HashMap_Remove(HashMap *map, const void *key); uint32 HashMap_Count(HashMap *map); diff --git a/open-vm-tools/lib/include/util.h b/open-vm-tools/lib/include/util.h index df0735e91..3e2d44130 100644 --- a/open-vm-tools/lib/include/util.h +++ b/open-vm-tools/lib/include/util.h @@ -1,5 +1,5 @@ /********************************************************* - * Copyright (C) 1998-2017 VMware, Inc. All rights reserved. + * Copyright (C) 1998-2018 VMware, Inc. All rights reserved. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -303,6 +303,9 @@ char *UtilSafeStrndup1(const char *s, size_t n, void *Util_Memdup(const void *src, size_t size); void *Util_Memcpy(void *dest, const void *src, size_t count); +Bool Util_ConstTimeMemDiff(const void *secret, const void *guess, size_t len); +Bool Util_ConstTimeStrDiff(const char *secret, const char *guess); + #ifndef VMKBOOT /* diff --git a/open-vm-tools/lib/misc/utilMem.c b/open-vm-tools/lib/misc/utilMem.c index 079249ae5..6a288a0d3 100644 --- a/open-vm-tools/lib/misc/utilMem.c +++ b/open-vm-tools/lib/misc/utilMem.c @@ -1,5 +1,5 @@ /********************************************************* - * Copyright (C) 2009-2017 VMware, Inc. All rights reserved. + * Copyright (C) 2009-2018 VMware, Inc. All rights reserved. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -40,9 +40,12 @@ #endif static NORETURN void UtilAllocationFailure0(void); -static NORETURN void UtilAllocationFailure1(int bugNumber, +static NORETURN void UtilAllocationFailure1(int bugNumber, const char *file, int lineno); +Bool UtilConstTimeMemDiff(const void *secret, const void *guess, size_t len, size_t *diffCount); +Bool UtilConstTimeStrDiff(const char *secret, const char *guess, size_t *diffCount); + static void UtilAllocationFailure0(void) @@ -483,3 +486,169 @@ Util_Memcpy(void *dest, // OUT: memcpy(dest, src, count); return dest; } + + +/* + *----------------------------------------------------------------------------- + * + * UtilConstTimeMemDiff -- + * + * The implementation of a constant time memory comparison. Unlike + * memcmp, this function does not return early if it finds a mismatch. + * It always examines the entire 'secret' and 'guess' buffers, so that + * the time spent in this function is constant for buffers of the same + * given 'len'. (We don't attempt to make the time invariant for + * different buffer lengths.) + * + * The reason why this function is externally visible (not static) + * and has a 'diffCount' argument is to try to prevent aggressive + * compiler optimization levels from short-circuiting the inner loop. + * The possibility of a call from outside this module with a non-NULL + * diffCount pointer prevents that optimization. If we didn't have + * to worry about that then we wouldn't need this function; we could + * have put the implementation directly into Util_ConstTimeMemDiff. + * + * Results: + * Returns true if the buffers differ, false if they are identical. + * If diffCount is non-NULL, sets *diffCount to the total number of + * differences between the buffers. + * + * Side Effects: + * None. + * + *----------------------------------------------------------------------------- + */ + +Bool +UtilConstTimeMemDiff(const void *secret, // IN + const void *guess, // IN + size_t len, // IN + size_t *diffCount) // OUT: optional +{ + const char *secretChar = secret; + const char *guessChar = guess; + + size_t numDiffs = 0; + + while (len--) { + numDiffs += !!(*secretChar ^ *guessChar); + ++secretChar; + ++guessChar; + } + + if (diffCount != NULL) { + *diffCount = numDiffs; + } + return numDiffs != 0; +} + + +/* + *----------------------------------------------------------------------------- + * + * Util_ConstTimeMemDiff -- + * + * Performs a constant time memory comparison. + * + * The return values are chosen to make this as close as possible to + * a drop-in replacement for memcmp, so we return false (0) if the + * buffers match (ie there are zero differences) and true (1) if the + * buffers differ. + * + * Results: + * Returns zero if the buffers are identical, 1 if they differ. + * + * Side Effects: + * None. + * + *----------------------------------------------------------------------------- + */ + +Bool +Util_ConstTimeMemDiff(const void *secret, // IN + const void *guess, // IN + size_t len) // IN +{ + return UtilConstTimeMemDiff(secret, guess, len, NULL); +} + + +/* + *----------------------------------------------------------------------------- + * + * UtilConstTimeStrDiff -- + * + * The implementation of a constant time string comparison. Unlike + * strcmp, this function does not return early if it finds a mismatch. + * It always compares the entire 'secret' string against however much + * of the 'guess' string is required for that comparison, so that the + * time spent in this function is constant for secrets of the same + * length. (We don't attempt to make the time invariant for secrets + * of different lengths.) + * + * The reason why this function is externally visible (not static) + * and has a 'diffCount' argument is to try to prevent aggressive + * compiler optimization levels from short-circuiting the inner + * loop. The possibility of a call from outside this module with a + * non-NULL diffCount pointer prevents that optimization. If we + * didn't have to worry about that then we wouldn't need this + * function; we could have put the implementation directly into + * Util_ConstTimeStrDiff. + * + * Results: + * Returns true if the strings differ, false if they are identical. + * If diffCount is non-NULL, sets *diffCount to the total number of + * differences between the strings. + * + * Side Effects: + * None. + * + *----------------------------------------------------------------------------- + */ + +Bool +UtilConstTimeStrDiff(const char *secret, // IN + const char *guess, // IN + size_t *diffCount) // OUT: optional +{ + size_t numDiffs = 0; + + do { + numDiffs += !!(*secret ^ *guess); + guess += !!(*guess); + } while (*secret++); + + if (diffCount != NULL) { + *diffCount = numDiffs; + } + return numDiffs != 0; +} + + +/* + *----------------------------------------------------------------------------- + * + * Util_ConstTimeStrDiff -- + * + * The implementation of a constant time string comparison. + * + * The return values are chosen to make this as close as possible + * to a drop-in replacement for strcmp, so we return 0 if + * the buffers match (ie there are zero differences) and 1 + * if the buffers differ. + * + * Results: + * Returns zero if the strings are identical, 1 if they differ. + * + * Side Effects: + * None. + * + *----------------------------------------------------------------------------- + */ + +Bool +Util_ConstTimeStrDiff(const char *secret, // IN + const char *guess) // IN +{ + return UtilConstTimeStrDiff(secret, guess, NULL); +}