Date: Tue, 14 Dec 2021 18:36:54 +0100
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: "James Pang (chaolpan)" <chaolpan@cisco.com>,
	"pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #17326: Postgres crashed when pg_reload_conf()  with ssl
 certificate parameters
Message-ID: <20211214173654.i54n54e7v73dvflj@localhost>
References: <17326-d40bbb1386b6c9c3@postgresql.org>
 <PH0PR11MB51915EF4E577B83F664F9AD8D66F9@PH0PR11MB5191.namprd11.prod.outlook.com>
 <20211208134601.gauysdttc65aoeze@localhost>
 <PH0PR11MB5191DFBF6685E3EF9632EAF0D6709@PH0PR11MB5191.namprd11.prod.outlook.com>
 <PH0PR11MB5191F4B6C7B80F2DC2BC5F89D6719@PH0PR11MB5191.namprd11.prod.outlook.com>
 <20211210142231.fvldmwwy4fr477h2@localhost>
 <PH0PR11MB51910828B552926204514891D6749@PH0PR11MB5191.namprd11.prod.outlook.com>
 <YbcqQXfYRhzCgud2@paquier.xyz>
 <20211214154604.cfmv4kpo2inkt2pp@localhost>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211214154604.cfmv4kpo2inkt2pp@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/20211214173654.i54n54e7v73dvflj%40localhost>
Precedence: bulk

> On Tue, Dec 14, 2021 at 04:46:04PM +0100, Dmitry Dolgov wrote:
> > On Mon, Dec 13, 2021 at 08:10:57PM +0900, Michael Paquier wrote:
> > On Mon, Dec 13, 2021 at 07:06:16AM +0000, James Pang (chaolpan) wrote:
> > > Edit postgresql.conf to change ssl_certificate parameter ,
> >
> > Do you mean ssl_cert_file here?  Also, something that's not completely
> > clear to me is if this is a problem with a vanilla PostgreSQL
> > instance or if this is related to the pgaudit extension set_user, as
> > it has been mentioned as one potential origin of the problem upthread,
> > but you are not telling if this is the case here.  So what do you have
> > for shared_preload_libraries in this crash?
> >
> > > #9  0x00007ff49a78059c in ssl_cert_clear_certs () from /lib64/libssl.so.1.1
> > > #10 0x00007ff49a780645 in ssl_cert_free () from /lib64/libssl.so.1.1
> > > #11 0x00007ff49a78a25c in SSL_CTX_free () from /lib64/libssl.so.1.1
> > > #12 0x000000000068b6b8 in be_tls_init ()
> > > #13 0x00000000007271e1 in SIGHUP_handler ()
> >
> > Why is secure_initialize() not showing up in this stack?  That would
> > be the caller of be_tls_init() in the SIGHUP handler.  The version of
> > OpenSSL you are linking your binaries to would be useful here.  That
> > would be a 1.1.0 or a 1.1.1, no?  Any specific minor version letter?
>
> I think I can actually reproduce the issue. In my case the stack is
> fine, it contains secure_initialize, and overall it looks like some sort
> of memory corruption -- at least openssl gets segfault because it can't
> access some memory address it tries to verify in asn1_primitive_free.
> Not sure yet why, investigating.

After a short investigation looks like it's set_user problem. The
extension has duplicating set of parameters, where one is the actual set
and another one is "deprecated options". If I have both sets set
simultaneously in configuration (e.g. set_user.superuser_whitelist and
set_user.superuser_allowlist), on sighup in

    set_config_option / PGC_STRING branch / makeDefault condition

something weird happens after set_extra_field, and after this point ssl
context memory seems to be corrupted. Right before that an assign_hook
from set_user is invoked to do something around "deprecated" options,
that's why it looks suspicious. As soon as no "deprecated" options left
in the config the issue disappears.