[MIR] fence-agents

Bug #1927004 reported by Lucas Kanashiro on 2021-05-03
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
fence-agents (Ubuntu)
Undecided
Ubuntu Security Team

Bug Description

[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 supported agents have been tested daily via Jenkins jobs. The idea is to have fence-agents-common and fence-agents-supported 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-supported 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-supported. 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, two new binary packages were created: fence-agents-common and fence-agents-supported. The -common package contains the common files used by supported and unsupported agents, and the -supported contains the agents supported by the Ubuntu Server team.

CVE References

Changed in fence-agents (Ubuntu):
assignee: nobody → Dan Streetman (ddstreet)

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) wrote :
Download full text (3.8 KiB)

[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 ...

Read more...

Changed in fence-agents (Ubuntu):
assignee: Dan Streetman (ddstreet) → Ubuntu Security Team (ubuntu-security)

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) wrote :

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) 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 this only needs security team review (and server team subscription to the package)

Dan Streetman (ddstreet) wrote :

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.

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) wrote :

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) wrote :

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) wrote :

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) wrote :

"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) wrote :

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.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers