-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[improvement](fe) add fe_meta_auth_token for FE meta-service internal HTTP auth #65551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,10 @@ | |
| import org.apache.doris.catalog.Env; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember there is another util like httpurlutil, InternalHttpsUtils.java |
||
| import org.apache.doris.cloud.security.SecurityChecker; | ||
| import org.apache.doris.common.Config; | ||
| import org.apache.doris.httpv2.meta.MetaBaseAction; | ||
| import org.apache.doris.system.SystemInfoService.HostInfo; | ||
|
|
||
| import com.google.common.base.Strings; | ||
| import com.google.common.collect.Maps; | ||
| import org.apache.http.conn.ssl.NoopHostnameVerifier; | ||
|
|
||
|
|
@@ -50,6 +52,10 @@ public static HttpURLConnection getConnectionWithNodeIdent(String request) throw | |
| HostInfo selfNode = Env.getServingEnv().getSelfNode(); | ||
| conn.setRequestProperty(Env.CLIENT_NODE_HOST_KEY, selfNode.getHost()); | ||
| conn.setRequestProperty(Env.CLIENT_NODE_PORT_KEY, selfNode.getPort() + ""); | ||
| String token = Config.fe_meta_auth_token; | ||
| if (!Strings.isNullOrEmpty(token)) { | ||
| conn.setRequestProperty(MetaBaseAction.TOKEN, token); | ||
| } | ||
| return conn; | ||
| } catch (Exception e) { | ||
| throw new IOException(e); | ||
|
|
@@ -65,6 +71,10 @@ public static Map<String, String> getNodeIdentHeaders() throws IOException { | |
| HostInfo selfNode = Env.getServingEnv().getSelfNode(); | ||
| headers.put(Env.CLIENT_NODE_HOST_KEY, selfNode.getHost()); | ||
| headers.put(Env.CLIENT_NODE_PORT_KEY, selfNode.getPort() + ""); | ||
| String token = Config.fe_meta_auth_token; | ||
| if (!Strings.isNullOrEmpty(token)) { | ||
| headers.put(MetaBaseAction.TOKEN, token); | ||
| } | ||
| return headers; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,9 +20,11 @@ | |
| import org.apache.doris.catalog.Env; | ||
| import org.apache.doris.common.Config; | ||
| import org.apache.doris.common.DdlException; | ||
| import org.apache.doris.common.util.HttpURLUtil; | ||
| import org.apache.doris.common.util.NetUtils; | ||
| import org.apache.doris.ha.FrontendNodeType; | ||
| import org.apache.doris.httpv2.entity.ResponseEntityBuilder; | ||
| import org.apache.doris.httpv2.exception.UnauthorizedException; | ||
| import org.apache.doris.httpv2.rest.RestBaseController; | ||
| import org.apache.doris.master.MetaHelper; | ||
| import org.apache.doris.persist.MetaCleaner; | ||
|
|
@@ -55,33 +57,50 @@ public class MetaService extends RestBaseController { | |
|
|
||
| private File imageDir = MetaHelper.getMasterImageDir(); | ||
|
|
||
| private boolean isFromValidFe(String clientHost, String clientPortStr) { | ||
| private Frontend getValidFe(String clientHost, String clientPortStr) { | ||
| Integer clientPort; | ||
| try { | ||
| clientPort = Integer.valueOf(clientPortStr); | ||
| } catch (Exception e) { | ||
| LOG.warn("get clientPort error. clientPortStr: {}", clientPortStr, e.getMessage()); | ||
| return false; | ||
| return null; | ||
| } | ||
|
|
||
| Frontend fe = Env.getCurrentEnv().checkFeExist(clientHost, clientPort); | ||
| if (fe == null) { | ||
| LOG.warn("request is not from valid FE. client: {}, {}", clientHost, clientPortStr); | ||
| return false; | ||
| } | ||
| return true; | ||
| return fe; | ||
| } | ||
|
|
||
| private void checkFromValidFe(HttpServletRequest request) | ||
| throws InvalidClientException { | ||
| throws UnauthorizedException { | ||
| String clientHost = request.getHeader(Env.CLIENT_NODE_HOST_KEY); | ||
| String clientPort = request.getHeader(Env.CLIENT_NODE_PORT_KEY); | ||
| if (!isFromValidFe(clientHost, clientPort)) { | ||
| throw new InvalidClientException("invalid client host: " + clientHost + ":" + clientPort | ||
| + ", request from " + request.getRemoteHost()); | ||
| Frontend fe = getValidFe(clientHost, clientPort); | ||
| if (fe == null) { | ||
| throw unauthorized(clientHost, clientPort, request); | ||
| } | ||
|
|
||
| // If a cluster meta auth token is configured, additionally require the request to | ||
| // carry a matching token. An empty token keeps the legacy node-host-only behavior, | ||
| // so existing clusters and rolling upgrades are unaffected. | ||
| String clusterToken = Config.fe_meta_auth_token; | ||
| if (!Strings.isNullOrEmpty(clusterToken)) { | ||
| String requestToken = request.getHeader(MetaBaseAction.TOKEN); | ||
| if (!clusterToken.equals(requestToken)) { | ||
| LOG.warn("reject meta request with invalid token. client: {}, {}, request from: {}", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you can output the prefix to check, the rest is masked, for example,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would there be a need for debugging? |
||
| clientHost, clientPort, request.getRemoteAddr()); | ||
| throw unauthorized(clientHost, clientPort, request); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private UnauthorizedException unauthorized(String clientHost, String clientPort, HttpServletRequest request) { | ||
| return new UnauthorizedException("invalid client host: " + clientHost + ":" + clientPort | ||
| + ", request from " + request.getRemoteAddr()); | ||
| } | ||
|
|
||
| @RequestMapping(path = "/image", method = RequestMethod.GET) | ||
| public Object image(HttpServletRequest request, HttpServletResponse response) { | ||
| checkFromValidFe(request); | ||
|
|
@@ -149,6 +168,12 @@ public Object put(HttpServletRequest request, HttpServletResponse response) thro | |
| if (port < 0 || port > 65535) { | ||
| return ResponseEntityBuilder.badRequest("port is invalid. The port number is between 0-65535"); | ||
| } | ||
| // The master pushes image using HttpURLUtil.getHttpPort() (https_port when enable_https=true, | ||
| // otherwise http_port), so the expected port must follow the same rule to stay consistent. | ||
| int expectedPort = HttpURLUtil.getHttpPort(); | ||
| if (port != expectedPort) { | ||
| return ResponseEntityBuilder.badRequest("port must be FE HTTP port: " + expectedPort); | ||
| } | ||
|
|
||
| String versionStr = request.getParameter(VERSION); | ||
| if (Strings.isNullOrEmpty(versionStr)) { | ||
|
|
@@ -245,9 +270,7 @@ public Object check(HttpServletRequest request, HttpServletResponse response) th | |
|
|
||
| @RequestMapping(value = "/dump", method = RequestMethod.GET) | ||
| public Object dump(HttpServletRequest request, HttpServletResponse response) throws DdlException { | ||
| if (Config.enable_all_http_auth) { | ||
| executeCheckPassword(request, response); | ||
| } | ||
| executeCheckPassword(request, response); | ||
|
|
||
| /* | ||
| * Before dump, we acquired the catalog read lock and all databases' read lock and all | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,7 +149,7 @@ private static void checkFile(File file) throws IOException { | |
|
|
||
| public static <T> ResponseBody doGet(String url, int timeout, Class<T> clazz) throws IOException { | ||
| Map<String, String> headers = HttpURLUtil.getNodeIdentHeaders(); | ||
| LOG.info("meta helper, url: {}, timeout: {}, headers: {}", url, timeout, headers); | ||
| LOG.info("meta helper, url: {}, timeout: {}, header names: {}", url, timeout, headers.keySet()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it enough just to anonymize tokens? Applying a one-size-fits-all approach might not be ideal.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer this approach, although it's conservative. If there's a genuine need to print headers, then it should be handled and intercepted in a public place. |
||
| String response = HttpUtils.doGet(url, headers, timeout); | ||
| try { | ||
| return parseResponse(response, clazz); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_tokenserves that map. Although/rest/v1/**goes throughAuthInterceptor, the BasicAuthorizationpath only checks the password and skipsADMIN_OR_NODEunlessConfig.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 asSHOW FRONTEND CONFIG, or add a sensitive/masked config mechanism and mark this token so every config dump API masks or omits it.