Re: exposing pg_controldata and pg_config as functions

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/24/2015 08:52 AM, Joe Conway wrote:
> On 08/24/2015 06:50 AM, Tom Lane wrote:
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>> On 08/23/2015 08:58 PM, Michael Paquier wrote:
>>>> I think that's a good thing to have, now I have concerns
>>>> about making this data readable for non-superusers. Cloud
>>>> deployments of Postgres are logically going to block the
>>>> access of this view.
>
>>> I don't think it exposes any information of great security
>>> value.
>
>> We just had that kerfuffle about whether WAL compression posed a
>> security risk; doesn't that imply that at least the data
>> relevant to WAL position has to be unreadable by non-superusers?
>
> So pg_config might be fully unrestricted, but pg_controldata might
> need certain rows filtered based on superuser status? Do you think
> those rows should be present but redacted, or completely filtered
> out?

Here is the next installment on this. It addresses most of the
previous comments, but does not yet address the issue raised here by Tom
.

Changes:
1.) pg_controldata() function and pg_controldata view added

2.) cleanup_path() moved to port/path.c

3.) extra PG_FUNCTION_INFO_V1() noise removed

Issues needing comment:
a.) Which items need hiding from non-superusers and should the value be
redacted or the entire result set row be suppressed?

b.) There is a difference in the formatting of timestamps between the
pg_controldata binary and the builtin function. To see it do:

diff -c <(pg_controldata) \
<(psql -qAt -c "select rpad(name || ':', 38) || setting
from pg_controldata")

What I see is:

pg_controldata
! pg_control last modified: Tue 25 Aug 2015 08:10:42 PM PDT
pg_controldata()
! pg_control last modified: Tue Aug 25 20:10:42 2015

Does it matter?

c.) There is some common code between pg_controldata.c in bin and
pg_controldata.c in backend/utils/misc. Specifically the string
definitions in printf statements match those in ControlData[], and
dbState() and wal_level_str() are in both places. Any opinions
on consolidating them in src/common somewhere?

d.) Still no docs yet - will get to it eventually.

Thanks,

Joe

- --
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJV3TNnAAoJEDfy90M199hl0OsQAIyTr0hqxwjrGenDpnS4qE8u
UJWVeCpqehFIobxcJ0TICTaQX835fzPGJIiTUwI1Dmz9sgjipvSG1wBmD4bRT93X
WO4e/+Yr5onZ9vNVXlUswPK2kuzehImhmzMSnJ6KDXKkfw2MOZmz56wBb3yIB3lq
K44FDukZ01w9RCGM8H5B/MPNMHIqfBB4wdlKARJZhqeUyPvTJ71iMiqeE77v3AIH
JLmW6kRw8c3NVu/Wa+GVz4FGjIZKR5oazlFYfDTeHXrxV8NIDUFNrKikAW1ScdVK
qSPVjFxoUlbX4W2dd1L1ciGeq83DktYbdKtpZZScQGXwhuq7Y1fHZQwzlxlraB/c
UiqNdxmi7IeUdOIncsKPDmjs7C5yeNj1CRnWHTAQRW98RM42A3TvT2Qlkxm0CVLQ
lZjFVOOMIf4pXYQv6PfiicO6QWYTUSXCa891s/10H2xkS/sMK1yHz3DWSZxVdDdI
dbh5gie/GFro1nwWd8gjkn5KCe917GDBAUBn+QE5TgUPnRhserq6FQBSyVXfZtOQ
o6nRM8vuv9Y06CRoeIgagtDWxippl0OAw442wHyme/PBQZ2842PW8GNNqw+B1HWz
Ir0V5FiZdLLQipwiKT152+8OsOa/NU6wxGFuJr8It/4471h3jU5dxuHO+wQqMDEb
xCn6ebwZaa9oSjHFrfy3
=oMOO
-----END PGP SIGNATURE-----