Checking for USAGE on SET search_path...
This patch does two things:
1) Changes the semantics of assign_search_path()/'SET search_path' so
that you can't set your search path to a schema you don't have USAGE
privs for.
2) Changes psql's \dn query and its schema tab completion query to
incorporate ACL checking so that \dn only lists schemas that a user has
USAGE privs on.
Rationale:
1) "SET search_path = 'noaccess';" shouldn't look like it succeeds when
the user doesn't have USAGE privs in the schema 'noaccess'. Currently,
the SET command succeeds and a list of objects in the schema also works
leading the user to believe that they're working in an empty schema.
'SET search_path' should have its behavior modified so that it fails
the same way that on the file system when an unprivileged user types
'cd noaccess'.
2) If a users doesn't have access to a schema, they shouldn't see it in
the listings provided by psql(1). If a user wants to know, they can
query the catalogs by hand (until there's some kind of row level
security on the system catalogs?), but what they don't have access to,
probably doesn't interest them or they shouldn't have flaunted in their
face. This also changes the behavior of tab completion for schemas.
Problems:
% psql test usr
test=> SET search_path = public,foo;
ERROR: permission denied for schema foo
test=> \c test dba
You are now connected to database "test" as user "dba".
test=# ALTER USER usr SET search_path = 'foo';
ALTER USER
test=# \c test usr
You are now connected to database "test" as user "usr".
test=> \dn
List of schemas
Name | Owner
--------------------+-------
information_schema | sean
pg_catalog | sean
public | sean
(3 rows)
test=> show search_path ;
search_path
-------------
foo
(1 row)
And that's the only problem I've found with this patch, but I wasn't
about ready to put in the extra foo in the ALTER USER command to have
it prevent an invalid search_path from being set. -sc
Attachments:
patch.txttext/plain; name=patch.txt; x-unix-mode=0644Download
Index: src/backend/catalog/namespace.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/catalog/namespace.c,v
retrieving revision 1.63
diff -u -r1.63 namespace.c
--- src/backend/catalog/namespace.c 13 Feb 2004 01:08:20 -0000 1.63
+++ src/backend/catalog/namespace.c 9 Apr 2004 02:57:36 -0000
@@ -1806,8 +1806,7 @@
/*
* Verify that all the names are either valid namespace names or
* "$user". We do not require $user to correspond to a valid
- * namespace. We do not check for USAGE rights, either; should
- * we?
+ * namespace.
*
* When source == PGC_S_TEST, we are checking the argument of an
* ALTER DATABASE SET or ALTER USER SET command. It could be that
@@ -1821,12 +1820,11 @@
if (strcmp(curname, "$user") == 0)
continue;
- if (!SearchSysCacheExists(NAMESPACENAME,
- CStringGetDatum(curname),
- 0, 0, 0))
- ereport((source == PGC_S_TEST) ? NOTICE : ERROR,
- (errcode(ERRCODE_UNDEFINED_SCHEMA),
- errmsg("schema \"%s\" does not exist", curname)));
+
+ /* Test to make sure the schema exists and
+ * that the user has USAGE privs on the
+ * schema. */
+ LookupExplicitNamespace(curname);
}
}
Index: src/bin/psql/describe.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/describe.c,v
retrieving revision 1.96
diff -u -r1.96 describe.c
--- src/bin/psql/describe.c 6 Apr 2004 04:05:17 -0000 1.96
+++ src/bin/psql/describe.c 9 Apr 2004 02:57:36 -0000
@@ -1612,7 +1612,9 @@
"FROM pg_catalog.pg_namespace n LEFT JOIN pg_catalog.pg_user u\n"
" ON n.nspowner=u.usesysid\n"
"WHERE (n.nspname NOT LIKE 'pg\\\\_temp\\\\_%%' OR\n"
- " n.nspname = (pg_catalog.current_schemas(true))[1])\n", /* temp schema is first */
+ " n.nspname = (pg_catalog.current_schemas(true))[1]) AND\n" /* temp schema is first */
+ " pg_catalog.has_schema_privilege(n.nspname, 'USAGE')",
+
_("Name"),
_("Owner"));
processNamePattern(&buf, pattern, true, false,
Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.104
diff -u -r1.104 tab-complete.c
--- src/bin/psql/tab-complete.c 5 Apr 2004 03:02:09 -0000 1.104
+++ src/bin/psql/tab-complete.c 9 Apr 2004 02:57:37 -0000
@@ -341,7 +341,8 @@
#define Query_for_list_of_schemas \
"SELECT pg_catalog.quote_ident(nspname) FROM pg_catalog.pg_namespace "\
-" WHERE substring(pg_catalog.quote_ident(nspname),1,%d)='%s'"
+" WHERE substring(pg_catalog.quote_ident(nspname),1,%d)='%s' "\
+" AND pg_catalog.has_schema_privilege(n.nspname, 'USAGE')"
#define Query_for_list_of_system_relations \
"SELECT pg_catalog.quote_ident(relname) "\
@@ -349,7 +350,8 @@
" WHERE c.relkind IN ('r', 'v', 's', 'S') "\
" AND substring(pg_catalog.quote_ident(relname),1,%d)='%s' "\
" AND c.relnamespace = n.oid "\
-" AND n.nspname = 'pg_catalog'"
+" AND n.nspname = 'pg_catalog'"\
+" AND pg_catalog.has_schema_privilege(n.nspname, 'USAGE')"
#define Query_for_list_of_users \
" SELECT pg_catalog.quote_ident(usename) "\
@@ -1546,7 +1548,7 @@
appendPQExpBuffer(&query_buffer, "\nUNION\n"
"SELECT pg_catalog.quote_ident(n.nspname) || '.' "
"FROM pg_catalog.pg_namespace n "
- "WHERE substring(pg_catalog.quote_ident(n.nspname) || '.',1,%d)='%s'",
+ "WHERE pg_catalog.has_schema_privilege(n.nspname, 'USAGE') AND substring(pg_catalog.quote_ident(n.nspname) || '.',1,%d)='%s'",
string_length, e_text);
appendPQExpBuffer(&query_buffer,
" AND (SELECT pg_catalog.count(*)"
@@ -1562,7 +1564,7 @@
appendPQExpBuffer(&query_buffer, "\nUNION\n"
"SELECT pg_catalog.quote_ident(n.nspname) || '.' || %s "
"FROM %s, pg_catalog.pg_namespace n "
- "WHERE %s = n.oid AND ",
+ "WHERE pg_catalog.has_schema_privilege(n.nspname, 'USAGE') AND %s = n.oid AND ",
qualresult,
completion_squery->catname,
completion_squery->namespace);
Sean Chittenden <sean@chittenden.org> writes:
This patch does two things:
1) Changes the semantics of assign_search_path()/'SET search_path' so
that you can't set your search path to a schema you don't have USAGE
privs for.
Why is that needed? It's already a no-op AFAIR. It also is
incompatible with the existing behavior, in which nonexistent schemas
(think "$user") are dropped silently rather than noisily. Your patch
also breaks the previous careful tweak to allow ALTER DATABASE SET
to succeed when mentioning a schema not present in the current database.
2) Changes psql's \dn query and its schema tab completion query to
incorporate ACL checking so that \dn only lists schemas that a user has
USAGE privs on.
This requires considerable discussion. Should \df only list functions
you are allowed to call? \dt only tables you are allowed to read?
\h only commands you are allowed to execute?
I'm not that thrilled with patches that propose basic changes in
behavior and have not been justified by any preceding discussion
on pghackers...
regards, tom lane
[ Discussion moved from patches@ to hackers@ ]
The gist of the discussion being: what are the ramifications of having
PostgreSQL and psql(1) hide information/schema bits that a user doesn't
have access to. This would have to be backend enforced, which would
mean changing the system catalogs to include some form of row level
security so that SELECTs from the system catalogs would only return the
rows that a user has privs to see. In more practical terms, in
psql(1), the consequences would be:
\dn only shows schemas/namespaces that a user has access to
\df could only show functions that a user can execute (or are visible
by namespace)
\dt only shows tables that you have some kind of privileges on
etc. Not much can be done about \l. That said, what are the thoughts
of the other developers/admins? Is hiding schema information a good
thing? Do people think that the concept of "secure by default" should
apply to schema information inside of the database? Should information
hiding be done in psql(1) or should this be managed by the backend and
all logic kept out of psql(1)? For sure, the advantage of having it
managed in the backend is that *all* clients (psql, pgadmin,
phpPgAdmin, etc.) would pick up the schema structure hiding.
[ Thread from patches@ + response below ]
This patch does two things:
1) Changes the semantics of assign_search_path()/'SET search_path' so
that you can't set your search path to a schema you don't have USAGE
privs for.Why is that needed? It's already a no-op AFAIR. It also is
incompatible with the existing behavior, in which nonexistent schemas
(think "$user") are dropped silently rather than noisily.
Actually, $user still works... and shouldn't:
test=# CREATE SCHEMA usr;
CREATE SCHEMA
test=# \c test usr
You are now connected to database "test" as user "usr".
test=> SET search_path = '$user';
SET
test=> \dn
List of schemas
Name | Owner
--------------------+-------
information_schema | sean
pg_catalog | sean
public | sean
(3 rows)
When the list element is '$user', the loop skips processing and doesn't
try and resolve $user to usr and then test to see if usr exists.
Your patch
also breaks the previous careful tweak to allow ALTER DATABASE SET
to succeed when mentioning a schema not present in the current
database.
This I haven't investigated... Should you be able to run ALTER
DATABASE SET if you're in a different database? *shrug* I'd say no,
but I'm biased because I've never used ALTER DATABASE SET outside of
the current database. Would it be acceptable to have all SET commands
succeed without checking if the current database is different than the
database that's being acted on? That's no different than the current
behavior.
2) Changes psql's \dn query and its schema tab completion query to
incorporate ACL checking so that \dn only lists schemas that a user
has
USAGE privs on.This requires considerable discussion.
Okey dokey... sending email to hackers@.
Should \df only list functions
you are allowed to call? \dt only tables you are allowed to read?
\h only commands you are allowed to execute?
IMHO, yes... but \h doesn't execute a query and shouldn't exercise any
control on what's shown/displayed. PostgreSQL is a rather open
database to non-admins. If PostgreSQL were a file system and a
database cluster is the root node, the layout would essentially be:
/[databases]/[schemas]/[tables]/[columns]
and the perms on that file system would be:
find / -type d -exec chmod 755 {} \;
find / -type f -exec chmod 600 {} \;
and schemas would be mod 711 (find / -type d -depth 2 -exec chmod 711
{} \;). Users can essentially browse the structure of the entire file
system/database. As an admin (network, system, database, or
otherwise), I want to expose to a user only what they need to be
exposed to. Changing a database/directory so that only a user or group
has access to it (mod 750/700) can only happen through modifications to
pg_hba.conf. *shudders* PostgreSQL makes itself "secure" by default
by turning off network access. Once a user is connected, however,
PostgreSQL is an open book and reminds me of the olden days when
/etc/passwd contained crypt(3)'ed passwords and home directories were
created mod 755 or umasks were 0 by default and a mask of 022 was a
umask for the "overly paranoid."
-sc
--
Sean Chittenden
Sean Chittenden wrote:
Is hiding schema information a good thing? Do people think that the
concept of "secure by default" should apply to schema information
inside of the database? Should information hiding be done in psql(1)
or should this be managed by the backend and all logic kept out of
psql(1)? For sure, the advantage of having it managed in the backend
is that *all* clients (psql, pgadmin, phpPgAdmin, etc.) would pick up
the schema structure hiding.
Anywhere else but the backend is pointless, IMNSHO. You might be
interested in my recent posting about zero knowledge users here:
http://archives.postgresql.org/pgsql-hackers/2004-04/msg00144.php
cheers
andrew
On Sun, 2004-04-11 at 17:10, Sean Chittenden wrote:
Should information hiding be done in psql(1) or should this be managed
by the backend and all logic kept out of psql(1)?
If the intent of this feature is security, it seems totally pointless to
implement it in psql (leaving aside whether it's actually a good idea or
not).
[ WRT to search_path and nonexistent schemas ]
Why is that needed? It's already a no-op AFAIR. It also is
incompatible with the existing behavior, in which nonexistent schemas
(think "$user") are dropped silently rather than noisily.Actually, $user still works..
I think the more important question is: "Why is that needed?"
(Consider the PATH environmental var, which is fairly analogous to
search_path -- that doesn't complain if you add nonexistent directories
to it.)
-Neil
Should information hiding be done in psql(1) or should this be managed
by the backend and all logic kept out of psql(1)?If the intent of this feature is security, it seems totally pointless
to
implement it in psql (leaving aside whether it's actually a good idea
or
not).[ WRT to search_path and nonexistent schemas ]
*nods* I completely agree that the best place for this to happen is in
the backend and not psql.
Why is that needed? It's already a no-op AFAIR. It also is
incompatible with the existing behavior, in which nonexistent schemas
(think "$user") are dropped silently rather than noisily.Actually, $user still works..
I think the more important question is: "Why is that needed?"
Two reasons come to mind. First, If you change your search_path to a
valid schema that you have no access to and try and look for database
objects, you get the impression that its an empty schema and not a
schema that you don't have access to. To prevent this, I changed the
behavior of SET search_path so that it validates its input. A warning
may be appropriate, but I'd rather have the SET search_path fail than
the CREATE [object] fail. Second, SET search_path, in my mind, is
little different than ALTER TABLE ADD CONSTRAINT: it's input can be
validated and permissions can be checked, therefor should it should be.
(Consider the PATH environmental var, which is fairly analogous to
search_path -- that doesn't complain if you add nonexistent directories
to it.)
Actually, search_path is closer in functionality to a union of the
chdir(2) syscall and the PATH environment variable. Any argument to
chdir(2) is validated by the operating system and chdir(2) is a system
call - not a library call - for this very reason. Can you imagine a
world where chdir(2) didn't validate the existence of directories as
well as the permissions?
-sc
--
Sean Chittenden
Sean Chittenden <sean@chittenden.org> writes:
Two reasons come to mind. First, If you change your search_path to a
valid schema that you have no access to and try and look for database
objects, you get the impression that its an empty schema and not a
schema that you don't have access to. To prevent this, I changed the
behavior of SET search_path so that it validates its input.
You can't actually do that. In many (most?) situations, the search_path
value is fixed before a backend even starts; if you try to error out
because you don't like the contents, you'll prevent backends from
starting at all.
Also consider the situation where backend A creates, deletes, or changes
the permissions on schemas that are mentioned in backend B's search
path. In the existing code these cases behave consistently and much
the same as Unix PATH searching does: at all times your effective path
consists of those elements of PATH that actually exist and are readable.
It would be possible to make interactive SET behave differently from the
non-interactive case, but I don't think that would be an improvement in
understandability or usability. It's certainly not worth doing if the
only argument for changing is the one you give above.
regards, tom lane
I said:
Sean Chittenden <sean@chittenden.org> writes:
To prevent this, I changed the
behavior of SET search_path so that it validates its input.
... It would be possible to make interactive SET behave differently
from the non-interactive case,
Wait a minute --- scratch what I said above; interactive "SET
search_path" already does behave differently from noninteractive.
So what did your patch change exactly?
regards, tom lane
To prevent this, I changed the
behavior of SET search_path so that it validates its input.... It would be possible to make interactive SET behave differently
from the non-interactive case,Wait a minute --- scratch what I said above; interactive "SET
search_path" already does behave differently from noninteractive.
So what did your patch change exactly?
I think (don't know all of the ways there are to SET search_path), the
interactive way to SET search_path. :) It changed assign_search_path
in catalog/namespace.c, iirc. I said in my original email that someone
could still do ALTER USER SET search_path and have that work without
checking.
-sc
--
Sean Chittenden