Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 93a6be6

Browse files
committed
Revise the permission checking on user mapping DDL commands.
CREATE/ALTER/DROP USER MAPPING are now allowed either by the server owner or by a user with USAGE privileges for his own user name. This is more or less what the SQL standard wants anyway (plus "implementation-defined") Hide information_schema.user_mapping_options.option_value, unless the current user is the one associated with the user mapping, or is the server owner and the mapping is for PUBLIC, or is a superuser. This is to protect passwords. Also, fix a bug in information_schema._pg_foreign_servers, which hid servers using wrappers where the current user did not have privileges on the wrapper. The correct behavior is to hide servers where the current user has no privileges on the server.
1 parent fe62698 commit 93a6be6

File tree

8 files changed

+89
-46
lines changed

8 files changed

+89
-46
lines changed

doc/src/sgml/information_schema.sgml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/information_schema.sgml,v 1.36 2008/12/19 16:25:16 petere Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/information_schema.sgml,v 1.37 2009/01/20 09:10:20 petere Exp $ -->
22

33
<chapter id="information-schema">
44
<title>The Information Schema</title>
@@ -5081,7 +5081,12 @@ ORDER BY c.ordinal_position;
50815081
<row>
50825082
<entry><literal>option_value</literal></entry>
50835083
<entry><type>character_data</type></entry>
5084-
<entry>Value of the option</entry>
5084+
<entry>Value of the option. This column will show as null
5085+
unless the current user is the user being mapped, or the mapping
5086+
is for <literal>PUBLIC</literal> and the current user is the
5087+
server owner, or the current user is a superuser. The intent is
5088+
to protect password information stored as user mapping
5089+
option.</entry>
50855090
</row>
50865091
</tbody>
50875092
</tgroup>

doc/src/sgml/ref/alter_user_mapping.sgml

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/alter_user_mapping.sgml,v 1.1 2008/12/19 16:25:16 petere Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/alter_user_mapping.sgml,v 1.2 2009/01/20 09:10:20 petere Exp $
33
PostgreSQL documentation
44
-->
55

@@ -31,10 +31,15 @@ ALTER USER MAPPING FOR { <replaceable class="parameter">username</replaceable> |
3131

3232
<para>
3333
<command>ALTER USER MAPPING</command> changes the definition of a
34-
user mapping. Only the owner of the server can change the user
35-
mappings of that server.
34+
user mapping.
3635
</para>
3736

37+
<para>
38+
The owner of a foreign server can alter user mappings for that
39+
server for any user. Also, a user can alter a user mapping for
40+
his own user name if <literal>USAGE</> privilege on the server has
41+
been granted to the user.
42+
</para>
3843
</refsect1>
3944

4045
<refsect1>

doc/src/sgml/ref/create_user_mapping.sgml

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/create_user_mapping.sgml,v 1.2 2009/01/17 04:24:41 neilc Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/create_user_mapping.sgml,v 1.3 2009/01/20 09:10:20 petere Exp $
33
PostgreSQL documentation
44
-->
55

@@ -31,10 +31,15 @@ CREATE USER MAPPING FOR { <replaceable class="parameter">username</replaceable>
3131

3232
<para>
3333
<command>CREATE USER MAPPING</command> defines a mapping of a user
34-
to a foreign server. You must be the owner of the server to define
35-
user mappings for it.
34+
to a foreign server.
3635
</para>
3736

37+
<para>
38+
The owner of a foreign server can create user mappings for that
39+
server for any user. Also, a user can create a user mapping for
40+
his own user name if <literal>USAGE</> privilege on the server has
41+
been granted to the user.
42+
</para>
3843
</refsect1>
3944

4045
<refsect1>

doc/src/sgml/ref/drop_user_mapping.sgml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/drop_user_mapping.sgml,v 1.1 2008/12/19 16:25:16 petere Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/drop_user_mapping.sgml,v 1.2 2009/01/20 09:10:20 petere Exp $
33
PostgreSQL documentation
44
-->
55

@@ -29,8 +29,14 @@ DROP USER MAPPING [ IF EXISTS ] FOR { <replaceable class="parameter">username</r
2929

3030
<para>
3131
<command>DROP USER MAPPING</command> removes an existing user
32-
mapping from foreign server. To execute this command, the current
33-
user must be the owner of the server containing the mapping.
32+
mapping from foreign server.
33+
</para>
34+
35+
<para>
36+
The owner of a foreign server can drop user mappings for that server
37+
for any user. Also, a user can drop a user mapping for his own
38+
user name if <literal>USAGE</> privilege on the server has been
39+
granted to the user.
3440
</para>
3541
</refsect1>
3642

src/backend/catalog/information_schema.sql

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*
55
* Copyright (c) 2003-2009, PostgreSQL Global Development Group
66
*
7-
* $PostgreSQL: pgsql/src/backend/catalog/information_schema.sql,v 1.49 2009/01/14 21:12:09 petere Exp $
7+
* $PostgreSQL: pgsql/src/backend/catalog/information_schema.sql,v 1.50 2009/01/20 09:10:20 petere Exp $
88
*/
99

1010
/*
@@ -2465,12 +2465,12 @@ CREATE VIEW _pg_foreign_servers AS
24652465
s.srvoptions,
24662466
CAST(current_database() AS sql_identifier) AS foreign_server_catalog,
24672467
CAST(srvname AS sql_identifier) AS foreign_server_name,
2468-
w.foreign_data_wrapper_catalog,
2469-
w.foreign_data_wrapper_name,
2468+
CAST(current_database() AS sql_identifier) AS foreign_data_wrapper_catalog,
2469+
CAST(w.fdwname AS sql_identifier) AS foreign_data_wrapper_name,
24702470
CAST(srvtype AS character_data) AS foreign_server_type,
24712471
CAST(srvversion AS character_data) AS foreign_server_version,
24722472
CAST(u.rolname AS sql_identifier) AS authorization_identifier
2473-
FROM pg_foreign_server s, _pg_foreign_data_wrappers w, pg_authid u
2473+
FROM pg_foreign_server s, pg_foreign_data_wrapper w, pg_authid u
24742474
WHERE w.oid = s.srvfdw
24752475
AND u.oid = s.srvowner
24762476
AND (pg_has_role(s.srvowner, 'USAGE')
@@ -2512,9 +2512,11 @@ GRANT SELECT ON foreign_servers TO PUBLIC;
25122512
CREATE VIEW _pg_user_mappings AS
25132513
SELECT um.oid,
25142514
um.umoptions,
2515+
um.umuser,
25152516
CAST(COALESCE(u.rolname,'PUBLIC') AS sql_identifier ) AS authorization_identifier,
25162517
s.foreign_server_catalog,
2517-
s.foreign_server_name
2518+
s.foreign_server_name,
2519+
s.authorization_identifier AS srvowner
25182520
FROM pg_user_mapping um LEFT JOIN pg_authid u ON (u.oid = um.umuser),
25192521
_pg_foreign_servers s
25202522
WHERE s.oid = um.umserver;
@@ -2529,7 +2531,10 @@ CREATE VIEW user_mapping_options AS
25292531
foreign_server_catalog,
25302532
foreign_server_name,
25312533
CAST((pg_options_to_table(um.umoptions)).option_name AS sql_identifier) AS option_name,
2532-
CAST((pg_options_to_table(um.umoptions)).option_value AS character_data) AS option_value
2534+
CAST(CASE WHEN (umuser <> 0 AND authorization_identifier = current_user)
2535+
OR (umuser = 0 AND pg_has_role(srvowner, 'USAGE'))
2536+
OR (SELECT rolsuper FROM pg_authid WHERE rolname = current_user) THEN (pg_options_to_table(um.umoptions)).option_value
2537+
ELSE NULL END AS character_data) AS option_value
25332538
FROM _pg_user_mappings um;
25342539

25352540
GRANT SELECT ON user_mapping_options TO PUBLIC;

src/backend/commands/foreigncmds.c

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/commands/foreigncmds.c,v 1.4 2009/01/01 17:23:38 momjian Exp $
10+
* $PostgreSQL: pgsql/src/backend/commands/foreigncmds.c,v 1.5 2009/01/20 09:10:20 petere Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -828,6 +828,33 @@ RemoveForeignServerById(Oid srvId)
828828
}
829829

830830

831+
/*
832+
* Common routine to check permission for user-mapping-related DDL
833+
* commands. We allow server owners to operate on any mapping, and
834+
* users to operate on their own mapping.
835+
*/
836+
static void
837+
user_mapping_ddl_aclcheck(Oid umuserid, Oid serverid, const char *servername)
838+
{
839+
Oid curuserid = GetUserId();
840+
841+
if (!pg_foreign_server_ownercheck(serverid, curuserid))
842+
{
843+
if (umuserid == curuserid)
844+
{
845+
AclResult aclresult;
846+
847+
aclresult = pg_foreign_server_aclcheck(serverid, curuserid, ACL_USAGE);
848+
if (aclresult != ACLCHECK_OK)
849+
aclcheck_error(aclresult, ACL_KIND_FOREIGN_SERVER, servername);
850+
}
851+
else
852+
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FOREIGN_SERVER,
853+
servername);
854+
}
855+
}
856+
857+
831858
/*
832859
* Create user mapping
833860
*/
@@ -841,24 +868,17 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
841868
HeapTuple tuple;
842869
Oid useId;
843870
Oid umId;
844-
Oid ownerId;
845871
ObjectAddress myself;
846872
ObjectAddress referenced;
847873
ForeignServer *srv;
848874
ForeignDataWrapper *fdw;
849875

850-
ownerId = GetUserId();
851-
852876
useId = GetUserOidFromMapping(stmt->username, false);
853877

854-
/*
855-
* Check that the server exists and that the we own it.
856-
*/
878+
/* Check that the server exists. */
857879
srv = GetForeignServerByName(stmt->servername, false);
858880

859-
if (!pg_foreign_server_ownercheck(srv->serverid, ownerId))
860-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FOREIGN_SERVER,
861-
stmt->servername);
881+
user_mapping_ddl_aclcheck(useId, srv->serverid, stmt->servername);
862882

863883
/*
864884
* Check that the user mapping is unique within server.
@@ -951,12 +971,7 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
951971
errmsg("user mapping \"%s\" does not exist for the server",
952972
MappingUserName(useId))));
953973

954-
/*
955-
* Must be owner of the server to alter user mapping.
956-
*/
957-
if (!pg_foreign_server_ownercheck(srv->serverid, GetUserId()))
958-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FOREIGN_SERVER,
959-
stmt->servername);
974+
user_mapping_ddl_aclcheck(useId, srv->serverid, stmt->servername);
960975

961976
tp = SearchSysCacheCopy(USERMAPPINGOID,
962977
ObjectIdGetDatum(umId),
@@ -1071,14 +1086,7 @@ RemoveUserMapping(DropUserMappingStmt *stmt)
10711086
return;
10721087
}
10731088

1074-
/*
1075-
* Only allow DROP if we own the server.
1076-
*/
1077-
if (!pg_foreign_server_ownercheck(srv->serverid, GetUserId()))
1078-
{
1079-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_FOREIGN_SERVER,
1080-
srv->servername);
1081-
}
1089+
user_mapping_ddl_aclcheck(useId, srv->serverid, srv->servername);
10821090

10831091
/*
10841092
* Do the deletion

src/test/regress/expected/foreign_data.out

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ SET ROLE regress_test_role;
545545
CREATE USER MAPPING FOR current_user SERVER s5;
546546
CREATE USER MAPPING FOR current_user SERVER s6 OPTIONS (username 'test');
547547
CREATE USER MAPPING FOR current_user SERVER s7; -- ERROR
548-
ERROR: must be owner of foreign server s7
548+
ERROR: permission denied for foreign server s7
549549
CREATE USER MAPPING FOR public SERVER s8; -- ERROR
550550
ERROR: must be owner of foreign server s8
551551
RESET ROLE;
@@ -736,6 +736,13 @@ SELECT * FROM information_schema.role_usage_grants WHERE object_type LIKE 'FOREI
736736
(2 rows)
737737

738738
DROP USER MAPPING FOR current_user SERVER st1;
739+
SET ROLE regress_test_role2;
740+
SELECT * FROM information_schema.user_mapping_options ORDER BY 1, 2, 3, 4;
741+
authorization_identifier | foreign_server_catalog | foreign_server_name | option_name | option_value
742+
--------------------------+------------------------+---------------------+-------------+--------------
743+
regress_test_role | regression | s6 | username |
744+
(1 row)
745+
739746
RESET ROLE;
740747
-- has_foreign_data_wrapper_privilege
741748
SELECT has_foreign_data_wrapper_privilege('regress_test_role',
@@ -932,8 +939,7 @@ ALTER SERVER s9 VERSION '1.2'; -- ERROR
932939
ERROR: must be owner of foreign server s9
933940
GRANT USAGE ON FOREIGN SERVER s9 TO regress_test_role; -- WARNING
934941
WARNING: no privileges were granted for "s9"
935-
CREATE USER MAPPING FOR current_user SERVER s9; -- ERROR
936-
ERROR: must be owner of foreign server s9
942+
CREATE USER MAPPING FOR current_user SERVER s9;
937943
DROP SERVER s9 CASCADE; -- ERROR
938944
ERROR: must be owner of foreign server s9
939945
RESET ROLE;
@@ -953,11 +959,12 @@ NOTICE: drop cascades to user mapping for public
953959
DROP SERVER st2;
954960
DROP USER MAPPING FOR regress_test_role SERVER s6;
955961
DROP FOREIGN DATA WRAPPER foo CASCADE;
956-
NOTICE: drop cascades to 4 other objects
962+
NOTICE: drop cascades to 5 other objects
957963
DETAIL: drop cascades to server s4
958964
drop cascades to user mapping for foreign_data_user
959965
drop cascades to server s6
960966
drop cascades to server s9
967+
drop cascades to user mapping for unprivileged_role
961968
DROP SERVER s8 CASCADE;
962969
NOTICE: drop cascades to 2 other objects
963970
DETAIL: drop cascades to user mapping for foreign_data_user

src/test/regress/sql/foreign_data.sql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,8 @@ SELECT * FROM information_schema.user_mapping_options ORDER BY 1, 2, 3, 4;
273273
SELECT * FROM information_schema.usage_privileges WHERE object_type LIKE 'FOREIGN%' ORDER BY 1, 2, 3, 4, 5;
274274
SELECT * FROM information_schema.role_usage_grants WHERE object_type LIKE 'FOREIGN%' ORDER BY 1, 2, 3, 4, 5;
275275
DROP USER MAPPING FOR current_user SERVER st1;
276+
SET ROLE regress_test_role2;
277+
SELECT * FROM information_schema.user_mapping_options ORDER BY 1, 2, 3, 4;
276278
RESET ROLE;
277279

278280

@@ -365,7 +367,7 @@ GRANT USAGE ON FOREIGN SERVER s9 TO unprivileged_role;
365367
SET ROLE unprivileged_role;
366368
ALTER SERVER s9 VERSION '1.2'; -- ERROR
367369
GRANT USAGE ON FOREIGN SERVER s9 TO regress_test_role; -- WARNING
368-
CREATE USER MAPPING FOR current_user SERVER s9; -- ERROR
370+
CREATE USER MAPPING FOR current_user SERVER s9;
369371
DROP SERVER s9 CASCADE; -- ERROR
370372
RESET ROLE;
371373

0 commit comments

Comments
 (0)