Support multiple "equivalent" security releases (per branch)

[jackaponte]

There are currently two secure releases of Backdrop core, 1.12.1 and 1.11.5, as can be seen on https://github.com/backdrop/backdrop/releases. The 1.11.5 release was needed because the update to the 1.12.x branch causes a PHP fatal error when the old options_element contributed module is still installed, as documented on issue #3485.

Even though 1.11.5 is a security release that includes the needed fixes, the Backdrop core update module shows an alert saying that a security update is needed on the Backdrop status page and, if configured to do so, sends email notifications saying that a security update is available that should be applied immediately.

Actual behavior
Backdrop core complains of a needed security update when a secure version of Backdrop is installed that is not the highest numbered security release available (e.g. Backdrop 1.11.5 vs 1.12.1).

Expected behavior
Backdrop core should be able to tell that two security releases on two different branches (e.g. 1.11.x and 1.12.x) are equally secure and should not show a warning on the status page or send email notifications from the update module.

[jackaponte]

Filing this bug report after a conversations with folks in Gitter back on January 29, see https://gitter.im/backdrop/backdrop-issues?at=5c51003441775971a0a6594e.

[klonos]

Thanks for filing this ticket @jackaponte 👍

It might be worth checking how D8 has solved the same problem...

With https://www.drupal.org/sa-core-2019-002 the security fix was applied to both 8.5.x and 8.6.x branches, with 8.5.9 and 8.6.6 being the respective security releases. On a site running 8.5.10, you do not get any notification of a security release.

How security releases work in Backdrop land is that we manually tick a Security release checkbox for the release node on b.org:

...but there currently is no way to mark 2 or more releases as "equivalent". So we need to figure the best way to go about doing that. for core, but also for contrib (because contrib can also have multiple branches, thus the same issue).

The next bit we'd need to figure is how the logic that determines if any release is a security release should work on the side of the actual product, to incorporate the change on the b.org side.

[klonos]

Here's an idea:

We could create a "Security release" content type, with just a single, multi-value entity reference field (which would replace/deprecate the per-release "Security release" checkbox). Then, each time we'd have a security release, we'd create a piece of content of that type, give it the SA-* string as a title. We could then reference as many "equivalent" releases to it via the "Security release" reference field.

We'd then need to change the logic in Backdrop core, to pull these from b.org, then compare the current version running on the site against the latest security release, and grab the security release for the respective branch (if there is one).

I think that would work, but there may be an easier way. TBD...

[olafgrabienski]

It might be worth checking how D8 has solved the same problem...

I guess, this is the respective D8 issue: If the next minor version of core has a security release, status still says "Security update required!" even if the site is on an equivalent, secure release already

[klonos]

Thanks for digging that up @olafgrabienski!

[klonos]

...before that issue, which aims to solve the problem in a more robust way, the _update_equivalent_security_releases function was introduced as a workaround. It uses hardcoded version numbers:

function _update_equivalent_security_releases() {
  trigger_error("_update_equivalent_security_releases() was a temporary fix and will be removed before 9.0.0. Use the 'Insecure' release type tag in update XML provided by Drupal.org to determine if releases are insecure.", E_USER_DEPRECATED);
  switch (\Drupal::VERSION) {
    case '8.3.8':
      return [
        '8.4.5',
        '8.5.0-rc1',
      ];
    case '8.3.9':
      return [
        '8.4.6',
        '8.5.1',
      ];
    case '8.4.5':
      return [
        '8.5.0-rc1',
      ];
    case '8.4.6':
      return [
        '8.5.1',
      ];
    case '8.4.7':
      return [
        '8.5.2',
      ];
    case '8.4.8':
      return [
        '8.5.3',
      ];
  }
  return [];
}

[quicksketch]

Here's another possibility: Each release includes in the XML feed the security release identifier, e.g. CORE-2019-001. Then when checking for security updates, the current version would check all newer releases for the already existing "security" flag, and check that all versions within the same minor release don't have the same security release identifier.

For reference, here's what our update feeds currently look like: https://updates.backdropcms.org/release-history/backdrop/1.x

Currently:

<release>
   <name>Backdrop 1.12.1</name>
   <version>1.12.1</version>
   <version_major>1</version_major>
   <version_minor>12</version_minor>
   <version_patch>1</version_patch>
   <status>published</status>
   <release_link>https://github.com/backdrop/backdrop/releases/tag/1.12.1</release_link>
   <download_link>https://github.com/backdrop/backdrop/releases/download/1.12.1/backdrop.zip</download_link>
   <date>1547680740</date>
   <filesize>9407029</filesize>
   <security_update>true</security_update>
   <terms>
   </terms>
</release>

I think we could make an additional entry for <security_identifiers> that contains 1 or more <security_identifier> tags, like this:

<security_identifiers>
  <security_identifier>SA-CORE-2019-001</security_identifier>
  <security_identifier>SA-CORE-2019-002</security_identifier>
</security_identifiers>

Then have update module do additional checking for if all the releases within it's minor version have all the identifiers for the newer versions. Potentially we could even link to each security vulnerability individually, which could help encourage updating.

[klonos]

Yep, that'd work too. How were you thinking that'd work implementation-wise on the b.org side? Would it be an additional field where we'd need to fill in the security ID string (or select it if a drop-down)?

[quicksketch]

Would it be an additional field where we'd need to fill in the security ID string (or select it if a drop-down)?

Yeah, that was my thinking. There is already a checkbox for "Security Release". If checked, a multi-value text field appears where you enter the SA identifiers. We could use references to the SA nodes, but IMO strings would be simple and do the job.

[jenlampton]

a multi-value text field appears where you enter the SA identifiers.

It looks like the checkbox is a custom element from project module, so I'm guessing this new form element should be one too, not a field-API field. Is that what you had in mind @quicksketch ?

[quicksketch]

Yes putting it in project module makes the most sense. As it provides the specialized feeds that are consumed by Update module.

[jenlampton]

It looks like this issue is blocked by changes to project module, so I've created an issue in that queue too: backdrop-contrib/project#34

[ghost]

An issue without a near-ready PR isn't eligible for the bug-fix milestone (probably not one that's blocked anyway). So removing milestone.