[MIR] masakari and masakari-monitors

[Corey Bryant]

[Availability]
Currently in universe.

[Rationale]
Masakari is an OpenStack project that we're ready to support in main.

[Security]
No security history.

[Quality Assurance]
Packages work out of the box with no prompting. There are no major bugs in Ubuntu and there are no major bugs in Debian. Unit tests are run during build.

[Dependencies]
All are in main.

[Standards Compliance]
FHS and Debian Policy compliant.

[Maintenance]
Python packages that the OpenStack Team will take care of.

[Background]
Masakari provides Virtual Machine High Availability (VMHA) service for OpenStack clouds by automatically recovering the KVM-based Virtual Machine(VM)s from failure events such as VM process down, provisioning process down, and nova-compute host failure. It also provides API service for manage and control the automated rescue mechanism.

[James Page (james-page)]

As we're late in cycle and this is a preview feature for disco/openstack stein happy to defer this until next cycle when we've had more field testing of this feature.

[Christian Ehrhardt  (paelzer)]

Assigned to James until this testing in the field happened.
Will be switched back to new when it is ready for review later in the 19.10 cycle.

[Christian Ehrhardt  (paelzer)]

I hope I find the time to review those two tomorrow morning

[Christian Ehrhardt  (paelzer)]

[Summary]
Coming from the same source the review results seemed virtually identical so let us handle it as one.

=> MIR Team ack to masakari and masakari-monitors.

@Security:
- IMHO this needs a review and AFAIK it is 20.04 material so it would have
  to be fast

@Openstack TODOs:
- some tests trigger errors which might be true issues, please check (low prio)

[Duplication]
Masakari provides Virtual Machine High Availability (VMHA) service for OpenStack
clouds, nothing else provides that.
The monitors are essentially the probes it uses to detect these events.

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

[Security]
OK:
- history of CVEs does not look concerning
- does not use webkit1,2
- does not use lib*v8 directly
- does not process arbitrary web content
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not use centralized online accounts

Problems:
- It does listen on a port (default conf says 15868) for its API interactions
- It does parse data formats as part of the API handling
- does not run a daemon as root, but is reachable as integrated through libapache2-mod-wsgi-py3 which could be running as root

As usual the approach is better safe then sorry, therefore this will need a security review.

[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
- openstack is subscribed
- no translation present, but none needed for this case (not non-admin visible)
- no new python2 dependency
- used dh_python

[Packaging red flags]
OK:
- Ubuntu does carry not carry a delta, it is Ubuntu-only right now
- symbols tracking not applicable for this kind of code.
- d/watch is present
- Upstream update history is good
- Ubuntu update history is good (Is Ubuntu only atm)
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- not using Built-Using
- not a golang package for further checks in that regard

Problems:
- d/watch didn't follow the update to version 9, but that isn't too important as long as the openstack team actively maintains it

[Upstream red flags]
OK:
- no hard Errors/warnings during the build (I've found a few, see below)
- no incautious use of malloc/sprintf (python code)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
  - the only bugs known are driven and under control by the openstack Team
- no dependency on webkit, qtwebkit, seed or libgoa-*
- no embedded source copies
- not part of the UI for extra checks

Problems:
The tests trigger a bunch of non fatal issues like:
- TypeError: 'int' object is not an iterator
- TypeError: foo() missing 1 required positional
- TypeError: 'NoneType' object is not callable
- KeyError: 'hypervisor_name'
- KeyError: 'id'
- LookupError: No section 'nonexistent app'
I've skipped those that seem intention...

[Alex Murray (alexmurray)]

I reviewed masakari 9.0.0~b2~git2020020609.8b122a8-0ubuntu2 as checked into
focal. This shouldn't be considered a full audit but rather a quick gauge
of maintainability.

masakari is a OpenStack component providing a high availability service for
instances - this allows KVM-based virtual machine instances to be restarted
automatically in the event of failure.

- No CVE History
- No particularly security relevant Build-Depends
  - apache2-dev, debhelper, dh-apache2, dh-python, openstack-pkg-tools,
    python3-all, python3-setuptools, python3-sphinx,
- pre/post inst/rm scripts
  - A bunch of automatically added scripts (from dh_apache2, dh_python3,
    dh_systemd_enable, dh_installinit) to setup apache site config etc for
    the API endpoint, compile python code and setup systemd units /
    initscript appropriately
  - postinst to add masakari group and user and ensure directories and
    databases etc are owned by the masakari user/group
    - NOTE: there appears to be a typo in the postinst script which uses
      chmod instead of chown:
      - chmod masakari:masakari /var/lib/masakari/masakari.sqlite
    - ^^^^^ should this be chown instead?
- init scripts
  - Simple init script to declare the masakari-engine and then uses
    openstack-pkg-tools to automatically generate the rest
- systemd units
  - Simple service to call out to the init script to start directly but
    managed by systemd as a service
- No dbus services
- No setuid binaries
- python3-masakari provides the following binaries in PATH
  - /usr/bin/masakari-api
  - /usr/bin/masakari-engine
  - /usr/bin/masakari-manage
  - /usr/bin/masakari-status
  - /usr/bin/masakari-wsgi
- No sudo fragments
- No polkit files
- No udev rules
- unit tests / autopkgtests
  - autopkgtest is a smoke-test ensures all daemons appear to start
    correctly
  - unit tests are run during build
- No cron jobs
- Build logs:
  - A few unit test failures during the build
  - lintian complains about recursive chown in postinst and systemd service
    wrapping init script (plus binaries without man-pages and an embedded
    copy of jquery/bootstrap etc) but these are not major issues

- No processes spawned
- Memory management is python
- No notable File IO
- Logging is clean
- No environment variable usage (other than in unit tests)
- No use of privileged functions
- No use of cryptography / random number sources etc
- No use of temp files (other than in unit tests)
- Network handling looks pretty safe
- No use of WebKit
- No use of PolicyKit

- No significant static analysis results from coverity / bandit

Other than the chmod/chown typo in the postinst masakari looks pretty good
and maintainable - Security team ACK for promoting masakari to main once
the postinst typo is investigated/fixed.

[Alex Murray (alexmurray)]

I reviewed masakari-monitors 9.0.0~b1~git2019121714.b717be1-0ubuntu1 as
checked into focal. This shouldn't be considered a full audit but rather a
quick gauge of maintainability.

masakari-monitors provides various binary packages containing separate
monitor instances for masakari-engine - these allow to monitor hosts /
processes etc and integrate with the masakari API to provide high
availability of instances in OpenStack.

- No CVE History
- No security relevant Build-Depends
- pre/post inst/rm scripts
  - Like other openstack packages, various auto-generated versions - these
    all look fine
  - masakari-monitors-common provides postinst to create the
    masakarimonitors group and user and chown/chmod various
    files/directories as appropriate
    - There doesn't appear to be an equivalent prerm - should there be?
- init scripts
  - Like masakari, uses the openstack packaging helpers to auto-generate
    init scripts for each monitor daemon - these look fine
- systemd units
  - Like masakari, these wrap the init scripts generated above
- No dbus services
- No setuid binaries
- python3-masakari-monitors provides the following binaries in PATH (one
  for each monitor):
  - /usr/bin/masakari-hostmonitor
  - /usr/bin/masakari-instancemonitor
  - /usr/bin/masakari-introspectiveinstancemonitor
  - /usr/bin/masakari-processmonitor
- sudo fragment in maskari-monitors-common to allow the masakarimonitors
  user to execute as root the privsep-helper binary (provided by oslo) as
  needed
- No polkit files
- No udev rules
- unit tests / autopkgtests
  - Like masakari, simple smoketest as autopkgtest to ensure daemons start
    / run
  - unit tests run during build
- No cron jobs
- Build logs are pretty clean
  - Like masakari lintian complains of systemd unit wrapping init script
    and too other minor issues

- Processes spawned
  - The process monitor uses ps to monitor for processes - this
    looks safe and doesn't appear to allow command-injection
- Memory management is python
- File IO
  - The process monitor loads YAML directly from
    /etc/masakarimonitors/process_list.yaml which is owned by root, group
    masakarimonitors so this should be fine as this is trusted
- Logging looks clean
- No obvious environment variable usage
- No obvious use of privileged functions
- No use of cryptography / random number sources etc
- No use of temp files
- No obvious direct use of networking
- No use of WebKit
- No use of PolicyKit
  - Instead uses oslo for privilege separation

- No significant static analysis (coverity/bandit)

Security team ACK for promoting masakari-monitors to main.

[James Page (james-page)]

masakari unit test error messages:

- TypeError: 'int' object is not an iterator

I could not find this error in the latest build log

- TypeError: foo() missing 1 required positional

By design - the test intentionally raises a TypeError.

- TypeError: 'NoneType' object is not callable

2020-02-17 16:27:11.508 10115 INFO masakari.test_uri_length_limit.wsgi.server [-] Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/eventlet/wsgi.py", line 566, in handle_one_response
    result = self.application(self.environ, start_response)
TypeError: 'NoneType' object is not callable

Looks suspect but the test looks fine and is still passing - worth further investigation.

- KeyError: 'hypervisor_name'

Not effecting the test but generates random noise

https://review.opendev.org/#/c/709675/

- KeyError: 'id'

Not effecting the test but generates random noise

https://review.opendev.org/#/c/709679/

- LookupError: No section 'nonexistent app'

By design - the test is validating that a top-level application definition does not exist.

[James Page (james-page)]

From masakari security review:

  - postinst to add masakari group and user and ensure directories and
    databases etc are owned by the masakari user/group
    - NOTE: there appears to be a typo in the postinst script which uses
      chmod instead of chown:
      - chmod masakari:masakari /var/lib/masakari/masakari.sqlite
    - ^^^^^ should this be chown instead?

yes it should - corrected and uploaded to focal.

@paelzer aside from that odd TypeError exception thrown in eventlet in one test the other queries around the unit test ERROR messages are non-issues or log cruft (two of which I have submitted fixes for but don't intend to hold as patches - we'll get them with the next set of snapshots).

[Alex Murray (alexmurray)]

@james-page - apologies for not looking closer at those unit test "failures" - thanks for investigating them and fixing the postinst typo :)

[Christian Ehrhardt  (paelzer)]

Ready for promotion but has gained some depending packages.
I set the status of this MIR here accordingly.

Jamespage is looking into those (new) dependencies.