From: | Bernd Helmle <mailings(at)oopsware(dot)de> |
---|---|
To: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Japin Li <japinli(at)hotmail(dot)com> |
Cc: | PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Modern SHA2- based password hashes for pgcrypto |
Date: | 2025-03-11 16:58:02 |
Message-ID: | e1590224d57bafb3caeb9b378daed1517e0e7357.camel@oopsware.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
> I definitely like that passlib have documented their thought process
> thoroughly.
>
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.
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 think using their strict mode is good on principle, but if we're
> going
> to do that, then the salt string should not be used verbatim, but
> instead base64-decoded first to get the actual salt bytes, like they
> do.
> Does this break compabitibility with other systems? Are
> passlib-generated password hashes incompatible with, say, "openssl
> passwd" which you (Bernd) mentioned at the beginning of the thread?
> Maybe if the password hashes are no longer compatible, then we should
> ditch the idea of restricting salts to base64 chars and accept the
> whole
> range of bytes, like Drepper.
>
> But in any case ISTM we should reject, as they suggest, the use of
> less
> than 4 bytes of salt (and should perhaps settle for a default of 16,
> as
> passlib suggests). I suppose this is why passlib returns NULL with
> empty salt. What we should do in that case IMO is ereport(ERROR).
>
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:
Empty salt
----------
# Drepper reference code
shacrypt password "" sha512crypt
$6$$bLTg4cpho8PIUrjfsE7qlU08Qx2UEfw..xOc6I1wpGVtyVYToGrr7BzRdAAnEr5lYFr
1Z9WcCf1xNZ1HG9qFW1
# Patch
SELECT crypt('password', '$6$$');
crypt
───────────────────────────────────────────────────────────────────────
─────────────────────
$6$$bLTg4cpho8PIUrjfsE7qlU08Qx2UEfw..xOc6I1wpGVtyVYToGrr7BzRdAAnEr5lYFr1
Z9WcCf1xNZ1HG9qFW1
(1 row)
# Passlib
from passlib.hash import sha256_crypt,sha512_crypt
hash = sha512_crypt.using(salt='').using(rounds=5000).hash("password")
print(hash)
$6$$bLTg4cpho8PIUrjfsE7qlU08Qx2UEfw..xOc6I1wpGVtyVYToGrr7BzRdAAnEr5lYFr
1Z9WcCf1xNZ1HG9qFW1
# OpenSSL
echo password | openssl passwd -6 -stdin -salt ''
<NULL>
Short salt
----------
# Drepper reference code
./shacrypt password abcdef sha512crypt
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH
1SXexabbdNXXOO11F7sdyurk/
# Patch
SELECT crypt('password', '$6$abcdef$');
crypt
───────────────────────────────────────────────────────────────────────
───────────────────────────
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH1
SXexabbdNXXOO11F7sdyurk/
(1 row)
# Passlib
from passlib.hash import sha256_crypt,sha512_crypt
hash =
sha512_crypt.using(salt='abcdef').using(rounds=5000).hash("password")
print(hash)
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH
1SXexabbdNXXOO11F7sdyurk/
# OpenSSL
echo password | openssl passwd -6 -stdin -salt 'abcdef'
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH
1SXexabbdNXXOO11F7sdyurk/
Salt > MAXLEN(16 Bytes)
-----------------------
# Drepper reference code
(truncates salt)
shacrypt password abcdefghijklmnopqrstuvwxyz sha512crypt
$6$abcdefghijklmnop$0aenUFHf897F9u0tURIHOeACWajSuVGa7jgJGyq.DKZm/WXl/IZ
FvPbneFydBjomEOgM.Sh1m0L3KsS1.H5b//
# Patch
(truncates salt)
SELECT crypt('password', '$6$abcdefghijklmnopqrstuvwxyz$');
crypt
───────────────────────────────────────────────────────────────────────
─────────────────────────────────────
$6$abcdefghijklmnop$0aenUFHf897F9u0tURIHOeACWajSuVGa7jgJGyq.DKZm/WXl/IZF
vPbneFydBjomEOgM.Sh1m0L3KsS1.H5b//
(1 row)
# Passlib
Errors out with
ValueError: salt too large (sha512_crypt requires <= 16 chars)
# OpenSSL
(truncates salt)
echo password | openssl passwd -6 -stdin -salt
'abcdefghijklmnopqrstuvwxyz'
$6$abcdefghijklmnop$0aenUFHf897F9u0tURIHOeACWajSuVGa7jgJGyq.DKZm/WXl/IZ
FvPbneFydBjomEOgM.Sh1m0L3KsS1.H5b//
Salt with "invalid" character
-----------------------------
# Drepper reference code
./shacrypt password abcdefghi%jklmno sha512crypt
$6$abcdefghi%jklmno$LMN/V1pW97IoK0rWSDVqCo9EYd6zpqP0TdTX9.cxFAFqsdSMWQM
jehkmMtDzL36VBKeG6dg.kFAQKoFvZpK0G.
# Patch
current version V4 (attached)
Errors out:
SELECT crypt('password', '$6$abcdefghi%jklmno$');
FEHLER: invalid character in salt string: "%"
Time: 0,217 ms
# Passlib
Errors out
ValueError: invalid characters in sha512_crypt salt
# OpenSSL
echo password | openssl passwd -6 -stdin -salt 'abcdefghi%jklmno'
$6$abcdefghi%jklmno$LMN/V1pW97IoK0rWSDVqCo9EYd6zpqP0TdTX9.cxFAFqsdSMWQM
jehkmMtDzL36VBKeG6dg.kFAQKoFvZpK0G.
So the hashes we produce are identical, but with being more strict we
differ in the handling of the provided salt.
Bernd
Attachment | Content-Type | Size |
---|---|---|
0001-Add-modern-SHA-2-based-password-hashes-to-pgcrypto-v4.patch | text/x-patch | 43.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Nathan Bossart | 2025-03-11 17:00:01 | Re: vacuumdb changes for stats import/export |
Previous Message | Dean Rasheed | 2025-03-11 16:32:09 | Re: Wrong results with subquery pullup and grouping sets |