From e584076acaa6e44bacfd8263609bf0126fcb3700 Mon Sep 17 00:00:00 2001 From: Bob Mottram Date: Thu, 3 Sep 2020 19:48:32 +0100 Subject: [PATCH] Unit test for constant time string check --- auth.py | 41 +++++++++++++++++++++++------------------ tests.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/auth.py b/auth.py index 317ca0bb..021a17b6 100644 --- a/auth.py +++ b/auth.py @@ -34,6 +34,28 @@ def getPasswordHash(salt: str, providedPassword: str) -> str: return binascii.hexlify(pwdhash).decode('ascii') +def constantTimeStringCheck(string1: str, string2: str) -> bool: + """Compares two string and returns if they are the same + using a constant amount of time + See https://sqreen.github.io/DevelopersSecurityBestPractices/ + timing-attack/python + """ + # strings must be of equal length + if len(string1) != len(string2): + return False + ctr = 0 + matched = True + for ch in string1: + if ch != string2[ctr]: + matched = False + else: + # this is to make the timing more even + # and not provide clues + matched = matched + ctr += 1 + return matched + + def verifyPassword(storedPassword: str, providedPassword: str) -> bool: """Verify a stored password against one provided by user """ @@ -44,24 +66,7 @@ def verifyPassword(storedPassword: str, providedPassword: str) -> bool: salt = storedPassword[:64] storedPassword = storedPassword[64:] pwHash = getPasswordHash(salt, providedPassword) - # check that hashes are of equal length - if len(pwHash) != len(storedPassword): - return False - # Compare all of the characters before returning true or false. - # Hence the match should take a constant amount of time. - # See https://sqreen.github.io/DevelopersSecurityBestPractices/ - # timing-attack/python - ctr = 0 - matched = True - for ch in pwHash: - if ch != storedPassword[ctr]: - matched = False - else: - # this is to make the timing more even - # and not provide clues - matched = matched - ctr += 1 - return matched + return constantTimeStringCheck(pwHash, storedPassword) def createBasicAuthHeader(nickname: str, password: str) -> str: diff --git a/tests.py b/tests.py index 127d0c79..ccbf216c 100644 --- a/tests.py +++ b/tests.py @@ -54,6 +54,7 @@ from person import setBio from skills import setSkillLevel from roles import setRole from roles import outboxDelegate +from auth import constantTimeStringCheck from auth import createBasicAuthHeader from auth import authorizeBasic from auth import storeBasicCredentials @@ -2085,8 +2086,36 @@ def testTranslations(): assert langJson.get(englishStr) +def testConstantTimeStringCheck(): + print('testConstantTimeStringCheck') + assert constantTimeStringCheck('testing', 'testing') + assert not constantTimeStringCheck('testing', '1234') + assert not constantTimeStringCheck('testing', '1234567') + + itterations = 256 + + start = time.time() + for timingTest in range(itterations): + constantTimeStringCheck('nnjfbefefbsnjsdnvbcueftqfeuqfbqefnjeniwufgy', + 'nnjfbefefbsnjsdnvbcueftqfeuqfbqefnjeniwufgy') + end = time.time() + avTime1 = ((end - start) * 1000000 / itterations) + + # change characters and observe timing difference + start = time.time() + for timingTest in range(itterations): + constantTimeStringCheck('nnjfbefefbsnjsdnvbcueftqfeuqfbqefnjeniwufgy', + 'nnjfbefefbsnjsdnvbcueftqfeuqfbqeznjeniwufgy') + end = time.time() + avTime2 = ((end - start) * 1000000 / itterations) + timeDiffMicroseconds = abs(avTime2 - avTime1) + # time difference should be less than 10uS + assert timeDiffMicroseconds < 10 + + def runAllTests(): print('Running tests...') + testConstantTimeStringCheck() testTranslations() testValidContentWarning() testRemoveIdEnding()