[MIR] octavia

[Summary]
Octavia provides Loadbalancing as a service as part of an OpenStack Cloud deployment.

Loadbalancers are provided as virtual machine appliances which run the Octavia amphorae agent for management control between the Octavia control plan and the loadbalancers (typically via a dedicated private virtual network).

The central control plan consists of an API service and three backend daemons - health-manager (which monitors Amphorae health, recreating if an LB fails), housekeeping (manages database housekeeping and the pool of spare amphorae workers) and worker (manages the allocation of Loadbalancers to end-users and other operations).

https://docs.openstack.org/octavia/queens/reference/introduction.html

Communication between the amphorae agent API and the central control plan API is secured with TLS using bi-direction certificates for authentication. This is part of the deployment process for Octavia rather than part of what the packaging provides.

This does need a security review, so assigning ubuntu-security

MIR team ack for inclusion in main (subject to security team review)

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
 - no other Dependencies to MIR due to this
   All identified as part of the MIR review.
 - no -dev/-debug/-doc packages that need exclusion

[Embedded sources and static linking]
OK:
 - no embedded source present
 - no static linking

[Security]
OK:
 - history of CVEs does not look concerning
   Some security history all effecting older Octavia versions
   than we have in Ubuntu (which is >= 5.0.0)
   https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=octavia

 - does not run a daemon as root
 - does not use webkit1,2
 - does not use lib*v8 directly
 - does not parse data formats
   API is REST based and parses JSON formatted data using the
   standard patterns as used by the majority of OpenStack
   services.

 - does not open a port
   API port (OK)
   Amphorae API port (see summary)

 - does not process arbitrary web content
 - does not use centralized online accounts
 - does not integrate arbitrary javascript into the desktop
 - does not deal with system authentication (e.g. pam), etc)

[Common blockers]
OK:
 - does not FTBFS currently
 - does have a test suite that runs at build time
   - test suite fails will fail the build upon error.
 - does have a test suite that runs as autopkgtest
 - The package has a team bug subscriber
   ubuntu-openstack
 - no translation present, but none needed for this case (user visible)?
   N/A
 - not a python package, no extra constraints to consider in that regard
 - no new python2 dependency
 - Python package that is using dh_python

[Packaging red flags]
OK:
 - Ubuntu does not carry a delta
   Ubuntu does carry a delta
 - Ubuntu does carry a delta, but it is reasonable and maintenance under control
   OpenStack in Ubuntu is typically ahead in terms of version compared
   to Debian and is managed by the Ubuntu OpenStack team.
 - symbols tracking not applicable for this kind of code.
 - d/watch is present and looks ok
 - Upstream update history is good
 - Debian/Ubuntu update history is good (but diverged)
 - ...

I'm starting in on this MIR and I'm surprised how many errors there are in the buildlogs. Both a version I built locally and some buildd build logs are noisy:

https://launchpad.net/ubuntu/+source/octavia/1:7.0.0+git2021012713.fbbc5f90-0ubuntu1/+build/20944559/+files/buildlog_ubuntu-hirsute-amd64.octavia_1%3A7.0.0+git2021012713.fbbc5f90-0ubuntu1_BUILDING.txt.gz

/usr/lib/python3/dist-packages/pbr/core.py:131: UserWarning: Unknown distribution option: 'requires_python'
  warnings.warn(msg)

Missing keepalived PID file /tmp/test/vrrp/octavia-keepalived.pid, skipping health heartbeat.
octavia.tests.unit.api.drivers.amphora_driver.v2.test_driver.TestAmphoraDriver.test_validate_flavor
octavia.tests.unit.api.drivers.amphora_driver.v2.test_driver.TestAmphoraDriver.test_validate_flavor ... ok
Failed to check keepalived and haproxy status due to exception , skipping health heartbeat.
Traceback (most recent call last):
  File "/<<BUILDDIR>>/octavia-7.0.0+git2021012713.fbbc5f90/octavia/amphorae/backends/health_daemon/health_daemon.py", line 121, in run_sender
    with open(keepalived_pid_path, 'r') as pid_file:
  File "/usr/lib/python3.9/unittest/mock.py", line 1093, in __call__
    return self._mock_call(*args, **kwargs)
  File "/usr/lib/python3.9/unittest/mock.py", line 1097, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
  File "/usr/lib/python3.9/unittest/mock.py", line 1152, in _execute_mock_call
    raise effect
OSError
Failed to check keepalived and haproxy status due to exception invalid literal for int() with base 10: 'foo', skipping health heartbeat.
Traceback (most recent call last):
  File "/<<BUILDDIR>>/octavia-7.0.0+git2021012713.fbbc5f90/octavia/amphorae/backends/health_daemon/health_daemon.py", line 122, in run_sender
    pid = int(pid_file.readline())
ValueError: invalid literal for int() with base 10: 'foo'
Keepalived is configured but not running, skipping health heartbeat.
Failed to check keepalived and haproxy status due to exception , skipping health heartbeat.
Traceback (most recent call last):
  File "/<<BUILDDIR>>/octavia-7.0.0+git2021012713.fbbc5f90/octavia/amphorae/backends/health_daemon/health_daemon.py", line 123, in run_sender
    os.kill(pid, 0)
  File "/usr/lib/python3.9/unittest/mock.py", line 1093, in __call__
    return self._mock_call(*args, **kwargs)
  File "/usr/lib/python3.9/unittest/mock.py", line 1097, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
  File "/usr/lib/python3.9/unittest/mock.py", line 1152, in _execute_mock_call
    raise effect
OSError
calculated hmac(hex=True): 64656665373265366363366664376235343632313263373937653933396336393236353035626462386566356533323138306561626637343036323965356366 not equal to msg hmac: 31393335326537356662646134393735346263633839633634376133386135343664626439613137323464316539333261363962303038653862323232383734 dropping packet
calculated hmac(hex=False): cb3d8c27bc9975ba1f9efc5a542a9a5cbf421f8aa28c79ec8f293647340cc0ae not equal to msg hmac: 3664626439613137323464316539333261363962303038653862323232383734 dropping packet
calculated hmac(hex=True): 3866613466306163663638346366653864363763353664356263383062346138626531666431663330326534...

I have serious reservations about this package.

The build logs are very messy and report a LOT of problems. How does one tell "these problems are completely normal" from "these problems indicate a regression in the package"?

There's many cases of building strings to execute, either via simple one-liner scripts, or via subprocess execution; it feels very strange to see a tool programmed in a high-level scripting language build up bash scripts through string operations and store them to disk, particularly into /sbin/.

I have reported two issues to the Openstack bug tracker for things that I believe may be security problems. It looks like Octavia may be written assuming malicious network inputs are impossible.
https://storyboard.openstack.org/#!/story/2008697
https://storyboard.openstack.org/#!/story/2008715

The MIR review included a comment that there's no root daemon in this package, but many of the operations it appears to perform require root-equivalent privileges. So I started looking for how the services in this package are started and had a great deal of difficulty figuring out how the debian/*.init.in files are turned into anything useful. (Hint for the future universe/o/openstack-pkg-tools/openstack-pkg-tools_113ubuntu1/pkgos.make ). I'm still not sure what user accounts are used when starting the services in this package -- or, by source inspection alone, how the services are started at all.

I don't think this package is ready for security support by the Ubuntu security team at this time. There's too many open questions about how this package functions and how quality assurance is maintained. I'm sorry I don't have concrete asks for this package, but consider:

- a debian/README.source file that describes how to work with this package
- an error-free build log, or at least notes emitted in the log at every expected error about what the expected error is, and why
- a clear statement that the HTTP endpoint is root-equivalent or changes to the HTTP server that would enforce stronger separation between API consumer and root.

Security team NAK for promoting octavia to main at this time.

Thanks

>> Unit Test Verbosity <<

I agree that the build log (specifically the unit test execution) is somewhat verbose during execution - I had a look through some of the references you made and it appears that the application logging in octavia is directly configured to stdout during unit test execution.

How does a package maintainer assert that this is OK? Mainly by ensuring that the unit tests are correctly inspecting and asserting the required return codes/exceptions raised etc during test execution - which again as far as I can tell and on reviewing the code I can see that this is the case.

>> debian/*.init.in <<

You'll find this pattern across all of the packages in the OpenStack package set; rather than maintain init scripts/systemd units/upstart configurations across > 100 daemons the Debian OpenStack team came up with a simple method of using a set of variables to generate all of those.

Its less relevant now (as only systemd is used) but it still saves copy/paste of systemd configuration between packages which are 99% exactly the same.

Daemons should all run under the 'octavia' account as defined by the PROJECT_NAME variable.

From the MIR meeting:
[16:33] <cpaelzer> jamespage: if you think with your explanations it should go back to re-review by them assign them
[16:34] <cpaelzer> I'll mark it incomplete for now as it does not seem to need a new MIR review

>> bugs reported upstream as potential security issues <<

An upstream developer responded to the bugs reported - the code identified relates to the amphora agent which is internal to octavia, and requires communication from the main octavia control process to the HTTP server in the amphora agent to be TLS encrypted with mutual authentication of client certificates.

Upstream acknowledged the potential bug but described the risk of exploit as low due to this mitigating control.

The OpenStack Charms for Octavia setup the TLS encryption and authentication as described.

I reviewed octavia 1:10.0.0-0ubuntu1 as checked into jammy. This shouldn't
be considered a full audit but rather a quick gauge of maintainability.

octavia is a load balancer used with the OpenStack cloud infrastructure.
Therefore, it is one of the network services inside OpenStack components.

- CVE History:
 - 3 medium CVEs in the history:
  CVE-2018-16856: RedHat specific issue, fixed
  CVE-2019-17134: upstream fixed and released
  CVE-2019-3895: upstream fixed and released
 - Upstream responded to two questions from the Security team, both took
  around 1 month to be replied, the answer is satisfying.
   - https://storyboard.openstack.org/#!/story/2008697
   - https://storyboard.openstack.org/#!/story/2008715
- Build-Depends:
 - depends on some packages from universe, such as
  python-setuptools, openstack-pkg-tools, python-sphinx*
 - for encryption, at least: python-cryptography (main), pyopenssl (main)
 - network, at least: networkx (main), netifaces (main),
  python-netaddr (main)
- pre/post inst/rm scripts
 - automatically added dh_* scripts
- init scripts:
 - daemon with start/stop/reload functions, template from
  openstack-pkg-tools package
- systemd units:
 - one for each daemon and 3 templates for amphorae
- dbus services
 - None
- No setuid binaries
 - None
- binaries in PATH:
 - Total of 13 binaries in PATH under /usr/bin
  -rwxr-xr-x root/root 157 2022-03-31 09:00 ./usr/bin/amphora-agent
  -rwxr-xr-x root/root 166 2022-03-31 09:00 ./usr/bin/amphora-health-checker
  -rwxr-xr-x root/root 161 2022-03-31 09:00 ./usr/bin/amphora-interface
  -rwxr-xr-x root/root 170 2022-03-31 09:00 ./usr/bin/haproxy-vrrp-check
  -rwxr-xr-x root/root 155 2022-03-31 09:00 ./usr/bin/octavia-api
  -rwxr-xr-x root/root 164 2022-03-31 09:00 ./usr/bin/octavia-db-manage
  -rwxr-xr-x root/root 164 2022-03-31 09:00 ./usr/bin/octavia-driver-agent
  -rwxr-xr-x root/root 166 2022-03-31 09:00 ./usr/bin/octavia-health-manager
  -rwxr-xr-x root/root 165 2022-03-31 09:00 ./usr/bin/octavia-housekeeping
  -rwxr-xr-x root/root 158 2022-03-31 09:00 ./usr/bin/octavia-status
  -rwxr-xr-x root/root 166 2022-03-31 09:00 ./usr/bin/octavia-worker
  -rwxr-xr-x root/root 1788 2022-03-31 09:00 ./usr/bin/octavia-wsgi
  -rwxr-xr-x root/root 168 2022-03-31 09:00 ./usr/bin/prometheus-proxy
- sudo fragments
 - None
- polkit files
 - None
- udev rules
 - None
- unit tests / autopkgtests:
 - 1958 unit tests. A little bit noisy, could be improved for clarity, as
  already discussed in other comments. Seems to have a good interaction
  with the code, plenty of tests executed.
 - autopkgtests only seems to check if the daemons are active, it could
  be considered enough as the unit tests does the majority of testing.
  They are passing for all architetures, except i386 that can't find
  octavia for this architeture, not clear why since only one build fits
  all archs.
- cron jobs
 - None
- Build logs:
 - Build is successfull but there are so many warnings, it was already
  discussed in other comments in this same bug, mainly on unit test.
 - Lintian has 1 error and 28 warning, nothing concerning but it worth
  ...