summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRemigiusz Janeczek <remigiusz.janeczek@nokia.com>2019-10-24 08:50:49 +0200
committerRemigiusz Janeczek <remigiusz.janeczek@nokia.com>2019-11-04 08:09:07 +0100
commitdb5f3e1fc72065397898bf5e8d1f03f3140600d0 (patch)
tree7043e6faf4fd457845b5b91a2f25ad3eba2511c8
parentb9a91b3967a5b415d91bf00454b428bb93a567b6 (diff)
Fix security issue in api interceptor
Issue-ID: DCAEGEN2-1880 Change-Id: I5b93dd8405ef9a0a364c6e1224afcfacc9df1fba Signed-off-by: Remigiusz Janeczek <remigiusz.janeczek@nokia.com>
-rw-r--r--src/main/java/org/onap/dcae/restapi/ApiAuthInterceptor.java14
-rw-r--r--src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java73
2 files changed, 61 insertions, 26 deletions
diff --git a/src/main/java/org/onap/dcae/restapi/ApiAuthInterceptor.java b/src/main/java/org/onap/dcae/restapi/ApiAuthInterceptor.java
index a9281594..f1734080 100644
--- a/src/main/java/org/onap/dcae/restapi/ApiAuthInterceptor.java
+++ b/src/main/java/org/onap/dcae/restapi/ApiAuthInterceptor.java
@@ -55,8 +55,8 @@ public class ApiAuthInterceptor extends HandlerInterceptorAdapter {
SubjectMatcher subjectMatcher = new SubjectMatcher(settings,(X509Certificate[]) request.getAttribute(CERTIFICATE_X_509));
- if(!settings.authMethod().equalsIgnoreCase(AuthMethodType.NO_AUTH.value()) && request.getServerPort() == settings.httpPort() ){
- if(request.getRequestURI().replaceAll("^/|/$", "").equalsIgnoreCase("healthcheck")){
+ if(isHttpPortCalledWithAuthTurnedOn(request)){
+ if(isHealthcheckCalledFromInsideCluster(request)){
return true;
}
response.getWriter().write("Operation not permitted");
@@ -78,6 +78,16 @@ public class ApiAuthInterceptor extends HandlerInterceptorAdapter {
return true;
}
+ private boolean isHttpPortCalledWithAuthTurnedOn(HttpServletRequest request) {
+ return !settings.authMethod().equalsIgnoreCase(AuthMethodType.NO_AUTH.value())
+ && request.getLocalPort() == settings.httpPort();
+ }
+
+ private boolean isHealthcheckCalledFromInsideCluster(HttpServletRequest request) {
+ return request.getRequestURI().replaceAll("^/|/$", "").equalsIgnoreCase("healthcheck")
+ && request.getServerPort() == settings.httpPort();
+ }
+
private boolean validateBasicHeader(HttpServletRequest request, HttpServletResponse response)
throws IOException {
String authorizationHeader = request.getHeader("Authorization");
diff --git a/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java b/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java
index c80b56cb..250292f3 100644
--- a/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java
+++ b/src/test/java/org/onap/dcae/restapi/ApiAuthInterceptionTest.java
@@ -32,6 +32,7 @@ import org.onap.dcae.common.configuration.AuthMethodType;
import org.slf4j.Logger;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
+import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
@@ -50,6 +51,9 @@ public class ApiAuthInterceptionTest {
private static final String USERNAME = "Foo";
private static final String PASSWORD = "Bar";
private static final Map<String, String> CREDENTIALS = HashMap.of(USERNAME, PASSWORD);
+ private static final int HTTP_PORT = 8080;
+ private static final int OUTSIDE_PORT = 30235;
+ public static final String HEALTHCHECK_URL = "/healthcheck";
@Mock
private Logger log;
@@ -70,21 +74,6 @@ public class ApiAuthInterceptionTest {
private ApiAuthInterceptor sut;
- private HttpServletRequest createEmptyRequest() {
- return MockMvcRequestBuilders
- .post("")
- .buildRequest(null);
- }
-
- private HttpServletRequest createRequestWithAuthorizationHeader() {
- return SecurityMockMvcRequestPostProcessors
- .httpBasic(USERNAME, PASSWORD)
- .postProcessRequest(
- MockMvcRequestBuilders
- .post("")
- .buildRequest(null));
- }
-
@Test
public void shouldSucceedWhenAuthorizationIsDisabled() throws IOException {
// given
@@ -176,16 +165,12 @@ public class ApiAuthInterceptionTest {
}
@Test
- public void shouldSucceedForHealthcheckOnHealthcheckPort() throws IOException {
+ public void shouldSucceedForHealthcheckOnHealthcheckPortWhenRequestFromInsideCluster() throws IOException {
// given
- final HttpServletRequest request =
- MockMvcRequestBuilders
- .get("/healthcheck")
- .buildRequest(null);
+ final HttpServletRequest request = createRequestWithPorts(HTTP_PORT, HTTP_PORT, HEALTHCHECK_URL);
when(settings.authMethod()).thenReturn(AuthMethodType.CERT_BASIC_AUTH.value());
- when(settings.httpPort()).thenReturn(request.getServerPort());
-
+ when(settings.httpPort()).thenReturn(HTTP_PORT);
// when
final boolean isAuthorized = sut.preHandle(request, response, obj);
@@ -194,12 +179,29 @@ public class ApiAuthInterceptionTest {
}
@Test
+ public void shouldFailForHealthcheckOnHealthcheckPortWhenRequestFromOutsideCluster() throws IOException {
+ // given
+ final HttpServletRequest request = createRequestWithPorts(HTTP_PORT, OUTSIDE_PORT, HEALTHCHECK_URL);
+
+ when(settings.authMethod()).thenReturn(AuthMethodType.CERT_BASIC_AUTH.value());
+ when(settings.httpPort()).thenReturn(HTTP_PORT);
+ when(response.getWriter()).thenReturn(writer);
+
+ // when
+ final boolean isAuthorized = sut.preHandle(request, response, obj);
+
+ // then
+ assertFalse(isAuthorized);
+ verify(response).setStatus(HttpStatus.BAD_REQUEST.value());
+ }
+
+ @Test
public void shouldFailDueToNotPermittedOperationOnHealthcheckPort() throws IOException {
// given
- final HttpServletRequest request = createEmptyRequest();
+ final HttpServletRequest request = createRequestWithPorts(HTTP_PORT, HTTP_PORT, "/");
when(settings.authMethod()).thenReturn(AuthMethodType.CERT_BASIC_AUTH.value());
- when(settings.httpPort()).thenReturn(request.getServerPort());
+ when(settings.httpPort()).thenReturn(HTTP_PORT);
when(response.getWriter()).thenReturn(writer);
// when
@@ -210,4 +212,27 @@ public class ApiAuthInterceptionTest {
verify(response).setStatus(HttpStatus.BAD_REQUEST.value());
}
+ private HttpServletRequest createEmptyRequest() {
+ return MockMvcRequestBuilders
+ .post("")
+ .buildRequest(null);
+ }
+
+ private HttpServletRequest createRequestWithAuthorizationHeader() {
+ return SecurityMockMvcRequestPostProcessors
+ .httpBasic(USERNAME, PASSWORD)
+ .postProcessRequest(
+ MockMvcRequestBuilders
+ .post("")
+ .buildRequest(null));
+ }
+
+ private HttpServletRequest createRequestWithPorts(int localPort, int serverPort, String urlTemplate) {
+ MockHttpServletRequest healthcheckRequest = MockMvcRequestBuilders
+ .get(urlTemplate)
+ .buildRequest(null);
+ healthcheckRequest.setLocalPort(localPort);
+ healthcheckRequest.setServerPort(serverPort);
+ return healthcheckRequest;
+ }
}