github-actions[bot] commented on code in PR #65551:
URL: https://github.com/apache/doris/pull/65551#discussion_r3578230863
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java:
##########
@@ -259,8 +259,12 @@ private static List<String> getBeList() {
*/
@RequestMapping(path = "/config", method = RequestMethod.GET)
public Object config(HttpServletRequest request, HttpServletResponse
response) {
- executeCheckPassword(request, response);
- checkDbAuth(ConnectContext.get().getCurrentUserIdentity(),
InfoSchemaDb.DATABASE_NAME, PrivPredicate.SELECT);
+ // This endpoint returns raw FE config values (unmasked, including
secrets such as
+ // auth_token / fe_meta_auth_token / ldap password), so it requires
ADMIN, consistent
+ // with the SQL "SHOW FRONTEND CONFIG". The internal FE-to-FE fan-out
(configurationInfo)
+ // is itself ADMIN-gated and forwards the admin credential, so it is
unaffected.
+ ActionAuthorizationInfo authInfo = executeCheckPassword(request,
response);
+ checkAdminAuth(authInfo.userIdentity);
Review Comment:
This still does not enforce ADMIN in the default configuration.
`checkAdminAuth` only calls `checkGlobalAuth(..., ADMIN)` when
`Config.enable_all_http_auth` is true; with the default `false`, it returns
without checking privileges. This endpoint then returns raw
`ConfigBase.getConfigInfo(null)` values, including sensitive config such as
`auth_token` and the new `fe_meta_auth_token`, so an authenticated non-admin
can still read them through `/rest/v2/manager/node/config` (and through the FE
branch of `/configuration_info`, which uses the same helper before fan-out). If
this should match `SHOW FRONTEND CONFIG`, use an unconditional ADMIN check
here, for example `checkGlobalAuth(authInfo.userIdentity,
PrivPredicate.ADMIN)`, and apply the same gate before FE config fan-out.
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -818,6 +818,18 @@ public class Config extends ConfigBase {
// check token when download image file.
@ConfField public static boolean enable_token_check = true;
+ @ConfField(description = {"Cluster token for FE meta-service internal HTTP
authentication. "
+ + "When set (non-empty), FE meta-service endpoints (such as
image/role/check/put/journal_id) "
+ + "additionally require the caller to present a matching token
header, on top of the existing "
+ + "node-host check. Empty (default) keeps the legacy behavior of
node-host check only, so "
+ + "existing clusters and rolling upgrades are unaffected. Must be
identical on all FEs and "
+ + "provisioned in fe.conf before enabling, otherwise FEs will
reject each other.",
+ "FE meta-service 内部 HTTP 鉴权使用的集群 token。设置(非空)后,meta-service 端点(如 "
+ + "image/role/check/put/journal_id)在原有 node-host 校验之上,额外要求调用方携带匹配的
token 头。"
+ + "为空(默认)时维持仅 node-host 校验的旧行为,存量集群与滚动升级不受影响。必须在所有 FE 上取值一致,"
+ + "并在启用前写入 fe.conf,否则 FE 之间会互相拒绝。"})
+ public static String fe_meta_auth_token = "";
Review Comment:
This token is added as a regular `@ConfField`, so it is also returned by the
legacy config API. `Config.dump()` includes every annotated field with the raw
value, and `/rest/v1/config/fe?conf_item=fe_meta_auth_token` serves that map.
Although `/rest/v1/**` goes through `AuthInterceptor`, the Basic
`Authorization` path only checks the password and skips `ADMIN_OR_NODE` unless
`Config.isCloudMode()` is true, so a normal authenticated on-prem user can read
the meta-service token. Please either gate this config reader with the same
unconditional ADMIN check as `SHOW FRONTEND CONFIG`, or add a sensitive/masked
config mechanism and mark this token so every config dump API masks or omits it.
--
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]
