pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
11+ messages / 7 participants
[nested] [flat]
* BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-11 20:10 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 11+ messages in thread
From: PG Bug reporting form @ 2024-03-11 20:10 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: maxim.boguk@gmail.com
The following bug has been logged on the website:
Bug reference: 18387
Logged by: Maxim Boguk
Email address: maxim.boguk@gmail.com
PostgreSQL version: 16.2
Operating system: Linux
Description:
Hi,
There are weird behavior with permission checks for refresh materialized
view for superuser.
Reproducer script (run from postgres):
create role test_role;
create database test with owner postgres;
\c test
create table test as select 1 val;
create materialized view test_mv as select * from test;
REFRESH MATERIALIZED VIEW test_mv;
create unique index test_mv_pk on test_mv(val);
REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
alter materialized view test_mv owner to test_role;
REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
select * from test;
revoke temporary on database test from public;
\c test
REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
REFRESH MATERIALIZED VIEW test_mv;
Reproducer log (starting from interesting part):
test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
REFRESH MATERIALIZED VIEW
test=# alter materialized view test_mv owner to test_role;
ALTER MATERIALIZED VIEW
test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
ERROR: permission denied for table test
--what??? N1
--check that im not hallucinating
test=# select * from test;
val
-----
1
(1 row)
test=# revoke temporary on database test from public;
REVOKE
test=# \c test
You are now connected to database "test" as user "postgres".
test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
ERROR: permission denied to create temporary tables in database "test"
--what??? N2
test=# REFRESH MATERIALIZED VIEW test_mv;
ERROR: permission denied for table test
--without CONCURRENTLY wrong behavior too
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-11 23:02 Heikki Linnakangas <hlinnaka@iki.fi>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 3 replies; 11+ messages in thread
From: Heikki Linnakangas @ 2024-03-11 23:02 UTC (permalink / raw)
To: maxim.boguk@gmail.com; pgsql-bugs@lists.postgresql.org
On 11/03/2024 22:10, PG Bug reporting form wrote:
> Reproducer log (starting from interesting part):
> test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
> REFRESH MATERIALIZED VIEW
> test=# alter materialized view test_mv owner to test_role;
> ALTER MATERIALIZED VIEW
> test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
> ERROR: permission denied for table test
> --what??? N1
>
> --check that im not hallucinating
> test=# select * from test;
> val
> -----
> 1
> (1 row)
So far, this is working correctly. REFRESH MATERIALIZED VIEW runs with
the permissions of the materialized view's owner. In this case, the
owner is 'test_role', which doesn't have select permission on the table.
> test=# revoke temporary on database test from public;
> REVOKE
> test=# \c test
> You are now connected to database "test" as user "postgres".
> test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
> ERROR: permission denied to create temporary tables in database "test"
> --what??? N2
That's interesting. REFRESH MATERIALIZED VIEW CONCURRENTLY uses
temporary tables internally, which fails if the user doesn't have
permissions to create temporary tables.
I guess we need to allow creating such internal temporary tables,
despite the missing permission. That'll need some careful analysis to
make sure we don't accidentally allow creating other temporary tables...
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-11 23:21 Bruce Momjian <bruce@momjian.us>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
2 siblings, 0 replies; 11+ messages in thread
From: Bruce Momjian @ 2024-03-11 23:21 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: maxim.boguk@gmail.com; pgsql-bugs@lists.postgresql.org
On Tue, Mar 12, 2024 at 01:02:16AM +0200, Heikki Linnakangas wrote:
> On 11/03/2024 22:10, PG Bug reporting form wrote:
> > Reproducer log (starting from interesting part):
> > test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
> > REFRESH MATERIALIZED VIEW
> > test=# alter materialized view test_mv owner to test_role;
> > ALTER MATERIALIZED VIEW
> > test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
> > ERROR: permission denied for table test
> > --what??? N1
> >
> > --check that im not hallucinating
> > test=# select * from test;
> > val
> > -----
> > 1
> > (1 row)
>
> So far, this is working correctly. REFRESH MATERIALIZED VIEW runs with the
> permissions of the materialized view's owner. In this case, the owner is
> 'test_role', which doesn't have select permission on the table.
Can we do a better job of suggesting the cause of the failure?
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-12 07:34 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
2 siblings, 2 replies; 11+ messages in thread
From: Laurenz Albe @ 2024-03-12 07:34 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; maxim.boguk@gmail.com; pgsql-bugs@lists.postgresql.org
On Tue, 2024-03-12 at 01:02 +0200, Heikki Linnakangas wrote:
> test=# \c test
> > You are now connected to database "test" as user "postgres".
> > test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
> > ERROR: permission denied to create temporary tables in database "test"
> > --what??? N2
>
> That's interesting. REFRESH MATERIALIZED VIEW CONCURRENTLY uses
> temporary tables internally, which fails if the user doesn't have
> permissions to create temporary tables.
>
> I guess we need to allow creating such internal temporary tables,
> despite the missing permission. That'll need some careful analysis to
> make sure we don't accidentally allow creating other temporary tables...
Wouldn't it be sufficient to document that fact, perhaps add an
error hint and require the MV owner to have TEMP on the database?
That's not an outrageous requirement, and it couldn't open any
security back doors.
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-12 10:40 Maxim Boguk <maxim.boguk@gmail.com>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
2 siblings, 1 reply; 11+ messages in thread
From: Maxim Boguk @ 2024-03-12 10:40 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: pgsql-bugs@lists.postgresql.org
>
> > test=# REFRESH MATERIALIZED VIEW CONCURRENTLY test_mv;
> > ERROR: permission denied for table test
> > --what??? N1
> >
> > --check that im not hallucinating
> > test=# select * from test;
> > val
> > -----
> > 1
> > (1 row)
>
> So far, this is working correctly. REFRESH MATERIALIZED VIEW runs with
> the permissions of the materialized view's owner. In this case, the
> owner is 'test_role', which doesn't have select permission on the table.
>
This decision led to a strange (and only one known to me) case when a
superuser cannot do something in the database.
(so far I have yet to see any other possible scenario when a command run by
superuser fails with permission error).
May I suggest a change to always allow superuser run REFRESH MATERIALIZED
VIEW (may be via set role or similar mechanics)?
Without that I think it's possible build a case of the database which could
be dumped but cannot be restored without errors
(restore from MV owner cannot be done because dump contains create
extension (for a sample) and restore from superuser cannot be done because
refresh MV permission check).
--
Maxim Boguk
Senior Postgresql DBA
Phone UA: +380 99 143 0000
Phone AU: +61 45 218 5678
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-12 12:22 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Maxim Boguk <maxim.boguk@gmail.com>
0 siblings, 1 reply; 11+ messages in thread
From: Laurenz Albe @ 2024-03-12 12:22 UTC (permalink / raw)
To: Maxim Boguk <maxim.boguk@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: pgsql-bugs@lists.postgresql.org
On Tue, 2024-03-12 at 12:40 +0200, Maxim Boguk wrote:
> May I suggest a change to always allow superuser run
> REFRESH MATERIALIZED VIEW (may be via set role or similar mechanics)?
If the query ran with superuser permissions, that would be
a security problem:
CREATE TABLE log (t text);
CREATE FUNCTION f() RETURNS integer LANGUAGE sql
AS 'INSERT INTO log VALUES (''x''); SELECT 42';
CREATE MATERIALIZED VIEW v AS SELECT f();
Now imagine you create a malicious trigger on "log" and
get a superuser to refresh the materialized view.
I don't see why it should be a problem if a superuser gets
"permission denied" in such a case. They can also get it if
they call a SECURITY DEFINER function owned by a non-superuser.
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 11+ messages in thread
* BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-12 13:51 Wetmore, Matthew (CTR) <Matthew.Wetmore@evernorth.com>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
1 sibling, 0 replies; 11+ messages in thread
From: Wetmore, Matthew (CTR) @ 2024-03-12 13:51 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; Heikki Linnakangas <hlinnaka@iki.fi>; maxim.boguk@gmail.com <maxim.boguk@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
> I guess we need to allow creating such internal temporary tables,
> despite the missing permission. That'll need some careful analysis to
> make sure we don't accidentally allow creating other temporary tables...
Wouldn't it be sufficient to document that fact, perhaps add an error hint and require the MV owner to have TEMP on the database?
That's not an outrageous requirement, and it couldn't open any security back doors.
Agree. We already have to create a new user (well, that’s what I do) for MV's anyway for the REFRESH by owner only, it would not be a burden to adjust that ROLE's settings at time of creation. The doco is completely clear about MV owner, we can just add to that note to make sure CREATE permission too.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-12 14:08 Tom Lane <tgl@sss.pgh.pa.us>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
1 sibling, 0 replies; 11+ messages in thread
From: Tom Lane @ 2024-03-12 14:08 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; maxim.boguk@gmail.com; pgsql-bugs@lists.postgresql.org
Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Tue, 2024-03-12 at 01:02 +0200, Heikki Linnakangas wrote:
>> I guess we need to allow creating such internal temporary tables,
>> despite the missing permission. That'll need some careful analysis to
>> make sure we don't accidentally allow creating other temporary tables...
> Wouldn't it be sufficient to document that fact, perhaps add an
> error hint and require the MV owner to have TEMP on the database?
+1. I don't see a need for this to be a low-permissions feature.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-13 17:08 Bruce Momjian <bruce@momjian.us>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 11+ messages in thread
From: Bruce Momjian @ 2024-03-13 17:08 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Maxim Boguk <maxim.boguk@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
On Tue, Mar 12, 2024 at 01:22:33PM +0100, Laurenz Albe wrote:
> On Tue, 2024-03-12 at 12:40 +0200, Maxim Boguk wrote:
> > May I suggest a change to always allow superuser run
> > REFRESH MATERIALIZED VIEW (may be via set role or similar mechanics)?
>
> If the query ran with superuser permissions, that would be
> a security problem:
>
> CREATE TABLE log (t text);
>
> CREATE FUNCTION f() RETURNS integer LANGUAGE sql
> AS 'INSERT INTO log VALUES (''x''); SELECT 42';
>
> CREATE MATERIALIZED VIEW v AS SELECT f();
>
> Now imagine you create a malicious trigger on "log" and
> get a superuser to refresh the materialized view.
>
>
> I don't see why it should be a problem if a superuser gets
> "permission denied" in such a case. They can also get it if
> they call a SECURITY DEFINER function owned by a non-superuser.
Can we improve the error that superusers get so they realize how to fix
it?
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-13 18:32 Tom Lane <tgl@sss.pgh.pa.us>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2024-03-13 18:32 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Laurenz Albe <laurenz.albe@cybertec.at>; Maxim Boguk <maxim.boguk@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
Bruce Momjian <bruce@momjian.us> writes:
> On Tue, Mar 12, 2024 at 01:22:33PM +0100, Laurenz Albe wrote:
>> I don't see why it should be a problem if a superuser gets
>> "permission denied" in such a case. They can also get it if
>> they call a SECURITY DEFINER function owned by a non-superuser.
> Can we improve the error that superusers get so they realize how to fix
> it?
I think there's been a policy of being minimalistic on
permission-denied errors to avoid giving away security information,
but I'm not sure how much sense that really makes. We already show
the specific object that didn't have permissions. I think it would
be good for these errors to also mention the specific role whose
permissions were checked. Perhaps also show the specific privileges
that were missing --- although it might be hard to do that in a
non-confusing way for complicated cases, such as queries that are
valid if you have either table- or column-level permissions.
If we just add the role I'd envision
ERROR: permission denied to role "foo" for [object]
although with any more detail that would get too long.
Another way could be
ERROR: permission denied for [object]
DETAIL: Role "foo" lacks permission [permission].
Mentioning the role that was checked should address the concern
of "I'm a superuser, why did I get this error?". However,
fixing it requires knowing which privilege to grant. I'm not
sure if that's always obvious.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view
@ 2024-03-15 02:36 Bruce Momjian <bruce@momjian.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 11+ messages in thread
From: Bruce Momjian @ 2024-03-15 02:36 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Laurenz Albe <laurenz.albe@cybertec.at>; Maxim Boguk <maxim.boguk@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
On Wed, Mar 13, 2024 at 02:32:55PM -0400, Tom Lane wrote:
> I think there's been a policy of being minimalistic on
> permission-denied errors to avoid giving away security information,
> but I'm not sure how much sense that really makes. We already show
> the specific object that didn't have permissions. I think it would
> be good for these errors to also mention the specific role whose
> permissions were checked. Perhaps also show the specific privileges
> that were missing --- although it might be hard to do that in a
> non-confusing way for complicated cases, such as queries that are
> valid if you have either table- or column-level permissions.
>
> If we just add the role I'd envision
>
> ERROR: permission denied to role "foo" for [object]
>
> although with any more detail that would get too long.
> Another way could be
>
> ERROR: permission denied for [object]
> DETAIL: Role "foo" lacks permission [permission].
>
> Mentioning the role that was checked should address the concern
> of "I'm a superuser, why did I get this error?". However,
> fixing it requires knowing which privilege to grant. I'm not
> sure if that's always obvious.
If we don't want to expand the error, and I can see why we might not
want to, giving the detailed error only for the superuser would be safe,
I think, since they are already the superuser.
Personal note: my son Matthew got this error when using photoview
software, and I was confused why the superuser was getting a permission
error.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2024-03-15 02:36 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-11 20:10 BUG #18387: Erroneous permission checks and/or misleading error messages with refresh materialized view PG Bug reporting form <noreply@postgresql.org>
2024-03-11 23:02 ` Heikki Linnakangas <hlinnaka@iki.fi>
2024-03-11 23:21 ` Bruce Momjian <bruce@momjian.us>
2024-03-12 07:34 ` Laurenz Albe <laurenz.albe@cybertec.at>
2024-03-12 13:51 ` Wetmore, Matthew (CTR) <Matthew.Wetmore@evernorth.com>
2024-03-12 14:08 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-12 10:40 ` Maxim Boguk <maxim.boguk@gmail.com>
2024-03-12 12:22 ` Laurenz Albe <laurenz.albe@cybertec.at>
2024-03-13 17:08 ` Bruce Momjian <bruce@momjian.us>
2024-03-13 18:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-15 02:36 ` Bruce Momjian <bruce@momjian.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox