git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

Fix "unexpected relkind" error when denying permissions on toast tables.

Tue, 5 Nov 2019 18:40:37 +0000 (13:40 -0500)

get_relkind_objtype, and hence get_object_type, failed when applied to a
toast table. This is not a good thing, because it prevents reporting of
perfectly legitimate permissions errors. (At present, these functions
are in fact *only* used to determine the ObjectType argument for
acl_error() calls.) It seems best to have them fall back to returning
OBJECT_TABLE in every case where they can't determine an object type
for a pg_class entry, so do that.

In passing, make some edits to alter.c to make it more obvious that
those calls of get_object_type() are used only for error reporting.
This might save a few cycles in the non-error code path, too.

Back-patch to v11 where this issue originated.

John Hsu, Michael Paquier, Tom Lane

Discussion: https://postgr.es/m/C652D3DF-2B0C-4128-9420-FB5379F6B1E4@amazon.com

src/backend/catalog/objectaddress.c patch | blob | blame | history

src/backend/commands/alter.c patch | blob | blame | history

src/test/regress/expected/create_index.out patch | blob | blame | history

src/test/regress/sql/create_index.sql patch | blob | blame | history

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c

index f5c647a86459c0f09690bd94cc09ce0546525c2a..8f6e8f05e03e85d919b6bbd73f155adb3e08714f 100644 (file)

--- a/src/backend/catalog/objectaddress.c

+++ b/src/backend/catalog/objectaddress.c

@@ -2564,6 +2564,13 @@ get_object_attnum_acl(Oid class_id)

return prop->attnum_acl;

}

+/*

+ * get_object_type

+ *

+ * Return the object type associated with a given object. This routine

+ * is primarily used to determine the object type to mention in ACL check

+ * error messages, so it's desirable for it to avoid failing.

+ */

ObjectType

get_object_type(Oid class_id, Oid object_id)

{

@@ -5274,6 +5281,16 @@ strlist_to_textarray(List *list)

return arr;

}

+/*

+ * get_relkind_objtype

+ *

+ * Return the object type for the relkind given by the caller.

+ *

+ * If an unexpected relkind is passed, we say OBJECT_TABLE rather than

+ * failing. That's because this is mostly used for generating error messages

+ * for failed ACL checks on relations, and we'd rather produce a generic

+ * message saying "table" than fail entirely.

+ */

ObjectType

get_relkind_objtype(char relkind)

{

@@ -5293,13 +5310,10 @@ get_relkind_objtype(char relkind)

return OBJECT_MATVIEW;

case RELKIND_FOREIGN_TABLE:

return OBJECT_FOREIGN_TABLE;

- /*

- * other relkinds are not supported here because they don't map to

- * OBJECT_* values

- */

+ case RELKIND_TOASTVALUE:

+ return OBJECT_TABLE;

default:

- elog(ERROR, "unexpected relkind: %d", relkind);

- return 0;

+ /* Per above, don't raise an error */

+ return OBJECT_TABLE;

}

diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c

index eff325cc7d00286c7259d0cbc688f3a6c4cf5ae3..4d443c13292c9559395cd28306378575e194c3c7 100644 (file)

--- a/src/backend/commands/alter.c

+++ b/src/backend/commands/alter.c

@@ -171,7 +171,6 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name)

AttrNumber Anum_name = get_object_attnum_name(classId);

AttrNumber Anum_namespace = get_object_attnum_namespace(classId);

AttrNumber Anum_owner = get_object_attnum_owner(classId);

- ObjectType objtype = get_object_type(classId, objectId);

HeapTuple oldtup;

HeapTuple newtup;

Datum datum;

@@ -223,7 +222,8 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name)

ownerId = DatumGetObjectId(datum);

if (!has_privs_of_role(GetUserId(), DatumGetObjectId(ownerId)))

- aclcheck_error(ACLCHECK_NOT_OWNER, objtype, old_name);

+ aclcheck_error(ACLCHECK_NOT_OWNER, get_object_type(classId, objectId),

+ old_name);

/* User must have CREATE privilege on the namespace */

if (OidIsValid(namespaceId))

@@ -663,7 +663,6 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)

AttrNumber Anum_name = get_object_attnum_name(classId);

AttrNumber Anum_namespace = get_object_attnum_namespace(classId);

AttrNumber Anum_owner = get_object_attnum_owner(classId);

- ObjectType objtype = get_object_type(classId, objid);

Oid oldNspOid;

Datum name,

namespace;

@@ -719,7 +718,7 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)

ownerId = DatumGetObjectId(owner);

if (!has_privs_of_role(GetUserId(), ownerId))

- aclcheck_error(ACLCHECK_NOT_OWNER, objtype,

+ aclcheck_error(ACLCHECK_NOT_OWNER, get_object_type(classId, objid),

NameStr(*(DatumGetName(name))));

/* User must have CREATE privilege on new namespace */

@@ -942,8 +941,6 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)

/* Superusers can bypass permission checks */

if (!superuser())

{

- ObjectType objtype = get_object_type(classId, objectId);

/* must be owner */

if (!has_privs_of_role(GetUserId(), old_ownerId))

{

@@ -963,7 +960,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)

HeapTupleGetOid(oldtup));

objname = namebuf;

}

- aclcheck_error(ACLCHECK_NOT_OWNER, objtype, objname);

+ aclcheck_error(ACLCHECK_NOT_OWNER, get_object_type(classId, objectId),

+ objname);

}

/* Must be able to become new owner */

check_is_member_of_role(GetUserId(), new_ownerId);

diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out

index be25101db24cbbc673aec6230e436dcf44eca5d8..90b671f5b91d4f15399e736aba1c41fd8173ad42 100644 (file)

--- a/src/test/regress/expected/create_index.out

+++ b/src/test/regress/expected/create_index.out

@@ -3134,8 +3134,17 @@ CREATE ROLE regress_reindexuser NOLOGIN;

SET SESSION ROLE regress_reindexuser;

REINDEX SCHEMA schema_to_reindex;

ERROR: must be owner of schema schema_to_reindex

+-- Permission failures with toast tables and indexes (pg_proc's toast here)

+RESET ROLE;

+GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;

+SET SESSION ROLE regress_reindexuser;

+REINDEX TABLE pg_toast.pg_toast_1255;

+ERROR: must be owner of table pg_toast_1255

+REINDEX INDEX pg_toast.pg_toast_1255_index;

+ERROR: must be owner of index pg_toast_1255_index

-- Clean up

RESET ROLE;

+REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser;

DROP ROLE regress_reindexuser;

\set VERBOSITY terse \\ -- suppress cascade details

DROP SCHEMA schema_to_reindex CASCADE;

diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql

index f9e7118f0d3d864325de765f83e6015f90bbd778..6eee92bf973ca0543dc92b0ee67202dcc9478bee 100644 (file)

--- a/src/test/regress/sql/create_index.sql

+++ b/src/test/regress/sql/create_index.sql

@@ -1126,9 +1126,16 @@ END;

CREATE ROLE regress_reindexuser NOLOGIN;