Old-style multiple IP addresses in guest wrongly reported in guest_metrics

[ydirson]

The old 2014-era guest agent that is still the only one available on FreeBSD reports multiple addresses on VIF using e.g.

xenstore write attr/eth0/ip "1.2.3.4 1.2.3.5"

This results in 3 "addresses" getting reported by XenOrchestra, which seems to get "1.2.3.4 1.2.3.5" as an extra IP address, in addition to the ones parsed from the string.

Full details in https://xcp-ng.org/forum/topic/7625/xen-orchestra-netbox-sync-error

[robhoes]

Yes, I think that key is meant for one address only. I've never seen this with multiple addresses and space separated...

[ydirson]

Looks like it: here's what gets in the guest metrics (using xen-api nodejs tool from XO team):

root@172.16.210.11> findAll({ $type: 'VM', uuid: 'b9cae43e-8e3f-ae16-fd46-4def1522e9b4' })[0].$guest_metrics.networks
{ '0/ip': '1.2.3.1 1.2.3.2', '0/ipv4/0': '1.2.3.1 1.2.3.2' }

If the protocol never supported this, then the old agent is just more buggy than we thought (and XAPI trusts it a bit too much ;)).

[robhoes]

Support for multiple addresses was added through the new format, where each address has its own key. So in the example above, it should be

0/ipv4/0 = "1.2.3.1"
0/ipv4/1 = "1.2.3.2"

[robhoes]

See https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_guest_agent.ml#L77-L99.

[ydirson]

Wouldn't it be reasonable to expect XAPI to sanitize the data it is relaying from the guests, and ensure it is only exposing valid IP addresses, at least in the new-style entries?

Maybe for compatibility with guests using those really old guest tools (I expect to provide a better one for FreeBSD guests soonish but we're not there yet) we should allow old-style to expose a space-separated list IPs, for the sake ot backward compatibility (eg. Xen Orchestra has been parsing it as such for some time).

[minglumlu]

Had a space-separated list IPs been supported or documented? If not, I think even sanitization in XAPI couldn't get it correct in this case. I.E. what's the expected output of XAPI in handling a space-separated list IPs?

[ydirson]

what's the expected output of XAPI in handling a space-separated list IPs?

Let's put it another way: there are FreeBSD guests in the wild with more than one IP on their VIF (and that's not an unreasonable thing, the only problem is that FreeBSD guests were never provided with an official guest agent, so they stick with their old fork of the old shell version). Such guests do cause XAPI to report space-separated IPs, so the question would rather be "what are XAPI clients doing with that today?" (in the case of XO the answer is "parsing this de-facto space-separated list).

The point is, allowing space-separated lists in old-style entries officially today cannot cause the situation to be worse than what we already have.
Now in new-style ones it would just make no sense, as the structure is explicitly made to handle multiple IPs. If we acknowledge that old-style entries are space-separated, then XAPI should possibly be adjusted to do space-splitting.

Other options could be:

• add sanitation to new-style entries only and leave old-style entries handling as they are: conversion of space-separated old-style would fail the new-style sanitation, and the guest metrics would only expose the old-style string
• sanitize old-style entries to accept only single IP, completely breaking address reporting for BSD guests
• stop parsing at the first space and sanitize that, partially breaking address reporting for BSD guests

[minglumlu]

I think sanitization makes sense, but supporting space-separated lists doesn't. Why need to consider an un-supported format? The interesting thing is without sanitization, the client would have a chance to support the space-separated lists. With sanitization, however, the information would be dropped by XAPI and couldn't be parsed by client anymore.

[ydirson]

What about this variation, then:
If we only add sanitation to the new-style items exposed by XAPI, it would fail and they would not be published, whereas the old-style 0/ip, getting no sanitation, would be exposed without filter, as it is today, and would avoid breaking FreeBSD guests.

[minglumlu]

I can't see any major problems on this variation at present. But unlike space-separated lists, old style is still in support, so ideally it should be updated with the sanitization as well. But this will break the BSD guests......