public inbox for [email protected]  

help / color / mirror / Atom feed

From: Bernd Helmle <[email protected]>
To: Tom Lane <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Japin Li <[email protected]>
Cc: PostgreSQL Development <[email protected]>
Subject: Re: Modern SHA2- based password hashes for pgcrypto
Date: Mon, 07 Apr 2025 09:03:30 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>

Am Sonntag, dem 06.04.2025 um 15:43 -0400 schrieb Tom Lane:
> What this is on about is that portable use of isalpha() or isdigit()
> requires casting a "char" value to "unsigned char".  I was about to
> make that simple change when I started to question if we actually
> want to be using <ctype.h> here at all.  Do you REALLY intend that
> any random non-ASCII letter is okay to include in the decoded_salt?
> Are you okay with that filter being locale-dependent?
> 
> I'd be more comfortable with a check like
> 
> 	if (strchr("...valid chars...", *ep) != NULL)
> 

Hmm, the idea was to allow a wider range of letters for the salt. This
came from my experiments that openssl allows more than just the ones
from _crypt_itoa64. But after reading this, i realized even isalpha()
isn't enough, since e.g. mulitbyte character wouldn't work anyways,
like openssl allows:

echo -n password | openssl passwd -5 -salt ÄÖÜ -stdin
$5$ÄÖÜ$NduBUgCdzvnW1lW8EMxW9DuxN6HmT0niS7H4ftRxuX0

So we would have to test for these cases, too. I looked again into
pyhon's passlib, and they don't accept any non-ASCII fancy characters,
neither.

So i think Tom has a point, we can restrict this to the characters from
_crypt_itoa64 and go that route. I am not sure about externally
generated hashes which are going to be stored in postgres and validated
later there. This can restrict the use case perhaps.

> It looks like "_crypt_itoa64" might be directly usable as the
> valid-chars string, too.  (BTW, why is _crypt_itoa64 not
> marked const?)

That's an oversight by me.

I can create a patch with the fixes (and the one Andres' reported) for
today.

Thanks

	Bernd

view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Modern SHA2- based password hashes for pgcrypto
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox