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

DeriveKey Scrypt unit test #886

Merged
merged 6 commits into from
Jul 8, 2019
Merged

Conversation

shargon
Copy link
Member

@shargon shargon commented Jul 5, 2019

The number of iterations is set to only to 1, last NIST recommendation (from 2016) is 10.000 (https://pages.nist.gov/800-63-3/sp800-63b.html#sec5).

@codecov-io
Copy link

codecov-io commented Jul 5, 2019

Codecov Report

Merging #886 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #886   +/-   ##
=======================================
  Coverage   45.04%   45.04%           
=======================================
  Files         178      178           
  Lines       12603    12603           
=======================================
  Hits         5677     5677           
  Misses       6926     6926

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aab9572...beabc1d. Read the comment docs.

@erikzhang
Copy link
Member

This does not change the results, right?

@shargon
Copy link
Member Author

shargon commented Jul 5, 2019

Yes, this change the results of the output hash

@erikzhang
Copy link
Member

I think it is not compatible with scrypt. In RFC 7914, it is fixed at 1.

@shargon
Copy link
Member Author

shargon commented Jul 5, 2019

Because scrypt add extra security layer (and this method is used only with scrypt), maybe is not needed. If you want i can remove all changes except the unit test

@erikzhang
Copy link
Member

Yes, please.

@shargon shargon changed the title Strengthen PBKDF2 DeriveKey Scrypt unit test Jul 5, 2019
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds very good, Shargon.

Just this time assert that is funny...aheuahuea
Maybe just report the time for now.

@shargon shargon merged commit 29fb4ed into neo-project:master Jul 8, 2019
@shargon shargon deleted the scrypt-iterations branch July 8, 2019 07:15
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
* 调整样式,兼容 IE10+

* 修复 Edge 兼容性问题
Tommo-L pushed a commit to Tommo-L/neo that referenced this pull request Jun 22, 2020
* Strengthen PBKDF2

* Update SCrypt.cs

* Scrypt DeriveKey ut

* Deterministic behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants