public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tom Lane <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Bernd Helmle <[email protected]>
Cc: Japin Li <[email protected]>
Cc: PostgreSQL Development <[email protected]>
Subject: Re: Modern SHA2- based password hashes for pgcrypto
Date: Sun, 06 Apr 2025 15:43:36 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
Alvaro Herrera <[email protected]> writes:
> I have pushed this now, hoping it won't explode.
mamba is not happy:
ccache cc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -Werror -fPIC -DPIC -fvisibility=hidden -I. -I. -I../../src/include -I/usr/pkg/include/libxml2 -I/usr/pkg/include -I/usr/pkg/include -I/usr/pkg/include -c -o pgcrypto.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=char-subscripts]
324 | if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
| ^
crypt-sha.c:324:32: error: array subscript has type 'char' [-Werror=char-subscripts]
324 | if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
| ^
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) != 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
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