From 488d79d4201bba99f04310980b9e08b414b57260 Mon Sep 17 00:00:00 2001 From: Edwin Wu Date: Tue, 23 Jun 2026 20:12:47 -0700 Subject: [PATCH] Harden Nearby Connections WifiLan bandwidth upgrade PiperOrigin-RevId: 937051504 --- .../mediums/wifi_lan_bwu_handler.cc | 33 +++++-- .../mediums/wifi_lan_bwu_handler_test.cc | 70 ++++++++++++- .../offline_frames_validator.cc | 5 +- .../offline_frames_validator_test.cc | 15 ++- internal/platform/BUILD | 2 - internal/platform/service_address.cc | 33 +++++++ internal/platform/service_address.h | 9 +- internal/platform/service_address_test.cc | 99 +++++++++++++++++++ 8 files changed, 241 insertions(+), 25 deletions(-) diff --git a/connections/implementation/mediums/wifi_lan_bwu_handler.cc b/connections/implementation/mediums/wifi_lan_bwu_handler.cc index a25cd81c..607c5b9f 100644 --- a/connections/implementation/mediums/wifi_lan_bwu_handler.cc +++ b/connections/implementation/mediums/wifi_lan_bwu_handler.cc @@ -79,21 +79,40 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( << address_candidate.ip_address().size(); continue; } + if (service_address.IsLoopbackAddress() || + service_address.IsLinkLocalAddress()) { + LOG(WARNING) << "Loopback/link-local address candidate is rejected."; + return { + Error(OperationResultCode::CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL)}; + } address_candidates.push_back(std::move(service_address)); } // Only use ip_address and wifi_port if address_candidates is empty. if (address_candidates.empty()) { - address_candidates.push_back(ServiceAddress{ - .address = - std::vector(upgrade_path_info_socket.ip_address().begin(), - upgrade_path_info_socket.ip_address().end()), - .port = static_cast(upgrade_path_info_socket.wifi_port())}); + if (upgrade_path_info_socket.ip_address().size() != 4) { + LOG(ERROR) << "WifiLanBwuHandler: fallback ip_address size is not 4 " + << "(IPv4 only)."; + return { + Error(OperationResultCode::CONNECTIVITY_WIFI_LAN_IP_ADDRESS_ERROR)}; + } + ServiceAddress service_address; + service_address.address = {upgrade_path_info_socket.ip_address().begin(), + upgrade_path_info_socket.ip_address().end()}; + service_address.port = + static_cast(upgrade_path_info_socket.wifi_port()); + if (service_address.IsLoopbackAddress() || + service_address.IsLinkLocalAddress()) { + LOG(WARNING) << "Loopback/link-local fallback address is rejected."; + return { + Error(OperationResultCode::CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL)}; + } + address_candidates.push_back(std::move(service_address)); } Error error; for (const auto& address_candidate : address_candidates) { VLOG(1) << "WifiLanBwuHandler is attempting to connect to available " - "WifiLan service (" << address_candidate << ") for endpoint " - << endpoint_id; + "WifiLan service (" + << address_candidate << ") for endpoint " << endpoint_id; std::shared_ptr cancellation_flag = client->GetCancellationFlag(endpoint_id); ErrorOr socket_result = wifi_lan_medium_.Connect( diff --git a/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc b/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc index 1cf09752..cf09e7bb 100644 --- a/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc +++ b/connections/implementation/mediums/wifi_lan_bwu_handler_test.cc @@ -209,6 +209,71 @@ TEST_F(WifiLanBwuHandlerTest, EXPECT_TRUE(result.has_value()); }; +TEST_F(WifiLanBwuHandlerTest, + CreateUpgradedEndpointChannel_RejectLoopbackAndLinkLocalCandidates) { + ClientProxy client; + client.AddCancellationFlag(std::string(kEndpointId)); + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo path_info; + + // 1st candidate: Loopback + auto* address_candidate = + path_info.mutable_wifi_lan_socket()->add_address_candidates(); + address_candidate->set_ip_address(std::string("\x7f\x00\x00\x01", 4)); + address_candidate->set_port(8080); + + // 2nd candidate: Link-Local (169.254.1.1) + address_candidate = + path_info.mutable_wifi_lan_socket()->add_address_candidates(); + address_candidate->set_ip_address("\xa9\xfe\x01\x01"); + address_candidate->set_port(8080); + + // 3rd candidate: Valid IP + address_candidate = + path_info.mutable_wifi_lan_socket()->add_address_candidates(); + address_candidate->set_ip_address(kIpv4Address); + address_candidate->set_port(8080); + + auto result = handler_.CreateUpgradedEndpointChannel( + &client, std::string(kServiceId), std::string(kEndpointId), + std::move(path_info)); + + EXPECT_FALSE(result.has_value()); +} + +TEST_F(WifiLanBwuHandlerTest, + CreateUpgradedEndpointChannel_RejectLoopbackFallbackIp) { + ClientProxy client; + client.AddCancellationFlag(std::string(kEndpointId)); + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo path_info; + path_info.mutable_wifi_lan_socket()->set_ip_address( + std::string("\x7f\x00\x00\x01", 4)); + path_info.mutable_wifi_lan_socket()->set_wifi_port(8080); + + auto result = handler_.CreateUpgradedEndpointChannel( + &client, std::string(kServiceId), std::string(kEndpointId), + std::move(path_info)); + + EXPECT_FALSE(result.has_value()); +} + +TEST_F(WifiLanBwuHandlerTest, + CreateUpgradedEndpointChannel_RejectInvalidFallbackIpLength) { + ClientProxy client; + client.AddCancellationFlag(std::string(kEndpointId)); + + BandwidthUpgradeNegotiationFrame::UpgradePathInfo path_info; + path_info.mutable_wifi_lan_socket()->set_ip_address("123"); + path_info.mutable_wifi_lan_socket()->set_wifi_port(8080); + + auto result = handler_.CreateUpgradedEndpointChannel( + &client, std::string(kServiceId), std::string(kEndpointId), + std::move(path_info)); + + EXPECT_FALSE(result.has_value()); +} + TEST_F(WifiLanBwuHandlerTest, InitializeUpgradedMediumForEndpoint_Success) { MediumEnvironment::Instance().Start({.use_simulated_clock = true}); ClientProxy client; @@ -289,9 +354,8 @@ TEST_F(WifiLanBwuHandlerTest, mediums_.GetWifiLan().IsAcceptingConnections("service_id_UPGRADE")); } -TEST_F( - WifiLanBwuHandlerTest, - InitializeUpgradedMediumForEndpoint_AlreadyAccepting_KeepAccepting) { +TEST_F(WifiLanBwuHandlerTest, + InitializeUpgradedMediumForEndpoint_AlreadyAccepting_KeepAccepting) { MediumEnvironment::Instance().Start({.use_simulated_clock = true}); ClientProxy client; client.AddCancellationFlag(std::string(kEndpointId)); diff --git a/connections/implementation/offline_frames_validator.cc b/connections/implementation/offline_frames_validator.cc index c42cd4a3..3ff44d21 100644 --- a/connections/implementation/offline_frames_validator.cc +++ b/connections/implementation/offline_frames_validator.cc @@ -240,7 +240,7 @@ Exception EnsureValidBandwidthUpgradeWifiHotspotPathAvailableFrame( const std::regex ip4_pattern(std::string(kIpv4PatternString).c_str()); if (!wifi_hotspot_credentials.gateway().empty() && !(std::regex_match(wifi_hotspot_credentials.gateway(), ip4_pattern))) { - return {Exception::kInvalidProtocolBuffer}; + return {Exception::kInvalidProtocolBuffer}; } for (const auto& address_candidate : wifi_hotspot_credentials.address_candidates()) { @@ -300,8 +300,7 @@ Exception EnsureValidBandwidthUpgradeWifiDirectPathAvailableFrame( kWifiPasswordSsidMinLength, kWifiPasswordSsidMaxLength); bool device_name_valid = wifi_direct_credentials.has_device_name() && - wifi_direct_credentials.device_name().length() < - kWifiDirectSsidMaxLength; + wifi_direct_credentials.device_name().length() < kWifiDirectSsidMaxLength; bool pin_valid = wifi_direct_credentials.has_pin() && WithinRange(wifi_direct_credentials.pin().length(), diff --git a/connections/implementation/offline_frames_validator_test.cc b/connections/implementation/offline_frames_validator_test.cc index 920db845..e6db4b56 100644 --- a/connections/implementation/offline_frames_validator_test.cc +++ b/connections/implementation/offline_frames_validator_test.cc @@ -768,9 +768,8 @@ TEST(OfflineFramesValidatorTest, std::string wifi_direct_pin_wrong_length = "01234567890123456"; std::string bytes = ForBwuWifiDirectPathAvailable( wifi_direct_ssid, std::string(kWifiDirectPassword), kPort, - kWifiDirectFrequency, kSupportsDisablingEncryption, - std::string(kGateway), std::string(kWifiDirectDeviceName), - wifi_direct_pin_wrong_length); + kWifiDirectFrequency, kSupportsDisablingEncryption, std::string(kGateway), + std::string(kWifiDirectDeviceName), wifi_direct_pin_wrong_length); offline_frame_1.ParseFromString(bytes); auto ret_value = EnsureValidOfflineFrame(offline_frame_1); @@ -784,9 +783,8 @@ TEST(OfflineFramesValidatorTest, "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456789"; bytes = ForBwuWifiDirectPathAvailable( wifi_direct_ssid_wrong_length, std::string(kWifiDirectPassword), kPort, - kWifiDirectFrequency, kSupportsDisablingEncryption, - std::string(kGateway), wifi_direct_device_name_wrong_length, - std::string(kWifiDirectPin)); + kWifiDirectFrequency, kSupportsDisablingEncryption, std::string(kGateway), + wifi_direct_device_name_wrong_length, std::string(kWifiDirectPin)); offline_frame_2.ParseFromString(bytes); ret_value = EnsureValidOfflineFrame(offline_frame_2); @@ -807,9 +805,8 @@ TEST(OfflineFramesValidatorTest, "AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz0123456789"; std::string bytes = ForBwuWifiDirectPathAvailable( std::string(kWifiDirectSsid), long_wifi_direct_password, kPort, - kWifiDirectFrequency, kSupportsDisablingEncryption, - std::string(kGateway), std::string(kWifiDirectDeviceName), - long_wifi_direct_pin); + kWifiDirectFrequency, kSupportsDisablingEncryption, std::string(kGateway), + std::string(kWifiDirectDeviceName), long_wifi_direct_pin); offline_frame_2.ParseFromString(bytes); auto ret_value = EnsureValidOfflineFrame(offline_frame_2); diff --git a/internal/platform/BUILD b/internal/platform/BUILD index e9efd49e..9ee65f96 100644 --- a/internal/platform/BUILD +++ b/internal/platform/BUILD @@ -80,8 +80,6 @@ cc_library( "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/functional:any_invocable", "@com_google_absl//absl/meta:type_traits", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/strings:string_view", diff --git a/internal/platform/service_address.cc b/internal/platform/service_address.cc index 90722d31..d29d0a3f 100644 --- a/internal/platform/service_address.cc +++ b/internal/platform/service_address.cc @@ -14,11 +14,17 @@ #include "internal/platform/service_address.h" +#include #include +#include "absl/strings/string_view.h" #include "connections/implementation/proto/offline_wire_formats.pb.h" namespace nearby { +namespace { +constexpr absl::string_view kIpv6LoopbackAddress( + "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\1", 16); +} // namespace void ServiceAddressToProto( const ServiceAddress& service_address, @@ -42,4 +48,31 @@ bool ServiceAddressFromProto( return true; } +bool ServiceAddress::IsLoopbackAddress() const { + if (address.size() == 4) { + // IPv4 loopback: 127.0.0.0/8 + return address[0] == 127; + } else if (address.size() == 16) { + // IPv6 loopback: ::1 + return absl::string_view(address.data(), address.size()) == + kIpv6LoopbackAddress; + } + return false; +} + +bool ServiceAddress::IsLinkLocalAddress() const { + if (address.size() == 4) { + // IPv4 link-local: 169.254.0.0/16 + uint8_t b0 = static_cast(address[0]); + uint8_t b1 = static_cast(address[1]); + return b0 == 169 && b1 == 254; + } else if (address.size() == 16) { + // IPv6 link-local: fe80::/10 + uint8_t b0 = static_cast(address[0]); + uint8_t b1 = static_cast(address[1]); + return b0 == 0xfe && (b1 & 0xc0) == 0x80; + } + return false; +} + } // namespace nearby diff --git a/internal/platform/service_address.h b/internal/platform/service_address.h index 887e0b18..8a8ed1ed 100644 --- a/internal/platform/service_address.h +++ b/internal/platform/service_address.h @@ -16,7 +16,6 @@ #define THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_SERVICE_ADDRESS_H_ #include -#include #include #include @@ -33,6 +32,14 @@ struct ServiceAddress { uint16_t port; bool operator==(const ServiceAddress& other) const = default; + + // Returns true if the address is a loopback address (IPv4 127.0.0.0/8 or + // IPv6 ::1). + bool IsLoopbackAddress() const; + + // Returns true if the address is a link-local address (IPv4 169.254.0.0/16 or + // IPv6 fe80::/10). + bool IsLinkLocalAddress() const; }; // Support logging of ServiceAddress. diff --git a/internal/platform/service_address_test.cc b/internal/platform/service_address_test.cc index 436a1966..b7aed877 100644 --- a/internal/platform/service_address_test.cc +++ b/internal/platform/service_address_test.cc @@ -118,5 +118,104 @@ TEST(ServiceAddressTest, ServiceAddressEquality) { EXPECT_EQ(service_address5, service_address6); } +TEST(ServiceAddressTest, IsLoopbackAddressIPv4) { + ServiceAddress service_address = { + .address = {127, 0, 0, 1}, + .port = 8080, + }; + EXPECT_TRUE(service_address.IsLoopbackAddress()); + + ServiceAddress not_loopback = { + .address = {10, 0, 0, 1}, + .port = 8080, + }; + EXPECT_FALSE(not_loopback.IsLoopbackAddress()); +} + +TEST(ServiceAddressTest, IsLoopbackAddressIPv6) { + ServiceAddress service_address = { + .address = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + .port = 8080, + }; + EXPECT_TRUE(service_address.IsLoopbackAddress()); + + ServiceAddress not_loopback = { + .address = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}, + .port = 8080, + }; + EXPECT_FALSE(not_loopback.IsLoopbackAddress()); +} + +TEST(ServiceAddressTest, IsLinkLocalAddressIPv4) { + ServiceAddress service_address = { + .address = {static_cast(169), static_cast(254), 0, 1}, + .port = 8080, + }; + EXPECT_TRUE(service_address.IsLinkLocalAddress()); + + ServiceAddress not_link_local = { + .address = {169, static_cast(253), 0, 1}, + .port = 8080, + }; + EXPECT_FALSE(not_link_local.IsLinkLocalAddress()); + + ServiceAddress not_link_local_b0 = { + .address = {100, static_cast(254), 0, 1}, + .port = 8080, + }; + EXPECT_FALSE(not_link_local_b0.IsLinkLocalAddress()); +} + +TEST(ServiceAddressTest, IsLinkLocalAddressIPv6) { + ServiceAddress service_address = { + .address = {static_cast(0xfe), static_cast(0x80), 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + .port = 8080, + }; + EXPECT_TRUE(service_address.IsLinkLocalAddress()); + + ServiceAddress not_link_local = { + .address = {static_cast(0xfd), static_cast(0x80), 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + .port = 8080, + }; + EXPECT_FALSE(not_link_local.IsLinkLocalAddress()); + + ServiceAddress not_link_local_b1 = { + .address = {static_cast(0xfe), 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 1}, + .port = 8080, + }; + EXPECT_FALSE(not_link_local_b1.IsLinkLocalAddress()); +} + +TEST(ServiceAddressTest, IsLoopbackAddressInvalidLength) { + ServiceAddress service_address = { + .address = {127, 0, 0}, + .port = 8080, + }; + EXPECT_FALSE(service_address.IsLoopbackAddress()); + + ServiceAddress empty_address = { + .address = {}, + .port = 8080, + }; + EXPECT_FALSE(empty_address.IsLoopbackAddress()); +} + +TEST(ServiceAddressTest, IsLinkLocalAddressInvalidLength) { + ServiceAddress service_address = { + .address = {static_cast(169), static_cast(254), 0}, + .port = 8080, + }; + EXPECT_FALSE(service_address.IsLinkLocalAddress()); + + ServiceAddress empty_address = { + .address = {}, + .port = 8080, + }; + EXPECT_FALSE(empty_address.IsLinkLocalAddress()); +} + } // namespace } // namespace nearby