From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>,
        Heikki Linnakangas <hlinnaka@iki.fi>
Subject: Re: Removing --disable-strong-random from the code
In-reply-to: <20181230143740.GB2734@paquier.xyz>
References: <20181230063219.GG608@paquier.xyz>
 <23371.1546152342@sss.pgh.pa.us> <20181230071549.GA2734@paquier.xyz>
 <20181230143740.GB2734@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
	message dated "Sun, 30 Dec 2018 23:37:40 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <20185.1546188423.1@sss.pgh.pa.us>
Date: Sun, 30 Dec 2018 11:47:03 -0500
Message-ID: <20186.1546188423@sss.pgh.pa.us>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> writes:
> And attached is an updated patch with all those fixes included.  Any
> thoughts or opinions?

contrib/pgcrypto has some variant expected-files for the no-strong-random
case that could be removed now.

BackendRandomLock should be removed, too.

Since pg_strong_random is declared to take "void *", the places that
cast arguments to "char *" could be simplified.  (I guess that's a
hangover from the rather random decision to make pg_backend_random
take char *?)

The wording for pgcrypto's PXE_NO_RANDOM error,

    {PXE_NO_RANDOM, "No strong random source"},

perhaps needs to be changed --- maybe "Failed to generate strong random bits"?

Not the fault of this patch, but surely this bit in pgcrypto's
pad_eme_pkcs1_v15()

            if (!pg_strong_random((char *) p, 1))
            {
                px_memset(buf, 0, res_len);
                px_free(buf);
                break;
            }

is insane, because the "break" makes it fall into code that will continue
to scribble on "buf".  I think the "break" needs to be "return
PXE_NO_RANDOM", and probably we'd better back-patch that as a bug fix.
(I'm also failing to see the point of that px_memset before freeing the
buffer --- at this point, it contains no sensitive data, surely.)

LGTM otherwise.

			regards, tom lane