From e4949f9fe55223dac74b0075b5483ad15e8e318c Mon Sep 17 00:00:00 2001 From: "Marc G. Fournier" Date: Wed, 12 Mar 1997 20:48:48 +0000 Subject: [PATCH] From: Dan McGuirk Subject: [HACKERS] better access control error messages This patch replaces the 'no such class or insufficient privilege' with distinct error messages that tell you whether the table really doesn't exist or whether access was denied. --- src/backend/commands/copy.c | 10 ++++---- src/backend/executor/execMain.c | 16 ++++++------- src/backend/parser/parse_query.c | 4 ++-- src/backend/tcop/aclchk.c | 41 +++++++++++++++++++------------- src/backend/tcop/utility.c | 21 +++++++++------- src/include/utils/acl.h | 14 +++++++---- 6 files changed, 63 insertions(+), 43 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index f46a848d42..12ee1c28f1 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -6,7 +6,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.21 1997/01/10 17:46:33 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.22 1997/03/12 20:47:32 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -102,13 +102,15 @@ DoCopy(char *relname, bool binary, bool oids, bool from, bool pipe, Relation rel; extern char *UserName; /* defined in global.c */ const AclMode required_access = from ? ACL_WR : ACL_RD; + int result; rel = heap_openr(relname); if (rel == NULL) elog(WARN, "COPY command failed. Class %s " "does not exist.", relname); - - if (!pg_aclcheck(relname, UserName, required_access)) - elog(WARN, "%s %s", relname, ACL_NO_PRIV_WARNING); + + result = pg_aclcheck(relname, UserName, required_access); + if(result != ACLCHECK_OK) + elog(WARN, "%s: %s", relname, aclcheck_error_strings[result]); /* Above should not return */ else if (!superuser() && !pipe) elog(WARN, "You must have Postgres superuser privilege to do a COPY " diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 11c8f91a8d..cff5081c37 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -26,7 +26,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.10 1997/01/22 05:26:27 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.11 1997/03/12 20:47:41 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -289,7 +289,7 @@ ExecCheckPerms(CmdType operation, HeapTuple htp; List *lp; List *qvars, *tvars; - int32 ok = 1; + int32 ok = 1, aclcheck_result = -1; char *opstr; NameData rname; char *userName; @@ -317,21 +317,21 @@ ExecCheckPerms(CmdType operation, if (intMember(resultRelation, qvars) || intMember(resultRelation, tvars)) { /* result relation is scanned */ - ok = CHECK(ACL_RD); + ok = ((aclcheck_result = CHECK(ACL_RD)) == ACLCHECK_OK); opstr = "read"; if (!ok) break; } switch (operation) { case CMD_INSERT: - ok = CHECK(ACL_AP) || - CHECK(ACL_WR); + ok = ((aclcheck_result = CHECK(ACL_AP)) == ACLCHECK_OK) || + ((aclcheck_result = CHECK(ACL_WR)) == ACLCHECK_OK); opstr = "append"; break; case CMD_NOTIFY: /* what does this mean?? -- jw, 1/6/94 */ case CMD_DELETE: case CMD_UPDATE: - ok = CHECK(ACL_WR); + ok = ((aclcheck_result = CHECK(ACL_WR)) == ACLCHECK_OK); opstr = "write"; break; default: @@ -340,7 +340,7 @@ ExecCheckPerms(CmdType operation, } } else { /* XXX NOTIFY?? */ - ok = CHECK(ACL_RD); + ok = ((aclcheck_result = CHECK(ACL_RD)) == ACLCHECK_OK); opstr = "read"; } if (!ok) @@ -352,7 +352,7 @@ ExecCheckPerms(CmdType operation, elog(WARN, "%s on \"%-.*s\": permission denied", opstr, NAMEDATALEN, rname.data); */ - elog(WARN, "%s %s", rname.data, ACL_NO_PRIV_WARNING); + elog(WARN, "%s: %s", rname.data, aclcheck_error_strings[aclcheck_result]); } } diff --git a/src/backend/parser/parse_query.c b/src/backend/parser/parse_query.c index 398685bed4..2411e26497 100644 --- a/src/backend/parser/parse_query.c +++ b/src/backend/parser/parse_query.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/parser/Attic/parse_query.c,v 1.14 1997/02/14 23:02:29 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/Attic/parse_query.c,v 1.15 1997/03/12 20:47:57 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -133,7 +133,7 @@ addRangeTableEntry(ParseState *pstate, relation = heap_openr(relname); if (relation == NULL) { elog(WARN,"%s: %s", - relname, ACL_NO_PRIV_WARNING); + relname, aclcheck_error_strings[ACLCHECK_NO_CLASS]); } /* diff --git a/src/backend/tcop/aclchk.c b/src/backend/tcop/aclchk.c index b58a750343..20748e16c6 100644 --- a/src/backend/tcop/aclchk.c +++ b/src/backend/tcop/aclchk.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/tcop/Attic/aclchk.c,v 1.6 1997/01/23 19:33:31 scrappy Exp $ + * $Header: /cvsroot/pgsql/src/backend/tcop/Attic/aclchk.c,v 1.7 1997/03/12 20:48:17 scrappy Exp $ * * NOTES * See acl.h. @@ -17,7 +17,7 @@ #include #include "postgres.h" -#include "utils/acl.h" /* where declarations for this file goes */ +#include "utils/acl.h" /* where declarations for this file go */ #include "access/heapam.h" #include "access/htup.h" #include "access/tupmacs.h" @@ -55,6 +55,15 @@ #define Name_pg_group "pggroup" #endif +/* warning messages, now more explicit. */ +/* should correspond to the order of the ACLCHK_* result codes above. */ +char *aclcheck_error_strings[] = { + "No error.", + "Permission denied.", + "Table does not exist.", + "Must be table owner." +}; + #ifdef ACLDEBUG_TRACE static dumpacl(Acl *acl) @@ -268,10 +277,10 @@ aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode) * the system never creates an empty ACL. */ if (num < 1) { -#ifdef ACLDEBUG_TRACE +#ifdef ACLDEBUG_TRACE || 1 elog(DEBUG, "aclcheck: zero-length ACL, returning 1"); #endif - return(1); + return ACLCHECK_OK; } switch (idtype) { @@ -284,7 +293,7 @@ aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode) elog(DEBUG, "aclcheck: found %d/%d", aip->ai_id, aip->ai_mode); #endif - return((aip->ai_mode & mode) ? 1 : 0); + return((aip->ai_mode & mode) ? ACLCHECK_OK : ACLCHECK_NO_PRIV); } } for (found_group = 0; @@ -304,7 +313,7 @@ aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode) elog(DEBUG, "aclcheck: found %d/%d", aip->ai_id, aip->ai_mode); #endif - return(0); + return ACLCHECK_NO_PRIV; } #endif } @@ -313,7 +322,7 @@ aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode) #ifdef ACLDEBUG_TRACE elog(DEBUG,"aclcheck: all groups ok"); #endif - return(1); + return ACLCHECK_OK; } break; case ACL_IDTYPE_GID: @@ -329,7 +338,7 @@ aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode) elog(DEBUG, "aclcheck: found %d/%d", aip->ai_id, aip->ai_mode); #endif - return((aip->ai_mode & mode) ? 1 : 0); + return((aip->ai_mode & mode) ? ACLCHECK_OK : ACLCHECK_NO_PRIV); } } break; @@ -343,7 +352,7 @@ aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode) #ifdef ACLDEBUG_TRACE elog(DEBUG, "aclcheck: using world=%d", aidat->ai_mode); #endif - return((aidat->ai_mode & mode) ? 1 : 0); + return((aidat->ai_mode & mode) ? ACLCHECK_OK : ACLCHECK_NO_PRIV); } int32 @@ -370,7 +379,7 @@ pg_aclcheck(char *relname, char *usename, AclMode mode) pg_database table, there is still additional permissions checking in dbcommands.c */ if (mode & ACL_AP) - return (1); + return ACLCHECK_OK; } /* @@ -383,7 +392,7 @@ pg_aclcheck(char *relname, char *usename, AclMode mode) !((Form_pg_user) GETSTRUCT(htp))->usecatupd) { elog(DEBUG, "pg_aclcheck: catalog update to \"%-.*s\": permission denied", NAMEDATALEN, relname); - return(0); + return ACLCHECK_NO_PRIV; } /* @@ -394,7 +403,7 @@ pg_aclcheck(char *relname, char *usename, AclMode mode) elog(DEBUG, "pg_aclcheck: \"%-.*s\" is superuser", NAMEDATALEN, usename); #endif - return(1); + return ACLCHECK_OK; } #ifndef ACLDEBUG @@ -403,7 +412,7 @@ pg_aclcheck(char *relname, char *usename, AclMode mode) if (!HeapTupleIsValid(htp)) { elog(WARN, "pg_aclcheck: class \"%-.*s\" not found", NAMEDATALEN, relname); - return(1); + /* an elog(WARN) kills us, so no need to return anything. */ } if (!heap_attisnull(htp, Anum_pg_class_relacl)) { relation = heap_openr(RelationRelationName); @@ -436,7 +445,7 @@ pg_aclcheck(char *relname, char *usename, AclMode mode) if (!RelationIsValid(relation)) { elog(NOTICE, "pg_checkacl: could not open \"%-.*s\"??", RelationRelationName); - return(1); + return ACLCHECK_NO_CLASS; } fmgr_info(NameEqualRegProcedure, &relkey[0].sk_func, @@ -494,8 +503,8 @@ pg_ownercheck(char *usename, switch (cacheid) { case OPROID: if (!HeapTupleIsValid(htp)) - elog(WARN, "pg_ownercheck: operator %d not found", - (int) value); + elog(WARN, "pg_ownercheck: operator %ld not found", + PointerGetDatum(value)); owner_id = ((OperatorTupleForm) GETSTRUCT(htp))->oprowner; break; case PRONAME: diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 693dadc834..065d01457c 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.11 1997/01/16 14:56:21 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.12 1997/03/12 20:48:27 scrappy Exp $ * *------------------------------------------------------------------------- */ @@ -381,10 +381,13 @@ ProcessUtility(Node *parsetree, case T_RuleStmt: /* CREATE RULE */ { RuleStmt *stmt = (RuleStmt *)parsetree; + int aclcheck_result; + #ifndef NO_SECURITY relname = stmt->object->relname; - if (!pg_aclcheck(relname, userName, ACL_RU)) - elog(WARN, "%s %s", relname, ACL_NO_PRIV_WARNING); + aclcheck_result = pg_aclcheck(relname, userName, ACL_RU); + if(aclcheck_result != ACLCHECK_OK) + elog(WARN, "%s: %s", relname, aclcheck_error_strings[aclcheck_result]); #endif commandTag = "CREATE"; CHECK_IF_ABORTED(); @@ -423,19 +426,21 @@ ProcessUtility(Node *parsetree, relname); #ifndef NO_SECURITY if (!pg_ownercheck(userName, relname, RELNAME)) - elog(WARN, "you do not own class \"%s\"", - relname); + elog(WARN, "%s: %s", relationName, aclcheck_error_strings[ACLCHECK_NOT_OWNER]); #endif RemoveIndex(relname); break; case RULE: { char *rulename = stmt->name; + int aclcheck_result; #ifndef NO_SECURITY relationName = RewriteGetRuleEventRel(rulename); - if (!pg_aclcheck(relationName, userName, ACL_RU)) - elog(WARN, "%s %s", relationName, ACL_NO_PRIV_WARNING); + aclcheck_result = pg_aclcheck(relationName, userName, ACL_RU); + if(aclcheck_result != ACLCHECK_OK) { + elog(WARN, "%s: %s", relationName, aclcheck_error_strings[aclcheck_result]); + } #endif RemoveRewriteRule(rulename); } @@ -457,7 +462,7 @@ ProcessUtility(Node *parsetree, ruleName = MakeRetrieveViewRuleName(viewName); relationName = RewriteGetRuleEventRel(ruleName); if (!pg_ownercheck(userName, relationName, RELNAME)) - elog(WARN, "%s %s", relationName, ACL_NO_PRIV_WARNING); + elog(WARN, "%s: %s", relationName, aclcheck_error_strings[ACLCHECK_NOT_OWNER]); pfree(ruleName); #endif RemoveView(viewName); diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 9789501504..bc4473d37d 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: acl.h,v 1.4 1996/11/10 03:06:14 momjian Exp $ + * $Id: acl.h,v 1.5 1997/03/12 20:48:48 scrappy Exp $ * * NOTES * For backward-compatability purposes we have to allow there @@ -111,10 +111,14 @@ typedef ArrayType IdList; #define ACL_MODE_WR_CHR 'w' #define ACL_MODE_RU_CHR 'R' -/* we use this warning string both for non-existent tables and - insufficient privilege so non-privileged users cannot ascertain whether - the class exists or not */ -#define ACL_NO_PRIV_WARNING "Either no such class or insufficient privilege" +/* result codes for pg_aclcheck */ +#define ACLCHECK_OK 0 +#define ACLCHECK_NO_PRIV 1 +#define ACLCHECK_NO_CLASS 2 +#define ACLCHECK_NOT_OWNER 3 + +/* warning messages. set these in aclchk.c. */ +extern char *aclcheck_error_strings[]; /* * Enable ACL execution tracing and table dumps