Обсуждение: Cascade view drop permission checks

Поиск
Список
Период
Сортировка

Cascade view drop permission checks

От
"m7onov@gmail.com"
Дата:
Hello guys!
I've faced an interesting case with cascade drops. If we drop some view that is dependency for another view then drop cascade will not check permissions for cascade-droppping views.
Short example is:

create user alice with password 'apassword';
create user bob with password 'bpassword';

create schema sandbox_a;
create schema sandbox_b;

grant all on schema sandbox_a to alice;
grant all on schema sandbox_b to bob;
grant usage on schema sandbox_a to bob;

-- alice
create or replace view sandbox_a.alice_view as
select category, name, setting
  from pg_catalog.pg_settings;

grant select on sandbox_a.alice_view to bob;

-- bob
create or replace view sandbox_b.bob_view as
select distinct category
  from sandbox_a.alice_view;

-- alice
drop view sandbox_a.alice_view cascade;

-- !!! will drop sandbox_b.bob_view although alice is not an owner of sandbox_b.bob_view

It seems strange to me that somebody who is not a member of owner role can drop an object bypassing permission checks.
Is this behaviour OK?

Re: Cascade view drop permission checks

От
"David G. Johnston"
Дата:
On Tuesday, April 5, 2022, m7onov@gmail.com <m7onov@gmail.com> wrote:

-- alice
create or replace view sandbox_a.alice_view as
select category, name, setting
  from pg_catalog.pg_settings;

grant select on sandbox_a.alice_view to bob;

-- bob
create or replace view sandbox_b.bob_view as
select distinct category
  from sandbox_a.alice_view;

-- alice
drop view sandbox_a.alice_view cascade;

-- !!! will drop sandbox_b.bob_view although alice is not an owner of sandbox_b.bob_view

It seems strange to me that somebody who is not a member of owner role can drop an object bypassing permission checks.
Is this behaviour OK?

The system dropped the now defunct view, not alice.  Bob accepted that risk by basing the view on an object owned by another role.  I suppose other behaviors are possible but not really worth exploring.  Namely it would nice to fix the problem with “create or replace view” and not have yet other object types maybe have to be dropped.  But if two users in the same database own objects they should be expected to play nicely with each other.  Not sure why we picked this behavior instead of an error (avoid DoS by bob is part of it though, but that seems like it should also be addressed by playing nicely…) or maybe it is a bug (others will need to chime in if that is the case).

I will say the lack of documentation here:


which CASCADE links to as well, may be an omission worth fixing (or please point me to where this is covered…)

David J.

Re: Cascade view drop permission checks

От
"m7onov@gmail.com"
Дата:
David, thank you for the clarification.
Should we consider raising log level for cascade drops from NOTICE to WARNING? By now cascade drops appears in log files only when log level >= NOTICE.

--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1105,7 +1105,7 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
                                           int flags,
                                           const ObjectAddress *origObject)
 {
- int msglevel = (flags & PERFORM_DELETION_QUIETLY) ? DEBUG2 : NOTICE;
+ int msglevel = (flags & PERFORM_DELETION_QUIETLY) ? DEBUG2 : WARNING;
        bool ok = true;
        StringInfoData clientdetail;
        StringInfoData logdetail;

On Wed, Apr 6, 2022, 10:13 David G. Johnston <david.g.johnston@gmail.com> wrote:
On Tuesday, April 5, 2022, m7onov@gmail.com <m7onov@gmail.com> wrote:

-- alice
create or replace view sandbox_a.alice_view as
select category, name, setting
  from pg_catalog.pg_settings;

grant select on sandbox_a.alice_view to bob;

-- bob
create or replace view sandbox_b.bob_view as
select distinct category
  from sandbox_a.alice_view;

-- alice
drop view sandbox_a.alice_view cascade;

-- !!! will drop sandbox_b.bob_view although alice is not an owner of sandbox_b.bob_view

It seems strange to me that somebody who is not a member of owner role can drop an object bypassing permission checks.
Is this behaviour OK?

The system dropped the now defunct view, not alice.  Bob accepted that risk by basing the view on an object owned by another role.  I suppose other behaviors are possible but not really worth exploring.  Namely it would nice to fix the problem with “create or replace view” and not have yet other object types maybe have to be dropped.  But if two users in the same database own objects they should be expected to play nicely with each other.  Not sure why we picked this behavior instead of an error (avoid DoS by bob is part of it though, but that seems like it should also be addressed by playing nicely…) or maybe it is a bug (others will need to chime in if that is the case).

I will say the lack of documentation here:


which CASCADE links to as well, may be an omission worth fixing (or please point me to where this is covered…)

David J.

Re: Cascade view drop permission checks

От
"David G. Johnston"
Дата:


On Wednesday, April 6, 2022, m7onov@gmail.com <m7onov@gmail.com> wrote:
David, thank you for the clarification.
Should we consider raising log level for cascade drops from NOTICE to WARNING? By now cascade drops appears in log files only when log level >= NOTICE.

--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1105,7 +1105,7 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
                                           int flags,
                                           const ObjectAddress *origObject)
 {
- int msglevel = (flags & PERFORM_DELETION_QUIETLY) ? DEBUG2 : NOTICE;
+ int msglevel = (flags & PERFORM_DELETION_QUIETLY) ? DEBUG2 : WARNING;
        bool ok = true;
        StringInfoData clientdetail;
        StringInfoData logdetail;

Please don’t top-post.

There is no point that I can see unless you also argue to warn/log about every dropped object.  Which can be done by the dba using event triggers if they really want to.

They said cascade and got cascade.  And can in-client set to notice and use a transaction.

David J.
 

Re: Cascade view drop permission checks

От
Tom Lane
Дата:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Tuesday, April 5, 2022, m7onov@gmail.com <m7onov@gmail.com> wrote:
>> It seems strange to me that somebody who is not a member of owner role can
>> drop an object bypassing permission checks.
>> Is this behaviour OK?

> The system dropped the now defunct view, not alice.  Bob accepted that risk
> by basing the view on an object owned by another role.  I suppose other
> behaviors are possible but not really worth exploring.

(a) this behavior is what is required by the SQL standard.

(b) what other behavior would be better?  Dropping the table and
leaving a broken view behind isn't good.  Neither is refusing to
let the owner drop her object.

            regards, tom lane

Re: Cascade view drop permission checks

От
"David G. Johnston"
Дата:
On Wed, Apr 6, 2022 at 6:48 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
(b) what other behavior would be better?  Dropping the table and
leaving a broken view behind isn't good.  Neither is refusing to
let the owner drop her object.


CREATE OR REPLACE VIEW name
SELECT null::type, null::type, null::type;

Where the column count and types allow the "OR REPLACE" behavior to work.

Now the owner of the view can put an alternate implementation in place, and the lack of a delete on the view prevents further cascading.

It has its flaws and benefits, but so does having objects drop.  I suppose if we did have this kind of behavior we'd probably also have a way to inform the system that, basically, there are no select privileges (or some other spelling of "invalid") on the view, so any attempt to query the view would fail even while the view still exists.

David J.