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

Fix /proc/net/route requirement that causes errors on FreeBSD #1638

Merged
merged 31 commits into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
db21ecf
- Fix /proc/net/route requirement on FreeBSD
Sep 18, 2019
c98c917
Moved route conversion code from read_route_table
Sep 19, 2019
ceeb1b1
- Fixed some typos
Sep 19, 2019
3b7b987
- Fixed some routes being ignored
Sep 19, 2019
1ee6e01
- Fix /proc/net/route requirement on FreeBSD
Sep 18, 2019
3d17625
Merge branch 'develop' of https://github.com/onenachi/WALinuxAgent in…
Sep 19, 2019
8a84b14
- Added Unit Tests
kawaiivoldemort Oct 3, 2019
a245b2d
- Fixed the patch
kawaiivoldemort Oct 3, 2019
12aaffb
- Fixed using osutil instead of FreeBSD osutil
kawaiivoldemort Oct 3, 2019
9e24775
- Patched the right function
kawaiivoldemort Oct 3, 2019
2e5eee2
- Fixed the route table
kawaiivoldemort Oct 3, 2019
92a1fd7
- fixed zero length field name in python 2.6
kawaiivoldemort Oct 3, 2019
8f221c4
- Fixed the routes in the valid test
kawaiivoldemort Oct 3, 2019
9e4b64a
- More fixes for valid routes
kawaiivoldemort Oct 3, 2019
6ae71f7
- Fixed some IP parsing
kawaiivoldemort Oct 3, 2019
b086c46
- Fixed the parsing of the ipv4 addresses
kawaiivoldemort Oct 3, 2019
766815b
- Fixed some typos (comma placement and params)
kawaiivoldemort Oct 3, 2019
f79cf03
- FIxed a typo
kawaiivoldemort Oct 3, 2019
db17060
- Ordering of netmask check
kawaiivoldemort Oct 3, 2019
c62829a
- Changed subnet in test
kawaiivoldemort Oct 3, 2019
cf6a3ee
- Fixed a typo
kawaiivoldemort Oct 3, 2019
5ea342e
- Fixed test route flags
kawaiivoldemort Oct 3, 2019
6c44645
- Fix /proc/net/route requirement on FreeBSD
Sep 18, 2019
8c2077c
resolving a conflict
kawaiivoldemort Oct 3, 2019
580e60b
- Fixed a bad merge
kawaiivoldemort Oct 3, 2019
515b672
- added exception handling to read_route_table in freebsd.py
kawaiivoldemort Oct 4, 2019
10819af
- maked netstat parsing code more readable
Oct 7, 2019
e9d7a64
- Fixed the FreeBSD tests
Oct 7, 2019
a0112e2
moved from run_get_output to run_command in _get_netstat_rn_ipv4_routes
Oct 15, 2019
72bddb7
- Fixed the freebsd tests to patch run_command
Oct 15, 2019
08ba686
- Fixed run_command in freebsd.py
Oct 21, 2019
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
7 changes: 3 additions & 4 deletions azurelinuxagent/common/dhcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,8 @@ def wireserver_route_exists(self):
route_exists = False
logger.info("Test for route to {0}".format(KNOWN_WIRESERVER_IP))
try:
route_file = '/proc/net/route'
if os.path.exists(route_file) and \
KNOWN_WIRESERVER_IP_ENTRY in open(route_file).read():
route_table = self.osutil.read_route_table()
if any([(KNOWN_WIRESERVER_IP_ENTRY in route) for route in route_table]):
Copy link
Member

Choose a reason for hiding this comment

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

In the original code there was a check for exists(route_file); if it was false, we would output a warning.
In the new code this would be reported as an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how to work around this. read_route_table will log an error by default and then return an empty list if /proc/net/route doesn't exist.

The only thing it guarantees is not to raise an exception.

Im not sure changing that function would be appropriate. I do think using that function would be appropriate since checking route is clearly a platform specific task.

Copy link
Contributor Author

@kawaiivoldemort kawaiivoldemort Oct 9, 2019

Choose a reason for hiding this comment

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

I also do think that this is an edge case and not really an issue as it will only log an error on systems without /proc/net/route (non Linux systems) which have not overridden that function with their own osutil (so now, neither Linux nor FreeBSD).

Copy link
Member

Choose a reason for hiding this comment

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

ok

# reset self.gateway and self.routes
# we do not need to alter the routing table
self.endpoint = KNOWN_WIRESERVER_IP
Expand All @@ -102,7 +101,7 @@ def wireserver_route_exists(self):
logger.error(
"Could not determine whether route exists to {0}: {1}".format(
KNOWN_WIRESERVER_IP, e))

return route_exists

@property
Expand Down
310 changes: 310 additions & 0 deletions azurelinuxagent/common/osutil/freebsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@
#
# Requires Python 2.6+ and Openssl 1.0+

import socket
import struct
import binascii
import azurelinuxagent.common.utils.fileutil as fileutil
import azurelinuxagent.common.utils.shellutil as shellutil
import azurelinuxagent.common.utils.textutil as textutil
from azurelinuxagent.common.utils.networkutil import RouteEntry
import azurelinuxagent.common.logger as logger
from azurelinuxagent.common.exception import OSUtilError
from azurelinuxagent.common.osutil.default import DefaultOSUtil
Expand Down Expand Up @@ -93,6 +97,304 @@ def get_if_mac(self, ifname):
def get_first_if(self):
return self._get_net_info()[:2]

@staticmethod
def read_route_table():
"""
Return a list of strings comprising the route table as in the Linux /proc/net/route format. The input taken is from FreeBSDs
`netstat -rn -f inet` command. Here is what the function does in detail:

1. Runs `netstat -rn -f inet` which outputs a column formatted list of ipv4 routes in priority order like so:

> Routing tables
>
> Internet:
> Destination Gateway Flags Refs Use Netif Expire
> default 61.221.xx.yy UGS 0 247 em1
> 10 10.10.110.5 UGS 0 50 em0
> 10.10.110/26 link#1 UC 0 0 em0
> 10.10.110.5 00:1b:0d:e6:58:40 UHLW 2 0 em0 1145
> 61.221.xx.yy/29 link#2 UC 0 0 em1
> 61.221.xx.yy 00:1b:0d:e6:57:c0 UHLW 2 0 em1 1055
> 61.221.xx/24 link#2 UC 0 0 em1
> 127.0.0.1 127.0.0.1 UH 0 0 lo0

2. Convert it to an array of lines that resemble an equivalent /proc/net/route content on a Linux system like so:

> Iface Destination Gateway Flags RefCnt Use Metric Mask MTU Window IRTT
> gre828 00000000 00000000 0001 0 0 0 000000F8 0 0 0
> ens160 00000000 FE04700A 0003 0 0 100 00000000 0 0 0
> gre828 00000008 00000000 0001 0 0 0 000000FE 0 0 0
> ens160 0004700A 00000000 0001 0 0 100 00FFFFFF 0 0 0
> gre828 2504700A 00000000 0005 0 0 0 FFFFFFFF 0 0 0
> gre828 3704700A 00000000 0005 0 0 0 FFFFFFFF 0 0 0
> gre828 4104700A 00000000 0005 0 0 0 FFFFFFFF 0 0 0

:return: Entries in the ipv4 route priority list from `netstat -rn -f inet` in the linux `/proc/net/route` style
:rtype: list(str)
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add an example of how the netstat -rn output differs with the linux based proc/net/route header just to make sure what this function is parsing

Copy link
Contributor Author

@kawaiivoldemort kawaiivoldemort Oct 7, 2019

Choose a reason for hiding this comment

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

Exmples added as requested in 10819af


def _get_netstat_rn_ipv4_routes():
"""
Runs `netstat -rn -f inet` and parses its output and returns a list of routes where the key is the column name
and the value is the value in the column, stripped of leading and trailing whitespace.

:return: List of dictionaries representing routes in the ipv4 route priority list from `netstat -rn -f inet`
:rtype: list(dict)
"""
cmd = [ "netstat", "-rn", "-f", "inet" ]
output = shellutil.run_command(cmd, log_error=True)
Copy link
Member

@narrieta narrieta Oct 16, 2019

Choose a reason for hiding this comment

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

thank you for the update.

run_command sets shell=False when invoking subprocess.Popen, so the command needs to be passed as an array of strings; see, for example https://github.com/Azure/WALinuxAgent/blob/develop/azurelinuxagent/common/utils/cryptutil.py#L55

Also, please be sure to tests this end-to-end on your side. Our test infrastructure does not validate FreeBSD.

Thanks

Copy link
Contributor Author

@kawaiivoldemort kawaiivoldemort Oct 21, 2019

Choose a reason for hiding this comment

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

Fixed in 08ba686

Tested in FreeBSD 11.2 on Azure:
image
Route table is read successfully.

Sorry about that.

output_lines = output.split("\n")
if len(output_lines) < 3:
raise OSUtilError("`netstat -rn -f inet` output seems to be empty")
output_lines = [ line.strip() for line in output_lines if line ]
if "Internet:" not in output_lines:
raise OSUtilError("`netstat -rn -f inet` output seems to contain no ipv4 routes")
route_header_line = output_lines.index("Internet:") + 1
# Parse the file structure and left justify the routes
route_start_line = route_header_line + 1
route_line_length = max([len(line) for line in output_lines[route_header_line:]])
netstat_route_list = [line.ljust(route_line_length) for line in output_lines[route_start_line:]]
# Parse the headers
_route_headers = output_lines[route_header_line].split()
n_route_headers = len(_route_headers)
route_columns = {}
for i in range(0, n_route_headers - 1):
route_columns[_route_headers[i]] = (
output_lines[route_header_line].index(_route_headers[i]),
(output_lines[route_header_line].index(_route_headers[i+1]) - 1)
)
route_columns[_route_headers[n_route_headers - 1]] = (
output_lines[route_header_line].index(_route_headers[n_route_headers - 1]),
None
)
# Parse the routes
netstat_routes = []
n_netstat_routes = len(netstat_route_list)
for i in range(0, n_netstat_routes):
netstat_route = {}
for column in route_columns:
netstat_route[column] = netstat_route_list[i][route_columns[column][0]:route_columns[column][1]].strip()
netstat_route["Metric"] = n_netstat_routes - i
netstat_routes.append(netstat_route)
# Return the Sections
return netstat_routes

def _ipv4_ascii_address_to_hex(ipv4_ascii_address):
"""
Converts an IPv4 32bit address from its ASCII notation (ie. 127.0.0.1) to an 8 digit padded hex notation
(ie. "0100007F") string.

:return: 8 character long hex string representation of the IP
:rtype: string
"""
# Raises socket.error if the IP is not a valid IPv4
return "%08X" % int(binascii.hexlify(struct.pack("!I", struct.unpack("=I", socket.inet_pton(socket.AF_INET, ipv4_ascii_address))[0])), 16)

def _ipv4_cidr_mask_to_hex(ipv4_cidr_mask):
"""
Converts an subnet mask from its CIDR integer notation (ie. 32) to an 8 digit padded hex notation
(ie. "FFFFFFFF") string representing its bitmask form.

:return: 8 character long hex string representation of the IP
:rtype: string
"""
return "{0:08x}".format(struct.unpack("=I", struct.pack("!I", (0xffffffff << (32 - ipv4_cidr_mask)) & 0xffffffff))[0]).upper()

def _ipv4_cidr_destination_to_hex(destination):
"""
Converts an destination address from its CIDR notation (ie. 127.0.0.1/32 or default or localhost) to an 8
digit padded hex notation (ie. "0100007F" or "00000000" or "0100007F") string and its subnet bitmask
also in hex (FFFFFFFF).

:return: tuple of 8 character long hex string representation of the IP and 8 character long hex string representation of the subnet mask
:rtype: tuple(string, int)
"""
destination_ip = "0.0.0.0"
destination_subnetmask = 32
if destination != "default":
if destination == "localhost":
destination_ip = "127.0.0.1"
else:
destination_ip = destination.split("/")
if len(destination_ip) > 1:
destination_subnetmask = int(destination_ip[1])
destination_ip = destination_ip[0]
hex_destination_ip = _ipv4_ascii_address_to_hex(destination_ip)
hex_destination_subnetmask = _ipv4_cidr_mask_to_hex(destination_subnetmask)
return hex_destination_ip, hex_destination_subnetmask

def _try_ipv4_gateway_to_hex(gateway):
"""
If the gateway is an IPv4 address, return its IP in hex, else, return "00000000"

:return: 8 character long hex string representation of the IP of the gateway
:rtype: string
"""
try:
return _ipv4_ascii_address_to_hex(gateway)
except socket.error:
return "00000000"

def _ascii_route_flags_to_bitmask(ascii_route_flags):
"""
Converts route flags to a bitmask of their equivalent linux/route.h values.

:return: integer representation of a 16 bit mask
:rtype: int
"""
bitmask_flags = 0
RTF_UP = 0x0001
RTF_GATEWAY = 0x0002
RTF_HOST = 0x0004
RTF_DYNAMIC = 0x0010
if "U" in ascii_route_flags:
bitmask_flags |= RTF_UP
if "G" in ascii_route_flags:
bitmask_flags |= RTF_GATEWAY
if "H" in ascii_route_flags:
bitmask_flags |= RTF_HOST
if "S" not in ascii_route_flags:
bitmask_flags |= RTF_DYNAMIC
return bitmask_flags

def _freebsd_netstat_rn_route_to_linux_proc_net_route(netstat_route):
"""
Converts a single FreeBSD `netstat -rn -f inet` route to its equivalent /proc/net/route line. ie:
> default 0.0.0.0 UGS 0 247 em1
to
> em1 00000000 00000000 0003 0 0 0 FFFFFFFF 0 0 0

:return: string representation of the equivalent /proc/net/route line
:rtype: string
"""
network_interface = netstat_route["Netif"]
hex_destination_ip, hex_destination_subnetmask = _ipv4_cidr_destination_to_hex(netstat_route["Destination"])
hex_gateway = _try_ipv4_gateway_to_hex(netstat_route["Gateway"])
bitmask_flags = _ascii_route_flags_to_bitmask(netstat_route["Flags"])
dummy_refcount = 0
dummy_use = 0
route_metric = netstat_route["Metric"]
dummy_mtu = 0
dummy_window = 0
dummy_irtt = 0
return "{0}\t{1}\t{2}\t{3}\t{4}\t{5}\t{6}\t{7}\t{8}\t{9}\t{10}".format(
network_interface,
hex_destination_ip,
hex_gateway,
bitmask_flags,
dummy_refcount,
dummy_use,
route_metric,
hex_destination_subnetmask,
dummy_mtu,
dummy_window,
dummy_irtt
)

linux_style_route_file = [ "Iface\tDestination\tGateway\tFlags\tRefCnt\tUse\tMetric\tMask\tMTU\tWindow\tIRTT" ]

try:
netstat_routes = _get_netstat_rn_ipv4_routes()
# Make sure the `netstat -rn -f inet` contains columns for Netif, Destination, Gateway and Flags which are needed to convert
# to the Linux Format
if len(netstat_routes) > 0:
missing_headers = []
if "Netif" not in netstat_routes[0]:
missing_headers.append("Netif")
if "Destination" not in netstat_routes[0]:
missing_headers.append("Destination")
if "Gateway" not in netstat_routes[0]:
missing_headers.append("Gateway")
if "Flags" not in netstat_routes[0]:
missing_headers.append("Flags")
if missing_headers:
raise KeyError("`netstat -rn -f inet` output is missing columns required to convert to the Linux /proc/net/route format; columns are [{0}]".format(missing_headers))
# Parse the Netstat IPv4 Routes
for netstat_route in netstat_routes:
try:
linux_style_route = _freebsd_netstat_rn_route_to_linux_proc_net_route(netstat_route)
linux_style_route_file.append(linux_style_route)
except Exception:
# Skip the route
continue
except Exception as e:
logger.error("Cannot read route table [{0}]", ustr(e))
return linux_style_route_file
Copy link
Contributor

Choose a reason for hiding this comment

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

The original implementation of this function handles all errors in function and doesn't propagate any exception further.

 try:
            with open('/proc/net/route') as routing_table:
                return list(map(str.strip, routing_table.readlines()))
        except Exception as e:
            logger.error("Cannot read route table [{0}]", ustr(e))

        return []

After you refactor this function, could you please ensure that the read_route_table function doesn't raise any exception so as to not break the agent

Copy link
Contributor Author

@kawaiivoldemort kawaiivoldemort Oct 4, 2019

Choose a reason for hiding this comment

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

done with 515b672


@staticmethod
def get_list_of_routes(route_table):
"""
Construct a list of all network routes known to this system.

:param list(str) route_table: List of text entries from route table, including headers
:return: a list of network routes
:rtype: list(RouteEntry)
"""
route_list = []
count = len(route_table)

if count < 1:
logger.error("netstat -rn -f inet is missing headers")
elif count == 1:
logger.error("netstat -rn -f inet contains no routes")
else:
route_list = DefaultOSUtil._build_route_list(route_table)
return route_list

def get_primary_interface(self):
"""
Get the name of the primary interface, which is the one with the
default route attached to it; if there are multiple default routes,
the primary has the lowest Metric.
:return: the interface which has the default route
"""
RTF_GATEWAY = 0x0002
DEFAULT_DEST = "00000000"

primary_interface = None

if not self.disable_route_warning:
logger.info("Examine `netstat -rn -f inet` for primary interface")

route_table = self.read_route_table()

def is_default(route):
return (route.destination == DEFAULT_DEST) and (RTF_GATEWAY & route.flags)

candidates = list(filter(is_default, self.get_list_of_routes(route_table)))

if len(candidates) > 0:
def get_metric(route):
return int(route.metric)
primary_route = min(candidates, key=get_metric)
primary_interface = primary_route.interface

if primary_interface is None:
primary_interface = ''
if not self.disable_route_warning:
logger.warn('Could not determine primary interface, '
'please ensure routes are correct')
logger.warn('Primary interface examination will retry silently')
self.disable_route_warning = True
else:
logger.info('Primary interface is [{0}]'.format(primary_interface))
self.disable_route_warning = False
return primary_interface

def is_primary_interface(self, ifname):
"""
Indicate whether the specified interface is the primary.
:param ifname: the name of the interface - eth0, lo, etc.
:return: True if this interface binds the default route
"""
return self.get_primary_interface() == ifname

def is_loopback(self, ifname):
"""
Determine if a named interface is loopback.
"""
return ifname.startswith("lo")

def route_add(self, net, mask, gateway):
cmd = 'route add {0} {1} {2}'.format(net, gateway, mask)
return shellutil.run(cmd, chk_err=False)
Expand All @@ -103,6 +405,14 @@ def is_missing_default_route(self):
specify the route manually to get it work in a VNET environment.
SEE ALSO: man ip(4) IP_ONESBCAST,
"""
RTF_GATEWAY = 0x0002
DEFAULT_DEST = "00000000"

route_table = self.read_route_table()
routes = self.get_list_of_routes(route_table)
for route in routes:
if (route.destination == DEFAULT_DEST) and (RTF_GATEWAY & route.flags):
return False
return True

def is_dhcp_enabled(self):
Expand Down
Loading