Re: Wrong security context for deferred triggers?

Hi,

Allow me to provide some background on how we came across this.

(This is my first time posting to a pgsql list so hopefully I've got
everything set up correctly.)

We have a db with a big legacy section that we're in the process of
modernizing. To compensate for some of the shortcomings we have designed
a layer of writable views to better represent the underlying data and
make working with it more convenient. The following should illustrate
what we're doing:

    -- this is the schema containing the view layer.
    create schema api;
    -- and this user is granted access to the api, but not the rest of
the legacy db.
    create role apiuser;
    grant usage on schema api to apiuser;

    -- some dummy objects in the legacy db - poorly laid out and poorly
named.
    create schema legacy;
    create table legacy.stock_base (
        id serial primary key
      , lbl text not null unique
      , num int not null
      -- etc
    );
    create table legacy.stock_extra (
        id int not null unique references legacy.stock_base (id)
      , man text
      -- etc
    );

    -- create the stock view which better names and logically groups
the data.
    create view api.stock as
      select sb.id
           , sb.lbl as description
           , sb.num as quantity
           , se.man as manufacturer
        from legacy.stock_base sb
          left join legacy.stock_extra se using (id);
    -- make it writable so it is easier to work with. use security
definer to allow access to legacy sections.
    create function api.stock_cud() returns trigger language plpgsql
security definer as $$
    begin
        -- insert/update legacy.stock_base and legacy.stock_extra
depending on trigger action, modified fields, etc.
        assert tg_op = 'INSERT'; -- assume insert for example's sake.
        insert into legacy.stock_base (lbl, num) values
(new.description, new.quantity) returning id into new.id;
        insert into legacy.stock_extra (id, man) values (new.id,
new.manufacturer);
        return new;
    end;
    $$;
    create trigger stock_cud
        instead of insert or update or delete on api.stock
        for each row execute function api.stock_cud();

    -- grant the apiuser permission to work with the view.
    grant insert, update, delete on api.stock to apiuser;

    -- insert as superuser - works as expected.
    insert into api.stock (description, quantity, manufacturer) values
('item1', 10, 'manufacturer1');
    -- insert as apiuser - works as expected.
    set role apiuser;
    insert into api.stock (description, quantity, manufacturer) values
('item2', 10, 'manufacturer2');

In some cases there are constraint triggers on the underlying tables to
validate certain states. It is, however, possible for a state to be
temporarily invalid between statements, so long as it is valid at tx
commit. For this reason the triggers are deferred by default. Consider
the following example:

    reset role;
    create function legacy.stock_check_state() returns trigger language
plpgsql as $$
    begin
        -- do some queries to check the state of stock based on
modified rows and error if invalid.
        raise notice 'current_user %', current_user;
        -- dummy validation code.
        perform * from legacy.stock_base sb left join
legacy.stock_extra se using (id) where sb.id = new.id;
        return new;
    end;
    $$;
    create constraint trigger stock_check_state
        after insert or update or delete on legacy.stock_base
        deferrable initially deferred
        for each row execute function legacy.stock_check_state();

Then repeat the inserts:

    -- insert as superuser - works as expected.
    reset role;
    insert into api.stock (description, quantity, manufacturer) values
('item3', 10, 'manufacturer3'); -- NOTICE:  current_user postgres
    -- insert as apiuser - fails.
    set role apiuser;
    insert into api.stock (description, quantity, manufacturer) values
('item4', 10, 'manufacturer4'); -- NOTICE:  current_user apiuser

    -- insert as apiuser, not deferred - works as expected.
    begin;
    set constraints all immediate;
    insert into api.stock (description, quantity, manufacturer) values
('item4', 10, 'manufacturer4'); -- NOTICE:  current_user postgres
    commit;

The insert as apiuser fails when the constraint trigger is deferred, but
works as expected when it is immediate.

Hopefully this helps to better paint the picture. Our workaround
currently is to just make `legacy.stock_check_state()` security definer
as well. As I told Laurenz, we're not looking to advocate for any
specific outcome here. We noticed this strange behaviour and thought it
to be a bug that should be fixed - whatever "fixed" ends up meaning.

Regards,

Bennie Swart

On 2024/06/26 17:53, Laurenz Albe wrote:
> On Wed, 2024-06-26 at 07:38 -0700, David G. Johnston wrote:
>> We have a few choices then:
>> 1. Status quo + documentation backpatch
>> 2. Change v18 narrowly + documentation backpatch
>> 3. Backpatch narrowly (one infers the new behavior after reading the existing documentation)
>> 4. Option 1, plus a new v18 owner-execution mode in lieu of the narrow change to fix the POLA violation
>>
>> I've been presenting option 4.
>>
>> Pondering further, I see now that having the owner-execution mode be the only way to avoid
>> the POLA violation in deferred triggers isn't great since many triggers benefit from the
>> implied security of being able to run in the invoker's execution context - especially if
>> the trigger doesn't do anything that PUBLIC cannot already do.
>>
>> So, I'm on board with option 2 at this point.
> Nice.
>
> I think we can safely rule out option 3.
> Even if it is a bug, it is not one that has bothered anybody so far that a backpatch
> is indicated.
>
> Yours,
> Laurenz Albe
>
>
>
>