Merge pull request #1623 from nkorange/feature_forbid_invalid_namespace
Fix namespace vulnerability
This commit is contained in:
commit
e5df89fde1
@ -15,10 +15,12 @@
|
|||||||
*/
|
*/
|
||||||
package com.alibaba.nacos.naming.web;
|
package com.alibaba.nacos.naming.web;
|
||||||
|
|
||||||
|
import com.alibaba.nacos.api.naming.CommonParams;
|
||||||
import com.alibaba.nacos.naming.acl.AuthChecker;
|
import com.alibaba.nacos.naming.acl.AuthChecker;
|
||||||
import com.alibaba.nacos.naming.misc.SwitchDomain;
|
import com.alibaba.nacos.naming.misc.SwitchDomain;
|
||||||
import com.alibaba.nacos.naming.misc.UtilsAndCommons;
|
import com.alibaba.nacos.naming.misc.UtilsAndCommons;
|
||||||
|
|
||||||
|
import org.apache.commons.lang3.StringUtils;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
|
|
||||||
import javax.servlet.*;
|
import javax.servlet.*;
|
||||||
@ -34,6 +36,8 @@ import java.security.AccessControlException;
|
|||||||
*/
|
*/
|
||||||
public class AuthFilter implements Filter {
|
public class AuthFilter implements Filter {
|
||||||
|
|
||||||
|
private static final String[] NAMESPACE_FORBIDDEN_STRINGS = new String[]{"..", "/"};
|
||||||
|
|
||||||
@Autowired
|
@Autowired
|
||||||
private AuthChecker authChecker;
|
private AuthChecker authChecker;
|
||||||
|
|
||||||
@ -64,11 +68,16 @@ public class AuthFilter implements Filter {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (method.isAnnotationPresent(NeedAuth.class) && !switchDomain.isEnableAuthentication()) {
|
if (method.isAnnotationPresent(NeedAuth.class) && !switchDomain.isEnableAuthentication()) {
|
||||||
|
// leave it empty.
|
||||||
|
}
|
||||||
|
|
||||||
if (path.contains(UtilsAndCommons.NACOS_NAMING_RAFT_CONTEXT)) {
|
// Check namespace:
|
||||||
authChecker.doRaftAuth(req);
|
String namespaceId = req.getParameter(CommonParams.NAMESPACE_ID);
|
||||||
} else {
|
|
||||||
authChecker.doAuth(req.getParameterMap(), req);
|
if (StringUtils.isNotBlank(namespaceId)) {
|
||||||
|
|
||||||
|
if (namespaceId.contains(NAMESPACE_FORBIDDEN_STRINGS[0]) || namespaceId.contains(NAMESPACE_FORBIDDEN_STRINGS[1])) {
|
||||||
|
throw new IllegalArgumentException("forbidden namespace: " + namespaceId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -78,9 +87,12 @@ public class AuthFilter implements Filter {
|
|||||||
} catch (NoSuchMethodException e) {
|
} catch (NoSuchMethodException e) {
|
||||||
resp.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED, "no such api");
|
resp.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED, "no such api");
|
||||||
return;
|
return;
|
||||||
|
} catch (IllegalArgumentException e) {
|
||||||
|
resp.sendError(HttpServletResponse.SC_BAD_REQUEST, UtilsAndCommons.getAllExceptionMsg(e));
|
||||||
|
return;
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
|
resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
|
||||||
"Server failed," + UtilsAndCommons.getAllExceptionMsg(e));
|
"Server failed," + UtilsAndCommons.getAllExceptionMsg(e));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
filterChain.doFilter(req, resp);
|
filterChain.doFilter(req, resp);
|
||||||
|
@ -52,7 +52,7 @@ public class NamingConfig {
|
|||||||
FilterRegistrationBean<AuthFilter> registration = new FilterRegistrationBean<>();
|
FilterRegistrationBean<AuthFilter> registration = new FilterRegistrationBean<>();
|
||||||
|
|
||||||
registration.setFilter(authFilter());
|
registration.setFilter(authFilter());
|
||||||
registration.addUrlPatterns("/api/*", "/raft/*");
|
registration.addUrlPatterns("/v1/ns/*");
|
||||||
registration.setName("authFilter");
|
registration.setName("authFilter");
|
||||||
registration.setOrder(5);
|
registration.setOrder(5);
|
||||||
|
|
||||||
|
@ -232,6 +232,32 @@ public class RestAPI_ITCase {
|
|||||||
namingServiceDelete(serviceName);
|
namingServiceDelete(serviceName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testInvalidNamespace() {
|
||||||
|
|
||||||
|
String serviceName = NamingBase.randomDomainName();
|
||||||
|
ResponseEntity<String> response = request(NamingBase.NAMING_CONTROLLER_PATH + "/service",
|
||||||
|
Params.newParams()
|
||||||
|
.appendParam("serviceName", serviceName)
|
||||||
|
.appendParam("protectThreshold", "0.6")
|
||||||
|
.appendParam("namespaceId", "..invalid-namespace")
|
||||||
|
.done(),
|
||||||
|
String.class,
|
||||||
|
HttpMethod.POST);
|
||||||
|
Assert.assertTrue(response.getStatusCode().is4xxClientError());
|
||||||
|
|
||||||
|
response = request(NamingBase.NAMING_CONTROLLER_PATH + "/service",
|
||||||
|
Params.newParams()
|
||||||
|
.appendParam("serviceName", serviceName)
|
||||||
|
.appendParam("protectThreshold", "0.6")
|
||||||
|
.appendParam("namespaceId", "/invalid-namespace")
|
||||||
|
.done(),
|
||||||
|
String.class,
|
||||||
|
HttpMethod.POST);
|
||||||
|
Assert.assertTrue(response.getStatusCode().is4xxClientError());
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
private void namingServiceDelete(String serviceName) {
|
private void namingServiceDelete(String serviceName) {
|
||||||
//delete service
|
//delete service
|
||||||
ResponseEntity<String> response = request(NamingBase.NAMING_CONTROLLER_PATH + "/service",
|
ResponseEntity<String> response = request(NamingBase.NAMING_CONTROLLER_PATH + "/service",
|
||||||
|
Loading…
Reference in New Issue
Block a user