branch-4.1: [improvement](auth) Scope mysql.user rows to the caller and mask password columns #67444

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));
 

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,16 +2103,31 @@ public String getDefaultCloudCluster(String user) {
     // ====== END CLOUD ======
 
     // for mysql.user table
-    public List<List<String>> getAllUserInfo() {
+    public List<List<String>> 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<List<String>> userInfos = Lists.newArrayList();
         readLock();
         try {
             Map<String, List<User>> nameToUsers = userManager.getNameToUsers();
             for (List<User> users : nameToUsers.values()) {
                 for (User user : users) {
                     if (!user.isSetByDomainResolver()) {
-                        List<String> 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<String> 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<List<String>> 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);
                     }
                 }

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<List<String>> userInfo = Env.getCurrentEnv().getAuth().getAllUserInfo();
+        UserIdentity currentUser = null;
+        if (request.isSetCurrentUserIdent()) {
+            currentUser = UserIdentity.fromThrift(request.current_user_ident);
+        }
+        List<List<String>> userInfo = Env.getCurrentEnv().getAuth().getAllUserInfo(currentUser);
         TShowUserResult result = new TShowUserResult();
         result.setUserinfoList(userInfo);
         return result;

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<List<String>> 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<String> 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<List<String>> 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<List<String>> 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

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 {

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}")
+}