[PATCH] Fix leaky VIEWs for RLS
in [PgSql]
|
From: Robert Haas on 4 Jun 2010 14:00 On Fri, Jun 4, 2010 at 1:46 PM, Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> wrote: > On 04/06/10 17:33, Tom Lane wrote: >> >> Heikki Linnakangas<heikki.linnakangas(a)enterprisedb.com> writes: >>> >>> On 04/06/10 07:57, Tom Lane wrote: >>>> >>>> The proposal some time back in this thread was to trust all built-in >>>> functions and no others. >> >>> I thought I debunked that idea already >>> (http://archives.postgresql.org/pgsql-hackers/2009-10/msg01428.php). Not >>> all built-in functions are safe. Consider casting integer to text, for >>> example. > > (I meant "text to integer", of course) > >> Maybe the entire idea is unworkable. I certainly don't find any comfort >> in your proposal in the above-referenced message to trust index >> operators; where is it written that those don't throw errors? > > Let's consider b-tree operators for an index on the secure table, for > starters. Surely a b-tree index comparison operator can't throw an error on > any value that's in the table already, you would've gotten an error trying > to insert that. > > Now, is it safe to expand that thinking to b-tree operators in general, even > if there's no such index on the table? I'm not sure. But indexable > operations are what we care about the most; the order of executing those > determines if you can use an index scan or not. Another idea I had was... would it be safe to trust functions defined by the same user who owns the view? If he's granted access to the view and the function to some other user, presumably he doesn't mind them being used together? Or is that too optimistic? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 4 Jun 2010 15:33 Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> writes: > On 04/06/10 17:33, Tom Lane wrote: >> Maybe the entire idea is unworkable. I certainly don't find any comfort >> in your proposal in the above-referenced message to trust index >> operators; where is it written that those don't throw errors? > Let's consider b-tree operators for an index on the secure table, for > starters. Surely a b-tree index comparison operator can't throw an error > on any value that's in the table already, you would've gotten an error > trying to insert that. Man, are *you* trusting. A counterexample: suppose we had a form of type "text" that carried a collation specifier internally, and the comparison routine threw an error if asked to compare values with incompatible specifiers. An index built on a column of all the same collation would work fine. A query that tried to compare against a constant of a different collation would throw an error. > I'm not sure. But indexable > operations are what we care about the most; the order of executing those > determines if you can use an index scan or not. Personally, I care just as much about hash and merge join operators... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Heikki Linnakangas on 4 Jun 2010 15:58 On 04/06/10 22:33, Tom Lane wrote: > Heikki Linnakangas<heikki.linnakangas(a)enterprisedb.com> writes: >> On 04/06/10 17:33, Tom Lane wrote: >>> Maybe the entire idea is unworkable. I certainly don't find any comfort >>> in your proposal in the above-referenced message to trust index >>> operators; where is it written that those don't throw errors? > >> Let's consider b-tree operators for an index on the secure table, for >> starters. Surely a b-tree index comparison operator can't throw an error >> on any value that's in the table already, you would've gotten an error >> trying to insert that. > > Man, are *you* trusting. > > A counterexample: suppose we had a form of type "text" that carried a > collation specifier internally, and the comparison routine threw an > error if asked to compare values with incompatible specifiers. An index > built on a column of all the same collation would work fine. A query > that tried to compare against a constant of a different collation would > throw an error. I can't take that example seriously. First of all, tacking a collation specifier to text values would be an awful hack. Secondly, it would be a bad idea to define the b-tree comparison operators to throw an error; it would be a lot more useful to impose an arbitrary order on the collations, so that all values with collation A are considered smaller than values with collation B. We do that for types like box; smaller or greater than don't make much sense for boxes, but we implement them in a pretty arbitrary way anyway to make it possible to build a b-tree index on them, and for the planner to use merge joins on them, and implement DISTINCT using sort etc. >> I'm not sure. But indexable >> operations are what we care about the most; the order of executing those >> determines if you can use an index scan or not. > > Personally, I care just as much about hash and merge join operators... Hash seems safe too. Don't merge joins just use the default b-tree operator? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 4 Jun 2010 16:12 Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> writes: > On 04/06/10 22:33, Tom Lane wrote: >> A counterexample: suppose we had a form of type "text" that carried a >> collation specifier internally, and the comparison routine threw an >> error if asked to compare values with incompatible specifiers. An index >> built on a column of all the same collation would work fine. A query >> that tried to compare against a constant of a different collation would >> throw an error. > I can't take that example seriously. First of all, tacking a collation > specifier to text values would be an awful hack. Really? I thought that was under serious discussion. But whether it applies to text or not is insignificant; I believe there are cases just like this in existence today for some datatypes (think postgis). The real point is that the comparison constant is under the control of the attacker, and it's not part of the index. Therefore "it didn't throw an error during index construction" proves nothing whatever. > ... Secondly, it would be a > bad idea to define the b-tree comparison operators to throw an error; You're still being far too trusting, by imagining that only *designed* error conditions matter here. Think about overflows, out-of-memory, (perhaps intentionally) corrupted data, etc etc. I think the only real fix would be something like what Marc suggested: if there's a security view involved in the query, we simply don't give the client the real error message. Of course, now our "security feature" is utterly disastrous on usability as well as performance counts ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 6 Jun 2010 21:38
On Fri, Jun 4, 2010 at 4:12 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> writes: >> On 04/06/10 22:33, Tom Lane wrote: >>> A counterexample: suppose we had a form of type "text" that carried a >>> collation specifier internally, and the comparison routine threw an >>> error if asked to compare values with incompatible specifiers. �An index >>> built on a column of all the same collation would work fine. �A query >>> that tried to compare against a constant of a different collation would >>> throw an error. > >> I can't take that example seriously. First of all, tacking a collation >> specifier to text values would be an awful hack. > > Really? �I thought that was under serious discussion. �But whether it > applies to text or not is insignificant; I believe there are cases just > like this in existence today for some datatypes (think postgis). > > The real point is that the comparison constant is under the control of > the attacker, and it's not part of the index. �Therefore "it didn't > throw an error during index construction" proves nothing whatever. > >> ... Secondly, it would be a >> bad idea to define the b-tree comparison operators to throw an error; > > You're still being far too trusting, by imagining that only *designed* > error conditions matter here. �Think about overflows, out-of-memory, > (perhaps intentionally) corrupted data, etc etc. btree comparison operators should handle overflow and corrupted data without blowing up. Maybe out-of-memory is worth worrying about, but I think that is a mighty thin excuse for abandoning this feature altogether. You'd have to contrive a situation where the system was just about out of memory and something about the value being compared against resulted in the comparison blowing up or not. I think that's likely to be rather hard in practice, and in any case it's a covert channel attack, which I think everyone already agrees is beyond the scope of what we can protect against. You can probably learn more information more quickly about the unseen data by fidding with EXPLAIN, analyzing query execution times, etc. As long as we are preventing the actual contents of the unseen tuples from being revealed, I feel reasonably good about it. > I think the only real fix would be something like what Marc suggested: > if there's a security view involved in the query, we simply don't give > the client the real error message. �Of course, now our "security > feature" is utterly disastrous on usability as well as performance > counts ... Not pushing anything into the view is an equally real fix, although I'll be pretty depressed if that's where we end up. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |