planety / prologue

Powerful and flexible web framework written in Nim
https://planety.github.io/prologue
Apache License 2.0
1.24k stars 47 forks source link

Prologue Encryption - Docs for hashing utils provided by prologue + maybe swap to or offer openssl implementation? #172

Open PhilippMDoerner opened 2 years ago

PhilippMDoerner commented 2 years ago

Heyho,

I've recently noticed that prologue offers its own hashing proc to allow simple pbkdf2_sha256 hashing, with an API similar to how its handled in Django. This is in the prologue/security/hasher module.

Suggestion 1) I think it would be useful to have a section in the quickstart docs that at least mention the hashing util procs there.

Secondly, from what I'm seeing, it is using nimcrypto to perform the hasing. From my own experience, using nimcrypto compared to openssl has a significant slowdown. Thus, and given that openssl is the more established library, I would want to make another suggestion.

Suggestion 2) Either replace the current pkdf2 hashing procs with one that makes use of openssl or alternatively just offer an alternative. I already have an implementation as you might be aware of, since hotdog from the discord provided me with one:

from std/openssl import DLLSSLName, EVP_MD, EVP_sha256, EVP_MD_size, DLLUtilName

proc PKCS5_PBKDF2_HMAC(
  pass: cstring,
  passLen: cint,
  salt: cstring,
  saltLen: cint,
  iter: cint,
  digest: EVP_MD,
  keylen: cint,
  output: cstring
): cint {.cdecl, dynlib: DLLSSLName, importc: "PKCS5_PBKDF2_HMAC".}

proc EVP_MD_size_fixed*(md: EVP_MD): cint {.cdecl, dynlib: DLLUtilName, importc: "EVP_MD_size".}
proc EVP_sha256_fixed*(): EVP_MD    {.cdecl, dynlib: DLLUtilName, importc: "EVP_sha256".}

proc openssl_calculate_SHA256_pbkdf2_hash(password: string, salt: string, iterations: int, secretKey: string): string {.gcsafe.} =
  if iterations > cint.high: raise newException(ValueError, "iterations too high")
  let
    digest: EVP_MD = EVP_sha256_fixed()
    keylen: cint = EVP_MD_size_fixed(digest)
    output = newString(keylen)

    retVal = PKCS5_PBKDF2_HMAC(
      password.cstring,
      -1,
      salt.cstring,
      len(salt).cint,
      iterations.cint,
      digest,
      keylen,
      output[0].unsafeAddr
    )

  doAssert retVal == 1
  result = encode(output)

What is your opinion on that one @xflywind ?

ringabout commented 2 years ago

Hi, hasher has already deprecated because of a typo => https://github.com/planety/prologue/blob/321d6e9a1fae4bc096cb65383834f95c5c6aecac/src/prologue/security/hasher.nim#L15

ref https://github.com/planety/prologue/issues/140

The hash util is subtle. Imo it should be put in a separate library either or it can stay on the Prologue org.

PhilippMDoerner commented 2 years ago

What is the prologue org? As for where to put it, I'm pretty ambivalent in that regard. If you prefer it could be one of the prologue extensions we have?

I would mostly want some level of docs on it, since crypto is one of those things that is immensely annoying to research when you want to do implement your password hashing.

ringabout commented 2 years ago

If you prefer it could be one of the prologue extensions we have

Yeah, I agree. It can start from an extension/library.

What is the prologue org?

Sorry I mean https://github.com/planety. If you need access to https://github.com/planety, feel free to tell me.