From d97903cf26d9ff2db94586f89826e88b9ef7760b Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Thu, 3 Sep 2026 14:55:32 +0800 Subject: [PATCH] [improvement](auth) Scope mysql.user rows to the caller and mask password columns (#67444) ### What this PR does Adjusts what `mysql.user` returns so the visible rows follow the requesting user's privileges, and keeps password-derived columns out of the result entirely. - Rows are now scoped to the caller: role administrators (`ADMIN_PRIV` or `GRANT_PRIV`) still see every account; other users see only their own account. - The `authentication_string` and `password_policy.history_passwords` columns are always rendered as `***` for every caller, including accounts with an empty password. To make row scoping possible, the caller identity is threaded through `TShowUserRequest` (the same pattern already used by the sibling schema-table scanners such as `user_privileges` and `processlist`), so the FE can filter rows. A request without an identity returns no rows. ### Compatibility - `current_user_ident` is an `optional` Thrift field, wire-compatible in both directions. - No metadata / editlog / storage-format change, so downgrade is clean. - During a rolling window where a new FE talks to an old BE that does not set the field, `mysql.user` returns no rows (fail-closed) until the BE is also upgraded; upgrading BE before FE avoids this. ### Tests - `FrontendServiceImplTest#testShowUser` asserts the administrator, normal-user, and no-identity behaviors, including that the password columns are masked. - `regression-test/suites/auth_p0/test_mysql_user_visibility.groovy` covers the end-to-end admin-vs-normal-user visibility and masking. --- .../schema_user_scanner.cpp | 3 + .../apache/doris/mysql/privilege/Auth.java | 27 ++++++- .../doris/service/FrontendServiceImpl.java | 6 +- .../service/FrontendServiceImplTest.java | 54 ++++++++++++-- gensrc/thrift/FrontendService.thrift | 1 + .../auth_p0/test_mysql_user_visibility.groovy | 74 +++++++++++++++++++ 6 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 regression-test/suites/auth_p0/test_mysql_user_visibility.groovy diff --git a/be/src/information_schema/schema_user_scanner.cpp b/be/src/information_schema/schema_user_scanner.cpp index 3b2211ee69bab7..260ee8a814fee4 100644 --- a/be/src/information_schema/schema_user_scanner.cpp +++ b/be/src/information_schema/schema_user_scanner.cpp @@ -70,6 +70,9 @@ SchemaUserScanner::~SchemaUserScanner() = default; Status SchemaUserScanner::start(RuntimeState* state) { TShowUserRequest request; + if (nullptr != _param->common_param->current_user_ident) { + request.__set_current_user_ident(*_param->common_param->current_user_ident); + } RETURN_IF_ERROR(SchemaHelper::show_user(*(_param->common_param->ip), _param->common_param->port, request, &_user_result)); diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java index ce3faecc1ca8c0..d6ed40710ea5ca 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java @@ -102,6 +102,8 @@ public class Auth implements Writable { // unknown user does not have any privilege, this is just to be compatible with old version. public static final String UNKNOWN_USER = "unknown"; public static final String DEFAULT_CATALOG = InternalCatalog.INTERNAL_CATALOG_NAME; + // Placeholder shown in mysql.user for password-derived columns, so no secret material leaks. + private static final String PASSWORD_MASK = "***"; // There is no concurrency control logic inside roleManager,userManager,userRoleManage and rpropertyMgr, // and it is completely managed by Auth. @@ -2101,7 +2103,12 @@ public String getDefaultCloudCluster(String user) { // ====== END CLOUD ====== // for mysql.user table - public List> getAllUserInfo() { + public List> getAllUserInfo(UserIdentity currentUser) { + // Only role administrators (ADMIN_PRIV or GRANT_PRIV) may see every account. A + // non-privileged user may only see their own account, so that mysql.user does not + // leak the cluster's account list and privilege topology to arbitrary users. + boolean canSeeAll = currentUser != null + && Env.getCurrentEnv().getAccessManager().checkGlobalPriv(currentUser, PrivPredicate.GRANT); List> userInfos = Lists.newArrayList(); readLock(); try { @@ -2109,8 +2116,18 @@ public List> getAllUserInfo() { for (List users : nameToUsers.values()) { for (User user : users) { if (!user.isSetByDomainResolver()) { - List userInfo = Lists.newArrayList(Collections.nCopies(32, "")); UserIdentity userIdent = user.getUserIdentity(); + // A non-privileged caller may only see its own account. user@hostA and + // user@hostB are distinct accounts with independent privileges, so match the + // exact identity (name and host) rather than the name alone; otherwise another + // same-named account's host and privilege state would leak. The caller identity + // carried here is ConnectContext.currentUserIdentity, i.e. the account + // definition that authentication resolved to (a domain account resolves back to + // its user@['domain'] identity), so this still matches the caller's own row. + if (!canSeeAll && (currentUser == null || !userIdent.equals(currentUser))) { + continue; + } + List userInfo = Lists.newArrayList(Collections.nCopies(32, "")); userInfo.set(0, userIdent.getHost()); userInfo.set(1, userIdent.getQualifiedUser()); for (int i = 2; i <= 13; i++) { @@ -2183,6 +2200,12 @@ public List> getAllUserInfo() { userInfo.set(24 + i, passWordPolicyInfo.get(i).get(1)); } } + // Never expose password-derived material through mysql.user. The + // authentication_string hash and the password_policy.history_passwords + // digests are always masked, for every caller and even when empty, so no + // secret material (or its presence/absence) leaks. + userInfo.set(23, PASSWORD_MASK); + userInfo.set(27, PASSWORD_MASK); userInfos.add(userInfo); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java index e6470e2074f264..bea967a1a476ca 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java +++ b/fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java @@ -5002,7 +5002,11 @@ public TShowProcessListResult showProcessList(TShowProcessListRequest request) { @Override public TShowUserResult showUser(TShowUserRequest request) { - List> userInfo = Env.getCurrentEnv().getAuth().getAllUserInfo(); + UserIdentity currentUser = null; + if (request.isSetCurrentUserIdent()) { + currentUser = UserIdentity.fromThrift(request.current_user_ident); + } + List> userInfo = Env.getCurrentEnv().getAuth().getAllUserInfo(currentUser); TShowUserResult result = new TShowUserResult(); result.setUserinfoList(userInfo); return result; diff --git a/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java b/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java index e45bed95a47b57..892d9b8f731fd7 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java @@ -71,7 +71,6 @@ import org.apache.doris.thrift.TSchemaTableName; import org.apache.doris.thrift.TSchemaTableRequestParams; import org.apache.doris.thrift.TShowUserRequest; -import org.apache.doris.thrift.TShowUserResult; import org.apache.doris.thrift.TStatusCode; import org.apache.doris.thrift.TTableStatus; import org.apache.doris.transaction.GlobalTransactionMgrIface; @@ -559,11 +558,56 @@ public void fetchSchemaTableData() throws Exception { } @Test - public void testShowUser() { + public void testShowUser() throws Exception { + // Column indexes in the mysql.user row layout that carry password-derived material. + final int authStringIdx = 23; // authentication_string + final int historyPwIdx = 27; // password_policy.history_passwords + final int userNameIdx = 1; // User + + addUser("show_user_a", true); + addUser("show_user_b", true); + FrontendServiceImpl impl = new FrontendServiceImpl(exeEnv); - TShowUserRequest request = new TShowUserRequest(); - TShowUserResult result = impl.showUser(request); - System.out.println(result); + + // A role administrator (root has ADMIN_PRIV) sees every account, but the password-derived + // columns are always masked, even for accounts with an empty password. + TShowUserRequest adminRequest = new TShowUserRequest(); + adminRequest.setCurrentUserIdent(UserIdentity.ROOT.toThrift()); + List> adminRows = impl.showUser(adminRequest).getUserinfoList(); + Assertions.assertTrue(adminRows.size() >= 2, "admin should see all accounts"); + Assertions.assertTrue(adminRows.stream().anyMatch(r -> "show_user_a".equals(r.get(userNameIdx)))); + Assertions.assertTrue(adminRows.stream().anyMatch(r -> "show_user_b".equals(r.get(userNameIdx)))); + for (List row : adminRows) { + Assertions.assertEquals("***", row.get(authStringIdx)); + Assertions.assertEquals("***", row.get(historyPwIdx)); + } + + // A non-privileged user only sees their own row, with the password columns masked, so + // mysql.user does not leak the cluster's account list or privilege topology. + TShowUserRequest userRequest = new TShowUserRequest(); + userRequest.setCurrentUserIdent( + UserIdentity.createAnalyzedUserIdentWithIp("show_user_a", "%").toThrift()); + List> userRows = impl.showUser(userRequest).getUserinfoList(); + Assertions.assertEquals(1, userRows.size()); + Assertions.assertEquals("show_user_a", userRows.get(0).get(userNameIdx)); + Assertions.assertEquals("***", userRows.get(0).get(authStringIdx)); + Assertions.assertEquals("***", userRows.get(0).get(historyPwIdx)); + + // Same name, different host are distinct accounts: a non-privileged caller must see only + // its exact user@host row, not the same-named account bound to another host. + executeCommand("create user 'dup_host_user'@'192.168.0.1'"); + executeCommand("create user 'dup_host_user'@'10.0.0.1'"); + TShowUserRequest dupRequest = new TShowUserRequest(); + dupRequest.setCurrentUserIdent( + UserIdentity.createAnalyzedUserIdentWithIp("dup_host_user", "192.168.0.1").toThrift()); + List> dupRows = impl.showUser(dupRequest).getUserinfoList(); + Assertions.assertEquals(1, dupRows.size()); + Assertions.assertEquals("dup_host_user", dupRows.get(0).get(userNameIdx)); + Assertions.assertEquals("192.168.0.1", dupRows.get(0).get(0)); + + // Fail closed: a request without a caller identity (e.g. a pre-upgrade BE that does not + // set the field) exposes no rows rather than leaking every account. + Assertions.assertTrue(impl.showUser(new TShowUserRequest()).getUserinfoList().isEmpty()); } @Test diff --git a/gensrc/thrift/FrontendService.thrift b/gensrc/thrift/FrontendService.thrift index f06ceb3ceb898d..4f6f362df400b4 100644 --- a/gensrc/thrift/FrontendService.thrift +++ b/gensrc/thrift/FrontendService.thrift @@ -1630,6 +1630,7 @@ struct TShowProcessListResult { } struct TShowUserRequest { + 1: optional Types.TUserIdentity current_user_ident // to filter rows by the requesting user's privileges } struct TShowUserResult { diff --git a/regression-test/suites/auth_p0/test_mysql_user_visibility.groovy b/regression-test/suites/auth_p0/test_mysql_user_visibility.groovy new file mode 100644 index 00000000000000..17faadda45d985 --- /dev/null +++ b/regression-test/suites/auth_p0/test_mysql_user_visibility.groovy @@ -0,0 +1,74 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_mysql_user_visibility", "p0,auth") { + String suiteName = "test_mysql_user_visibility" + String user1 = "${suiteName}_user1" + String user2 = "${suiteName}_user2" + String pwd = 'C123_567p' + + try_sql("DROP USER ${user1}") + try_sql("DROP USER ${user2}") + sql """CREATE USER '${user1}' IDENTIFIED BY '${pwd}'""" + sql """CREATE USER '${user2}' IDENTIFIED BY '${pwd}'""" + + // cloud-mode: a user needs cluster usage before it can run any query. + if (isCloudMode()) { + def clusters = sql " SHOW CLUSTERS; " + assertTrue(!clusters.isEmpty()) + def validCluster = clusters[0][0] + sql """GRANT USAGE_PRIV ON CLUSTER `${validCluster}` TO ${user1}""" + sql """GRANT USAGE_PRIV ON CLUSTER `${validCluster}` TO ${user2}""" + } + + // The connection targets the regression_test database, so user1 needs a privilege on it + // to establish the session. This is unrelated to mysql.user visibility (that comes from + // the default role's SELECT on mysql.*), it only makes connect() below succeed. + sql """GRANT SELECT_PRIV ON regression_test TO ${user1}""" + + // A role administrator (root here) sees every account, but the password-derived + // columns are always masked, even though these users have a non-empty password. + def adminRows = sql """ + SELECT User, authentication_string, `password_policy.history_passwords` + FROM mysql.user + """ + assertTrue(adminRows.any { it[0] == user1 }, "admin should see ${user1}") + assertTrue(adminRows.any { it[0] == user2 }, "admin should see ${user2}") + adminRows.each { + assertEquals("***", it[1], "authentication_string must be masked for admin") + assertEquals("***", it[2], "history_passwords must be masked for admin") + } + + // A non-privileged user only sees their own row, with the password columns masked, + // and must not be able to enumerate other accounts through mysql.user. + connect(user1, "${pwd}", context.config.jdbcUrl) { + def rows = sql """ + SELECT User, authentication_string, `password_policy.history_passwords` + FROM mysql.user + """ + assertTrue(!rows.isEmpty(), "${user1} should see its own row") + rows.each { + assertEquals(user1, it[0], "${user1} should only see its own account") + assertEquals("***", it[1], "authentication_string must be masked") + assertEquals("***", it[2], "history_passwords must be masked") + } + assertFalse(rows.any { it[0] == user2 }, "${user1} must not see ${user2}") + } + + try_sql("DROP USER ${user1}") + try_sql("DROP USER ${user2}") +}