From 9a2f6fa8aaa3dd5c8828ac79e0a537770bee5c96 Mon Sep 17 00:00:00 2001 From: Craig Andrews Date: Mon, 26 Apr 2021 17:42:42 -0400 Subject: [PATCH 1/4] Use the CONNECT method URI as host fallback When a request is made for proxy interception using the `CONNECT` method but not including a `host` header, then a `NullPointerException` is thrown in `com.browserup.bup.mitm.manager.ImpersonatingMitmManager.getHostnameImpersonatingSslContext(String, SSLSession)` because the `hostnameToImpersonate` is null. As a fallback, use the CONNECT URI as a host. --- .../java/com/browserup/bup/util/HttpUtil.java | 41 ++++++++++++++++++- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java b/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java index 55f0f2d81..c49f2027b 100644 --- a/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java +++ b/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java @@ -2,6 +2,7 @@ import com.google.common.net.HostAndPort; import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpRequest; import java.net.URI; @@ -37,6 +38,11 @@ public static String getHostFromRequest(HttpRequest httpRequest) { host = parseHostHeader(httpRequest, false); } + // if there was not a Host header, and the method is CONNECT, use that as the host + if (host == null || host.isEmpty()) { + host = hostFromConnect(httpRequest, false); + } + return host; } @@ -48,15 +54,26 @@ public static String getHostFromRequest(HttpRequest httpRequest) { * @return host and port of the request */ public static String getHostAndPortFromRequest(HttpRequest httpRequest) { + String host = null; if (startsWithHttpOrHttps(httpRequest.uri())) { try { - return getHostAndPortFromUri(httpRequest.uri()); + host = getHostAndPortFromUri(httpRequest.uri()); } catch (URISyntaxException e) { // the URI could not be parsed, so return the host and port in the Host header } } - return parseHostHeader(httpRequest, true); + // if there was no host in the URI, attempt to grab the host from the Host header + if (host == null || host.isEmpty()) { + host = parseHostHeader(httpRequest, true); + } + + // if there was not Host header, and the method is CONNECT, use that as the host + if (host == null || host.isEmpty()) { + host = hostFromConnect(httpRequest, true); + } + + return host; } /** @@ -120,4 +137,24 @@ private static String parseHostHeader(HttpRequest httpRequest, boolean includePo return null; } } + + /** + * Retrieves the host and, optionally, the port from the specified request's URI if the method is CONNECT. + * + * @param httpRequest HTTP request + * @param includePort when true, include the port + * @return the host and, optionally, the port specified in the request's URI + */ + private static String hostFromConnect(HttpRequest httpRequest, boolean includePort) { + if (HttpMethod.CONNECT.equals(httpRequest.method())) { + if (includePort) { + return httpRequest.uri(); + } else { + HostAndPort parsedHostAndPort = HostAndPort.fromString(httpRequest.uri()); + return parsedHostAndPort.getHost(); + } + } else { + return null; + } + } } From f4eeb4784c6d56d6b22e442a194367e9df620ad7 Mon Sep 17 00:00:00 2001 From: Craig Andrews Date: Fri, 13 May 2022 13:05:36 -0400 Subject: [PATCH 2/4] Test HTTP/1.0 support in ImpersonatingMitmManager --- .../LittleProxyIntegrationTest.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java b/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java index 56364425b..f218f1977 100644 --- a/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java +++ b/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java @@ -6,6 +6,7 @@ import io.netty.handler.codec.http.HttpResponse; import com.browserup.bup.mitm.manager.ImpersonatingMitmManager; import org.apache.http.HttpHost; +import org.apache.http.HttpVersion; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; @@ -94,6 +95,67 @@ public HttpObject serverToProxyResponse(HttpObject httpObject) { proxyServer.abort(); } + @Test + public void testLittleProxyMitmHttp1_0() throws IOException, InterruptedException { + final AtomicBoolean interceptedGetRequest = new AtomicBoolean(); + final AtomicBoolean interceptedGetResponse = new AtomicBoolean(); + + HttpFiltersSource filtersSource = new HttpFiltersSourceAdapter() { + @Override + public HttpFilters filterRequest(HttpRequest originalRequest) { + return new HttpFiltersAdapter(originalRequest) { + @Override + public HttpResponse proxyToServerRequest(HttpObject httpObject) { + if (httpObject instanceof HttpRequest) { + HttpRequest httpRequest = (HttpRequest) httpObject; + if (httpRequest.getMethod().equals(HttpMethod.GET)) { + interceptedGetRequest.set(true); + } + } + + return super.proxyToServerRequest(httpObject); + } + + @Override + public HttpObject serverToProxyResponse(HttpObject httpObject) { + if (httpObject instanceof HttpResponse) { + HttpResponse httpResponse = (HttpResponse) httpObject; + if (httpResponse.getStatus().code() == 200) { + interceptedGetResponse.set(true); + } + } + return super.serverToProxyResponse(httpObject); + } + }; + } + }; + + ImpersonatingMitmManager mitmManager = ImpersonatingMitmManager.builder().build(); + + HttpProxyServer proxyServer = DefaultHttpProxyServer.bootstrap() + .withPort(0) + .withManInTheMiddle(mitmManager) + .withFiltersSource(filtersSource) + .start(); + + try (CloseableHttpClient httpClient = getNewHttpClient(proxyServer.getListenAddress().getPort())) { + HttpGet httpGet = new HttpGet("https://www.google.com"); + httpGet.setProtocolVersion(HttpVersion.HTTP_1_0); + try (CloseableHttpResponse response = httpClient.execute(httpGet)) { + assertEquals("Expected to receive an HTTP 200 from http://www.google.com", 200, response.getStatusLine().getStatusCode()); + + EntityUtils.consume(response.getEntity()); + } + } + + Thread.sleep(500); + + assertTrue("Expected HttpFilters to successfully intercept the HTTP GET request", interceptedGetRequest.get()); + assertTrue("Expected HttpFilters to successfully intercept the server's response to the HTTP GET", interceptedGetResponse.get()); + + proxyServer.abort(); + } + /** * Creates an HTTP client that trusts all upstream servers and uses a localhost proxy on the specified port. */ From cd2305630ed3f6672b1a9d9510900e3333c3ed46 Mon Sep 17 00:00:00 2001 From: Valery Yatsynovich Date: Sun, 15 May 2022 01:01:03 +0300 Subject: [PATCH 3/4] Polish `HttpUtil` to avoid duplications --- .../java/com/browserup/bup/util/HttpUtil.java | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java b/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java index c49f2027b..e2f70bce3 100644 --- a/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java +++ b/browserup-proxy-mitm/src/main/java/com/browserup/bup/util/HttpUtil.java @@ -33,17 +33,7 @@ public static String getHostFromRequest(HttpRequest httpRequest) { } } - // if there was no host in the URI, attempt to grab the host from the Host header - if (host == null || host.isEmpty()) { - host = parseHostHeader(httpRequest, false); - } - - // if there was not a Host header, and the method is CONNECT, use that as the host - if (host == null || host.isEmpty()) { - host = hostFromConnect(httpRequest, false); - } - - return host; + return parseHost(host, httpRequest, false); } /** @@ -63,17 +53,7 @@ public static String getHostAndPortFromRequest(HttpRequest httpRequest) { } } - // if there was no host in the URI, attempt to grab the host from the Host header - if (host == null || host.isEmpty()) { - host = parseHostHeader(httpRequest, true); - } - - // if there was not Host header, and the method is CONNECT, use that as the host - if (host == null || host.isEmpty()) { - host = hostFromConnect(httpRequest, true); - } - - return host; + return parseHost(host, httpRequest, true); } /** @@ -114,6 +94,20 @@ public static String getHostAndPortFromUri(String uriString) throws URISyntaxExc } } + private static String parseHost(String host, HttpRequest httpRequest, boolean includePort) { + // if there was no host in the URI, attempt to grab the host from the Host header + if (isEmpty(host)) { + host = parseHostHeader(httpRequest, includePort); + } + + // if there was not a Host header, and the method is CONNECT, use that as the host + if (isEmpty(host)) { + host = hostFromConnect(httpRequest, includePort); + } + + return host; + } + /** * Retrieves the host and, optionally, the port from the specified request's Host header. * @@ -149,12 +143,14 @@ private static String hostFromConnect(HttpRequest httpRequest, boolean includePo if (HttpMethod.CONNECT.equals(httpRequest.method())) { if (includePort) { return httpRequest.uri(); - } else { - HostAndPort parsedHostAndPort = HostAndPort.fromString(httpRequest.uri()); - return parsedHostAndPort.getHost(); } - } else { - return null; + HostAndPort parsedHostAndPort = HostAndPort.fromString(httpRequest.uri()); + return parsedHostAndPort.getHost(); } + return null; + } + + private static boolean isEmpty(String str) { + return str == null || str.isEmpty(); } } From 4f9c333410fd6f4b5f701fb7f0b784e5a553b88a Mon Sep 17 00:00:00 2001 From: Valery Yatsynovich Date: Sun, 15 May 2022 01:02:46 +0300 Subject: [PATCH 4/4] Polish `LittleProxyIntegrationTest` to avoid duplications --- .../LittleProxyIntegrationTest.java | 64 +++---------------- 1 file changed, 8 insertions(+), 56 deletions(-) diff --git a/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java b/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java index f218f1977..6f466a080 100644 --- a/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java +++ b/browserup-proxy-mitm/src/test/java/com/browserup/bup/mitm/integration/LittleProxyIntegrationTest.java @@ -7,6 +7,7 @@ import com.browserup.bup.mitm.manager.ImpersonatingMitmManager; import org.apache.http.HttpHost; import org.apache.http.HttpVersion; +import org.apache.http.ProtocolVersion; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; @@ -38,65 +39,16 @@ public class LittleProxyIntegrationTest { @Test public void testLittleProxyMitm() throws IOException, InterruptedException { - final AtomicBoolean interceptedGetRequest = new AtomicBoolean(); - final AtomicBoolean interceptedGetResponse = new AtomicBoolean(); - - HttpFiltersSource filtersSource = new HttpFiltersSourceAdapter() { - @Override - public HttpFilters filterRequest(HttpRequest originalRequest) { - return new HttpFiltersAdapter(originalRequest) { - @Override - public HttpResponse proxyToServerRequest(HttpObject httpObject) { - if (httpObject instanceof HttpRequest) { - HttpRequest httpRequest = (HttpRequest) httpObject; - if (httpRequest.getMethod().equals(HttpMethod.GET)) { - interceptedGetRequest.set(true); - } - } - - return super.proxyToServerRequest(httpObject); - } - - @Override - public HttpObject serverToProxyResponse(HttpObject httpObject) { - if (httpObject instanceof HttpResponse) { - HttpResponse httpResponse = (HttpResponse) httpObject; - if (httpResponse.getStatus().code() == 200) { - interceptedGetResponse.set(true); - } - } - return super.serverToProxyResponse(httpObject); - } - }; - } - }; - - ImpersonatingMitmManager mitmManager = ImpersonatingMitmManager.builder().build(); - - HttpProxyServer proxyServer = DefaultHttpProxyServer.bootstrap() - .withPort(0) - .withManInTheMiddle(mitmManager) - .withFiltersSource(filtersSource) - .start(); - - try (CloseableHttpClient httpClient = getNewHttpClient(proxyServer.getListenAddress().getPort())) { - try (CloseableHttpResponse response = httpClient.execute(new HttpGet("https://www.google.com"))) { - assertEquals("Expected to receive an HTTP 200 from http://www.google.com", 200, response.getStatusLine().getStatusCode()); - - EntityUtils.consume(response.getEntity()); - } - } - - Thread.sleep(500); - - assertTrue("Expected HttpFilters to successfully intercept the HTTP GET request", interceptedGetRequest.get()); - assertTrue("Expected HttpFilters to successfully intercept the server's response to the HTTP GET", interceptedGetResponse.get()); - - proxyServer.abort(); + testLittleProxyMitm(null); } @Test public void testLittleProxyMitmHttp1_0() throws IOException, InterruptedException { + testLittleProxyMitm(HttpVersion.HTTP_1_0); + } + + private void testLittleProxyMitm(ProtocolVersion protocolVersion) throws IOException, InterruptedException + { final AtomicBoolean interceptedGetRequest = new AtomicBoolean(); final AtomicBoolean interceptedGetResponse = new AtomicBoolean(); @@ -140,7 +92,7 @@ public HttpObject serverToProxyResponse(HttpObject httpObject) { try (CloseableHttpClient httpClient = getNewHttpClient(proxyServer.getListenAddress().getPort())) { HttpGet httpGet = new HttpGet("https://www.google.com"); - httpGet.setProtocolVersion(HttpVersion.HTTP_1_0); + httpGet.setProtocolVersion(protocolVersion); try (CloseableHttpResponse response = httpClient.execute(httpGet)) { assertEquals("Expected to receive an HTTP 200 from http://www.google.com", 200, response.getStatusLine().getStatusCode());