From 39e88cfc3b7b3801a40197c429bbb1e34032ea1f Mon Sep 17 00:00:00 2001 From: PhilippMDoerner Date: Sat, 9 Sep 2023 20:07:25 +0200 Subject: [PATCH] #6 api refactor (#7) * #6 Make API more secure It shouldn't be possible for passwords to accidentally leak. Thus a new type was introduced: `Password` Now all `hashPassword` procs only accept that type. They also now return `seq[byte]` which is a more correct representation of a hash. In turn, encodeHash accepts seq[byte] and encodes it itself. For clarity purposes the `Hash` type was introduced for this. * #6 update docs to reflect API change * #6 add missing API changes * Attempt test-workflow upgrade * #6 attempt to fix test * #6 improve test accuracy by checking if cli execution even worked * Add test dependency * Remove testing for macos and windows I can't test for them, I don't know their setup * Improve test failure message --- .github/workflows/tests.yml | 33 +++++++++++++++++++++---- README.md | 2 +- src/nimword.nim | 7 ++++-- src/nimword/argon2.nim | 36 +++++++++++++++------------- src/nimword/pbkdf2_sha256.nim | 23 ++++++++++-------- src/nimword/pbkdf2_sha512.nim | 21 +++++++++------- src/nimword/private/pbkdf2_utils.nim | 18 ++++++++------ src/nimword/private/types.nim | 5 ++++ tests/t_argon2.nim | 13 ++++++---- tests/t_pbkdf2_sha256.nim | 2 +- tests/t_pbkdf2_sha512.nim | 2 +- 11 files changed, 105 insertions(+), 57 deletions(-) create mode 100644 src/nimword/private/types.nim diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e4f660e..05893e8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,12 +12,35 @@ on: jobs: Tests: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - name: tests - run: docker-compose run tests + strategy: + matrix: + nimversion: + - binary:2.0.0 + - binary:1.6.10 + os: + - ubuntu-latest + #- windows-latest + #- macOS-latest + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + + name: Nim ${{ matrix.nimversion }} - ${{ matrix.os }} + + steps: + - uses: actions/checkout@v1 + - uses: iffy/install-nim@v4 + with: + version: ${{ matrix.nimversion }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Test + run: | + sudo apt-get update + sudo apt-get install -y openssl libsodium-dev xz-utils argon2 + nimble install -y + nimble test + Docs: runs-on: ubuntu-latest steps: diff --git a/README.md b/README.md index 9a141f6..e5ddf77 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ These core procs are also available in the individual modules for each algorithm The individual algorithm-modules further provide 2 procs in case some customization is needed: - `hashPassword`: - Proc to create base 64 encoded hashes like `hashEncodePassword`, but returns the hash directly from there without turning it into a specific format like `hashEncodePassword` does. + Proc to create unencoded raw hashes like `hashEncodePassword`, but returns the hash-bytes directly from there without turning it into a specific format like `hashEncodePassword` does. - `encodeHash`: Proc to generate strings of the format that `hashEncodePassword` outputs, but without doing any of the hashing itself. The output can be used with `isValidPassword`. diff --git a/src/nimword.nim b/src/nimword.nim index 9fdef98..480b453 100644 --- a/src/nimword.nim +++ b/src/nimword.nim @@ -3,6 +3,9 @@ import std/[strutils, strformat] export argon2.SodiumError export pbkdf2_sha256.Pbkdf2Error +export Password +export toPassword +export Hash type NimwordHashingAlgorithm* = enum ## The number of different hashing algorithms nimword supports @@ -15,7 +18,7 @@ type NimwordHashingAlgorithm* = enum type UnknownAlgorithmError = object of ValueError proc hashEncodePassword*( - password: string, + password: Password, iterations: int, algorithm: NimwordHashingAlgorithm = nhaDefault ): string = @@ -45,7 +48,7 @@ proc hashEncodePassword*( proc isValidPassword*( - password: string, + password: Password, encodedHash: string ): bool {.raises: {UnknownAlgorithmError, ValueError, Pbkdf2Error, SodiumError, Exception}.} = ## Verifies that a given plain-text password can be used to generate diff --git a/src/nimword/argon2.nim b/src/nimword/argon2.nim index 84fe823..f7ef790 100644 --- a/src/nimword/argon2.nim +++ b/src/nimword/argon2.nim @@ -1,11 +1,15 @@ import std/[strformat, base64, strutils] import libsodium/[sodium, sodium_sizes] +import ./private/types export sodium.PasswordHashingAlgorithm export sodium.SodiumError +export types.Password +export types.toPassword +export types.Hash proc encodeHash*( - hash: string, + hash: Hash, salt: seq[byte], iterations: int, algorithm: PasswordHashingAlgorithm; @@ -14,8 +18,8 @@ proc encodeHash*( ## Encodes all relevant data for a password hash in a string. ## ## The returned string can be used with `isValidPassword<#isValidPassword%2Cstring%2Cstring>`_ . + ## Hash is a seq[byte] like salt and gets turned into a base64 encoded string with all padding suffix characters of "=" removed". ## - ## Hash is assumed to be a base64 encoded strings. ## Salt gets turned into a base64 encoded string with all padding suffix character of "=" removed. ## memoryLimitKibiBytes is the number of KiB used for the hashing process. ## algorithm is either "argon2id" or "argon2i". @@ -24,6 +28,8 @@ proc encodeHash*( ## $$v=19$m=,t=,p=1$$ var encodedSalt = salt.encode() encodedSalt.removeSuffix('=') + var encodedHash = hash.encode() + encodedHash.removeSuffix('=') let algorithmStr = case algorithm: of phaDefault, phaArgon2id13: @@ -31,21 +37,20 @@ proc encodeHash*( of phaArgon2i13: "argon2i" - result = fmt"${algorithmStr}$v=19$m={memoryLimitKibiBytes},t={iterations},p=1${encodedSalt}${hash}" + result = fmt"${algorithmStr}$v=19$m={memoryLimitKibiBytes},t={iterations},p=1${encodedSalt}${encodedHash}" proc hashPassword*( - password: string, + password: Password, salt: seq[byte], iterations: int = crypto_pwhash_opslimit_moderate().int, hashLength: int = 32, algorithm: PasswordHashingAlgorithm = phaDefault, memoryLimitKibiBytes: int = (crypto_pwhash_memlimit_moderate().int / 1024).int -): string {.raises: {SodiumError, ValueError}.} = +): Hash {.raises: {SodiumError, ValueError}.} = ## Hashes the given password using the argon2 algorithm from libsodium. - ## Returns the hash as a base64 encoded string with any padding "=" suffix - ## character removed. + ## Returns the hash as seq[byte] ## ## Salt must be exactly 16 bytes long. ## @@ -54,7 +59,7 @@ proc hashPassword*( ## `libsodium-documentation`_ ## for the `opslimit` value. ## - ## hashLength is the number of characters that the hash should be long. + ## hashLength is the number of bytes that the hash should be long. ## For guidance on how to choose a number for this value, consult the ## `libsodium-documentation`_ ## for the `outlen` value. @@ -72,19 +77,18 @@ proc hashPassword*( ## Raises SodiumError for invalid values for memoryLimit or iterations. let memoryLimitBytes = memoryLimitKibiBytes * 1024 - let hashBytes = crypto_pwhash( - password, + let hash: Hash = crypto_pwhash( + password.string, salt, hashLength, algorithm, iterations.csize_t, memoryLimitBytes.csize_t ) - result = hashBytes.encode() - result.removeSuffix("=") + return hash proc hashEncodePassword*( - password: string, + password: Password, iterations: int = crypto_pwhash_opslimit_moderate().int, algorithm: PasswordHashingAlgorithm = phaDefault, memoryLimitKibiBytes: int = (crypto_pwhash_memlimit_moderate().int / 1024).int @@ -104,13 +108,13 @@ proc hashEncodePassword*( ## Raises SodiumError for invalid values for memoryLimit or iterations. let memoryLimitBytes: int = memoryLimitKibiBytes * 1024 result = crypto_pwhash_str( - password, + password.string, algorithm, iterations.csize_t, memoryLimitBytes.csize_t ) -proc isValidPassword*(password: string, encodedHash: string): bool {.raises: SodiumError.} = +proc isValidPassword*(password: Password, encodedHash: string): bool {.raises: SodiumError.} = ## Verifies that a given plain-text password can be used to generate ## the hash contained in `encodedHash` with the parameters provided in `encodedHash`. ## @@ -118,4 +122,4 @@ proc isValidPassword*(password: string, encodedHash: string): bool {.raises: Sod ## and `hashEncodePassword<#hashEncodePassword%2Cstring%2Cint>`_ generate. ## ## Raises SodiumError if an error happens during the process. - result = crypto_pwhash_str_verify(encodedHash, password) \ No newline at end of file + result = crypto_pwhash_str_verify(encodedHash, password.string) \ No newline at end of file diff --git a/src/nimword/pbkdf2_sha256.nim b/src/nimword/pbkdf2_sha256.nim index 5ba5911..081244d 100644 --- a/src/nimword/pbkdf2_sha256.nim +++ b/src/nimword/pbkdf2_sha256.nim @@ -3,13 +3,16 @@ from std/openssl import DLLSSLName, EVP_MD, DLLUtilName import ./private/[base64_utils, pbkdf2_utils] export pbkdf2_utils.Pbkdf2Error +export Password +export toPassword +export Hash # Imports that sometimes break when importing from std/openssl - START proc EVP_sha256_fixed(): EVP_MD {.cdecl, dynlib: DLLUtilName, importc: "EVP_sha256".} # Imports that sometimes break when importing from std/openssl - END proc encodeHash*( - hash: string, + hash: Hash, salt: seq[byte], iterations: SomeInteger, ): string = @@ -22,11 +25,11 @@ proc encodeHash*( result = encodeHash(hash, salt, iterations, Pbkdf2Algorithm.pbkdf2_sha256) -proc hashPassword*(password: string, salt: seq[byte], iterations: int): string {.gcsafe.} = +proc hashPassword*(password: Password, salt: seq[byte], iterations: int): Hash {.gcsafe.} = ## Hashes the given plain-text password with the PBKDF2 using an HMAC ## with the SHA256 hashing algorithm from openssl. ## - ## Returns the hash as string. + ## Returns the hash as Hash type. ## ## Salt can be of any size, but is recommended to be at least 16 bytes long. ## @@ -39,11 +42,11 @@ proc hashPassword*(password: string, salt: seq[byte], iterations: int): string { let digestFunction: EVP_MD = EVP_sha256_fixed() result = hashPbkdf2(password, salt, iterations, digestFunction) -proc hashEncodePassword*(password: string, iterations: int): string {.gcsafe.} = +proc hashEncodePassword*(password: Password, iterations: int): string {.gcsafe.} = ## Hashes and encodes the given password with the PBKDF2 using an HMAC ## with the SHA256 hashing algorithm from openssl. ## - ## Returns the hash as part of a larger string containing hash, iterations and salt. + ## Returns the hash in an encoded form as part of a larger string containing it, iterations and salt. ## For information about the pattern see `encodeHash<#encodeHash%2Cstring%2Cseq[byte]%2CSomeInteger>`_ ## ## The return value can be used with `isValidPassword<#isValidPassword%2Cstring%2Cstring>`_ . @@ -53,10 +56,10 @@ proc hashEncodePassword*(password: string, iterations: int): string {.gcsafe.} = ## ## The salt used for the hash is randomly generated during the process. let salt = urandom(16) - let hash = hashPassword(password, salt, iterations) + let hash: Hash = hashPassword(password, salt, iterations) result = hash.encodeHash(salt, iterations) -proc isValidPassword*(password: string, encodedHash: string): bool {.raises: {Pbkdf2Error, Exception} .} = +proc isValidPassword*(password: Password, encodedHash: string): bool {.raises: {Pbkdf2Error, Exception} .} = ## Verifies that a given plain-text password can be used to generate ## the hash contained in `encodedHash` with the parameters provided in `encodedHash`. ## @@ -70,14 +73,14 @@ proc isValidPassword*(password: string, encodedHash: string): bool {.raises: {Pb let iterations: int = parseInt(hashPieces[1]) let salt: seq[byte] = hashPieces[2].decode() - let passwordHash: string = password.hashPassword(salt, iterations) + let passwordHash: Hash = password.hashPassword(salt, iterations) - let hash: string = hashPieces[3] + let hash: Hash = hashPieces[3].decode() result = passwordHash == hash except CatchableError as e: raise newException( Pbkdf2Error, - fmt"Could not calculate password hash from the data encoded in '{encodedHash}'", + fmt"Could not calculate password hash from the encoded Hash string", e ) \ No newline at end of file diff --git a/src/nimword/pbkdf2_sha512.nim b/src/nimword/pbkdf2_sha512.nim index 6f39d11..edc5f11 100644 --- a/src/nimword/pbkdf2_sha512.nim +++ b/src/nimword/pbkdf2_sha512.nim @@ -3,13 +3,16 @@ from std/openssl import DLLSSLName, EVP_MD, DLLUtilName import ./private/[base64_utils, pbkdf2_utils] export pbkdf2_utils.Pbkdf2Error +export Password +export toPassword +export Hash # Imports that sometimes break when importing from std/openssl - START proc EVP_sha512_fixed(): EVP_MD {.cdecl, dynlib: DLLUtilName, importc: "EVP_sha512".} # Imports that sometimes break when importing from std/openssl - END proc encodeHash*( - hash: string, + hash: Hash, salt: seq[byte], iterations: SomeInteger, ): string = @@ -22,11 +25,11 @@ proc encodeHash*( result = encodeHash(hash, salt, iterations, Pbkdf2Algorithm.pbkdf2_sha512) -proc hashPassword*(password: string, salt: seq[byte], iterations: int): string {.gcsafe.} = +proc hashPassword*(password: Password, salt: seq[byte], iterations: int): Hash {.gcsafe.} = ## Hashes the given plain-text password with the PBKDF2 using an HMAC ## with the SHA512 hashing algorithm from openssl. ## - ## Returns the hash as string. + ## Returns the hash as Hash type. ## ## Salt can be of any size, but is recommended to be at least 16 bytes long. ## @@ -40,11 +43,11 @@ proc hashPassword*(password: string, salt: seq[byte], iterations: int): string { result = hashPbkdf2(password, salt, iterations, digestFunction) -proc hashEncodePassword*(password: string, iterations: int): string {.gcsafe.} = +proc hashEncodePassword*(password: Password, iterations: int): string {.gcsafe.} = ## Hashes and encodes the given password with the PBKDF2 using an HMAC ## with the SHA256 hashing algorithm from openssl. ## - ## Returns the hash as part of a larger string containing hash, iterations and salt. + ## Returns the hash in an encoded form as part of a larger string containing hash, iterations and salt. ## For information about the pattern see `encodeHash<#encodeHash%2Cstring%2Cseq[byte]%2CSomeInteger>`_ ## ## The return value can be used with `isValidPassword<#isValidPassword%2Cstring%2Cstring>`_ . @@ -58,7 +61,7 @@ proc hashEncodePassword*(password: string, iterations: int): string {.gcsafe.} = let hash = hashPassword(password, salt, iterations) result = hash.encodeHash(salt, iterations, Pbkdf2Algorithm.pbkdf2_sha512) -proc isValidPassword*(password: string, encodedHash: string): bool {.raises: {Pbkdf2Error, Exception} .}= +proc isValidPassword*(password: Password, encodedHash: string): bool {.raises: {Pbkdf2Error, Exception} .}= ## Verifies that a given plain-text password can be used to generate ## the hash contained in `encodedHash` with the parameters provided in `encodedHash`. ## @@ -72,14 +75,14 @@ proc isValidPassword*(password: string, encodedHash: string): bool {.raises: {Pb let iterations: int = parseInt(hashPieces[1]) let salt: seq[byte] = hashPieces[2].decode() - let passwordHash: string = password.hashPassword(salt, iterations) + let passwordHash: Hash = password.hashPassword(salt, iterations) - let hash: string = hashPieces[3] + let hash: Hash = hashPieces[3].decode() result = passwordHash == hash except CatchableError as e: raise newException( Pbkdf2Error, - fmt"Could not calculate password hash from the data encoded in '{encodedHash}'", + fmt"Could not calculate password hash from the encodedHash", e ) \ No newline at end of file diff --git a/src/nimword/private/pbkdf2_utils.nim b/src/nimword/private/pbkdf2_utils.nim index d364ca1..c9c5793 100644 --- a/src/nimword/private/pbkdf2_utils.nim +++ b/src/nimword/private/pbkdf2_utils.nim @@ -1,6 +1,9 @@ from std/openssl import DLLSSLName, EVP_MD, DLLUtilName, getOpenSSLVersion import std/[strformat, strutils, dynlib] import ./base64_utils +import ./types + +export types type Pbkdf2Error* = object of ValueError @@ -46,22 +49,24 @@ proc `$`(s: seq[byte]): string = result = cast[ptr string](unsafeAddr s)[] proc encodeHash*( - hash: string, + hash: Hash, salt: seq[byte], iterations: SomeInteger, algorithm: Pbkdf2Algorithm, ): string = ## Encodes all relevant data for a password hash in a string. ## - ## Hash is assumed to be a base64 encoded strings. + ## Hash is a seq[byte] like salt and gets turned into a base64 encoded string with all padding suffix characters of "=" removed". ## Salt gets turned into a base64 encoded string with all padding suffix character of "=" removed. ## Algorithm is either "pbkdf2_sha256" or "pbkdf2_sha512" ## ## The pattern is: ## $$$$ + var encodedHash = hash.encode() + encodedHash.removeSuffix('=') var encodedSalt = salt.encode() encodedSalt.removeSuffix('=') - result = fmt"${algorithm}${iterations}${encodedSalt}${hash}" + result = fmt"${algorithm}${iterations}${encodedSalt}${encodedHash}" proc PKCS5_PBKDF2_HMAC( pass: cstring, @@ -93,7 +98,7 @@ proc PKCS5_PBKDF2_HMAC( ## The size of the out buffer is specified via keylen. -proc hashPbkdf2*(password: string, salt: seq[byte], iterations: int, digestFunction: EVP_MD): string {.gcsafe.} = +proc hashPbkdf2*(password: Password, salt: seq[byte], iterations: int, digestFunction: EVP_MD): Hash {.gcsafe.} = ## Hashes the given password with a SHA256 digest and the PBKDF2 hashing function ## from openSSL. This will execute the PBKDF2. ## HMAC = Hash based message authentication code @@ -102,7 +107,7 @@ proc hashPbkdf2*(password: string, salt: seq[byte], iterations: int, digestFunct raise newException(ValueError, fmt"You can not have more iterations than a c integer can carry. Choose a number below {cint.high}") let hashLength: cint = EVP_MD_size_fixed(digestFunction) - let output = newString(hashLength) + let output: string = newString(hashLength) let outputStartingpoint: cstring = cast[cstring](output[0].unsafeAddr) let hashOperationReturnCode = PKCS5_PBKDF2_HMAC( @@ -119,5 +124,4 @@ proc hashPbkdf2*(password: string, salt: seq[byte], iterations: int, digestFunct let wasHashSuccessful = hashOperationReturnCode == 1 doAssert wasHashSuccessful - result = encode(output) - result.removeSuffix("=") + result = cast[Hash](output) \ No newline at end of file diff --git a/src/nimword/private/types.nim b/src/nimword/private/types.nim new file mode 100644 index 0000000..9f42728 --- /dev/null +++ b/src/nimword/private/types.nim @@ -0,0 +1,5 @@ +type Password* = distinct string + +converter toPassword*(str: string): Password = str.Password + +type Hash* = seq[byte] \ No newline at end of file diff --git a/tests/t_argon2.nim b/tests/t_argon2.nim index 6c199d6..a27e1cb 100644 --- a/tests/t_argon2.nim +++ b/tests/t_argon2.nim @@ -83,7 +83,7 @@ suite "nimword-basics": let expectedEncodedHash: string = hashEncodePassword(password, iterations, phaArgon2id13, memoryLimitInKiB) let encodedSalt: string = expectedEncodedHash.split("$")[^2] let salt: seq[byte] = encodedSalt.decode() - let hash: string = hashPassword( + let hash: Hash = hashPassword( password, salt, iterations, @@ -145,12 +145,15 @@ suite "Argon2 specific": """: # Given let argonCommand = fmt"echo -n {password} | argon2 {saltStr} -v 13 -id -p 1 -k {memoryLimitInKiB} -t {iterations} -l {hashLength} -e" - let cliEncodedHash = execCmdEx(argonCommand).output - var cliHash: string = cliEncodedHash.split('$')[^1] - cliHash.removeSuffix("\n") + let cliResult = execCmdEx(argonCommand) + doAssert cliResult.exitCode == 0, "Failed to compute cli-hash. Problem was: " & cliResult.output + let cliEncodedHash = cliResult.output + + echo cliEncodedHash.split('$') # TODO: Remove + var cliHash: Hash = cliEncodedHash.split('$')[^1].decode() # When - let libHash = hashPassword( + let libHash: Hash = hashPassword( password, salt, iterations, diff --git a/tests/t_pbkdf2_sha256.nim b/tests/t_pbkdf2_sha256.nim index 2743c9c..f53f8a1 100644 --- a/tests/t_pbkdf2_sha256.nim +++ b/tests/t_pbkdf2_sha256.nim @@ -67,7 +67,7 @@ suite "PBKDF2-HMAC-SHA256": let expectedEncodedHash: string = hashEncodePassword(password, iterations) let encodedSalt: string = expectedEncodedHash.split("$")[^2] let salt: seq[byte] = encodedSalt.decode() - let hash: string = hashPassword( + let hash: Hash = hashPassword( password, salt, iterations diff --git a/tests/t_pbkdf2_sha512.nim b/tests/t_pbkdf2_sha512.nim index 096f7fb..82f29bc 100644 --- a/tests/t_pbkdf2_sha512.nim +++ b/tests/t_pbkdf2_sha512.nim @@ -67,7 +67,7 @@ suite "PBKDF2-HMAC-SHA512": let expectedEncodedHash: string = hashEncodePassword(password, iterations) let encodedSalt: string = expectedEncodedHash.split("$")[^2] let salt: seq[byte] = encodedSalt.decode() - let hash: string = hashPassword( + let hash: Hash = hashPassword( password, salt, iterations