Re: [v9.4] row level security

From: Gregory Smith <gregsmithpgsql(at)gmail(dot)com>
To: Craig Ringer <craig(at)2ndquadrant(dot)com>, Greg Stark <stark(at)mit(dot)edu>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>, Josh Berkus <josh(at)agliodbs(dot)com>, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, "ktm(at)rice(dot)edu" <ktm(at)rice(dot)edu>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, jeff(dot)mccormick(at)crunchydatasolutions(dot)com
Subject: Re: [v9.4] row level security
Date: 2013-12-14 03:24:28
Message-ID: 52ABCF6C.4010409@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

While Craig has been soldiering onward to the write side of things, I've
been working with some groups who are evaluating if the RLS feature is
good enough for switching some real world apps over to use PostgreSQL.
Amazingly I can even say that these are applications involving the US
Department of Defense, and they're going well so far. There has been
some concern on this list as to whether this feature is being evaluated
by people who are serious about real-world database security issues.
The crowd I'm talking to now sure is. There's some interesting bad
news, like how we're going to have to completely refactor all the "if
(!superuser())" shortcuts into real permissions one day to make them
happy. But for the most part the prototype conversions have been
working out. The RLS feature set available with the CF submission is
good enough for those projects to continue exploring PostgreSQL. I feel
good now that if the code issues around what's already been developed
are sorted out to commit quality, the users really will queue once this
hits.

== Review demo code ==

Attached is a small demo piece built by my new co-worker Jeff McCormick
that I've been hacking lightly on. The idea was to get something a step
beyond trivial that maps into real-world need, well enough that we could
get people to agree it was usefully representative. What they did is
take the sort of security levels these apps have, which often includes
two different criteria for each row, and mapped that into what the RLS
submission supports. There are a set of roles for the permission to see
various shapes (triangle/circle/square/heart) and colors, and then
people can see a row only if they have both color and shape
permissions. The part that I thought was cute when I first read it is
how this query looks up data in the catalog to accomplish that:

ALTER TABLE colors_shapes
SET ROW SECURITY FOR ALL TO (
color IN (
SELECT permission_role.rolname FROM pg_roles active_role,
pg_auth_members, pg_roles permission_role WHERE
active_role.rolname = current_user AND
active_role.oid = pg_auth_members.member AND
permission_role.oid = pg_auth_members.roleid
)
AND shape IN (
SELECT permission_role.rolname FROM pg_roles active_role,
pg_auth_members, pg_roles permission_role WHERE
active_role.rolname = current_user AND
active_role.oid = pg_auth_members.member AND
permission_role.oid = pg_auth_members.roleid
)
);

Insert some data that looks like this:

id | color | shape
----+-------------+---------------
1 | blue_role | triangle_role
2 | red_role | heart_role
3 | purple_role | circle_role

And then users can see rows only if they have a pair of matches on both
those role sets. The fact that you can just plug in whatever you want
into SET ROW SECURITY allows all kinds of things. I have a more
complicated sample we're still poking at, using a full pl/pgsql function
there, but I've already got enough to try and cover in this message to
bring that in today. One reason the users I've been talking to feel
comfortable that they'll be able to build apps usefully with this
feature is this interface. It feels more like a pluggable API for
defining rules than a specific implementation. Congratulations to the
usual community members who expect complete feature overkill in every
design.

== Code review ==

Onto coding. Rather than trying to cope with patches I pulled KaiGai's
tree from https://github.com/kaigai/sepgsql/tree/rowsec to try things
out. That hasn't been merged against head in a while and it's suffered
serious but not terminal bit rot. The REPLICA IDENTITY changes in
particular touched adjacent code all over the place, since it and RLS
are both added things to the end of existing lists. The merges to get
thing back to working again were only hard when my eyes crossed on the
always fun pg_class bootstrapping table. I did all that myself up to
committed changes on Dec 3. The only real change I made was to punt the
OID number assignment into a whole different range, to make future merge
rot less likely. Those are going to get reassigned at commit time
anyway, no need to try and guess the appropriate next number for each today.

I just pushed a fork of KaiGai's repo to
https://github.com/gregs1104/sepgsql/tree/rls that includes all of the
must fix items to get the feature working again. That's what the
attached demo code was tested again. The regression tests are still
seriously busted, beyond just regenerating new output. I pushed that
off for now, I need to study recent commits better to catch up on what
changed. For community reference I'm attaching an updated patch that
forks master after dfd5151c5800448a2be521797f95e1aa63d87b67 and then has
the merging I did. I'm hoping that getting a positive functional review
will lure KaiGai back to help me update things. Hopefully pulling or
looking at commits from my merge attempt helps with that.

The (blue) elephant in the room here for this feature is the list of
committers in particular who are very picky about the parts of the code
this feature is touching (*cough* Tom). One reason I'm trying to get
these demo pieces together--Jeff here is building a whole RLS test
sample repo--is to try and make the job of testing this out easier. As
I dig myself out of the hole I'm in after getting really sick instead of
going to PG.EU, I've got a whole lot of time set aside to work on
community features lined up.

== Outstanding issues ==

Things I can already see to work on here are:

-Fix the regression tests
-Catch up to master again
-Continue to expand the functional tests
-Is there enough information about row security available in psql
output? For example, there's nothing in "\d" output that suggests it
might exist. pg_rowsecurity is a monster to try and read.
-Documentation needs plenty of editing, which I can take care of.

== Cover Channels ==

One last topic since I know it's been a sore point. Covert channels.
There's still some obvious ones around. In this example I was plenty
uneasy about some of what you can learn from the EXPLAIN output too.
But no one I've talked to about deploying PostgreSQL RLS seem terrible
uncomfortable with the exposure at this point. RLS is just one layer of
extra protection on checks that are also happening in the application
API exposing the data to users. No one expects any one layer to be the
impenetrable one. In the interest of CYA, I've talked through some
scenarios where attackers have resources like a hacked libpq client and
enough info to search the key space. Instead of expecting that the
database can survive that, which is pretty unreasonable, what I'm
getting requests for now is better audit logging to make it easier to
detect that. We should certainly close any holes we can, but where
things are at today doesn't seem functionally unacceptable anymore.

--
Greg Smith greg(dot)smith(at)crunchydatasolutions(dot)com
Chief PostgreSQL Evangelist - http://crunchydatasolutions.com/

Attachment Content-Type Size
rlsdemo-v1.sql text/plain 2.3 KB
rls-gs-1.patch text/plain 169.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2013-12-14 04:40:48 Re: [v9.4] row level security
Previous Message Tatsuo Ishii 2013-12-14 02:35:05 encoding name "SHIFT_JIS" is absent in chklocale.c