Thread: Insufficient attention to security in contrib (mostly)
I noticed a number of functions, most but not all in contrib, which allow an unprivileged user to obtain varying amounts of information about a relation he has no permissions to access: dblink_get_pkey() will tell you what its primary key is. This is not a big deal seeing that you can get the info by looking into the system catalogs, but someone who was trying to lock down the system catalogs wouldn't be happy. I think this should require SELECT privilege on the table. btreefuncs.c is a security hole a mile wide: it will happily dump the entire data content of an index for you. It's a good thing this hasn't shipped in any release yet. While we could possibly make it look up the index's parent table and check if you have SELECT privilege on that, it'd be easier just to make the functions demand superuser privilege, which is what the rest of the functions in this contrib module require. Comments? pgrowlocks tells you about row lock states, which maybe is not that interesting for security, but still it's information that one wouldn't expect to be exposed to someone who isn't allowed to read the table. I suppose knowing the number of live tuples might in itself be sensitive information. If you do think that's sensitive information, you probably won't be happy that pgstattuple and pgstatindex likewise have no permission checks. The same goes for the various size-calculation functions in the backend's adt/dbsize.c file. The latter should probably require superuser, particularly for the functions that are willing to tell you about whole databases you can't get into. currtid_byrelname() and currtid_byreloid() have no permission checks either. I'm not sure that TID relationships can be used for anything interesting, but ... Lastly, int4notin() and oidnotin() have no permission checking, which means you can find out whether specific values are or are not present in an int4 or oid column you shouldn't read. This code is so old, crufty, and undocumented that I'm strongly inclined to remove it instead of fix it --- it really has no excuse to live when we support IN (sub-SELECT) constructs. Comments? regards, tom lane
"Tom Lane" <tgl@sss.pgh.pa.us> writes: > Lastly, int4notin() and oidnotin() have no permission checking, which > means you can find out whether specific values are or are not present > in an int4 or oid column you shouldn't read. This code is so old, > crufty, and undocumented that I'm strongly inclined to remove it > instead of fix it --- it really has no excuse to live when we support > IN (sub-SELECT) constructs. > > Comments? Wow, those are strange beasts. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com
Tom, > dblink_get_pkey() will tell you what its primary key is. This is not > a big deal seeing that you can get the info by looking into the system > catalogs, but someone who was trying to lock down the system catalogs > wouldn't be happy. I think this should require SELECT privilege on > the table. +1 > btreefuncs.c is a security hole a mile wide: it will happily dump the > entire data content of an index for you. It's a good thing this hasn't > shipped in any release yet. While we could possibly make it look up > the index's parent table and check if you have SELECT privilege on > that, it'd be easier just to make the functions demand superuser > privilege, which is what the rest of the functions in this contrib > module require. Comments? Superuser only should be fine. Heck, I don't know anyone who actually uses this function ... > pgrowlocks tells you about row lock states, which maybe is not that > interesting for security, but still it's information that one wouldn't > expect to be exposed to someone who isn't allowed to read the table. > I suppose knowing the number of live tuples might in itself be > sensitive information. Here I think the advantage of being able to run this as a non-superuser (and thus not have the superuser password on the client machine) outweighs any data which can be reverse-engineered from the lock information. > If you do think that's sensitive information, you probably won't be > happy that pgstattuple and pgstatindex likewise have no permission > checks. The same goes for the various size-calculation functions in > the backend's adt/dbsize.c file. The latter should probably require > superuser, particularly for the functions that are willing to tell > you about whole databases you can't get into. Hmmm, we can't really require anything greater than SELECT permission for dbsize. Remember that every commonly-used function/utility which requires superuser or object owner status is an invitation to put the super or owner password on the client machine (or, worse, to run the whole application as the DB owner), or to create SECURITY DEFINER functions, both of which then become potential security holes much larger than the issue we were trying to fix. One of the things which makes PostgreSQL secure is the ability to run an entire application with extremely restricted permissions on the client side. This is something which MySQL and SQL Server completely fail to do, and why we are much more secure than they are. Don't break it! So I think dbsize needs to run with the least reasonable security privs, namely SELECT. I'd agree that it needs SELECT, though; you don't want someone who doesn't have rights to see a table getting an approximate rowcount. > currtid_byrelname() and currtid_byreloid() have no permission checks > either. I'm not sure that TID relationships can be used for anything > interesting, but ... Hmmm. I'm not even sure how to use these. Will poke around. > Lastly, int4notin() and oidnotin() have no permission checking, which > means you can find out whether specific values are or are not present > in an int4 or oid column you shouldn't read. This code is so old, > crufty, and undocumented that I'm strongly inclined to remove it > instead of fix it --- it really has no excuse to live when we support > IN (sub-SELECT) constructs. +1 for removal. -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes: >> pgrowlocks tells you about row lock states, which maybe is not that >> interesting for security, but still it's information that one wouldn't >> expect to be exposed to someone who isn't allowed to read the table. >> I suppose knowing the number of live tuples might in itself be >> sensitive information. > Here I think the advantage of being able to run this as a non-superuser > (and thus not have the superuser password on the client machine) outweighs > any data which can be reverse-engineered from the lock information. I have no objection to knocking this down to demanding only SELECT privs on the table. It's hard to think that it is OK to be totally unsecured. > Hmmm, we can't really require anything greater than SELECT permission for > dbsize. That's OK for individual tables, but we have no equivalent concept for whole databases or tablespaces. What do you propose for them? regards, tom lane
Tom, > That's OK for individual tables, but we have no equivalent concept for > whole databases or tablespaces. What do you propose for them? CONNECT, of course. There are no permissions for tablespaces except CREATE, so presumably if the user can connect to the db, they ought to be able to see the size of the tablespaces. Although it seems like SCHEMA USAGE ought to fit in there somewhere, I just don't know how. -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes: > Tom, >> That's OK for individual tables, but we have no equivalent concept for >> whole databases or tablespaces. What do you propose for them? > CONNECT, of course. There are no permissions for tablespaces except > CREATE, so presumably if the user can connect to the db, they ought to be > able to see the size of the tablespaces. Uh, if you agree that a user shouldn't be able to find out the size of a table he can't read, why is it OK to find out the size of the database or tablespace containing it? In typical cases where there's just one or a few large tables, the latter number would go far towards giving you the former. For instance, if you are allowed to access all but one table in a DB, subtraction will give you its size exactly. In the same way at the next level up, access to tablespace stats would tell a user a lot about the size of databases he isn't supposed to have access to. [ thinks for awhile... ] It does strike me that there's a good reason for not restricting the table-size functions: anyone who can look at pg_class.relpages can get a maybe-not-up-to-date version of the same number. There are probably some corner cases where exact knowledge is more interesting than approximate knowledge, but it's hard to justify a blanket prohibition. So maybe we should forget the restriction on the table size functions. However, the pg_class argument doesn't extend to letting someone find out the size of a database he's not supposed to be able to get into. One possibility would be to make the tablespace size function silently ignore any per-database subdirectories for which the current user doesn't have CONNECT privilege. I'm not exactly thrilled with that, though --- functions that silently lie to you are a great foot-gun. Another idea is to not hardwire any restriction into the C code, but instead have initdb revoke the default public execute access on the tablespace size function. It would still work for superusers, and a particular DBA could choose to grant execute to trustworthy people. The problem is that you'd have to repeat the grant over and over (in particular, pg_dump wouldn't save its effects). regards, tom lane
Tom, > Another idea is to not hardwire any restriction into the C code, but > instead have initdb revoke the default public execute access on the > tablespace size function. It would still work for superusers, and > a particular DBA could choose to grant execute to trustworthy people. > The problem is that you'd have to repeat the grant over and over > (in particular, pg_dump wouldn't save its effects). Yeah, the big issue here is that Tablespaces do not have any kind of "read" permission, so there's nothing for us to go by. There's a good reason for them not to, since they're orthagonal to schema and databases, but it leaves us without a "handle" for tablespace size. On the other hand, how useful is the information that a tablespace is 35GB in size and that includes 16GB of stuff you're not allowed to see? Are we hypothesizing that some attacker would not have CONNECT on a DB, but would know exactly which tables that DB stores on which tablespace? This seems very corner-case. -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes: > Yeah, the big issue here is that Tablespaces do not have any kind of "read" > permission, so there's nothing for us to go by. There's a good reason for > them not to, since they're orthagonal to schema and databases, but it > leaves us without a "handle" for tablespace size. Yeah --- we do have a "usage" permission for tablespaces but it's not clear that that should extend to letting people deduce info about other databases. > On the other hand, how useful is the information that a tablespace is 35GB > in size and that includes 16GB of stuff you're not allowed to see? Are we > hypothesizing that some attacker would not have CONNECT on a DB, but would > know exactly which tables that DB stores on which tablespace? This seems > very corner-case. No, it's exactly the sort of "side channel" that blackhats look for all the time. For instance, many of openssl's recent security fixes have been about closing ways to infer what the program is doing from secondary information, like how long it takes to respond. Now you can argue that approximate database size information simply isn't that useful to an attacker, and maybe that's true. But are we prepared to make a policy decision that we aren't going to try to protect such information at all? It's analogous to saying that we don't try to protect system catalog contents, which is a policy that we've held to but it's a frequent source of complaints. Also, we do have one answer for people who complain about that: give mutually untrusting users separate databases. The functions at issue here give some visibility into other databases, with *no* recourse for someone who finds that unhappy-making, short of running multiple postmasters (which is a pretty inefficient solution). regards, tom lane
Tom, > Now you can argue that approximate database size information simply > isn't that useful to an attacker, and maybe that's true. But are > we prepared to make a policy decision that we aren't going to try to > protect such information at all? But it's not making *no* attempt. This is a special case; it only applies when a limited number of databases share the same tablespace. If the admin is concerned about protecting private info about database size, then either put the DBs in separate tablespaces, or make sure there's so many dbs in the tablespace that no useful information can be derived. Hmmm ... execept we're not requiring even permission on *one* DB in the tablespace are we? That *is* an issue. How difficult would it be to require that the requestor have CONNECT on at least one DB in the tablespace? Like by requiring them to be connected to that DB, or to be the Superuser? -- Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes: > Hmmm ... execept we're not requiring even permission on *one* DB in the > tablespace are we? The status-quo-ante was that any user could get the number for any database and/or any tablespace. I'm prepared to admit that what I committed is too strong, but no restriction at all still seems too weak. > How difficult would it be to require > that the requestor have CONNECT on at least one DB in the tablespace? ... in particular, that restriction seems pretty content-free for most practical layouts. And it's got interesting security behaviors: DBA A, by more-or-less innocently allowing some tables in his database B to be created in tablespace C, might be allowing his unrelated user D to find out info about some other database E that shares use of C. I'd like there to have to be some direct, intended connection of D to E before D can measure E's size ... regards, tom lane
Tom Lane wrote: > btreefuncs.c is a security hole a mile wide: it will happily dump the > entire data content of an index for you. It's a good thing this hasn't > shipped in any release yet. While we could possibly make it look up > the index's parent table and check if you have SELECT privilege on > that, it'd be easier just to make the functions demand superuser > privilege, which is what the rest of the functions in this contrib > module require. Comments? Oh dear. Those functions were actually just moved from pgstattuple, and has been there since 8.2. Better backpatch that to the pgstattuple functions in 8.2. It didn't occur to me to check the permissions on the existing functions while I added the new ones. I doubt there's any tools out there using those functions, so restricting them to superuser only is probably ok. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Tom, > ... in particular, that restriction seems pretty content-free for most > practical layouts. And it's got interesting security behaviors: > DBA A, by more-or-less innocently allowing some tables in his database B > to be created in tablespace C, might be allowing his unrelated user D to > find out info about some other database E that shares use of C. I'd > like there to have to be some direct, intended connection of D to E > before D can measure E's size ... Well, that puts us back in the position of requiring a "read" or "metadata" permission for tablespaces, or requiring superuser access. The latter is unpalatable because there are existing tools in the field which work without superuser access; the former is troublesome because it wouldn't be used for anything other than the dbsize function, at least not right now. -- Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes: > Well, that puts us back in the position of requiring a "read" or "metadata" > permission for tablespaces, or requiring superuser access. The latter is > unpalatable because there are existing tools in the field which work without > superuser access; the former is troublesome because it wouldn't be used for > anything other than the dbsize function, at least not right now. Well, what about something like the following: * no restriction on table-size function (on the grounds that you could look into pg_class) * no restriction on database-size function *when applied to the current database* (again, you could look into pg_class); to apply to some other database, you must have connect privileges. (Actually, on the assumption that you must have connect privs to current DB, I guess we could simplify that to connect privs on target DB, full stop.) * tablespace-size function requires being owner of current DB. There is nothing particularly principled about the last choice, but it's not superuser and not wide open either. Another option for tablespace-size is that you must have CREATE on the target tablespace. This is also not real principled, since CREATE is more of a "write" privilege than a "read" one, but it'd at least give DBAs some tool to work with. Also, I don't see anything very wrong with defining USAGE on a tablespace to mean that you can use the tablespace-size function on it. AFAIR the overhead for allowing an existing privilege keyword to apply to another type of object is just about nil --- a couple macro changes. With any of these three options, the tablespace-size function would change from "allowed to anyone" to "not allowed by default", unless you happened to be the DB owner. Not sure which would be preferable from the point of view of those existing tools you mention. regards, tom lane
Tom Lane wrote: > * no restriction on database-size function *when applied to the current > database* (again, you could look into pg_class); to apply to some other > database, you must have connect privileges. (Actually, on the > assumption that you must have connect privs to current DB, I guess we > could simplify that to connect privs on target DB, full stop.) The latter would be preferrable for pgAdmin which queries database-level info from the maintenance DB (usually postgres). > * tablespace-size function requires being owner of current DB. I assume superusers will also be able to use it, not just the actual owner? /D
Dave Page <dpage@postgresql.org> writes: > Tom Lane wrote: >> * tablespace-size function requires being owner of current DB. > I assume superusers will also be able to use it, not just the actual owner? Right --- it'd be an "ownercheck" call which means that superusers and anyone who's been granted membership in the owning role will succeed, not just exact matches to the role OID. However the privilege-bit alternatives might be easier to manage. regards, tom lane
We seem to be down to arguing about what permissions are needed to execute the tablespace-size functions. I wrote: > * tablespace-size function requires being owner of current DB. > There is nothing particularly principled about the last choice, but > it's not superuser and not wide open either. > Another option for tablespace-size is that you must have CREATE on the > target tablespace. This is also not real principled, since CREATE is > more of a "write" privilege than a "read" one, but it'd at least give > DBAs some tool to work with. > Also, I don't see anything very wrong with defining USAGE on a > tablespace to mean that you can use the tablespace-size function on it. > AFAIR the overhead for allowing an existing privilege keyword to apply > to another type of object is just about nil --- a couple macro changes. Bruce and I were just chatting about this, and we came up with a couple of fairly good arguments against the first option (that DB owners can use it): * DB owners are supposed to have special permissions within their own DB, but not necessarily system-wide; so being a DB owner shouldn't in itself grant privileges to check tablespace sizes for tablespaces you otherwise have no access to. * Dave was concerned about being able to use tools that connect to postgres or template1 by default. Well, those databases are almost certainly going to be owned by postgres, so for such a tool to be able to use a DB-owner-only function, it's gonna have to be superuser or darn near. So I think I don't like that idea at all. And the third one does seem like overkill. Bruce likes the CREATE-privilege option on the grounds that if you have the right to make tables in a tablespace, you should have the right to find out how big it is. From a security point of view, you can find it out indirectly if you have some idea how big a disk partition it's mounted on: fill up a temp table until you run out of disk space, and subtract. So trying to deny the knowledge seems a bit pointless anyway. Therefore, I propose the same choices as before for table-size (no restriction) and database-size (must have CONNECT priv), and this for tablespace-size: you must have the ability to create tables in the target tablespace. This could be either an explicit CREATE grant, or implicit because you are in a DB that has it as the default tablespace. Comments? regards, tom lane
"Heikki Linnakangas" <heikki@enterprisedb.com> writes: > Tom Lane wrote: >> btreefuncs.c is a security hole a mile wide: it will happily dump the >> entire data content of an index for you. It's a good thing this hasn't >> shipped in any release yet. While we could possibly make it look up >> the index's parent table and check if you have SELECT privilege on >> that, it'd be easier just to make the functions demand superuser >> privilege, which is what the rest of the functions in this contrib >> module require. Comments? > Oh dear. Those functions were actually just moved from pgstattuple, and > has been there since 8.2. Better backpatch that to the pgstattuple > functions in 8.2. Done, thanks for the heads-up. > I doubt there's any tools out there using those functions, so > restricting them to superuser only is probably ok. If anyone complains, we could probably knock them down to SELECT privilege, but finding the parent table to apply the select priv check to seems a bit of a pain for the index cases. So I won't bother unless someone does complain. regards, tom lane
On Tuesday 28 August 2007 15:38, Tom Lane wrote: > Therefore, I propose the same choices as before for table-size (no > restriction) and database-size (must have CONNECT priv), and this > for tablespace-size: you must have the ability to create tables in > the target tablespace. This could be either an explicit CREATE > grant, or implicit because you are in a DB that has it as the default > tablespace. +1 -- Josh Berkus PostgreSQL @ Sun San Francisco