Re: Modern SHA2- based password hashes for pgcrypto
Alvaro Herrera <alvherre@alvh.no-ip.org>
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Bernd Helmle <mailings@oopsware.de>
Cc: Japin Li <japinli@hotmail.com>, PostgreSQL Development <pgsql-hackers@postgresql.org>
Date: 2025-04-03T18:39:18Z
Lists: pgsql-hackers
Attachments
- 0001-pgindent.patch.txt (text/plain)
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)
Commits
-
Follow-up fixes for SHA-2 patch (commit 749a9e20c).
- 969ab9d4f5d1 18.0 landed
-
Add modern SHA-2 based password hashes to pgcrypto.
- 749a9e20c979 18.0 landed