Skip to content

Commit

Permalink
envoy: allowing alt-svc from IPs, fixing HTTP/3 cert bypass (envoypro…
Browse files Browse the repository at this point in the history
…xy#35551)

This adds a real HTTP/3 test for Envoy mobile's Cronet APIs.
In order for this to work it fixes 2 issues in Envoy, the first that the
connection grid didn't do lookups for ip based hostname. The second that
ACCEPT_UNTRUSTED didn't fully work for QUIC.

Risk Level: low
Testing: e2e test
Docs Changes: n/a
Release Notes: inline
[Optional Runtime guard:] guarded both functional changes

---------

Signed-off-by: Alyssa Wilk <[email protected]>
  • Loading branch information
alyssawilk authored Aug 6, 2024
1 parent dae96ec commit aa8d0f7
Show file tree
Hide file tree
Showing 19 changed files with 260 additions and 28 deletions.
8 changes: 8 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ minor_behavior_changes:
change: |
Added support for :ref:`connection_pool_per_downstream_connection
<envoy_v3_api_field_config.cluster.v3.Cluster.connection_pool_per_downstream_connection>` flag in tcp connection pool.
- area: http3
change: |
The ACCEPT_UNTRUSTED option now works more consistently for HTTP/3 requests. This change is
guarded by ``envoy.reloadable_features.extend_h3_accept_untrusted``.
- area: http3
change: |
HTTP/3 alt-svc headers will now be respected from IP-address-based hostnames. This change is
guarded by runtime guard ``envoy.reloadable_features.allow_alt_svc_for_ips``.
- area: lua
change: |
When Lua script executes httpCall, backpressure is exercised when receiving body from downstream client. This behavior can be reverted
Expand Down
11 changes: 6 additions & 5 deletions mobile/test/common/integration/test_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
namespace Envoy {
namespace {

std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> baseProxyConfig(bool http) {
std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> baseProxyConfig(bool http, int port) {
std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> bootstrap =
std::make_unique<envoy::config::bootstrap::v3::Bootstrap>();

Expand All @@ -40,7 +40,7 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> baseProxyConfig(bool ht
auto* base_address = listener->mutable_address();
base_address->mutable_socket_address()->set_protocol(envoy::config::core::v3::SocketAddress::TCP);
base_address->mutable_socket_address()->set_address("127.0.0.1");
base_address->mutable_socket_address()->set_port_value(0);
base_address->mutable_socket_address()->set_port_value(port);

envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager hcm;
hcm.set_stat_prefix("remote hcm");
Expand Down Expand Up @@ -137,7 +137,8 @@ TestServer::TestServer()
Envoy::ExtensionRegistry::registerFactories();
}

void TestServer::start(TestServerType type) {
void TestServer::start(TestServerType type, int port) {
port_ = port;
ASSERT(!upstream_);
// pre-setup: see https://github.com/envoyproxy/envoy/blob/main/test/test_runner.cc
Logger::Context logging_state(spdlog::level::level_enum::err,
Expand Down Expand Up @@ -180,7 +181,7 @@ void TestServer::start(TestServerType type) {
test_server_ = IntegrationTestServer::create(
"", Network::Address::IpVersion::v4, nullptr, nullptr, {}, time_system_, *api_, false,
absl::nullopt, Server::FieldValidationConfig(), 1, std::chrono::seconds(1),
Server::DrainStrategy::Gradual, nullptr, false, false, baseProxyConfig(true));
Server::DrainStrategy::Gradual, nullptr, false, false, baseProxyConfig(true, port_));
test_server_->waitUntilListenersReady();
ENVOY_LOG_MISC(debug, "Http proxy is now running");
return;
Expand All @@ -195,7 +196,7 @@ void TestServer::start(TestServerType type) {
test_server_ = IntegrationTestServer::create(
"", Network::Address::IpVersion::v4, nullptr, nullptr, {}, time_system_, *api_, false,
absl::nullopt, Server::FieldValidationConfig(), 1, std::chrono::seconds(1),
Server::DrainStrategy::Gradual, nullptr, false, false, baseProxyConfig(false));
Server::DrainStrategy::Gradual, nullptr, false, false, baseProxyConfig(false, port_));
test_server_->waitUntilListenersReady();
ENVOY_LOG_MISC(debug, "Https proxy is now running");
return;
Expand Down
2 changes: 1 addition & 1 deletion mobile/test/common/integration/test_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class TestServer : public ListenerHooks {
/**
* Starts the test server. This function blocks until the test server is ready to accept requests.
*/
void start(TestServerType type);
void start(TestServerType type, int port = 0);

/**
* Shutdowns the server server. This function blocks until all the resources have been freed.
Expand Down
2 changes: 1 addition & 1 deletion mobile/test/common/integration/test_server_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ void start_server(Envoy::TestServerType test_server_type) {
weak_test_server_ = strong_test_server_;

if (auto server = test_server()) {
server->start(test_server_type);
server->start(test_server_type, 0);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ private HttpTestServer(long handle, String ipAddress, int port, String address)
* @param trailers the response headers
* @return the `HttpTestServer` instance
*/
public static native HttpTestServer start(int type, Map<String, String> headers, String body,
Map<String, String> trailers);
public static native HttpTestServer start(int type, int port, Map<String, String> headers,
String body, Map<String, String> trailers);

/**
* A convenience method to start the server with an empty response headers, body, and trailers.
Expand All @@ -64,6 +64,6 @@ public static native HttpTestServer start(int type, Map<String, String> headers,
* @return the `HttpTestServer` instance
*/
public static HttpTestServer start(int type) {
return start(type, Collections.emptyMap(), "", Collections.emptyMap());
return start(type, 0, Collections.emptyMap(), "", Collections.emptyMap());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void setUpEngine() throws Exception {
Map<String, String> headers = new HashMap<>();
headers.put("Cache-Control", "max-age=0");
headers.put("Content-Type", "text/plain");
httpTestServer = HttpTestServerFactory.start(HttpTestServerFactory.Type.HTTP3, headers,
httpTestServer = HttpTestServerFactory.start(HttpTestServerFactory.Type.HTTP3, 0, headers,
"This is a simple text file served by QUIC.\n",
Collections.emptyMap());

Expand Down
27 changes: 27 additions & 0 deletions mobile/test/java/org/chromium/net/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,33 @@ envoy_mobile_android_test(
],
)

envoy_mobile_android_test(
name = "cronet_http3_test",
srcs = [
"CronetHttp3Test.java",
],
native_deps = [
"//test/jni:libenvoy_jni_with_test_extensions.so",
] + select({
"@platforms//os:macos": [
"//test/jni:libenvoy_jni_with_test_extensions_jnilib",
],
"//conditions:default": [],
}),
native_lib_name = "envoy_jni_with_test_extensions",
test_class = "org.chromium.net.CronetHttp3Test",
deps = [
"//library/java/io/envoyproxy/envoymobile/engine:envoy_base_engine_lib",
"//library/java/io/envoyproxy/envoymobile/engine:envoy_engine_lib",
"//library/java/org/chromium/net",
"//library/java/org/chromium/net/impl:cronvoy",
"//library/kotlin/io/envoyproxy/envoymobile:envoy_interfaces_lib",
"//library/kotlin/io/envoyproxy/envoymobile:envoy_lib",
"//test/java/io/envoyproxy/envoymobile/engine/testing:http_test_server_factory_lib",
"//test/java/org/chromium/net/testing",
],
)

envoy_mobile_android_test(
name = "cronet_url_request_context_test",
srcs = [
Expand Down
139 changes: 139 additions & 0 deletions mobile/test/java/org/chromium/net/CronetHttp3Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package org.chromium.net;

import static org.chromium.net.testing.CronetTestRule.getContext;
import static org.junit.Assert.assertEquals;

import org.chromium.net.impl.CronvoyUrlRequestContext;
import io.envoyproxy.envoymobile.engine.EnvoyEngine;
import org.chromium.net.impl.CronvoyLogger;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.filters.SmallTest;
import org.chromium.net.impl.CronvoyUrlRequestContext;
import org.chromium.net.impl.NativeCronvoyEngineBuilderImpl;
import org.chromium.net.testing.CronetTestRule;
import org.chromium.net.testing.CronetTestRule.CronetTestFramework;
import org.chromium.net.testing.CronetTestRule.RequiresMinApi;
import org.chromium.net.testing.Feature;
import org.chromium.net.testing.TestUrlRequestCallback;
import org.chromium.net.testing.TestUrlRequestCallback.ResponseStep;
import io.envoyproxy.envoymobile.engine.JniLibrary;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import io.envoyproxy.envoymobile.engine.testing.HttpTestServerFactory;
import java.util.HashMap;
import java.util.Map;
import java.util.Collections;

/**
* Test CronetEngine with production HTTP/3 logic
*/
@RunWith(RobolectricTestRunner.class)
public class CronetHttp3Test {
@Rule public final CronetTestRule mTestRule = new CronetTestRule();

private static final String TAG = CronetHttp3Test.class.getSimpleName();

// URLs used for tests.

// If true, dump envoy logs on test completion.
// Ideally we could override this from the command line but that's TBD.
private boolean printEnvoyLogs = false;
// The HTTP/2 server, set up to alt-svc to the HTTP/3 server
private HttpTestServerFactory.HttpTestServer http2TestServer;
// The HTTP/3 server
private HttpTestServerFactory.HttpTestServer http3TestServer;
// An optional CronvoyLogger, set up if printEnvoyLogs is true.
private CronvoyLogger logger;
// The engine for this test.
private CronvoyUrlRequestContext cronvoyEngine;

@BeforeClass
public static void loadJniLibrary() {
JniLibrary.loadTestLibrary();
}

public void setUp(boolean setUpLogging) throws Exception {
// Set up the HTTP/3 server
Map<String, String> headers = new HashMap<>();
http3TestServer = HttpTestServerFactory.start(HttpTestServerFactory.Type.HTTP3, 0, headers,
"This is a simple text file served by QUIC.\n",
Collections.emptyMap());
// Next set up the HTTP/2 server, advertising HTTP/3 support for the HTTP/3 server
String altSvc = "h3=\":" + http3TestServer.getPort() + "\"; ma=86400";
headers.put("alt-svc", altSvc);
// Note that the HTTP/2 server must start on the same port as Envoy currently does not accept
// alt-svc with differing ports. This may cause problems if this UDP port is in use at which
// point listening on 127.0.0.N where N!=1 may improve flakiness.
http2TestServer = HttpTestServerFactory.start(
HttpTestServerFactory.Type.HTTP2_WITH_TLS, http3TestServer.getPort(), headers,
"This is a simple text file served by QUIC.\n", Collections.emptyMap());

// Optionally, set up logging. This will slow down the tests a bit but make debugging much
// easier.
if (setUpLogging) {
logger = new CronvoyLogger() {
@Override
public void log(int logLevel, String message) {
System.out.print(message);
}
};
}
}

@After
public void tearDown() throws Exception {
// Shut down Envoy and the test servers.
cronvoyEngine.shutdown();
http2TestServer.shutdown();
http3TestServer.shutdown();
}

@Test
@SmallTest
@Feature({"Cronet"})
public void testInitEngineAndStartRequest() throws Exception {
// Ideally we could override this from the command line but that's TBD.
setUp(printEnvoyLogs);

// Set up the Envoy engine.
NativeCronvoyEngineBuilderImpl nativeCronetEngineBuilder =
new NativeCronvoyEngineBuilderImpl(ApplicationProvider.getApplicationContext());
if (printEnvoyLogs) {
nativeCronetEngineBuilder.setLogger(logger);
nativeCronetEngineBuilder.setLogLevel(EnvoyEngine.LogLevel.TRACE);
}
// Make sure the handshake will work despite lack of real certs.
nativeCronetEngineBuilder.setMockCertVerifierForTesting();
cronvoyEngine = new CronvoyUrlRequestContext(nativeCronetEngineBuilder);

// Do a request to https://127.0.0.1:test_server_port/
TestUrlRequestCallback callback1 = new TestUrlRequestCallback();
String newUrl = "https://" + http2TestServer.getAddress() + "/";
UrlRequest.Builder urlRequestBuilder =
cronvoyEngine.newUrlRequestBuilder(newUrl, callback1, callback1.getExecutor());
urlRequestBuilder.build().start();
callback1.blockForDone();

// Make sure the request succeeded. It should go out over HTTP/2 as it's the first
// request and HTTP/3 support is not established.
assertEquals(200, callback1.mResponseInfo.getHttpStatusCode());
assertEquals("h2", callback1.mResponseInfo.getNegotiatedProtocol());

// Set up a second request, which will hopefully go out over HTTP/3 due to alt-svc
// advertisement.
TestUrlRequestCallback callback2 = new TestUrlRequestCallback();
UrlRequest.Builder urlRequestBuilder2 =
cronvoyEngine.newUrlRequestBuilder(newUrl, callback2, callback2.getExecutor());
urlRequestBuilder2.build().start();
callback2.blockForDone();

// Verify the second request used HTTP/3
assertEquals(200, callback2.mResponseInfo.getHttpStatusCode());
assertEquals("h3", callback2.mResponseInfo.getNegotiatedProtocol());
}
}
2 changes: 1 addition & 1 deletion mobile/test/jni/jni_http_proxy_test_server_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Java_io_envoyproxy_envoymobile_engine_testing_HttpProxyTestServerFactory_start(J

Envoy::ExtensionRegistry::registerFactories();
Envoy::TestServer* test_server = new Envoy::TestServer();
test_server->start(static_cast<Envoy::TestServerType>(type));
test_server->start(static_cast<Envoy::TestServerType>(type), 0);

jclass java_http_proxy_server_factory_class = jni_helper.findClass(
"io/envoyproxy/envoymobile/engine/testing/HttpProxyTestServerFactory$HttpProxyTestServer");
Expand Down
5 changes: 3 additions & 2 deletions mobile/test/jni/jni_http_test_server_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* /* reserved */) {

extern "C" JNIEXPORT jobject JNICALL
Java_io_envoyproxy_envoymobile_engine_testing_HttpTestServerFactory_start(
JNIEnv* env, jclass, jint type, jobject headers, jstring body, jobject trailers) {
JNIEnv* env, jclass, jint type, jint requested_port, jobject headers, jstring body,
jobject trailers) {
Envoy::JNI::JniHelper jni_helper(env);

Envoy::ExtensionRegistry::registerFactories();
Envoy::TestServer* test_server = new Envoy::TestServer();
test_server->start(static_cast<Envoy::TestServerType>(type));
test_server->start(static_cast<Envoy::TestServerType>(type), requested_port);

auto cpp_headers = Envoy::JNI::javaMapToCppMap(jni_helper, headers);
auto cpp_body = Envoy::JNI::javaStringToCppString(jni_helper, body);
Expand Down
1 change: 1 addition & 0 deletions mobile/test/kotlin/integration/ReceiveDataTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class ReceiveDataTest {
httpTestServer =
HttpTestServerFactory.start(
HttpTestServerFactory.Type.HTTP2_WITH_TLS,
0,
mapOf(),
"data",
mapOf()
Expand Down
1 change: 1 addition & 0 deletions mobile/test/kotlin/integration/ReceiveTrailersTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class ReceiveTrailersTest {
httpTestServer =
HttpTestServerFactory.start(
HttpTestServerFactory.Type.HTTP2_WITH_TLS,
0,
mapOf(),
"data",
mapOf(TRAILER_NAME to TRAILER_VALUE)
Expand Down
1 change: 1 addition & 0 deletions mobile/test/kotlin/integration/SendDataTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class SendDataTest {
httpTestServer =
HttpTestServerFactory.start(
HttpTestServerFactory.Type.HTTP2_WITH_TLS,
0,
mapOf(),
"data",
mapOf()
Expand Down
15 changes: 11 additions & 4 deletions source/common/http/conn_pool_grid.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,19 @@ absl::string_view describePool(const ConnectionPool::Instance& pool) {

static constexpr uint32_t kDefaultTimeoutMs = 300;

std::string getSni(const Network::TransportSocketOptionsConstSharedPtr& options,
Network::UpstreamTransportSocketFactory& transport_socket_factory) {
std::string getTargetHostname(const Network::TransportSocketOptionsConstSharedPtr& options,
Upstream::HostConstSharedPtr& host) {
if (options && options->serverNameOverride().has_value()) {
return options->serverNameOverride().value();
}
return std::string(transport_socket_factory.defaultServerNameIndication());
std::string default_sni =
std::string(host->transportSocketFactory().defaultServerNameIndication());
if (!default_sni.empty() ||
!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.allow_alt_svc_for_ips")) {
return default_sni;
}
// If there's no configured SNI the hostname is probably an IP address. Return it here.
return host->hostname();
}

} // namespace
Expand Down Expand Up @@ -297,7 +304,7 @@ ConnectivityGrid::ConnectivityGrid(
time_source_(time_source), alternate_protocols_(alternate_protocols),
quic_stat_names_(quic_stat_names), scope_(scope),
// TODO(RyanTheOptimist): Figure out how scheme gets plumbed in here.
origin_("https", getSni(transport_socket_options, host_->transportSocketFactory()),
origin_("https", getTargetHostname(transport_socket_options, host_),
host_->address()->ip()->port()),
quic_info_(quic_info), priority_(priority) {
// ProdClusterManagerFactory::allocateConnPool verifies the protocols are HTTP/1, HTTP/2 and
Expand Down
Loading

0 comments on commit aa8d0f7

Please sign in to comment.