[MIR] fence-agents

[Lucas Kanashiro]

[Availability]

fence-agents is present in the Ubuntu archive since Precise, it builds fine and tests pass in all supported architectures. It was promoted in Precise:

https://bugs.launchpad.net/ubuntu/+source/fence-agents/+bug/897492

In Trusty it was demoted, not sure about the reason.

[Rationale]

The fence-agents package provides a set of scripts used to fence (STONITH) nodes in a Corosync/Pacemaker cluster. It has been heavily used in HA solutions and the Ubuntu Server team is starting to support some of those scripts in the fence-agents-supported binary package. The curated agents have been tested daily via Jenkins jobs. The idea is to have fence-agents-common and fence-agents-base binary packages in main.

[Security]

A couple of CVEs were reported against fence-agents:

- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-10153
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-0104

They are already fixed in the archive, CVE-2014-0104 in version 4.0.17-1 and CVE-2019-10153 in version 4.3.3-2.

[Quality assurance]

The package is quite simple, no configuration file is needed and it works out-of-the-box. There is no outstanding bug reported upstream, nor in Debian and Ubuntu. The Debian maintainer is very active and willing to collaborate with us.

Upstream does not provide a test suite but all the scripts are tested during build time to check if they at least can load everything needed. We also have some DEP-8 tests in place (they do not cover most of the scripts). But thinking about the quality of the package, all the agents shipped in the fence-agents-base binary package are going to have automated tests running daily in our testing infrastructure.

[UI standards]

N/A

[Dependencies]

The binaries to be promoted to main are: fence-agents-common and fence-agents-base. All their runtime dependencies are in main already which are python3-pexpect and python3-pycurl.

[Standards compliance]

There is one error reported by lintian:

E: fence-agents-common: dir-or-file-in-var-run var/run/cluster/

This directory is created by the package because a couple of agents expect this directory:

agents/mpath/fence_mpath.py
184: options["--store-path"] = "/var/run/cluster"

agents/scsi/fence_scsi.py
16:STORE_PATH = "/var/run/cluster/fence_scsi"

Those two fence agents are not in the supported list yet but a bug was filed upstream to get it fixed:

https://github.com/ClusterLabs/fence-agents/issues/405

[Maintenance]

This package has been maintained by the Ubuntu Server team and this will continue to apply as part of the work on the HA (High Availability) stack.

[Background information]

In Impish, three new binary packages were created: fence-agents-common and fence-agents-base and fence-agents-extra. The -common package contains the common files used by supported and unsupported agents, and the -base contains the agents curated by the Ubuntu Server team.

[Lucas Kanashiro (lucaskanashiro)]

FWIW I submitted a pacemaker merge proposal which add changes to recommend fence-agents-supported. Since pacemaker is already in main the fence-agents-supported package will need to be pulled in.

https://code.launchpad.net/~lucaskanashiro/ubuntu/+source/pacemaker/+git/pacemaker/+merge/402284

[Dan Streetman (ddstreet)]

[Summary]
There are only some minor concerns listed below, which shouldn't block
this, so this is ACK from MIR team. Please see Notes and TODOs below.

This does need a security review (though I considered skipping it,
but want to err on the side of safety), so I'll assign ubuntu-security

List of specific binary packages to be promoted to main:
- fence-agents-common
- fence-agents-supported

Just for clarification, list of binary packages to remain in universe:
- fence-agents

Notes:
1) The Ubuntu Server team isn't yet subscribed.
   This is allowed per the new MIR process, but please remember to
   subscribe before the package is promoted to main.
2) The bug description states CVE-2019-10153 was already fixed,
   but that doesn't appear correct per the Ubuntu CVE page:
   https://ubuntu.com/security/CVE-2019-10153
   I don't think this medium CVE needs to block MIR, however.
3) I'm concerned about the split of the binary packages which
   leaves one package in universe; I have no problem with leaving
   a package in universe, but I'm concerned about the naming.
   After this MIR, the packages 'fence-agents-supported' and
   'fence-agents-common' will be in main and fully supported,
   but the generically-named binary package 'fence-agents' will
   remain in universe. I feel like this is going to lead to
   confusion by users of fence agents in the unsupported
   'fence-agents' binary package.

Required TODOs:
1) Ubuntu Server Team must subscribe to the package before
   it's promoted to main.

Recommended TODOs:
1) It might be adding more obviousness than is really needed,
   but I suggest renaming the binary package 'fence-agents'
   to 'fence-agents-unsupported' to make it clear (to users)
   that the agents in the package aren't supported.

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

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- 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
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port
- does not parse data formats
- 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 (eg, 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
- no translation present, but none needed for this case
- not a python/go package, no extra constraints to consider in that regard

Problems:
- The package needs Ubuntu Server team to subscribe

[Packaging red flags]
OK:
- Ubuntu does carry a delta, but it is reasonable and maintenance under control
- 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
- the (almost) current release is packaged
- promoting this does not ...

[Lucas Kanashiro (lucaskanashiro)]

Thanks for the review Dan! Some comments below:

- I'll make sure the Server team is subscribed to this package.

- Regarding the CVE-2019-10153 fix, it is mentioned in the changelog:

$ cat debian/changelog | grep -B2 -A4 CVE-2019-10153
fence-agents (4.3.3-2) unstable; urgency=high

  * fence_rhevm: add patch for CVE-2019-10153 (Closes: #930887)
    Including non-ASCII characters in a guest VM's comment or other fields
    would cause fence_rhevm to exit with an exception.

 -- Valentin Vidic <email address hidden> Sun, 23 Jun 2019 19:53:35 +0200

Which means it is fixed from Focal on. The Ubuntu security tracker might have missed this one, I do not know how it works.

- I brought up to the Server team this question mark that I have regarding the naming of those binary packages. After some discussion, the team agreed to implement a different approach:

  + Create a transitional binary package called "fence-agents". It will make the upgrade smoother for people already using it. This transitional package will depend on the supported and unsupported agents binary packages.
  + Rename the current "fence-agents-supported" to "fence-agents-core". Some members of the team said that "supported" might not be good because it might lead to different interpretations of the word. However, we are opened for other suggestions here.
  + Rename the current "fence-agents" with unsupported agents to "fence-agents-extra".

WDYT about the approach suggested above Dan? Does that sound good to you? I am going to implement it if we all agree, and in this case we will need to promote the following binary packages:

- fence-agents-common
- fence-agents-core (instead of fence-agents-supported)

[Alex Murray (alexmurray)]

Lucas - thanks for the heads up, I have marked this CVE-2019-10153 as not-affected for focal+ in the Ubuntu CVE Tracker - https://git.launchpad.net/ubuntu-cve-tracker/commit/?id=2b90bcddc25956d0c037e6119460f2778ee64c0a

[Dan Streetman (ddstreet)]

Lucas, thanks! The naming sounds good to me, using -core and -extra does seem better than supported/unsupported. No more concerns from me, so this only needs security team review (and server team subscription to the package)

[Dan Streetman (ddstreet)]

And of course please do update this bug description with the adjusted binary package names to promote to main, once the packaging is updated with the new names.

[Seth Arnold (seth-arnold)]

On Fri, May 07, 2021 at 03:21:26PM -0000, Dan Streetman wrote:
> Lucas, thanks! The naming sounds good to me, using -core and -extra does
> seem better than supported/unsupported. No more concerns from me, so

I know it's cliche to bikeshed on names, but:

- "core" is already an overloaded word with too many meanings

- the apparmor project has had far too many people installing the
  apparmor-profiles-extra package without realizing the inconsistent
  quality of the profiles. I don't think "extra" conveys a strong enough
  feeling

I suggest gstreamer's approach: -good, -bad, and -ugly. It's a bit coarse,
maybe even rude, but I think it does a better job of answering the
question "do I want to rely on this?"

-supported imho gives a promise it can't keep: supported by whom? for how
long? mint users or pop_OS users probably won't get support for this
package from their respective communities, but -good -bad and -ugly still
describes our impressions of a package's maturity and suitability for use.

Thanks

[Dan Streetman (ddstreet)]

I agree -supported is a bad term to use, it indeed is far too ambiguous; the meaning of 'supported' varies wildly depending on who you ask. I think even between us three discussing this the meaning probably is different ;-)

I don't have particularly strong feelings on the specific naming of each package, other than not using the plain name 'fence-agents'. I wonder if this topic is worth an email to ubuntu-devel-discuss to come up with some standardized naming for packages that are split into 'good' and 'bad' (and 'ugly') binary debs? Or maybe even debian-devel-discuss...possibly some of the old-school debian/ubuntu people might have already discussed this topic.

The -good, -bad, -ugly naming may be most appropriate but we probably shouldn't have people pick naming arbitrarily each time a package is split into multiple debs like this. For example another naming approach might be -stable and -unstable.

[Richard Harding (rharding)]

Thanks for the feedback folks. We spent some time thinking through this and the goals of the split and have come up with using -base and -extras. There's some prior art to this in the archive and we don't want to go with any naming convention that would indicate a "value" judgment on the two different packages.

The goal is to call out the ones that the Ubuntu Server team has tested and validated and the others which the community provides and are valid for specific use cases, but do not have that level of assurance from our team as we work with these packages.

[Dan Streetman (ddstreet)]

Thanks Rick.

> There's some prior art to this in the archive

I think it would really be good for the server team to follow up on this naming convention and get some agreement across the distro (not only for server team maintained packages), as we don't want different teams/people coming up with different naming conventions/styles for the same problem. And then it would be great if it was documented somewhere, maybe at https://packaging.ubuntu.com/html/packaging-new-software.html and/or we could add it to the MIR wiki page (though MIR isn't really the only, or even correct, time for package binary deb naming to be evaluated).

Also note that while 'base' and 'extra' might have been used before, you might not have seen Seth's comment 7 that indicated it didn't turn out very well for apparmor, so maybe 'base' and 'extra' aren't the best names, even if they've been used before.

[Richard Harding (rharding)]

"Also note that while 'base' and 'extra' might have been used before, you might not have seen Seth's comment 7 that indicated it didn't turn out very well for apparmor, so maybe 'base' and 'extra' aren't the best names, even if they've been used before."

Thanks, I did see that and we discussed it. That was my direct reply about avoiding terms that imply judgment.

I appreciate that it would be good to have a common ground on how to go about this problem. That said, in our discussions, we're finding different cases at play. In our case, the agents that we're looking to place into Main are ones that we feel cover some 80% of the typical use cases along with some particular ones that our team has an interest in adding testing and validation.

The other agents are not bad or any less, it's just an admission that the ones in Main our team will be working on that testing, adding documentation to the server guide, etc. We cannot provide that for all of them and thus the split to help reality and expectations align.

Honestly, most use cases will only leverage a single fence-agent or resource-agent and in an ideal world they'd all be independent packages that could be treated as individuals but the burden of 45+ different packages for this would be unsustainable from the team.

In our case -core doesn't work because you don't have to have that package installed to use anything from -extras. You also can't replace using something in -extras with something in base. Good/bad are not legitimate distinguishers imo. Supported is indeed overloaded and not the best wording.

In the end, I don't know that a single "this is how to do it" fits in the myriad of use cases we've discussed so far. Given that there's not a current single-use case I've encouraged the team to not let perfect be the enemy of the good and help move forward the work we're doing to build out a well-supported set of tools for HA on Ubuntu.

I very much appreciate the feedback and think we're heading in a better direction than started. I'm happy to dive more into the discussions and if there's a better pattern that's been identified that works with the concerns I've raised I'm happy to learn about them.

[Dan Streetman (ddstreet)]

I think you might be too focused on this particular package, as I'm not seeing any 'myriad' of use cases; the breakdown is:
1) a 'common' binary deb that provides something that other binary debs use
2) a 'maintained' or 'stable' binary deb that provides stuff that is 'maintained' and/or 'works mostly well'
3) a 'unmaintained' or 'unstable' binary deb that provides stuff that is 'not maintained' and/or 'doesnt work all the time'

The specific adjectives may vary, but I think that's the pattern here and I don't think this package (or the resource-agents package) is unique in having that kind of split.

All I'm saying is that if more than a couple packages in Ubuntu break down (some of) their binary debs in this kind of way, it would be good to have consistency in the naming of the packages by having a distro-wide discussion (however short that discusssion might be) and then documenting the guidelines in some appropriate place.

Otherwise, this will just get repeated for every package that gets split up in the same manner. The Ubuntu community might have some helpful input into this if we give them the chance to voice an opinion by starting a discussion on ubuntu-devel-discuss, instead of just deciding in-house.

In any case, I do think that discussing the naming in this MIR bug any further would absolutely be bikeshedding, so I recommend we don't continue here. This remains an ACK from the MIR team, as the naming was never a blocker for MIR.

[Lucas Kanashiro (lucaskanashiro)]

The description of this bug was updated after the renaming of the binary packages happened. Now we have src:fence-agents providing the following binary packages in Impish:

- fence-agents (transitional package)
- fence-agents-common
- fence-agents-base
- fence-agents-extra

If I got everything right we are waiting for the +1 from the security team. Is that correct?

[Dan Streetman (ddstreet)]

Yep this is just waiting on security team review.

Also just for easy clarification for the archive admin reading this later, the packages to promote to main are:

- fence-agents-common
- fence-agents-base

The packages to leave in universe are:

- fence-agents (transitional package)
- fence-agents-extra

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Seth Arnold (seth-arnold)]

I reviewed fence-agents 4.7.1-1ubuntu6 as checked into impish. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

fence-agents provides per-hardware or cloud details on how to forcibly
remove a machine from a high-availability setup, whether it's by yanking
power or by yanking network. It's nothing but sharp edges and corner
cases.

- CVE History:
  - two cves: one not validating tls certificates, one a non-ascii error
    causing a failure to fence

- Build-Depends: autoconf,
               automake,
               debhelper-compat (= 13),
               dh-python,
               iputils-ping,
               libglib2.0-dev,
               libnspr4-dev,
               libnss3-dev,
               libtool,
               perl,
               python3,
               python3-boto3,
               python3-googleapi,
               python3-oauth2client,
               python3-pexpect,
               python3-pycurl,
               python3-requests,
               python3-suds,
               time,
               xsltproc,
               libxml2-utils,
               libnet-telnet-perl
- pre/post inst/rm scripts?
  - Automatically-added dh_python3
- init scripts?
  - none
- systemd units?
  - none
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - many, all start /usr/sbin/fence_
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - I didn't spot any unit tests
  - autopkgtests provide a simple smoke test, better than nothing
- cron jobs?
  - none
- Build logs:
  - nothing too drastic, but they do give the impression they are unread
  - lintian unhappy:
E: fence-agents-common: dir-or-file-in-var-run var/run/cluster/

- Processes spawned?
  - that's the whole point of the project; the commands shut machines off,
    so pretty much only handling trusted inputs, so I didn't inspect calls
    carefully
- Memory management?
  - Python
- File IO?
  - under control of callers
- Logging?
  - extensive -- sometimes logs passwords. I opened an issue for this.
- Environment variable usage?
  - Nothing explicit
- Use of privileged functions?
  - extensive, the point of the project
- Use of cryptography / random number sources etc?
  - it's very easy to bypass certificate requirements
- Use of temp files?
  - two fixed path names in /tmp, one in 'dummy' the other in 'rhevm'.
    Probably not in the threat model of the project.
- Use of networking?
  - extensive, the point of the thing; nothing stood out as drastically
    unsafe but it does feel like it assumes everything is trusted
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - none
- Any significant Coverity results?
  - not really, some dead code due to bug, I opened an issue
- Any significant shellcheck results?
  - not really
- Any significant bandit results?
  - points out some exec(), system(), subprocess(), assert(), and
    something about unsafely handling xml. Nothing sounds like a surprise.

fence-agents sits in an interesting space with lots of networking and
highly privileged operations, command executions, etc, as the main goal.
It's...

[Christian Ehrhardt  (paelzer)]

https://launchpad.net/ubuntu/+source/fence-agents/4.7.1-1ubuntu7 resolved the last missing bit and is built in imposh-proposed.

It shows in https://people.canonical.com/~ubuntu-archive/component-mismatches-proposed.html so it is ready.
Setting Fix Committed.

Asking the Archive-Admins to please promote this to main.

[Steve Langasek (vorlon)]

Override component to main
fence-agents 4.7.1-1ubuntu6 in impish amd64: universe/admin/optional/100% -> main
fence-agents 4.7.1-1ubuntu6 in impish arm64: universe/admin/optional/100% -> main
fence-agents 4.7.1-1ubuntu6 in impish armhf: universe/admin/optional/100% -> main
fence-agents 4.7.1-1ubuntu6 in impish i386: universe/admin/optional/100% -> main
fence-agents 4.7.1-1ubuntu6 in impish ppc64el: universe/admin/optional/100% -> main
fence-agents 4.7.1-1ubuntu6 in impish riscv64: universe/admin/optional/100% -> main
fence-agents 4.7.1-1ubuntu6 in impish s390x: universe/admin/optional/100% -> main
7 publications overridden.

[Andy Whitcroft (apw)]

$ change-override -s impish-proposed -c main -t fence-agents
Override component to main
fence-agents 4.7.1-1ubuntu7 in impish: universe/admin -> main
1 publication overridden.
$ change-override -s impish-proposed -c main fence-agents-base fence-agents-common
Override component to main
fence-agents-base 4.7.1-1ubuntu7 in impish amd64: universe/admin/optional/100% -> main
fence-agents-base 4.7.1-1ubuntu7 in impish arm64: universe/admin/optional/100% -> main
fence-agents-base 4.7.1-1ubuntu7 in impish armhf: universe/admin/optional/100% -> main
fence-agents-base 4.7.1-1ubuntu7 in impish i386: universe/admin/optional/100% -> main
fence-agents-base 4.7.1-1ubuntu7 in impish ppc64el: universe/admin/optional/100% -> main
fence-agents-base 4.7.1-1ubuntu7 in impish riscv64: universe/admin/optional/100% -> main
fence-agents-base 4.7.1-1ubuntu7 in impish s390x: universe/admin/optional/100% -> main
fence-agents-common 4.7.1-1ubuntu7 in impish amd64: universe/admin/optional/100% -> main
fence-agents-common 4.7.1-1ubuntu7 in impish arm64: universe/admin/optional/100% -> main
fence-agents-common 4.7.1-1ubuntu7 in impish armhf: universe/admin/optional/100% -> main
fence-agents-common 4.7.1-1ubuntu7 in impish ppc64el: universe/admin/optional/100% -> main
fence-agents-common 4.7.1-1ubuntu7 in impish riscv64: universe/admin/optional/100% -> main
fence-agents-common 4.7.1-1ubuntu7 in impish s390x: universe/admin/optional/100% -> main
13 publications overridden.