diff --git a/src/cloud/config_manager/controllers/vizier_feature_flags.go b/src/cloud/config_manager/controllers/vizier_feature_flags.go index 23025a3e89e..6802f5616c1 100644 --- a/src/cloud/config_manager/controllers/vizier_feature_flags.go +++ b/src/cloud/config_manager/controllers/vizier_feature_flags.go @@ -42,11 +42,6 @@ var availableFeatureFlags = []*featureFlag{ VizierFlagName: "PL_PROFILER_JAVA_SYMBOLS", DefaultValue: true, }, - { - FeatureFlagName: "access-tls-socket-fd-via-syscall", - VizierFlagName: "PL_ACCESS_TLS_SOCKET_FD_VIA_SYSCALL", - DefaultValue: false, - }, } // NewVizierFeatureFlagClient creates a LaunchDarkly feature flag client if the SDK key is provided, diff --git a/src/stirling/source_connectors/socket_tracer/bcc_bpf/socket_trace.c b/src/stirling/source_connectors/socket_tracer/bcc_bpf/socket_trace.c index c42a4a24c23..ba5df955dde 100644 --- a/src/stirling/source_connectors/socket_tracer/bcc_bpf/socket_trace.c +++ b/src/stirling/source_connectors/socket_tracer/bcc_bpf/socket_trace.c @@ -1058,9 +1058,7 @@ int syscall__probe_ret_write(struct pt_regs* ctx) { // Syscalls that aren't exclusively used for networking must be // validated to be a sock_event before propagating a socket fd to the // tls tracing probes - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, write_args->fd); - } + propagate_fd_to_user_space_call(id, write_args->fd); process_syscall_data(ctx, id, kEgress, write_args, bytes_count); } @@ -1072,9 +1070,7 @@ int syscall__probe_ret_write(struct pt_regs* ctx) { int syscall__probe_entry_send(struct pt_regs* ctx, int sockfd, char* buf, size_t len) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); // Stash arguments. struct data_args_t write_args = {}; @@ -1124,9 +1120,7 @@ int syscall__probe_ret_read(struct pt_regs* ctx) { // Syscalls that aren't exclusively used for networking must be // validated to be a sock_event before propagating a socket fd to the // tls tracing probes - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, read_args->fd); - } + propagate_fd_to_user_space_call(id, read_args->fd); process_syscall_data(ctx, id, kIngress, read_args, bytes_count); } @@ -1138,9 +1132,7 @@ int syscall__probe_ret_read(struct pt_regs* ctx) { int syscall__probe_entry_recv(struct pt_regs* ctx, int sockfd, char* buf, size_t len) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); // Stash arguments. struct data_args_t read_args = {}; @@ -1172,9 +1164,7 @@ int syscall__probe_entry_sendto(struct pt_regs* ctx, int sockfd, char* buf, size const struct sockaddr* dest_addr, socklen_t addrlen) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); // Stash arguments. if (dest_addr != NULL) { @@ -1235,9 +1225,7 @@ int syscall__probe_entry_recvfrom(struct pt_regs* ctx, int sockfd, char* buf, si struct sockaddr* src_addr, socklen_t* addrlen) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); // Stash arguments. if (src_addr != NULL) { @@ -1283,9 +1271,7 @@ int syscall__probe_entry_sendmsg(struct pt_regs* ctx, int sockfd, const struct user_msghdr* msghdr) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); if (msghdr != NULL) { // Stash arguments. @@ -1333,9 +1319,7 @@ int syscall__probe_entry_sendmmsg(struct pt_regs* ctx, int sockfd, struct mmsghd unsigned int vlen) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); // TODO(oazizi): Right now, we only trace the first message in a sendmmsg() call. if (msgvec != NULL && vlen >= 1) { @@ -1389,9 +1373,7 @@ int syscall__probe_ret_sendmmsg(struct pt_regs* ctx) { int syscall__probe_entry_recvmsg(struct pt_regs* ctx, int sockfd, struct user_msghdr* msghdr) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); if (msghdr != NULL) { // Stash arguments. @@ -1441,9 +1423,7 @@ int syscall__probe_entry_recvmmsg(struct pt_regs* ctx, int sockfd, struct mmsghd unsigned int vlen) { uint64_t id = bpf_get_current_pid_tgid(); - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, sockfd); - } + propagate_fd_to_user_space_call(id, sockfd); // TODO(oazizi): Right now, we only trace the first message in a recvmmsg() call. if (msgvec != NULL && vlen >= 1) { @@ -1518,9 +1498,7 @@ int syscall__probe_ret_writev(struct pt_regs* ctx) { // Syscalls that aren't exclusively used for networking must be // validated to be a sock_event before propagating a socket fd to the // tls tracing probes - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, write_args->fd); - } + propagate_fd_to_user_space_call(id, write_args->fd); process_syscall_data_vecs(ctx, id, kEgress, write_args, bytes_count); } @@ -1553,9 +1531,7 @@ int syscall__probe_ret_readv(struct pt_regs* ctx) { // Syscalls that aren't exclusively used for networking must be // validated to be a sock_event before propagating a socket fd to the // tls tracing probes - if (ACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL) { - propagate_fd_to_user_space_call(id, read_args->fd); - } + propagate_fd_to_user_space_call(id, read_args->fd); process_syscall_data_vecs(ctx, id, kIngress, read_args, bytes_count); } diff --git a/src/stirling/source_connectors/socket_tracer/openssl_trace_bpf_test.cc b/src/stirling/source_connectors/socket_tracer/openssl_trace_bpf_test.cc index 160a8f69834..e256ebd3900 100644 --- a/src/stirling/source_connectors/socket_tracer/openssl_trace_bpf_test.cc +++ b/src/stirling/source_connectors/socket_tracer/openssl_trace_bpf_test.cc @@ -97,10 +97,10 @@ struct TraceRecords { std::vector remote_address; }; -template -class BaseOpenSSLTraceTest : public SocketTraceBPFTestFixture { +template +class OpenSSLTraceTest : public SocketTraceBPFTestFixture { protected: - BaseOpenSSLTraceTest() { + OpenSSLTraceTest() { // Run the nginx HTTPS server. // The container runner will make sure it is in the ready state before unblocking. // Stirling will run after this unblocks, as part of SocketTraceBPFTest SetUp(). @@ -111,12 +111,6 @@ class BaseOpenSSLTraceTest : public SocketTraceBPFTestFixture tablets = @@ -178,26 +172,12 @@ http::Record GetExpectedHTTPRecord() { using OpenSSLServerImplementations = Types; -using OpenSSLServerNestedSyscallFDImplementations = - Types; - -template -using OpenSSLTraceTest = BaseOpenSSLTraceTest; - -template -using OpenSSLTraceNestedSyscallFD = BaseOpenSSLTraceTest; - -#define OPENSSL_TYPED_TEST(TestCase, CodeBlock) \ - TYPED_TEST(OpenSSLTraceTest, TestCase) \ - CodeBlock TYPED_TEST(OpenSSLTraceNestedSyscallFD, TestCase) \ - CodeBlock TYPED_TEST_SUITE(OpenSSLTraceTest, OpenSSLServerImplementations); -TYPED_TEST_SUITE(OpenSSLTraceNestedSyscallFD, OpenSSLServerNestedSyscallFDImplementations); -OPENSSL_TYPED_TEST(ssl_capture_curl_client, { +TYPED_TEST(OpenSSLTraceTest, ssl_capture_curl_client) { this->StartTransferDataThread(); // Make an SSL request with curl. @@ -218,9 +198,9 @@ OPENSSL_TYPED_TEST(ssl_capture_curl_client, { EXPECT_THAT(records.http_records, UnorderedElementsAre(EqHTTPRecord(expected_record))); EXPECT_THAT(records.remote_address, UnorderedElementsAre(StrEq("127.0.0.1"))); -}) +} -OPENSSL_TYPED_TEST(ssl_capture_ruby_client, { +TYPED_TEST(OpenSSLTraceTest, ssl_capture_ruby_client) { this->StartTransferDataThread(); // Make multiple requests and make sure we capture all of them. @@ -261,9 +241,9 @@ OPENSSL_TYPED_TEST(ssl_capture_ruby_client, { EqHTTPRecord(expected_record))); EXPECT_THAT(records.remote_address, UnorderedElementsAre(StrEq("127.0.0.1"), StrEq("127.0.0.1"), StrEq("127.0.0.1"))); -}) +} -OPENSSL_TYPED_TEST(ssl_capture_node_client, { +TYPED_TEST(OpenSSLTraceTest, ssl_capture_node_client) { this->StartTransferDataThread(); // Make an SSL request with the client. @@ -280,7 +260,7 @@ OPENSSL_TYPED_TEST(ssl_capture_node_client, { EXPECT_THAT(records.http_records, UnorderedElementsAre(EqHTTPRecord(expected_record))); EXPECT_THAT(records.remote_address, UnorderedElementsAre(StrEq("127.0.0.1"))); -}) +} } // namespace stirling } // namespace px diff --git a/src/stirling/source_connectors/socket_tracer/socket_trace_connector.cc b/src/stirling/source_connectors/socket_tracer/socket_trace_connector.cc index d30dfa3d006..4c2caa7d434 100644 --- a/src/stirling/source_connectors/socket_tracer/socket_trace_connector.cc +++ b/src/stirling/source_connectors/socket_tracer/socket_trace_connector.cc @@ -147,13 +147,6 @@ DEFINE_uint32(datastream_buffer_retention_size, DEFINE_uint64(max_body_bytes, gflags::Uint64FromEnv("PL_STIRLING_MAX_BODY_BYTES", 512), "The maximum number of bytes in the body of protocols like HTTP"); -DEFINE_bool( - access_tls_socket_fd_via_syscall, - gflags::BoolFromEnv("PL_ACCESS_TLS_SOCKET_FD_VIA_SYSCALL", true), - "If true, stirling will identify a socket's fd based on the underlying syscall (read, write, " - "etc) while a user space tls function call occurs. When false, stirling attempts to access the " - "socket fd by walking user space data structures which may be brittle."); - OBJ_STRVIEW(socket_trace_bcc_script, socket_trace); namespace px { @@ -418,8 +411,6 @@ Status SocketTraceConnector::InitBPF() { absl::StrCat("-DENABLE_NATS_TRACING=", protocol_transfer_specs_[kProtocolNATS].enabled), absl::StrCat("-DENABLE_AMQP_TRACING=", protocol_transfer_specs_[kProtocolAMQP].enabled), absl::StrCat("-DENABLE_MONGO_TRACING=", "true"), - absl::StrCat("-DACCESS_TLS_SK_FD_VIA_ACTIVE_SYSCALL=", - FLAGS_access_tls_socket_fd_via_syscall), }; PX_RETURN_IF_ERROR(InitBPFProgram(socket_trace_bcc_script, defines)); diff --git a/src/stirling/source_connectors/socket_tracer/uprobe_manager.cc b/src/stirling/source_connectors/socket_tracer/uprobe_manager.cc index ff79daa1674..221cc0aacd4 100644 --- a/src/stirling/source_connectors/socket_tracer/uprobe_manager.cc +++ b/src/stirling/source_connectors/socket_tracer/uprobe_manager.cc @@ -276,6 +276,15 @@ StatusOr> FindHostPathForPIDLibs( HostPathForPIDPathSearchType::kSearchTypeEndsWith); } +enum class SSLSocketFDAccess { + // Specifies that a connection's socket fd will be identified by accessing struct members + // of the SSL struct exposed by OpenSSL's API when the SSL_write/SSL_read functions are called. + kUserSpaceOffsets, + // Specifies that a connection's socket fd will be identified based on the underlying syscall + // (read, write, etc) while a user space tls function is on the stack. + kNestedSyscall, +}; + // SSLLibMatcher allows customizing the search of shared object files // that need to be traced with the SSL_write and SSL_read uprobes. // In dynamically linked cases, it's likely that there are two @@ -285,6 +294,7 @@ struct SSLLibMatcher { std::string_view libssl; std::string_view libcrypto; HostPathForPIDPathSearchType search_type; + SSLSocketFDAccess socket_fd_access; }; constexpr char kLibSSL_1_1[] = "libssl.so.1.1"; @@ -296,11 +306,13 @@ static constexpr const auto kLibSSLMatchers = MakeArray({ .libssl = kLibSSL_1_1, .libcrypto = "libcrypto.so.1.1", .search_type = HostPathForPIDPathSearchType::kSearchTypeEndsWith, + .socket_fd_access = SSLSocketFDAccess::kNestedSyscall, }, SSLLibMatcher{ .libssl = kLibSSL_3, .libcrypto = "libcrypto.so.3", .search_type = HostPathForPIDPathSearchType::kSearchTypeEndsWith, + .socket_fd_access = SSLSocketFDAccess::kNestedSyscall, }, SSLLibMatcher{ // This must match independent of python version and INSTSONAME suffix @@ -308,11 +320,15 @@ static constexpr const auto kLibSSLMatchers = MakeArray({ .libssl = kLibPython, .libcrypto = kLibPython, .search_type = HostPathForPIDPathSearchType::kSearchTypeContains, + .socket_fd_access = SSLSocketFDAccess::kNestedSyscall, }, + // non BIO native TLS applications cannot be probed by accessing the socket fd + // within the underlying syscall. SSLLibMatcher{ .libssl = kLibNettyTcnativePrefix, .libcrypto = kLibNettyTcnativePrefix, .search_type = HostPathForPIDPathSearchType::kSearchTypeContains, + .socket_fd_access = SSLSocketFDAccess::kUserSpaceOffsets, }, }); @@ -332,6 +348,19 @@ ssl_source_t SSLSourceFromLib(std::string_view libssl) { return kSSLUnspecified; } +std::string ProbeFuncForSocketAccessMethod(std::string_view probe_fn, + SSLSocketFDAccess socket_fd_access) { + std::string probe_suffix = ""; + switch (socket_fd_access) { + case SSLSocketFDAccess::kUserSpaceOffsets: + break; + case SSLSocketFDAccess::kNestedSyscall: + probe_suffix = "_syscall_fd_access"; + } + + return absl::StrCat(probe_fn, probe_suffix); +} + // Return error if something unexpected occurs. // Return 0 if nothing unexpected, but there is nothing to deploy (e.g. no OpenSSL detected). StatusOr UProbeManager::AttachOpenSSLUProbesOnDynamicLib(uint32_t pid) { @@ -339,12 +368,6 @@ StatusOr UProbeManager::AttachOpenSSLUProbesOnDynamicLib(uint32_t pid) { const auto libssl = ssl_library_match.libssl; const auto libcrypto = ssl_library_match.libcrypto; - // TODO(ddelnano): The legacy tls tracing implementation does not support OpenSSL v3. - // Remove this once that implementation is removed in addition to the feature toggle. - if (!FLAGS_access_tls_socket_fd_via_syscall && absl::EndsWith(libssl, "so.3")) { - continue; - } - const std::vector lib_names = {libssl, libcrypto}; const auto search_type = ssl_library_match.search_type; @@ -373,7 +396,7 @@ StatusOr UProbeManager::AttachOpenSSLUProbesOnDynamicLib(uint32_t pid) { return error::Internal("libcrypto not found [path = $0]", container_libcrypto.string()); } - if (!FLAGS_access_tls_socket_fd_via_syscall || libssl == kLibNettyTcnativePrefix) { + if (libssl == kLibNettyTcnativePrefix) { auto fptr_manager = std::make_unique(container_libcrypto); PX_RETURN_IF_ERROR(UpdateOpenSSLSymAddrs(fptr_manager.get(), container_libcrypto, pid)); @@ -393,12 +416,8 @@ StatusOr UProbeManager::AttachOpenSSLUProbesOnDynamicLib(uint32_t pid) { openssl_source_map_->UpdateValue(pid, ssl_source); for (auto spec : kOpenSSLUProbes) { spec.binary_path = container_libssl.string(); - - // TODO(ddelnano): Remove this conditional logic once the new tls tracing - // implementation is the default. - if (FLAGS_access_tls_socket_fd_via_syscall && libssl != kLibNettyTcnativePrefix) { - spec.probe_fn = absl::Substitute("$0_syscall_fd_access", spec.probe_fn); - } + spec.probe_fn = + ProbeFuncForSocketAccessMethod(spec.probe_fn, ssl_library_match.socket_fd_access); PX_RETURN_IF_ERROR(LogAndAttachUProbe(spec)); } diff --git a/src/stirling/source_connectors/socket_tracer/uprobe_manager.h b/src/stirling/source_connectors/socket_tracer/uprobe_manager.h index fe3a317109a..ca4ca5785bf 100644 --- a/src/stirling/source_connectors/socket_tracer/uprobe_manager.h +++ b/src/stirling/source_connectors/socket_tracer/uprobe_manager.h @@ -45,7 +45,6 @@ DECLARE_bool(stirling_rescan_for_dlopen); DECLARE_bool(stirling_enable_grpc_c_tracing); DECLARE_double(stirling_rescan_exp_backoff_factor); -DECLARE_bool(access_tls_socket_fd_via_syscall); namespace px { namespace stirling {