[MIR] xe-guest-utilities

[Dimitri John Ledkov]

[Availability]
* Since pre-precise, available on all architectures
* Previously it was an arch:all, since recently it is arc:any as it got rewritten in golang upstream.

[Rationale]
* Multiple clouds which use XEN like hypervisors, use xe-guest-utilities to communicate with the XEN host, to retrieve cloud-config drive.

[Security]
* Ships a daemon
* Adds a mountpoint of /proc/xen
* Uses /proc/xen
* Adds udev rules for hotplug cpus
* golang...

[Quality assurance]
* well maintained upstream
* well maintain debian package
* simply packaging

[Dependencies]
* init-system-helpers... the rest is statically linked golang

[Standards compliance]
* Complies with debian policy

[Maintenance]
* little, to none required.

[Background information]
* Used by some xen based Openstack public clouds.

[Mathieu Trudel-Lapierre (cyphermox)]

Let's get this reviewed by Security.

I have not looked at it at all yet, but it is a daemon dealing with Xen domains, etc.

[Seth Arnold (seth-arnold)]

/usr/sbin/xe-linux-distribution is pretty gross code; shellcheck reports two dozen issues, many of which might be reliability issues. The use of 'eval' on data returned from the various consulted files might be a security issue. (While one might expect that the files used to identify e.g. Ubuntu would be under strict control, I can't promise the same about files used to identify Asianux, Turbo, Kylin, Yinhe, Linx, etc.)

Can we instead just write the expected values to /var/cache/xe-linux-distribution ourselves in the /lib/systemd/system/xe-daemon.service file?

Thanks

[Seth Arnold (seth-arnold)]

Here's the cppcheck output -- thankfully, not everything here makes it into our packages. (For example, the code in ./mk/debian/xe-guest-utilities.postinst installs new APT sources. This is not ideal. As far as I can tell we don't ship this.)

./mk/testcases/lsb:5:6: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/testcases/lsb:15:17: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/testcases/lsb:20:3: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/testcases/lsb:33:16: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/testcases/lsb:34:25: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/testcases/lsb:35:21: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/xe-linux-distribution:43:11: warning: Declare and assign separately to avoid masking return values. [SC2155]
./mk/xe-linux-distribution:46:2: warning: MAJOR appears unused. Verify it or export it. [SC2034]
./mk/xe-linux-distribution:47:2: warning: MINOR appears unused. Verify it or export it. [SC2034]
./mk/xe-linux-distribution:48:2: warning: DISTRO appears unused. Verify it or export it. [SC2034]
./mk/xe-linux-distribution:49:2: warning: UNAME appears unused. Verify it or export it. [SC2034]
./mk/xe-linux-distribution:76:10: warning: Quote this to prevent word splitting. [SC2046]
./mk/xe-linux-distribution:76:39: note: You don't break lines with \ in single quotes, it results in literal backslash-linefeed. [SC1004]
./mk/xe-linux-distribution:84:72: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/xe-linux-distribution:138:10: warning: Quote this to prevent word splitting. [SC2046]
./mk/xe-linux-distribution:141:10: warning: Quote this to prevent word splitting. [SC2046]
./mk/xe-linux-distribution:143:123: note: This word is outside of quotes. Did you intend to 'nest '"'single quotes'"' instead'? [SC2026]
./mk/xe-linux-distribution:167:14: note: expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]]. [SC2003]
./mk/xe-linux-distribution:167:19: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/xe-linux-distribution:176:68: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/xe-linux-distribution:213:10: warning: Quote this to prevent word splitting. [SC2046]
./mk/xe-linux-distribution:231:63: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/xe-linux-distribution:252:51: note: Use '[:upper:]' to support accents and foreign alphabets. [SC2019]
./mk/xe-linux-distribution:252:57: note: Use '[:lower:]' to support accents and foreign alphabets. [SC2018]
./mk/xe-linux-distribution:270:10: warning: Quote this to prevent word splitting. [SC2046]
./mk/xe-linux-distribution:270:17: note: Double quote to prevent globbing and word splitting. [SC2086]
./mk/xe-linux-distribution:270:68: note: You don't break lines with \ in single quotes, it results in literal backslash-linefeed. [SC1004]
./mk/xe-linux-distribution:271:68: note: You don't break lines with \ in single quotes, it results in literal backslash-linefeed. [SC1004]
./mk/xe-linux-distribution:298:...

[Seth Arnold (seth-arnold)]

I reviewed xe-guest-utilities version 7.10.0-0ubuntu1 as checked into
bionic. This should not be considered a full security audit but rather a
quick gauge of maintainability.

- collects and reports distribution version, uname, memory, IP addresses,
  MAC addresses, memory information, balloon status, CPUs, etc. through
  xenstore data collection
- No CVEs in our database
- Build-Depends: debhelper, gawk, golang | gccgo-go (<< 1.3)
- Does not itself daemonize
- pre/post inst/rm scripts automatically generated
- Two systemd unit files, one to mount /proc/xen, one to start the
  xe-daemon
- No dbus services
- No setuid files
- xe-daemon and xe-linux-distribution in path
- No sudo fragments
- Udev rule appears to auto-online new CPUs
- There's some testing framework of some sort but it doesn't appear to be
  run during the build; I don't see how it would help us much.
- Clean build logs

- Subprocesses are spawned extensively to collect data; it appears to use
  go's array-based execve() wrappers
- standard go memory handling
- Opens files based on a few well-known paths as well as glob() on other
  paths, including /dev/, /sys/class/net/, /sys/block/*/device
- Logging can go to syslog or stderr, looked okay
- I didn't spot environment variable use
- I didn't spot explicit privileged actions
- No cryptography
- No networking
- No privileged portions of code
- No temporary files
- No webkit
- No policykit

- xe-linux-distribution is fairly gross code, and may present security
  issues. I'd really like to ditch this code entirely. Ideally the daemon
  would just run lsb_release -a and uname -a and return that unchanged to
  xenstore.

- enumNetworkAddresses() discards err from runCmd() calls

Security team ACK for promoting xe-guest-utilities to main, but it'd be
really nice to remove the shell script for 18.10.

Thanks

[Matthias Klose (doko)]

filed LP: #1766451 for the shell script removal

[Matthias Klose (doko)]

Override component to main
xe-guest-utilities 7.10.0-0ubuntu1 in bionic: universe/admin -> main
xe-guest-utilities 7.10.0-0ubuntu1 in bionic amd64: universe/admin/optional/100% -> main
xe-guest-utilities 7.10.0-0ubuntu1 in bionic arm64: universe/admin/optional/100% -> main
xe-guest-utilities 7.10.0-0ubuntu1 in bionic armhf: universe/admin/optional/100% -> main
xe-guest-utilities 7.10.0-0ubuntu1 in bionic i386: universe/admin/optional/100% -> main
xe-guest-utilities 7.10.0-0ubuntu1 in bionic ppc64el: universe/admin/optional/100% -> main
xe-guest-utilities 7.10.0-0ubuntu1 in bionic s390x: universe/admin/optional/100% -> main
7 publications overridden.

[Matthias Klose (doko)]

closing ...