Horton

Thread

Commits

  1. For foreign keys, check REFERENCES privilege only on the referenced table.

  1. Postgres Permissions Article

    Paul A Jungwirth <pj@illuminatedcomputing.com> — 2017-03-28T16:47:40Z

    Hi All,

I wrote a blog post about the Postgres permissions system, and I thought 
I'd share:

http://illuminatedcomputing.com/posts/2017/03/postgres-permissions/

The main point I wanted to convey, which I somehow never grasped 
confidently from reading the docs or other articles, is how permissions 
are purely additive. Also I wanted to gather in one place how to *see 
the current permissions*, which seems to be missing/incomplete in many 
other articles. Anyway, maybe it will be helpful for some of you! Or for 
the true experts, if you see any errors, I'd be happy to know so I can 
correct them.

I also shared a few opinions amidst the facts (like that `USAGE` for 
schemas doesn't add much), so I am very pleased to have those 
challenged. You can consider them my own outstanding questions. I'd be 
especially grateful for any feedback there.

Yours,
Paul

  2. Re: Postgres Permissions Article

    Karsten Hilbert <karsten.hilbert@gmx.net> — 2017-03-29T09:37:15Z

    On Tue, Mar 28, 2017 at 09:47:40AM -0700, Paul Jungwirth wrote:

> I wrote a blog post about the Postgres permissions system, and I thought I'd
> share:
> 
> http://illuminatedcomputing.com/posts/2017/03/postgres-permissions/

> I also shared a few opinions amidst the facts (like that `USAGE` for schemas
> doesn't add much), so I am very pleased to have those challenged. You can
> consider them my own outstanding questions. I'd be especially grateful for
> any feedback there.

Not that I am an expert in any way but here's a thought on
why a permission on foreign key creation might be useful:

Being able to create foreign keys may allow to indirectly
discover whether certain values exists in a table which I
don't otherwise have access to (by means of failure or
success to create a judiciously crafted FK).

Karsten
-- 
GPG key ID E4071346 @ eu.pool.sks-keyservers.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346

  3. Re: Postgres Permissions Article

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-03-29T13:36:07Z

    Karsten Hilbert <Karsten.Hilbert@gmx.net> writes:
> On Tue, Mar 28, 2017 at 09:47:40AM -0700, Paul Jungwirth wrote:
>> I wrote a blog post about the Postgres permissions system, and I thought I'd
>> share:
>> http://illuminatedcomputing.com/posts/2017/03/postgres-permissions/

> Not that I am an expert in any way but here's a thought on
> why a permission on foreign key creation might be useful:

> Being able to create foreign keys may allow to indirectly
> discover whether certain values exists in a table which I
> don't otherwise have access to (by means of failure or
> success to create a judiciously crafted FK).

Aside from that, an FK can easily be used to cause effective
denial-of-service, for example preventing rows from being deleted
within a table, or adding enormous overhead to such a deletion.

			regards, tom lane

  4. Re: Postgres Permissions Article

    Paul A Jungwirth <pj@illuminatedcomputing.com> — 2017-03-29T15:05:23Z

    On 03/29/2017 06:36 AM, Tom Lane wrote:
> Karsten Hilbert <Karsten.Hilbert@gmx.net> writes:
>> Being able to create foreign keys may allow to indirectly
>> discover whether certain values exists in a table which I
>> don't otherwise have access to (by means of failure or
>> success to create a judiciously crafted FK).
>
> Aside from that, an FK can easily be used to cause effective
> denial-of-service, for example preventing rows from being deleted
> within a table, or adding enormous overhead to such a deletion.

Thank you both for taking a look! I agree those are both worthwhile 
concerns. It still seems a little strange it is not just part of the 
CREATE permission (for example). I understand why not everyone can 
create a foreign key, I just have trouble imagining a use case where it 
is helpful to separate it from other DDL commands. Anyway, I didn't 
write the article to nitpick details like that, but sometimes by asking 
"why" you learn new things. I really appreciate your offering your thoughts!

Paul

  5. Re: Postgres Permissions Article

    Peter J. Holzer <hjp-pgsql@hjp.at> — 2017-03-30T13:57:45Z

    On 2017-03-29 08:05:23 -0700, Paul Jungwirth wrote:
> On 03/29/2017 06:36 AM, Tom Lane wrote:
> >Karsten Hilbert <Karsten.Hilbert@gmx.net> writes:
> >>Being able to create foreign keys may allow to indirectly
> >>discover whether certain values exists in a table which I
> >>don't otherwise have access to (by means of failure or
> >>success to create a judiciously crafted FK).
> >
> >Aside from that, an FK can easily be used to cause effective
> >denial-of-service, for example preventing rows from being deleted
> >within a table, or adding enormous overhead to such a deletion.
> 
> Thank you both for taking a look! I agree those are both worthwhile
> concerns. It still seems a little strange it is not just part of the CREATE
> permission (for example). I understand why not everyone can create a foreign
> key, I just have trouble imagining a use case where it is helpful to
> separate it from other DDL commands.

A foreign key affects not only the table on which it is defined but also
the table it references. 

If Alice creates a table “master” and Bob creates a table “detail”
referencing “master”, Bob can prevent Alice from deleting entries from
her own table. So Alice must be able to decide whom she allows to
reference her tables.

I don't see how how this could be part of the create privilege - I
certainly want different roles to be able to create their own tables (or
views, or whatever) without being able to DOS each other (accidentally
or intentionally).

(Also I don't understand why you wrote “You need the permission on both
tables”: Only the owner of a table can add constraints to it - this
privilege cannot be granted to other roles at all. So to create a
foreign key constraint you need to be the owner of the referencing table
and have the references privilege on the referenced table. It's not
symmetrical.)

        hp

-- 
   _  | Peter J. Holzer    | A coding theorist is someone who doesn't
|_|_) |                    | think Alice is crazy.
| |   | hjp@hjp.at         | -- John Gordon
__/   | http://www.hjp.at/ |    http://downlode.org/Etext/alicebob.html

  6. Re: Postgres Permissions Article

    Paul A Jungwirth <pj@illuminatedcomputing.com> — 2017-03-30T20:19:52Z

    > Also I don't understand why you wrote “You need the permission on both
> tables”: Only the owner of a table can add constraints to it

Ah, this piece was really helpful for me in making it click. Thanks so 
much! I added a couple new paragraphs to my post with a link back to 
this thread. I feel like it all makes sense now! :-)

FYI "You need this permission on both tables" is what the docs say 
(https://www.postgresql.org/docs/9.6/static/sql-grant.html):

 > To create a foreign key constraint, it is necessary to have this 
privilege on both the referencing and referenced columns.

Maybe it would be worth clarifying there that you need to *own* the 
referencing table, and you need REFERENCES on the referenced table?

In any case, thanks again to you all for your help figuring this out!

Paul

  7. REFERENCES privilege should not be symmetric (was Re: Postgres Permissions Article)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-03-30T20:45:55Z

    Paul Jungwirth <pj@illuminatedcomputing.com> writes:
>> Also I don't understand why you wrote “You need the permission on both
>> tables”: Only the owner of a table can add constraints to it

> Ah, this piece was really helpful for me in making it click. Thanks so 
> much! I added a couple new paragraphs to my post with a link back to 
> this thread. I feel like it all makes sense now! :-)

> FYI "You need this permission on both tables" is what the docs say 
> (https://www.postgresql.org/docs/9.6/static/sql-grant.html):

>>> To create a foreign key constraint, it is necessary to have this 
>>> privilege on both the referencing and referenced columns.

> Maybe it would be worth clarifying there that you need to *own* the 
> referencing table, and you need REFERENCES on the referenced table?

Hmm ... interesting.  A bit of excavating in tablecmds.c shows that
in order to do ADD FOREIGN KEY, you need to be owner of the table
the constraint is being attached to (checked by ATSimplePermissions,
which is used for AT_AddConstraint by ATPrepCmd), *and* you need
REFERENCES on both tables, or at least on the columns involved in
the proposed FK constraint (checked by checkFkeyPermissions, which
is invoked against each of the tables by ATAddForeignKeyConstraint).

So yeah, this seems a little bit redundant.  In principle, a table owner
could revoke her own REFERENCES permissions on the table and thereby
disable creation of FKs leading out of it, but it'd be pretty unusual
to want to do so.

Moreover, this definition seems neither intuitive (REFERENCES doesn't
seem like it should be symmetric) nor compliant with the SQL standard.
In SQL:2011 section 11.8 <referential constraint definition> I read

Access Rules
1) The applicable privileges for the owner of T shall include REFERENCES
for each referenced column.

(T is the referenced table.)  I see nothing suggesting that the command
requires REFERENCES privilege on the referencing table.  Now this is a
little garbled, because surely they meant the owner of the referencing
table (who is issuing the command) not the owner of the referenced table,
but I think the intent is clear enough.

In short, it seems like this statement in the docs is correctly describing
our code's behavior, but said behavior is wrong and should be changed.
I'd propose fixing it like that in HEAD; I'm not sure if the back branches
should also be changed.

			regards, tom lane

  8. Re: REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

    Robert Haas <robertmhaas@gmail.com> — 2017-03-31T13:22:01Z

    On Thu, Mar 30, 2017 at 4:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> In short, it seems like this statement in the docs is correctly describing
> our code's behavior, but said behavior is wrong and should be changed.
> I'd propose fixing it like that in HEAD; I'm not sure if the back branches
> should also be changed.

Sounds reasonable, but I don't see much advantage to changing it in
the back-branches.

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

  9. Re: REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-03-31T15:29:24Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Mar 30, 2017 at 4:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In short, it seems like this statement in the docs is correctly describing
>> our code's behavior, but said behavior is wrong and should be changed.
>> I'd propose fixing it like that in HEAD; I'm not sure if the back branches
>> should also be changed.

> Sounds reasonable, but I don't see much advantage to changing it in
> the back-branches.

Well, it's a SQL-compliance bug, and we often back-patch bug fixes.

The argument for not back-patching a bug fix usually boils down to
fear of breaking existing applications, but it's hard to see how
removal of a permission check could break a working application ---
especially when the permission check is as hard to trigger as this one.
How many table owners ever revoke their own REFERENCES permission?

			regards, tom lane

  10. Re: REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

    Robert Haas <robertmhaas@gmail.com> — 2017-03-31T17:26:39Z

    On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Thu, Mar 30, 2017 at 4:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> In short, it seems like this statement in the docs is correctly describing
>>> our code's behavior, but said behavior is wrong and should be changed.
>>> I'd propose fixing it like that in HEAD; I'm not sure if the back branches
>>> should also be changed.
>
>> Sounds reasonable, but I don't see much advantage to changing it in
>> the back-branches.
>
> Well, it's a SQL-compliance bug, and we often back-patch bug fixes.

Personally, I'd be more inclined to view it as a definitional change.
Maybe we picked the wrong definition before, but it's well-established
behavior at this point.  The SQL standard also says that identifiers
are supposed to be folded to upper case, so I don't think the theory
that every deviation from the standard should be fixed and
back-patched holds a lot of water.

> The argument for not back-patching a bug fix usually boils down to
> fear of breaking existing applications, but it's hard to see how
> removal of a permission check could break a working application ---
> especially when the permission check is as hard to trigger as this one.
> How many table owners ever revoke their own REFERENCES permission?

Sure, but that argument cuts both ways.  If nobody ever does that, who
will be helped by back-patching this?

I certainly agree that back-patching this change is pretty low risk.
I just don't think it has any real benefits.

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

  11. Re: REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-03-31T17:40:00Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The argument for not back-patching a bug fix usually boils down to
>> fear of breaking existing applications, but it's hard to see how
>> removal of a permission check could break a working application ---
>> especially when the permission check is as hard to trigger as this one.
>> How many table owners ever revoke their own REFERENCES permission?

> Sure, but that argument cuts both ways.  If nobody ever does that, who
> will be helped by back-patching this?
> I certainly agree that back-patching this change is pretty low risk.
> I just don't think it has any real benefits.

I think the benefit is reduction of user confusion.  Admittedly, since
Paul is the first person I can remember ever having complained about it,
maybe nobody else is confused.

			regards, tom lane

  12. Re: REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

    David G. Johnston <david.g.johnston@gmail.com> — 2017-03-31T17:56:30Z

    On Fri, Mar 31, 2017 at 10:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Robert Haas <robertmhaas@gmail.com> writes:
> > On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> The argument for not back-patching a bug fix usually boils down to
> >> fear of breaking existing applications, but it's hard to see how
> >> removal of a permission check could break a working application ---
> >> especially when the permission check is as hard to trigger as this one.
> >> How many table owners ever revoke their own REFERENCES permission?
>
> > Sure, but that argument cuts both ways.  If nobody ever does that, who
> > will be helped by back-patching this?
> > I certainly agree that back-patching this change is pretty low risk.
> > I just don't think it has any real benefits.
>
> I think the benefit is reduction of user confusion.  Admittedly, since
> Paul is the first person I can remember ever having complained about it,
> maybe nobody else is confused.
>

​After going back-and-forth on this (and not being able to independently
come to the conclusion that what we are adhering to is actually a typo) I'm
going to toss my +1 in with Robert's.  If anyone actually complains about
the behavior and not just the documentation we could consider back-patching
if any release before 10.0 is still under support.

There have been non-bug fix improvements to the docs that didn't get
back-patched covering topics more confusing than this.  Expecting those
learning the system to consult the most recent version of the docs is
standard fare here.  From a practical perspective the revised current docs
will be applicable for past versions as long as one doesn't go a get their
REFERENCES permission revoked somehow.  If they do, and wonder why, the
docs and these list will be able to explain it reasonably well.

David J.​

  13. Re: REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

    Magnus Hagander <magnus@hagander.net> — 2017-03-31T17:57:32Z

    On Fri, Mar 31, 2017 at 7:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Robert Haas <robertmhaas@gmail.com> writes:
> > On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> The argument for not back-patching a bug fix usually boils down to
> >> fear of breaking existing applications, but it's hard to see how
> >> removal of a permission check could break a working application ---
> >> especially when the permission check is as hard to trigger as this one.
> >> How many table owners ever revoke their own REFERENCES permission?
>
> > Sure, but that argument cuts both ways.  If nobody ever does that, who
> > will be helped by back-patching this?
> > I certainly agree that back-patching this change is pretty low risk.
> > I just don't think it has any real benefits.
>
> I think the benefit is reduction of user confusion.  Admittedly, since
> Paul is the first person I can remember ever having complained about it,
> maybe nobody else is confused.
>

I think we also need to be extra careful about changing *security related*
behavior in back branches, even more so than other behavior. In this case I
think it's quite unlikely that it would hit somebody, but the risk is
there. And people generally auto-upgrade to the latest minor releases,
whereas they at least in theory read the top of the release notes when
doing a major upgrade (ok, most people probably don't, but at least some
do).

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

  14. Re: REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-03-31T21:24:02Z

    "David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Fri, Mar 31, 2017 at 10:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think the benefit is reduction of user confusion.  Admittedly, since
>> Paul is the first person I can remember ever having complained about it,
>> maybe nobody else is confused.

> ​After going back-and-forth on this (and not being able to independently
> come to the conclusion that what we are adhering to is actually a typo) I'm
> going to toss my +1 in with Robert's.

OK, the consensus seems clearly against changing this in the back branches,
so I'll just fix it in HEAD.

			regards, tom lane