Skip to content

DeriveKey Scrypt unit test#886

Merged
shargon merged 6 commits intoneo-project:masterfrom
shargon:scrypt-iterations
Jul 8, 2019
Merged

DeriveKey Scrypt unit test#886
shargon merged 6 commits intoneo-project:masterfrom
shargon:scrypt-iterations

Conversation

@shargon
Copy link
Copy Markdown
Member

@shargon shargon commented Jul 5, 2019 •
edited
Loading

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
Copy Markdown

codecov-io commented Jul 5, 2019 •
edited
Loading

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
erikzhang reviewed Jul 5, 2019
View reviewed changes
@erikzhang
Copy link
Copy Markdown
Member

erikzhang commented Jul 5, 2019

This does not change the results, right?

@shargon
Copy link
Copy Markdown
Member Author

shargon commented Jul 5, 2019

Yes, this change the results of the output hash

@erikzhang
Copy link
Copy Markdown
Member

erikzhang commented Jul 5, 2019

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

@shargon
Copy link
Copy Markdown
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
Copy Markdown
Member

erikzhang commented Jul 5, 2019

Yes, please.

@shargon shargon changed the title Strengthen PBKDF2 DeriveKey Scrypt unit test Jul 5, 2019
erikzhang
erikzhang reviewed Jul 5, 2019
View reviewed changes
vncoelho
vncoelho reviewed Jul 5, 2019
View reviewed changes
Copy link
Copy Markdown
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
@Celia18305
修复 Edge 兼容性问题 (neo-project#886)
56a8640 
* 调整样式，兼容 IE10+

* 修复 Edge 兼容性问题
Tommo-L pushed a commit to Tommo-L/neo that referenced this pull request Jun 22, 2020
@shargon @Tommo-L
DeriveKey Scrypt unit test (neo-project#886)
6073040 
* 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

Reviewers

@erikzhang erikzhang erikzhang left review comments

@vncoelho vncoelho vncoelho left review comments

@igormcoelho igormcoelho Awaiting requested review from igormcoelho

+1 more reviewer

@lock9 lock9 lock9 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@shargon @codecov-io @erikzhang @igormcoelho @vncoelho @lock9