From 3b7b927dd01fc96efcda618430851e691ebdb313 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Sun, 12 Apr 2015 17:43:44 +0200 Subject: [PATCH] libbnetapi: Add BNetworkRoute to replace use of route_entry. The BNetworkRoute class manages a route_entry and the sockaddr's associated with it. It replaces the direct use of route_entry in the BNetworkInterface API. Using route_entry is fragile and inconvenient as it only holds pointers to the sockaddr's. When getting a list of routes from the kernel, each route_entry is set up so that its pointers point into the single flat buffer that is passed around. Creating a copy of the route_entry and then deleting the flat buffer makes the pointers in the copy stale. Returning these route entries therefore always lead to a use-after-free when they were eventually used. BNetworkRoute also takes over the code and functionallity of getting routes from RouteSupport. The corresponding method in BNetworkRoster is replaced by a static method in BNetworkRoute. Also distinguish between the default route and gateway of an interface. GetDefaultRoute() now gets the default BNetworkRoute for the interface while GetDefaultGateway() gets the associated gateway address within that default route. Adjust network preferences panel to this change. Note that we currently only seem to have per interface default routes and not an actual global default route. This was already the case before these changes and I did not further investigate what this means. --- headers/os/net/NetworkInterface.h | 11 +- headers/os/net/NetworkRoster.h | 4 - headers/os/net/NetworkRoute.h | 75 ++++ headers/private/net/RouteSupport.h | 22 -- src/kits/network/libnetapi/Jamfile | 2 +- .../network/libnetapi/NetworkInterface.cpp | 73 ++-- src/kits/network/libnetapi/NetworkRoster.cpp | 8 - src/kits/network/libnetapi/NetworkRoute.cpp | 352 ++++++++++++++++++ src/kits/network/libnetapi/RouteSupport.cpp | 82 ---- .../network/InterfaceAddressView.cpp | 2 +- 10 files changed, 462 insertions(+), 169 deletions(-) create mode 100644 headers/os/net/NetworkRoute.h delete mode 100644 headers/private/net/RouteSupport.h create mode 100644 src/kits/network/libnetapi/NetworkRoute.cpp delete mode 100644 src/kits/network/libnetapi/RouteSupport.cpp diff --git a/headers/os/net/NetworkInterface.h b/headers/os/net/NetworkInterface.h index 4e672533d4..d966c34c0b 100644 --- a/headers/os/net/NetworkInterface.h +++ b/headers/os/net/NetworkInterface.h @@ -14,6 +14,7 @@ class BNetworkInterface; +class BNetworkRoute; class BNetworkInterfaceAddress { @@ -100,15 +101,17 @@ public: status_t GetHardwareAddress(BNetworkAddress& address); - status_t AddRoute(const route_entry& route); + status_t AddRoute(const BNetworkRoute& route); status_t AddDefaultRoute(const BNetworkAddress& gateway); - status_t RemoveRoute(const route_entry& route); + status_t RemoveRoute(const BNetworkRoute& route); status_t RemoveRoute(int family, - const route_entry& route); + const BNetworkRoute& route); status_t RemoveDefaultRoute(int family); status_t GetRoutes(int family, - BObjectList& routes) const; + BObjectList& routes) const; status_t GetDefaultRoute(int family, + BNetworkRoute& route) const; + status_t GetDefaultGateway(int family, BNetworkAddress& gateway) const; status_t AutoConfigure(int family); diff --git a/headers/os/net/NetworkRoster.h b/headers/os/net/NetworkRoster.h index 0e1fc56262..536b31a53d 100644 --- a/headers/os/net/NetworkRoster.h +++ b/headers/os/net/NetworkRoster.h @@ -12,7 +12,6 @@ class BMessenger; class BNetworkInterface; -struct route_entry; struct wireless_network; @@ -31,9 +30,6 @@ public: status_t RemoveInterface( const BNetworkInterface& interface); - status_t GetRoutes(int family, - BObjectList& routes) const; - int32 CountPersistentNetworks() const; status_t GetNextPersistentNetwork(uint32* cookie, wireless_network& network) const; diff --git a/headers/os/net/NetworkRoute.h b/headers/os/net/NetworkRoute.h new file mode 100644 index 0000000000..e30730ed7c --- /dev/null +++ b/headers/os/net/NetworkRoute.h @@ -0,0 +1,75 @@ +/* + * Copyright 2015, Haiku, Inc. All Rights Reserved. + * Distributed under the terms of the MIT License. + */ +#ifndef _NETWORK_ROUTE_H +#define _NETWORK_ROUTE_H + +#include + +#include + + +class BNetworkRoute { +public: + BNetworkRoute(); + ~BNetworkRoute(); + + status_t SetTo(const BNetworkRoute& other); + status_t SetTo(const route_entry& routeEntry); + + void Adopt(BNetworkRoute& other); + + const route_entry& RouteEntry() const; + + const sockaddr* Destination() const; + status_t SetDestination(const sockaddr& destination); + void UnsetDestination(); + + const sockaddr* Mask() const; + status_t SetMask(const sockaddr& mask); + void UnsetMask(); + + const sockaddr* Gateway() const; + status_t SetGateway(const sockaddr& gateway); + void UnsetGateway(); + + const sockaddr* Source() const; + status_t SetSource(const sockaddr& source); + void UnsetSource(); + + uint32 Flags() const; + void SetFlags(uint32 flags); + + uint32 MTU() const; + void SetMTU(uint32 mtu); + + int AddressFamily() const; + +static status_t GetDefaultRoute(int family, + const char* interfaceName, + BNetworkRoute& route); +static status_t GetDefaultGateway(int family, + const char* interfaceName, + sockaddr& gateway); + +static status_t GetRoutes(int family, + BObjectList& routes); +static status_t GetRoutes(int family, const char* interfaceName, + BObjectList& routes); +static status_t GetRoutes(int family, const char* interfaceName, + uint32 filterFlags, + BObjectList& routes); + +private: + BNetworkRoute(const BNetworkRoute& other); + // unimplemented to disallow copying + + status_t _AllocateAndSetAddress(const sockaddr& from, + sockaddr*& to); + void _FreeAndUnsetAddress(sockaddr*& address); + + route_entry fRouteEntry; +}; + +#endif // _NETWORK_ROUTE_H diff --git a/headers/private/net/RouteSupport.h b/headers/private/net/RouteSupport.h deleted file mode 100644 index 7c8e524fc9..0000000000 --- a/headers/private/net/RouteSupport.h +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright 2013, Haiku, Inc. All Rights Reserved. - * Distributed under the terms of the MIT License. - */ -#ifndef ROUTESUPPORT_H_ -#define ROUTESUPPORT_H_ - - -#include - - -namespace BPrivate { - - -status_t get_routes(const char* interfaceName, - int family, BObjectList& routes); - - -} - - -#endif /* __ROUTESUPPORT_H_ */ diff --git a/src/kits/network/libnetapi/Jamfile b/src/kits/network/libnetapi/Jamfile index 39d3edc7ff..8fb2ce4f72 100644 --- a/src/kits/network/libnetapi/Jamfile +++ b/src/kits/network/libnetapi/Jamfile @@ -57,8 +57,8 @@ for architectureObject in [ MultiArchSubDirSetup ] { NetworkDevice.cpp NetworkInterface.cpp NetworkRoster.cpp + NetworkRoute.cpp NetworkSettings.cpp - RouteSupport.cpp AbstractSocket.cpp DatagramSocket.cpp diff --git a/src/kits/network/libnetapi/NetworkInterface.cpp b/src/kits/network/libnetapi/NetworkInterface.cpp index fcb0413e8d..e59091240f 100644 --- a/src/kits/network/libnetapi/NetworkInterface.cpp +++ b/src/kits/network/libnetapi/NetworkInterface.cpp @@ -13,7 +13,8 @@ #include #include #include -#include +#include + static int family_from_interface_address(const BNetworkInterfaceAddress& address) @@ -29,22 +30,6 @@ family_from_interface_address(const BNetworkInterfaceAddress& address) } -static int -family_from_route(const route_entry& route) -{ - if (route.destination != NULL && route.destination->sa_family != AF_UNSPEC) - return route.destination->sa_family; - if (route.mask != NULL && route.mask->sa_family != AF_UNSPEC) - return route.mask->sa_family; - if (route.gateway != NULL && route.gateway->sa_family != AF_UNSPEC) - return route.gateway->sa_family; - if (route.source != NULL && route.source->sa_family != AF_UNSPEC) - return route.source->sa_family; - - return AF_UNSPEC; -} - - static status_t do_ifaliasreq(const char* name, int32 option, BNetworkInterfaceAddress& address, bool readBack = false) @@ -513,14 +498,14 @@ BNetworkInterface::GetHardwareAddress(BNetworkAddress& address) status_t -BNetworkInterface::AddRoute(const route_entry& route) +BNetworkInterface::AddRoute(const BNetworkRoute& route) { - int family = family_from_route(route); + int family = route.AddressFamily(); if (family == AF_UNSPEC) return B_BAD_VALUE; ifreq request; - request.ifr_route = route; + request.ifr_route = route.RouteEntry(); return do_request(family, request, Name(), SIOCADDRT); } @@ -528,19 +513,20 @@ BNetworkInterface::AddRoute(const route_entry& route) status_t BNetworkInterface::AddDefaultRoute(const BNetworkAddress& gateway) { - route_entry route; - memset(&route, 0, sizeof(route_entry)); - route.flags = RTF_STATIC | RTF_DEFAULT | RTF_GATEWAY; - route.gateway = const_cast(&gateway.SockAddr()); + BNetworkRoute route; + status_t result = route.SetGateway(gateway); + if (result != B_OK) + return result; + route.SetFlags(RTF_STATIC | RTF_DEFAULT | RTF_GATEWAY); return AddRoute(route); } status_t -BNetworkInterface::RemoveRoute(const route_entry& route) +BNetworkInterface::RemoveRoute(const BNetworkRoute& route) { - int family = family_from_route(route); + int family = route.AddressFamily(); if (family == AF_UNSPEC) return B_BAD_VALUE; @@ -549,10 +535,10 @@ BNetworkInterface::RemoveRoute(const route_entry& route) status_t -BNetworkInterface::RemoveRoute(int family, const route_entry& route) +BNetworkInterface::RemoveRoute(int family, const BNetworkRoute& route) { ifreq request; - request.ifr_route = route; + request.ifr_route = route.RouteEntry(); return do_request(family, request, Name(), SIOCDELRT); } @@ -560,38 +546,31 @@ BNetworkInterface::RemoveRoute(int family, const route_entry& route) status_t BNetworkInterface::RemoveDefaultRoute(int family) { - route_entry route; - memset(&route, 0, sizeof(route_entry)); - route.flags = RTF_STATIC | RTF_DEFAULT; - + BNetworkRoute route; + route.SetFlags(RTF_STATIC | RTF_DEFAULT); return RemoveRoute(family, route); } status_t -BNetworkInterface::GetRoutes(int family, BObjectList& routes) const +BNetworkInterface::GetRoutes(int family, + BObjectList& routes) const { - return BPrivate::get_routes(Name(), family, routes); + return BNetworkRoute::GetRoutes(family, Name(), routes); } status_t -BNetworkInterface::GetDefaultRoute(int family, BNetworkAddress& gateway) const +BNetworkInterface::GetDefaultRoute(int family, BNetworkRoute& route) const { - BObjectList routes(1, true); - status_t status = GetRoutes(family, routes); - if (status != B_OK) - return status; + return BNetworkRoute::GetDefaultRoute(family, Name(), route); +} - for (int32 i = routes.CountItems() - 1; i >= 0; i--) { - route_entry* entry = routes.ItemAt(i); - if (entry->flags & RTF_DEFAULT) { - gateway.SetTo(*entry->gateway); - break; - } - } - return B_OK; +status_t +BNetworkInterface::GetDefaultGateway(int family, BNetworkAddress& gateway) const +{ + return BNetworkRoute::GetDefaultGateway(family, Name(), gateway); } diff --git a/src/kits/network/libnetapi/NetworkRoster.cpp b/src/kits/network/libnetapi/NetworkRoster.cpp index 0e969d3011..3171aaffaf 100644 --- a/src/kits/network/libnetapi/NetworkRoster.cpp +++ b/src/kits/network/libnetapi/NetworkRoster.cpp @@ -15,7 +15,6 @@ #include #include #include -#include // TODO: using AF_INET for the socket isn't really a smart idea, as one @@ -163,13 +162,6 @@ BNetworkRoster::RemoveInterface(const BNetworkInterface& interface) } -status_t -BNetworkRoster::GetRoutes(int family, BObjectList& routes) const -{ - return BPrivate::get_routes(NULL, family, routes); -} - - int32 BNetworkRoster::CountPersistentNetworks() const { diff --git a/src/kits/network/libnetapi/NetworkRoute.cpp b/src/kits/network/libnetapi/NetworkRoute.cpp new file mode 100644 index 0000000000..8dde3108f0 --- /dev/null +++ b/src/kits/network/libnetapi/NetworkRoute.cpp @@ -0,0 +1,352 @@ +/* + * Copyright 2013-2015 Haiku, Inc. All rights reserved. + * Distributed under the terms of the MIT License. + */ + +#include + +#include +#include +#include + +#include + + +BNetworkRoute::BNetworkRoute() +{ + memset(&fRouteEntry, 0, sizeof(route_entry)); +} + + +BNetworkRoute::~BNetworkRoute() +{ + UnsetDestination(); + UnsetMask(); + UnsetGateway(); + UnsetSource(); +} + + +status_t +BNetworkRoute::SetTo(const BNetworkRoute& other) +{ + return SetTo(other.RouteEntry()); +} + + +status_t +BNetworkRoute::SetTo(const route_entry& routeEntry) +{ + #define SET_ADDRESS(address, setFunction) \ + if (routeEntry.address != NULL) { \ + result = setFunction(*routeEntry.address); \ + if (result != B_OK) \ + return result; \ + } + + status_t result; + SET_ADDRESS(destination, SetDestination) + SET_ADDRESS(mask, SetMask) + SET_ADDRESS(gateway, SetGateway) + SET_ADDRESS(source, SetSource) + + SetFlags(routeEntry.flags); + SetMTU(routeEntry.mtu); + return B_OK; +} + + +void +BNetworkRoute::Adopt(BNetworkRoute& other) +{ + memcpy(&fRouteEntry, &other.fRouteEntry, sizeof(route_entry)); + memset(&other.fRouteEntry, 0, sizeof(route_entry)); +} + + +const route_entry& +BNetworkRoute::RouteEntry() const +{ + return fRouteEntry; +} + + +const sockaddr* +BNetworkRoute::Destination() const +{ + return fRouteEntry.destination; +} + + +status_t +BNetworkRoute::SetDestination(const sockaddr& destination) +{ + return _AllocateAndSetAddress(destination, fRouteEntry.destination); +} + + +void +BNetworkRoute::UnsetDestination() +{ + _FreeAndUnsetAddress(fRouteEntry.destination); +} + + +const sockaddr* +BNetworkRoute::Mask() const +{ + return fRouteEntry.mask; +} + + +status_t +BNetworkRoute::SetMask(const sockaddr& mask) +{ + return _AllocateAndSetAddress(mask, fRouteEntry.mask); +} + + +void +BNetworkRoute::UnsetMask() +{ + _FreeAndUnsetAddress(fRouteEntry.mask); +} + + +const sockaddr* +BNetworkRoute::Gateway() const +{ + return fRouteEntry.gateway; +} + + +status_t +BNetworkRoute::SetGateway(const sockaddr& gateway) +{ + return _AllocateAndSetAddress(gateway, fRouteEntry.gateway); +} + + +void +BNetworkRoute::UnsetGateway() +{ + _FreeAndUnsetAddress(fRouteEntry.gateway); +} + + +const sockaddr* +BNetworkRoute::Source() const +{ + return fRouteEntry.source; +} + + +status_t +BNetworkRoute::SetSource(const sockaddr& source) +{ + return _AllocateAndSetAddress(source, fRouteEntry.source); +} + + +void +BNetworkRoute::UnsetSource() +{ + _FreeAndUnsetAddress(fRouteEntry.source); +} + + +uint32 +BNetworkRoute::Flags() const +{ + return fRouteEntry.flags; +} + + +void +BNetworkRoute::SetFlags(uint32 flags) +{ + fRouteEntry.flags = flags; +} + + +uint32 +BNetworkRoute::MTU() const +{ + return fRouteEntry.mtu; +} + + +void +BNetworkRoute::SetMTU(uint32 mtu) +{ + fRouteEntry.mtu = mtu; +} + + +int +BNetworkRoute::AddressFamily() const +{ + #define RETURN_FAMILY_IF_SET(address) \ + if (fRouteEntry.address != NULL \ + && fRouteEntry.address->sa_family != AF_UNSPEC) { \ + return fRouteEntry.address->sa_family; \ + } + + RETURN_FAMILY_IF_SET(destination) + RETURN_FAMILY_IF_SET(mask) + RETURN_FAMILY_IF_SET(gateway) + RETURN_FAMILY_IF_SET(source) + + return AF_UNSPEC; +} + + +status_t +BNetworkRoute::GetDefaultRoute(int family, const char* interfaceName, + BNetworkRoute& route) +{ + BObjectList routes(1, true); + status_t result = GetRoutes(family, interfaceName, RTF_DEFAULT, routes); + if (result != B_OK) + return result; + + if (routes.CountItems() == 0) + return B_ENTRY_NOT_FOUND; + + route.Adopt(*routes.ItemAt(0)); + return B_OK; +} + + +status_t +BNetworkRoute::GetDefaultGateway(int family, const char* interfaceName, + sockaddr& gateway) +{ + BNetworkRoute route; + status_t result = GetDefaultRoute(family, interfaceName, route); + if (result != B_OK) + return result; + + const sockaddr* defaultGateway = route.Gateway(); + if (defaultGateway == NULL) + return B_ENTRY_NOT_FOUND; + + memcpy(&gateway, defaultGateway, defaultGateway->sa_len); + return B_OK; +} + + +status_t +BNetworkRoute::GetRoutes(int family, BObjectList& routes) +{ + return GetRoutes(family, NULL, 0, routes); +} + + +status_t +BNetworkRoute::GetRoutes(int family, const char* interfaceName, + BObjectList& routes) +{ + return GetRoutes(family, interfaceName, 0, routes); +} + + +status_t +BNetworkRoute::GetRoutes(int family, const char* interfaceName, + uint32 filterFlags, BObjectList& routes) +{ + int socket = ::socket(family, SOCK_DGRAM, 0); + if (socket < 0) + return errno; + + FileDescriptorCloser fdCloser(socket); + + ifconf config; + config.ifc_len = sizeof(config.ifc_value); + if (ioctl(socket, SIOCGRTSIZE, &config, sizeof(struct ifconf)) < 0) + return errno; + + uint32 size = (uint32)config.ifc_value; + if (size == 0) + return B_OK; + + void* buffer = malloc(size); + if (buffer == NULL) + return B_NO_MEMORY; + + MemoryDeleter bufferDeleter(buffer); + config.ifc_len = size; + config.ifc_buf = buffer; + + if (ioctl(socket, SIOCGRTTABLE, &config, sizeof(struct ifconf)) < 0) + return errno; + + ifreq* interface = (ifreq*)buffer; + ifreq* end = (ifreq*)((uint8*)buffer + size); + + while (interface < end) { + route_entry& routeEntry = interface->ifr_route; + + if ((interfaceName == NULL + || strcmp(interface->ifr_name, interfaceName) == 0) + && (filterFlags == 0 || (routeEntry.flags & filterFlags) != 0)) { + + BNetworkRoute* route = new(std::nothrow) BNetworkRoute; + if (route == NULL) + return B_NO_MEMORY; + + // Note that source is not provided in the buffer. + routeEntry.source = NULL; + + status_t result = route->SetTo(routeEntry); + if (result != B_OK) { + delete route; + return result; + } + + if (!routes.AddItem(route)) { + delete route; + return B_NO_MEMORY; + } + } + + size_t addressSize = 0; + if (routeEntry.destination != NULL) + addressSize += routeEntry.destination->sa_len; + if (routeEntry.mask != NULL) + addressSize += routeEntry.mask->sa_len; + if (routeEntry.gateway != NULL) + addressSize += routeEntry.gateway->sa_len; + + interface = (ifreq *)((addr_t)interface + IF_NAMESIZE + + sizeof(route_entry) + addressSize); + } + + return B_OK; +} + + +status_t +BNetworkRoute::_AllocateAndSetAddress(const sockaddr& from, + sockaddr*& to) +{ + if (from.sa_len > sizeof(sockaddr_storage)) + return B_BAD_VALUE; + + if (to == NULL) { + to = (sockaddr*)malloc(sizeof(sockaddr_storage)); + if (to == NULL) + return B_NO_MEMORY; + } + + memcpy(to, &from, from.sa_len); + return B_OK; +} + + +void +BNetworkRoute::_FreeAndUnsetAddress(sockaddr*& address) +{ + free(address); + address = NULL; +} diff --git a/src/kits/network/libnetapi/RouteSupport.cpp b/src/kits/network/libnetapi/RouteSupport.cpp deleted file mode 100644 index 291097c724..0000000000 --- a/src/kits/network/libnetapi/RouteSupport.cpp +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright 2013, Haiku, Inc. All Rights Reserved. - * Distributed under the terms of the MIT License. - */ - - -#include -#include -#include - -#include -#include -#include - - -namespace BPrivate { - - -status_t -get_routes(const char* interfaceName, int family, BObjectList& routes) -{ - int socket = ::socket(family, SOCK_DGRAM, 0); - if (socket < 0) - return errno; - - FileDescriptorCloser fdCloser(socket); - - // Obtain gateway - ifconf config; - config.ifc_len = sizeof(config.ifc_value); - if (ioctl(socket, SIOCGRTSIZE, &config, sizeof(struct ifconf)) < 0) - return errno; - - uint32 size = (uint32)config.ifc_value; - if (size == 0) - return B_ERROR; - - void* buffer = malloc(size); - if (buffer == NULL) - return B_NO_MEMORY; - - MemoryDeleter bufferDeleter(buffer); - config.ifc_len = size; - config.ifc_buf = buffer; - - if (ioctl(socket, SIOCGRTTABLE, &config, sizeof(struct ifconf)) < 0) - return errno; - - ifreq* interface = (ifreq*)buffer; - ifreq* end = (ifreq*)((uint8*)buffer + size); - - while (interface < end) { - route_entry& route = interface->ifr_route; - if (interfaceName == NULL - || !strcmp(interface->ifr_name, interfaceName)) { - route_entry* newRoute = new (std::nothrow) route_entry; - if (newRoute == NULL) - return B_NO_MEMORY; - memcpy(newRoute, &interface->ifr_route, sizeof(route_entry)); - if (!routes.AddItem(newRoute)) { - delete newRoute; - return B_NO_MEMORY; - } - } - - int32 addressSize = 0; - if (route.destination != NULL) - addressSize += route.destination->sa_len; - if (route.mask != NULL) - addressSize += route.mask->sa_len; - if (route.gateway != NULL) - addressSize += route.gateway->sa_len; - - interface = (ifreq *)((addr_t)interface + IF_NAMESIZE - + sizeof(route_entry) + addressSize); - } - - return B_OK; -} - - -} diff --git a/src/preferences/network/InterfaceAddressView.cpp b/src/preferences/network/InterfaceAddressView.cpp index 5c79a17a25..520dd5834f 100644 --- a/src/preferences/network/InterfaceAddressView.cpp +++ b/src/preferences/network/InterfaceAddressView.cpp @@ -239,7 +239,7 @@ InterfaceAddressView::_UpdateFields() fNetmaskField->SetText(address.Mask().ToString()); BNetworkAddress gateway; - if (fInterface.GetDefaultRoute(fFamily, gateway) == B_OK) + if (fInterface.GetDefaultGateway(fFamily, gateway) == B_OK) fGatewayField->SetText(gateway.ToString()); else fGatewayField->SetText(NULL);