Thread: [HACKERS] Provide list of subscriptions and publications in psql's completion

Hi all,

While testing a bit the logical replication facility, I have bumped on
the fast that psql's completion does not show the list of things
already created. Attached is a patch.
Thanks,
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Feb 2, 2017 at 2:48 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Hi all,
>
> While testing a bit the logical replication facility, I have bumped on
> the fast that psql's completion does not show the list of things
> already created. Attached is a patch.

+#define Query_for_list_of_subscriptions \
+" SELECT pg_catalog.quote_ident(subname) "\
+"   FROM pg_catalog.pg_subscription "\
+"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"

Since non-superuser is not allowed to access to pg_subscription,
pg_stat_subscription should be accessed here, instead. Thought?

Regards,

-- 
Fujii Masao

On 2/2/17 12:48 PM, Fujii Masao wrote:
> +#define Query_for_list_of_subscriptions \
> +" SELECT pg_catalog.quote_ident(subname) "\
> +"   FROM pg_catalog.pg_subscription "\
> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
> 
> Since non-superuser is not allowed to access to pg_subscription,
> pg_stat_subscription should be accessed here, instead. Thought?

Arguably, you could leave it like that, assuming it fails cleanly for
nonsuperusers.  Nonsuperusers are not going to be able to run any
commands on subscriptions anyway, so there is little use for it.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2/2/17 12:48 AM, Michael Paquier wrote:
> +#define Query_for_list_of_subscriptions \
> +" SELECT pg_catalog.quote_ident(subname) "\
> +"   FROM pg_catalog.pg_subscription "\
> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"

This query should also be qualified by current database.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Feb 3, 2017 at 3:56 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 2/2/17 12:48 AM, Michael Paquier wrote:
>> +#define Query_for_list_of_subscriptions \
>> +" SELECT pg_catalog.quote_ident(subname) "\
>> +"   FROM pg_catalog.pg_subscription "\
>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>
> This query should also be qualified by current database.

Indeed. Here is an updated patch.
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Feb 3, 2017 at 3:55 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 2/2/17 12:48 PM, Fujii Masao wrote:
>> +#define Query_for_list_of_subscriptions \
>> +" SELECT pg_catalog.quote_ident(subname) "\
>> +"   FROM pg_catalog.pg_subscription "\
>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>>
>> Since non-superuser is not allowed to access to pg_subscription,
>> pg_stat_subscription should be accessed here, instead. Thought?
>
> Arguably, you could leave it like that, assuming it fails cleanly for
> nonsuperusers.  Nonsuperusers are not going to be able to run any
> commands on subscriptions anyway, so there is little use for it.

No. You can get rid of superuser privilege from the owner of the subscription
and that nonsuperuser owner can run some commands on the subscriptin.
It's a bit artificial, but you can. I'm not sure if we should add the check to
prevent the owner from becoming nonsuperuser. But if the owner always must
have a superuser privilege per the specification of logical replication,
I think that such check would be necessary.

Also I prefer to tab-complete the subscriptions even for nonsuperusers.
There are some objects that only superuser or owner can manage, but their
names are currently tab-completed even when current user is "normal" one.
So I'm afraid that handling only subscriptions differently might be more
confusing.

Regards,

-- 
Fujii Masao

On Sat, Feb 4, 2017 at 9:01 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Fri, Feb 3, 2017 at 3:56 AM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> On 2/2/17 12:48 AM, Michael Paquier wrote:
>>> +#define Query_for_list_of_subscriptions \
>>> +" SELECT pg_catalog.quote_ident(subname) "\
>>> +"   FROM pg_catalog.pg_subscription "\
>>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>>
>> This query should also be qualified by current database.
>
> Indeed. Here is an updated patch.

With this patch, when normal users type TAB after SUBSCRIPTION,
they got "permission denied" error. I don't think that's acceptable.

In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
PUBLICATION" cases, the publication defined in the publisher side should be
specified. But, with this patch, the tab-completion for PUBLICATION shows
the publications defined in local server (ie, subscriber side in those cases).
This might be confusing.

Regards,

-- 
Fujii Masao

On Sun, Feb 5, 2017 at 5:04 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> With this patch, when normal users type TAB after SUBSCRIPTION,
> they got "permission denied" error. I don't think that's acceptable.

Right, I can see that now. pg_stat_get_subscription() does not offer
as well a way to filter by databases... Perhaps we could add it? it is
stored as LogicalRepWorker->dbid so making it visible is sort of easy.

> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
> PUBLICATION" cases, the publication defined in the publisher side should be
> specified. But, with this patch, the tab-completion for PUBLICATION shows
> the publications defined in local server (ie, subscriber side in those cases).
> This might be confusing.

Which would mean that psql tab completion should try to connect the
remote server using the connection string defined with the
subscription to get this information, which looks unsafe to me. To be
honest, I'd rather see a list of local objects defined than nothing..
-- 
Michael

On Mon, Feb 6, 2017 at 1:52 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sun, Feb 5, 2017 at 5:04 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> With this patch, when normal users type TAB after SUBSCRIPTION,
>> they got "permission denied" error. I don't think that's acceptable.
>
> Right, I can see that now. pg_stat_get_subscription() does not offer
> as well a way to filter by databases... Perhaps we could add it? it is
> stored as LogicalRepWorker->dbid so making it visible is sort of easy.

Yes, that's an option. And, if we add dbid to pg_stat_subscription,
I'm tempted to add all the pg_subscription's columns except subconninfo
into pg_stat_subscription. Since subconninfo may contain security-sensitive
information, it should be excluded. But it seems useful to expose other
columns. Then, if we expose all the columns except subconninfo, maybe
it's better to just revoke subconninfo column on pg_subscription instead of
all columns. Thought?

BTW, I found that psql's \dRs command has the same problem;
the permission denied error occurs when normal user runs \dRs.
This issue should be fixed in the same way as tab-completion one.

Also I found that tab-completion for psql's meta-commands doesn't
show \dRp and \dRs.

>> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
>> PUBLICATION" cases, the publication defined in the publisher side should be
>> specified. But, with this patch, the tab-completion for PUBLICATION shows
>> the publications defined in local server (ie, subscriber side in those cases).
>> This might be confusing.
>
> Which would mean that psql tab completion should try to connect the
> remote server using the connection string defined with the
> subscription to get this information, which looks unsafe to me. To be
> honest, I'd rather see a list of local objects defined than nothing..

IMO showing nothing is better. But many people think that even local
objects should be showed in that case, I can live with that.

Regards,

-- 
Fujii Masao

On 2/5/17 11:52 PM, Michael Paquier wrote:
>> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
>> PUBLICATION" cases, the publication defined in the publisher side should be
>> specified. But, with this patch, the tab-completion for PUBLICATION shows
>> the publications defined in local server (ie, subscriber side in those cases).
>> This might be confusing.
> Which would mean that psql tab completion should try to connect the
> remote server using the connection string defined with the
> subscription to get this information, which looks unsafe to me. To be
> honest, I'd rather see a list of local objects defined than nothing..

But it's the wrong list.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2/6/17 10:54 AM, Fujii Masao wrote:
> Yes, that's an option. And, if we add dbid to pg_stat_subscription,
> I'm tempted to add all the pg_subscription's columns except subconninfo
> into pg_stat_subscription. Since subconninfo may contain security-sensitive
> information, it should be excluded. But it seems useful to expose other
> columns. Then, if we expose all the columns except subconninfo, maybe
> it's better to just revoke subconninfo column on pg_subscription instead of
> all columns. Thought?

I think previous practice is to provide a view such as pg_subscriptions
that contains all the nonprivileged information.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Feb 7, 2017 at 6:35 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 2/6/17 10:54 AM, Fujii Masao wrote:
>> Yes, that's an option. And, if we add dbid to pg_stat_subscription,
>> I'm tempted to add all the pg_subscription's columns except subconninfo
>> into pg_stat_subscription. Since subconninfo may contain security-sensitive
>> information, it should be excluded. But it seems useful to expose other
>> columns. Then, if we expose all the columns except subconninfo, maybe
>> it's better to just revoke subconninfo column on pg_subscription instead of
>> all columns. Thought?
>
> I think previous practice is to provide a view such as pg_subscriptions
> that contains all the nonprivileged information.

OK, I think that I see the point you are coming at:
pg_stat_get_subscription (or stat tables) should not be used for
psql's tab completion. So gathering all things discussed, we have:
- No tab completion for publications.
- Fix the meta-commands.
- Addition of a view pg_subscriptions with all the non-sensitive data.
(- Or really extend pg_stat_subscriptions with the database ID and use
it for tab completion?)
Am I missing something?
-- 
Michael

On Fri, Feb 10, 2017 at 1:52 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Feb 7, 2017 at 6:35 AM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> On 2/6/17 10:54 AM, Fujii Masao wrote:
>>> Yes, that's an option. And, if we add dbid to pg_stat_subscription,
>>> I'm tempted to add all the pg_subscription's columns except subconninfo
>>> into pg_stat_subscription. Since subconninfo may contain security-sensitive
>>> information, it should be excluded. But it seems useful to expose other
>>> columns. Then, if we expose all the columns except subconninfo, maybe
>>> it's better to just revoke subconninfo column on pg_subscription instead of
>>> all columns. Thought?
>>
>> I think previous practice is to provide a view such as pg_subscriptions
>> that contains all the nonprivileged information.
>
> OK, I think that I see the point you are coming at:
> pg_stat_get_subscription (or stat tables) should not be used for
> psql's tab completion. So gathering all things discussed, we have:
> - No tab completion for publications.

No, the tab-completions for ALTER/DROP PUBLICATION should show the local
publications because those commands drop and alter the local ones. OTOH,
CREATE/ALTER SUBSCRIPTOIN ... PUBLICATION should show nothing because
the remote publications in the publisher side should be specified there.

> - Fix the meta-commands.

Yes.

> - Addition of a view pg_subscriptions with all the non-sensitive data.
> (- Or really extend pg_stat_subscriptions with the database ID and use
> it for tab completion?)

Probably I failed to get Peter's point... Anyway IMO that we can expose all the
columns except the sensitive information (i.e., subconninfo field)
in pg_subscription to even non-superusers. Then we can use pg_subscription
for the tab-completion for ALTER/DROP SUBSCRIPTION.

Regards,

-- 
Fujii Masao

On Tue, Feb 14, 2017 at 2:07 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> No, the tab-completions for ALTER/DROP PUBLICATION should show the local
> publications because those commands drop and alter the local ones. OTOH,
> CREATE/ALTER SUBSCRIPTOIN ... PUBLICATION should show nothing because
> the remote publications in the publisher side should be specified there.

Doh, yes. You are right about that.

>> - Addition of a view pg_subscriptions with all the non-sensitive data.
>> (- Or really extend pg_stat_subscriptions with the database ID and use
>> it for tab completion?)
>
> Probably I failed to get Peter's point... Anyway IMO that we can expose all the
> columns except the sensitive information (i.e., subconninfo field)
> in pg_subscription to even non-superusers. Then we can use pg_subscription
> for the tab-completion for ALTER/DROP SUBSCRIPTION.

To be honest, I find subconninfo quite useful to check where a
subscription is getting its changes from, so I'd rather not touch it.
It looks as well a bit overkill to just create a new view on an object
type non-superusers cannot even use... There are already 1 view and 1
system catalog related to it, so I'd be of the opinion to let it fail
silently with a permission error and keep it as an empty list for
them.
-- 
Michael

On 2/13/17 9:36 PM, Michael Paquier wrote:
>> Probably I failed to get Peter's point... Anyway IMO that we can expose all the
>> columns except the sensitive information (i.e., subconninfo field)
>> in pg_subscription to even non-superusers. Then we can use pg_subscription
>> for the tab-completion for ALTER/DROP SUBSCRIPTION.
> To be honest, I find subconninfo quite useful to check where a
> subscription is getting its changes from, so I'd rather not touch it.
> It looks as well a bit overkill to just create a new view on an object
> type non-superusers cannot even use... There are already 1 view and 1
> system catalog related to it, so I'd be of the opinion to let it fail
> silently with a permission error and keep it as an empty list for
> them.

Why not do what we do for pg_stat_activity.current_query and leave it 
NULL for non-SU?

Even better would be if we could simply strip out password info. 
Presumably we already know how to parse the contents, so I'd think that 
shouldn't be that difficult.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

On Wed, Feb 15, 2017 at 3:18 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
> Why not do what we do for pg_stat_activity.current_query and leave it NULL for non-SU?

If subcriptions are designed to be superuser-only, it seems fair to me
to do so. Some other system SRFs do that already.

> Even better would be if we could simply strip out password info. Presumably
> we already know how to parse the contents, so I'd think that shouldn't be
> that difficult.

I thought that this was correctly clobbered... But... No that's not
the case by looking at the code. And honestly I think that it is
unacceptable to show potentially security-sensitive information in
system catalogs via a connection string. We are really careful about
not showing anything bad in pg_stat_wal_receiver, which also sets to
NULL fields for non-superusers and even clobbered values in the
printed connection string for superusers, but pg_subscription fails on
those points.

I am adding an open item on the wiki regarding that. FWIW, a patch
needs to refactor libpqrcv_check_conninfo and libpqrcv_get_conninfo so
as the connection string build of PQconninfoOption data goes through
the same process. If everybody agrees on those lines, I have no
problems in producing a patch.
-- 
Michael

On 2/13/17 12:07, Fujii Masao wrote:
> Anyway IMO that we can expose all the
> columns except the sensitive information (i.e., subconninfo field)
> in pg_subscription to even non-superusers.

You mean with column privileges?

We could probably do that.  I don't know if we have done that before on
system catalogs.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> On 2/13/17 12:07, Fujii Masao wrote:
> > Anyway IMO that we can expose all the
> > columns except the sensitive information (i.e., subconninfo field)
> > in pg_subscription to even non-superusers.
>
> You mean with column privileges?
>
> We could probably do that.  I don't know if we have done that before on
> system catalogs.

I don't believe we have, though I'm not against doing so.  I can't think
of any reason off-hand why it wouldn't work.

If we did that then perhaps we could remove some of the other views that
we have which are just to provide a subset of the columns of some other
table (eg: pg_roles)...

Thanks!

Stephen

On 15/02/17 05:56, Michael Paquier wrote:
> On Wed, Feb 15, 2017 at 3:18 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
>> Why not do what we do for pg_stat_activity.current_query and leave it NULL for non-SU?
> 
> If subcriptions are designed to be superuser-only, it seems fair to me
> to do so. Some other system SRFs do that already.
> 
>> Even better would be if we could simply strip out password info. Presumably
>> we already know how to parse the contents, so I'd think that shouldn't be
>> that difficult.
> 
> I thought that this was correctly clobbered... But... No that's not
> the case by looking at the code. And honestly I think that it is
> unacceptable to show potentially security-sensitive information in
> system catalogs via a connection string. We are really careful about
> not showing anything bad in pg_stat_wal_receiver, which also sets to
> NULL fields for non-superusers and even clobbered values in the
> printed connection string for superusers, but pg_subscription fails on
> those points.
> 

I am not following here, pg_subscription is currently superuser only
catalog, similarly to pg_user_mapping, there is no leaking.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services

On Sat, Feb 18, 2017 at 11:57 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
> On 15/02/17 05:56, Michael Paquier wrote:
>> I thought that this was correctly clobbered... But... No that's not
>> the case by looking at the code. And honestly I think that it is
>> unacceptable to show potentially security-sensitive information in
>> system catalogs via a connection string. We are really careful about
>> not showing anything bad in pg_stat_wal_receiver, which also sets to
>> NULL fields for non-superusers and even clobbered values in the
>> printed connection string for superusers, but pg_subscription fails on
>> those points.
>>
>
> I am not following here, pg_subscription is currently superuser only
> catalog, similarly to pg_user_mapping, there is no leaking.

Even if it is a superuser-only view, pg_subscription does not hide
sensitive values in connection strings while it should. See similar
discussion for pg_stat_wal_receiver here which is also superuser-only
(it does display null values for non-superusers):
https://www.postgresql.org/message-id/562f6c7f-6a47-0a8a-e189-2de9ea896849@2ndquadrant.com
Something needs to be done at least for that, independently on the
psql completion.
-- 
Michael

On 19/02/17 00:02, Michael Paquier wrote:
> On Sat, Feb 18, 2017 at 11:57 PM, Petr Jelinek
> <petr.jelinek@2ndquadrant.com> wrote:
>> On 15/02/17 05:56, Michael Paquier wrote:
>>> I thought that this was correctly clobbered... But... No that's not
>>> the case by looking at the code. And honestly I think that it is
>>> unacceptable to show potentially security-sensitive information in
>>> system catalogs via a connection string. We are really careful about
>>> not showing anything bad in pg_stat_wal_receiver, which also sets to
>>> NULL fields for non-superusers and even clobbered values in the
>>> printed connection string for superusers, but pg_subscription fails on
>>> those points.
>>>
>>
>> I am not following here, pg_subscription is currently superuser only
>> catalog, similarly to pg_user_mapping, there is no leaking.
> 
> Even if it is a superuser-only view, pg_subscription does not hide
> sensitive values in connection strings while it should. See similar

It's not a view it's system catalog which actually stores the data, how
would it hide anything?

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services

On Sun, Feb 19, 2017 at 8:05 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
> It's not a view it's system catalog which actually stores the data, how
> would it hide anything?

I have been poking at it, and yeah... I missed the fact that
pg_subcription is not a view. I thought that check_conninfo was being
called in this context only..
-- 
Michael

On Sun, Feb 19, 2017 at 9:50 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> I have been poking at it, and yeah... I missed the fact that
> pg_subcription is not a view. I thought that check_conninfo was being
> called in this context only..

Still, storing plain passwords in system catalogs is a practice that
should be discouraged as base backup data can go over a network as
well... At least adding a note or a warning in the documentation would
be nice about the fact that any kind of security-sensitive data should
be avoided here.
-- 
Michael

On 19/02/17 12:03, Magnus Hagander wrote:
> 
> 
> On Sun, Feb 19, 2017 at 2:01 AM, Michael Paquier
> <michael.paquier@gmail.com <mailto:michael.paquier@gmail.com>> wrote:
> 
>     On Sun, Feb 19, 2017 at 9:50 AM, Michael Paquier
>     <michael.paquier@gmail.com <mailto:michael.paquier@gmail.com>> wrote:
>     > I have been poking at it, and yeah... I missed the fact that
>     > pg_subcription is not a view. I thought that check_conninfo was being
>     > called in this context only..
> 
>     Still, storing plain passwords in system catalogs is a practice that
>     should be discouraged as base backup data can go over a network as
>     well... At least adding a note or a warning in the documentation would
>     be nice about the fact that any kind of security-sensitive data should
>     be avoided here.
> 
> 
> Isn't that moving the goalposts quite a bit? We already allow passwords
> in CREATE USER MAPPING without any warnings against it (in fact, we
> suggest that's what you should do), which is a similar situation. Same
> goes for dblink.
> 
> If password auth is used, we have to store the password in plaintext
> equivalent somewhere. Meaning it's by definition going to be exposed to
> superusers and replication downstreams. Or are you suggesting a scheme
> whereby you have to enter all your subscription passwords in a prompt of
> some kind when starting the postmaster, to avoid it?
> 

The subscriptions will happily use .pgpass for example so it's not like
users are forced to put password to catalog (well barring some DBaaS
solutions). But I guess it would not hurt to give extra notice in docs
about dangers of the various catalogs storing passwords.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services

On Sun, Feb 19, 2017 at 8:03 PM, Magnus Hagander <magnus@hagander.net> wrote:
> If password auth is used, we have to store the password in plaintext
> equivalent somewhere. Meaning it's by definition going to be exposed to
> superusers and replication downstreams.

Another possibility is to mention the use of the new passfile
parameter for connection strings in the docs... This removes the need
to have plain passwords directly stored in the database. Not sure if
that's better though because that still mean that the password is
present in plain format somewhere.

> Or are you suggesting a scheme
> whereby you have to enter all your subscription passwords in a prompt of
> some kind when starting the postmaster, to avoid it?

Thinking about that now we could have a connection string parameter
called for example password_command, that could be used for example
with gpg2 to get back a decrypted password value.
-- 
Michael

On Sun, Feb 19, 2017 at 6:13 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sun, Feb 19, 2017 at 8:03 PM, Magnus Hagander <magnus@hagander.net> wrote:
>> If password auth is used, we have to store the password in plaintext
>> equivalent somewhere. Meaning it's by definition going to be exposed to
>> superusers and replication downstreams.
>
> Another possibility is to mention the use of the new passfile
> parameter for connection strings in the docs... This removes the need
> to have plain passwords directly stored in the database. Not sure if
> that's better though because that still mean that the password is
> present in plain format somewhere.

The real solution to "the password is present in plain form somewhere"
is probably "don't use passwords for authentication".  Because,
ultimately, a password by its nature has to exist in plain form
somewhere, at least in someone's brain, and very likely in their
password manager or the post-it stuck to their desk or the Notes app
on their iPhone or similar.  If the password is simple enough that the
DBA can be certain of remembering it without any sort of memory aid,
it's probably dumb simple.  If the DBA has few enough distinct
passwords that he doesn't need a memory aid just on the basis of sheer
volume of passwords needing to be remembered, that's probably not good
either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Sun, Feb 19, 2017 at 6:13 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > On Sun, Feb 19, 2017 at 8:03 PM, Magnus Hagander <magnus@hagander.net> wrote:
> >> If password auth is used, we have to store the password in plaintext
> >> equivalent somewhere. Meaning it's by definition going to be exposed to
> >> superusers and replication downstreams.
> >
> > Another possibility is to mention the use of the new passfile
> > parameter for connection strings in the docs... This removes the need
> > to have plain passwords directly stored in the database. Not sure if
> > that's better though because that still mean that the password is
> > present in plain format somewhere.
>
> The real solution to "the password is present in plain form somewhere"
> is probably "don't use passwords for authentication".  Because,
> ultimately, a password by its nature has to exist in plain form
> somewhere, at least in someone's brain, and very likely in their
> password manager or the post-it stuck to their desk or the Notes app
> on their iPhone or similar.  If the password is simple enough that the
> DBA can be certain of remembering it without any sort of memory aid,
> it's probably dumb simple.  If the DBA has few enough distinct
> passwords that he doesn't need a memory aid just on the basis of sheer
> volume of passwords needing to be remembered, that's probably not good
> either.

I agree that the use of plaintext passwords should be discouraged, but
we don't actually provide any way to address these kinds of proxy
authentication issues today except to store *some* secret somewhere on
the system which, if compromised, would allow the attacker to gain
access to the downstream system.

There are solutions to authentication proxying and it would be great if
we would implement them (Kerberos Delegation, for example, and there's a
mechanism to do something similar in TLS also, iirc, and we should be
able to come up with something for same-system cross-database or
cross-cluster unix-socket based auth, really...), but I don't think
anyone is currently working on implementing them, which is unfortunate.

I tend to agree with Michael that we should make it very clear in
our documentation that the password/private key/whatever has to be
stored unencrypted on the filesystem somewhere today.  Perhaps it
doesn't need to be a big "WARNING WARNING" type of message, but it's
important information which we should make sure the user is aware of.

If we do implement proper proxy authentication, we will want to include
documentation as to just what that means regarding attacks too (Kerberos
Delegation, for example, usually means that a ticket for the remote
service is stored somewhere and is valid for a period of time, meaning
that a privileged attacker who has access to the system at the same time
the user is using the system would be able to gain access to the remote
system by stealing the delegated ticket).

Thanks!

Stephen

On Fri, Feb 17, 2017 at 11:17 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 2/13/17 12:07, Fujii Masao wrote:
>> Anyway IMO that we can expose all the
>> columns except the sensitive information (i.e., subconninfo field)
>> in pg_subscription to even non-superusers.
>
> You mean with column privileges?

Yes.

So there are several approaches...

1) Expose all the columns except subconninfo in pg_subscription to   non-superusers. In this idea, the tab-completion
andpsql meta-command   for subscription still sees pg_subscription. One good point of   idea is that even
non-superuserscan see all the useful information   about subscriptions other than sensitive information like conninfo.
 

2) Change pg_stat_subscription so that it also shows all the columns except   subconninfo in pg_subscription. Also
changethe tab-completion and   psql meta-command for subscription so that they see pg_stat_subscription   instead of
pg_subscription.One good point is that pg_stat_subscription   exposes all the useful information about subscription to
even  non-superusers. OTOH, pg_subscription and pg_stat_subscription have   to have several same columns. This would be
redundantand a bit confusing.
 

3) Expose subdbid in pg_stat_subscription. Also change the tab-completion   and psql meta-command for subscription so
thatthey see   pg_stat_subscription. This change is very simple. But non-superusers cannot   see useful information
likesubslotname because of privilege problem.
 

I like #1, but any better approach?

Regards,

-- 
Fujii Masao

Greetings,

* Fujii Masao (masao.fujii@gmail.com) wrote:
> On Fri, Feb 17, 2017 at 11:17 PM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
> > On 2/13/17 12:07, Fujii Masao wrote:
> >> Anyway IMO that we can expose all the
> >> columns except the sensitive information (i.e., subconninfo field)
> >> in pg_subscription to even non-superusers.
> >
> > You mean with column privileges?
>
> Yes.
>
> So there are several approaches...
>
> 1) Expose all the columns except subconninfo in pg_subscription to
>     non-superusers. In this idea, the tab-completion and psql meta-command
>     for subscription still sees pg_subscription. One good point of
>     idea is that even non-superusers can see all the useful information
>     about subscriptions other than sensitive information like conninfo.
>
> 2) Change pg_stat_subscription so that it also shows all the columns except
>     subconninfo in pg_subscription. Also change the tab-completion and
>     psql meta-command for subscription so that they see pg_stat_subscription
>     instead of pg_subscription. One good point is that pg_stat_subscription
>     exposes all the useful information about subscription to even
>     non-superusers. OTOH, pg_subscription and pg_stat_subscription have
>     to have several same columns. This would be redundant and a bit confusing.
>
> 3) Expose subdbid in pg_stat_subscription. Also change the tab-completion
>     and psql meta-command for subscription so that they see
>     pg_stat_subscription. This change is very simple. But non-superusers cannot
>     see useful information like subslotname because of privilege problem.
>
> I like #1, but any better approach?

#1 seems alright to me, at least.  We could start using column-level
privs in other places also, as I mentioned up-thread.

I don't particularly like the suggestions to have psql use pg_stat_X
views or to put things into pg_stat_X views which are object definitions
for non-superusers.  If we really don't want to use column-level
privileges then we should have an appropriate view create instead which
provides non-superusers with the non-sensitive object information.

Thanks!

Stephen

On Tue, Feb 21, 2017 at 12:48 AM, Stephen Frost <sfrost@snowman.net> wrote:
> * Fujii Masao (masao.fujii@gmail.com) wrote:
>> On Fri, Feb 17, 2017 at 11:17 PM, Peter Eisentraut
>> 1) Expose all the columns except subconninfo in pg_subscription to
>>     non-superusers. In this idea, the tab-completion and psql meta-command
>>     for subscription still sees pg_subscription. One good point of
>>     idea is that even non-superusers can see all the useful information
>>     about subscriptions other than sensitive information like conninfo.
>>
>> 2) Change pg_stat_subscription so that it also shows all the columns except
>>     subconninfo in pg_subscription. Also change the tab-completion and
>>     psql meta-command for subscription so that they see pg_stat_subscription
>>     instead of pg_subscription. One good point is that pg_stat_subscription
>>     exposes all the useful information about subscription to even
>>     non-superusers. OTOH, pg_subscription and pg_stat_subscription have
>>     to have several same columns. This would be redundant and a bit confusing.
>>
>> 3) Expose subdbid in pg_stat_subscription. Also change the tab-completion
>>     and psql meta-command for subscription so that they see
>>     pg_stat_subscription. This change is very simple. But non-superusers cannot
>>     see useful information like subslotname because of privilege problem.
>>
>> I like #1, but any better approach?
>
> #1 seems alright to me, at least.  We could start using column-level
> privs in other places also, as I mentioned up-thread.

Among the three, this looks like a good one.

> I don't particularly like the suggestions to have psql use pg_stat_X
> views or to put things into pg_stat_X views which are object definitions
> for non-superusers.  If we really don't want to use column-level
> privileges then we should have an appropriate view create instead which
> provides non-superusers with the non-sensitive object information.

Neither do I, those are by definition tables for statistics. Having
tab completion depend on them is not right.

So what do you think about the patch attached? This does the following:
- complete subscription list for CREATE/ALTER SUBSCRIPTION
- complete publication list for CREATE/ALTER PUBLICATION
- complete to nothing for PUBLICATION in CREATE/ALTER SUBSCRIPTION as
this refers to remote objects defined in subconninfo.
- authorize read access to public for all columns of pg_subscription
except subconninfo
Thoughts?
-- 
Michael

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 21/02/17 07:50, Michael Paquier wrote:
> On Tue, Feb 21, 2017 at 12:48 AM, Stephen Frost <sfrost@snowman.net> wrote:
>> * Fujii Masao (masao.fujii@gmail.com) wrote:
>>> On Fri, Feb 17, 2017 at 11:17 PM, Peter Eisentraut
>>> 1) Expose all the columns except subconninfo in pg_subscription to
>>>     non-superusers. In this idea, the tab-completion and psql meta-command
>>>     for subscription still sees pg_subscription. One good point of
>>>     idea is that even non-superusers can see all the useful information
>>>     about subscriptions other than sensitive information like conninfo.
>>>
>>> 2) Change pg_stat_subscription so that it also shows all the columns except
>>>     subconninfo in pg_subscription. Also change the tab-completion and
>>>     psql meta-command for subscription so that they see pg_stat_subscription
>>>     instead of pg_subscription. One good point is that pg_stat_subscription
>>>     exposes all the useful information about subscription to even
>>>     non-superusers. OTOH, pg_subscription and pg_stat_subscription have
>>>     to have several same columns. This would be redundant and a bit confusing.
>>>
>>> 3) Expose subdbid in pg_stat_subscription. Also change the tab-completion
>>>     and psql meta-command for subscription so that they see
>>>     pg_stat_subscription. This change is very simple. But non-superusers cannot
>>>     see useful information like subslotname because of privilege problem.
>>>
>>> I like #1, but any better approach?
>>
>> #1 seems alright to me, at least.  We could start using column-level
>> privs in other places also, as I mentioned up-thread.
> 
> Among the three, this looks like a good one.
> 
>> I don't particularly like the suggestions to have psql use pg_stat_X
>> views or to put things into pg_stat_X views which are object definitions
>> for non-superusers.  If we really don't want to use column-level
>> privileges then we should have an appropriate view create instead which
>> provides non-superusers with the non-sensitive object information.
> 
> Neither do I, those are by definition tables for statistics. Having
> tab completion depend on them is not right.
> 
> So what do you think about the patch attached? This does the following:
> - complete subscription list for CREATE/ALTER SUBSCRIPTION
> - complete publication list for CREATE/ALTER PUBLICATION
> - complete to nothing for PUBLICATION in CREATE/ALTER SUBSCRIPTION as
> this refers to remote objects defined in subconninfo.
> - authorize read access to public for all columns of pg_subscription
> except subconninfo
> Thoughts?
> 

Maybe it would make sense to also have pg_subscriptions view which
selects everything but subconninfo (or does some removal of sensitive
info there, if we have function for that)? The reason why I am thinking
about this is that, it's much more convenient to do SELECT * than
specifying all columns you have access to.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services

On 2/21/17 01:50, Michael Paquier wrote:
> So what do you think about the patch attached? This does the following:
> - complete subscription list for CREATE/ALTER SUBSCRIPTION
> - complete publication list for CREATE/ALTER PUBLICATION
> - complete to nothing for PUBLICATION in CREATE/ALTER SUBSCRIPTION as
> this refers to remote objects defined in subconninfo.
> - authorize read access to public for all columns of pg_subscription
> except subconninfo

committed

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services