Re: [v9.2] Object access hooks with arguments support (v1)

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.2] Object access hooks with arguments support (v1)
Date: 2011-10-21 16:44:33
Message-ID: CADyhKSX8YKqrgstb8W+WuE7HWtyDuRQMDu0FeqEtuVGzmPtktg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> When someone comes along in another year or two and adds materialized
> views, will they need to pass some additional data to the object
> access hook?  Probably, but I bet you're the only one who can quickly
> figure out what it is.  That's no good.  We're not going to make
> changes to PostgreSQL core that only you can maintain, and that are
> likely to be broken by future commits.  At this point I feel pretty
> good that someone can look at the stuff that we've done so far with
> SECURITY LABEL and the object access hooks, and if they add a new
> object type, they can make those things apply to the new object type
> too by copying what's already there, without making any reference to
> the sepgsql code.  There's a clear abstraction boundary, and people
> who are working on one side of the boundary do not need to understand
> the details of what happens on the other side of the boundary.
>
I had checked my older implementation based on 8.4.x or 9.0.x that
includes all the features that I want to implement.
At least, it does not require so much different information from ones
needed by DAC model, although SELECT INTO was an exception.
It might be quite natural because both works similar things.

For example, sepgsql required Oid of source database to compute
default security label on new database at createdb(). It was used to
permission checks in DAC model also.
For another example, sepgsql also required Oids of type-functions
to check permissions on them at DefineType(). It was also used to
DAC model except for these checks were commented out by
#ifdef NOT_USED because of superuser() was already checked.

So, how do we launch this efforts according to the principles:
- Hooks being used to security checks also should be deployed
around existing DAC checks.
- The delivered arguments should not be model specific.

I don't have clear idea to rework existing routines like as I proposed
long before; that wrap-up a series of DAC checks and entrypoint of
MAC hooks into a single function.

A straightforward idea is to deploy object-access-hook around existing
DAC checks with new OAT_* label, such as OAT_CREATE.
In the case of relation creation, it shall be DefineRelation() and OpenIntoRel()
to bypass "internal" invocation of heap_create_with_catalog().

Please tell me if we have different idea of code reworking to simplify
deployment of the hooks.

> In this particular case, I think it might be reasonable to change the
> DAC behavior, so that a CREATE TABLE AS SELECT or SELECT INTO requires
> insert privileges on the new table as well as permission to create it
> in the first place.  I don't particularly see any reason to require
> different privileges for CREATE TABLE followed by INSERT .. SELECT
> than what we require when the two commands are rolled into one.  Prior
> to 9.0, this case couldn't arise, because we didn't have default
> privileges, so I'm inclined to think that the way it works now is a
> historical accident rather than a deliberate design decision.
>
It will help this mismatch between DAC and MAC.
I'll submit it as a separate patch to handle this behavior.
Probably, all we need to do here is invocation of ExecCheckRTPerms().

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-10-21 16:56:11 Re: [v9.2] Object access hooks with arguments support (v1)
Previous Message Robert Haas 2011-10-21 16:37:56 So, is COUNT(*) fast now?