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 <morningman....@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-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
