* Robert Haas (robertmh...@gmail.com) wrote: > I'm really concerned about the security implications of this patch. I > think we're setting ourselves up for a whole lot of hurt for somewhat > unclear gain.
I'm certainly of a different opinion and, for the most part, I feel that if there are security concerns then they need to be addressed- and better by us than by asking users to use some other mechanism to implement RLS. > In my view, commit 842faa714c0454d67e523f5a0b6df6500e9bc1a5 basically > *is* row-level security: instead of applying a row-level security > policy to a table, just create a security-barrier view over the table > and grant access to the view. Forget that the table ever existed. > Done. This argument could have been made for column-level privileges also, no? Yet I don't hear any calls for that to be ripped out now that you could implement it through updatable security-barrier views. That commit was the ground-work to allow us to finally get proper RLS and I'm very disappointed to hear that the mechanical pieces around making RLS easy for users to use (and getting that check-box taken care of in a wide variety of fields that we are being exposed to now, see the PGConf.NYC keynote speakers...) is receiving such push-back. > With this approach, there's a lot of stuff that we don't have to > reinvent. We've talked a lot about whether row-level security should > only be concerned with the rows it scans, or whether it should also > restrict the new rows that can be created. You can get either > behavior by choosing whether or not to use WITH CHECK OPTION. And > then there's this question of who should be RLS-exempt; that's > basically a question of to whom you grant privileges on the underlying > table. Note that this can be very fine-grained: for example, you can > allow someone to exempt themselves for selects but not for updates by > granting them SELECT privileges but not UPDATE privileges on the > underlying table. And potentially-exempt users can choose whether > they want a particular access to actually be exempt by targeting the > view when they don't want to be exempt and the table when they do. I agree that views, or even security-definer functions, offer a great deal of flexibility, and that may be necessary in some use-cases, but I fail to see why that means we should avoid providing the mechanics to achieve simple and usable RLS akin to what other major RDBMS's have. > That's mighty useful for debugging, at least IMHO. And, if you want > to have several row-level security policies for different classes of > users, just create more than one view and grant different privileges > on each. I'm really not impressed with the idea that RLS should be done with multiple different views of the same underlying table. > By contrast, it seems to me that every design so far proposed for > something that is actually called row-level security - as opposed to > commit 842faa714c0454d67e523f5a0b6df6500e9bc1a5, which *really is* > row-level security, is extremely limited. Look back at all the things > listed in the previous paragraph; can you do those things easily with > the designs that have been proposed? As far as I can see, not really. I don't feel that RLS will, or even *should*, have the same level of flexibility that you can achieve with views and/or security definer functions. I expect that, over time, we will add more capabilities to it, but it's never going to be able to redefine the contents of a column as a view can, nor will it be able to add columns to a table as views can. I don't see those as reasons against having support for RLS. > Your (Craig's) rls-9.4-upd-sb-views patch seems to have a rough > equivalent of WITH CHECK OPTION, probably because we've talked a lot > about that specific issue, but it doesn't line up exactly to what WITH > CHECK OPTION actually does. There's no independently-grantable > RLS-exemption privilege - and even when we talk about that, it's > usually some kind of global bit that applies to all tables and all > operations equally - whereas with the above approach it can be > per-table and per-operation and doesn't require superuser intervention > to flip the bit. I'm glad to hear your thoughts on the level of granularity which might be nice to have with RLS. What would be great is to spend a bit more time reviewing what other systems provide in this area and considering what makes sense for us. This will also be a feature and an area which we will be improving for a long time to come, but we do need this capability and we have to start somewhere. > There's no way for users who are RLS exempt to turn > off their exemption for testing purposes, let alone on a per-table > basis. I don't follow this argument entirely- users can't turn off the existing permissions system for testing either, unless an authorized user with the correct permissions makes the change to allow it- or the user bumps themselves up to superuser, or to a role which has broader permissions, both of which would also be possible to do with RLS. > There's no way to have multiple RLS policies on a single > table. All of those are things that we get "for free" in the > view-over-table model, and implementing formal RLS basically requires > us to either invent a new RLS-specific way of doing each of those > things, or suffer along with a subset of the functionality. Yuck. What would probably be good is to review the use-cases which the current patch already addresses- and we've had good responses from actual users who are already playing with the patch and are hearing that it is addressing their requirements. > But what's really awful about this whole design is that it breaks the > invariant that reading from a table doesn't run anybody else's code. You're suggesting that we use views instead, which clearly could run someone else's code. Perhaps the user will notice that they're selecting from a view instead of a table, but I've never seen a security design around making sure that what is being select'd from is a table vs. a view. Have you seen applications which implement such a check prior to running a query? > It's already the case that users need to be awfully careful about > modifying tables, because that might fire triggers that do bad things. > But at least you can SELECT from a table and it will either work, or > it will fail with a permission denied error. What it will not do is > unexpectedly run some code that you weren't expecting it to run. You > can't be so blithe about selecting from views, but reading a plain > table is always OK. Now, as soon as we introduce the concept that > selecting from a table might not really mean "read from the table" but > "read from the table after applying this owner-specified qual", we're > opening up a whole new set of attack surfaces. With this, I agree, there is risk associated with the implementation we're looking at for RLS. We could narrow the case by reducing the capabilities of RLS in PG by only allowing certain functions to be used in the definition of a RLS policy (eg: btree operators of known data types, or something similar to our "leak-proof" attribute), but I don't see that it really buys us much. There are a *lot* of ways in which an individual who has the ability to create objects inside the database can cause problems, but that comes with the flexibility we provide users with. That will always be a balance but, I believe, we wouldn't have the same level of success or have such an awesome system without that flexibility. > Every pg_dump is an > opportunity to hack somebody else's account, or at least audit their > activity. Protecting the superuser against everybody else is nice, > but I think it's just as important to protect non-superusers against > each other, and I think that's going to be hard -- because in the RLS > world, SELECT * FROM tab is now *fundamentally* ambiguous. Maybe it's > reading from the table, and maybe it's really clandestinely reading > from a view over the table, and the user has no way of being really > clear about which behavior they want. From a security point of view, > that seems very bad. I don't see this as being an insurmountable issue. I agree that having a way for pg_dump to run safely is important and the superuser check does address that, given that we don't have a "read-only (and everything)" capability today. Once we do (and I surely hope that will come sooner rather than later), such a role should also have the 'no RLS' bit, as it wouldn't make any sense for such a role anyway. The lack of that is not a strike against RLS though. > To recap: > > 1. Reinventing RLS-specific ways to do all of the things that can > already be done in the view-over-table model is a lot of work. I agree that there's a fair bit of work involved, but I do not see reimplementing views as RLS as the goal. > 2. There's a danger that the functionality available in the two models > will diverge, so that certain things can only be done in one world or > the other. They will always be distinct, intentionally so. > 3. On the whole, it seems likely that the RLS-specific world will > remain impoverished compared to the view-over-table model. Agreed. As is the case with views vs. security definer functions. > 4. Making SELECT * FROM tab ambiguous seems likely to be a security minefield. While I agree that we need to consider this, I don't think it will be a "minefield", but rather something we need to document and educate our users about. If you'd like a "disable-all-RLS" GUC, I'm all for it. Tossing out any hope of having RLS in PG is tossing the baby out with the bathwater though, imv. Thanks, Stephen
Description: Digital signature