From 236d57a550380d8a5808f38cb0c8bbb005ec29b0 Mon Sep 17 00:00:00 2001 From: babenhauserheide Date: Thu, 3 Sep 2020 00:14:12 +0200 Subject: [PATCH 1/4] Add parsing of Forwarded header for schemeHostAndPort again. WIP. --- src/freenet/clients/http/FProxyToadlet.java | 36 +++++++++++++++++-- .../utils/UriFilterProxyHeaderParser.java | 13 +++---- .../utils/UriFilterProxyHeaderParserTest.java | 7 ++-- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/freenet/clients/http/FProxyToadlet.java b/src/freenet/clients/http/FProxyToadlet.java index 2f5a86d79df..a25d007b7f0 100644 --- a/src/freenet/clients/http/FProxyToadlet.java +++ b/src/freenet/clients/http/FProxyToadlet.java @@ -15,8 +15,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Date; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import freenet.client.DefaultMIMETypes; @@ -996,11 +998,41 @@ private String getSchemeHostAndPort(ToadletContext ctx) { // get uri host and headers MultiValueTable headers = ctx.getHeaders(); - // TODO: parse the Forwarded header, too. Skipped here to reduce the scope. + Map forwarded = parseForwardedHeader(headers.get("forwarded")); String uriScheme = ctx.getUri().getScheme(); String uriHost = ctx.getUri().getHost(); - return UriFilterProxyHeaderParser.parse(fProxyPort, fProxyBindTo, uriScheme, uriHost, headers).toString(); + return UriFilterProxyHeaderParser.parse(fProxyPort, fProxyBindTo, uriScheme, uriHost, headers, forwarded).toString(); + } + + private Map parseForwardedHeader(String forwarded) { + if (forwarded == null || forwarded.trim().isEmpty()) { + return new HashMap<>(); + } + Map headerParams = new HashMap<>(); + + // if a multi-value header is given, only use the first value. + int indexOfComma = forwarded.indexOf(','); + if (indexOfComma != -1) { + forwarded = forwarded.substring(0, indexOfComma); + } + boolean hasAtLeastOneKey = forwarded.indexOf('=') != -1; + boolean hasMultipleKeys = forwarded.indexOf(';') != -1; + String[] fields; + if (hasMultipleKeys) { + fields = forwarded.split(";"); + } else if (hasAtLeastOneKey) { + fields = new String[]{ forwarded }; + } else { + return headerParams; + } + for (String field : fields) { + if (field.indexOf('=') != 1) { + String[] keyAndValue = field.split("="); + headerParams.put(keyAndValue[0], keyAndValue[1]); + } + } + return headerParams; } private boolean isBrowser(String ua) { diff --git a/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java b/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java index 6b3fe54eb15..0ef9e33ef25 100644 --- a/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java +++ b/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java @@ -3,6 +3,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -17,7 +18,8 @@ public static SchemeAndHostWithPort parse ( Option fProxyBindToConfig, String uriScheme, String uriHost, - MultiValueTable headers + MultiValueTable headers, + Map forwarded ) { Set safeProtocols = new HashSet<>(Arrays.asList("http", "https")); @@ -38,14 +40,13 @@ public static SchemeAndHostWithPort parse ( safeHosts.addAll(safeHosts.stream() .map(host -> host + ":" + port) .collect(Collectors.toList())); - // check uri host and headers - String protocol = headers.containsKey("x-forwarded-proto") + String protocol = forwarded.getOrDefault("proto", headers.containsKey("x-forwarded-proto") ? headers.get("x-forwarded-proto") - : uriScheme != null && !uriScheme.trim().isEmpty() ? uriScheme : "http"; - String host = headers.containsKey("x-forwarded-host") + : uriScheme != null && !uriScheme.trim().isEmpty() ? uriScheme : "http"); + String host = forwarded.getOrDefault("host", headers.containsKey("x-forwarded-host") ? headers.get("x-forwarded-host") - : uriHost != null && !uriHost.trim().isEmpty() ? uriHost : headers.get("host"); + : uriHost != null && !uriHost.trim().isEmpty() ? uriHost : headers.get("host")); // check allow list if (!safeProtocols.contains(protocol)) { protocol = "http"; diff --git a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java index bac35e4ca44..814953949d0 100644 --- a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java +++ b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java @@ -2,11 +2,11 @@ import static org.junit.Assert.assertTrue; +import java.util.HashMap; + import org.junit.Test; import freenet.config.Config; -import freenet.config.InvalidConfigValueException; -import freenet.config.NodeNeedRestartException; import freenet.config.StringOption; import freenet.support.MultiValueTable; import freenet.support.api.StringCallback; @@ -287,7 +287,8 @@ private void testUriPrefixMatchesExpected( fakeBindToOption(fProxyBindTo), uriScheme, uriHost, - headers) + headers, + new HashMap<>()) .toString(); assertTrue( String.format( From 7ca6adb0644072430e7822189fced82d1043e3b5 Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Sun, 22 May 2022 10:39:54 +0200 Subject: [PATCH 2/4] refactor: move forwarded parsing into the UriFilterProxyHeaderParser --- src/freenet/clients/http/FProxyToadlet.java | 35 +---------------- .../utils/UriFilterProxyHeaderParser.java | 39 +++++++++++++++++-- .../utils/UriFilterProxyHeaderParserTest.java | 5 +-- 3 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/freenet/clients/http/FProxyToadlet.java b/src/freenet/clients/http/FProxyToadlet.java index a25d007b7f0..4b6f6c27425 100644 --- a/src/freenet/clients/http/FProxyToadlet.java +++ b/src/freenet/clients/http/FProxyToadlet.java @@ -15,10 +15,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Date; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import freenet.client.DefaultMIMETypes; @@ -998,41 +996,10 @@ private String getSchemeHostAndPort(ToadletContext ctx) { // get uri host and headers MultiValueTable headers = ctx.getHeaders(); - Map forwarded = parseForwardedHeader(headers.get("forwarded")); String uriScheme = ctx.getUri().getScheme(); String uriHost = ctx.getUri().getHost(); - return UriFilterProxyHeaderParser.parse(fProxyPort, fProxyBindTo, uriScheme, uriHost, headers, forwarded).toString(); - } - - private Map parseForwardedHeader(String forwarded) { - if (forwarded == null || forwarded.trim().isEmpty()) { - return new HashMap<>(); - } - Map headerParams = new HashMap<>(); - - // if a multi-value header is given, only use the first value. - int indexOfComma = forwarded.indexOf(','); - if (indexOfComma != -1) { - forwarded = forwarded.substring(0, indexOfComma); - } - boolean hasAtLeastOneKey = forwarded.indexOf('=') != -1; - boolean hasMultipleKeys = forwarded.indexOf(';') != -1; - String[] fields; - if (hasMultipleKeys) { - fields = forwarded.split(";"); - } else if (hasAtLeastOneKey) { - fields = new String[]{ forwarded }; - } else { - return headerParams; - } - for (String field : fields) { - if (field.indexOf('=') != 1) { - String[] keyAndValue = field.split("="); - headerParams.put(keyAndValue[0], keyAndValue[1]); - } - } - return headerParams; + return UriFilterProxyHeaderParser.parse(fProxyPort, fProxyBindTo, uriScheme, uriHost, headers).toString(); } private boolean isBrowser(String ua) { diff --git a/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java b/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java index 0ef9e33ef25..95b4452e55d 100644 --- a/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java +++ b/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java @@ -1,6 +1,7 @@ package freenet.clients.http.utils; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -13,14 +14,13 @@ public class UriFilterProxyHeaderParser { private UriFilterProxyHeaderParser() {} - public static SchemeAndHostWithPort parse ( + public static SchemeAndHostWithPort parse( Option fProxyPortConfig, Option fProxyBindToConfig, String uriScheme, String uriHost, - MultiValueTable headers, - Map forwarded - ) { + MultiValueTable headers + ) { Set safeProtocols = new HashSet<>(Arrays.asList("http", "https")); List bindToHosts = Arrays.stream(fProxyBindToConfig.getValueString().split(",")) @@ -41,6 +41,7 @@ public static SchemeAndHostWithPort parse ( .map(host -> host + ":" + port) .collect(Collectors.toList())); // check uri host and headers + Map forwarded = parseForwardedHeader(headers.get("forwarded")); String protocol = forwarded.getOrDefault("proto", headers.containsKey("x-forwarded-proto") ? headers.get("x-forwarded-proto") : uriScheme != null && !uriScheme.trim().isEmpty() ? uriScheme : "http"); @@ -73,4 +74,34 @@ public String toString() { return scheme + "://" + host; } } + + private static Map parseForwardedHeader(String forwarded) { + if (forwarded == null || forwarded.trim().isEmpty()) { + return new HashMap<>(); + } + Map headerParams = new HashMap<>(); + + // if a multi-value header is given, only use the first value. + int indexOfComma = forwarded.indexOf(','); + if (indexOfComma != -1) { + forwarded = forwarded.substring(0, indexOfComma); + } + boolean hasAtLeastOneKey = forwarded.indexOf('=') != -1; + boolean hasMultipleKeys = forwarded.indexOf(';') != -1; + String[] fields; + if (hasMultipleKeys) { + fields = forwarded.split(";"); + } else if (hasAtLeastOneKey) { + fields = new String[]{ forwarded }; + } else { + return headerParams; + } + for (String field : fields) { + if (field.indexOf('=') != 1) { + String[] keyAndValue = field.split("="); + headerParams.put(keyAndValue[0], keyAndValue[1]); + } + } + return headerParams; + } } diff --git a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java index 814953949d0..ea3ff2f4800 100644 --- a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java +++ b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java @@ -2,8 +2,6 @@ import static org.junit.Assert.assertTrue; -import java.util.HashMap; - import org.junit.Test; import freenet.config.Config; @@ -287,8 +285,7 @@ private void testUriPrefixMatchesExpected( fakeBindToOption(fProxyBindTo), uriScheme, uriHost, - headers, - new HashMap<>()) + headers) .toString(); assertTrue( String.format( From ca7cb3e1acec0c9322112d6a8bd2035263346c08 Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Sat, 14 Oct 2023 12:43:53 +0200 Subject: [PATCH 3/4] Fix: parsing must be case-insensitive --- .../clients/http/utils/UriFilterProxyHeaderParser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java b/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java index 95b4452e55d..e6fa283cc6b 100644 --- a/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java +++ b/src/freenet/clients/http/utils/UriFilterProxyHeaderParser.java @@ -75,7 +75,7 @@ public String toString() { } } - private static Map parseForwardedHeader(String forwarded) { + static Map parseForwardedHeader(String forwarded) { if (forwarded == null || forwarded.trim().isEmpty()) { return new HashMap<>(); } @@ -99,7 +99,7 @@ private static Map parseForwardedHeader(String forwarded) { for (String field : fields) { if (field.indexOf('=') != 1) { String[] keyAndValue = field.split("="); - headerParams.put(keyAndValue[0], keyAndValue[1]); + headerParams.put(keyAndValue[0].toLowerCase(), keyAndValue[1]); } } return headerParams; From 6d5118469824143e4c581eb88b9877d9dc802857 Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Sat, 14 Oct 2023 12:44:03 +0200 Subject: [PATCH 4/4] Add tests for forwarded Header parsing --- .../utils/UriFilterProxyHeaderParserTest.java | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java index ea3ff2f4800..43077170196 100644 --- a/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java +++ b/test/freenet/clients/http/utils/UriFilterProxyHeaderParserTest.java @@ -1,7 +1,12 @@ package freenet.clients.http.utils; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import java.util.HashMap; +import java.util.Map; + +import org.hamcrest.Matchers; import org.junit.Test; import freenet.config.Config; @@ -273,6 +278,60 @@ public void disallowedUriWithAllowedHostButDisallowedPortIsIgnored() "http://127.0.0.1:8888"); } + @Test + public void forwardedHeaderProvidesIp() { + String forwarded = "for=192.0.2.172"; + Map parsedHeader = UriFilterProxyHeaderParser.parseForwardedHeader( + forwarded); + assertThat(parsedHeader.keySet(), Matchers.containsInAnyOrder("for"));; + assertThat(parsedHeader.values(), Matchers.containsInAnyOrder("192.0.2.172")); + } + + @Test + public void forwardedHeaderProvidesIpv6AndPort() { + String forwarded = "for=[2001:db8:cafe::17]:4711"; + Map parsedHeader = UriFilterProxyHeaderParser.parseForwardedHeader( + forwarded); + assertThat(parsedHeader.keySet(), Matchers.containsInAnyOrder("for"));; + assertThat(parsedHeader.values(), Matchers.containsInAnyOrder("[2001:db8:cafe::17]:4711")); + } + + @Test + public void forwardedHeaderProvidesIpCaseInsensitive() { + String forwarded = "For=192.0.2.172"; + Map parsedHeader = UriFilterProxyHeaderParser.parseForwardedHeader( + forwarded); + assertThat(parsedHeader.keySet(), Matchers.containsInAnyOrder("for"));; + assertThat(parsedHeader.values(), Matchers.containsInAnyOrder("192.0.2.172")); + } + + @Test + public void forwardedHeaderProvidesIpProtoBy() { + String forwarded = "for=192.0.2.60;proto=http;by=203.0.113.43"; + Map parsedHeader = UriFilterProxyHeaderParser.parseForwardedHeader( + forwarded); + assertThat(parsedHeader.keySet(), Matchers.containsInAnyOrder("for", "proto", "by"));; + assertThat(parsedHeader.values(), Matchers.containsInAnyOrder("192.0.2.60", "http", "203.0.113.43")); + } + + @Test + public void forwardedHeaderProvidesIpProtoByOnlyAllowTheFirst() { + String forwarded = "for=192.0.2.60;proto=http;by=203.0.113.43, for=198.51.100.17;proto=http;by=203.0.113.43"; + Map parsedHeader = UriFilterProxyHeaderParser.parseForwardedHeader( + forwarded); + assertThat(parsedHeader.keySet(), Matchers.containsInAnyOrder("for", "proto", "by"));; + assertThat(parsedHeader.values(), Matchers.containsInAnyOrder("192.0.2.60", "http", "203.0.113.43")); + } + + @Test + public void forwardedHeaderProvidesIpOnlyAllowTheFirst() { + String forwarded = "for=192.0.2.60, for=198.51.100.17;proto=http;by=203.0.113.43"; + Map parsedHeader = UriFilterProxyHeaderParser.parseForwardedHeader( + forwarded); + assertThat(parsedHeader.keySet(), Matchers.containsInAnyOrder("for"));; + assertThat(parsedHeader.values(), Matchers.containsInAnyOrder("192.0.2.60")); + } + private void testUriPrefixMatchesExpected( String fProxyPort, String fProxyBindTo,