From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Subject: | Re: Removing --disable-strong-random from the code |
Date: | 2019-01-01 01:55:18 |
Message-ID: | 20190101015518.GB3243@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Dec 31, 2018 at 10:20:28AM +0900, Michael Paquier wrote:
> On Sun, Dec 30, 2018 at 11:47:03AM -0500, Tom Lane wrote:
>> 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.)
>
> Good catch. As far as I understand this code, the message is not
> included yet and random bytes are just added to avoid having 0 in the
> padding. So I agree that the memset is not really meaningful to
> have on the whole buffer. I can take care of that as well, and of
> course you get the credits. If you want to commit and back-patch the
> fix yourself, please feel free to do so.
I have fixed this one and back-patched down to 10. In what has been
committed I have kept the memset which is a logic present since
e94dd6a back from 2005. On my second lookup, the logic is correct
without it, still it felt safer to keep it.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Gierth | 2019-01-01 02:18:31 | Re: Early WIP/PoC for inlining CTEs |
Previous Message | Michael Paquier | 2019-01-01 01:36:58 | Re: Clean up some elog messages and comments for do_pg_stop_backup and do_pg_start_backup |