On Wed, Oct 19, 2011 at 6:18 AM, Kohei KaiGai <kai...@kaigai.gr.jp> wrote: > 2011/10/18 Robert Haas <robertmh...@gmail.com>: >>> In the example table creation, heap_create_with_catalog() is invoked >>> by 5 routines, however, 3 of them are just internal usages, so it is not >>> preferable to apply permission checks on table creation.... >> >> Some wit once made the remark that if a function has 10 arguments, it >> has 11 arguments, meaning that functions with very large numbers of >> arguments typically indicate a poor choice of abstraction boundary. >> That's pretty much what I think is going on with >> heap_create_with_catalog(). I think maybe some refactoring is in >> order there, though I am not sure quite what. >> > Sorry, are you complained about the number of arguments in > heap_create_with_catalog? or hooks of security checks?
I'm just saying that heap_create_with_catalog() is big and complex and does a lot of different things. The fact it does security checks even though it's also sometimes called as an internal function strikes me as a modularity violation. Normally I would expect to have a high-level routine (which is invoked more or less directly from SQL) that does security checks and then calls a low-level routine (that actually does the work) if they pass. Other parts of the code that want to perform the same operation without the security checks can call the low-level function directly. But that's not the way it's set up here. > Yes, I never say SELECT INTO without DAC checks cause actual > security hole, because owner can change its access permissions by > himself, unlike MAC. > Please suppose that there are two security labels: confidential table > (TC) and public table (PT). Typical MAC policy (including selinux) > does not allow users who can read from tables with TC to write out > data into tables with PT, because PT is readable for public as literal, > so confidential data may be leaked to public domain in the result. > > It is a significant characteristic of MAC; called as data-flow-control. > So, it damages significant part of its worth, if MAC could not distinct > CREATE TABLE from SELECT INTO in PostgreSQL, and it is the > reason why I strongly requires a flag of contextual information here. > > Although you say it is not possible to maintain, doesn't the above > introduction help us to understand nothing why MAC needs to > distinct SELECT INTO from CREATE TABLE?, and why it needs > a flag to distinct two cases? Sure. But what is going to happen when someone needs to modify that code in a year or two? In order to understand what to do with the object access hook, they're going to need to understand all those details you just mentioned. And those details do not exist in the patch as submitted. Worse, let's suppose we'd committed a patch like the one you have here before foreign tables went in. Then, whoever added foreign tables would have needed to know to add the foreign table server name to the object access hook invocation, because apparently sepgsql needs that. How would they know they needed to do that? I've committed practically all of the sepgsql-related patches, and I would not have known that, so it seems likely to me that other people adding new functionality to the server wouldn't know it either. 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. 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers