Doris-Breakwater commented on issue #67345: URL: https://github.com/apache/doris/issues/67345#issuecomment-5476775191
Breakwater-GitHub-Analysis-Slot: slot_74fe15a6b273 ### Initial assessment **Valid, high-confidence authorization-boundary bug.** This is not only a naming inconsistency: a restricted authenticated user can make a privilege granted for one object type authorize metadata reads or state-changing operations on a distinct object type. Under the repository threat model's RBAC property (ยง4.8(5)), this is an in-scope security-sensitive issue. The issue currently has no labels, assignee, or milestone. I validated the code paths at the reported commit `4a9956e0a9355407b0388001b1f1d7c70ea22cd0`; I did not need additional logs or a Doris profile, and I did not run a cluster reproduction. ### Verified facts - Dictionaries are indexed independently by database/name in [`DictionaryManager`](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/dictionary/DictionaryManager.java#L95-L102). Its create path checks only the dictionary map before insertion ([lines 164-185](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/dictionary/DictionaryManager.java#L164-L185)). Conversely, internal table creation checks only `Database.isTableExist()` ([`InternalCatalog`, lines 1276-1285](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java#L1276-L1285)), whose lookup is backed by the table map ([`Database`, lines 398-405](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/catalog/Database.java#L398-L405 )). Therefore a table and dictionary with the same `(db, name)` are accepted in either creation order. - `CREATE DICTIONARY` and `DROP DICTIONARY` authorize the dictionary name through `checkTblPriv(internal, db, dict, CREATE/DROP)` ([create](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateDictionaryCommand.java#L92-L109), [drop](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropDictionaryCommand.java#L64-L75)). `Role.checkTblPriv` resolves those keys only as strings, and `SHOW` is also satisfied by any matching column privilege ([lines 418-429](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java#L418-L429)). `PrivPredicate.SHOW` includes `SELECT`, `LOAD`, `ALTER`, `CREATE`, `SHOW_VIEW`, and `DROP` ([lines 26-34](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70e a22cd0/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/PrivPredicate.java#L26-L34)). This confirms the cross-object privilege match. - `REFRESH DICTIONARY` at this commit reaches the inherited insert authorization, which checks `LOAD` using the `Dictionary` object's database/name ([`InsertIntoTableCommand`, lines 255-277](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java#L255-L277)); since `Dictionary` extends `Table`, a same-name table `LOAD` grant satisfies it. - Exact per-dictionary grants are not generally usable today: [`Auth.checkTablePatternExist`](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java#L766-L793) validates only `Database.getTableNullable()` and bypasses existence only for `CREATE_PRIV`. Thus non-`CREATE` grants for a dictionary-only name fail, leaving database/catalog/global scope for native authorization. - Sequencing nuance: at `4a9956e0a93`, `SHOW DICTIONARIES`, `EXPLAIN DICTIONARY`, and the refresh entry point still lack the preflight checks proposed in draft PR #67343. Therefore the supplied `SHOW`/`EXPLAIN` reproduction demonstrates the broader missing-check problem on that exact master snapshot, not the collision in isolation. The collision is nevertheless independently present, and applying #67343's table-keyed `SHOW`/`LOAD` checks would extend the same cross-object match to those commands. - Ranger has the same namespace issue: `checkTblPriv` constructs a `DorisObjectType.TABLE` resource with catalog/database/table-name keys ([lines 188-207](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/catalog/authorizer/ranger/doris/RangerDorisAccessController.java#L188-L207)); there is no dictionary object type in [`DorisObjectType`](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/catalog/authorizer/ranger/doris/DorisObjectType.java#L20-L22). ### Missing decisions / compatibility questions No additional issue-reporter diagnostics are required to confirm the bug. Maintainers do need to decide: 1. Whether table/dictionary coexistence is a supported requirement. If not, a shared namespace is the smaller and safer fix; if yes, a dedicated privilege object is necessary. 2. How to handle clusters that already contain collisions. Rejecting only future creates does not remove the existing privilege crossover; startup/image replay must not be broken without a migration or explicit conflict policy. 3. Whether exact native grants should work for dictionaries and what syntax/privileges they use. Collision rejection alone does not fix `Auth.checkTablePatternExist`; without changing it, non-`CREATE` dictionary authorization remains database-level. 4. The canonical case rules. Table lookup honors `lower_case_table_names`, while the dictionary name map currently uses the supplied string directly, so case variants need an explicit contract. 5. The intended read privilege for `dict_get`/`dict_get_many`. Both currently resolve `DictionaryManager` directly without an explicit access check ([`DictGet`](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DictGet.java#L89-L105), [`DictGetMany`](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DictGetMany.java#L95-L114)). This is adjacent to, but broader than, the reported collision and should be resolved as part of defining a dictionary privilege model. ### Recommended next step For a focused fix/backport, prefer a single shared database object-name namespace, provided same-name coexistence is not a product requirement: 1. Reject collisions in both creation directions and cover every internal table-like creation path that shares `Database`'s table map. 2. Make the check/reservation atomic. Two independent prechecks are racy because `Database` and `DictionaryManager` have separate locks. Preserve one lock order or introduce a central name reservation; note that `DROP TABLE` currently holds the database write lock before entering `DictionaryManager` ([lines 987-1055](https://github.com/apache/doris/blob/4a9956e0a9355407b0388001b1f1d7c70ea22cd0/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java#L987-L1055)). 3. Update native grant existence validation to recognize dictionary-only names, or explicitly document that dictionary privileges remain database-scoped. 4. Add an upgrade-time detection/migration policy for pre-existing collisions. If coexistence must remain legal, the dedicated-object alternative is substantially wider: SQL grant/revoke grammar and display, privilege persistence/replay and role matching, `CatalogAccessController`/`AccessControllerManager`, Ranger resource definitions/service configuration, command checks, and dictionary-function read checks all need to move together. Regression coverage should include both creation orders, concurrent table/dictionary creation, `lower_case_table_names` variants, pre-existing upgrade collisions, table-level and column-level grants, `CREATE`/`DROP`/`SHOW`/`EXPLAIN`/`REFRESH`, exact dictionary grants (if supported), and native plus Ranger authorization. The current `auth_call/test_ddl_dictionary_auth` test covers database-level `CREATE`/`DROP` only and does not exercise a same-name object collision. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
