From 5a1ec0c591fea2e3de643563b8c89b26efc5077c Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Thu, 20 Jun 2024 11:00:22 +0530 Subject: [PATCH] chore: Disallow plugin requests to localhost (#34250) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The microservices that run inside the Appsmith container, trust each other, and may expose sensitive API endpoints to other internal microservices. These sensitive APIs aren't accessible by outside the Appsmith container, protected by Caddy's routing. This means that the backend server's ability to make user-configured HTTP requests, can lead to SSRFs to such sensitive API calls, if it's allowed to call APIs on localhost. In other words, Caddy establishes a trust boundary that protects these internal APIs from outside the container. But we lack such a trust boundary for the backend's plugins (API plugin, Elasticsearch plugin, etc.). This PR solves that. In this PR, we block both IPv4 and IPv6 loopback addresses. No additional changes needed on EE, no conflicts, and all unit and Cypress tests pass. **/test all** > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 5445c70aa873942c3edae9fbfcc57a6d2554b815 > Cypress dashboard. > Tags: `` ## Summary by CodeRabbit - **New Features** - Improved handling of disallowed hosts by dynamically computing based on environment variables, offering more flexibility and control. - **Refactor** - Enhanced the `makeWebClient()` method to use a more efficient approach for creating WebClient objects with custom configurations. - **Chores** - Added an `ENV` declaration for `IN_DOCKER` in the Dockerfile to better manage Docker-specific configurations. --- Dockerfile | 2 ++ .../com/appsmith/util/WebClientUtils.java | 17 ++++++++++++-- .../appsmith/server/helpers/RTSCaller.java | 22 ++++++++++++------- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8b72f70618..7d30696d4a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,8 @@ ARG BASE FROM ${BASE} +ENV IN_DOCKER=1 + # Add backend server - Application Layer ARG JAR_FILE=./app/server/dist/server-*.jar ARG PLUGIN_JARS=./app/server/dist/plugins/*.jar 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 3f7de88877..189b8e7d06 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 @@ -25,14 +25,15 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; @Slf4j public class WebClientUtils { - private static final Set DISALLOWED_HOSTS = - Set.of("169.254.169.254", "0:0:0:0:0:0:a9fe:a9fe", "fd00:ec2:0:0:0:0:0:254", "metadata.google.internal"); + private static final Set DISALLOWED_HOSTS = computeDisallowedHosts(); public static final String HOST_NOT_ALLOWED = "Host not allowed."; @@ -45,6 +46,18 @@ public class WebClientUtils { private WebClientUtils() {} + private static Set computeDisallowedHosts() { + final Set hosts = new HashSet<>(Set.of( + "169.254.169.254", "0:0:0:0:0:0:a9fe:a9fe", "fd00:ec2:0:0:0:0:0:254", "metadata.google.internal")); + + if ("1".equals(System.getenv("IN_DOCKER"))) { + hosts.add("127.0.0.1"); + hosts.add("0:0:0:0:0:0:0:1"); + } + + return Collections.unmodifiableSet(hosts); + } + public static WebClient create() { return builder().build(); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RTSCaller.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RTSCaller.java index 09b5959002..c5c2711584 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RTSCaller.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RTSCaller.java @@ -1,15 +1,16 @@ package com.appsmith.server.helpers; -import com.appsmith.util.WebClientUtils; import jakarta.annotation.PostConstruct; import lombok.NonNull; import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.http.client.reactive.ReactorClientHttpConnector; import org.springframework.stereotype.Component; import org.springframework.web.reactive.function.BodyInserters; import org.springframework.web.reactive.function.client.WebClient; import reactor.core.publisher.Mono; +import reactor.netty.http.client.HttpClient; import reactor.netty.resources.ConnectionProvider; import java.time.Duration; @@ -33,13 +34,18 @@ public class RTSCaller { rtsPort = "8091"; } - webClient = WebClientUtils.builder(ConnectionProvider.builder("rts-provider") - .maxConnections(100) - .maxIdleTime(Duration.ofSeconds(30)) - .maxLifeTime(Duration.ofSeconds(40)) - .pendingAcquireTimeout(Duration.ofSeconds(10)) - .pendingAcquireMaxCount(-1) - .build()) + final ConnectionProvider connectionProvider = ConnectionProvider.builder("rts-provider") + .maxConnections(100) + .maxIdleTime(Duration.ofSeconds(30)) + .maxLifeTime(Duration.ofSeconds(40)) + .pendingAcquireTimeout(Duration.ofSeconds(10)) + .pendingAcquireMaxCount(-1) + .build(); + + // We do NOT use `WebClientUtils` here, intentionally, since we don't allow connections to 127.0.0.1, + // which is exactly the _only_ host we want to hit from here. + webClient = WebClient.builder() + .clientConnector(new ReactorClientHttpConnector(HttpClient.create(connectionProvider))) .baseUrl("http://127.0.0.1:" + rtsPort) .build(); }