Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter extraction functions #55

Closed
Vlix opened this issue Jan 26, 2022 · 7 comments
Closed

Parameter extraction functions #55

Vlix opened this issue Jan 26, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@Vlix
Copy link
Collaborator

Vlix commented Jan 26, 2022

Might be nice to also expose the functions that take a PasswordHash a and return the parameters for that hash. (in case the hash is parsable)
e.g. extractParams :: PasswordHash Scrypt -> Maybe ScryptParams

Which later on might be extended to extractParamsWithFormat :: ScryptFormat -> PasswordHash Scrypt -> Maybe ScryptParams when we also support different hashing formats.

Kind of part of the #22 enhancement, so users can check hashes. e.g. when filtering hashes from a DB.

@Vlix Vlix added the enhancement New feature or request label Jan 26, 2022
@blackheaven
Copy link
Contributor

I was going to tackle it, but I had hard time figuring out how to extract the salt, should I use the default to fake it?

@Vlix
Copy link
Collaborator Author

Vlix commented Oct 2, 2022

@blackheaven Thank you for taking a stab at this.

Every checkPassword function parses the necessary parameters and salt from the hash, so I imagine you can factor out those parts as extractParams 🤔

@blackheaven
Copy link
Contributor

After a second read you're right but (except Bcrypt's checkPassword which does not extract the salt at all):
All checkPassword uses/extracts Salt a and default their salt parameter (which is the salt size, not the salt itself).

So, just to be sure: we want to have functions which extracts *Params with a correct salt size?

@Vlix
Copy link
Collaborator Author

Vlix commented Oct 2, 2022

Be aware that the parameters for passwords never contain salts. Salts are supposed to be random for security's sake.

The checkPassword functions default the salt parameter just to not have any undefined record fields, since the salt parameter isn't used when checking the password. The extractPassword functions would do well to check the salt's length to construct an accurate salt size parameter.

Also, bcrypt as an algorithm requires the salt to be 16 bytes, so that is not part of the adjustable parameters.

@blackheaven
Copy link
Contributor

I've made an attempt in #61.

There's two ways to mitigate de default value issue coming to my mind:

  • Create dedicated types (eg. Argon2ParamsParsed)
  • Having a Phantom type and a type family (see trees that grow)

Let me know what you think, and whether I should handle it it in my PR, another one, or not at all.

@Vlix
Copy link
Collaborator Author

Vlix commented Oct 2, 2022

I'm pretty sure that the following holds:

-- This doesn't actually work, since 'newSalt' is in IO, but you get the idea
testProperty "length of salt = size of salt" $ \i ->
  let Salt salt = Data.Password.Internal.newSalt i
  in Data.ByteString.length salt === i

So you can just get the salt size from the salt:

-- e.g. the Argon2 one
extractParams passHash = do
  (params, Salt salt, _) <- parseArgon2PasswordHashParams passHash
  let saltSize = Data.ByteString.length salt
  pure params{argon2Salt = saltSize}

@Vlix
Copy link
Collaborator Author

Vlix commented Oct 8, 2022

#61 implemented this

@Vlix Vlix closed this as completed Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants