Re: primary_conninfo missing from pg_stat_wal_receiver

On Wed, Jun 29, 2016 at 6:42 AM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Michael Paquier wrote:
>
>> I have been thinking more about that, and came up with the following
>> idea... We do not want to link libpq directly to the server, so let's
>> add a new routine to libpqwalreceiver that builds an obfuscated
>> connection string and let's have walreceiver.c save it in shared
>> memory. Then pg_stat_wal_receiver just makes use of this string. This
>> results in a rather light patch, proposed as attached. Connection URIs
>> get as well translated as connection strings via PQconninfo(), then
>> the new interface routine of libpqwalreceiver looks at dispchar to
>> determine if it should dump a field or not and obfuscates it with more
>> or less '****'.
>
> Seems a reasonable idea to me, but some details seem a bit strange:
>
> * Why obfuscate debug options instead of skipping them?

Those are hidden in postgres_fdw/ and 'D' marks options only used for
debugging purposes or options that should not be shown. That7s why I
did so.

> * why not use PQExpBuffer?

Yes, that would be better.

> * Why have the return param be an output argument instead of a plain
> return value? i.e. static char *libpqrcv_get_conninfo(void).

Oh, yes. That's something I forgot to change. We cannot be completely
sure that the connstr will fit in MAXCONNINFO, so it makes little
sense to store the result in a pre-allocated string.

> On the security aspect of "conninfo" itself, which persists in shared
> memory: do we absolutely need to keep that data? In my reading of the
> code, it's only used once to establish the initial connection to the
> walsender, and then never afterwards. We could remove the disclosure by
> the simple expedient of overwriting that struct member with the
> obfuscated one, right after establishing that connection. Then we don't
> need an additional struct member safe_conninfo. Is there a reason why
> this wouldn't work?

[Wait a minute...]
I don't see why that would not work. By reading the code we do not
reattempt a connection, and leave WalReceiverMain if there is a
disconnection.

> I have already edited the patch following some of these ideas. Will
> post a new version later.

Cool, thanks.
--
Michael