From d73a99b1c0e82b2f63098c3f57fec524029808b4 Mon Sep 17 00:00:00 2001 From: LSmyrnaios Date: Mon, 12 Sep 2022 16:38:44 +0300 Subject: [PATCH] - Increase the security of "shutdownWorker" and "cancelShutdownWorker" endpoints, by only allowing the requests, which come from the same machine. - Update the "UriBuilder.java" to be able to take the running port of the server, in case the port-number was initially set to "random" (0). --- .../urls_worker/UrlsWorkerApplication.java | 5 +- .../controllers/GeneralController.java | 65 +++++++++++++------ .../openaire/urls_worker/util/UriBuilder.java | 20 +++--- 3 files changed, 56 insertions(+), 34 deletions(-) diff --git a/src/main/java/eu/openaire/urls_worker/UrlsWorkerApplication.java b/src/main/java/eu/openaire/urls_worker/UrlsWorkerApplication.java index 253e875..89d8226 100644 --- a/src/main/java/eu/openaire/urls_worker/UrlsWorkerApplication.java +++ b/src/main/java/eu/openaire/urls_worker/UrlsWorkerApplication.java @@ -13,6 +13,7 @@ import org.springframework.boot.CommandLineRunner; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.core.env.Environment; @@ -109,9 +110,9 @@ public class UrlsWorkerApplication { } @Bean - public CommandLineRunner setServerBaseUrl(Environment environment) + public CommandLineRunner setServerBaseUrl(Environment environment, ServletWebServerApplicationContext webServerAppCtxt) { - return args -> new UriBuilder(environment); + return args -> new UriBuilder(environment, webServerAppCtxt); } diff --git a/src/main/java/eu/openaire/urls_worker/controllers/GeneralController.java b/src/main/java/eu/openaire/urls_worker/controllers/GeneralController.java index 4be5f27..e962a95 100644 --- a/src/main/java/eu/openaire/urls_worker/controllers/GeneralController.java +++ b/src/main/java/eu/openaire/urls_worker/controllers/GeneralController.java @@ -1,6 +1,7 @@ package eu.openaire.urls_worker.controllers; import eu.openaire.urls_worker.UrlsWorkerApplication; +import eu.openaire.urls_worker.util.UriBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; @@ -10,6 +11,7 @@ import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; +import javax.servlet.http.HttpServletRequest; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -35,34 +37,32 @@ public class GeneralController { public static boolean shouldShutdownWorker = false; @GetMapping("shutdownWorker/{shutdownCode}") - public ResponseEntity shutdownWorkerGracefully(@PathVariable String shutdownCode) + public ResponseEntity shutdownWorkerGracefully(@PathVariable String shutdownCode, HttpServletRequest request) { String initMsg = "Received a \"shutdownWorker\" request."; - if ( shutdownCode.equals(UrlsWorkerApplication.shutdownOrCancelCode) ) { - shouldShutdownWorker = true; - logger.info(initMsg + " The worker will shutdown, after finishing current work."); - return ResponseEntity.ok().build(); - } else { - String errorMsg = initMsg + " But, it contains an invalid \"shutdownCode\": " + shutdownCode; - logger.error(errorMsg); - return ResponseEntity.status(HttpStatus.FORBIDDEN).body(errorMsg); - } + + ResponseEntity responseEntity = passSecurityChecks(request, shutdownCode, initMsg); + if ( responseEntity != null ) + return responseEntity; + + shouldShutdownWorker = true; + logger.info(initMsg + " The worker will shutdown, after finishing current work."); + return ResponseEntity.ok().build(); } @GetMapping("cancelShutdownWorker/{cancelCode}") - public ResponseEntity cancelShutdownWorkerGracefully(@PathVariable String cancelCode) + public ResponseEntity cancelShutdownWorkerGracefully(@PathVariable String cancelCode, HttpServletRequest request) { String initMsg = "Received a \"cancelShutdownWorker\" request."; - if ( cancelCode.equals(UrlsWorkerApplication.shutdownOrCancelCode) ) { - shouldShutdownWorker = false; - logger.info(initMsg + " Any previous \"shutdownWorker\"-request is canceled. The \"maxAssignmentsBatchesToHandleBeforeShutdown\" will still be honored (if it's set)."); - return ResponseEntity.ok().build(); - } else { - String errorMsg = initMsg + " But, it contains an invalid \"cancelCode\": " + cancelCode; - logger.error(errorMsg); - return ResponseEntity.status(HttpStatus.FORBIDDEN).body(errorMsg); - } + + ResponseEntity responseEntity = passSecurityChecks(request, cancelCode, initMsg); + if ( responseEntity != null ) + return responseEntity; + + shouldShutdownWorker = false; + logger.info(initMsg + " Any previous \"shutdownWorker\"-request is canceled. The \"maxAssignmentsBatchesToHandleBeforeShutdown\" will still be honored (if it's set)."); + return ResponseEntity.ok().build(); } @@ -78,4 +78,29 @@ public class GeneralController { return ResponseEntity.ok(handledAssignmentsCounts); } + + public static ResponseEntity passSecurityChecks(HttpServletRequest request, String code, String initMsg) + { + if ( request == null ) { + logger.error(initMsg + " The \"HttpServletRequest\" is null!"); + return ResponseEntity.internalServerError().build(); + } + String remoteAddr = request.getHeader("X-FORWARDED-FOR"); + if ( remoteAddr == null || "".equals(remoteAddr) ) + remoteAddr = request.getRemoteAddr(); + + if ( ! (remoteAddr.equals("127.0.0.1") || remoteAddr.equals(UriBuilder.ip)) ) { + logger.error(initMsg + " The request came from another IP: " + remoteAddr + " | while this worker has the IP: " + UriBuilder.ip); + return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); + } + + if ( !code.equals(UrlsWorkerApplication.shutdownOrCancelCode) ) { + String errorMsg = initMsg + " But, it contains an invalid code: " + code; + logger.error(errorMsg); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(errorMsg); + } + + return null; // The checks are passing. + } + } diff --git a/src/main/java/eu/openaire/urls_worker/util/UriBuilder.java b/src/main/java/eu/openaire/urls_worker/util/UriBuilder.java index 3ffa9bd..1ddcaad 100644 --- a/src/main/java/eu/openaire/urls_worker/util/UriBuilder.java +++ b/src/main/java/eu/openaire/urls_worker/util/UriBuilder.java @@ -3,6 +3,7 @@ package eu.openaire.urls_worker.util; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext; import org.springframework.core.env.Environment; import java.io.BufferedReader; @@ -15,9 +16,11 @@ public class UriBuilder { private static final Logger logger = LoggerFactory.getLogger(UriBuilder.class); + + public static String ip = null; public static String baseUrl = null; - public UriBuilder(Environment environment) { + public UriBuilder(Environment environment, ServletWebServerApplicationContext webServerAppCtxt) { baseUrl = "http"; String sslEnabled = environment.getProperty("server.ssl.enabled"); @@ -28,18 +31,11 @@ public class UriBuilder { baseUrl += sslEnabled.equals("true") ? "s" : ""; baseUrl += "://"; - String hostName = getPublicIP(); - if ( hostName == null ) - hostName = InetAddress.getLoopbackAddress().getHostName(); // Non-null. + ip = getPublicIP(); + if ( ip == null ) + ip = InetAddress.getLoopbackAddress().getHostAddress(); // Non-null. - baseUrl += hostName; - - String serverPort = environment.getProperty("server.port"); - if (serverPort == null) { // This is unacceptable! - logger.error("No property \"server.port\" was found in \"application.properties\"!"); - System.exit(-1); // Well, I guess the Spring Boot would not start in this case anyway. - } - baseUrl += ":" + serverPort; + baseUrl += ip + ":" + webServerAppCtxt.getWebServer().getPort(); String baseInternalPath = environment.getProperty("server.servlet.context-path"); if ( baseInternalPath != null ) {