Fixes for Issue #204.

[besser82]

• crypt: Zeroize initialized and reserved data members after computation.
• crypt: Zeroize and initialize (re)allocated memory in crypt_ra.
• test/short-outbuf: Add two more cases for crypt_ra.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.15%. Comparing base (0cada50) to head (ac873d4).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #205      +/-   ##
===========================================
+ Coverage    90.12%   90.15%   +0.02%     
===========================================
  Files           32       32              
  Lines         3627     3626       -1     
  Branches       690      689       -1     
===========================================
  Hits          3269     3269              
+ Misses         227      226       -1     
  Partials       131      131              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[solardiz]

BTW, realloc gives us no way to zeroize the previous data if it happens to relocate it to a new address. This could be worth a source code comment maybe, if we started to care about these things here.

[besser82]

BTW, realloc gives us no way to zeroize the previous data if it happens to relocate it to a new address. This could be worth a source code comment maybe, if we started to care about these things here.

I've taken that into account in the rebased commits.

[solardiz]

The code on its own looks good to me now, although I suggest to use a simple memset after the realloc because in that place it's pure initialization and not zeroization of sensitive data.

While at it, I once again notice that we have some unfortunate (in my opinion) recommendations in the man page: "Applications are encouraged, but not required, to use the phrase and setting fields to store the strings that they will pass as phrase and setting to crypt_r." I think it's very wrong that we don't just keep this struct opaque to the callers (except maybe for the initialized field). We may want to fix that separately.

[besser82]

The code on its own looks good to me now, although I suggest to use a simple memset after the realloc because in that place it's pure initialization and not zeroization of sensitive data.

Fixed in merged commits.

While at it, I once again notice that we have some unfortunate (in my opinion) recommendations in the man page: "Applications are encouraged, but not required, to use the phrase and setting fields to store the strings that they will pass as phrase and setting to crypt_r." I think it's very wrong that we don't just keep this struct opaque to the callers (except maybe for the initialized field). We may want to fix that separately.

Let's do this in a seperate PR.

[solardiz]

I suggest we don't zeroize reserved. This serves no purpose with current libxcrypt, and it could unnecessarily limit how we're able to use the reserved space in future/mixed versions.

[solardiz]

Oh, I didn't realize you merged this already. Nevermind, then.

[besser82]

I suggest we don't zeroize reserved. This serves no purpose with current libxcrypt, and it could unnecessarily limit how we're able to use the reserved space in future/mixed versions.

I think we can drop it, if we decide to use that field to store a state or sth. in later versions.