fscrypt icon indicating copy to clipboard operation
fscrypt copied to clipboard

Fix Parallelism Cost when NumCPU() > 255

Open josephlr opened this issue 3 years ago • 1 comments

As pointed out in #363, we are not currently handling cases with 256 or more CPUs correctly. Our current behavior:

  • Casts costs.parallelism to a uint8
  • Panics if NumCPU is a multiple of 256
  • If costs.parallelism is > 256, we silently use the truncated parallelism cost. This is the worst part.

This PR does multiple things (see the commits for more detailed information):

  • Increases cost error checking before calling into the Argon2 library.
  • Adds a truncation_fixed boolean field to the HashingCosts proto, indicating that this bug is fixed.
  • In getHashingCosts, cap the number of threads to 255.

josephlr avatar Aug 27 '22 08:08 josephlr

Thanks for doing this! Two comments:

  • Bug: at the bottom of getHashingCosts(), a HashingCosts is being initialized without TruncationFixed: true.
  • func (h *HashingCosts) CheckValidity() already does some of the new checks that are being added to PassphraseHash(). Should they be consolidated into one place?

ebiggers avatar Aug 31 '22 06:08 ebiggers