diff --git a/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactStoreController.java b/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactStoreController.java index 85a326f9a..75d502c47 100644 --- a/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactStoreController.java +++ b/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactStoreController.java @@ -134,7 +134,7 @@ public class DdiArtifactStoreController implements DdiDlArtifactStoreControllerR private Action checkAndReportDownloadByTarget(final HttpServletRequest request, final String targetid, final LocalArtifact artifact) { final Target target = controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(request, securityProperties.getClients().getRemoteIpHeader())); + IpUtil.getClientIpFromRequest(request, securityProperties)); final Action action = controllerManagement .getActionForDownloadByTargetAndSoftwareModule(target.getControllerId(), artifact.getSoftwareModule()); diff --git a/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java b/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java index 20d1b2e29..120e6c246 100644 --- a/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java +++ b/hawkbit-ddi-resource/src/main/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootController.java @@ -119,16 +119,14 @@ public class DdiRootController implements DdiRootControllerRestApi { public ResponseEntity getControllerBase(@PathVariable("targetid") final String targetid) { LOG.debug("getControllerBase({})", targetid); - final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); if (target.getTargetInfo().getUpdateStatus() == TargetUpdateStatus.UNKNOWN) { LOG.debug("target with {} extsisted but was in status UNKNOWN -> REGISTERED)", targetid); controllerManagement.updateTargetStatus(target.getTargetInfo(), TargetUpdateStatus.REGISTERED, - System.currentTimeMillis(), - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + System.currentTimeMillis(), IpUtil.getClientIpFromRequest( + requestResponseContextHolder.getHttpServletRequest(), securityProperties)); } return new ResponseEntity<>( @@ -143,9 +141,8 @@ public class DdiRootController implements DdiRootControllerRestApi { @PathVariable("fileName") final String fileName) { ResponseEntity result; - final Target target = controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); final SoftwareModule module = softwareManagement.findSoftwareModuleById(softwareModuleId); if (checkModule(fileName, module)) { @@ -200,9 +197,8 @@ public class DdiRootController implements DdiRootControllerRestApi { public ResponseEntity downloadArtifactMd5(@PathVariable("targetid") final String targetid, @PathVariable("softwareModuleId") final Long softwareModuleId, @PathVariable("fileName") final String fileName) { - controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + controllerManagement.updateLastTargetQuery(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); final SoftwareModule module = softwareManagement.findSoftwareModuleById(softwareModuleId); @@ -228,9 +224,8 @@ public class DdiRootController implements DdiRootControllerRestApi { @RequestParam(value = "c", required = false, defaultValue = "-1") final int resource) { LOG.debug("getControllerBasedeploymentAction({},{})", targetid, resource); - final Target target = controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); final Action action = findActionWithExceptionIfNotFound(actionId); if (!action.getTarget().getId().equals(target.getId())) { @@ -263,9 +258,8 @@ public class DdiRootController implements DdiRootControllerRestApi { @PathVariable("targetid") final String targetid, @PathVariable("actionId") @NotEmpty final Long actionId) { LOG.debug("provideBasedeploymentActionFeedback for target [{},{}]: {}", targetid, actionId, feedback); - final Target target = controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); if (!actionId.equals(feedback.getId())) { LOG.warn( @@ -357,9 +351,8 @@ public class DdiRootController implements DdiRootControllerRestApi { @Override public ResponseEntity putConfigData(@Valid @RequestBody final DdiConfigData configData, @PathVariable("targetid") final String targetid) { - controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + controllerManagement.updateLastTargetQuery(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); controllerManagement.updateControllerAttributes(targetid, configData.getData()); @@ -372,9 +365,8 @@ public class DdiRootController implements DdiRootControllerRestApi { @PathVariable("actionId") @NotEmpty final Long actionId) { LOG.debug("getControllerCancelAction({})", targetid); - final Target target = controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); final Action action = findActionWithExceptionIfNotFound(actionId); if (!action.getTarget().getId().equals(target.getId())) { @@ -403,9 +395,8 @@ public class DdiRootController implements DdiRootControllerRestApi { @PathVariable("actionId") @NotEmpty final Long actionId) { LOG.debug("provideCancelActionFeedback for target [{}]: {}", targetid, feedback); - final Target target = controllerManagement.updateLastTargetQuery(targetid, - IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), - securityProperties.getClients().getRemoteIpHeader())); + final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil + .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties)); if (!actionId.equals(feedback.getId())) { LOG.warn( diff --git a/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java b/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java index 135754604..98e9f6838 100644 --- a/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java +++ b/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java @@ -33,9 +33,11 @@ import org.eclipse.hawkbit.repository.util.WithUser; import org.eclipse.hawkbit.rest.AbstractRestIntegrationTestWithMongoDB; import org.eclipse.hawkbit.rest.util.JsonBuilder; import org.eclipse.hawkbit.rest.util.MockMvcResultPrinter; +import org.eclipse.hawkbit.security.HawkbitSecurityProperties; import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationKey; import org.eclipse.hawkbit.util.IpUtil; import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.hateoas.MediaTypes; import org.springframework.http.MediaType; @@ -50,6 +52,9 @@ import ru.yandex.qatools.allure.annotations.Stories; @Stories("Root Poll Resource") public class DdiRootControllerTest extends AbstractRestIntegrationTestWithMongoDB { + @Autowired + private HawkbitSecurityProperties securityProperties; + @Test @Description("Ensures that targets cannot be created e.g. in plug'n play scenarios when tenant does not exists but can be created if the tenant exists.") @WithUser(tenantId = "tenantDoesNotExists", allSpPermissions = true, authorities = "ROLE_CONTROLLER", autoCreateTenant = false) @@ -257,6 +262,23 @@ public class DdiRootControllerTest extends AbstractRestIntegrationTestWithMongoD } + @Test + @Description("Ensures that the source IP address of the polling target is not stored in repository if disabled") + public void rootRsIpAddressNotStoredIfDisabled() throws Exception { + securityProperties.getClients().setTrackRemoteIp(false); + + // test + final String knownControllerId1 = "0815"; + mvc.perform(get("/{tenant}/controller/v1/{controllerId}", tenantAware.getCurrentTenant(), knownControllerId1)) + .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()); + + // verify + final Target target = targetManagement.findTargetByControllerID(knownControllerId1); + assertThat(target.getTargetInfo().getAddress()).isEqualTo(IpUtil.createHttpUri("***")); + + securityProperties.getClients().setTrackRemoteIp(true); + } + @Test @Description("Controller trys to finish an update process after it has been finished by an error action status.") public void tryToFinishAnUpdateProcessAfterItHasBeenFinished() throws Exception { diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java index f0d8f7d48..961b418a0 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java @@ -110,7 +110,7 @@ public class DosFilter extends OncePerRequestFilter { boolean processChain; - final String ip = IpUtil.getClientIpFromRequest(request, forwardHeader).getHost(); + final String ip = IpUtil.getClientIpFromRequest(request, forwardHeader, true).getHost(); if (checkIpFails(ip)) { processChain = handleMissingIpAddress(response); } else { diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java index 6192a8f31..3f0be994a 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java @@ -82,10 +82,16 @@ public class HawkbitSecurityProperties { private String blacklist = ""; /** - * Name of the http header from which the remote ip is extracted. + * Name of the http header from which the remote ip is extracted for DDI + * connected clients. */ private String remoteIpHeader = "X-Forwarded-For"; + /** + * Set to true if DDI clients remote IP should be stored. + */ + private boolean trackRemoteIp = true; + public String getBlacklist() { return blacklist; } @@ -101,6 +107,14 @@ public class HawkbitSecurityProperties { public void setRemoteIpHeader(final String remoteIpHeader) { this.remoteIpHeader = remoteIpHeader; } + + public boolean isTrackRemoteIp() { + return trackRemoteIp; + } + + public void setTrackRemoteIp(final boolean trackRemoteIp) { + this.trackRemoteIp = trackRemoteIp; + } } /** diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/util/IpUtil.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/util/IpUtil.java index 4e08d8bfe..96fc557aa 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/util/IpUtil.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/util/IpUtil.java @@ -15,6 +15,8 @@ import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; +import org.eclipse.hawkbit.security.HawkbitSecurityProperties; + import com.google.common.net.HttpHeaders; /** @@ -45,17 +47,49 @@ public final class IpUtil { * @param request * the {@link HttpServletRequest} to determine the IP address * where this request has been sent from - * @param forwardHeader - * the header name containing the IP address e.g. forwarded by a - * proxy {@code x-forwarded-for} + * @param securityProperties + * hawkBit security properties. * @return the {@link URI} based IP address from the client which sent the * request */ - public static URI getClientIpFromRequest(final HttpServletRequest request, final String forwardHeader) { - String ip = request.getHeader(forwardHeader); - if (ip == null || (ip = findClientIpAddress(ip)) == null) { - ip = request.getRemoteAddr(); + public static URI getClientIpFromRequest(final HttpServletRequest request, + final HawkbitSecurityProperties securityProperties) { + + return getClientIpFromRequest(request, securityProperties.getClients().getRemoteIpHeader(), + securityProperties.getClients().isTrackRemoteIp()); + } + + /** + * Retrieves the string based IP address from a given + * {@link HttpServletRequest} by either the + * {@link HttpHeaders#X_FORWARDED_FOR} or by the + * {@link HttpServletRequest#getRemoteAddr()} methods. + * + * @param request + * the {@link HttpServletRequest} to determine the IP address + * where this request has been sent from + * @param forwardHeader + * the header name containing the IP address e.g. forwarded by a + * proxy {@code x-forwarded-for} + * + * @param trackRemoteIp + * to true if remote IP should be tracked. + * @return the {@link URI} based IP address from the client which sent the + * request + */ + public static URI getClientIpFromRequest(final HttpServletRequest request, final String forwardHeader, + final boolean trackRemoteIp) { + String ip; + + if (trackRemoteIp) { + ip = request.getHeader(forwardHeader); + if (ip == null || (ip = findClientIpAddress(ip)) == null) { + ip = request.getRemoteAddr(); + } + } else { + ip = "***"; } + return createHttpUri(ip); } diff --git a/hawkbit-security-core/src/test/java/org/eclipse/hawkbit/util/IpUtilTest.java b/hawkbit-security-core/src/test/java/org/eclipse/hawkbit/util/IpUtilTest.java index 9eb83d2a9..0f4a01d26 100644 --- a/hawkbit-security-core/src/test/java/org/eclipse/hawkbit/util/IpUtilTest.java +++ b/hawkbit-security-core/src/test/java/org/eclipse/hawkbit/util/IpUtilTest.java @@ -50,7 +50,7 @@ public class IpUtilTest { when(requestMock.getRemoteAddr()).thenReturn(knownRemoteClientIP.getHost()); // test - final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "bumlux"); + final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "bumlux", true); // verify assertThat(remoteAddr).as("The remote address should be as the known client IP address") @@ -59,6 +59,25 @@ public class IpUtilTest { verify(requestMock, times(1)).getRemoteAddr(); } + @Test + @Description("Tests create uri from request with masked IP when IP tracking is disabled") + public void maskRemoteAddrIfDisabled() { + // known values + final URI knownRemoteClientIP = IpUtil.createHttpUri("***"); + // mock + when(requestMock.getHeader(HttpHeaders.X_FORWARDED_FOR)).thenReturn(null); + when(requestMock.getRemoteAddr()).thenReturn(knownRemoteClientIP.getHost()); + + // test + final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "bumlux", false); + + // verify + assertThat(remoteAddr).as("The remote address should be as the known client IP address") + .isEqualTo(knownRemoteClientIP); + verify(requestMock, times(0)).getHeader("bumlux"); + verify(requestMock, times(0)).getRemoteAddr(); + } + @Test @Description("Tests create uri from x forward header") public void getRemoteAddrFromXForwardedForHeader() { @@ -69,7 +88,7 @@ public class IpUtilTest { when(requestMock.getRemoteAddr()).thenReturn(null); // test - final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "X-Forwarded-For"); + final URI remoteAddr = IpUtil.getClientIpFromRequest(requestMock, "X-Forwarded-For", true); // verify assertThat(remoteAddr).as("The remote address should be as the known client IP address")