mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-06 21:19:21 +02:00
Fix "unexpected relkind" error when denying permissions on toast tables.
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
This commit is contained in:
parent
a9db37a180
commit
7918641936
@ -2612,6 +2612,13 @@ get_object_attnum_acl(Oid class_id)
|
|||||||
return prop->attnum_acl;
|
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
|
ObjectType
|
||||||
get_object_type(Oid class_id, Oid object_id)
|
get_object_type(Oid class_id, Oid object_id)
|
||||||
{
|
{
|
||||||
@ -5333,6 +5340,16 @@ strlist_to_textarray(List *list)
|
|||||||
return arr;
|
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
|
ObjectType
|
||||||
get_relkind_objtype(char relkind)
|
get_relkind_objtype(char relkind)
|
||||||
{
|
{
|
||||||
@ -5352,13 +5369,10 @@ get_relkind_objtype(char relkind)
|
|||||||
return OBJECT_MATVIEW;
|
return OBJECT_MATVIEW;
|
||||||
case RELKIND_FOREIGN_TABLE:
|
case RELKIND_FOREIGN_TABLE:
|
||||||
return OBJECT_FOREIGN_TABLE;
|
return OBJECT_FOREIGN_TABLE;
|
||||||
|
case RELKIND_TOASTVALUE:
|
||||||
/*
|
return OBJECT_TABLE;
|
||||||
* other relkinds are not supported here because they don't map to
|
|
||||||
* OBJECT_* values
|
|
||||||
*/
|
|
||||||
default:
|
default:
|
||||||
elog(ERROR, "unexpected relkind: %d", relkind);
|
/* Per above, don't raise an error */
|
||||||
return 0;
|
return OBJECT_TABLE;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -172,7 +172,6 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name)
|
|||||||
AttrNumber Anum_name = get_object_attnum_name(classId);
|
AttrNumber Anum_name = get_object_attnum_name(classId);
|
||||||
AttrNumber Anum_namespace = get_object_attnum_namespace(classId);
|
AttrNumber Anum_namespace = get_object_attnum_namespace(classId);
|
||||||
AttrNumber Anum_owner = get_object_attnum_owner(classId);
|
AttrNumber Anum_owner = get_object_attnum_owner(classId);
|
||||||
ObjectType objtype = get_object_type(classId, objectId);
|
|
||||||
HeapTuple oldtup;
|
HeapTuple oldtup;
|
||||||
HeapTuple newtup;
|
HeapTuple newtup;
|
||||||
Datum datum;
|
Datum datum;
|
||||||
@ -224,7 +223,8 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name)
|
|||||||
ownerId = DatumGetObjectId(datum);
|
ownerId = DatumGetObjectId(datum);
|
||||||
|
|
||||||
if (!has_privs_of_role(GetUserId(), DatumGetObjectId(ownerId)))
|
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 */
|
/* User must have CREATE privilege on the namespace */
|
||||||
if (OidIsValid(namespaceId))
|
if (OidIsValid(namespaceId))
|
||||||
@ -670,7 +670,6 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
|
|||||||
AttrNumber Anum_name = get_object_attnum_name(classId);
|
AttrNumber Anum_name = get_object_attnum_name(classId);
|
||||||
AttrNumber Anum_namespace = get_object_attnum_namespace(classId);
|
AttrNumber Anum_namespace = get_object_attnum_namespace(classId);
|
||||||
AttrNumber Anum_owner = get_object_attnum_owner(classId);
|
AttrNumber Anum_owner = get_object_attnum_owner(classId);
|
||||||
ObjectType objtype = get_object_type(classId, objid);
|
|
||||||
Oid oldNspOid;
|
Oid oldNspOid;
|
||||||
Datum name,
|
Datum name,
|
||||||
namespace;
|
namespace;
|
||||||
@ -726,7 +725,7 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
|
|||||||
ownerId = DatumGetObjectId(owner);
|
ownerId = DatumGetObjectId(owner);
|
||||||
|
|
||||||
if (!has_privs_of_role(GetUserId(), ownerId))
|
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))));
|
NameStr(*(DatumGetName(name))));
|
||||||
|
|
||||||
/* User must have CREATE privilege on new namespace */
|
/* User must have CREATE privilege on new namespace */
|
||||||
@ -950,8 +949,6 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
|
|||||||
/* Superusers can bypass permission checks */
|
/* Superusers can bypass permission checks */
|
||||||
if (!superuser())
|
if (!superuser())
|
||||||
{
|
{
|
||||||
ObjectType objtype = get_object_type(classId, objectId);
|
|
||||||
|
|
||||||
/* must be owner */
|
/* must be owner */
|
||||||
if (!has_privs_of_role(GetUserId(), old_ownerId))
|
if (!has_privs_of_role(GetUserId(), old_ownerId))
|
||||||
{
|
{
|
||||||
@ -970,7 +967,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
|
|||||||
snprintf(namebuf, sizeof(namebuf), "%u", objectId);
|
snprintf(namebuf, sizeof(namebuf), "%u", objectId);
|
||||||
objname = namebuf;
|
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 */
|
/* Must be able to become new owner */
|
||||||
check_is_member_of_role(GetUserId(), new_ownerId);
|
check_is_member_of_role(GetUserId(), new_ownerId);
|
||||||
|
@ -2434,8 +2434,17 @@ CREATE ROLE regress_reindexuser NOLOGIN;
|
|||||||
SET SESSION ROLE regress_reindexuser;
|
SET SESSION ROLE regress_reindexuser;
|
||||||
REINDEX SCHEMA schema_to_reindex;
|
REINDEX SCHEMA schema_to_reindex;
|
||||||
ERROR: must be owner of schema schema_to_reindex
|
ERROR: must be owner of schema schema_to_reindex
|
||||||
|
-- Permission failures with toast tables and indexes (pg_authid here)
|
||||||
|
RESET ROLE;
|
||||||
|
GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;
|
||||||
|
SET SESSION ROLE regress_reindexuser;
|
||||||
|
REINDEX TABLE pg_toast.pg_toast_1260;
|
||||||
|
ERROR: must be owner of table pg_toast_1260
|
||||||
|
REINDEX INDEX pg_toast.pg_toast_1260_index;
|
||||||
|
ERROR: must be owner of index pg_toast_1260_index
|
||||||
-- Clean up
|
-- Clean up
|
||||||
RESET ROLE;
|
RESET ROLE;
|
||||||
|
REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser;
|
||||||
DROP ROLE regress_reindexuser;
|
DROP ROLE regress_reindexuser;
|
||||||
DROP SCHEMA schema_to_reindex CASCADE;
|
DROP SCHEMA schema_to_reindex CASCADE;
|
||||||
NOTICE: drop cascades to 6 other objects
|
NOTICE: drop cascades to 6 other objects
|
||||||
|
@ -1003,8 +1003,15 @@ REINDEX SCHEMA CONCURRENTLY schema_to_reindex;
|
|||||||
CREATE ROLE regress_reindexuser NOLOGIN;
|
CREATE ROLE regress_reindexuser NOLOGIN;
|
||||||
SET SESSION ROLE regress_reindexuser;
|
SET SESSION ROLE regress_reindexuser;
|
||||||
REINDEX SCHEMA schema_to_reindex;
|
REINDEX SCHEMA schema_to_reindex;
|
||||||
|
-- Permission failures with toast tables and indexes (pg_authid here)
|
||||||
|
RESET ROLE;
|
||||||
|
GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;
|
||||||
|
SET SESSION ROLE regress_reindexuser;
|
||||||
|
REINDEX TABLE pg_toast.pg_toast_1260;
|
||||||
|
REINDEX INDEX pg_toast.pg_toast_1260_index;
|
||||||
|
|
||||||
-- Clean up
|
-- Clean up
|
||||||
RESET ROLE;
|
RESET ROLE;
|
||||||
|
REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser;
|
||||||
DROP ROLE regress_reindexuser;
|
DROP ROLE regress_reindexuser;
|
||||||
DROP SCHEMA schema_to_reindex CASCADE;
|
DROP SCHEMA schema_to_reindex CASCADE;
|
||||||
|
Loading…
Reference in New Issue
Block a user