Re: Modern SHA2- based password hashes for pgcrypto

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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-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