block local addresses to prevent SSRF attacks

commafeed-server/doc/commafeed.md

@@ -287,6 +287,29 @@ MemorySize [🛈](#memory-size-note-anchor)
 `5M`
 </td>
 </tr>
+<tr>
+<td>
+
+`commafeed.http-client.block-local-addresses`
+
+Prevent access to local addresses to mitigate server-side request forgery (SSRF) attacks, which could potentially expose internal
+resources.
+
+You may want to disable this if you subscribe to feeds that are only available on your local network and you trust all users of
+your CommaFeed instance.
+
+
+
+Environment variable: `COMMAFEED_HTTP_CLIENT_BLOCK_LOCAL_ADDRESSES`</td>
+<td>
+
+boolean
+</td>
+<td>
+
+`true`
+</td>
+</tr>
 <thead>
 <tr>
 <th align="left" colspan="3">

commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java

@@ -138,6 +138,16 @@ public interface CommaFeedConfiguration {
 		@WithDefault("5M")
 		MemorySize maxResponseSize();
 
+		/**
+		 * Prevent access to local addresses to mitigate server-side request forgery (SSRF) attacks, which could potentially expose internal
+		 * resources.
+		 *
+		 * You may want to disable this if you subscribe to feeds that are only available on your local network and you trust all users of
+		 * your CommaFeed instance.
+		 */
+		@WithDefault("true")
+		boolean blockLocalAddresses();
+
 		/**
 		 * HTTP client cache configuration
 		 */

commafeed-server/src/main/java/com/commafeed/backend/HttpGetter.java

@@ -2,7 +2,9 @@ package com.commafeed.backend;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.InetAddress;
 import java.net.URI;
+import java.net.UnknownHostException;
 import java.time.Duration;
 import java.time.Instant;
 import java.time.InstantSource;
@@ -10,8 +12,11 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
+import java.util.stream.Stream;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.hc.client5.http.DnsResolver;
+import org.apache.hc.client5.http.SystemDefaultDnsResolver;
 import org.apache.hc.client5.http.config.ConnectionConfig;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.config.TlsConfig;
@@ -66,6 +71,7 @@ public class HttpGetter {
 	private final InstantSource instantSource;
 	private final CloseableHttpClient client;
 	private final Cache<HttpRequest, HttpResponse> cache;
+	private final DnsResolver dnsResolver = SystemDefaultDnsResolver.INSTANCE;
 
 	public HttpGetter(CommaFeedConfiguration config, InstantSource instantSource, CommaFeedVersion version, MetricRegistry metrics) {
 		this.config = config;
@@ -89,11 +95,20 @@ public class HttpGetter {
 				() -> cache == null ? 0 : cache.asMap().values().stream().mapToInt(e -> e.content != null ? e.content.length : 0).sum());
 	}
 
-	public HttpResult get(String url) throws IOException, NotModifiedException, TooManyRequestsException {
+	public HttpResult get(String url)
+			throws IOException, NotModifiedException, TooManyRequestsException, SchemeNotAllowedException, HostNotAllowedException {
 		return get(HttpRequest.builder(url).build());
 	}
 
-	public HttpResult get(HttpRequest request) throws IOException, NotModifiedException, TooManyRequestsException {
+	public HttpResult get(HttpRequest request)
+			throws IOException, NotModifiedException, TooManyRequestsException, SchemeNotAllowedException, HostNotAllowedException {
+		URI uri = URI.create(request.getUrl());
+		ensureHttpScheme(uri.getScheme());
+
+		if (config.httpClient().blockLocalAddresses()) {
+			ensurePublicAddress(uri.getHost());
+		}
+
 		final HttpResponse response;
 		if (cache == null) {
 			response = invoke(request);
@@ -141,6 +156,28 @@ public class HttpGetter {
 				response.getUrlAfterRedirect(), validFor);
 	}
 
+	private void ensureHttpScheme(String scheme) throws SchemeNotAllowedException {
+		if (!"http".equals(scheme) && !"https".equals(scheme)) {
+			throw new SchemeNotAllowedException(scheme);
+		}
+	}
+
+	private void ensurePublicAddress(String host) throws HostNotAllowedException, UnknownHostException {
+		if (host == null) {
+			throw new HostNotAllowedException(null);
+		}
+
+		InetAddress[] addresses = dnsResolver.resolve(host);
+		if (Stream.of(addresses).anyMatch(this::isPrivateAddress)) {
+			throw new HostNotAllowedException(host);
+		}
+	}
+
+	private boolean isPrivateAddress(InetAddress address) {
+		return address.isSiteLocalAddress() || address.isAnyLocalAddress() || address.isLinkLocalAddress() || address.isLoopbackAddress()
+				|| address.isMulticastAddress();
+	}
+
 	private HttpResponse invoke(HttpRequest request) throws IOException {
 		log.debug("fetching {}", request.getUrl());
 
@@ -229,7 +266,7 @@ public class HttpGetter {
 		}
 	}
 
-	private static PoolingHttpClientConnectionManager newConnectionManager(CommaFeedConfiguration config) {
+	private PoolingHttpClientConnectionManager newConnectionManager(CommaFeedConfiguration config) {
 		SSLFactory sslFactory = SSLFactory.builder().withUnsafeTrustMaterial().withUnsafeHostnameVerifier().build();
 
 		int poolSize = config.feedRefresh().httpThreads();
@@ -243,6 +280,7 @@ public class HttpGetter {
 				.setDefaultTlsConfig(TlsConfig.custom().setHandshakeTimeout(Timeout.of(config.httpClient().sslHandshakeTimeout())).build())
 				.setMaxConnPerRoute(poolSize)
 				.setMaxConnTotal(poolSize)
+				.setDnsResolver(dnsResolver)
 				.build();
 
 	}
@@ -279,6 +317,22 @@ public class HttpGetter {
 				.build();
 	}
 
+	public static class SchemeNotAllowedException extends Exception {
+		private static final long serialVersionUID = 1L;
+
+		public SchemeNotAllowedException(String scheme) {
+			super("Scheme not allowed: " + scheme);
+		}
+	}
+
+	public static class HostNotAllowedException extends Exception {
+		private static final long serialVersionUID = 1L;
+
+		public HostNotAllowedException(String host) {
+			super("Host not allowed: " + host);
+		}
+	}
+
 	@Getter
 	public static class NotModifiedException extends Exception {
 		private static final long serialVersionUID = 1L;

commafeed-server/src/main/java/com/commafeed/backend/favicon/YoutubeFaviconFetcher.java

@@ -11,8 +11,10 @@ import org.apache.hc.core5.net.URIBuilder;
 
 import com.commafeed.CommaFeedConfiguration;
 import com.commafeed.backend.HttpGetter;
+import com.commafeed.backend.HttpGetter.HostNotAllowedException;
 import com.commafeed.backend.HttpGetter.HttpResult;
 import com.commafeed.backend.HttpGetter.NotModifiedException;
+import com.commafeed.backend.HttpGetter.SchemeNotAllowedException;
 import com.commafeed.backend.HttpGetter.TooManyRequestsException;
 import com.commafeed.backend.model.Feed;
 import com.fasterxml.jackson.core.JsonPointer;
@@ -92,7 +94,8 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher {
 		return new Favicon(bytes, contentType);
 	}
 
-	private byte[] fetchForUser(String googleAuthKey, String userId) throws IOException, NotModifiedException, TooManyRequestsException {
+	private byte[] fetchForUser(String googleAuthKey, String userId)
+			throws IOException, NotModifiedException, TooManyRequestsException, HostNotAllowedException, SchemeNotAllowedException {
 		URI uri = UriBuilder.fromUri("https://www.googleapis.com/youtube/v3/channels")
 				.queryParam("part", "snippet")
 				.queryParam("key", googleAuthKey)
@@ -102,7 +105,7 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher {
 	}
 
 	private byte[] fetchForChannel(String googleAuthKey, String channelId)
-			throws IOException, NotModifiedException, TooManyRequestsException {
+			throws IOException, NotModifiedException, TooManyRequestsException, HostNotAllowedException, SchemeNotAllowedException {
 		URI uri = UriBuilder.fromUri("https://www.googleapis.com/youtube/v3/channels")
 				.queryParam("part", "snippet")
 				.queryParam("key", googleAuthKey)
@@ -112,7 +115,7 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher {
 	}
 
 	private byte[] fetchForPlaylist(String googleAuthKey, String playlistId)
-			throws IOException, NotModifiedException, TooManyRequestsException {
+			throws IOException, NotModifiedException, TooManyRequestsException, HostNotAllowedException, SchemeNotAllowedException {
 		URI uri = UriBuilder.fromUri("https://www.googleapis.com/youtube/v3/playlists")
 				.queryParam("part", "snippet")
 				.queryParam("key", googleAuthKey)

commafeed-server/src/main/java/com/commafeed/backend/feed/FeedFetcher.java

@@ -10,9 +10,11 @@ import org.apache.commons.lang3.StringUtils;
 
 import com.commafeed.backend.Digests;
 import com.commafeed.backend.HttpGetter;
+import com.commafeed.backend.HttpGetter.HostNotAllowedException;
 import com.commafeed.backend.HttpGetter.HttpRequest;
 import com.commafeed.backend.HttpGetter.HttpResult;
 import com.commafeed.backend.HttpGetter.NotModifiedException;
+import com.commafeed.backend.HttpGetter.SchemeNotAllowedException;
 import com.commafeed.backend.HttpGetter.TooManyRequestsException;
 import com.commafeed.backend.feed.parser.FeedParser;
 import com.commafeed.backend.feed.parser.FeedParserResult;
@@ -41,8 +43,8 @@ public class FeedFetcher {
 	}
 
 	public FeedFetcherResult fetch(String feedUrl, boolean extractFeedUrlFromHtml, String lastModified, String eTag,
-			Instant lastPublishedDate, String lastContentHash)
+			Instant lastPublishedDate, String lastContentHash) throws FeedException, IOException, NotModifiedException,
-			throws FeedException, IOException, NotModifiedException, TooManyRequestsException {
+			TooManyRequestsException, SchemeNotAllowedException, HostNotAllowedException {
 		log.debug("Fetching feed {}", feedUrl);
 
 		HttpResult result = getter.get(HttpRequest.builder(feedUrl).lastModified(lastModified).eTag(eTag).build());

commafeed-server/src/main/resources/application.properties

@@ -50,6 +50,7 @@ quarkus.native.add-all-charsets=true
 %test.commafeed.users.allow-registrations=true
 %test.commafeed.password-recovery-enabled=true
 %test.commafeed.http-client.cache.enabled=false
+%test.commafeed.http-client.block-local-addresses=false
 %test.commafeed.database.cleanup.entries-max-age=0
 %test.commafeed.feed-refresh.force-refresh-cooldown-duration=1m
 

commafeed-server/src/test/java/com/commafeed/backend/HttpGetterTest.java

@@ -236,7 +236,7 @@ class HttpGetterTest {
 	}
 
 	@Test
-	void cacheSubsequentCalls() throws IOException, NotModifiedException, TooManyRequestsException {
+	void cacheSubsequentCalls() throws Exception {
 		AtomicInteger calls = new AtomicInteger();
 
 		this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(req -> {
@@ -302,17 +302,16 @@ class HttpGetterTest {
 	class Compression {
 
 		@Test
-		void deflate() throws IOException, NotModifiedException, TooManyRequestsException {
+		void deflate() throws Exception {
 			supportsCompression("deflate", DeflaterOutputStream::new);
 		}
 
 		@Test
-		void gzip() throws IOException, NotModifiedException, TooManyRequestsException {
+		void gzip() throws Exception {
 			supportsCompression("gzip", GZIPOutputStream::new);
 		}
 
-		void supportsCompression(String encoding, CompressionOutputStreamFunction compressionOutputStreamFunction)
+		void supportsCompression(String encoding, CompressionOutputStreamFunction compressionOutputStreamFunction) throws Exception {
-				throws IOException, NotModifiedException, TooManyRequestsException {
 			String body = "my body";
 
 			HttpGetterTest.this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(req -> {
@@ -340,4 +339,64 @@ class HttpGetterTest {
 
 	}
 
+	@Nested
+	class SchemeNotAllowed {
+		@Test
+		void file() {
+			Assertions.assertThrows(HttpGetter.SchemeNotAllowedException.class, () -> getter.get("file://localhost"));
+		}
+
+		@Test
+		void ftp() {
+			Assertions.assertThrows(HttpGetter.SchemeNotAllowedException.class, () -> getter.get("ftp://localhost"));
+		}
+	}
+
+	@Nested
+	class HostNotAllowed {
+
+		@BeforeEach
+		void init() {
+			Mockito.when(config.httpClient().blockLocalAddresses()).thenReturn(true);
+			getter = new HttpGetter(config, () -> NOW, Mockito.mock(CommaFeedVersion.class), Mockito.mock(MetricRegistry.class));
+		}
+
+		@Test
+		void localhost() {
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://localhost"));
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://127.0.0.1"));
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://2130706433"));
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://0x7F.0x00.0x00.0X01"));
+		}
+
+		@Test
+		void zero() {
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://0.0.0.0"));
+		}
+
+		@Test
+		void linkLocal() {
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://169.254.12.34"));
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://169.254.169.254"));
+		}
+
+		@Test
+		void multicast() {
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://224.2.3.4"));
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://239.255.255.254"));
+		}
+
+		@Test
+		void privateIpv4Ranges() {
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://10.0.0.1"));
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://172.16.0.1"));
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://192.168.0.1"));
+		}
+
+		@Test
+		void privateIpv6Ranges() {
+			Assertions.assertThrows(HttpGetter.HostNotAllowedException.class, () -> getter.get("http://fd12:3456:789a:1::1"));
+		}
+	}
+
 }

commafeed-server/src/test/java/com/commafeed/e2e/ReadingIT.java

@@ -3,6 +3,7 @@ package com.commafeed.e2e;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.hc.core5.http.HttpStatus;