Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a filter for applying Android socket tags #2423

Merged
merged 36 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
31f5ed1
Implement Android Socket Tagging
RyanTheOptimist Jul 14, 2022
2fe087f
Working
RyanTheOptimist Jul 14, 2022
9c571da
format
RyanTheOptimist Jul 14, 2022
6c337f5
cleanup
RyanTheOptimist Jul 14, 2022
dd1990e
fix
RyanTheOptimist Jul 15, 2022
3fe5055
Add config to botstrap and trigger in test
RyanTheOptimist Jul 15, 2022
30669c0
Merge branch 'main' into SocketTag
RyanTheOptimist Jul 18, 2022
838b687
Add tagSocket from cronet
RyanTheOptimist Jul 19, 2022
3f1b8fa
format
RyanTheOptimist Jul 19, 2022
3b70695
address comments
RyanTheOptimist Jul 20, 2022
285bb29
Add config knobs
RyanTheOptimist Jul 22, 2022
e8ce39b
format
RyanTheOptimist Jul 22, 2022
702781a
Fix cronet
RyanTheOptimist Jul 22, 2022
bf91e03
Fix EnvoyConfigurationTest.kt
RyanTheOptimist Jul 22, 2022
4f69f6f
Address comments
RyanTheOptimist Jul 25, 2022
8308110
format
RyanTheOptimist Jul 25, 2022
97302cd
Address comments
RyanTheOptimist Jul 27, 2022
6221682
Format
RyanTheOptimist Jul 27, 2022
18030a7
Comment
RyanTheOptimist Jul 28, 2022
f409a57
Merge branch 'main' into SocketTag
RyanTheOptimist Aug 1, 2022
ec5ae8f
Update library/common/extensions/filters/http/socket_tag/filter.proto
RyanTheOptimist Aug 3, 2022
daf58c6
Update library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt
RyanTheOptimist Aug 3, 2022
fe92d93
Merge branch 'main' into SocketTagOld
RyanTheOptimist Aug 3, 2022
fd83daa
Simplify test
RyanTheOptimist Aug 3, 2022
c539773
Address comments
RyanTheOptimist Aug 3, 2022
07af4a8
Rename
RyanTheOptimist Aug 3, 2022
6b44c79
Format
RyanTheOptimist Aug 3, 2022
4539347
Spelling
RyanTheOptimist Aug 3, 2022
99a1d03
Address comments
RyanTheOptimist Aug 10, 2022
4eebefe
Address comments
RyanTheOptimist Aug 11, 2022
ec65417
Format
RyanTheOptimist Aug 11, 2022
a1febff
add "org/chromium/net/*"
RyanTheOptimist Aug 19, 2022
dbff49a
Merge branch 'main' into SocketTag
RyanTheOptimist Aug 19, 2022
e38a859
use find_class
RyanTheOptimist Aug 19, 2022
4f40e1d
Address more comments
RyanTheOptimist Aug 22, 2022
633704b
Format!
RyanTheOptimist Aug 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bazel/android_artifacts.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def _create_classes_jar(name, manifest, android_library):
classes_dir=$$(mktemp -d)
echo "Creating classes.jar from $(SRCS)"
pushd $$classes_dir
unzip $$original_directory/$(SRCS) "io/envoyproxy/*" "META-INF/" > /dev/null
unzip $$original_directory/$(SRCS) "io/envoyproxy/*" "org/chromium/net/*" "META-INF/" > /dev/null
zip -r classes.jar * > /dev/null
popd
cp $$classes_dir/classes.jar $@
Expand Down
11 changes: 11 additions & 0 deletions docs/root/api/starting_envoy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,17 @@ Specify whether to enable transparent response Brotli decompression. Defaults to
// Swift
builder.enableBrotli(true)

~~~~~~~~~~~~~~~~~~~~~~~
``enableSocketTagging``
~~~~~~~~~~~~~~~~~~~~~~~

Specify whether to enable support for Android socket tagging. Defaults to false.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Specify whether to enable support for Android socket tagging. Defaults to false.
Specify whether to enable support for Android socket tagging. Unavailable on iOS. Defaults to false.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this note to make it clear that it's no intended for iOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, whoops. I missed your suggestion earlier. Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not want to be annoying but i still do not see it being updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? Hm. If I go to https:/envoyproxy/envoy-mobile/pull/2423/files I see:


~~~~~~~~~~~~~~~~~~~~~~~
``enableSocketTagging``
~~~~~~~~~~~~~~~~~~~~~~~

Specify whether to enable support for Android socket tagging. Unavailable on iOS. Defaults to false.

**Example**::

  // Kotlin
  builder.enableSocketTagging(true)

I wonder if this is one of the many different ways that the github UI hides comments and changes by default :(


**Example**::

// Kotlin
builder.enableSocketTagging(true)

~~~~~~~~~~~~~~~~~~~~~~~~~~
``enableInterfaceBinding``
~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Features:
- api: add option to control whether Envoy should drain connections after a soft DNS refresh completes. (:issue:`#2225 <2225>`, :issue:`#2242 <2242>`)
- api: add option to disable the gzip decompressor. (:issue: `#2321 <2321>`) (:issue: `#2349 <2349>`)
- api: add option to enable the brotli decompressor. (:issue `#2342 <2342>`) (:issue: `#2349 <2349>`)
- api: add option to enable socket tagging. (:issue `#1512 <1521>`)
RyanTheOptimist marked this conversation as resolved.
Show resolved Hide resolved
- configuration: enable h2 ping by default. (:issue: `#2270 <2270>`)
- android: enable the filtering of unroutable families by default. (:issues: `#2267 <2267>`)
- instrumentation: add timers and warnings to platform-provided callbacks (:issue: `#2300 <2300>`)
Expand Down
1 change: 1 addition & 0 deletions envoy_build_config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ envoy_cc_library(
"@envoy_mobile//library/common/extensions/filters/http/network_configuration:config",
"@envoy_mobile//library/common/extensions/filters/http/platform_bridge:config",
"@envoy_mobile//library/common/extensions/filters/http/route_cache_reset:config",
"@envoy_mobile//library/common/extensions/filters/http/socket_tag:config",
"@envoy_mobile//library/common/extensions/retry/options/network_configuration:config",
],
)
Expand Down
11 changes: 11 additions & 0 deletions library/cc/engine_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ EngineBuilder& EngineBuilder::enableBrotli(bool brotli_on) {
return *this;
}

EngineBuilder& EngineBuilder::enableSocketTagging(bool socket_tagging_on) {
this->socket_tagging_filter_ = socket_tagging_on;
return *this;
}

std::string EngineBuilder::generateConfigStr() {
#if defined(__APPLE__)
std::string dns_resolver_name = "envoy.network.dns_resolver.apple";
Expand Down Expand Up @@ -184,6 +189,12 @@ std::string EngineBuilder::generateConfigStr() {
&config_template_);
}

if (this->socket_tagging_filter_) {
absl::StrReplaceAll(
{{"#{custom_filters}", absl::StrCat("#{custom_filters}\n", socket_tag_config_insert)}},
&config_template_);
}

config_builder << config_template_;

auto config_str = config_builder.str();
Expand Down
2 changes: 2 additions & 0 deletions library/cc/engine_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class EngineBuilder {
EngineBuilder& setStreamIdleTimeoutSeconds(int stream_idle_timeout_seconds);
EngineBuilder& enableGzip(bool gzip_on);
EngineBuilder& enableBrotli(bool brotli_on);
EngineBuilder& enableSocketTagging(bool socket_tagging_on);
RyanTheOptimist marked this conversation as resolved.
Show resolved Hide resolved

// this is separated from build() for the sake of testability
std::string generateConfigStr();
Expand Down Expand Up @@ -81,6 +82,7 @@ class EngineBuilder {
int per_try_idle_timeout_seconds_ = 15;
bool gzip_filter_ = true;
bool brotli_filter_ = false;
bool socket_tagging_filter_ = false;

absl::flat_hash_map<std::string, KeyValueStoreSharedPtr> key_value_stores_{};

Expand Down
6 changes: 6 additions & 0 deletions library/common/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ const char* brotli_config_insert = R"(
ignore_no_transform_header: true
)";

const char* socket_tag_config_insert = R"(
- name: envoy.filters.http.socket_tag
typed_config:
"@type": type.googleapis.com/envoymobile.extensions.filters.http.socket_tag.SocketTag
)";

// clang-format off
const std::string config_header = R"(
!ignore default_defs:
Expand Down
4 changes: 4 additions & 0 deletions library/common/config/templates.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ extern const char* gzip_config_insert;
*/
extern const char* brotli_config_insert;

/* Insert that enables a socket tagging filter.
*/
extern const char* socket_tag_config_insert;

/**
* Insert that enables the route cache reset filter in the filter chain.
* Should only be added when the route cache should be cleared on every request
Expand Down
42 changes: 42 additions & 0 deletions library/common/extensions/filters/http/socket_tag/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
load(
"@envoy//bazel:envoy_build_system.bzl",
"envoy_cc_extension",
"envoy_extension_package",
"envoy_proto_library",
)

licenses(["notice"]) # Apache 2

envoy_extension_package()

envoy_proto_library(
name = "filter",
srcs = ["filter.proto"],
)

envoy_cc_extension(
name = "socket_tag_filter_lib",
srcs = ["filter.cc"],
hdrs = ["filter.h"],
repository = "@envoy",
deps = [
":filter_cc_proto",
"//library/common/http:internal_headers_lib",
"//library/common/network:socket_tag_socket_option_lib",
"//library/common/types:c_types_lib",
"@envoy//envoy/http:codes_interface",
"@envoy//envoy/http:filter_interface",
"@envoy//source/extensions/filters/http/common:pass_through_filter_lib",
],
)

envoy_cc_extension(
name = "config",
srcs = ["config.cc"],
hdrs = ["config.h"],
repository = "@envoy",
deps = [
":socket_tag_filter_lib",
"@envoy//source/extensions/filters/http/common:factory_base_lib",
],
)
33 changes: 33 additions & 0 deletions library/common/extensions/filters/http/socket_tag/config.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#include "library/common/extensions/filters/http/socket_tag/config.h"

#include <stdint.h>
#include <sys/types.h>
#include <unistd.h>

#include "envoy/network/listen_socket.h"

#include "library/common/extensions/filters/http/socket_tag/filter.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketTag {

Http::FilterFactoryCb SocketTagFilterFactory::createFilterFactoryFromProtoTyped(
const envoymobile::extensions::filters::http::socket_tag::SocketTag& /*proto_config*/,
const std::string&, Server::Configuration::FactoryContext& /*context*/) {

return [](Http::FilterChainFactoryCallbacks& callbacks) -> void {
callbacks.addStreamFilter(std::make_shared<SocketTagFilter>());
};
}

/**
* Static registration for the SocketTag filter. @see NamedHttpFilterConfigFactory.
*/
REGISTER_FACTORY(SocketTagFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory);

} // namespace SocketTag
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
32 changes: 32 additions & 0 deletions library/common/extensions/filters/http/socket_tag/config.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#include <string>

#include "source/extensions/filters/http/common/factory_base.h"

#include "library/common/extensions/filters/http/socket_tag/filter.pb.h"
#include "library/common/extensions/filters/http/socket_tag/filter.pb.validate.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketTag {

/**
* Config registration for the socket tag filter. @see NamedHttpFilterConfigFactory.
*/
class SocketTagFilterFactory
: public Common::FactoryBase<envoymobile::extensions::filters::http::socket_tag::SocketTag> {
public:
SocketTagFilterFactory() : FactoryBase("socket_tag") {}

private:
::Envoy::Http::FilterFactoryCb createFilterFactoryFromProtoTyped(
const envoymobile::extensions::filters::http::socket_tag::SocketTag& config,
const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;
};

DECLARE_FACTORY(SocketTagFilterFactory);

} // namespace SocketTag
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
44 changes: 44 additions & 0 deletions library/common/extensions/filters/http/socket_tag/filter.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#include "library/common/extensions/filters/http/socket_tag/filter.h"

#include "envoy/server/filter_config.h"

#include "library/common/network/socket_tag_socket_option_impl.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketTag {

Http::FilterHeadersStatus SocketTagFilter::decodeHeaders(Http::RequestHeaderMap& request_headers,
bool) {
static auto socket_tag_header = Http::LowerCaseString("x-envoy-mobile-socket-tag");
Http::RequestHeaderMap::GetResult header = request_headers.get(socket_tag_header);
if (header.empty()) {
return Http::FilterHeadersStatus::Continue;
}

// The x-envoy-mobile-socket-tag header must contain a pair of number separated by a comma, e.g.:
// x-envoy-mobile-socket-tag: 123,456
// The first number contains the UID and the second contains the traffic stats tag.
std::string tag_string(header[0]->value().getStringView());
std::pair<std::string, std::string> data = absl::StrSplit(tag_string, ',');
uid_t uid;
uint32_t traffic_stats_tag;
if (!absl::SimpleAtoi(data.first, &uid) || !absl::SimpleAtoi(data.second, &traffic_stats_tag)) {
decoder_callbacks_->sendLocalReply(
Http::Code::BadRequest,
absl::StrCat("Invalid x-envoy-mobile-socket-tag header: ", tag_string), nullptr,
absl::nullopt, "");
return Http::FilterHeadersStatus::StopIteration;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StopIteration or sendLocalReply with error? I'd think we wanted the latter? do we have an e2e test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. sendLocalReply sounds like a good idea. I think I've done the needful.

There are no end-to-end tests because the socket tagging code, like isCleartextPermitted, needs to run in a simulator and I think we're waiting on Lyft to sort that out?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh but this is a failure case based on getting bad headers from the API - you should totally be able to test sending an invalid request with the filter configured, and getting the local response you expect, so let's do that!

you could also optionally add a test for the happy path, which would create the custom transport socket and make sure hashing works, if we don't have that tested upstream. even if the tagging is a no-op it would at least cover that the memory model works.

also clean up the commented out return value above this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok. Java tests added and removed the commented out code.

I looked at test/common/integration/client_integration_test.cc, which we discussed offline. I wasn't quite sure how to configure it to add the socket tagging support. Maybe we can discuss that offine?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I opened a GH issue to track the work related to adding support for running tests on Android emulator An ability to run tests on Android emulator #2450.
  2. In the meantime you can run Envoy Mobile example app to verify your changes following the setup steps from https://envoymobile.io/docs/envoy-mobile/latest/development/debugging/android_local.html
  3. It's strongly recommended that you enable your changes in Android experimental app. You can do this following this change from one of the previous PRs https:/envoyproxy/envoy-mobile/pull/2379/files#diff-b15ec59c29644d8c6089ad0ea7370859a690563f75d02c819679270b61ab9520R60. This experimental app as runs on an Android emulator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the issue! I'll give the demo app a shot. Updated test/kotlin/apps/experimental/MainActivity.kt per that previous commit. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I am happy to help with the setup of the demo app if you run into any issues with it. It's also possible that the documentation is lacking or making some assumptions - let me know and I am happy to help.

Re experimental app - I think that you can go even one step further (sorry for not pointing this out in my previous comment) and adding a socket-tag header to requests performed by the experimental app. This way you would get an end-to-end confirmation that requests with socket-tag go through (we verify whether our experimental example is getting 200 from the server). You could add the header somewhere in here

}
auto options = std::make_shared<Network::Socket::Options>();
options->push_back(std::make_shared<Network::SocketTagSocketOptionImpl>(uid, traffic_stats_tag));
decoder_callbacks_->addUpstreamSocketOptions(options);
request_headers.remove(socket_tag_header);
return Http::FilterHeadersStatus::Continue;
}

} // namespace SocketTag
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
30 changes: 30 additions & 0 deletions library/common/extensions/filters/http/socket_tag/filter.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#pragma once

#include "envoy/http/filter.h"

#include "source/common/common/logger.h"
#include "source/extensions/filters/http/common/pass_through_filter.h"

#include "library/common/extensions/filters/http/socket_tag/filter.pb.h"
#include "library/common/types/c_types.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketTag {

/**
* Filter to set upstream socket tags based on a request header.
* See: https://source.android.com/devices/tech/datausage/tags-explained
*/
class SocketTagFilter final : public Http::PassThroughFilter,
public Logger::Loggable<Logger::Id::filter> {
public:
// Http::PassThroughDecoderFilter
Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& request_headers, bool) override;
};

} // namespace SocketTag
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
13 changes: 13 additions & 0 deletions library/common/extensions/filters/http/socket_tag/filter.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
syntax = "proto3";

package envoymobile.extensions.filters.http.socket_tag;

// Configuration for the socket tagging filer. This filter uses data from the
// x-envoy-mobile-socket-tag request header to apply an Android Socket Tag to the upstream
// socket.
// See: https://source.android.com/devices/tech/datausage/tags-explained
// See: https://developer.android.com/reference/android/net/TrafficStats#setThreadStatsTag(int)
// See: https://developer.android.com/reference/android/net/TrafficStats#setThreadStatsUid(int)
// See: https://developer.android.com/reference/android/net/TrafficStats#tagSocket(java.net.Socket)
message SocketTag {
alyssawilk marked this conversation as resolved.
Show resolved Hide resolved
}
14 changes: 14 additions & 0 deletions library/common/jni/android_jni_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,17 @@ bool is_cleartext_permitted(absl::string_view hostname) {
return true;
#endif
}

void tag_socket(int ifd, int uid, int tag) {
RyanTheOptimist marked this conversation as resolved.
Show resolved Hide resolved
#if defined(__ANDROID_API__)
JNIEnv* env = get_env();
jclass jcls_AndroidNetworkLibrary = find_class("org/chromium/net/AndroidNetworkLibrary");
jmethodID jmid_tagSocket =
env->GetStaticMethodID(jcls_AndroidNetworkLibrary, "tagSocket", "(III)V");
env->CallStaticVoidMethod(jcls_AndroidNetworkLibrary, jmid_tagSocket, ifd, uid, tag);
#else
UNREFERENCED_PARAMETER(ifd);
UNREFERENCED_PARAMETER(uid);
UNREFERENCED_PARAMETER(tag);
#endif
}
6 changes: 6 additions & 0 deletions library/common/jni/android_jni_utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@
* For other platforms simply returns true.
*/
bool is_cleartext_permitted(absl::string_view hostname);

/* For android, calls up through JNI to apply
* host.
* For other platforms simply returns true.
*/
void tag_socket(int ifd, int uid, int tag);
6 changes: 6 additions & 0 deletions library/common/jni/jni_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ Java_io_envoyproxy_envoymobile_engine_JniLibrary_brotliConfigInsert(JNIEnv* env,
return result;
}

extern "C" JNIEXPORT jstring JNICALL
Java_io_envoyproxy_envoymobile_engine_JniLibrary_socketTagConfigInsert(JNIEnv* env, jclass) {
jstring result = env->NewStringUTF(socket_tag_config_insert);
return result;
}

extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_recordCounterInc(
JNIEnv* env,
jclass, // class
Expand Down
14 changes: 14 additions & 0 deletions library/common/network/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,20 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "socket_tag_socket_option_lib",
srcs = ["socket_tag_socket_option_impl.cc"],
hdrs = ["socket_tag_socket_option_impl.h"],
repository = "@envoy",
deps = [
"//library/common/jni:android_jni_utility_lib",
"@envoy//envoy/network:address_interface",
"@envoy//source/common/common:scalar_to_byte_vector_lib",
"@envoy//source/common/network:socket_option_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
)

envoy_cc_library(
name = "synthetic_address_lib",
hdrs = ["synthetic_address_impl.h"],
Expand Down
Loading