From: | Bernd Helmle <mailings(at)oopsware(dot)de> |
---|---|
To: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Cc: | Japin Li <japinli(at)hotmail(dot)com>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Modern SHA2- based password hashes for pgcrypto |
Date: | 2025-01-28 10:25:51 |
Message-ID: | 0ea9c29350e76987e7564fac5662e8a5741ddfbf.camel@oopsware.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Am Freitag, dem 24.01.2025 um 19:06 +0100 schrieb Alvaro Herrera:
> > So we behave exactly the same way as px_crypt_md5(): It stops after
> > the
> > first '$' after the magic byte preamble. For shacrypt, this could
> > be
> > the next '$' after the closing one of the non-mandatory 'rounds'
> > option, but with your example this doesn't happen since it gets
> > never
> > parsed. The salt length will be set to 0.
>
> IMO silently using no salt or 0 iterations because the input is
> somewhat
> broken is bad security and should be rejected. If we did so in the
> past
> without noticing, that's bad already, but we should not replicate
> that
> behavior any further.
Following Japin's example we parse the broken input after the magic
byte into
$6$rounds=10000$/Zg436s2vmTwsoSz
So this is what gets passed over to the next steps after extracting the
magic byte.
We then try to parse the optional rounds option, which is expected to
be at the very first characters at this stage, which isn't true. In
that case we fall back to default 5000 iterations. So no problem with
iterations here. Note that we don't output the rounds option in that
case. I adapted this from Drepper's code to be compatible with the
tests (it suppresses the rounds option in case the default is used). I
forgot to adjust the docs about this change.
But we don't extract the salt because of the first '$', which confused
the input handling that there isn't anything interesting for it. And
that get unnoticed currently, which might be a problem for the caller.
Well, you see that something is wrong when looking at the result, but
if that happens programmatically it might be hidden.
I've looked what others do:
Drepper's code with the input above produces the following:
$5$=10000$cWSkAaJa1mL912.HQqjIhODJ9b3S7hmgsb/k9Efp7.7
It parses the salt from the input as "=10000".
Python's passlib is very strict when it comes to supported characters
within a salt string. It rejects everything thats not matching '[./0-
9A-Za-z]'. So when you provide the example above you get
File "/usr/lib/python3.13/site-packages/passlib/utils/handlers.py",
line 1459, in _norm_salt
raise ValueError("invalid characters in %s salt" % cls.name)
ValueError: invalid characters in sha256_crypt salt
openssl-passwd does this:
echo test | openssl passwd -5 -stdin -salt
'$6$rounds=10000$/Zg436s2vmTwsoSz'
$5$$6$rounds=10000$$BFtTxJrvpb/ra8SnnXkiNCJ1HGZ3OOmwvyQ2TJZx44B
It absorbs everything up to 16 bytes and uses that as the salt string.
Interestingly, python-passlib and Drepper's code accept both empty
salt, openssl-passwd seems to be buggy:
echo test | openssl passwd -5 -stdin -salt ''
<NULL>
So at least, they do *something* with the input and don't silently omit
salting the input passphrase, but the results aren't the same hash.
So i think we have two options: adapt Drepper's code and throw away
everything or be very strict like python-passlib. I myself can find
support for both, so not sure.
Bernd
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Bapat | 2025-01-28 10:26:45 | Re: SQL Property Graph Queries (SQL/PGQ) |
Previous Message | Masahiko Sawada | 2025-01-28 10:25:27 | Re: Skip collecting decoded changes of already-aborted transactions |