Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps  (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1u1VuK-003eMn-0s
	for pgsql-hackers@arkaria.postgresql.org; Sun, 06 Apr 2025 19:43:52 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.94.2)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1u1VuI-001Z5R-DR
	for pgsql-hackers@arkaria.postgresql.org; Sun, 06 Apr 2025 19:43:50 +0000
Received: from makus.postgresql.org ([2001:4800:3e1:1::229])
	by malur.postgresql.org with esmtps  (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1u1VuI-001Z5J-3c
	for pgsql-hackers@lists.postgresql.org; Sun, 06 Apr 2025 19:43:50 +0000
Received: from sss.pgh.pa.us ([68.162.161.243])
	by makus.postgresql.org with esmtps  (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.96)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1u1VuF-003Kr8-1W
	for pgsql-hackers@postgresql.org;
	Sun, 06 Apr 2025 19:43:49 +0000
Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1])
	by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 536JhaFu2977906;
	Sun, 6 Apr 2025 15:43:36 -0400
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
cc: Bernd Helmle <mailings@oopsware.de>, Japin Li <japinli@hotmail.com>,
        PostgreSQL Development <pgsql-hackers@postgresql.org>
Subject: Re: Modern SHA2- based password hashes for pgcrypto
In-reply-to: <202504051722.3jv3ashzaibd@alvherre.pgsql>
References: <202504051722.3jv3ashzaibd@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@alvh.no-ip.org>
	message dated "Sat, 05 Apr 2025 19:22:58 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2977904.1743968616.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 06 Apr 2025 15:43:36 -0400
Message-ID: <2977905.1743968616@sss.pgh.pa.us>
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: <https://www.postgresql.org/message-id/2977905.1743968616%40sss.pgh.pa.us>
Precedence: bulk

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> I have pushed this now, hoping it won't explode.

mamba is not happy:

ccache cc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-s=
tatement -Werror=3Dvla -Wendif-labels -Wmissing-format-attribute -Wimplici=
t-fallthrough=3D3 -Wcast-function-type -Wshadow=3Dcompatible-local -Wforma=
t-security -fno-strict-aliasing -fwrapv -fexcess-precision=3Dstandard -Wno=
-format-truncation -Wno-stringop-truncation -g -O2 -Werror -fPIC -DPIC -fv=
isibility=3Dhidden -I. -I. -I../../src/include   -I/usr/pkg/include/libxml=
2 -I/usr/pkg/include -I/usr/pkg/include  -I/usr/pkg/include  -c -o pgcrypt=
o.o pgcrypto.c
In file included from /usr/include/ctype.h:100,
                 from ../../src/include/port.h:16,
                 from ../../src/include/c.h:1345,
                 from ../../src/include/postgres.h:48,
                 from crypt-sha.c:46:
crypt-sha.c: In function 'px_crypt_shacrypt':
crypt-sha.c:324:16: error: array subscript has type 'char' [-Werror=3Dchar=
-subscripts]
  324 |    if (isalpha(*ep) || isdigit(*ep) || (*ep =3D=3D '.') || (*ep =3D=
=3D '/'))
      |                ^
crypt-sha.c:324:32: error: array subscript has type 'char' [-Werror=3Dchar=
-subscripts]
  324 |    if (isalpha(*ep) || isdigit(*ep) || (*ep =3D=3D '.') || (*ep =3D=
=3D '/'))
      |                                ^
cc1: all warnings being treated as errors
gmake[1]: *** [<builtin>: crypt-sha.o] Error 1

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) !=3D NULL)

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

			regards, tom lane