Обсуждение: Log enhancement for aclcheck permissions failures
Hi hackers,
I was wondering if we can improve the error messages for acl permission failures.
Current implementation to report errors is in "backend/catalog/aclchk.c"
void aclcheck_error(AclResult aclerr, ObjectType objtype, const char *objectname);
based on the AclResult type, it print log messages like
"permission denied for schema %s"
which tells the admins what could be the domain of the permission-deny,
like table name or schema name.
However, I find that the log messages *lack* more details, i.e., the
*exact permission* that causes the permission-deny. For the novice users,
they may end up over-granting the permission to fix the issues
and cause security vulnerability in the database.
I think the log messages can be better if we add some diagnostic
information like which *role* is denied and what *permission* it lacks.
This way the users know which permission to grant exactly
without the trial-and-errors.
It is not hard to improve the log messages after looking into the code.
Most places use the function aclcheck_error() exactly after the permission
check, e.g., pg_type_aclcheck(), pg_tablespace_aclcheck().
For example, in backend/commands/dbcommands.c, it checks whether
the user has CREATE permission.
aclresult = pg_tablespace_aclcheck(dst_deftablespace, GetUserId(), ACL_CREATE);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, OBJECT_TABLESPACE, tablespacename);
We can simply change the aclcheck_error() function parameter a bit,
then we can pass the exact permission to the function, and tell the users
exactly why the permission is denied. Something would be like
void aclcheck_error(AclResult aclerr, ObjectType objtype,
const char *objectname,
void aclcheck_error(AclResult aclerr, ObjectType objtype,
const char *objectname,
const char *privilegename)
Any thoughts would be appreciated. Thanks!
Best regards,
Any thoughts would be appreciated. Thanks!
Best regards,
Bingyu
On Sat, May 1, 2021 at 5:26 AM Bingyu Shen <ahshenbingyu@gmail.com> wrote: > Hi hackers, > > I was wondering if we can improve the error messages for acl permission failures. > Current implementation to report errors is in "backend/catalog/aclchk.c" > void aclcheck_error(AclResult aclerr, ObjectType objtype, const char *objectname); > > based on the AclResult type, it print log messages like > "permission denied for schema %s" > which tells the admins what could be the domain of the permission-deny, > like table name or schema name. > > However, I find that the log messages *lack* more details, i.e., the > *exact permission* that causes the permission-deny. For the novice users, > they may end up over-granting the permission to fix the issues > and cause security vulnerability in the database. > > I think the log messages can be better if we add some diagnostic > information like which *role* is denied and what *permission* it lacks. > This way the users know which permission to grant exactly > without the trial-and-errors. I think it's easy for users (even if they are novice) to know exactly what permission they are lacking by just looking at the query. See, the permissions we have in parsenodes.h with ACL_XXXX, they are quite clear and can be understood by the type of query. So, I don't think printing that obvious information in the log message is something we would want to improve. To know the current role with which the query is run, users can use CURRENT_ROLE or pg_roles. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com