From f0d091e1177f1f587e4079b5d4b9efb4aa1fe22f Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Mon, 13 Jan 2025 18:38:54 +0100 Subject: [PATCH] avoid server-side request forgery through CompareServersResource and MasterDataImportResource --- .../common/SharedLandscapeConstants.java | 28 +++++++++ .../landscape/test/TestTrustedDomains.java | 51 +++++++++++++++ .../META-INF/MANIFEST.MF | 3 +- .../jaxrs/api/CompareServersResource.java | 14 ++++- .../jaxrs/api/MasterDataImportResource.java | 63 +++++++++++-------- 5 files changed, 128 insertions(+), 31 deletions(-) create mode 100644 java/com.sap.sailing.landscape.test/src/com/sap/sailing/landscape/test/TestTrustedDomains.java diff --git a/java/com.sap.sailing.landscape.common/src/com/sap/sailing/landscape/common/SharedLandscapeConstants.java b/java/com.sap.sailing.landscape.common/src/com/sap/sailing/landscape/common/SharedLandscapeConstants.java index 831bf09c2b3..3f0cd532dd0 100755 --- a/java/com.sap.sailing.landscape.common/src/com/sap/sailing/landscape/common/SharedLandscapeConstants.java +++ b/java/com.sap.sailing.landscape.common/src/com/sap/sailing/landscape/common/SharedLandscapeConstants.java @@ -1,5 +1,10 @@ package com.sap.sailing.landscape.common; +import java.util.Arrays; +import java.util.Collections; + +import com.sap.sse.common.Util; + public interface SharedLandscapeConstants { /** * If no specific domain name is provided, e.g., when creating a new application replica set, this will be @@ -7,6 +12,29 @@ public interface SharedLandscapeConstants { * replica set's name. */ String DEFAULT_DOMAIN_NAME = "sapsailing.com"; + + /** + * Servers in any of these domains we want to trust. This can and shall be used, e.g., to guard server-side requests + * to URLs that may have been provided through an API or UI by some potentially untrusted client or user. + */ + Iterable TRUSTED_DOMAINS = Collections.unmodifiableCollection(Arrays.asList(new String[] { + DEFAULT_DOMAIN_NAME, "sailing.omegatiming.com", "localhost", "127.0.0.1" + })); + + /** + * Checks that {@code domain} equals one of {@link #TRUSTED_DOMAINS} or is a sub-domain of any of these + */ + static boolean isTrustedDomain(String domain) { + while (Util.hasLength(domain)) { + if (Util.contains(TRUSTED_DOMAINS, domain)) { + return true; + } else { + final int indexOfSubdomainSeparator = domain.indexOf('.'); + domain = indexOfSubdomainSeparator >= 0 ? domain.substring(indexOfSubdomainSeparator+1) : ""; + } + } + return false; + } /** * If a shared security realm is to be used for a domain then this constant tells the name of the application diff --git a/java/com.sap.sailing.landscape.test/src/com/sap/sailing/landscape/test/TestTrustedDomains.java b/java/com.sap.sailing.landscape.test/src/com/sap/sailing/landscape/test/TestTrustedDomains.java new file mode 100644 index 00000000000..8a77b53078a --- /dev/null +++ b/java/com.sap.sailing.landscape.test/src/com/sap/sailing/landscape/test/TestTrustedDomains.java @@ -0,0 +1,51 @@ +package com.sap.sailing.landscape.test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.sap.sailing.landscape.common.SharedLandscapeConstants; + +public class TestTrustedDomains { + @Test + public void testSapSailingCom() { + assertTrue(SharedLandscapeConstants.isTrustedDomain("sapsailing.com")); + } + + @Test + public void testSapSailingComSubdomain() { + assertTrue(SharedLandscapeConstants.isTrustedDomain("wind.sapsailing.com")); + } + + @Test + public void testSapSailingComSubSubdomain() { + assertTrue(SharedLandscapeConstants.isTrustedDomain("test.wind.sapsailing.com")); + } + + @Test + public void testSapSailingComInDomain() { + assertFalse(SharedLandscapeConstants.isTrustedDomain("wind.sapsailing.com.something")); + } + + @Test + public void testDotAtEnd() { + assertFalse(SharedLandscapeConstants.isTrustedDomain("sapsailing.com.")); + } + + @Test + public void testDotAtBeginning() { + assertFalse(SharedLandscapeConstants.isTrustedDomain(".sapsailing.com.")); + } + + @Test + public void testIncorrectDomain() { + assertFalse(SharedLandscapeConstants.isTrustedDomain("sap_sailing.com.")); + } + + @Test + public void testLocalhost() { + assertTrue(SharedLandscapeConstants.isTrustedDomain("127.0.0.1")); + assertTrue(SharedLandscapeConstants.isTrustedDomain("localhost")); + } +} diff --git a/java/com.sap.sailing.server.gateway/META-INF/MANIFEST.MF b/java/com.sap.sailing.server.gateway/META-INF/MANIFEST.MF index f557ea678d4..bdda505a649 100644 --- a/java/com.sap.sailing.server.gateway/META-INF/MANIFEST.MF +++ b/java/com.sap.sailing.server.gateway/META-INF/MANIFEST.MF @@ -69,7 +69,8 @@ Require-Bundle: com.sap.sailing.domain, com.sap.sailing.shared.server.gateway, org.apache.httpcomponents.httpclient, org.apache.httpcomponents.httpcore, - com.sap.sailing.server.gateway.interfaces + com.sap.sailing.server.gateway.interfaces, + com.sap.sailing.landscape.common Bundle-ClassPath: . Web-ContextPath: /sailingserver Export-Package: com.sap.sailing.server.gateway, diff --git a/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/CompareServersResource.java b/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/CompareServersResource.java index 288d0ef159f..86ebdd73048 100644 --- a/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/CompareServersResource.java +++ b/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/CompareServersResource.java @@ -4,6 +4,7 @@ import java.io.FileNotFoundException; import java.io.InputStreamReader; import java.net.ConnectException; import java.net.HttpURLConnection; +import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; @@ -33,6 +34,7 @@ import org.json.simple.JSONValue; import org.json.simple.parser.JSONParser; import com.sap.sailing.domain.common.LeaderboardNameConstants; +import com.sap.sailing.landscape.common.SharedLandscapeConstants; import com.sap.sailing.server.gateway.serialization.LeaderboardGroupConstants; import com.sap.sailing.shared.server.gateway.jaxrs.AbstractSailingServerResource; import com.sap.sse.common.Util; @@ -153,12 +155,18 @@ public class CompareServersResource extends AbstractSailingServerResource { @FormParam(PASSWORD1_FORM_PARAM) String password1, @FormParam(PASSWORD2_FORM_PARAM) String password2, @FormParam(BEARER1_FORM_PARAM) String bearer1, - @FormParam(BEARER2_FORM_PARAM) String bearer2) { + @FormParam(BEARER2_FORM_PARAM) String bearer2) throws MalformedURLException { final Map> result = new HashMap<>(); Response response = null; final String effectiveServer1 = !Util.hasLength(server1) ? uriInfo.getBaseUri().getAuthority() : server1; - if (!validateParameters(server2, uuidset, user1, user2, password1, password2, bearer1, bearer2)) { - response = badRequest("Specify two server names and optionally a set of valid leaderboardgroup UUIDs."); + final URL url1 = RemoteServerUtil.createBaseUrl(effectiveServer1); + final URL url2 = RemoteServerUtil.createBaseUrl(server2); + if (!SharedLandscapeConstants.isTrustedDomain(url1.getHost())) { + response = badRequest("Untrusted domain for "+url1); + } else if (!SharedLandscapeConstants.isTrustedDomain(url2.getHost())) { + response = badRequest("Untrusted domain for "+url2); + } else if (!validateParameters(server2, uuidset, user1, user2, password1, password2, bearer1, bearer2)) { + response = badRequest("Specify two trusted server names and optionally a set of valid leaderboardgroup UUIDs."); } else { final String token1 = getSecurityService().getOrCreateTargetServerBearerToken(effectiveServer1, user1, password1, bearer1); final String token2 = getSecurityService().getOrCreateTargetServerBearerToken(server2, user2, password2, bearer2); diff --git a/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/MasterDataImportResource.java b/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/MasterDataImportResource.java index f6567503c9a..084ccaf123a 100644 --- a/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/MasterDataImportResource.java +++ b/java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/jaxrs/api/MasterDataImportResource.java @@ -1,5 +1,7 @@ package com.sap.sailing.server.gateway.jaxrs.api; +import java.net.MalformedURLException; +import java.net.URL; import java.util.List; import java.util.Map; import java.util.UUID; @@ -23,6 +25,7 @@ import org.json.simple.JSONObject; import com.sap.sailing.domain.base.Event; import com.sap.sailing.domain.common.DataImportProgress; import com.sap.sailing.domain.leaderboard.LeaderboardGroup; +import com.sap.sailing.landscape.common.SharedLandscapeConstants; import com.sap.sailing.server.gateway.dto.MasterDataImportResultImpl; import com.sap.sailing.server.gateway.interfaces.MasterDataImportResult; import com.sap.sailing.server.gateway.serialization.impl.DataImportProgressJsonSerializer; @@ -30,6 +33,7 @@ import com.sap.sailing.server.gateway.serialization.impl.MasterDataImportResultJ import com.sap.sailing.shared.server.gateway.jaxrs.AbstractSailingServerResource; import com.sap.sse.common.Util; import com.sap.sse.security.shared.impl.SecuredSecurityTypes.ServerActions; +import com.sap.sse.security.util.RemoteServerUtil; @Path(MasterDataImportResource.V1_MASTERDATAIMPORT) public class MasterDataImportResource extends AbstractSailingServerResource { @@ -59,37 +63,42 @@ public class MasterDataImportResource extends AbstractSailingServerResource { @FormParam(MasterDataImportResultJsonSerializer.EXPORT_WIND_FORM_PARAM) @DefaultValue("true") Boolean exportWind, @FormParam(MasterDataImportResultJsonSerializer.EXPORT_DEVICE_CONFIGS_FORM_PARAM) @DefaultValue("false") Boolean exportDeviceConfigs, @FormParam(MasterDataImportResultJsonSerializer.EXPORT_TRACKED_RACES_AND_START_TRACKING_FORM_PARAM) @DefaultValue("true") Boolean exportTrackedRacesAndStartTracking, - @FormParam(PROGRESS_TRACKING_UUID_FORM_PARAM) String progressTrackingUuid) { + @FormParam(PROGRESS_TRACKING_UUID_FORM_PARAM) String progressTrackingUuid) throws MalformedURLException { Response response = null; if (!Util.hasLength(remoteServerUrlAsString)) { response = badRequest("Remote server URL parameter "+REMOTE_SERVER_URL_FORM_PARAM+" must be present and non-empty"); - } else if (!validateAuthenticationParameters(remoteServerUsername, remoteServerPassword, remoteServerBearerToken)) { - response = badRequest("Specify "+REMOTE_SERVER_USERNAME_FORM_PARAM+" and "+REMOTE_SERVER_PASSWORD_FORM_PARAM+" or alternatively "+REMOTE_SERVER_BEARER_TOKEN_FORM_PARAM+" or none of them."); } else { - final UUID importMasterDataUid = progressTrackingUuid == null ? UUID.randomUUID() : UUID.fromString(progressTrackingUuid); - try { - getSecurityService().checkCurrentUserServerPermission(ServerActions.CAN_IMPORT_MASTERDATA); - final Map> eventsForLeaderboardGroups = getService() - .importMasterData(remoteServerUrlAsString, - requestedLeaderboardGroupIds.toArray(new UUID[requestedLeaderboardGroupIds.size()]), - override, compress, exportWind, exportDeviceConfigs, remoteServerUsername, - remoteServerPassword, remoteServerBearerToken, exportTrackedRacesAndStartTracking, - importMasterDataUid); - final MasterDataImportResult result = new MasterDataImportResultImpl( - eventsForLeaderboardGroups, remoteServerUrlAsString, override, exportWind, - exportDeviceConfigs, exportTrackedRacesAndStartTracking); - final JSONObject jsonResponse = new MasterDataImportResultJsonSerializer().serialize(result); - response = Response.ok(streamingOutput(jsonResponse)).build(); - } catch (UnauthorizedException e) { - response = Response.status(Status.UNAUTHORIZED).build(); - logger.warning(e.getMessage() + " for user: " + getSecurityService().getCurrentUser()); - } catch (IllegalArgumentException e) { - response = Response.status(Status.BAD_REQUEST).entity(e.getMessage()).type(MediaType.TEXT_PLAIN).build(); - logger.warning(e.getMessage()); - } catch (Throwable e) { - response = Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()) - .type(MediaType.TEXT_PLAIN).build(); - logger.severe(e.toString()); + final URL url = RemoteServerUtil.createBaseUrl(remoteServerUrlAsString); + if (!SharedLandscapeConstants.isTrustedDomain(url.getHost())) { + response = badRequest("Untrusted domain for "+url); + } else if (!validateAuthenticationParameters(remoteServerUsername, remoteServerPassword, remoteServerBearerToken)) { + response = badRequest("Specify "+REMOTE_SERVER_USERNAME_FORM_PARAM+" and "+REMOTE_SERVER_PASSWORD_FORM_PARAM+" or alternatively "+REMOTE_SERVER_BEARER_TOKEN_FORM_PARAM+" or none of them."); + } else { + final UUID importMasterDataUid = progressTrackingUuid == null ? UUID.randomUUID() : UUID.fromString(progressTrackingUuid); + try { + getSecurityService().checkCurrentUserServerPermission(ServerActions.CAN_IMPORT_MASTERDATA); + final Map> eventsForLeaderboardGroups = getService() + .importMasterData(remoteServerUrlAsString, + requestedLeaderboardGroupIds.toArray(new UUID[requestedLeaderboardGroupIds.size()]), + override, compress, exportWind, exportDeviceConfigs, remoteServerUsername, + remoteServerPassword, remoteServerBearerToken, exportTrackedRacesAndStartTracking, + importMasterDataUid); + final MasterDataImportResult result = new MasterDataImportResultImpl( + eventsForLeaderboardGroups, remoteServerUrlAsString, override, exportWind, + exportDeviceConfigs, exportTrackedRacesAndStartTracking); + final JSONObject jsonResponse = new MasterDataImportResultJsonSerializer().serialize(result); + response = Response.ok(streamingOutput(jsonResponse)).build(); + } catch (UnauthorizedException e) { + response = Response.status(Status.UNAUTHORIZED).build(); + logger.warning(e.getMessage() + " for user: " + getSecurityService().getCurrentUser()); + } catch (IllegalArgumentException e) { + response = Response.status(Status.BAD_REQUEST).entity(e.getMessage()).type(MediaType.TEXT_PLAIN).build(); + logger.warning(e.getMessage()); + } catch (Throwable e) { + response = Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()) + .type(MediaType.TEXT_PLAIN).build(); + logger.severe(e.toString()); + } } } return response;