On 2025-Mar-11, Bernd Helmle wrote:
> Please find attached v4 of this patch. I added the following changes:
>
> - Check for non-supported characters in the salt like passlib does.
> - Check for reserved tokens when parsing the salt string (i find this
> very strict, but it covers the cases Japin Li pointed out).
>
> I didn't implement being more strict when it comes to the salt length:
> the salt might be provided from external sources and the password hash
> is just used for validating within the database.
>
> When hashing the password within postgres, users always should use
> gen_salt(), which generates a reasonable salt string.
This sounds reasonable to me.
> Maybe, in case of empty salts, we should issue a WARNING instead of
> erroring out and put additional documentation on how to use it right.
I don't know, that doesn't seem ideal to me, because it's very easy to
run stuff and never see the warnings. If we find that people are
desperate to use empty salts, we can relax that later (turn the error to
a warning), but I'd rather not have it in the first cut.
> Hmm, i didn't understand that passlib does decode them first, i thought
> they use it encoded... at least, in our current form we're pretty much
> compatible with Drepper, passlib and OpenSSL, as far as i tested:
I am ready to believe that I misinterpreted what I read.
> So the hashes we produce are identical, but with being more strict we
> differ in the handling of the provided salt.
Okay.
I can offer a few cosmetic changes. 0001 is a pgindent run, and 0002 is
some manual adjustments after that. There are only two nontrivial
changes
1. the calculation for rounds was using type long, which is confusing
because the range is different according to the platform. Since it's
limited by the macro definitions to no more than 999999999, we can make
it an int32 instead. So we use strtoint() instead of strtoul() to parse
the value, and remove the "l" suffixes from the macros that define the
limits and default, which were bugging me a bit when used in the
gen_list struct.
2. I changed "if (block % 3)" to "if ((block % 3) != 0)".
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"El destino baraja y nosotros jugamos" (A. Schopenhauer)
