Opened 3 weeks ago

Closed 5 days ago

Last modified 5 days ago

#36744 closed Cleanup/optimization (fixed)

Improve/correct scrypt password hasher documentation

Reported by: Dmitry Chestnykh Owned by: Dmitry Chestnykh
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Dmitry Chestnykh Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The current documentation for scrypt password hasher lacks some constraints and definitions:

  1. work_factor: described incompletely, missing the requirement that this value must be a power of 2
  2. block_size: has no description
  3. parallelism: needs to be defined, perhaps, as "independent computations" rather than "threads", because the current OpenSSL implementation is not multithreaded.

If OpenSSL becomes multithreaded, the "Estimating memory usage" note also needs to be adjusted to account for parallelism.

I think it would also be helpful to mention that these parameters are N, r, p, as these are standard terminology for those familiar with scrypt.

I'll send a pull request shortly.

Change History (6)

comment:1 by Natalia Bidart, 3 weeks ago

Triage Stage: Unreviewed → Accepted

Documentation is always welcomed! Thank you for volunteering.

I see you checked the "has patch" flag. Is there a PR ready for this?

comment:2 by Dmitry Chestnykh, 3 weeks ago

Thanks, Natalia! Yes, the PR is here: ​https://github.com/django/django/pull/20269

comment:3 by Mariusz Felisiak, 6 days ago

Patch needs improvement: set

comment:4 by Mariusz Felisiak, 5 days ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 5 days ago

Resolution: → fixed
Status: assigned → closed

In 0ca3a06:

Fixed #36744 -- Improved scrypt password hasher docs.

  • Corrected work_factor description and its requirements.
  • Added block_size description.
  • Changed parallelism description to mention computations, rather than threads (currently it's not multithreaded.)
  • For all of the above, added standard scrypt terminology (N, r, p).
  • Mentioned that in multithreaded implementations, parallelism also influences the memory requirements.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 5 days ago

In 719db78:

[6.0.x] Fixed #36744 -- Improved scrypt password hasher docs.

  • Corrected work_factor description and its requirements.
  • Added block_size description.
  • Changed parallelism description to mention computations, rather than threads (currently it's not multithreaded.)
  • For all of the above, added standard scrypt terminology (N, r, p).
  • Mentioned that in multithreaded implementations, parallelism also influences the memory requirements.

Backport of 0ca3a0661173b02e2cbb0183d8543e790e7e4a55 from main

Note: See TracTickets for help on using tickets.
Back to Top