Mailing-List: contact commits-help@hawq.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hawq.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: espino@apache.org
To: commits@hawq.incubator.apache.org
Date: Fri, 03 Feb 2017 09:00:11 -0000
Message-Id: <7c77f2eadf0947f3941a1d51ef4c7e65@git.apache.org>
In-Reply-To: <40325a477040484aa0872e11ff2aa29a@git.apache.org>
References: <40325a477040484aa0872e11ff2aa29a@git.apache.org>
Subject: [09/50] [abbrv] incubator-hawq git commit: HAWQ-1249. Don't do ACL
 checks on segments
archived-at: Fri, 03 Feb 2017 09:00:11 -0000

HAWQ-1249. Don't do ACL checks on segments


Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/2f5910f2
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/2f5910f2
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/2f5910f2

Branch: refs/heads/2.1.0.0-incubating
Commit: 2f5910f2b0c2877e524c4c428ed963255c176378
Parents: 8d22582
Author: Chunling Wang <wangchunling14@126.com>
Authored: Mon Jan 9 14:35:11 2017 +0800
Committer: Chunling Wang <wangchunling14@126.com>
Committed: Mon Jan 9 14:35:11 2017 +0800

----------------------------------------------------------------------
 src/backend/catalog/aclchk.c        | 85 +++++++++++++++++++++++---------
 src/backend/executor/execMain.c     | 37 +-------------
 src/backend/parser/parse_relation.c | 35 +++----------
 3 files changed, 72 insertions(+), 85 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/2f5910f2/src/backend/catalog/aclchk.c
----------------------------------------------------------------------
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index d19a045..01a4f94 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -224,8 +224,9 @@ restrict_and_check_grant(bool is_grant, AclMode avail_goptions, bool all_privs,
 	 * If we found no grant options, consider whether to issue a hard error.
 	 * Per spec, having any privilege at all on the object will get you by
 	 * here.
+	 * QE bypass all permission checking.
 	 */
-	if (avail_goptions == ACL_NO_RIGHTS)
+	if (avail_goptions == ACL_NO_RIGHTS && Gp_role != GP_ROLE_EXECUTE)
 	{
 	  if (enable_ranger && !fallBackToNativeCheck(objkind, objectId, grantorId)) {
 	    if (pg_rangercheck(objkind, objectId, grantorId,
@@ -2948,9 +2949,9 @@ pg_class_aclmask(Oid table_oid, Oid roleid,
 		}
 	}
 	/*
-	 * Otherwise, superusers or on QE bypass all permission-checking.
+	 * Otherwise, superusers bypass all permission-checking.
 	 */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	if (superuser_arg(roleid))
 	{
 #ifdef ACLDEBUG
 		elog(DEBUG2, "OID %u is superuser, home free", roleid);
@@ -3006,8 +3007,8 @@ pg_database_aclmask(Oid db_oid, Oid roleid,
 	Oid			ownerId;
 	cqContext  *pcqCtx;
 
-	/* Superusers or on QE bypass all permission checking. */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
 
 	/*
@@ -3069,8 +3070,8 @@ pg_proc_aclmask(Oid proc_oid, Oid roleid,
 	Oid			ownerId;
 	cqContext  *pcqCtx;
 
-	/* Superusers or on QE bypass all permission checking. */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
 
 	/*
@@ -3131,8 +3132,8 @@ pg_language_aclmask(Oid lang_oid, Oid roleid,
 	Oid			ownerId;
 	cqContext  *pcqCtx;
 
-	/* Superusers or on QE bypass all permission checking. */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
 
 	/*
@@ -3194,8 +3195,8 @@ pg_namespace_aclmask(Oid nsp_oid, Oid roleid,
 	Oid			ownerId;
 	cqContext  *pcqCtx;
 
-	/* Superusers or on QE bypass all permission checking. */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
 
 	/*
@@ -3293,8 +3294,8 @@ pg_tablespace_aclmask(Oid spc_oid, Oid roleid,
 	if (spc_oid == GLOBALTABLESPACE_OID && !(IsBootstrapProcessingMode()||gp_upgrade_mode))
 		return 0;
 
-	/* Otherwise, superusers or on QE bypass all permission checking. */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
 
 	/*
@@ -3366,8 +3367,8 @@ pg_foreign_data_wrapper_aclmask(Oid fdw_oid, Oid roleid,
 
 	Form_pg_foreign_data_wrapper fdwForm;
 
-	/* Bypass permission checks for superusers or on QE */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
 
 	/*
@@ -3435,8 +3436,8 @@ pg_foreign_server_aclmask(Oid srv_oid, Oid roleid,
 
 	Form_pg_foreign_server srvForm;
 
-	/* Bypass permission checks for superusers or on QE */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
 
 	/*
@@ -3505,10 +3506,10 @@ pg_extprotocol_aclmask(Oid ptcOid, Oid roleid,
 	cqContext	cqc;
 	cqContext  *pcqCtx;
 
-	/* Bypass permission checks for superusers or on QE */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
 		return mask;
-	
+
 	rel = heap_open(ExtprotocolRelationId, AccessShareLock);
 
 	pcqCtx = caql_beginscan(
@@ -3585,8 +3586,8 @@ pg_filesystem_aclmask(Oid fsysOid, Oid roleid,
 	ScanKeyData entry[1];
 
 
-	/* Bypass permission checks for superusers or on QE */
-	if (GP_ROLE_EXECUTE == Gp_role || superuser_arg(roleid))
+	/* Bypass permission checks for superusers */
+	if (superuser_arg(roleid))
 		return mask;
 	
 	/*
@@ -3788,6 +3789,10 @@ pg_filesystem_nativecheck(Oid fsysid, Oid roleid, AclMode mode)
 AclResult
 pg_class_aclcheck(Oid table_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_CLASS, table_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_CLASS, table_oid, roleid, mode, ACLMASK_ANY);
@@ -3804,6 +3809,10 @@ pg_class_aclcheck(Oid table_oid, Oid roleid, AclMode mode)
 AclResult
 pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_DATABASE, db_oid, roleid))
    {
      return pg_rangercheck(ACL_KIND_DATABASE, db_oid, roleid, mode, ACLMASK_ANY);
@@ -3820,6 +3829,10 @@ pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode)
 AclResult
 pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_PROC, proc_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_PROC, proc_oid, roleid, mode, ACLMASK_ANY);
@@ -3836,6 +3849,10 @@ pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode)
 AclResult
 pg_language_aclcheck(Oid lang_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_LANGUAGE, lang_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_LANGUAGE, lang_oid, roleid, mode, ACLMASK_ANY);
@@ -3852,6 +3869,10 @@ pg_language_aclcheck(Oid lang_oid, Oid roleid, AclMode mode)
 AclResult
 pg_namespace_aclcheck(Oid nsp_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_NAMESPACE, nsp_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_NAMESPACE, nsp_oid, roleid, mode, ACLMASK_ANY);
@@ -3868,6 +3889,10 @@ pg_namespace_aclcheck(Oid nsp_oid, Oid roleid, AclMode mode)
 AclResult
 pg_tablespace_aclcheck(Oid spc_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_TABLESPACE, spc_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_TABLESPACE, spc_oid, roleid, mode, ACLMASK_ANY);
@@ -3885,6 +3910,10 @@ pg_tablespace_aclcheck(Oid spc_oid, Oid roleid, AclMode mode)
 AclResult
 pg_foreign_data_wrapper_aclcheck(Oid fdw_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_FDW, fdw_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_FDW, fdw_oid, roleid, mode, ACLMASK_ANY);
@@ -3902,6 +3931,10 @@ pg_foreign_data_wrapper_aclcheck(Oid fdw_oid, Oid roleid, AclMode mode)
 AclResult
 pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_FOREIGN_SERVER, srv_oid, roleid))
   {
     return pg_rangercheck(ACL_KIND_FOREIGN_SERVER, srv_oid, roleid, mode, ACLMASK_ANY);
@@ -3919,6 +3952,10 @@ pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, AclMode mode)
 AclResult
 pg_extprotocol_aclcheck(Oid ptcid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_EXTPROTOCOL, ptcid, roleid))
   {
     return pg_rangercheck(ACL_KIND_EXTPROTOCOL, ptcid, roleid, mode, ACLMASK_ANY);
@@ -3935,6 +3972,10 @@ pg_extprotocol_aclcheck(Oid ptcid, Oid roleid, AclMode mode)
 AclResult
 pg_filesystem_aclcheck(Oid fsysid, Oid roleid, AclMode mode)
 {
+  /* Bypass all permission checking on QE. */
+  if (Gp_role == GP_ROLE_EXECUTE)
+    return ACLCHECK_OK;
+
   if(enable_ranger && !fallBackToNativeCheck(ACL_KIND_FILESYSTEM, fsysid, roleid))
   {
     return pg_rangercheck(ACL_KIND_FILESYSTEM, fsysid, roleid, mode, ACLMASK_ANY);

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/2f5910f2/src/backend/executor/execMain.c
----------------------------------------------------------------------
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 30f6d09..666d16f 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1912,45 +1912,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 	 * rangetable here --- subplan RTEs will be checked during
 	 * ExecInitSubPlan().
 	 */
-	if (operation != CMD_SELECT ||
-			(Gp_role != GP_ROLE_EXECUTE &&
-			 !(shouldDispatch && cdbpathlocus_querysegmentcatalogs)))
+	if (Gp_role != GP_ROLE_EXECUTE)
 	{
 		ExecCheckRTPerms(plannedstmt->rtable);
 	}
-	else
-	{
-		/*
-		 * We don't check the rights here, so we can query pg_statistic even if we are a non-privileged user.
-		 * This shouldn't cause a problem, because "cdbpathlocus_querysegmentcatalogs" can only be true if we
-		 * are doing special catalog queries for ANALYZE.  Otherwise, the QD will execute the normal access right
-		 * check.  This does open a security hole, as it's possible for a hacker to connect to a segdb with GP_ROLE_EXECUTE,
-		 * (at least, in theory, although it isn't easy) and then do a query.  But all they can see is
-		 * pg_statistic and pg_class, and pg_class is normally readable by everyone.
-		 */
-
-		ListCell *lc = NULL;
-
-		foreach(lc, plannedstmt->rtable)
-		{
-			RangeTblEntry *rte = lfirst(lc);
-
-			if (rte->rtekind != RTE_RELATION)
-				continue;
-
-			if (rte->requiredPerms == 0)
-				continue;
-
-			/*
-			 * Ignore access rights check on pg_statistic and pg_class, so
-			 * the QD can retreive the statistics from the QEs.
-			 */
-			if (rte->relid != StatisticRelationId && rte->relid != RelationRelationId)
-			{
-				ExecCheckRTEPerms(rte);
-			}
-		}
-	}
 
 	/*
 	 * get information from query descriptor

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/2f5910f2/src/backend/parser/parse_relation.c
----------------------------------------------------------------------
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 7dbe496..f9444ef 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -2841,33 +2841,14 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
 	/*
 	 * We must have *all* the requiredPerms bits, so use aclmask not aclcheck.
 	 */
-	if (enable_ranger && !fallBackToNativeCheck(ACL_KIND_CLASS, relOid, userid))
-	{
-	  elog(LOG, "ExecCheckRTEPerms: here");
-	  /* ranger check required permission should all be approved.*/
-    if (pg_rangercheck(ACL_KIND_CLASS, relOid, userid, requiredPerms, ACLMASK_ALL)
-        != RANGERCHECK_OK)
-    {
-      /*
-       * If the table is a partition, return an error message that includes
-       * the name of the parent table.
-       */
-      const char *rel_name = get_rel_name_partition(relOid);
-      aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
-    }
-	}
-	else
-	{
-	  if (pg_class_aclmask(relOid, userid, requiredPerms, ACLMASK_ALL)
-	        != requiredPerms)
-    {
-      /*
-       * If the table is a partition, return an error message that includes
-       * the name of the parent table.
-       */
-      const char *rel_name = get_rel_name_partition(relOid);
-      aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
-    }
+	if (pg_class_aclmask(relOid, userid, requiredPerms, ACLMASK_ALL)
+			!= requiredPerms) {
+		/*
+		 * If the table is a partition, return an error message that includes
+		 * the name of the parent table.
+		 */
+		const char *rel_name = get_rel_name_partition(relOid);
+		aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
 	}
 }