crypto-password
crypto-password copied to clipboard
Add argon2 hash functions
Adds argon2 encryption to the encryption methods offered by this library
Thanks for the patch. Can you remove any code unrelated to the feature being implemented?
Hello Mr. Reeves,
Firstly, thank you for your patience, and sorry for the mess I will squash down my commits, just see if it is ok for you now.
About the parameters, I followed this https://github.com/phxql/argon2-jvm#recommended-parameters, and take the default-iterations number by running the function,
(de.mkammerer.argon2.Argon2Helper/findIterations argon2 1000 default-memory-cost default-parallelization-parameter)
;;; (findIterations argon2 maxMillisecs memory parallelism)
;;; argon2 <-- argon2 instance
;;; maxMillisecs <-- Millisecs that the hash must take
;;; memory <-- Sets memory usage to x kibibytes
;;; parallelism <-- Number of threads and compute lane
Which give me the answer 22 used at default-iterations
For reference:
I was thinking of doing something like this on my side here:
(def default-memory-cost ,,,)
(def default-parallelization-parameter ,,,)
(defn n-iters []
(Argon2Helper/findIterations argon2 1000 default-memory-cost default-parallelization-parameter))
(def ^:private default-iterations
(Long/parseLong (System/getProperty "crypto.password.argon2.default-iterations" (n-iters))))
Hi, Mr. Reeves sorry the delay, busy here, but did not forget it. I will finished by this weekend. Thanks again, Gustavo Valente
On Sun, Apr 9, 2023 at 11:10 AM James Reeves @.***> wrote:
@.**** requested changes on this pull request.
Thanks for the PR, and apologies for the delay. There's some very minor styling changes needed, but other than that the code looks fine. After you've addressed the changes, can you squash your commits down?
I have a small contributing https://github.com/weavejester/contributing/blob/master/CONTRIBUTING.md document for my other repos that I haven't copied into this one quite yet.
In src/crypto/password/argon2.clj https://github.com/weavejester/crypto-password/pull/16#discussion_r1161291286 :
- See: https://infosecscout.com/best-algorithm-password-storage
- https://github.com/phxql/argon2-jvm"
- (:import [de.mkammerer.argon2 Argon2 Argon2Factory Argon2Advanced]))
+(def argon2 (Argon2Factory/create)) + +(def ^:private default-iterations
- (Long/parseLong (System/getProperty "crypto.password.argon2.default-iterations" "22")))
+(def ^:private default-memory-cost
- (Long/parseLong (System/getProperty "crypto.password.argon2.default-memory-cost" "65536")))
+(def ^:private default-parallelization-parameter
- (Long/parseLong (System/getProperty "crypto.password.argon2.default-parallelization-parameter" "1")))
Can you remove this extra blank line?
In src/crypto/password/argon2.clj https://github.com/weavejester/crypto-password/pull/16#discussion_r1161291777 :
iter- the number of iterations, defaults to 22 (see:
- Argon2Helper/findIterations)
mem- the memory cost, defaults to 65536
parallel- the parallelization parameter, defaults to 1"I don't think the default needs to be justified in the docstring. Better to just keep the relevant information for usage, and line up the definitions for clarity. So:
iter- the number of iterations, defaults to 22mem- the memory cost, defaults to 65536parallel- the parallelization parameter, defaults to 1
In src/crypto/password/argon2.clj https://github.com/weavejester/crypto-password/pull/16#discussion_r1161291967 :
- (Long/parseLong (System/getProperty "crypto.password.argon2.default-memory-cost" "65536")))
+(def ^:private default-parallelization-parameter
- (Long/parseLong (System/getProperty "crypto.password.argon2.default-parallelization-parameter" "1")))
+(defn encrypt
- "Encrypt a password string using the argon2 algorithm. This function takes
- three optional parameters:
iter- the number of iterations, defaults to 22 (see:- Argon2Helper/findIterations)
mem- the memory cost, defaults to 65536
parallel- the parallelization parameter, defaults to 1"- ([raw]
- (encrypt raw default-iterations default-memory-cost default-parallelization-parameter))
Can you keep this line within 80 characters? I know this rule is violated for the default parameter variables in all the namespaces, but we can fix that separately.
In src/crypto/password/argon2.clj https://github.com/weavejester/crypto-password/pull/16#discussion_r1161292131 :
+(defn encrypt
- "Encrypt a password string using the argon2 algorithm. This function takes
- three optional parameters:
iter- the number of iterations, defaults to 22 (see:- Argon2Helper/findIterations)
mem- the memory cost, defaults to 65536
parallel- the parallelization parameter, defaults to 1"- ([raw]
- (encrypt raw default-iterations default-memory-cost default-parallelization-parameter))
- ([raw iter mem parallel]
- (.hash argon2 iter mem parallel raw)))
+(defn check
- "Compare a raw string with a string encrypted with the [[encrypt]]
- function. Returns true if the string matches, false otherwise."
Should be a single space after the .. So:
function. Returns true if the string matches, false otherwise."
— Reply to this email directly, view it on GitHub https://github.com/weavejester/crypto-password/pull/16#pullrequestreview-1376915809, or unsubscribe https://github.com/notifications/unsubscribe-auth/AW2SZV523Q7AVMETBRE6D6DXAK7OBANCNFSM6AAAAAAVNNKOVA . You are receiving this because you authored the thread.Message ID: @.***>
Hi Mr. Reeves, please see if the changes are ok. And sorry about the delay!
Hi @weavejester @gmsvalente, any update about this PR :)
Thanks,
Marius