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

V2.0 Security Audit #28

Closed
defuse opened this Issue Aug 13, 2014 · 12 comments

Comments

Projects
None yet
2 participants
@defuse
Copy link
Owner

defuse commented Aug 13, 2014

Just before we release v2.0, after all of the other tickets in the milestone have been closed, it should be audited by me and some other volunteers. Maybe we can set up a bug bounty.

(Maybe @sarciszewski will be interested).

@defuse defuse added this to the v2.0 milestone Aug 13, 2014

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Aug 16, 2014

Pay attention to changes from #14, as this is where issues were most likely to be introduced.

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Aug 16, 2014

I'll do this tomorrow if all the other tickets miraculously close themselves today.

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Apr 30, 2015

I'm really wondering, why, on August 16, 2014, that I thought all of the tickets would miraculously close that day.

@defuse defuse modified the milestones: v2.0, v2.0 Release Feb 9, 2016

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 9, 2016

Taylor from 2016 agrees with Taylor from 2015 and wonders why, on August 16, 2014, that I thought all of the tickets would miraculously close that day.

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 9, 2016

Dear Taylor from 2017: If this ticket still hasn't been closed, you need to re-evaluate your life choices.

@defuse defuse referenced this issue Feb 9, 2016

Closed

Unit tests #5

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 9, 2016

As a part of this audit, go over the tests and make sure everything important is covered.

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 9, 2016

All the other 2.0 tickets are closed, so the audit may now commence (tomorrow)!

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 9, 2016

Check that we're not generating too much output with PBKDF2. There's that thing where if you ask for too much it does double the work with no benefit.

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 9, 2016

CALL FOR CODE REVIEW:

This code does PBKDF2-SHA1 password storage in PHP, Ruby, C#, and Java. The implementations are compatible with each other. Since we have better things now (bcrypt, scrypt, yescrypt, argon2, etc.) I'm ceasing development of this library and putting out the final version. I'd like to not touch this code anymore, so now's the time to find all the security bugs!

If you'd like to help, please try to find bugs in the compatible-versions branch! Open tickets if you find any.

Thanks!

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 9, 2016

Taylor's Audit (4.5 hours):

  • Plan the audit (25min)
  • Brainstorming phase (25min)
  • PasswordStorage.cs (25min)
  • PasswordStorage.php (25min)
  • PasswordStorage.rb (25min)
  • PasswordStorage.java (25min)
  • C#, PHP, Ruby, and Java tests are adequate. (25min) (note: I didn't carefully looked over the tests for errors, just checked that they were testing all the things they should be testing).
  • Cross-compatibility tests are adequate (25min) (note: same as above).
  • Documentation is readable/usable (25min).
  • Add links to all the issues I discovered and fix commits to this comment.

Audit Results:

Issues:

Fixes:

@paragonie-scott

This comment has been minimized.

Copy link
Contributor

paragonie-scott commented Feb 10, 2016

#68 + #69 are worth looking at.

@defuse

This comment has been minimized.

Copy link
Owner Author

defuse commented Feb 10, 2016

Okay, I'm satisfied by this code's review status! There's a few open issues and then this will finally be done!!!!

@defuse defuse closed this Feb 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment