Ip Address tracking configurable.

Signed-off-by: Kai Zimmermann <kai.zimmermann@bosch-si.com>

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());

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<DdiControllerBase> getControllerBase(@PathVariable("targetid") final String targetid) {
         LOG.debug("getControllerBase({})", targetid);
 
-        final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist(targetid,
+        final Target target = controllerManagement.findOrRegisterTargetIfItDoesNotexist(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
 
         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(),
+                    System.currentTimeMillis(), IpUtil.getClientIpFromRequest(
-                    IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                            requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                            securityProperties.getClients().getRemoteIpHeader()));
         }
 
         return new ResponseEntity<>(
@@ -143,9 +141,8 @@ public class DdiRootController implements DdiRootControllerRestApi {
             @PathVariable("fileName") final String fileName) {
         ResponseEntity<InputStream> result;
 
-        final Target target = controllerManagement.updateLastTargetQuery(targetid,
+        final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
         final SoftwareModule module = softwareManagement.findSoftwareModuleById(softwareModuleId);
 
         if (checkModule(fileName, module)) {
@@ -200,9 +197,8 @@ public class DdiRootController implements DdiRootControllerRestApi {
     public ResponseEntity<Void> downloadArtifactMd5(@PathVariable("targetid") final String targetid,
             @PathVariable("softwareModuleId") final Long softwareModuleId,
             @PathVariable("fileName") final String fileName) {
-        controllerManagement.updateLastTargetQuery(targetid,
+        controllerManagement.updateLastTargetQuery(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
 
         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,
+        final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
 
         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,
+        final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
 
         if (!actionId.equals(feedback.getId())) {
             LOG.warn(
@@ -357,9 +351,8 @@ public class DdiRootController implements DdiRootControllerRestApi {
     @Override
     public ResponseEntity<Void> putConfigData(@Valid @RequestBody final DdiConfigData configData,
             @PathVariable("targetid") final String targetid) {
-        controllerManagement.updateLastTargetQuery(targetid,
+        controllerManagement.updateLastTargetQuery(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
 
         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,
+        final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
 
         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,
+        final Target target = controllerManagement.updateLastTargetQuery(targetid, IpUtil
-                IpUtil.getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(),
+                .getClientIpFromRequest(requestResponseContextHolder.getHttpServletRequest(), securityProperties));
-                        securityProperties.getClients().getRemoteIpHeader()));
 
         if (!actionId.equals(feedback.getId())) {
             LOG.warn(

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 {

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 {

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 <code>true</code> 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;
+        }
     }
 
     /**

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
+     * @param securityProperties
-     *            the header name containing the IP address e.g. forwarded by a
+     *            hawkBit security properties.
-     *            proxy {@code x-forwarded-for}
      * @return the {@link URI} based IP address from the client which sent the
      *         request
      */
-    public static URI getClientIpFromRequest(final HttpServletRequest request, final String forwardHeader) {
+    public static URI getClientIpFromRequest(final HttpServletRequest request,
-        String ip = request.getHeader(forwardHeader);
+            final HawkbitSecurityProperties securityProperties) {
-        if (ip == null || (ip = findClientIpAddress(ip)) == null) {
+
-            ip = request.getRemoteAddr();
+        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 <code>true</code> 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);
     }
 

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")