diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java new file mode 100644 index 000000000000..911d689537a7 --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/RestrictedHostFilter.java @@ -0,0 +1,606 @@ +package com.appsmith.util; + +import io.netty.util.concurrent.Promise; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.validator.routines.InetAddressValidator; +import org.springframework.util.StringUtils; + +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.URI; +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Locale; +import java.util.Optional; +import java.util.Set; + +/** + * Host-allowed/blocked filter for Appsmith's SSRF protections. Today used by: + * + * + *

Not applied to the JDBC plugins (Postgres, MySQL, MSSQL, Oracle, Redshift, Snowflake, + * Databricks), Mongo, ArangoDB, or the SMTP runtime send path. Several of those support SSH + * tunneling, where the user-entered host is resolved on the SSH server rather than in the + * Java process — a literal/IP check here would false-positive any datasource tunneling to + * loopback or an RFC1918 address on the far side of the tunnel. Closing those gaps needs + * per-plugin design, not a drop-in of {@link #isHostBlocked(String)}. + * + *

Two entry points for the two access patterns: {@link #isHostBlocked(String)} is + * DNS-aware and intended for plugin connection paths that run on a scheduler tolerating + * blocking I/O (e.g. Redis's {@code datasourceCreate}, wrapped in + * {@code Mono.fromCallable(...).subscribeOn(boundedElastic)}); {@link #isLiteralBlocked(String)} + * is literal-only, for pre-resolver fast paths where the Netty/driver resolver runs DNS + * separately, or for sync paths that can't afford a DNS round-trip. + */ +@Slf4j +public final class RestrictedHostFilter { + + public static final String HOST_NOT_ALLOWED = "Host not allowed."; + + /** + * Opt-out env var for the SSRF filter. Default is on (secure by default). Set + * {@code APPSMITH_DISABLE_SSRF_FILTER=true} in production only when the deployment + * intentionally needs to allow datasources / WebClient targets that point at loopback, + * link-local, or cloud-metadata addresses — e.g. local development against a service on + * {@code localhost}, or a single-tenant install where the operator accepts the risk. + * + *

The surefire test JVM also sets the {@code appsmith.test.bypass.ssrf} system property + * (see root pom) so that integration tests using MockWebServer / Testcontainers on loopback + * don't fight the filter. The two filter-specific test classes + * ({@code RestrictedHostFilterTest}, {@code WebClientUtilsTest}) flip the filter back on in + * {@code @BeforeAll} and use {@link #resetSsrfFilterDisabledForTesting()} in + * {@code @AfterAll} to restore the test-JVM-wide default. + * + *

When the filter is disabled, all of {@link #isHostBlocked(String)}, + * {@link #isLiteralBlocked(String)}, {@link #isDisallowedAndFail(String, Promise)}, and + * {@link #resolveIfAllowed(String)} skip their checks. The address-class predicates + * ({@link #isBlockedIpAddressClass(String)}, {@link #matchesBlockedAddressClass(InetAddress)}) + * still return facts about the address itself; callers that want to honor the kill-switch + * should consult the high-level "is this allowed" methods instead. + */ + private static final boolean INITIAL_SSRF_FILTER_DISABLED = computeSsrfFilterDisabled(); + + private static volatile boolean ssrfFilterDisabled = INITIAL_SSRF_FILTER_DISABLED; + + private static final InetAddressValidator inetAddressValidator = InetAddressValidator.getInstance(); + + private static final Set DISALLOWED_HOSTS = computeDisallowedHosts(); + + /** + * Hostnames of the internal Redis instances Appsmith runs for its own state — the session + * store ({@code APPSMITH_REDIS_URL}) and the Git Redis used by the workspace git + * import/sync flow ({@code APPSMITH_REDIS_GIT_URL}). Both are parsed at JVM start; the set + * is empty if neither env var is set or both are unparseable. The Git Redis URL falls back + * to the main URL via Spring property resolution, so in practice this set is usually a + * single entry — but we read both env vars directly here in case they're pointed at + * different hosts. + * + *

Used by {@link #isHostBlocked(String)} and {@link #isLiteralBlocked(String)} to + * prevent datasource plugins from being pointed at either internal Redis via direct + * hostname or via an IP that one of those hostnames currently resolves to. + * + *

Volatile + public setter (for tests only) instead of {@code final} so unit tests can + * exercise the overlap-detection logic without relying on the JVM's launch environment. + * Production code never mutates this. + */ + private static volatile Set internalRedisHosts = computeInternalRedisHosts(); + + /** + * Test-only override that lets specific hosts bypass {@link #isHostBlocked(String)}. Used by + * integration tests that spin up real services on loopback (e.g. Testcontainers-exposed + * Redis on localhost) where the production block on loopback would otherwise prevent the + * test from connecting. Empty in production runs; not mutated by production code. + */ + private static volatile Set alwaysAllowedHostsForTesting = Collections.emptySet(); + + private RestrictedHostFilter() {} + + private static Set computeDisallowedHosts() { + final Set hosts = new HashSet<>(); + addDisallowedHosts( + hosts, + "169.254.169.254", + "168.63.129.16", + "fd00:ec2::254", + "fd20:ce::254", + "100.100.100.200", + "169.254.10.10", + "169.254.170.2", + "metadata.google.internal", + "metadata.tencentyun.com"); + + // Loopback literals — always denied. matchesBlockedAddressClass catches them via + // isLoopbackAddress() on a resolved InetAddress, but isBlockedIpAddressClass operates + // on the canonical *string* form, and the IPv4-compat normalization in normalizeIpAddress + // turns "::1" into "0.0.0.1" (the embedded IPv4 bytes) which is no longer recognized as + // loopback. Seeding both literals here closes that gap: they get stored under their + // canonical forms, so any input that normalizes to the same form is blocked via this set. + // (Previously gated behind IN_DOCKER=1; now unconditional — secure for every deployment + // shape, with APPSMITH_DISABLE_SSRF_FILTER as the opt-out.) + addDisallowedHosts(hosts, "127.0.0.1", "::1"); + + return Collections.unmodifiableSet(hosts); + } + + private static boolean computeSsrfFilterDisabled() { + return "true".equalsIgnoreCase(System.getenv("APPSMITH_DISABLE_SSRF_FILTER")) + || "true".equalsIgnoreCase(System.getProperty("appsmith.test.bypass.ssrf")); + } + + /** Visible for testing only. Pairs with the {@code APPSMITH_DISABLE_SSRF_FILTER} env var. */ + public static void setSsrfFilterDisabledForTesting(boolean disabled) { + ssrfFilterDisabled = disabled; + } + + /** + * Restores the SSRF-filter state to whatever the JVM env / system properties produced at + * startup. Used by filter-testing classes in {@code @AfterAll} to undo a temporary enable + * without hardcoding the expected default. + */ + public static void resetSsrfFilterDisabledForTesting() { + ssrfFilterDisabled = INITIAL_SSRF_FILTER_DISABLED; + } + + private static void addDisallowedHosts(Set hosts, String... hostCandidates) { + for (String hostCandidate : hostCandidates) { + try { + hosts.add(normalizeHostForComparison(hostCandidate)); + } catch (UnknownHostException e) { + throw new IllegalStateException("Invalid disallowed host configured: " + hostCandidate, e); + } + } + } + + private static Set computeInternalRedisHosts() { + final Set hosts = new HashSet<>(); + addInternalRedisHostFromEnv(hosts, "APPSMITH_REDIS_URL"); + addInternalRedisHostFromEnv(hosts, "APPSMITH_REDIS_GIT_URL"); + return hosts.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(hosts); + } + + private static void addInternalRedisHostFromEnv(Set hosts, String envVar) { + addInternalRedisHostFromUrl(hosts, System.getenv(envVar), envVar); + } + + private static void addInternalRedisHostFromUrl(Set hosts, String url, String sourceLabel) { + if (!StringUtils.hasText(url)) { + return; + } + try { + final URI uri = URI.create(url.trim()); + final String host = uri.getHost(); + if (StringUtils.hasText(host)) { + // Normalize through the same canonicalization that isHostBlocked / + // isLiteralBlocked use for the compare side — strips IPv6 brackets, lowercases, + // collapses IPv4-compat / IPv4-mapped IPv6 to the embedded IPv4. Without this, + // a redis://[::1] env var stores "[::1]" but lookups canonicalize to "0.0.0.1" + // and silently miss. + hosts.add(normalizeHostForComparisonQuietly(host)); + } + } catch (IllegalArgumentException e) { + log.warn("Could not parse {} as a Redis URI; that internal Redis host won't be filtered.", sourceLabel); + } + } + + /** + * Registers internal Appsmith Redis hosts from their connection URLs as resolved by the + * application (Spring), not the process environment. The static initializer seeds + * {@link #internalRedisHosts} from the {@code APPSMITH_REDIS_URL} / {@code APPSMITH_REDIS_GIT_URL} + * environment variables, but the app actually binds {@code appsmith.redis.url} via Spring — which + * can also come from {@code application.properties}, a {@code -D} system property, or a profile. + * An operator who configures Redis through one of those (without the env var) would otherwise + * leave this set empty, and the internal-Redis block fails open (loopback / cloud-metadata + * literals are still blocked, but an in-cluster Redis on an RFC1918 address or service hostname + * would be reachable). The server calls this at startup with the resolved property values so the + * filter reflects the Redis the app truly uses, regardless of how it was configured. + * + *

Unions with — does not replace — the env-seeded set, so configuring Redis through both + * channels blocks both. Safe to call before/after the static seed; null/blank URLs are ignored. + */ + public static void registerInternalRedisHosts(String... redisUrls) { + if (redisUrls == null || redisUrls.length == 0) { + return; + } + final Set merged = new HashSet<>(internalRedisHosts); + for (String url : redisUrls) { + addInternalRedisHostFromUrl(merged, url, "configured Redis URL"); + } + internalRedisHosts = merged.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(merged); + } + + /** Visible for testing only. Production code never mutates the internal Redis hosts set. */ + public static void setInternalRedisHostsForTesting(String... hosts) { + if (hosts == null || hosts.length == 0) { + internalRedisHosts = Collections.emptySet(); + return; + } + final Set normalized = new HashSet<>(hosts.length); + for (String host : hosts) { + if (StringUtils.hasText(host)) { + // Same canonicalization as the compare side — see addInternalRedisHostFromEnv. + normalized.add(normalizeHostForComparisonQuietly(host)); + } + } + internalRedisHosts = normalized.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(normalized); + } + + /** + * Visible for testing only. Adds hosts that should be exempt from {@link #isHostBlocked(String)} + * for the duration of a test run. Tests that drive Testcontainers-exposed services on loopback + * should call this in {@code @BeforeAll} with the container's host string, then clear it with + * {@link #clearAlwaysAllowedHostsForTesting()} in {@code @AfterAll}. + */ + public static void setAlwaysAllowedHostsForTesting(String... hosts) { + if (hosts == null || hosts.length == 0) { + alwaysAllowedHostsForTesting = Collections.emptySet(); + return; + } + final Set normalized = new HashSet<>(hosts.length); + for (String host : hosts) { + if (StringUtils.hasText(host)) { + normalized.add(normalizeHostForComparisonQuietly(host)); + } + } + alwaysAllowedHostsForTesting = Collections.unmodifiableSet(normalized); + } + + /** Visible for testing only. Pairs with {@link #setAlwaysAllowedHostsForTesting(String...)}. */ + public static void clearAlwaysAllowedHostsForTesting() { + alwaysAllowedHostsForTesting = Collections.emptySet(); + } + + /** + * Resolves a hostname and validates that none of its addresses are disallowed for + * outbound connections from non-HTTP paths (e.g. SMTP via JavaMail). Checks against + * the cloud-metadata denylist, loopback, link-local, any-local, multicast, and IPv6 + * Unique Local Addresses (fc00::/7). Returns the first validated resolved address so + * callers can connect to it directly, preventing DNS-rebinding TOCTOU bypasses. + * + *

RFC 1918 site-local ranges (10/8, 172.16/12, 192.168/16) are intentionally + * allowed because legitimate SMTP servers frequently reside on private networks. + * + * @return the resolved {@link InetAddress} if the host is allowed, or empty if blocked + */ + public static Optional resolveIfAllowed(String host) { + if (!StringUtils.hasText(host)) { + return Optional.empty(); + } + + final InetAddress[] resolved; + try { + resolved = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + return Optional.empty(); + } + + if (ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + // Operator has opted out (or this is a test-allowlisted host) — return the resolved + // address but skip the deny-set / address-class checks. The empty-on-unresolvable + // contract above still holds. + return Optional.of(resolved[0]); + } + + final String canonicalHost = normalizeHostForComparisonQuietly(host); + if (DISALLOWED_HOSTS.contains(canonicalHost)) { + return Optional.empty(); + } + + for (InetAddress addr : resolved) { + if (DISALLOWED_HOSTS.contains(normalizeHostForComparisonQuietly(addr.getHostAddress())) + || matchesBlockedAddressClass(addr)) { + return Optional.empty(); + } + } + + return Optional.of(resolved[0]); + } + + private static boolean isExplicitlyAllowedForTesting(String host) { + if (alwaysAllowedHostsForTesting.isEmpty() || !StringUtils.hasText(host)) { + return false; + } + return alwaysAllowedHostsForTesting.contains(normalizeHostForComparisonQuietly(host)); + } + + /** + * Returns {@code true} if {@code host} is definitively in the disallowed set: either the + * literal/canonical host string is on the static denylist, the literal is a non-routable + * address class (loopback, any-local, link-local, multicast, IPv6 ULA), the host matches + * any configured internal Redis hostname (session store + git Redis), or DNS resolves it + * to at least one address that intersects with the denylist, a blocked address class, or + * the IPs that any configured internal Redis hostname currently resolves to. + * + *

Returns {@code false} for an unresolvable hostname so a transient DNS failure + * doesn't reject a legitimate but temporarily unreachable host. The driver's own + * connection-time error will surface naturally in that case. + * + *

Currently called from the Redis plugin's {@code datasourceCreate}, which is wrapped + * in {@code Mono.fromCallable(...).subscribeOn(boundedElastic)} so the blocking DNS call + * is safe. The other deny-list-aware paths use sibling methods: + * {@link #isLiteralBlocked(String)} for the WebClient request-filter fast path, + * {@link #isDisallowedAndFail(String, Promise)} for the Netty / Apache HttpAsyncClient + * resolver hooks, and {@link #resolveIfAllowed(String)} for the SMTP test-email path. + */ + public static boolean isHostBlocked(String host) { + if (!StringUtils.hasText(host) || ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return false; + } + + final String canonicalHost = normalizeHostForComparisonQuietly(host); + + if (isCanonicalHostBlocked(canonicalHost)) { + return true; + } + + final InetAddress[] userAddresses; + try { + userAddresses = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + // Host can't be resolved right now — don't block at save time. A truly bad host will + // fail later at connection time with a clearer driver-specific error. + return false; + } + + return isAnyResolvedAddressBlocked(userAddresses); + } + + /** + * Shared deny-logic (1 of 2): is the canonical host string itself blocked, without any DNS? + * Covers the static denylist (cloud-metadata literals, loopback), a non-routable IP-class + * literal, and a literal match against an internal Appsmith Redis hostname. Extracted so + * {@link #isHostBlocked(String)} and {@link #firstAllowedRedisAddress(String, InetAddress[])} + * evaluate the deny list identically and cannot drift into an SSRF gap. + */ + private static boolean isCanonicalHostBlocked(String canonicalHost) { + return DISALLOWED_HOSTS.contains(canonicalHost) + || isBlockedIpAddressClass(canonicalHost) + || internalRedisHosts.contains(canonicalHost); + } + + /** + * Shared deny-logic (2 of 2): does any already-resolved address fall in the denylist, a + * blocked address class, or overlap with the IPs an internal Appsmith Redis hostname currently + * resolves to? Extracted alongside {@link #isCanonicalHostBlocked(String)} so both callers + * share one address-level deny evaluation. + */ + private static boolean isAnyResolvedAddressBlocked(InetAddress[] resolvedAddresses) { + final Set internalRedisIps = resolveInternalRedisIps(internalRedisHosts); + for (InetAddress addr : resolvedAddresses) { + final String addrString = normalizeHostForComparisonQuietly(addr.getHostAddress()); + if (DISALLOWED_HOSTS.contains(addrString) + || matchesBlockedAddressClass(addr) + || internalRedisIps.contains(addrString)) { + return true; + } + } + return false; + } + + private static Set resolveInternalRedisIps(Set redisHosts) { + if (redisHosts.isEmpty()) { + return Collections.emptySet(); + } + final Set ips = new HashSet<>(); + for (String redisHost : redisHosts) { + try { + for (InetAddress addr : InetAddress.getAllByName(redisHost)) { + ips.add(normalizeHostForComparisonQuietly(addr.getHostAddress())); + } + } catch (UnknownHostException e) { + // Hostname doesn't currently resolve — skip it in the overlap check; literal + // match still applies for the same hostname string. + log.debug("Internal Redis hostname {} could not be resolved; skipping in IP overlap check.", redisHost); + } + } + return ips; + } + + /** + * Connection-time policy check for the Redis plugin's socket factory. Given a host and the + * addresses it has ALREADY been resolved to (by the caller, in a single DNS lookup), returns + * the first address that is safe to connect to, or empty if any resolved address is blocked — + * static denylist (cloud-metadata literals), non-routable address class (loopback, any-local, + * link-local, multicast, IPv6 ULA), a literal match against an internal Appsmith Redis + * hostname, or overlap with the IPs an internal Redis hostname currently resolves to. + * + *

This is the half of the fix that closes the DNS-rebinding TOCTOU. {@link + * #isHostBlocked(String)} resolves at datasource-create time, but Jedis would resolve the + * hostname again when it opens the socket. {@code RestrictedHostJedisSocketFactory} (in the + * redisPlugin module) resolves exactly once at connect time and passes those addresses here, + * then connects directly to the returned {@link InetAddress} — so the check and the connect + * share one resolution and the driver never re-resolves the hostname. + * + *

The caller owns DNS resolution deliberately: passing addresses in (rather than resolving + * here) lets the caller distinguish an unresolvable host (a genuine connection error) from a + * policy block (this method returning empty), and guarantees the validated address is the one + * connected to. Honors the {@code APPSMITH_DISABLE_SSRF_FILTER} kill-switch and the test + * allowlist, in which case the first resolved address is returned without policy checks. + */ + public static Optional firstAllowedRedisAddress(String host, InetAddress[] resolvedAddresses) { + if (resolvedAddresses == null || resolvedAddresses.length == 0) { + return Optional.empty(); + } + + if (ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return Optional.of(resolvedAddresses[0]); + } + + final String canonicalHost = normalizeHostForComparisonQuietly(host); + if (isCanonicalHostBlocked(canonicalHost) || isAnyResolvedAddressBlocked(resolvedAddresses)) { + return Optional.empty(); + } + + return Optional.of(resolvedAddresses[0]); + } + + /** + * Best-effort, log-only description of the addresses {@code host} currently resolves to, for + * annotating SSRF block log lines. Does its own lookup and is NOT a security decision — it only + * explains a block, e.g. distinguishing a host typed as an internal IP literally from a hostname + * that resolved to loopback / cloud-metadata. Returns {@code "unresolved"} when it can't be + * resolved (the block may have fired on a literal/denylist hostname). Because it re-resolves, a + * hostile flipping resolver could in theory report a different address than the one that + * triggered the block; the annotation is point-in-time and diagnostic only. + */ + public static String describeResolvedAddresses(String host) { + try { + return String.join( + ", ", + Arrays.stream(InetAddress.getAllByName(host)) + .map(InetAddress::getHostAddress) + .toList()); + } catch (UnknownHostException e) { + return "unresolved"; + } + } + + /** + * Literal/canonical-only block check — no DNS, no network I/O. Catches the static denylist + * (cloud-metadata IPs), literal non-routable IP-class addresses, and a literal match + * against any configured internal Redis hostname (session store + git Redis). + * + *

Returns {@code false} for unparseable input and for anything that requires DNS to + * decide. Callers that need the DNS-resolved check (IP overlap with the internal Redis, + * hostname-resolves-to-loopback, etc.) should use {@link #isHostBlocked(String)} from an + * async path that tolerates blocking I/O. + * + *

Used by {@link WebClientUtils} request filter as the pre-resolver fast path — the + * Netty resolver runs the DNS-aware check separately via + * {@link #isDisallowedAndFail(String, Promise)}. + */ + public static boolean isLiteralBlocked(String host) { + if (!StringUtils.hasText(host) || ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return false; + } + final String canonicalHost; + try { + canonicalHost = normalizeHostForComparison(host); + } catch (UnknownHostException e) { + return false; + } + if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { + return true; + } + return internalRedisHosts.contains(canonicalHost); + } + + public static boolean isDisallowedAndFail(String host, Promise promise) { + if (ssrfFilterDisabled || isExplicitlyAllowedForTesting(host)) { + return false; + } + final String canonicalHost = normalizeHostForComparisonQuietly(host); + if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedIpAddressClass(canonicalHost)) { + log.warn("Host {} is disallowed. Failing the request.", host); + if (promise != null) { + promise.setFailure(new UnknownHostException(HOST_NOT_ALLOWED)); + } + return true; + } + return false; + } + + public static boolean isBlockedIpAddressClass(String canonicalHost) { + if (!isValidIpAddress(canonicalHost)) { + return false; + } + try { + return matchesBlockedAddressClass(InetAddress.getByName(canonicalHost)); + } catch (UnknownHostException e) { + return false; + } + } + + public static boolean matchesBlockedAddressClass(InetAddress address) { + if (address.isLoopbackAddress() + || address.isAnyLocalAddress() + || address.isLinkLocalAddress() + || address.isMulticastAddress()) { + return true; + } + if (address instanceof Inet6Address) { + // fc00::/7 — IPv6 Unique Local Addresses + byte firstByte = address.getAddress()[0]; + return (firstByte & (byte) 0xFE) == (byte) 0xFC; + } + return false; + } + + private static boolean isValidIpAddress(String host) { + if (!StringUtils.hasText(host)) { + return false; + } + host = stripHostDecorators(host); + return inetAddressValidator.isValid(host); + } + + static String normalizeHostForComparison(String host) throws UnknownHostException { + if (!StringUtils.hasText(host)) { + return host; + } + + final String normalizedHost = stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)); + return isValidIpAddress(normalizedHost) ? normalizeIpAddress(normalizedHost) : normalizedHost; + } + + private static String normalizeHostForComparisonQuietly(String host) { + try { + return normalizeHostForComparison(host); + } catch (UnknownHostException e) { + return StringUtils.hasText(host) ? stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)) : host; + } + } + + private static String stripHostDecorators(String host) { + String sanitizedHost = host; + while (sanitizedHost.endsWith(".")) { + sanitizedHost = sanitizedHost.substring(0, sanitizedHost.length() - 1); + } + if (sanitizedHost.startsWith("[") && sanitizedHost.endsWith("]")) { + sanitizedHost = sanitizedHost.substring(1, sanitizedHost.length() - 1); + } + return sanitizedHost; + } + + private static String normalizeIpAddress(String host) throws UnknownHostException { + final InetAddress address = InetAddress.getByName(host); + + if (address instanceof Inet6Address) { + final byte[] addressBytes = address.getAddress(); + // Normalize IPv4-compatible and IPv4-mapped IPv6 literals back to the embedded IPv4 address so a single + // denylist entry blocks equivalent literal representations such as `100.100.100.200` and + // `[::100.100.100.200]`. + if (isIpv4CompatibleOrMapped(addressBytes)) { + return InetAddress.getByAddress(Arrays.copyOfRange(addressBytes, 12, 16)) + .getHostAddress(); + } + } + + return address.getHostAddress(); + } + + private static boolean isIpv4CompatibleOrMapped(byte[] addressBytes) { + if (addressBytes.length != 16) { + return false; + } + + for (int i = 0; i < 10; i++) { + if (addressBytes[i] != 0) { + return false; + } + } + + return (addressBytes[10] == 0 && addressBytes[11] == 0) + || (addressBytes[10] == (byte) 0xff && addressBytes[11] == (byte) 0xff); + } +} 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 196f5dac5b5b..d529392ee90b 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 @@ -10,7 +10,6 @@ import io.netty.util.concurrent.Promise; import io.netty.util.internal.SocketUtils; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.validator.routines.InetAddressValidator; import org.springframework.http.client.reactive.ReactorClientHttpConnector; import org.springframework.util.StringUtils; import org.springframework.web.reactive.function.client.ClientRequest; @@ -21,28 +20,32 @@ import reactor.netty.http.client.HttpClient; import reactor.netty.resources.ConnectionProvider; -import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.time.Duration; import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Locale; -import java.util.Optional; -import java.util.Set; +/** + * Factory for {@link WebClient} instances pre-wired with the SSRF host filter + * ({@link RestrictedHostFilter}). The filter is enforced at two layers: + * + *

    + *
  1. A request-stage {@link ExchangeFilterFunction} ({@link #IP_CHECK_FILTER}) that + * rejects literal/canonical hosts on the deny set before DNS even runs.
    2. + *
  2. A custom Netty {@link AddressResolver} ({@link ResolverGroup}) that runs DNS + * itself and rejects when any resolved address lands on the deny set or matches a + * non-routable address class.
    3. + *
+ * + *

Host-filter logic lives in {@link RestrictedHostFilter} so non-HTTP plugins (Redis, + * SMTP, the Elasticsearch HttpAsyncClient hook, etc.) can call into the same denylist + * without depending on Spring/Netty. + */ @Slf4j public class WebClientUtils { - private static final InetAddressValidator inetAddressValidator = InetAddressValidator.getInstance(); - - private static final Set DISALLOWED_HOSTS = computeDisallowedHosts(); - - public static final String HOST_NOT_ALLOWED = "Host not allowed."; - private static final int MAX_IN_MEMORY_SIZE_IN_BYTES = 16 * 1024 * 1024; public static final ExchangeFilterFunction IP_CHECK_FILTER = @@ -66,37 +69,6 @@ public class WebClientUtils { private WebClientUtils() {} - private static Set computeDisallowedHosts() { - final Set hosts = new HashSet<>(); - addDisallowedHosts( - hosts, - "169.254.169.254", - "168.63.129.16", - "fd00:ec2::254", - "fd20:ce::254", - "100.100.100.200", - "169.254.10.10", - "169.254.170.2", - "metadata.google.internal", - "metadata.tencentyun.com"); - - if ("1".equals(System.getenv("IN_DOCKER"))) { - addDisallowedHosts(hosts, "127.0.0.1", "::1"); - } - - return Collections.unmodifiableSet(hosts); - } - - private static void addDisallowedHosts(Set hosts, String... hostCandidates) { - for (String hostCandidate : hostCandidates) { - try { - hosts.add(normalizeHostForComparison(hostCandidate)); - } catch (UnknownHostException e) { - throw new IllegalStateException("Invalid disallowed host configured: " + hostCandidate, e); - } - } - } - public static WebClient create() { return builder().build(); } @@ -199,58 +171,6 @@ protected AddressResolver newResolver(EventExecutor executor) } } - /** - * Resolves a hostname and validates that none of its addresses are disallowed for - * outbound connections from non-HTTP paths (e.g. SMTP via JavaMail). Checks against - * the cloud-metadata denylist, loopback, link-local, any-local, multicast, and IPv6 - * Unique Local Addresses (fc00::/7). Returns the first validated resolved address so - * callers can connect to it directly, preventing DNS-rebinding TOCTOU bypasses. - * - *

RFC 1918 site-local ranges (10/8, 172.16/12, 192.168/16) are intentionally - * allowed because legitimate SMTP servers frequently reside on private networks. - * - * @return the resolved {@link InetAddress} if the host is allowed, or empty if blocked - */ - public static Optional resolveIfAllowed(String host) { - if (!StringUtils.hasText(host)) { - return Optional.empty(); - } - - final String canonicalHost = normalizeHostForComparisonQuietly(host); - - if (DISALLOWED_HOSTS.contains(canonicalHost)) { - return Optional.empty(); - } - - final InetAddress[] resolved; - try { - resolved = InetAddress.getAllByName(host); - } catch (UnknownHostException e) { - return Optional.empty(); - } - - for (InetAddress addr : resolved) { - if (DISALLOWED_HOSTS.contains(normalizeHostForComparisonQuietly(addr.getHostAddress())) - || matchesBlockedAddressClass(addr)) { - return Optional.empty(); - } - } - - return Optional.of(resolved[0]); - } - - public static boolean isDisallowedAndFail(String host, Promise promise) { - final String canonicalHost = normalizeHostForComparisonQuietly(host); - if (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedAddressClassInDocker(canonicalHost)) { - log.warn("Host {} is disallowed. Failing the request.", host); - if (promise != null) { - promise.setFailure(new UnknownHostException(HOST_NOT_ALLOWED)); - } - return true; - } - return false; - } - private static Mono requestFilterFn(ClientRequest request) { final String host = request.url().getHost(); @@ -259,119 +179,11 @@ private static Mono requestFilterFn(ClientRequest request) { AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, "Requested url host is null or empty")); } - final String canonicalHost; - try { - canonicalHost = normalizeHostForComparison(host); - } catch (UnknownHostException e) { - // This exception is thrown, if the given host couldn't be resolved to an IP address. But, since we only - // canonicalize after ensuring that `host` is a valid IP address, this exception should never occur. - return Mono.error(new AppsmithPluginException( - AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, "IP Address resolution is invalid")); - } - - return (DISALLOWED_HOSTS.contains(canonicalHost) || isBlockedAddressClassInDocker(canonicalHost)) - ? Mono.error(new UnknownHostException(HOST_NOT_ALLOWED)) + return RestrictedHostFilter.isLiteralBlocked(host) + ? Mono.error(new UnknownHostException(RestrictedHostFilter.HOST_NOT_ALLOWED)) : Mono.just(request); } - static boolean isBlockedIpAddressClass(String canonicalHost) { - if (!isValidIpAddress(canonicalHost)) { - return false; - } - try { - return matchesBlockedAddressClass(InetAddress.getByName(canonicalHost)); - } catch (UnknownHostException e) { - return false; - } - } - - private static boolean matchesBlockedAddressClass(InetAddress address) { - if (address.isLoopbackAddress() - || address.isAnyLocalAddress() - || address.isLinkLocalAddress() - || address.isMulticastAddress()) { - return true; - } - if (address instanceof Inet6Address) { - // fc00::/7 — IPv6 Unique Local Addresses - byte firstByte = address.getAddress()[0]; - return (firstByte & (byte) 0xFE) == (byte) 0xFC; - } - return false; - } - - private static boolean isBlockedAddressClassInDocker(String canonicalHost) { - return "1".equals(System.getenv("IN_DOCKER")) && isBlockedIpAddressClass(canonicalHost); - } - - private static boolean isValidIpAddress(String host) { - if (!StringUtils.hasText(host)) { - return false; - } - host = stripHostDecorators(host); - return inetAddressValidator.isValid(host); - } - - private static String normalizeHostForComparison(String host) throws UnknownHostException { - if (!StringUtils.hasText(host)) { - return host; - } - - final String normalizedHost = stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)); - return isValidIpAddress(normalizedHost) ? normalizeIpAddress(normalizedHost) : normalizedHost; - } - - private static String normalizeHostForComparisonQuietly(String host) { - try { - return normalizeHostForComparison(host); - } catch (UnknownHostException e) { - return StringUtils.hasText(host) ? stripHostDecorators(host.trim().toLowerCase(Locale.ROOT)) : host; - } - } - - private static String stripHostDecorators(String host) { - String sanitizedHost = host; - while (sanitizedHost.endsWith(".")) { - sanitizedHost = sanitizedHost.substring(0, sanitizedHost.length() - 1); - } - if (sanitizedHost.startsWith("[") && sanitizedHost.endsWith("]")) { - sanitizedHost = sanitizedHost.substring(1, sanitizedHost.length() - 1); - } - return sanitizedHost; - } - - private static String normalizeIpAddress(String host) throws UnknownHostException { - final InetAddress address = InetAddress.getByName(host); - - if (address instanceof Inet6Address) { - final byte[] addressBytes = address.getAddress(); - // Normalize IPv4-compatible and IPv4-mapped IPv6 literals back to the embedded IPv4 address so a single - // denylist entry blocks equivalent literal representations such as `100.100.100.200` and - // `[::100.100.100.200]`. - if (isIpv4CompatibleOrMapped(addressBytes)) { - return InetAddress.getByAddress(Arrays.copyOfRange(addressBytes, 12, 16)) - .getHostAddress(); - } - } - - return address.getHostAddress(); - } - - private static boolean isIpv4CompatibleOrMapped(byte[] addressBytes) { - if (addressBytes.length != 16) { - return false; - } - - for (int i = 0; i < 10; i++) { - if (addressBytes[i] != 0) { - return false; - } - } - - return (addressBytes[10] == 0 && addressBytes[11] == 0) - || (addressBytes[10] == (byte) 0xff && addressBytes[11] == (byte) 0xff); - } - private static class NameResolver extends InetNameResolver { public NameResolver(EventExecutor executor) { @@ -380,7 +192,7 @@ public NameResolver(EventExecutor executor) { @Override protected void doResolve(String inetHost, Promise promise) { - if (isDisallowedAndFail(inetHost, promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(inetHost, promise)) { return; } @@ -392,7 +204,7 @@ protected void doResolve(String inetHost, Promise promise) { return; } - if (isDisallowedAndFail(address.getHostAddress(), promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(address.getHostAddress(), promise)) { return; } @@ -401,7 +213,7 @@ protected void doResolve(String inetHost, Promise promise) { @Override protected void doResolveAll(String inetHost, Promise> promise) { - if (isDisallowedAndFail(inetHost, promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(inetHost, promise)) { return; } @@ -415,7 +227,7 @@ protected void doResolveAll(String inetHost, Promise> promise) // Even if _one_ of the addresses is disallowed, we fail the request. for (InetAddress address : addresses) { - if (isDisallowedAndFail(address.getHostAddress(), promise)) { + if (RestrictedHostFilter.isDisallowedAndFail(address.getHostAddress(), promise)) { return; } } diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java index d9793b99da56..422a8c581216 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/connections/OAuth2ClientCredentialsTest.java @@ -39,6 +39,9 @@ public class OAuth2ClientCredentialsTest { public static void setUp() throws IOException { mockEndpoint = new MockWebServer(); mockEndpoint.start(); + // The SSRF filter is JVM-wide disabled for all surefire tests (see root pom), so this + // MockWebServer-on-loopback test works without further setup. The filter is exercised + // separately in RestrictedHostFilterTest / WebClientUtilsTest. } @AfterAll diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java new file mode 100644 index 000000000000..5aed92284d25 --- /dev/null +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/RestrictedHostFilterTest.java @@ -0,0 +1,404 @@ +package com.appsmith.util; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.net.InetAddress; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class RestrictedHostFilterTest { + + @BeforeAll + public static void enableFilterForThisClass() { + // Surefire defaults the test JVM to bypass=true (see root pom). The filter-behavior + // tests in this class need the filter actually ON to verify their assertions, so flip + // the kill-switch back. resetSsrfFilterDisabledForTesting() in @AfterAll restores + // whatever the surefire-set default was. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + } + + @AfterAll + public static void restoreSurefireDefault() { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + + @AfterEach + public void clearTestOverrides() { + // Restore the internal Redis host filter and the always-allowed override to whatever + // the JVM env produced at startup, so tests don't leak state into each other. The + // disable-knob is also reset to the per-class state set in @BeforeAll above (filter on) + // — individual tests that toggle it (e.g. the kill-switch tests) still need to flip it + // back themselves between cases. + RestrictedHostFilter.setInternalRedisHostsForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + } + + @ParameterizedTest + @ValueSource(strings = {"metadata.tencentyun.com", "Metadata.TencentYun.Com.", "METAdata.google.internal."}) + public void testIsDisallowedAndFailNormalizesMetadataHostnames(String host) { + assertTrue(RestrictedHostFilter.isDisallowedAndFail(host, null)); + } + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "::1", "169.254.0.1", "fe80::1", "0.0.0.0", "fc00::1"}) + public void isDisallowedAndFail_blocksNonRoutableLiteralsUnconditionally(String host) { + // Previously gated behind IN_DOCKER=1; now blocked everywhere (secure-by-default). + assertTrue( + RestrictedHostFilter.isDisallowedAndFail(host, null), + "Expected " + host + " to be blocked by isDisallowedAndFail without IN_DOCKER"); + } + + // ---------- resolveIfAllowed: used by the SMTP test-email path (EnvManagerCEImpl) ---------- + + @ParameterizedTest + @ValueSource( + strings = { + "127.0.0.1", + "169.254.169.254", + "169.254.10.10", + "100.100.100.200", + "168.63.129.16", + "0.0.0.0", + }) + public void resolveIfAllowed_blocksLoopbackMetadataAndSpecialHosts(String host) { + Optional result = RestrictedHostFilter.resolveIfAllowed(host); + assertTrue(result.isEmpty(), "Expected host " + host + " to be blocked"); + } + + @Test + public void resolveIfAllowed_blocksNullAndEmpty() { + assertTrue(RestrictedHostFilter.resolveIfAllowed(null).isEmpty()); + assertTrue(RestrictedHostFilter.resolveIfAllowed("").isEmpty()); + assertTrue(RestrictedHostFilter.resolveIfAllowed(" ").isEmpty()); + } + + @Test + public void resolveIfAllowed_blocksLocalhostHostname() { + Optional result = RestrictedHostFilter.resolveIfAllowed("localhost"); + assertTrue(result.isEmpty(), "Expected 'localhost' to be blocked"); + } + + @ParameterizedTest + @ValueSource(strings = {"smtp.gmail.com", "email-smtp.us-east-1.amazonaws.com", "smtp.sendgrid.net"}) + public void resolveIfAllowed_allowsLegitimateSmtpHosts(String host) { + Optional result = RestrictedHostFilter.resolveIfAllowed(host); + assertTrue(result.isPresent(), "Expected host " + host + " to be allowed"); + } + + @Test + public void resolveIfAllowed_blocksUnresolvableHost() { + Optional result = + RestrictedHostFilter.resolveIfAllowed("definitely-not-a-real-host-xyz123.invalid"); + assertTrue(result.isEmpty(), "Expected unresolvable host to be blocked"); + } + + @Test + public void resolveIfAllowed_returnsResolvedAddress() { + // Use a literal IP rather than a public hostname — avoids a DNS lookup in CI and + // dodges IPv6-preferred resolvers that would have failed the old IPv4-only regex + // assertion on the resolved address. + Optional result = RestrictedHostFilter.resolveIfAllowed("1.1.1.1"); + assertTrue(result.isPresent()); + assertEquals("1.1.1.1", result.get().getHostAddress()); + } + + // ---------- isBlockedIpAddressClass: literal-only IP-class check ---------- + + @ParameterizedTest + @ValueSource( + strings = { + // Loopback + "127.0.0.1", + "127.0.0.2", + "127.0.0.254", + "127.1.2.3", + "127.255.255.255", + "::1", + // Any-local + "0.0.0.0", + "::", + // Link-local + "169.254.0.1", + "169.254.169.254", + "fe80::1", + // Multicast + "224.0.0.1", + "239.255.255.250", + "ff02::1", + // IPv6 ULA (fc00::/7) + "fc00::1", + "fd00::1", + "fdff::ffff", + }) + public void isBlockedIpAddressClass_recognizesNonRoutableClasses(String host) { + assertTrue( + RestrictedHostFilter.isBlockedIpAddressClass(host), + "Expected " + host + " to be recognized as a blocked address class"); + } + + @ParameterizedTest + @ValueSource( + strings = { + "1.1.1.1", + "8.8.8.8", + // RFC 1918 site-local — intentionally allowed for internal REST API targets + "192.168.1.1", + "10.0.0.1", + "172.16.0.1", + // Non-literals + "smtp.gmail.com", + "localhost", + }) + public void isBlockedIpAddressClass_doesNotMatchOtherHosts(String host) { + assertFalse( + RestrictedHostFilter.isBlockedIpAddressClass(host), + "Did not expect " + host + " to be recognized as a blocked address class"); + } + + // ---------- isHostBlocked: used by the Redis plugin (GHSA-qhfj-g87x-m39w) ---------- + + @Test + public void isHostBlocked_returnsFalseForNullOrEmpty() { + assertFalse(RestrictedHostFilter.isHostBlocked(null)); + assertFalse(RestrictedHostFilter.isHostBlocked("")); + assertFalse(RestrictedHostFilter.isHostBlocked(" ")); + } + + @ParameterizedTest + @ValueSource( + strings = { + // Static metadata-endpoint denylist literals + "169.254.169.254", + "100.100.100.200", + "168.63.129.16", + "metadata.google.internal", + "metadata.tencentyun.com", + "METAdata.google.internal.", + // Non-routable address classes — blocked unconditionally for Redis (unlike the + // IN_DOCKER-gated HTTP filter), since the GHSA targets loopback in any + // deployment that doesn't sit behind Docker. + "127.0.0.1", + "127.0.0.42", + "0.0.0.0", + "169.254.10.10", + "::1", + "fe80::1", + "fc00::1", + "fdff::ffff", + "224.0.0.1", + }) + public void isHostBlocked_blocksDenylistAndNonRoutableLiterals(String host) { + assertTrue(RestrictedHostFilter.isHostBlocked(host), "Expected " + host + " to be blocked"); + } + + @Test + public void isHostBlocked_blocksLocalhostHostname() { + // "localhost" resolves to 127.0.0.1 / ::1 — caught via the resolved-address loopback check. + assertTrue(RestrictedHostFilter.isHostBlocked("localhost")); + } + + @Test + public void isHostBlocked_returnsFalseForUnresolvableHost() { + // Key difference vs. resolveIfAllowed(): an unresolvable host is NOT blocked so that a + // transient DNS failure at config-save time doesn't reject an otherwise legitimate + // datasource. The driver will surface the real connection error later. + assertFalse(RestrictedHostFilter.isHostBlocked("definitely-not-a-real-host-xyz123.invalid")); + } + + @ParameterizedTest + @ValueSource(strings = {"smtp.gmail.com", "email-smtp.us-east-1.amazonaws.com", "1.1.1.1"}) + public void isHostBlocked_allowsLegitimateHosts(String host) { + assertFalse(RestrictedHostFilter.isHostBlocked(host), "Did not expect " + host + " to be blocked"); + } + + @Test + public void isHostBlocked_matchesConfiguredInternalRedisHostnameLiterally() { + RestrictedHostFilter.setInternalRedisHostsForTesting("internal-redis.svc.cluster.local"); + assertTrue(RestrictedHostFilter.isHostBlocked("internal-redis.svc.cluster.local")); + // Case-insensitive — datasource configs are user-entered. + assertTrue(RestrictedHostFilter.isHostBlocked("INTERNAL-Redis.svc.cluster.local")); + } + + @Test + public void isHostBlocked_blocksWhenUserHostResolvesToSameIpsAsInternalRedis() { + // Same hostname for both sides — guarantees IP overlap on whatever the test environment + // resolves it to. Validates the dynamic-resolution overlap path. Uses a stable public + // domain rather than depending on the env-resolved internal Redis hostname. + RestrictedHostFilter.setInternalRedisHostsForTesting("one.one.one.one"); + assertTrue(RestrictedHostFilter.isHostBlocked("one.one.one.one")); + } + + @Test + public void isHostBlocked_doesNotBlockUnrelatedHostWhenInternalRedisConfigured() { + RestrictedHostFilter.setInternalRedisHostsForTesting("internal-redis.svc.cluster.local"); + assertFalse(RestrictedHostFilter.isHostBlocked("smtp.gmail.com")); + } + + @Test + public void isHostBlocked_blocksAllConfiguredInternalRedisHosts() { + // Production reads APPSMITH_REDIS_URL (session store) and APPSMITH_REDIS_GIT_URL (git + // Redis used by the workspace git import/sync flow). When they're set to different + // hosts, both must be in the deny set. + RestrictedHostFilter.setInternalRedisHostsForTesting( + "session-redis.svc.cluster.local", "git-redis.svc.cluster.local"); + assertTrue(RestrictedHostFilter.isHostBlocked("session-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isHostBlocked("git-redis.svc.cluster.local")); + // Unrelated host stays allowed. + assertFalse(RestrictedHostFilter.isHostBlocked("redis.example.com")); + } + + @Test + public void registerInternalRedisHosts_blocksHostFromSpringResolvedUrl() { + // Mirrors the server's startup path (RedisConfig#registerInternalRedisHostsWithSsrfFilter): + // the app binds appsmith.redis.url via Spring, which the env-only static seed misses when + // Redis is configured via application.properties / -D. Hosts registered from the resolved + // URLs must be blocked, closing the fail-open gap. See GHSA-qhfj-g87x-m39w. + RestrictedHostFilter.setInternalRedisHostsForTesting(); // start empty (no env var set) + RestrictedHostFilter.registerInternalRedisHosts( + "redis://session-redis.svc.cluster.local:6379", "rediss://git-redis.svc.cluster.local:6380"); + assertTrue(RestrictedHostFilter.isHostBlocked("session-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isHostBlocked("git-redis.svc.cluster.local")); + // Null / blank URLs are ignored and unrelated hosts stay allowed. + RestrictedHostFilter.registerInternalRedisHosts((String) null, "", " "); + assertFalse(RestrictedHostFilter.isHostBlocked("redis.example.com")); + } + + @Test + public void registerInternalRedisHosts_unionsWithExistingHosts() { + // Registration must not clobber the env-seeded set: configuring Redis through both the env + // var and a property must block both. + RestrictedHostFilter.setInternalRedisHostsForTesting("env-seeded-redis.svc.cluster.local"); + RestrictedHostFilter.registerInternalRedisHosts("redis://property-redis.svc.cluster.local:6379"); + assertTrue(RestrictedHostFilter.isHostBlocked("env-seeded-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isHostBlocked("property-redis.svc.cluster.local")); + } + + @Test + public void describeResolvedAddresses_reportsResolvedIpOrUnresolved() { + // Log-only annotation for the SSRF block lines. An IP literal resolves to itself; the + // reserved .invalid TLD never resolves (RFC 2606) so it reports "unresolved". + assertEquals("127.0.0.1", RestrictedHostFilter.describeResolvedAddresses("127.0.0.1")); + assertEquals("unresolved", RestrictedHostFilter.describeResolvedAddresses("definitely-not-a-host.invalid")); + } + + // ---------- alwaysAllowedHostsForTesting: opt-in test escape hatch ---------- + + @Test + public void alwaysAllowedHostsForTesting_lets_an_otherwise_blocked_host_through() { + // Sanity: 127.0.0.1 is normally blocked. + assertTrue(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + // Allow it explicitly. + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1"); + assertFalse(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + // Other blocked hosts are still blocked. + assertTrue(RestrictedHostFilter.isHostBlocked("169.254.169.254")); + } + + // ---------- isLiteralBlocked: synchronous fast path, no DNS ---------- + + @Test + public void isLiteralBlocked_returnsFalseForNullEmptyAndUnresolvable() { + assertFalse(RestrictedHostFilter.isLiteralBlocked(null)); + assertFalse(RestrictedHostFilter.isLiteralBlocked("")); + assertFalse(RestrictedHostFilter.isLiteralBlocked(" ")); + // Unresolvable hostname must not trigger a DNS lookup; the literal isn't on the deny set + // so it falls through to "not blocked". + assertFalse(RestrictedHostFilter.isLiteralBlocked("definitely-not-a-real-host-xyz123.invalid")); + } + + @ParameterizedTest + @ValueSource( + strings = { + "169.254.169.254", + "metadata.google.internal", + "127.0.0.1", + "::1", + "169.254.10.10", + "fc00::1", + }) + public void isLiteralBlocked_blocksDenylistAndNonRoutableLiterals(String host) { + assertTrue(RestrictedHostFilter.isLiteralBlocked(host), "Expected " + host + " to be blocked"); + } + + @Test + public void isLiteralBlocked_blocksConfiguredInternalRedisHostnameLiterally() { + RestrictedHostFilter.setInternalRedisHostsForTesting("internal-redis.svc.cluster.local"); + assertTrue(RestrictedHostFilter.isLiteralBlocked("internal-redis.svc.cluster.local")); + assertTrue(RestrictedHostFilter.isLiteralBlocked("INTERNAL-Redis.svc.cluster.local")); + } + + @Test + public void isLiteralBlocked_doesNotResolveHostnamesToCheckBlockedClass() { + // "localhost" resolves to 127.0.0.1, but isLiteralBlocked must not do DNS — and "localhost" + // itself isn't on the static denylist, so it passes. (isHostBlocked would catch it via DNS; + // that's the correct method to call from the async connection path.) + assertFalse(RestrictedHostFilter.isLiteralBlocked("localhost")); + } + + @Test + public void alwaysAllowedHostsForTesting_clears_cleanly() { + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1"); + assertFalse(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + assertTrue(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + } + + // ---------- APPSMITH_DISABLE_SSRF_FILTER kill-switch ---------- + + @Test + public void ssrfFilterDisabled_bypassesIsHostBlocked() { + // Sanity: blocked by default. + assertTrue(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + assertTrue(RestrictedHostFilter.isHostBlocked("169.254.169.254")); + // Flip the kill-switch — everything passes. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertFalse(RestrictedHostFilter.isHostBlocked("127.0.0.1")); + assertFalse(RestrictedHostFilter.isHostBlocked("169.254.169.254")); + assertFalse(RestrictedHostFilter.isHostBlocked("localhost")); + } + + @Test + public void ssrfFilterDisabled_bypassesIsLiteralBlocked() { + assertTrue(RestrictedHostFilter.isLiteralBlocked("127.0.0.1")); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertFalse(RestrictedHostFilter.isLiteralBlocked("127.0.0.1")); + assertFalse(RestrictedHostFilter.isLiteralBlocked("169.254.169.254")); + } + + @Test + public void ssrfFilterDisabled_bypassesIsDisallowedAndFail() { + assertTrue(RestrictedHostFilter.isDisallowedAndFail("127.0.0.1", null)); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertFalse(RestrictedHostFilter.isDisallowedAndFail("127.0.0.1", null)); + assertFalse(RestrictedHostFilter.isDisallowedAndFail("169.254.169.254", null)); + } + + @Test + public void ssrfFilterDisabled_bypassesResolveIfAllowed() { + // Loopback normally returns empty. + assertTrue(RestrictedHostFilter.resolveIfAllowed("127.0.0.1").isEmpty()); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + // With the kill-switch on, the resolved address comes back. + Optional result = RestrictedHostFilter.resolveIfAllowed("127.0.0.1"); + assertTrue(result.isPresent()); + assertEquals("127.0.0.1", result.get().getHostAddress()); + } + + @Test + public void ssrfFilterDisabled_doesNotAffectAddressClassPredicates() { + // The kill-switch turns off enforcement, not facts about the address itself. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(true); + assertTrue(RestrictedHostFilter.isBlockedIpAddressClass("127.0.0.1")); + assertTrue(RestrictedHostFilter.isBlockedIpAddressClass("169.254.0.1")); + assertTrue(RestrictedHostFilter.isBlockedIpAddressClass("fc00::1")); + } +} diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java index d80a84351e70..652faa24365c 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java @@ -1,6 +1,7 @@ package com.appsmith.util; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; import org.springframework.http.HttpMethod; @@ -9,17 +10,32 @@ import reactor.test.StepVerifier; import java.lang.reflect.Method; -import java.net.InetAddress; import java.net.URI; import java.net.UnknownHostException; -import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +/** + * Integration test for the WebClient request filter wiring. The filter logic itself + * lives in {@link RestrictedHostFilter} and is tested in {@link RestrictedHostFilterTest}; + * this class just verifies that {@link WebClientUtils#IP_CHECK_FILTER} actually rejects + * disallowed hosts at the WebClient layer. + */ public class WebClientUtilsTest { + @BeforeAll + public static void enableFilterForThisClass() { + // Surefire defaults to bypass=true (see root pom); re-enable the filter so the + // request-filter integration test below can verify blocking behavior. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + } + + @AfterAll + public static void restoreSurefireDefault() { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + @ParameterizedTest @ValueSource( strings = { @@ -36,116 +52,11 @@ public void testRequestFilterFnRejectsExpandedMetadataEndpoints(String url) thro StepVerifier.create(invokeRequestFilterFn(url)) .expectErrorSatisfies(throwable -> { assertTrue(throwable instanceof UnknownHostException); - assertEquals(WebClientUtils.HOST_NOT_ALLOWED, throwable.getMessage()); + assertEquals(RestrictedHostFilter.HOST_NOT_ALLOWED, throwable.getMessage()); }) .verify(); } - @ParameterizedTest - @ValueSource(strings = {"metadata.tencentyun.com", "Metadata.TencentYun.Com.", "METAdata.google.internal."}) - public void testIsDisallowedAndFailNormalizesMetadataHostnames(String host) { - assertTrue(WebClientUtils.isDisallowedAndFail(host, null)); - } - - @ParameterizedTest - @ValueSource( - strings = { - "127.0.0.1", - "169.254.169.254", - "169.254.10.10", - "100.100.100.200", - "168.63.129.16", - "0.0.0.0", - }) - public void resolveIfAllowed_blocksLoopbackMetadataAndSpecialHosts(String host) { - Optional result = WebClientUtils.resolveIfAllowed(host); - assertTrue(result.isEmpty(), "Expected host " + host + " to be blocked"); - } - - @Test - public void resolveIfAllowed_blocksNullAndEmpty() { - assertTrue(WebClientUtils.resolveIfAllowed(null).isEmpty()); - assertTrue(WebClientUtils.resolveIfAllowed("").isEmpty()); - assertTrue(WebClientUtils.resolveIfAllowed(" ").isEmpty()); - } - - @Test - public void resolveIfAllowed_blocksLocalhostHostname() { - Optional result = WebClientUtils.resolveIfAllowed("localhost"); - assertTrue(result.isEmpty(), "Expected 'localhost' to be blocked"); - } - - @ParameterizedTest - @ValueSource(strings = {"smtp.gmail.com", "email-smtp.us-east-1.amazonaws.com", "smtp.sendgrid.net"}) - public void resolveIfAllowed_allowsLegitimateSmtpHosts(String host) { - Optional result = WebClientUtils.resolveIfAllowed(host); - assertTrue(result.isPresent(), "Expected host " + host + " to be allowed"); - } - - @Test - public void resolveIfAllowed_blocksUnresolvableHost() { - Optional result = WebClientUtils.resolveIfAllowed("definitely-not-a-real-host-xyz123.invalid"); - assertTrue(result.isEmpty(), "Expected unresolvable host to be blocked"); - } - - @Test - public void resolveIfAllowed_returnsResolvedAddress() { - Optional result = WebClientUtils.resolveIfAllowed("smtp.gmail.com"); - assertTrue(result.isPresent()); - assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+")); - } - - @ParameterizedTest - @ValueSource( - strings = { - // Loopback - "127.0.0.1", - "127.0.0.2", - "127.0.0.254", - "127.1.2.3", - "127.255.255.255", - "::1", - // Any-local - "0.0.0.0", - "::", - // Link-local - "169.254.0.1", - "169.254.169.254", - "fe80::1", - // Multicast - "224.0.0.1", - "239.255.255.250", - "ff02::1", - // IPv6 ULA (fc00::/7) - "fc00::1", - "fd00::1", - "fdff::ffff", - }) - public void isBlockedIpAddressClass_recognizesNonRoutableClasses(String host) { - assertTrue( - WebClientUtils.isBlockedIpAddressClass(host), - "Expected " + host + " to be recognized as a blocked address class"); - } - - @ParameterizedTest - @ValueSource( - strings = { - "1.1.1.1", - "8.8.8.8", - // RFC 1918 site-local — intentionally allowed for internal REST API targets - "192.168.1.1", - "10.0.0.1", - "172.16.0.1", - // Non-literals - "smtp.gmail.com", - "localhost", - }) - public void isBlockedIpAddressClass_doesNotMatchOtherHosts(String host) { - assertFalse( - WebClientUtils.isBlockedIpAddressClass(host), - "Did not expect " + host + " to be recognized as a blocked address class"); - } - @SuppressWarnings("unchecked") private Mono invokeRequestFilterFn(String url) throws Exception { final Method method = WebClientUtils.class.getDeclaredMethod("requestFilterFn", ClientRequest.class); diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java b/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java index 13c8a348fe37..68ef0afa488f 100644 --- a/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java +++ b/app/server/appsmith-plugins/elasticSearchPlugin/src/main/java/org/apache/http/impl/nio/client/HttpAsyncClientBuilder.java @@ -29,7 +29,7 @@ package org.apache.http.impl.nio.client; -import com.appsmith.util.WebClientUtils; +import com.appsmith.util.RestrictedHostFilter; import org.apache.http.ConnectionReuseStrategy; import org.apache.http.Header; import org.apache.http.HttpHost; @@ -654,12 +654,12 @@ public CloseableHttpAsyncClient build() { // This `dnsResolver` is the only thing different from the original class. // In the original class, it is set to SystemDefaultDnsResolver.INSTANCE, inlined. final DnsResolver dnsResolver = host -> { - if (WebClientUtils.isDisallowedAndFail(host, null)) { + if (RestrictedHostFilter.isDisallowedAndFail(host, null)) { throw new UnknownHostException("Host " + host + " is not allowed"); } final InetAddress[] addresses = InetAddress.getAllByName(host); for (InetAddress address : addresses) { - if (WebClientUtils.isDisallowedAndFail(address.getHostAddress(), null)) { + if (RestrictedHostFilter.isDisallowedAndFail(address.getHostAddress(), null)) { throw new UnknownHostException("Host " + host + " is not allowed"); } } diff --git a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java index c410833a411f..c3abb3e737f2 100755 --- a/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java +++ b/app/server/appsmith-plugins/elasticSearchPlugin/src/test/java/com/external/plugins/ElasticSearchPluginTest.java @@ -7,6 +7,7 @@ import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Endpoint; import com.appsmith.external.models.RequestParamDTO; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.ElasticSearchPluginError; import lombok.extern.slf4j.Slf4j; import mockwebserver3.MockResponse; @@ -393,206 +394,258 @@ public void shouldVerifyNotFound() { .verifyComplete(); } + // The itShouldDeny* / itShouldRejectGetToMetadata* tests below all explicitly verify the + // SSRF filter's blocking behavior. Surefire bypasses the filter JVM-wide (see root pom) for + // the benefit of the rest of this class (which talks to the Testcontainers ES on loopback), + // so each test method flips it back on for its body. The redirect variants additionally + // allowlist loopback so the MockWebServer that serves the 301 is reachable. + @Test public void itShouldDenyTestDatasourceWithInstanceMetadataAws() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) - .assertNext(result -> { - assertFalse(result.getInvalids().isEmpty()); - assertTrue(result.getInvalids() - .contains("Error running HEAD request: Host 169.254.169.254 is not allowed")); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) + .assertNext(result -> { + assertFalse(result.getInvalids().isEmpty()); + assertTrue(result.getInvalids() + .contains("Error running HEAD request: Host 169.254.169.254 is not allowed")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldDenyTestDatasourceWithInstanceMetadataAwsWithDnsResolution() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254.nip.io"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) - .assertNext(result -> { - assertFalse(result.getInvalids().isEmpty()); - assertTrue(result.getInvalids() - .contains("Error running HEAD request: Host 169.254.169.254.nip.io is not allowed")); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254.nip.io"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) + .assertNext(result -> { + assertFalse(result.getInvalids().isEmpty()); + assertTrue(result.getInvalids() + .contains("Error running HEAD request: Host 169.254.169.254.nip.io is not allowed")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldDenyTestDatasourceWithInstanceMetadataGcp() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://metadata.google.internal"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) - .assertNext(result -> { - assertFalse(result.getInvalids().isEmpty()); - assertTrue(result.getInvalids() - .contains("Error running HEAD request: Host metadata.google.internal is not allowed")); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://metadata.google.internal"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.testDatasource(datasourceConfiguration)) + .assertNext(result -> { + assertFalse(result.getInvalids().isEmpty()); + assertTrue(result.getInvalids() + .contains("Error running HEAD request: Host metadata.google.internal is not allowed")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataAws() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host 169.254.169.254 is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host 169.254.169.254 is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataAwsWithDnsResolution() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://169.254.169.254.nip.io"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host 169.254.169.254.nip.io is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://169.254.169.254.nip.io"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host 169.254.169.254.nip.io is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataAwsWithDnsResolutionAndRedirect() throws IOException { + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); MockWebServer mockWebServer = new MockWebServer(); - MockResponse mockRedirectResponse = new MockResponse() - .setResponseCode(301) - .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); - mockWebServer.enqueue(mockRedirectResponse); - mockWebServer.start(); - - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://" + mockWebServer.getHostName()); - endpoint.setPort((long) mockWebServer.getPort()); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host 169.254.169.254.nip.io is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + try { + MockResponse mockRedirectResponse = new MockResponse() + .setResponseCode(301) + .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); + mockWebServer.enqueue(mockRedirectResponse); + mockWebServer.start(); + + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://" + mockWebServer.getHostName()); + endpoint.setPort((long) mockWebServer.getPort()); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host 169.254.169.254.nip.io is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + mockWebServer.shutdown(); + } } @Test public void itShouldRejectGetToMetadataGcp() { - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://metadata.google.internal"); - endpoint.setPort(Long.valueOf(port)); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host metadata.google.internal is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://metadata.google.internal"); + endpoint.setPort(Long.valueOf(port)); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host metadata.google.internal is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void itShouldRejectGetToMetadataGcpAndRedirect() throws IOException { + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); MockWebServer mockWebServer = new MockWebServer(); - MockResponse mockRedirectResponse = - new MockResponse().setResponseCode(301).addHeader("Location", "http://metadata.google.internal"); - mockWebServer.enqueue(mockRedirectResponse); - mockWebServer.start(); - - DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); - datasourceConfiguration.setAuthentication(elasticInstanceCredentials); - Endpoint endpoint = new Endpoint(); - endpoint.setHost("http://" + mockWebServer.getHostName()); - endpoint.setPort((long) mockWebServer.getPort()); - datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); - - final ActionConfiguration actionConfiguration = new ActionConfiguration(); - actionConfiguration.setHttpMethod(HttpMethod.GET); - actionConfiguration.setPath("/"); - - final Mono resultMono = pluginExecutor - .datasourceCreate(datasourceConfiguration) - .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); - - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertEquals( - "Host metadata.google.internal is not allowed", - result.getPluginErrorDetails().getDownstreamErrorMessage()); - }) - .verifyComplete(); + try { + MockResponse mockRedirectResponse = + new MockResponse().setResponseCode(301).addHeader("Location", "http://metadata.google.internal"); + mockWebServer.enqueue(mockRedirectResponse); + mockWebServer.start(); + + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setAuthentication(elasticInstanceCredentials); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("http://" + mockWebServer.getHostName()); + endpoint.setPort((long) mockWebServer.getPort()); + datasourceConfiguration.setEndpoints(Collections.singletonList(endpoint)); + + final ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("/"); + + final Mono resultMono = pluginExecutor + .datasourceCreate(datasourceConfiguration) + .flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertEquals( + "Host metadata.google.internal is not allowed", + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + mockWebServer.shutdown(); + } } @Test diff --git a/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java b/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java index 534a15845a6e..6707e57f96db 100644 --- a/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java +++ b/app/server/appsmith-plugins/graphqlPlugin/src/test/java/com/external/plugins/GraphQLPluginTest.java @@ -16,6 +16,7 @@ import com.appsmith.external.models.Param; import com.appsmith.external.models.Property; import com.appsmith.external.services.SharedConfig; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.GraphQLPluginError; import com.external.utils.GraphQLHintMessageUtils; import com.fasterxml.jackson.databind.JsonNode; @@ -1160,49 +1161,94 @@ public void testQueryParamsInDatasource() { .verifyComplete(); } + // These deny tests previously only checked that the request did not succeed, which would + // pass on any ordinary network failure as well — making them useless as SSRF regression + // coverage. They now (a) re-enable the filter (surefire bypasses it JVM-wide; see root pom) + // and (b) assert the downstream error message specifically contains "Host not allowed.", + // so a network outage to the target host can no longer mask a filter regression. + @Test public void testDenyInstanceMetadataAws() { - DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); - dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); - - ActionConfiguration actionConfig = getDefaultActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); + dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); + + ActionConfiguration actionConfig = getDefaultActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCname() { - DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); - dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); - - ActionConfiguration actionConfig = getDefaultActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); + dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); + + ActionConfiguration actionConfig = getDefaultActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataGcp() { - DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); - dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); - - ActionConfiguration actionConfig = getDefaultActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = getDefaultDatasourceConfig(); + dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); + + ActionConfiguration actionConfig = getDefaultActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } /** diff --git a/app/server/appsmith-plugins/redisPlugin/pom.xml b/app/server/appsmith-plugins/redisPlugin/pom.xml index 13396cde0f45..caadf59dd011 100644 --- a/app/server/appsmith-plugins/redisPlugin/pom.xml +++ b/app/server/appsmith-plugins/redisPlugin/pom.xml @@ -19,7 +19,7 @@ redis.clients jedis - 3.3.0 + 5.2.0 org.slf4j diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java index 0ba19679f656..a383276da654 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RedisPlugin.java @@ -13,6 +13,7 @@ import com.appsmith.external.models.SSLDetails; import com.appsmith.external.plugins.BasePlugin; import com.appsmith.external.plugins.PluginExecutor; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.RedisErrorMessages; import com.external.plugins.exceptions.RedisPluginError; import com.external.utils.RedisURIUtils; @@ -25,11 +26,16 @@ import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; +import redis.clients.jedis.DefaultJedisClientConfig; +import redis.clients.jedis.HostAndPort; import redis.clients.jedis.Jedis; +import redis.clients.jedis.JedisClientConfig; import redis.clients.jedis.JedisPool; import redis.clients.jedis.JedisPoolConfig; +import redis.clients.jedis.JedisSocketFactory; import redis.clients.jedis.Protocol; import redis.clients.jedis.exceptions.JedisException; +import redis.clients.jedis.util.JedisURIHelper; import redis.clients.jedis.util.SafeEncoder; import java.net.URI; @@ -258,18 +264,71 @@ private JedisPoolConfig buildPoolConfig() { public Mono datasourceCreate(DatasourceConfiguration datasourceConfiguration) { log.debug(Thread.currentThread().getName() + ": datasourceCreate() called for Redis plugin."); return Mono.fromCallable(() -> { + // SSRF enforcement is layered (GHSA-qhfj-g87x-m39w): + // 1. assertHostAllowed below is an early, create-time pre-check so the user + // sees "Host not allowed." immediately on Save/Test for an obviously + // denylisted host. validateDatasource intentionally does not duplicate it + // (its contract is format-only; host-policy belongs on the connection path). + // 2. The real, TOCTOU-proof guarantee is RestrictedHostJedisSocketFactory + // (wired below), which re-validates with a SINGLE DNS resolution at the + // moment the pool opens a socket and connects to the validated IP — so a + // DNS-rebinding resolver cannot slip a different IP past the pre-check. + assertHostAllowed(datasourceConfiguration); final JedisPoolConfig poolConfig = buildPoolConfig(); boolean isTlsEnabled = isTlsEnabled(datasourceConfiguration); int timeout = (int) Duration.ofSeconds(CONNECTION_TIMEOUT).toMillis(); URI uri = RedisURIUtils.getURI(datasourceConfiguration, isTlsEnabled); - JedisPool jedisPool = new JedisPool(poolConfig, uri, timeout); + + // Derive the client config exactly as Jedis's own URI constructor does (user, + // password, db index, ssl scheme — see JedisFactory(URI, ...)), then route + // socket creation through RestrictedHostJedisSocketFactory. This preserves all + // existing URI parsing semantics while pinning the connection to a single + // SSRF-validated DNS resolution. + HostAndPort hostAndPort = new HostAndPort(uri.getHost(), uri.getPort()); + JedisClientConfig clientConfig = DefaultJedisClientConfig.builder() + .connectionTimeoutMillis(timeout) + .socketTimeoutMillis(timeout) + .user(JedisURIHelper.getUser(uri)) + .password(JedisURIHelper.getPassword(uri)) + .database(JedisURIHelper.getDBIndex(uri)) + .ssl(JedisURIHelper.isRedisSSLScheme(uri)) + .build(); + JedisSocketFactory socketFactory = + new RestrictedHostJedisSocketFactory(hostAndPort, clientConfig); + JedisPool jedisPool = new JedisPool(poolConfig, socketFactory, clientConfig); log.debug(Thread.currentThread().getName() + ": Created Jedis pool."); return jedisPool; }) .subscribeOn(scheduler); } + private void assertHostAllowed(DatasourceConfiguration datasourceConfiguration) throws AppsmithPluginException { + if (isEndpointMissing(datasourceConfiguration.getEndpoints())) { + // Let the existing missing-host validation surface the error. + return; + } + final String host = datasourceConfiguration.getEndpoints().get(0).getHost(); + // Create-time pre-check for fast, clear UX on Save/Test. This resolution and the one + // Jedis performs at connect time are technically distinct, so this check alone is + // subject to DNS rebinding — but it is no longer the security boundary: + // RestrictedHostJedisSocketFactory re-validates with a single resolution at connect + // time and is what actually closes the rebinding TOCTOU. See GHSA-qhfj-g87x-m39w. + if (RestrictedHostFilter.isHostBlocked(host)) { + // Most blocks land here (a user pointing a datasource at a disallowed host), so log + // it as the primary observability signal — the log framework tags this with the + // requesting userEmail / orgId / traceId, which makes a spike a useful recon flag. + // The connect-time sibling (RestrictedHostJedisSocketFactory) logs the rarer + // rebinding case. + log.warn( + "Blocked a Redis datasource pointed at disallowed host '{}' (resolves to [{}]) — SSRF filter.", + host, + RestrictedHostFilter.describeResolvedAddresses(host)); + throw new AppsmithPluginException( + AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, RestrictedHostFilter.HOST_NOT_ALLOWED); + } + } + private boolean isTlsEnabled(DatasourceConfiguration datasourceConfiguration) throws AppsmithPluginException { if (datasourceConfiguration.getConnection() == null || datasourceConfiguration.getConnection().getSsl() == null @@ -318,6 +377,14 @@ public Set validateDatasource(DatasourceConfiguration datasourceConfigur if (isEndpointMissing(datasourceConfiguration.getEndpoints())) { invalids.add(RedisErrorMessages.DS_MISSING_HOST_ADDRESS_ERROR_MSG); } + // SSRF host-allowed enforcement intentionally lives in datasourceCreate (which goes + // through assertHostAllowed → RestrictedHostFilter.isHostBlocked on the + // bounded-elastic scheduler) rather than here. PluginExecutor#validateDatasource is + // documented as "mandatory fields and format only — does NOT check validity of + // those fields; use testDatasource for that". Host-on-denylist is a policy/validity + // check, not a format check, so it belongs on the connection path. testDatasource + // calls datasourceCreate to build the pool and surfaces "Host not allowed." + // immediately when the user clicks Test. See GHSA-qhfj-g87x-m39w. DBAuth auth = (DBAuth) datasourceConfiguration.getAuthentication(); if (isAuthenticationMissing(auth)) { @@ -401,8 +468,15 @@ public Mono testDatasource(JedisPool connectionPool) { return Mono.just(connectionPool) .flatMap(c -> verifyPing(connectionPool)) .then(Mono.just(new DatasourceTestResult())) - .onErrorResume(error -> - Mono.just(new DatasourceTestResult(error.getCause().getMessage()))); + .onErrorResume(error -> { + // Prefer the cause's message (driver wraps connection failures with an + // IOException cause), but fall back to the error itself so a causeless + // exception doesn't NPE here. The connection-time SSRF rejection throws a + // causeless JedisConnectionException whose message is "Host not allowed."; + // without this fallback it surfaced as a generic connection error. + final Throwable reported = error.getCause() != null ? error.getCause() : error; + return Mono.just(new DatasourceTestResult(reported.getMessage())); + }); } } } diff --git a/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java new file mode 100644 index 000000000000..63ed27d2c6c3 --- /dev/null +++ b/app/server/appsmith-plugins/redisPlugin/src/main/java/com/external/plugins/RestrictedHostJedisSocketFactory.java @@ -0,0 +1,187 @@ +package com.external.plugins; + +import com.appsmith.util.RestrictedHostFilter; +import lombok.extern.slf4j.Slf4j; +import redis.clients.jedis.HostAndPort; +import redis.clients.jedis.JedisClientConfig; +import redis.clients.jedis.JedisSocketFactory; +import redis.clients.jedis.SSLSocketWrapper; +import redis.clients.jedis.exceptions.JedisConnectionException; + +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.UnknownHostException; +import java.util.Arrays; + +/** + * A Jedis {@link JedisSocketFactory} that enforces Appsmith's SSRF host policy at the exact + * moment the socket is opened, using a SINGLE DNS resolution shared between the policy check and + * the TCP connect. + * + *

Background (GHSA-qhfj-g87x-m39w): the Redis plugin pre-checks the host in + * {@code datasourceCreate} via {@link RestrictedHostFilter#isHostBlocked(String)}, but the stock + * {@link redis.clients.jedis.DefaultJedisSocketFactory} resolves the hostname again when it opens + * the pool's first socket. Those two independent resolutions are a DNS-rebinding TOCTOU window: a + * hostile resolver can return an allowed IP to the pre-check and the internal Redis IP to the + * driver. This factory closes the window by resolving once, validating the resolved addresses via + * {@link RestrictedHostFilter#firstAllowedRedisAddress(String, InetAddress[])}, and connecting the + * socket directly to the returned {@link InetAddress} — the driver never re-resolves the hostname. + * + *

For TLS ({@code rediss://}) the plaintext socket connects to the pinned IP, but the original + * hostname is handed to the {@link SSLSocketFactory}, so SNI and certificate verification run + * against the hostname the user configured, not the IP. Unlike stock Jedis 5.2.0 — which leaves + * hostname verification off by default — this factory enforces it by setting the endpoint + * identification algorithm to {@code HTTPS}, so a CA-trusted-but-mismatched cert on the pinned IP + * is rejected. The connect and TLS handshake logic otherwise mirrors {@code DefaultJedisSocketFactory} + * (socket options, {@link SSLSocketWrapper}); the deviations are the single-resolution SSRF check + * in place of the stock {@code InetAddress.getAllByName(...)} + connect, and the enforced + * endpoint identification. + */ +@Slf4j +public class RestrictedHostJedisSocketFactory implements JedisSocketFactory { + + private final String host; + private final int port; + private final int connectionTimeout; + private final int socketTimeout; + private final boolean ssl; + private final SSLSocketFactory sslSocketFactory; + private final SSLParameters sslParameters; + private final HostnameVerifier hostnameVerifier; + + public RestrictedHostJedisSocketFactory(HostAndPort hostAndPort, JedisClientConfig config) { + this.host = hostAndPort.getHost(); + this.port = hostAndPort.getPort(); + this.connectionTimeout = config.getConnectionTimeoutMillis(); + this.socketTimeout = config.getSocketTimeoutMillis(); + this.ssl = config.isSsl(); + this.sslSocketFactory = config.getSslSocketFactory(); + this.sslParameters = config.getSslParameters(); + this.hostnameVerifier = config.getHostnameVerifier(); + } + + @Override + public Socket createSocket() throws JedisConnectionException { + // SINGLE resolution. A genuine resolution failure surfaces as a connection error (clearer + // than "Host not allowed."); only a resolved-but-blocked host is a policy rejection. + final InetAddress[] resolved; + try { + resolved = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + throw new JedisConnectionException("Failed to resolve Redis host '" + host + "'.", e); + } + + // Gate: firstAllowedRedisAddress returns present only when EVERY resolved address passed the + // filter, so a present result means all of `resolved` are safe to connect to. The driver + // never re-resolves, so a rebinding resolver cannot swap in a different (internal) IP. + if (RestrictedHostFilter.firstAllowedRedisAddress(host, resolved).isEmpty()) { + // Connection-time SSRF rejection (incl. the DNS-rebinding case where the resolved + // address differs from the create-time pre-check). Log it — this is the only place the + // block is observable server-side, since the pre-check rejection happens elsewhere. + log.warn( + "Refusing Redis connection: host '{}' resolved to a disallowed address [{}].", + host, + String.join( + ", ", + Arrays.stream(resolved) + .map(InetAddress::getHostAddress) + .toList())); + throw new JedisConnectionException(RestrictedHostFilter.HOST_NOT_ALLOWED); + } + + Socket socket = null; + try { + // Connect to the first reachable validated address. Trying all of them (not just the + // first) means a dual-stack host whose leading record is an unreachable IPv6 still falls + // back to its IPv4, matching DefaultJedisSocketFactory. Every candidate was validated + // above, so the failover does not widen the SSRF surface. No second DNS lookup happens. + socket = connectToFirstReachable(resolved); + socket.setSoTimeout(socketTimeout); + + if (ssl) { + SSLSocketFactory factory = + sslSocketFactory != null ? sslSocketFactory : (SSLSocketFactory) SSLSocketFactory.getDefault(); + final Socket plainSocket = socket; + // Hand the original hostname (not the pinned IP) to the TLS layer so SNI and + // certificate hostname verification run against the configured hostname. + socket = factory.createSocket(plainSocket, host, port, true); + + final SSLSocket sslSocket = (SSLSocket) socket; + // Enforce TLS certificate hostname verification (RFC 2818): the cert's SAN/CN must + // match the configured hostname. Jedis 5.2.0 leaves this off by default, which would + // let a MITM present any CA-trusted-but-mismatched cert on the pinned IP. Verification + // runs against `host` (passed to createSocket above), not the IP we connected to. + // Only default to HTTPS when a caller hasn't deliberately set its own endpoint- + // identification policy. + final SSLParameters params = sslParameters != null ? sslParameters : sslSocket.getSSLParameters(); + final String endpointIdAlgorithm = params.getEndpointIdentificationAlgorithm(); + if (endpointIdAlgorithm == null || endpointIdAlgorithm.isEmpty()) { + params.setEndpointIdentificationAlgorithm("HTTPS"); + } + sslSocket.setSSLParameters(params); + + socket = new SSLSocketWrapper(sslSocket, plainSocket); + if (hostnameVerifier != null && !hostnameVerifier.verify(host, ((SSLSocket) socket).getSession())) { + throw new JedisConnectionException( + String.format("The connection to '%s' failed ssl/tls hostname verification.", host)); + } + } + + return socket; + } catch (JedisConnectionException e) { + closeQuietly(socket); + throw e; + } catch (Exception e) { + closeQuietly(socket); + throw new JedisConnectionException("Failed to create socket.", e); + } + } + + /** + * Opens a plain TCP socket to the first reachable address, trying each in order (mirrors + * {@code DefaultJedisSocketFactory#connectToFirstSuccessfulHost}). All addresses passed here + * have already cleared the SSRF filter, so iterating only adds connection resilience — e.g. a + * dual-stack host whose first record is an unreachable IPv6 still connects via its IPv4. Throws + * once every candidate has failed, with each failure attached as a suppressed exception. + */ + private Socket connectToFirstReachable(InetAddress[] addresses) throws JedisConnectionException { + JedisConnectionException failure = null; + for (InetAddress address : addresses) { + Socket candidate = null; + try { + candidate = new Socket(); + // Socket options mirror DefaultJedisSocketFactory. + candidate.setReuseAddress(true); + candidate.setKeepAlive(true); + candidate.setTcpNoDelay(true); + candidate.setSoLinger(true, 0); + candidate.connect(new InetSocketAddress(address, port), connectionTimeout); + return candidate; + } catch (Exception e) { + closeQuietly(candidate); + if (failure == null) { + failure = new JedisConnectionException("Failed to connect to Redis host '" + host + "'."); + } + failure.addSuppressed(e); + } + } + throw failure != null + ? failure + : new JedisConnectionException("Failed to connect to Redis host '" + host + "'."); + } + + private static void closeQuietly(Socket socket) { + if (socket != null) { + try { + socket.close(); + } catch (Exception ignored) { + // Best-effort cleanup on a failed connect; nothing actionable here. + } + } + } +} diff --git a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java index c8e4964fa681..80af94771665 100644 --- a/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java +++ b/app/server/appsmith-plugins/redisPlugin/src/test/java/com/external/plugins/RedisPluginTest.java @@ -11,11 +11,13 @@ import com.appsmith.external.models.Endpoint; import com.appsmith.external.models.RequestParamDTO; import com.appsmith.external.models.SSLDetails; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.RedisErrorMessages; import com.external.plugins.exceptions.RedisPluginError; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -25,7 +27,11 @@ import org.testcontainers.utility.DockerImageName; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import redis.clients.jedis.DefaultJedisClientConfig; +import redis.clients.jedis.HostAndPort; +import redis.clients.jedis.JedisClientConfig; import redis.clients.jedis.JedisPool; +import redis.clients.jedis.exceptions.JedisConnectionException; import java.util.ArrayList; import java.util.Arrays; @@ -56,6 +62,15 @@ public class RedisPluginTest { public static void setup() { host = redis.getContainerIpAddress(); port = redis.getFirstMappedPort(); + // Testcontainers exposes Redis on loopback, which the production isHostBlocked filter + // rejects (GHSA-qhfj-g87x-m39w). Allow this specific host for the duration of the test + // class so the integration tests can drive the real Redis. Cleared in @AfterAll. + RestrictedHostFilter.setAlwaysAllowedHostsForTesting(host); + } + + @AfterAll + public static void teardown() { + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); } private DatasourceConfiguration createDatasourceConfiguration() { @@ -125,6 +140,58 @@ public void itShouldThrowErrorForUnsupportedTlsAuthType() { .verify(); } + @Test + public void datasourceCreate_blockedHost_failsWithHostNotAllowed() { + // End-to-end check that the Redis plugin actually wires through to the SSRF filter + // (GHSA-qhfj-g87x-m39w): a config pointing at a denylist host fails datasourceCreate + // with HOST_NOT_ALLOWED, never opening a Jedis pool. Surefire bypasses the filter + // JVM-wide (see root pom) so the rest of this class can talk to Testcontainers Redis + // on loopback via the @BeforeAll allowlist; this test flips the filter back on for + // its body so the deny path is exercised. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration blockedConfig = new DatasourceConfiguration(); + Endpoint endpoint = new Endpoint(); + endpoint.setHost("169.254.169.254"); // AWS instance metadata + endpoint.setPort(6379L); + blockedConfig.setEndpoints(Collections.singletonList(endpoint)); + + StepVerifier.create(pluginExecutor.datasourceCreate(blockedConfig)) + .expectErrorSatisfies(error -> { + assertTrue(error instanceof AppsmithPluginException); + assertTrue( + error.getMessage().contains(RestrictedHostFilter.HOST_NOT_ALLOWED), + "Expected '" + RestrictedHostFilter.HOST_NOT_ALLOWED + "', got: " + error.getMessage()); + }) + .verify(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + } + + @Test + public void socketFactory_blockedHost_failsAtConnectTime() { + // The connect-time guarantee that closes the DNS-rebinding TOCTOU (GHSA-qhfj-g87x-m39w): + // RestrictedHostJedisSocketFactory re-validates with a single DNS resolution at the moment + // it opens the socket, so even if the datasourceCreate pre-check were bypassed by a flipping + // resolver, the factory still refuses to connect to a denylisted address. Surefire bypasses + // the filter JVM-wide (see root pom); flip it back on for this body. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + JedisClientConfig clientConfig = DefaultJedisClientConfig.builder().build(); + RestrictedHostJedisSocketFactory factory = + new RestrictedHostJedisSocketFactory(new HostAndPort("169.254.169.254", 6379), clientConfig); + + JedisConnectionException ex = + Assertions.assertThrows(JedisConnectionException.class, factory::createSocket); + assertTrue( + ex.getMessage().contains(RestrictedHostFilter.HOST_NOT_ALLOWED), + "Expected '" + RestrictedHostFilter.HOST_NOT_ALLOWED + "', got: " + ex.getMessage()); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } + } + @Test public void itShouldValidateDatasourceWithNoEndpoints() { DatasourceConfiguration invalidDatasourceConfiguration = new DatasourceConfiguration(); @@ -210,7 +277,6 @@ public void itShouldTestDatasource() { public void itShouldThrowErrorIfHostnameIsInvalid() { String invalidHost = "invalidHost"; - String errorMessage = "Failed connecting to " + invalidHost + ":" + port; DatasourceConfiguration datasourceConfiguration = createDatasourceConfiguration(); Endpoint endpoint = new Endpoint(); @@ -223,8 +289,12 @@ public void itShouldThrowErrorIfHostnameIsInvalid() { StepVerifier.create(datasourceTestResultMono) .assertNext(datasourceTestResult -> { assertNotNull(datasourceTestResult); + // An invalid/unreachable host must fail the test with an error reported back to + // the user. We deliberately don't assert the exact message: Jedis 5.x reports an + // unresolvable host with a different string than 3.x did, so pinning to a + // driver-internal message makes the test brittle across upgrades. assertFalse(datasourceTestResult.isSuccess()); - assertTrue(datasourceTestResult.getInvalids().contains(errorMessage)); + assertFalse(datasourceTestResult.getInvalids().isEmpty()); }) .verifyComplete(); } diff --git a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java index 3723c9b8f730..1c0301672b23 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java @@ -19,6 +19,7 @@ import com.appsmith.external.models.Param; import com.appsmith.external.models.Property; import com.appsmith.external.services.SharedConfig; +import com.appsmith.util.RestrictedHostFilter; import com.external.plugins.exceptions.RestApiPluginError; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -1879,129 +1880,181 @@ public void testQueryParamsInDatasource() { .verifyComplete(); } + // The testDenyInstanceMetadata* tests below all explicitly verify the SSRF filter's blocking + // behavior. Surefire bypasses the filter JVM-wide (see root pom) for the benefit of tests + // that use MockWebServer on loopback, so each of these methods flips it back on for its body. + @Test public void testDenyInstanceMetadataAws() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://169.254.169.254/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue( - result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed."), - "Unexpected error message. Did this fail for a different reason?"); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Unexpected error message. Did this fail for a different reason?"); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCname() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://169.254.169.254.nip.io/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertThat(result.getPluginErrorDetails().getDownstreamErrorMessage()) - .endsWith("Host not allowed."); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertThat(result.getPluginErrorDetails().getDownstreamErrorMessage()) + .endsWith("Host not allowed."); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCnameIpv6() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://0--a9fe-a9fe.sslip.io/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://0--a9fe-a9fe.sslip.io/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue(result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed.")); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue(result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed.")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsViaCompatibleIpv6Address() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://[::169.254.169.254]/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://[::169.254.169.254]/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue(result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed.")); - }) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue(result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed.")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataGcp() { - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl("http://metadata.google.internal/latest/meta-data"); - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> assertFalse(result.getIsExecutionSuccess())) - .verifyComplete(); + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue( + result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed."), + "Expected the SSRF filter to block, got: " + + result.getPluginErrorDetails().getDownstreamErrorMessage()); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @Test public void testDenyInstanceMetadataAwsWithRedirect() throws IOException { - // Generate a mock response which redirects to the invalid host - mockEndpoint = new MockWebServer(); - MockResponse mockRedirectResponse = new MockResponse() - .setResponseCode(301) - .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); - mockEndpoint.enqueue(mockRedirectResponse); - mockEndpoint.start(); - - HttpUrl mockHttpUrl = mockEndpoint.url("/mock/redirect"); - DatasourceConfiguration dsConfig = new DatasourceConfiguration(); - dsConfig.setUrl(mockHttpUrl.toString()); - - ActionConfiguration actionConfig = new ActionConfiguration(); - actionConfig.setHttpMethod(HttpMethod.GET); - - Mono resultMono = - pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); - StepVerifier.create(resultMono) - .assertNext(result -> { - assertFalse(result.getIsExecutionSuccess()); - assertTrue(result.getPluginErrorDetails() - .getDownstreamErrorMessage() - .contains("Host not allowed.")); - }) - .verifyComplete(); + // Combined fix: re-enable the filter so the redirect target (169.254.169.254) is blocked, + // but allowlist loopback so the initial MockWebServer call (which serves the 301) goes + // through. "::1" is in the allowlist because the IPv4-compat normalizer canonicalizes it + // to "0.0.0.1", matching whatever form the Netty resolver hands the filter on IPv6 hosts. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + RestrictedHostFilter.setAlwaysAllowedHostsForTesting("127.0.0.1", "localhost", "::1"); + // Use a local server rather than reassigning the @BeforeEach `mockEndpoint` field — + // overwriting that field would orphan the original (tearDown only shuts down the + // current value of the field) and leak server threads/sockets across the suite. + MockWebServer redirectMockServer = new MockWebServer(); + try { + // Generate a mock response which redirects to the invalid host + MockResponse mockRedirectResponse = new MockResponse() + .setResponseCode(301) + .addHeader("Location", "http://169.254.169.254.nip.io/latest/meta-data"); + redirectMockServer.enqueue(mockRedirectResponse); + redirectMockServer.start(); + + HttpUrl mockHttpUrl = redirectMockServer.url("/mock/redirect"); + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + dsConfig.setUrl(mockHttpUrl.toString()); + + ActionConfiguration actionConfig = new ActionConfiguration(); + actionConfig.setHttpMethod(HttpMethod.GET); + + Mono resultMono = + pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig); + StepVerifier.create(resultMono) + .assertNext(result -> { + assertFalse(result.getIsExecutionSuccess()); + assertTrue(result.getPluginErrorDetails() + .getDownstreamErrorMessage() + .contains("Host not allowed.")); + }) + .verifyComplete(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + RestrictedHostFilter.clearAlwaysAllowedHostsForTesting(); + redirectMockServer.shutdown(); + } } @Test diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java index 6a8a6cc6c7b2..26f06496ab12 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/RedisConfig.java @@ -3,6 +3,7 @@ import com.appsmith.server.domains.LoginSource; import com.appsmith.server.dtos.OAuth2AuthorizedClientDTO; import com.appsmith.server.dtos.UserSessionDTO; +import com.appsmith.util.RestrictedHostFilter; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; import io.lettuce.core.AbstractRedisClient; @@ -13,6 +14,7 @@ import io.lettuce.core.cluster.RedisClusterClient; import io.lettuce.core.resource.ClientResources; import io.micrometer.observation.ObservationRegistry; +import jakarta.annotation.PostConstruct; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Value; @@ -59,6 +61,22 @@ public class RedisConfig { @Value("${appsmith.redis.url:}") private String redisURL; + @Value("${appsmith.redis.git.url:}") + private String redisGitURL; + + /** + * Teach the SSRF host filter which Redis the app is actually configured against, using the + * Spring-resolved property rather than only the {@code APPSMITH_REDIS_URL} env var the filter + * reads at static init. This closes a fail-open gap: an operator who sets {@code appsmith.redis.url} + * via application.properties or a {@code -D} system property (no env var) would otherwise leave + * the internal-Redis denylist empty, letting a datasource reach an in-cluster Redis. Runs once + * at startup, before any datasource can be tested. See GHSA-qhfj-g87x-m39w. + */ + @PostConstruct + public void registerInternalRedisHostsWithSsrfFilter() { + RestrictedHostFilter.registerInternalRedisHosts(redisURL, redisGitURL); + } + /** * This is the topic to which we will publish & subscribe to. We can have multiple topics based on the messages * that we wish to broadcast. Starting with a single one for now. diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java index b1954e63140a..d806df46869f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java @@ -28,7 +28,7 @@ import com.appsmith.server.services.PermissionGroupService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserService; -import com.appsmith.util.WebClientUtils; +import com.appsmith.util.RestrictedHostFilter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.mail.MessagingException; @@ -816,7 +816,7 @@ public Mono sendTestEmail(TestEmailConfigRequestDTO requestDTO) { new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Invalid SMTP configuration.")); } - var resolvedAddress = WebClientUtils.resolveIfAllowed(requestDTO.getSmtpHost()); + var resolvedAddress = RestrictedHostFilter.resolveIfAllowed(requestDTO.getSmtpHost()); if (resolvedAddress.isEmpty()) { return Mono.error( new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Invalid SMTP configuration.")); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java index a514fdefd141..a113ce0d542e 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java @@ -20,6 +20,7 @@ import com.appsmith.server.services.PermissionGroupService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserService; +import com.appsmith.util.RestrictedHostFilter; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.BeforeEach; @@ -410,14 +411,22 @@ private TestEmailConfigRequestDTO buildDto(String smtpHost, int port) { @ParameterizedTest @ValueSource(strings = {"127.0.0.1", "169.254.169.254", "localhost"}) public void sendTestEmail_WhenBlockedHost_ThrowsException(String host) { - mockSuperUser(); - - StepVerifier.create(envManager.sendTestEmail(buildDto(host))) - .expectErrorSatisfies(e -> { - assertThat(e).isInstanceOf(AppsmithException.class); - assertThat(e.getMessage()).contains("Invalid SMTP configuration"); - }) - .verify(); + // Surefire bypasses the SSRF filter JVM-wide (see root pom) for the benefit of tests that + // use MockWebServer / Testcontainers on loopback. This test explicitly verifies the + // filter, so re-enable it for the test body. + RestrictedHostFilter.setSsrfFilterDisabledForTesting(false); + try { + mockSuperUser(); + + StepVerifier.create(envManager.sendTestEmail(buildDto(host))) + .expectErrorSatisfies(e -> { + assertThat(e).isInstanceOf(AppsmithException.class); + assertThat(e.getMessage()).contains("Invalid SMTP configuration"); + }) + .verify(); + } finally { + RestrictedHostFilter.resetSsrfFilterDisabledForTesting(); + } } @ParameterizedTest diff --git a/app/server/pom.xml b/app/server/pom.xml index 7204205fa307..9630729ce1ac 100644 --- a/app/server/pom.xml +++ b/app/server/pom.xml @@ -125,6 +125,28 @@ --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.time=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED + + + true + src/test/java ${skipUTs}