Skip to content
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

[ISSUE #4593] Add server identity to replace user-agent white list. #4683

Merged
merged 1 commit into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions auth/src/main/java/com/alibaba/nacos/auth/common/AuthConfigs.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ public class AuthConfigs {
@Value("${nacos.core.auth.system.type:}")
private String nacosAuthSystemType;

@Value("${nacos.core.auth.server.identity.key:}")
private String serverIdentityKey;

@Value(("${nacos.core.auth.server.identity.value:}"))
private String serverIdentityValue;

@Value(("${nacos.core.auth.enable.userAgentAuthWhite:true}"))
private boolean enableUserAgentAuthWhite;

public byte[] getSecretKeyBytes() {
if (secretKeyBytes == null) {
secretKeyBytes = Decoders.BASE64.decode(secretKey);
Expand All @@ -77,6 +86,18 @@ public String getNacosAuthSystemType() {
return nacosAuthSystemType;
}

public String getServerIdentityKey() {
return serverIdentityKey;
}

public String getServerIdentityValue() {
return serverIdentityValue;
}

public boolean isEnableUserAgentAuthWhite() {
return enableUserAgentAuthWhite;
}

/**
* auth function is open.
*
Expand Down
43 changes: 43 additions & 0 deletions auth/src/main/java/com/alibaba/nacos/auth/util/AuthHeaderUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright 1999-2020 Alibaba Group Holding Ltd.
*
* Licensed 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.
*/

package com.alibaba.nacos.auth.util;

import com.alibaba.nacos.auth.common.AuthConfigs;
import com.alibaba.nacos.common.http.param.Header;
import com.alibaba.nacos.common.utils.StringUtils;
import com.alibaba.nacos.sys.utils.ApplicationUtils;

/**
* Auth header util.
*
* @author xiweng.yy
*/
public class AuthHeaderUtil {

/**
* Add identity info to Http header.
*
* @param header http header
*/
public static void addIdentityToHeader(Header header) {
AuthConfigs authConfigs = ApplicationUtils.getBean(AuthConfigs.class);
if (StringUtils.isNotBlank(authConfigs.getServerIdentityKey())) {
header.addParam(authConfigs.getServerIdentityKey(), authConfigs.getServerIdentityValue());
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.alibaba.nacos.config.server.service.notify;

import com.alibaba.nacos.auth.util.AuthHeaderUtil;
import com.alibaba.nacos.common.http.Callback;
import com.alibaba.nacos.common.http.client.NacosAsyncRestTemplate;
import com.alibaba.nacos.common.http.param.Header;
Expand Down Expand Up @@ -138,6 +139,7 @@ private void executeAsyncInvoke() {
if (task.isBeta) {
header.addParam("isBeta", "true");
}
AuthHeaderUtil.addIdentityToHeader(header);
restTemplate.get(task.url, header, Query.EMPTY, String.class, new AsyncNotifyCallBack(task));
}
}
Expand Down
25 changes: 20 additions & 5 deletions core/src/main/java/com/alibaba/nacos/core/auth/AuthFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import com.alibaba.nacos.auth.parser.ResourceParser;
import com.alibaba.nacos.common.utils.ExceptionUtil;
import com.alibaba.nacos.core.code.ControllerMethodsCache;
import com.alibaba.nacos.sys.env.Constants;
import com.alibaba.nacos.core.utils.Loggers;
import com.alibaba.nacos.core.utils.WebUtils;
import com.alibaba.nacos.sys.env.Constants;
import org.apache.commons.lang3.StringUtils;
import org.springframework.beans.factory.annotation.Autowired;

Expand Down Expand Up @@ -73,10 +73,25 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
HttpServletRequest req = (HttpServletRequest) request;
HttpServletResponse resp = (HttpServletResponse) response;

String userAgent = WebUtils.getUserAgent(req);

if (StringUtils.startsWith(userAgent, Constants.NACOS_SERVER_HEADER)) {
chain.doFilter(request, response);
if (authConfigs.isEnableUserAgentAuthWhite()) {
String userAgent = WebUtils.getUserAgent(req);
if (StringUtils.startsWith(userAgent, Constants.NACOS_SERVER_HEADER)) {
chain.doFilter(request, response);
return;
}
} else if (StringUtils.isNotBlank(authConfigs.getServerIdentityKey()) && StringUtils
.isNotBlank(authConfigs.getServerIdentityValue())) {
String serverIdentity = req.getHeader(authConfigs.getServerIdentityKey());
if (authConfigs.getServerIdentityValue().equals(serverIdentity)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that comparing credential with equals makes your application vulnerable to timing attack

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to solve it? Can you provide some advise for this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, there is enough for equals. reason follow:

  1. Nacos is Internal IDC application, we recommend users use Nacos with internal network not public network, If hacker want to timing attack, the network has been controlled by hacker. And Cost much time to analyze package between nacos server.
  2. The key and value is defined by configuration file, as the first item said, if the internal network is in controlled by hacker. they can find the key and value by configuration file directly.

I'm not sure whether the timing attack is send a long string to hold equals resource. If so, I think we has no ability to solve all public application security problem for an internal network application.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KomachiSion FYI maybe you should use MessageDigest.isEquals

chain.doFilter(request, response);
return;
}
Loggers.AUTH.warn("Invalid server identity value for {} from {}", authConfigs.getServerIdentityKey(),
req.getRemoteHost());
} else {
resp.sendError(HttpServletResponse.SC_FORBIDDEN,
"Invalid server identity key or value, Please make sure set `nacos.core.auth.server.identity.key`"
+ " and `nacos.core.auth.server.identity.value`, or open `nacos.core.auth.enable.userAgentAuthWhite`");
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import com.alibaba.nacos.core.distributed.raft.JRaftServer;
import com.alibaba.nacos.core.distributed.raft.processor.NacosGetRequestProcessor;
import com.alibaba.nacos.core.distributed.raft.processor.NacosLogProcessor;
import com.alibaba.nacos.core.distributed.raft.processor.NacosReadRequestProcessor;
import com.alibaba.nacos.core.distributed.raft.processor.NacosWriteRequestProcessor;
import com.alibaba.nacos.sys.utils.ApplicationUtils;
import com.alibaba.nacos.core.utils.Loggers;
import com.alibaba.nacos.sys.utils.DiskUtils;
Expand Down Expand Up @@ -80,7 +82,9 @@ public static RpcServer initRpcServer(JRaftServer server, PeerId peerId) {

rpcServer.registerProcessor(new NacosLogProcessor(server, SerializeFactory.getDefault()));
rpcServer.registerProcessor(new NacosGetRequestProcessor(server, SerializeFactory.getDefault()));


rpcServer.registerProcessor(new NacosWriteRequestProcessor(SerializeFactory.getDefault()));
rpcServer.registerProcessor(new NacosReadRequestProcessor(SerializeFactory.getDefault()));
return rpcServer;
}

Expand Down
7 changes: 7 additions & 0 deletions distribution/conf/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ nacos.core.auth.default.token.secret.key=SecretKey012345678901234567890123456789
### Turn on/off caching of auth information. By turning on this switch, the update of auth information would have a 15 seconds delay.
nacos.core.auth.caching.enabled=true

### Since 1.4.1, Turn on/off white auth for user-agent: nacos-server, only for upgrade from old version.
nacos.core.auth.enable.userAgentAuthWhite=true

### Since 1.4.1, worked when nacos.core.auth.enabled=true and nacos.core.auth.enable.userAgentAuthWhite=false.
### The two properties is the white list for auth and used by identity the request from other server.
nacos.core.auth.server.identity.key=
nacos.core.auth.server.identity.value=

#*************** Istio Related Configurations ***************#
### If turn on the MCP server:
Expand Down
12 changes: 10 additions & 2 deletions naming/src/main/java/com/alibaba/nacos/naming/misc/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.alibaba.nacos.naming.misc;

import com.alibaba.nacos.auth.util.AuthHeaderUtil;
import com.alibaba.nacos.common.constant.HttpHeaderConsts;
import com.alibaba.nacos.common.http.Callback;
import com.alibaba.nacos.common.http.HttpClientConfig;
Expand Down Expand Up @@ -103,6 +104,7 @@ public static RestResult<String> request(String url, List<String> headers, Map<S
header.addParam(HttpHeaderConsts.USER_AGENT_HEADER, UtilsAndCommons.SERVER_VERSION);
header.addParam(HttpHeaderConsts.REQUEST_SOURCE_HEADER, EnvUtil.getLocalAddress());
header.addParam(HttpHeaderConsts.ACCEPT_CHARSET, encoding);
AuthHeaderUtil.addIdentityToHeader(header);

HttpClientConfig httpClientConfig = HttpClientConfig.builder().setConTimeOutMillis(connectTimeout)
.setReadTimeOutMillis(readTimeout).build();
Expand Down Expand Up @@ -178,7 +180,7 @@ public static void asyncHttpRequest(String url, List<String> headers, Map<String
header.addAll(headers);
}
header.addParam(HttpHeaderConsts.ACCEPT_CHARSET, "UTF-8");

AuthHeaderUtil.addIdentityToHeader(header);
switch (method) {
case HttpMethod.GET:
ASYNC_REST_TEMPLATE.get(url, header, query, String.class, callback);
Expand Down Expand Up @@ -224,6 +226,7 @@ public static void asyncHttpPostLarge(String url, List<String> headers, byte[] c
if (CollectionUtils.isNotEmpty(headers)) {
header.addAll(headers);
}
AuthHeaderUtil.addIdentityToHeader(header);
ASYNC_REST_TEMPLATE.post(url, header, Query.EMPTY, content, String.class, callback);
}

Expand All @@ -241,6 +244,7 @@ public static void asyncHttpDeleteLarge(String url, List<String> headers, String
if (CollectionUtils.isNotEmpty(headers)) {
header.addAll(headers);
}
AuthHeaderUtil.addIdentityToHeader(header);
ASYNC_REST_TEMPLATE.delete(url, header, content, String.class, callback);
}

Expand All @@ -265,7 +269,7 @@ public static RestResult<String> httpPost(String url, List<String> headers, Map<
header.addAll(headers);
}
header.addParam(HttpHeaderConsts.ACCEPT_CHARSET, encoding);

AuthHeaderUtil.addIdentityToHeader(header);
HttpClientConfig httpClientConfig = HttpClientConfig.builder().setConTimeOutMillis(5000).setReadTimeOutMillis(5000)
.setConnectionRequestTimeout(5000).setMaxRedirects(5).build();
return APACHE_SYNC_NACOS_REST_TEMPLATE.postForm(url, httpClientConfig, header, paramValues, String.class);
Expand All @@ -288,6 +292,7 @@ public static void asyncHttpPutLarge(String url, Map<String, String> headers, by
if (MapUtils.isNotEmpty(headers)) {
header.addAll(headers);
}
AuthHeaderUtil.addIdentityToHeader(header);
ASYNC_REST_TEMPLATE.put(url, header, Query.EMPTY, content, String.class, callback);
}

Expand All @@ -304,6 +309,7 @@ public static RestResult<String> httpPutLarge(String url, Map<String, String> he
if (MapUtils.isNotEmpty(headers)) {
header.addAll(headers);
}
AuthHeaderUtil.addIdentityToHeader(header);
try {
return APACHE_SYNC_NACOS_REST_TEMPLATE.put(url, header, Query.EMPTY, content, String.class);
} catch (Exception e) {
Expand All @@ -324,6 +330,7 @@ public static RestResult<String> httpGetLarge(String url, Map<String, String> he
if (MapUtils.isNotEmpty(headers)) {
header.addAll(headers);
}
AuthHeaderUtil.addIdentityToHeader(header);
try {
return APACHE_SYNC_NACOS_REST_TEMPLATE.getLarge(url, header, Query.EMPTY, content, String.class);
} catch (Exception e) {
Expand All @@ -344,6 +351,7 @@ public static RestResult<String> httpPostLarge(String url, Map<String, String> h
if (MapUtils.isNotEmpty(headers)) {
header.addAll(headers);
}
AuthHeaderUtil.addIdentityToHeader(header);
try {
return APACHE_SYNC_NACOS_REST_TEMPLATE.postJson(url, header, content, String.class);
} catch (Exception e) {
Expand Down
33 changes: 16 additions & 17 deletions naming/src/main/java/com/alibaba/nacos/naming/misc/NamingProxy.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
package com.alibaba.nacos.naming.misc;

import com.alibaba.nacos.common.constant.HttpHeaderConsts;
import com.alibaba.nacos.common.utils.IPUtil;
import com.alibaba.nacos.common.http.Callback;
import com.alibaba.nacos.common.model.RestResult;
import com.alibaba.nacos.common.utils.IPUtil;
import com.alibaba.nacos.common.utils.JacksonUtils;
import com.alibaba.nacos.common.utils.VersionUtils;
import com.alibaba.nacos.sys.env.EnvUtil;
Expand Down Expand Up @@ -85,17 +85,16 @@ public void onReceive(RestResult<String> result) {
result.getCode(), result.getMessage());
}
}

@Override
public void onError(Throwable throwable) {
Loggers.DISTRO
.error("failed to req API:" + "http://" + server + EnvUtil.getContextPath()
+ UtilsAndCommons.NACOS_NAMING_CONTEXT + TIMESTAMP_SYNC_URL, throwable);
Loggers.DISTRO.error("failed to req API:" + "http://" + server + EnvUtil.getContextPath()
+ UtilsAndCommons.NACOS_NAMING_CONTEXT + TIMESTAMP_SYNC_URL, throwable);
}

@Override
public void onCancel() {

}
});
} catch (Exception e) {
Expand All @@ -116,8 +115,8 @@ public static byte[] getData(List<String> keys, String server) throws Exception
Map<String, String> params = new HashMap<>(8);
params.put("keys", StringUtils.join(keys, ","));
RestResult<String> result = HttpClient.httpGetLarge(
"http://" + server + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT
+ DATA_GET_URL, new HashMap<>(8), JacksonUtils.toJson(params));
"http://" + server + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT + DATA_GET_URL,
new HashMap<>(8), JacksonUtils.toJson(params));

if (result.ok()) {
return result.getData().getBytes();
Expand All @@ -139,8 +138,8 @@ public static byte[] getAllData(String server) throws Exception {

Map<String, String> params = new HashMap<>(8);
RestResult<String> result = HttpClient.httpGet(
"http://" + server + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT
+ ALL_DATA_GET_URL, new ArrayList<>(), params);
"http://" + server + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT + ALL_DATA_GET_URL,
new ArrayList<>(), params);

if (result.ok()) {
return result.getData().getBytes();
Expand Down Expand Up @@ -301,12 +300,12 @@ public static String reqCommon(String path, Map<String, String> params, String c

if (isPost) {
result = HttpClient.httpPost(
"http://" + curServer + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT
+ path, headers, params);
"http://" + curServer + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT + path,
headers, params);
} else {
result = HttpClient.httpGet(
"http://" + curServer + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT
+ path, headers, params);
"http://" + curServer + EnvUtil.getContextPath() + UtilsAndCommons.NACOS_NAMING_CONTEXT + path,
headers, params);
}

if (result.ok()) {
Expand All @@ -318,8 +317,8 @@ public static String reqCommon(String path, Map<String, String> params, String c
}

throw new IOException("failed to req API:" + "http://" + curServer + EnvUtil.getContextPath()
+ UtilsAndCommons.NACOS_NAMING_CONTEXT + path + ". code:" + result.getCode() + " msg: "
+ result.getMessage());
+ UtilsAndCommons.NACOS_NAMING_CONTEXT + path + ". code:" + result.getCode() + " msg: " + result
.getMessage());
} catch (Exception e) {
Loggers.SRV_LOG.warn("NamingProxy", e);
}
Expand Down