You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2022/07/27 08:51:20 UTC
[doris] branch master updated: [fix](auth) Forbid grant USAGE_PRIV to database.table (#11234)
This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new be2ac6aa59 [fix](auth) Forbid grant USAGE_PRIV to database.table (#11234)
be2ac6aa59 is described below
commit be2ac6aa59b3906caa70a8af7c4bf82e5571d99e
Author: Mingyu Chen <mo...@gmail.com>
AuthorDate: Wed Jul 27 16:51:15 2022 +0800
[fix](auth) Forbid grant USAGE_PRIV to database.table (#11234)
---
.../org/apache/doris/analysis/CreateRoleStmt.java | 2 +-
.../java/org/apache/doris/analysis/GrantStmt.java | 49 ++++++++++++----------
.../java/org/apache/doris/analysis/RevokeStmt.java | 4 +-
.../org/apache/doris/mysql/privilege/PaloAuth.java | 48 +++++++++++++--------
.../org/apache/doris/mysql/privilege/AuthTest.java | 20 ++++-----
5 files changed, 70 insertions(+), 53 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
index c218ff3197..ec35fcb7b8 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateRoleStmt.java
@@ -56,7 +56,7 @@ public class CreateRoleStmt extends DdlStmt {
// check if current user has GRANT priv on GLOBAL level.
if (!Env.getCurrentEnv().getAuth().checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "CREATE USER");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "CREATE ROLE");
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
index afe5387b35..3dcdec07cb 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
@@ -121,9 +121,9 @@ public class GrantStmt extends DdlStmt {
}
if (tblPattern != null) {
- checkPrivileges(analyzer, privileges, role, tblPattern);
+ checkTablePrivileges(privileges, role, tblPattern);
} else {
- checkPrivileges(analyzer, privileges, role, resourcePattern);
+ checkResourcePrivileges(privileges, role, resourcePattern);
}
}
@@ -136,86 +136,91 @@ public class GrantStmt extends DdlStmt {
* 5.1 User should has GLOBAL level GRANT_PRIV
* 5.2 or user has DATABASE/TABLE level GRANT_PRIV if grant/revoke to/from certain database or table.
* 5.3 or user should has 'resource' GRANT_PRIV if grant/revoke to/from certain 'resource'
+ * 6. Can not grant USAGE_PRIV to database or table
*
- * @param analyzer
* @param privileges
* @param role
* @param tblPattern
* @throws AnalysisException
*/
- public static void checkPrivileges(Analyzer analyzer, List<PaloPrivilege> privileges,
- String role, TablePattern tblPattern) throws AnalysisException {
+ public static void checkTablePrivileges(List<PaloPrivilege> privileges, String role, TablePattern tblPattern)
+ throws AnalysisException {
// Rule 1
if (tblPattern.getPrivLevel() != PrivLevel.GLOBAL && (privileges.contains(PaloPrivilege.ADMIN_PRIV)
|| privileges.contains(PaloPrivilege.NODE_PRIV))) {
- throw new AnalysisException("ADMIN_PRIV and NODE_PRIV can only be granted on *.*.*");
+ throw new AnalysisException("ADMIN_PRIV and NODE_PRIV can only be granted/revoke on/from *.*.*");
}
// Rule 2
if (privileges.contains(PaloPrivilege.NODE_PRIV) && !Env.getCurrentEnv().getAuth()
.checkGlobalPriv(ConnectContext.get(), PrivPredicate.OPERATOR)) {
- throw new AnalysisException("Only the user with NODE_PRIV can grant NODE_PRIV to other user");
+ throw new AnalysisException("Only user with NODE_PRIV can grant/revoke NODE_PRIV to other user");
}
if (role != null) {
// Rule 3 and 4
if (!Env.getCurrentEnv().getAuth().checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
} else {
// Rule 5.1 and 5.2
if (tblPattern.getPrivLevel() == PrivLevel.GLOBAL) {
if (!Env.getCurrentEnv().getAuth().checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
} else if (tblPattern.getPrivLevel() == PrivLevel.CATALOG) {
if (!Env.getCurrentEnv().getAuth().checkCtlPriv(ConnectContext.get(),
tblPattern.getQualifiedCtl(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
} else if (tblPattern.getPrivLevel() == PrivLevel.DATABASE) {
if (!Env.getCurrentEnv().getAuth().checkDbPriv(ConnectContext.get(),
tblPattern.getQualifiedCtl(), tblPattern.getQualifiedDb(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
} else {
// table level
- if (!Env.getCurrentEnv().getAuth().checkTblPriv(ConnectContext.get(),
- tblPattern.getQualifiedCtl(), tblPattern.getQualifiedDb(),
- tblPattern.getTbl(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ if (!Env.getCurrentEnv().getAuth()
+ .checkTblPriv(ConnectContext.get(), tblPattern.getQualifiedCtl(), tblPattern.getQualifiedDb(),
+ tblPattern.getTbl(), PrivPredicate.GRANT)) {
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
}
}
+
+ // Rule 6
+ if (privileges.contains(PaloPrivilege.USAGE_PRIV)) {
+ throw new AnalysisException("Can not grant/revoke USAGE_PRIV to/from database or table");
+ }
}
- public static void checkPrivileges(Analyzer analyzer, List<PaloPrivilege> privileges,
- String role, ResourcePattern resourcePattern) throws AnalysisException {
+ public static void checkResourcePrivileges(List<PaloPrivilege> privileges, String role,
+ ResourcePattern resourcePattern) throws AnalysisException {
// Rule 1
if (privileges.contains(PaloPrivilege.NODE_PRIV)) {
- throw new AnalysisException("Can not grant NODE_PRIV to any other users or roles");
+ throw new AnalysisException("Can not grant/revoke NODE_PRIV to/from any other users or roles");
}
// Rule 2
if (resourcePattern.getPrivLevel() != PrivLevel.GLOBAL && privileges.contains(PaloPrivilege.ADMIN_PRIV)) {
- throw new AnalysisException("ADMIN_PRIV privilege can only be granted on resource *");
+ throw new AnalysisException("ADMIN_PRIV privilege can only be granted/revoked on/from resource *");
}
if (role != null) {
// Rule 3 and 4
if (!Env.getCurrentEnv().getAuth().checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
} else {
// Rule 5.1 and 5.3
if (resourcePattern.getPrivLevel() == PrivLevel.GLOBAL) {
if (!Env.getCurrentEnv().getAuth().checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
} else {
if (!Env.getCurrentEnv().getAuth().checkResourcePriv(ConnectContext.get(),
resourcePattern.getResourceName(), PrivPredicate.GRANT)) {
- ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
+ ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE");
}
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
index 98b36b4968..a2239d520c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
@@ -112,9 +112,9 @@ public class RevokeStmt extends DdlStmt {
// Revoke operation obey the same rule as Grant operation. reuse the same method
if (tblPattern != null) {
- GrantStmt.checkPrivileges(analyzer, privileges, role, tblPattern);
+ GrantStmt.checkTablePrivileges(privileges, role, tblPattern);
} else {
- GrantStmt.checkPrivileges(analyzer, privileges, role, resourcePattern);
+ GrantStmt.checkResourcePrivileges(privileges, role, resourcePattern);
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java
index 314375197b..5a5eb54325 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PaloAuth.java
@@ -712,25 +712,37 @@ public class PaloAuth implements Writable {
}
// 3. set password
- setPasswordInternal(userIdent, password, null, false /* err on non exist */,
- false /* set by resolver */, true /* is replay */);
-
- // 4. grant privs of role to user
- grantPrivsByRole(userIdent, role);
-
- // other user properties
- propertyMgr.addUserResource(userIdent.getQualifiedUser(), false /* not system user */);
-
- if (!userIdent.getQualifiedUser().equals(ROOT_USER) && !userIdent.getQualifiedUser().equals(ADMIN_USER)) {
- // grant read privs to database information_schema
- TablePattern tblPattern = new TablePattern(DEFAULT_CATALOG, InfoSchemaDb.DATABASE_NAME, "*");
- try {
- tblPattern.analyze(ClusterNamespace.getClusterNameFromFullName(userIdent.getQualifiedUser()));
- } catch (AnalysisException e) {
- LOG.warn("should not happen", e);
+ setPasswordInternal(userIdent, password, null, false /* err on non exist */, false /* set by resolver */,
+ true /* is replay */);
+ try {
+ // 4. grant privs of role to user
+ grantPrivsByRole(userIdent, role);
+
+ // other user properties
+ propertyMgr.addUserResource(userIdent.getQualifiedUser(), false /* not system user */);
+
+ if (!userIdent.getQualifiedUser().equals(ROOT_USER) && !userIdent.getQualifiedUser()
+ .equals(ADMIN_USER)) {
+ // grant read privs to database information_schema
+ TablePattern tblPattern = new TablePattern(DEFAULT_CATALOG, InfoSchemaDb.DATABASE_NAME, "*");
+ try {
+ tblPattern.analyze(ClusterNamespace.getClusterNameFromFullName(userIdent.getQualifiedUser()));
+ } catch (AnalysisException e) {
+ LOG.warn("should not happen", e);
+ }
+ grantInternal(userIdent, null /* role */, tblPattern, PrivBitSet.of(PaloPrivilege.SELECT_PRIV),
+ false /* err on non exist */, true /* is replay */);
}
- grantInternal(userIdent, null /* role */, tblPattern, PrivBitSet.of(PaloPrivilege.SELECT_PRIV),
- false /* err on non exist */, true /* is replay */);
+ } catch (Throwable t) {
+ // This is a temp protection to avoid bug such as described in
+ // https://github.com/apache/doris/issues/11235
+ // Normally, all operations in try..catch block should not fail
+ // Why add try..catch block after "setPasswordInternal"?
+ // Because after calling "setPasswordInternal()", the in-memory state has been changed,
+ // so we should make sure the following operations not throw any exception, if it throws,
+ // exit the process because there is no way to rollback in-memory state.
+ LOG.error("got unexpected exception when creating user. exit", t);
+ System.exit(-1);
}
if (!isReplay) {
diff --git a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
index 8753b8f251..4b3641e595 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java
@@ -34,6 +34,7 @@ import org.apache.doris.catalog.Env;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
+import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
import org.apache.doris.datasource.InternalDataSource;
import org.apache.doris.persist.EditLog;
@@ -1591,15 +1592,14 @@ public class AuthTest {
// 2. grant resource priv to db table
TablePattern tablePattern = new TablePattern("db1", "*");
- grantStmt = new GrantStmt(userIdentity, null, tablePattern, usagePrivileges);
- hasException = false;
- try {
- grantStmt.analyze(analyzer);
- auth.grant(grantStmt);
- } catch (UserException e) {
- e.printStackTrace();
- hasException = true;
- }
- Assert.assertTrue(hasException);
+ GrantStmt grantStmt2 = new GrantStmt(userIdentity, null, tablePattern, usagePrivileges);
+ ExceptionChecker.expectThrowsWithMsg(AnalysisException.class,
+ "Can not grant/revoke USAGE_PRIV to/from database or table", () -> grantStmt2.analyze(analyzer));
+
+ // 3. grant resource prov to role on db.table
+ tablePattern = new TablePattern("db1", "*");
+ GrantStmt grantStmt3 = new GrantStmt(userIdentity, "test_role", tablePattern, usagePrivileges);
+ ExceptionChecker.expectThrowsWithMsg(AnalysisException.class,
+ "Can not grant/revoke USAGE_PRIV to/from database or table", () -> grantStmt3.analyze(analyzer));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org