CVE-2019-20138: Use of a Broken or Risky Cryptographic Algorithm (CWE-327)
value: The HTTP Authentication library before 2019-12-27 for Nim has weak password hashing because the default algorithm for libsodium's crypto_pwhash_str is not used.
CWE description: The product uses a broken or risky cryptographic algorithm or protocol.
CVSS v3 base score: 7.5
Published: 2019-12-30T14:15Z
Repository: https://github.com/FedericoCeratto/nim-httpauth
Language: Other
Files changed: README.adoc, httpauth.nim, httpauth.nimble, makefile, nim.cfg, tests/unit.nim
Commit date: 2019-12-28 00:13:48 +0000
Commit message: 0.2.0 implement libsodium crypto_pwhash_str

Fix #23
Add tests, improve docs

Diff: diff --git a/README.adoc b/README.adoc
index 400e6b7..8c7a032 100644
--- a/README.adoc
+++ b/README.adoc
@@ -4,10 +4,12 @@
 image:https://circleci.com/gh/FedericoCeratto/nim-httpauth.svg?style=svg["CircleCI", link="https://circleci.com/gh/FedericoCeratto/nim-httpauth"]
 image:https://img.shields.io/badge/status-stable-green.svg[badge]
 image:https://img.shields.io/github/tag/FedericoCeratto/nim-httpauth.svg[tags]
 image:https://img.shields.io/badge/License-LGPL%20v3-blue.svg[License]
 
+WARNING: Version 0.2.0 is not backward compatible due previously weak password hashing
+
 .Features:
 [none]
 - [x] Encrypted+signed session cookies
 - [x] Sync SQLite backend
 - [x] Sync MySQL backend
@@ -19,14 +21,10 @@ image:https://img.shields.io/badge/License-LGPL%20v3-blue.svg[License]
 - [x] Send password recovery emails
 - [ ] FIDO U2F 2nd factor authentication
 - [ ] Async JSON backend
 - [ ] Async backends (when async libraries will be available)
 
-.Not planned:
-- bcrypt (obsolete)
-- pbkdf2 (obsolete)
-
 Install the dependencies:
 [source,bash]
 ----
 sudo apt-get install libsodium18
 ----
@@ -120,16 +118,15 @@ var auth = newHTTPAuth("localhost", backend)
 
 ==== Security
 
 Cookie-based sessions are encrypted and signed to protect from cookie exfiltration from the browser.
 Registration and password reset tokens are signed and have a timeout.
-Passwords are stored using Scrypt or Argon2.
+Passwords are hashed using libsodium default algorithm.
+The hash is regenerated automatically on login if needed.
 
 WARNING: Remember to filter user input to prevent XSS, SQL injections and similar attacks.
 
-WARNING: Scrypt and Argon2 will not be robust enough forever. You'll have to update the library and rehash passwords.
-
 ==== Usage example:
 [source,nim]
 ----
 import asyncdispatch, strutils, json
 import httpauth, jester
diff --git a/httpauth.nim b/httpauth.nim
index f814000..0955d43 100755
--- a/httpauth.nim
+++ b/httpauth.nim
@@ -53,11 +53,10 @@ const
 type
   HashingAlgorithm* = enum
     Argon2, Scrypt
 
   HTTPAuth* = object of RootObj
-    preferred_hashing_algorithm*: HashingAlgorithm
     password_reset_timeout*: int
     domain*: string
     cookie_name*: string
     cookie_domain*: string
     session_key: string  # secret used for cookie crypto + sign
@@ -99,41 +98,19 @@ proc safe_decode(i: string): string =
 
 # 4096 bytes cookie limit FIXME
 
 # Crypto
 
-const scrypt_salt_size = 32
+proc password_pwhash_str*(password: string): string =
+  ## Hash password using libsodium `crypto_pwhash_str`
+  ## using the recommended algorithm. The output is ASCII-only
+  ## and database-safe.
+  crypto_pwhash_str(password)
 
-proc hash_scrypt(username, password: string, salt=""): string =
-  ## Scrypt hashing. Return b64 encoded('s' + salt + hash)
-  let salt =
-    if salt == "":
-      randombytes(scrypt_salt_size)
-    else:
-      salt
-  assert salt.len == scrypt_salt_size
-
-  let cleartext = "$#\0$#" % [username, password]
-  # FIXME let h = scrypt.hash(cleartext, salt)
-  let h = cleartext
-  let hashed = "s" & salt & h  # 's' for scrypt
-  return safe_encode(hashed)  # FIXME encoding/decoding required?
-
-proc hash_argon2(username, password: string, salt=""): string =
-  ##
-  ""
+proc password_needs_rehashing*(password: string): bool =
+  crypto_pwhash_str_needs_rehash(password) != 0
 
-proc hash(self: HTTPAuth, username, pwd: string, algo:HashingAlgorithm, salt=""): string =
-  ## Hash username and password, generating salt value if required
-  assert algo == Scrypt
-
-  case algo
-  of HashingAlgorithm.Scrypt:
-    return hash_scrypt(username, pwd, salt=salt)
-
-  of HashingAlgorithm.Argon2:
-    return hash_argon2(username, pwd, salt=salt)
 
 # # Cookies and session
 
 proc set_auth_cookie*(self: HTTPAuth, value: string) =
   ## Set auth session cookie
@@ -209,26 +186,13 @@ proc is_user_anonymous*(self: HTTPAuth): bool =
   return false
 
 
 # # Procs that can be run by unauthenticated users
 
-proc verify_password(username, password, hash: string): bool =
-  ## Verify username/password pair against a salted hash
-  assert hash != ""
-  let decoded = safe_decode(hash)
-  case decoded[0]
-  of 's':
-    let salt = decoded[1..(scrypt_salt_size)]
-    assert salt.len == scrypt_salt_size, $salt.len
-    let newhash = hash_scrypt(username, password, salt=salt)
-    let newhash2 = hash_scrypt(username, password, salt=salt)
-    assert newhash == newhash2
-    #FIXME time independent comparison?
-    return newhash == hash
-
-  else:
-    raise newException(AuthError, "Unknown hashing algorithm")
+proc verify_password(password, pwhash: string): bool =
+  ## Verify password hashed by libsodium
+  return crypto_pwhash_str_verify(pwhash, password)
 
 proc login*(self: HTTPAuth, username, password: string) =
   ## Check login credentials and set a cookie on success
   ## Do not leak out if a login is failed due to incorrect password
   ## or inexistent user to prevent user enumeration.
@@ -238,17 +202,18 @@ proc login*(self: HTTPAuth, username, password: string) =
   except UserNotFoundError:
     raise newException(LoginError, "Failed login")
   # FIXME prevent timing attacks
 
   assert user.hash != ""
-  let authenticated = verify_password(
-    username,
-    password,
-    user.hash,
-  )
+  let authenticated = verify_password(password, user.hash)
   if not authenticated:
     raise newException(LoginError, "Failed login")
+
+  if password_needs_rehashing(user.hash):
+    user.hash = password_pwhash_str(password)
+    self.backend.set_user(user)
+
   self.store_session(username)
 
   # if login_time
   # user.last_login = getEpoch().utcnow()
   # self.backend.set_user(User)
@@ -308,16 +273,16 @@ proc register*(self: HTTPAuth, username, password, email_addr: string, role="use
 
   # send registration email
   asyncCheck self.mailer.send_email(email_addr, subject, email_text)
 
   # store pending registration
-  let user_pwd_hash = self.hash(username, password, self.preferred_hashing_algorithm)
+  let pwhash = password_pwhash_str(password)
 
   self.backend.set_pending_registration(registration_code, PendingRegistration(
     username: username,
     role: role,
-    hash: user_pwd_hash,
+    hash: pwhash,
     email_addr: email_addr,
     description: description,
     creation_date: creation_date,
   ))
   self.backend.save_pending_registrations()
@@ -424,11 +389,11 @@ proc reset_password*(self: HTTPAuth, reset_code, password: string) =
   ## The username is extracted from the reset_code token
   let (username, email_addr) = self.validate_reset_code(reset_code)
   var user = self.backend.get_user(username)
   if user.email_addr != email_addr:
     raise newException(AuthError, "Incorrect email address in reset code")
-  user.hash = self.hash(username, password, self.preferred_hashing_algorithm)
+  user.hash = password_pwhash_str(password)
   self.backend.set_user(user)
   self.backend.save_users()
 
 
 
@@ -467,16 +432,16 @@ proc create_user*(self: HTTPAuth, username, password: string, role = "user",
   try:
     role_o = self.backend.get_role(role)
   except:
     raise newException(AuthError, "Nonexistent user role.")
 
-  let h = self.hash(username, password, self.preferred_hashing_algorithm)
+  let pwhash = password_pwhash_str(password)
   let tstamp = getTime().getGMTime
   self.backend.set_user(User(
     username: username,
     role: role,
-    hash: h,
+    hash: pwhash,
     email_addr: email_addr,
     description: description,
     creation_date: tstamp,
     last_login: tstamp
   ))
@@ -537,17 +502,15 @@ proc delete_role*(self: HTTPAuth, role: string) =
 # Internals
 
 const one_day = 24 * 3600
 
 proc newHTTPAuth*(domain: string, backend: HTTPAuthBackend, cookie_name="", cookie_domain="",
-    preferred_hashing_algorithm = HashingAlgorithm.Scrypt,
     password_reset_timeout=one_day, session_key="", https_only_cookies=true): HTTPAuth =
   ## Initialize HTTPAuth
   result = HTTPAuth(domain: domain, backend: backend,
     password_reset_timeout: password_reset_timeout,
     https_only_cookies: https_only_cookies,
-    preferred_hashing_algorithm: preferred_hashing_algorithm,
   )
   result.mailer = newMailer() #FIXME: pass arguments
   assert result.backend != nil
 
   result.cookie_name = if cookie_name == "": domain else: cookie_name
@@ -588,17 +551,16 @@ proc initialize_admin_user*(self: HTTPAuth, username="admin", password="", role=
   self.backend.set_role(Role(
     name: role,
     level: admin_level
   ))
   self.backend.save_roles()
-  let h = self.hash(username, password, self.preferred_hashing_algorithm)
-  # FIXME assert h.len == 52, $h.len
+  let pwhash = password_pwhash_str(password)
   let tstamp = getTime().getGMTime
   self.backend.set_user(User(
     username: username,
     role: role,
-    hash: h,
+    hash: pwhash,
     email_addr: email_addr,
     description: description,
     creation_date: tstamp,
     last_login: tstamp
   ))
@@ -639,11 +601,11 @@ proc require*(self: HTTPAuth, username="", role="", fixed_role=false) =
 
 
 proc update_user_password*(self: HTTPAuth, username, password: string) =
   ## Update user password
   var user = self.backend.get_user(username)
-  user.hash = self.hash(username, password, self.preferred_hashing_algorithm)
+  user.hash = password_pwhash_str(password)
   self.backend.set_user(user)
   self.backend.save_users()
 
 proc update_user_description*(self: HTTPAuth, username, description: string) =
   ## Update user description
diff --git a/httpauth.nimble b/httpauth.nimble
index 02cd92a..8b3a673 100644
--- a/httpauth.nimble
+++ b/httpauth.nimble
@@ -1,8 +1,8 @@
 # Package
 
-version       = "0.1.3"
+version       = "0.2.0"
 author        = "Federico Ceratto"
 description   = "HTTP Authentication and Authorization"
 license       = "LGPLv3"
 
 bin           = @["httpauth"]
diff --git a/makefile b/makefile
index 45499df..d04489d 100644
--- a/makefile
+++ b/makefile
@@ -65,15 +65,18 @@ functional_mongodb:
 	mongo httpauth_test --eval 'db.pending_registrations.drop()'
 	mongo httpauth_test --eval 'db.roles.drop()'
 	mongo httpauth_test --eval 'db.users.drop()'
 	./tests/functional mongodb://127.0.0.1/httpauth_test
 
-functional: build_functional_tests functional_sqlite functional_mysql functional_etcd build_functional_tests_mongodb functional_mongodb
+unit:
+	nim c -p=. -r tests/unit.nim
+
+functional: unit build_functional_tests functional_sqlite functional_mysql functional_etcd build_functional_tests_mongodb functional_mongodb
 
 # CircleCI
 # FIXME MySQL Etcd Redis
-circleci: build_functional_tests functional_sqlite build_functional_tests_mongodb functional_mongodb
+circleci: unit build_functional_tests functional_sqlite build_functional_tests_mongodb functional_mongodb
 
 start_databases:
 	sudo systemctl start etcd.service
 	sudo systemctl start mongodb.service
 	sudo systemctl start mysql.service
diff --git a/nim.cfg b/nim.cfg
index 521e21d..1aad1b3 100644
--- a/nim.cfg
+++ b/nim.cfg
@@ -1 +1 @@
--d:ssl
+-d:ssl --hints:off
diff --git a/tests/unit.nim b/tests/unit.nim
new file mode 100644
index 0000000..c64962e
--- /dev/null
+++ b/tests/unit.nim
@@ -0,0 +1,22 @@
+
+## Nim HTTP Authentication and Authorization
+## Functional tests. HTTP headers are mocked
+## Copyright 2019 Federico Ceratto <federico.ceratto@gmail.com>
+## Released under LGPLv3 License, see LICENSE file
+
+import unittest,
+  strutils
+
+from libsodium/sodium import crypto_pwhash_str_verify
+
+import httpauthpkg/base
+
+import httpauth
+
+suite "hashing":
+  test "password_pwhash_str":
+    const pwd = "Correct Horse Battery Staple"
+    let h = password_pwhash_str(pwd)
+    check crypto_pwhash_str_verify(h, pwd) == true
+    check crypto_pwhash_str_verify(h, pwd & "!") == false
+    check password_needs_rehashing(h) == false

Vulnerable code: .Not planned:
- bcrypt (obsolete)
- pbkdf2 (obsolete)

Passwords are stored using Scrypt or Argon2.
WARNING: Scrypt and Argon2 will not be robust enough forever. You'll have to update the library and rehash passwords.

    preferred_hashing_algorithm*: HashingAlgorithm
const scrypt_salt_size = 32
proc hash_scrypt(username, password: string, salt=""): string =
  ## Scrypt hashing. Return b64 encoded('s' + salt + hash)
  let salt =
    if salt == "":
      randombytes(scrypt_salt_size)
    else:
      salt
  assert salt.len == scrypt_salt_size

  let cleartext = "$#\0$#" % [username, password]
  # FIXME let h = scrypt.hash(cleartext, salt)
  let h = cleartext
  let hashed = "s" & salt & h  # 's' for scrypt
  return safe_encode(hashed)  # FIXME encoding/decoding required?

proc hash_argon2(username, password: string, salt=""): string =
  ##
  ""
proc hash(self: HTTPAuth, username, pwd: string, algo:HashingAlgorithm, salt=""): string =
  ## Hash username and password, generating salt value if required
  assert algo == Scrypt

  case algo
  of HashingAlgorithm.Scrypt:
    return hash_scrypt(username, pwd, salt=salt)

  of HashingAlgorithm.Argon2:
    return hash_argon2(username, pwd, salt=salt)
proc verify_password(username, password, hash: string): bool =
  ## Verify username/password pair against a salted hash
  assert hash != ""
  let decoded = safe_decode(hash)
  case decoded[0]
  of 's':
    let salt = decoded[1..(scrypt_salt_size)]
    assert salt.len == scrypt_salt_size, $salt.len
    let newhash = hash_scrypt(username, password, salt=salt)
    let newhash2 = hash_scrypt(username, password, salt=salt)
    assert newhash == newhash2
    #FIXME time independent comparison?
    return newhash == hash

  else:
    raise newException(AuthError, "Unknown hashing algorithm")
  let authenticated = verify_password(
    username,
    password,
    user.hash,
  )
  let user_pwd_hash = self.hash(username, password, self.preferred_hashing_algorithm)
    hash: user_pwd_hash,
  user.hash = self.hash(username, password, self.preferred_hashing_algorithm)
  let h = self.hash(username, password, self.preferred_hashing_algorithm)
    hash: h,
    preferred_hashing_algorithm = HashingAlgorithm.Scrypt,
    preferred_hashing_algorithm: preferred_hashing_algorithm,
  let h = self.hash(username, password, self.preferred_hashing_algorithm)
  # FIXME assert h.len == 52, $h.len
    hash: h,
  user.hash = self.hash(username, password, self.preferred_hashing_algorithm)
version       = "0.1.3"
functional: build_functional_tests functional_sqlite functional_mysql functional_etcd build_functional_tests_mongodb functional_mongodb
circleci: build_functional_tests functional_sqlite build_functional_tests_mongodb functional_mongodb
-d:ssl
Fixed code: WARNING: Version 0.2.0 is not backward compatible due previously weak password hashing

Passwords are hashed using libsodium default algorithm.
The hash is regenerated automatically on login if needed.
proc password_pwhash_str*(password: string): string =
  ## Hash password using libsodium `crypto_pwhash_str`
  ## using the recommended algorithm. The output is ASCII-only
  ## and database-safe.
  crypto_pwhash_str(password)
proc password_needs_rehashing*(password: string): bool =
  crypto_pwhash_str_needs_rehash(password) != 0
proc verify_password(password, pwhash: string): bool =
  ## Verify password hashed by libsodium
  return crypto_pwhash_str_verify(pwhash, password)
  let authenticated = verify_password(password, user.hash)

  if password_needs_rehashing(user.hash):
    user.hash = password_pwhash_str(password)
    self.backend.set_user(user)

  let pwhash = password_pwhash_str(password)
    hash: pwhash,
  user.hash = password_pwhash_str(password)
  let pwhash = password_pwhash_str(password)
    hash: pwhash,
  let pwhash = password_pwhash_str(password)
    hash: pwhash,
  user.hash = password_pwhash_str(password)
version       = "0.2.0"
unit:
	nim c -p=. -r tests/unit.nim

functional: unit build_functional_tests functional_sqlite functional_mysql functional_etcd build_functional_tests_mongodb functional_mongodb
circleci: unit build_functional_tests functional_sqlite build_functional_tests_mongodb functional_mongodb
-d:ssl --hints:off

## Nim HTTP Authentication and Authorization
## Functional tests. HTTP headers are mocked
## Copyright 2019 Federico Ceratto <federico.ceratto@gmail.com>
## Released under LGPLv3 License, see LICENSE file

import unittest,
  strutils

from libsodium/sodium import crypto_pwhash_str_verify

import httpauthpkg/base

import httpauth

suite "hashing":
  test "password_pwhash_str":
    const pwd = "Correct Horse Battery Staple"
    let h = password_pwhash_str(pwd)
    check crypto_pwhash_str_verify(h, pwd) == true
    check crypto_pwhash_str_verify(h, pwd & "!") == false
    check password_needs_rehashing(h) == false