#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
PhilippMDoerner · Sep 9, 2023 · 39e88cf · 39e88cf
1 parent ea04e42
commit 39e88cf
Show file tree

Hide file tree

Showing 11 changed files with 105 additions and 57 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,28 +28,29 @@ proc encodeHash*(
   ## $<algorithm>$v=19$m=<memoryLimit>,t=<iterations>,p=1$<salt>$<hash>
   var encodedSalt = salt.encode()
   encodedSalt.removeSuffix('=')
+  var encodedHash = hash.encode()
+  encodedHash.removeSuffix('=')
 
   let algorithmStr = case algorithm:
     of phaDefault, phaArgon2id13:
       "argon2id"
     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<https://doc.libsodium.org/password_hashing/default_phf#guidelines-for-choosing-the-parameters>`_
   ## 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<https://doc.libsodium.org/password_hashing/default_phf#key-derivation>`_ 
   ## 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,18 +108,18 @@ 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`.
   ## 
   ## `encodedHash` must be a string with the kind of pattern that `encodeHash<#encodeHash%2Cstring%2Cseq[byte]%2CSomeInteger>`_
   ## and `hashEncodePassword<#hashEncodePassword%2Cstring%2Cint>`_ generate. 
   ## 
   ## Raises SodiumError if an error happens during the process.
-  result = crypto_pwhash_str_verify(encodedHash, password) 
+  result = crypto_pwhash_str_verify(encodedHash, password.string) 
diff --git 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
     )
diff --git 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
     )