From 769719ccfe667f059fe0b107a19ec9feb90f2e40 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Mon, 19 Sep 2022 12:41:57 +0530 Subject: [PATCH] fix: Better support for disallowed hosts (#16842) --- .../restApiUtils/helpers/TriggerUtils.java | 11 +-- .../restApiUtils/helpers/URIUtils.java | 16 +-- .../com/appsmith/util/WebClientUtils.java | 97 ++++++++++++++++++- .../com/external/plugins/GraphQLPlugin.java | 19 +--- .../com/external/plugins/RestApiPlugin.java | 20 +--- 5 files changed, 104 insertions(+), 59 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/TriggerUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/TriggerUtils.java index 99987e1ce40..2f3556d864d 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/TriggerUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/TriggerUtils.java @@ -35,10 +35,8 @@ import javax.crypto.SecretKey; import java.io.IOException; -import java.net.InetAddress; import java.net.URI; import java.net.URISyntaxException; -import java.net.UnknownHostException; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; @@ -46,7 +44,6 @@ import java.util.List; import java.util.Set; -import static com.appsmith.external.helpers.restApiUtils.helpers.URIUtils.DISALLOWED_HOSTS; import static org.apache.commons.lang3.StringUtils.isNotEmpty; @NoArgsConstructor @@ -204,14 +201,10 @@ protected Mono httpCall(WebClient webClient, HttpMethod httpMeth * It redirects to partial URI : /api/character/ * In this scenario we should convert the partial URI to complete URI */ - URI redirectUri; + final URI redirectUri; try { redirectUri = new URI(redirectUrl); - if (DISALLOWED_HOSTS.contains(redirectUri.getHost()) - || DISALLOWED_HOSTS.contains(InetAddress.getByName(redirectUri.getHost()).getHostAddress())) { - return Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR, "Host not allowed.")); - } - } catch (URISyntaxException | UnknownHostException e) { + } catch (URISyntaxException e) { return Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e)); } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/URIUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/URIUtils.java index cc3c761b133..70c9b5af977 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/URIUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/URIUtils.java @@ -3,30 +3,21 @@ import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Property; -import lombok.AccessLevel; import lombok.NoArgsConstructor; -import org.apache.commons.lang3.StringUtils; import org.springframework.web.util.UriComponentsBuilder; -import java.net.InetAddress; import java.net.URI; import java.net.URISyntaxException; import java.net.URLEncoder; -import java.net.UnknownHostException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; -import java.util.Set; import static org.apache.commons.collections.CollectionUtils.isEmpty; import static org.apache.commons.lang3.StringUtils.isNotEmpty; @NoArgsConstructor public class URIUtils { - public static final Set DISALLOWED_HOSTS = Set.of( - "169.254.169.254", - "metadata.google.internal" - ); public URI createUriWithQueryParams(ActionConfiguration actionConfiguration, DatasourceConfiguration datasourceConfiguration, String url, @@ -53,7 +44,7 @@ public URI addQueryParamsToURI(URI uri, List allQueryParams, boolean e for (Property queryParam : allQueryParams) { String key = queryParam.getKey(); if (isNotEmpty(key)) { - if (encodeParamsToggle == true) { + if (encodeParamsToggle) { uriBuilder.queryParam( URLEncoder.encode(key, StandardCharsets.UTF_8), URLEncoder.encode((String) queryParam.getValue(), StandardCharsets.UTF_8) @@ -78,9 +69,4 @@ protected String addHttpToUrlWhenPrefixNotPresent(String url) { return "http://" + url; } - public boolean isHostDisallowed(URI uri) throws UnknownHostException { - String host = uri.getHost(); - return StringUtils.isEmpty(host) || DISALLOWED_HOSTS.contains(host) - || DISALLOWED_HOSTS.contains(InetAddress.getByName(host).getHostAddress()); - } } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java index 8861dd16e58..f87b62fa309 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java @@ -1,11 +1,31 @@ package com.appsmith.util; +import io.netty.resolver.AddressResolver; +import io.netty.resolver.AddressResolverGroup; +import io.netty.resolver.InetNameResolver; +import io.netty.resolver.InetSocketAddressResolver; +import io.netty.util.concurrent.EventExecutor; +import io.netty.util.concurrent.Promise; +import io.netty.util.internal.SocketUtils; +import lombok.extern.slf4j.Slf4j; import org.springframework.http.client.reactive.ReactorClientHttpConnector; import org.springframework.web.reactive.function.client.WebClient; import reactor.netty.http.client.HttpClient; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.List; +import java.util.Set; + public class WebClientUtils { + private static final Set DISALLOWED_HOSTS = Set.of( + "169.254.169.254", + "metadata.google.internal" + ); + private WebClientUtils() { } @@ -31,15 +51,86 @@ public static WebClient.Builder builder() { public static WebClient.Builder builder(HttpClient httpClient) { return WebClient.builder() - .clientConnector(new ReactorClientHttpConnector(applyProxyIfConfigured(httpClient))); + .clientConnector(new ReactorClientHttpConnector(makeSafeHttpClient(httpClient))); } - private static HttpClient applyProxyIfConfigured(HttpClient httpClient) { + private static HttpClient makeSafeHttpClient(HttpClient httpClient) { if (shouldUseSystemProxy()) { httpClient = httpClient.proxyWithSystemProperties(); } - return httpClient; + return httpClient.resolver(ResolverGroup.INSTANCE); + } + + private static class ResolverGroup extends AddressResolverGroup { + public static final ResolverGroup INSTANCE = new ResolverGroup(); + + @Override + protected AddressResolver newResolver(EventExecutor executor) { + return new InetSocketAddressResolver(executor, new NameResolver(executor)); + } + } + + @Slf4j + private static class NameResolver extends InetNameResolver { + + public NameResolver(EventExecutor executor) { + super(executor); + } + + private static boolean isDisallowedAndFail(String host, Promise promise) { + if (DISALLOWED_HOSTS.contains(host)) { + log.warn("Host {} is disallowed. Failing the request.", host); + promise.setFailure(new UnknownHostException("Host not allowed.")); + return true; + } + return false; + } + + @Override + protected void doResolve(String inetHost, Promise promise) { + if (isDisallowedAndFail(inetHost, promise)) { + return; + } + + final InetAddress address; + try { + address = SocketUtils.addressByName(inetHost); + } catch (UnknownHostException e) { + promise.setFailure(e); + return; + } + + if (isDisallowedAndFail(address.getHostAddress(), promise)) { + return; + } + + promise.setSuccess(address); + } + + @Override + protected void doResolveAll(String inetHost, Promise> promise) { + if (isDisallowedAndFail(inetHost, promise)) { + return; + } + + final List addresses; + try { + addresses = Arrays.asList(SocketUtils.allAddressesByName(inetHost)); + } catch (UnknownHostException e) { + promise.setFailure(e); + return; + } + + // Even if _one_ of the addresses is disallowed, we fail the request. + for (InetAddress address : addresses) { + if (isDisallowedAndFail(address.getHostAddress(), promise)) { + return; + } + } + + promise.setSuccess(addresses); + } } } diff --git a/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java b/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java index 8cda9bd1840..8453494cce3 100644 --- a/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java +++ b/app/server/appsmith-plugins/graphqlPlugin/src/main/java/com/external/plugins/GraphQLPlugin.java @@ -27,7 +27,6 @@ import java.net.URI; import java.net.URISyntaxException; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -37,12 +36,12 @@ import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromPropertyList; import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInPropertyList; import static com.external.utils.GraphQLBodyUtils.PAGINATION_DATA_INDEX; -import static com.external.utils.GraphQLDataTypeUtils.smartlyReplaceGraphQLQueryBodyPlaceholderWithValue; -import static com.external.utils.GraphQLPaginationUtils.updateVariablesWithPaginationValues; import static com.external.utils.GraphQLBodyUtils.QUERY_VARIABLES_INDEX; import static com.external.utils.GraphQLBodyUtils.convertToGraphQLPOSTBodyFormat; import static com.external.utils.GraphQLBodyUtils.getGraphQLQueryParamsForBodyAndVariables; import static com.external.utils.GraphQLBodyUtils.validateBodyAndVariablesSyntax; +import static com.external.utils.GraphQLDataTypeUtils.smartlyReplaceGraphQLQueryBodyPlaceholderWithValue; +import static com.external.utils.GraphQLPaginationUtils.updateVariablesWithPaginationValues; import static java.lang.Boolean.TRUE; import static org.apache.commons.lang3.StringUtils.isBlank; @@ -186,18 +185,6 @@ public Mono executeCommon(APIConnection apiConnection, ActionExecutionRequest actionExecutionRequest = RequestCaptureFilter.populateRequestFields(actionConfiguration, uri, insertedParams, objectMapper); - try { - if (uriUtils.isHostDisallowed(uri)) { - errorResult.setBody(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR.getMessage("Host not allowed.")); - errorResult.setRequest(actionExecutionRequest); - return Mono.just(errorResult); - } - } catch (UnknownHostException e) { - errorResult.setBody(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR.getMessage("Unknown host.")); - errorResult.setRequest(actionExecutionRequest); - return Mono.just(errorResult); - } - WebClient.Builder webClientBuilder = triggerUtils.getWebClientBuilder(actionConfiguration, datasourceConfiguration); @@ -282,7 +269,7 @@ else if (HttpMethod.GET.equals(httpMethod)) { EXCHANGE_STRATEGIES, requestCaptureFilter); /* Triggering the actual REST API call */ - Set hintMessages = new HashSet(); + Set hintMessages = new HashSet<>(); return triggerUtils.triggerApiCall(client, httpMethod, uri, requestBodyObj, actionExecutionRequest, objectMapper, hintMessages, errorResult, requestCaptureFilter); diff --git a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java index ec1ae5b9102..86f9101cca0 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java @@ -5,6 +5,8 @@ import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.helpers.DataTypeStringUtils; import com.appsmith.external.helpers.MustacheHelper; +import com.appsmith.external.helpers.restApiUtils.connections.APIConnection; +import com.appsmith.external.helpers.restApiUtils.helpers.RequestCaptureFilter; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.ActionExecutionRequest; import com.appsmith.external.models.ActionExecutionResult; @@ -15,18 +17,16 @@ import com.appsmith.external.plugins.BasePlugin; import com.appsmith.external.plugins.BaseRestApiPluginExecutor; import com.appsmith.external.services.SharedConfig; -import com.appsmith.external.helpers.restApiUtils.connections.APIConnection; -import com.appsmith.external.helpers.restApiUtils.helpers.RequestCaptureFilter; import lombok.extern.slf4j.Slf4j; import org.pf4j.Extension; import org.pf4j.PluginWrapper; import org.springframework.http.HttpMethod; import org.springframework.web.reactive.function.client.WebClient; import reactor.core.publisher.Mono; + import java.net.URI; import java.net.URISyntaxException; import java.net.URLDecoder; -import java.net.UnknownHostException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashSet; @@ -125,7 +125,7 @@ public Mono executeCommon(APIConnection apiConnection, initUtils.initializeResponseWithError(errorResult); // Set of hint messages that can be returned to the user. - Set hintMessages = new HashSet(); + Set hintMessages = new HashSet<>(); // Initializing request URL String url = initUtils.initializeRequestUrl(actionConfiguration, datasourceConfiguration); @@ -148,18 +148,6 @@ public Mono executeCommon(APIConnection apiConnection, ActionExecutionRequest actionExecutionRequest = RequestCaptureFilter.populateRequestFields(actionConfiguration, uri, insertedParams, objectMapper); - try { - if (uriUtils.isHostDisallowed(uri)) { - errorResult.setBody(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR.getMessage("Host not allowed.")); - errorResult.setRequest(actionExecutionRequest); - return Mono.just(errorResult); - } - } catch (UnknownHostException e) { - errorResult.setBody(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR.getMessage("Unknown host.")); - errorResult.setRequest(actionExecutionRequest); - return Mono.just(errorResult); - } - WebClient.Builder webClientBuilder = triggerUtils.getWebClientBuilder(actionConfiguration, datasourceConfiguration); String reqContentType = headerUtils.getRequestContentType(actionConfiguration, datasourceConfiguration);