Обсуждение: pgsql: Replace PostmasterRandom() with a stronger source, second attemp
Replace PostmasterRandom() with a stronger source, second attempt. This adds a new routine, pg_strong_random() for generating random bytes, for use in both frontend and backend. At the moment, it's only used in the backend, but the upcoming SCRAM authentication patches need strong random numbers in libpq as well. pg_strong_random() is based on, and replaces, the existing implementation in pgcrypto. It can acquire strong random numbers from a number of sources, depending on what's available: - OpenSSL RAND_bytes(), if built with OpenSSL - On Windows, the native cryptographic functions are used - /dev/urandom Unlike the current pgcrypto function, the source is chosen by configure. That makes it easier to test different implementations, and ensures that we don't accidentally fall back to a less secure implementation, if the primary source fails. All of those methods are quite reliable, it would be pretty surprising for them to fail, so we'd rather find out by failing hard. If no strong random source is available, we fall back to using erand48(), seeded from current timestamp, like PostmasterRandom() was. That isn't cryptographically secure, but allows us to still work on platforms that don't have any of the above stronger sources. Because it's not very secure, the built-in implementation is only used if explicitly requested with --disable-strong-random. This replaces the more complicated Fortuna algorithm we used to have in pgcrypto, which is unfortunate, but all modern platforms have /dev/urandom, so it doesn't seem worth the maintenance effort to keep that. pgcrypto functions that require strong random numbers will be disabled with --disable-strong-random. Original patch by Magnus Hagander, tons of further work by Michael Paquier and me. Discussion: https://www.postgresql.org/message-id/CAB7nPqRy3krN8quR9XujMVVHYtXJ0_60nqgVc6oUk8ygyVkZsA@mail.gmail.com Discussion: https://www.postgresql.org/message-id/CAB7nPqRWkNYRRPJA7-cF+LfroYV10pvjdz6GNvxk-Eee9FypKA@mail.gmail.com Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/fe0a0b5993dfe24e4b3bcf52fa64ff41a444b8f1 Modified Files -------------- configure | 109 +++++ configure.in | 52 +++ contrib/pgcrypto/Makefile | 2 +- contrib/pgcrypto/expected/pgp-compression_1.out | 42 ++ contrib/pgcrypto/expected/pgp-decrypt_1.out | 424 +++++++++++++++++++ contrib/pgcrypto/expected/pgp-encrypt_1.out | 161 +++++++ contrib/pgcrypto/expected/pgp-pubkey-encrypt_1.out | 62 +++ contrib/pgcrypto/fortuna.c | 463 --------------------- contrib/pgcrypto/fortuna.h | 38 -- contrib/pgcrypto/internal.c | 63 --- contrib/pgcrypto/openssl.c | 46 -- contrib/pgcrypto/pgcrypto.c | 27 +- contrib/pgcrypto/pgp-encrypt.c | 22 +- contrib/pgcrypto/pgp-mpi-internal.c | 10 +- contrib/pgcrypto/pgp-pgsql.c | 95 +---- contrib/pgcrypto/pgp-pubenc.c | 26 +- contrib/pgcrypto/pgp-s2k.c | 15 +- contrib/pgcrypto/px-crypt.c | 7 +- contrib/pgcrypto/px.c | 34 +- contrib/pgcrypto/px.h | 7 +- contrib/pgcrypto/random.c | 247 ----------- doc/src/sgml/installation.sgml | 17 + src/Makefile.global.in | 1 + src/backend/libpq/auth.c | 62 ++- src/backend/libpq/crypt.c | 10 +- src/backend/postmaster/postmaster.c | 146 ++++--- src/backend/storage/ipc/ipci.c | 3 + src/backend/storage/lmgr/lwlocknames.txt | 1 + src/backend/utils/init/globals.c | 2 +- src/backend/utils/misc/Makefile | 5 +- src/backend/utils/misc/backend_random.c | 158 +++++++ src/include/libpq/crypt.h | 2 +- src/include/libpq/libpq-be.h | 1 - src/include/miscadmin.h | 2 +- src/include/pg_config.h.in | 12 + src/include/pg_config.h.win32 | 12 + src/include/port.h | 6 + src/include/utils/backend_random.h | 19 + src/port/Makefile | 4 + src/port/erand48.c | 7 + src/port/pg_strong_random.c | 149 +++++++ src/tools/msvc/Mkvcbuild.pm | 5 +- 42 files changed, 1472 insertions(+), 1104 deletions(-)
On 2016-12-05 11:44:37 +0000, Heikki Linnakangas wrote: > Replace PostmasterRandom() with a stronger source, second attempt. Since this went in I've seen 2016-12-06 14:42:17.005 PST [23658][] LOG: wrong key in cancel request for process 23657 2016-12-06 14:42:19.010 PST [23662][] LOG: wrong key in cancel request for process 23657 a couple times. I've not yet started debugging it. But it seems to only happen in the first (few?) queries in a new connection. Andres
On Tue, Dec 6, 2016 at 12:44 AM, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote: > src/backend/storage/lmgr/lwlocknames.txt | 1 + Missing one tab? -- Thomas Munro http://www.enterprisedb.com
Вложения
Re: pgsql: Replace PostmasterRandom() with a stronger source, second attemp
От
Michael Paquier
Дата:
On Wed, Dec 7, 2016 at 7:46 AM, Andres Freund <andres@anarazel.de> wrote: > On 2016-12-05 11:44:37 +0000, Heikki Linnakangas wrote: >> Replace PostmasterRandom() with a stronger source, second attempt. > > Since this went in I've seen > 2016-12-06 14:42:17.005 PST [23658][] LOG: wrong key in cancel request for process 23657 > 2016-12-06 14:42:19.010 PST [23662][] LOG: wrong key in cancel request for process 23657 > a couple times. > > I've not yet started debugging it. But it seems to only happen in the > first (few?) queries in a new connection. Hm. I recall testing this patch if cancel keys are working or not, but connecting with pgbench -C runnung in parallel I am seeing the following spurious error: =# select pg_sleep(10); ^CCancel request sent Time: 10001.496 ms (00:10.001) With the logs complaining as well: [1-1] db=[unknown],user=[unknown],app=[unknown],client=[local] LOG: wrong key in cancel request for process 57361 As far as I can see processCancelRequest uses a long variable, and it gets compared with an int32. MyCancelKey has been changed to an int32 in globals.c with the above commit. So on 64b machines the current code is aimed to fail. Attached is a patch to fix the issue. It is taking me at most 10 times to reproduce the failure without the patch, after trying 20~ times with the patch I am still able to cancel the queries. -- Michael
Вложения
Re: pgsql: Replace PostmasterRandom() with a stronger source, second attemp
От
Heikki Linnakangas
Дата:
On 12/07/2016 01:56 AM, Thomas Munro wrote: > On Tue, Dec 6, 2016 at 12:44 AM, Heikki Linnakangas > <heikki.linnakangas@iki.fi> wrote: >> src/backend/storage/lmgr/lwlocknames.txt | 1 + > > Missing one tab? Fixed, thanks. I missed this because my editor is set up to use 4-space tabs for .c and .h files, but not for .txt files. With 8-space tabs, it seemed to align with the previous lines. - Heikki
Re: pgsql: Replace PostmasterRandom() with a stronger source, second attemp
От
Heikki Linnakangas
Дата:
On 12/07/2016 02:30 AM, Michael Paquier wrote: > With the logs complaining as well: > [1-1] db=[unknown],user=[unknown],app=[unknown],client=[local] LOG: > wrong key in cancel request for process 57361 > > As far as I can see processCancelRequest uses a long variable, and it > gets compared with an int32. MyCancelKey has been changed to an int32 > in globals.c with the above commit. So on 64b machines the current > code is aimed to fail. Attached is a patch to fix the issue. Pushed, thanks! - Heikki