Problem/Motivation

Thanks for @pwolanin raising this issue again

\Drupal\package_manager\ProjectInfo::getInstallableReleases relies solely on drupal.org Update to determine what updates are available.

These update XML files are not currently slated to TUF protected according to #3343490: Deploy rugged for TUF signing to production

Proposed resolution

We should rely on TUF protected resources to determine on what versions are secure
to options

Once #3343490: Deploy rugged for TUF signing to production is done on drupal.org then we can do #3358504: Require PHP-TUF's Composer integration plugin

After this all of our downloads of Composer metadata and targets will be TUF protected.

Then we can change the authority of what releases are available and what versions are secure to Composer instead of Update XML.

  1. composer show drupal/core -a --format=json should be able to tell us all available versions
  2. composer audit --format=json should be able to tell us which versions are secure

Since #3350568: Drop support for Composer 2.2, require Composer >=2.5.5 we only support Composer versions that have composer audit

In either case we would have to combine this with the Update XML to provide information about security releases.

Remaining tasks

TBD. Will this allow us to drop the dependency on the Update module?
Right now we don't send emails about security updates because the Update module does that. But if we don't trust the Update XML then should we not rely on it to send the emails?
We can't really change the Update module to move from Update XML to composer commands because that would require any site that wanted this functionality have the ability to run composer commands(though not staged)

User interface changes

API changes

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

tedbow created an issue. See original summary.

tedbow’s picture

tedbow’s picture

Title: Rely on TUF protected resources to determine updates are available » Rely on TUF protected resources to determine which updates are available
tedbow’s picture

good news it looks like the new composer audit command is aware of drupal security status. I made a project with Drupal core 10.0.7 which is insecure

Running composer audit --format=json got me

{
    "advisories": {
        "drupal/core": [
            {
                "advisoryId": "PKSA-h7d4-5mdz-2965",
                "packageName": "drupal/core",
                "affectedVersions": ">=7.0.0,<7.96|>=9.0.0,<9.4.14|>=9.5.0,<9.5.8|>=10.0.0,<10.0.8",
                "title": "Access bypass in Drupal core",
                "cve": "CVE-2023-31250",
                "link": "https://github.com/advisories/GHSA-8849-cv9f-vccm",
                "reportedAt": "2023-04-26T21:30:37+00:00",
                "sources": [
                    {
                        "name": "GitHub",
                        "remoteId": "GHSA-8849-cv9f-vccm"
                    }
                ]
            }
        ]
    }
}

So we could probably use this to determine if need to do updates

wim leers’s picture

Issue tags: -alpha-target +alpha target
wim leers’s picture

There's not enough detail yet in the issue summary for this to be completely actionable (which is why it is tagged Needs issue summary update). Assigning to @tedbow for that. Note that there is no mention at all so far of finding non-security updates using this mechanism, i.e. bug fix releases. composer audit does not seem to be able to give us that.

I asked @omkar.podey to go ahead and start adding at least 2 methods to ProjectInfo to help shape the direction, based on the title ("determine which update is available" [using a different mechanism]):

  1. determine whether there is some update available: infer this based on the currently installed version being marked as insecure
  2. determine which version is the first secure version: infer this based on the version just beyond the range being marked as insecure — e.g. >=10.0.0,<10.0.8 implies that 10.0.8 is the first secure version
omkar.podey’s picture

Assigned: Unassigned » tedbow
tedbow’s picture

Priority: Major » Minor

Sorry postponing this issue for now.

I need to triage issues and figure what we need to work to finish #3284443: Enable unattended updates. This issue won't block that because we don't have TUF implementation done on drupal.org yet until #3343490: Deploy rugged for TUF signing to production. So could switch to using composer audit but that would not be TUF protected

Actually what could happen now is an issue summary update:

  1. Needs to updated to reflect relying on composer audit would require Composer > 2.4
  2. Need research when Composer 2.2 is no longer support and post that on the issue

Changing to minor for now because the current actions are not urgent but once is close to be done this becomes major or critical.

tedbow’s picture

Assigned: tedbow » Unassigned

anyone can do the 2 items in #8 and then assign it back to me

omkar.podey’s picture

Assigned: Unassigned » omkar.podey
omkar.podey’s picture

Issue summary: View changes
omkar.podey’s picture

Assigned: omkar.podey » tedbow

updated issue summary.

wim leers’s picture

Issue summary: View changes
wim leers’s picture

Title: Rely on TUF protected resources to determine which updates are available » Rely on TUF-protected resources to determine which updates are available
Issue tags: -Needs issue summary update

@omkar.podey updated the issue summary. I added one more connection.

If this is an alpha target, why is this marked Minor?

Wim Leers credited mxr576.

wim leers’s picture

Status: Active » Postponed (maintainer needs more info)

By the way, based on this superb comment on composer/composer by @mxr576, composer audit as described above does actually not provide complete enough information? Definitely not for contrib (which is not yet a concern), but not even for core?! 😳 Not sure how to verify those statements though.

wim leers’s picture

Interesting prior art by @mxr756:

catch’s picture

Does this mean we should eventually refactor update status to rely on composer audit if/when that's available?

tedbow’s picture

@catch eventually I think that would be good but there a couple complications.

  1. We probably don't want to make all sites that want to know about security updates enable Package Manager because it currently has system requirements base around whether the site can rewrite its own codebase(though this could might not be the web user see (#3351895)
  2. Running composer audit doesn't require rewriting the codebase so relying on Package Manager might be overkill. We could just do simpler method of calling the Composer executable and parsing the results but the site might have Composer available on the production environment.

    I talked with @xjm about shipping a Composer phar with core as way to be certain of the version of Composer we were using in Package Manager and she said this would not be possible(I can't remember of enough her reasoning so won't want to get it wrong here). But I assume that means we want not want to ship the phar for Update Status either

    We could invoke Composer audit(probably) from the Composer php API. But that would mean we would have to add dependency on composer/composer which we decided not to do in #3243899: [policy] Move composer/composer from a dev dependency to a production dependency

tedbow’s picture

We can rely on Composer eventually for composer audit but right now this does not tell us if a version is unsupported.

Drupal packagist could add this somehow

Wim Leers credited drumm.

wim leers’s picture

This was discussed at DrupalCon between @tedbow, myself and the Drupal Security Team. Conclusion: this should indeed become the only mechanism we use.

The Drupal Association is working on (well, @drumm is! 😄) on bringing the necessary infrastructure live.

The issue summary needs to be updated with relevant information: sequencing and infrastructure blockers.

tedbow’s picture

Issue tags: +Pittsburgh2023
drumm’s picture

tedbow’s picture

Assigned: tedbow » Unassigned
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Postponed
Issue tags: -Needs issue summary update

We can't actually commit this until #3301876: Implement “list security advisories” Packagist/Composer API. It is probably not worth working to much on because it will probably be a pretty change that we would have to keep merging with 3.0.x. We should still be able to release contrib 3.0.x versions without this functionality

effulgentsia’s picture

Title: Rely on TUF-protected resources to determine which updates are available » [PP-1] Rely on TUF-protected resources to determine which updates are available

Looks like at a minimum, this is postponed on #3301876: Implement “list security advisories” Packagist/Composer API, so adding "PP-1" to the issue title. If this is also postponed on #3343490: Deploy rugged for TUF signing to production and/or #3358504: Require PHP-TUF's Composer integration plugin, then that number should be raised accordingly, but I think there's code here than can be committed ahead of those latter two issues (but not ahead of the first one), is that right?

wim leers’s picture

Assigned: Unassigned » tedbow

For answering #27.

wim leers’s picture

Status: Postponed » Postponed (maintainer needs more info)

To clarify the need for additional info 😊

tedbow’s picture

Title: [PP-1] Rely on TUF-protected resources to determine which updates are available » [PP-3] Rely on TUF-protected resources to determine which updates are available
Assigned: tedbow » Unassigned
Status: Postponed (maintainer needs more info) » Postponed

We could commit this issue before #3343490: Deploy rugged for TUF signing to production and #3358504: Require PHP-TUF's Composer integration plugin but it would not solve the stated problem of using TUF-protected sources.

It would basically switch from using Update XML to composer audit but the metadata in Composer not be TUF-protected so I am not sure it would helpful

tedbow’s picture

Status: Postponed » Needs work

I think we can work on this again.

there are number of drupal.org infrastructure issues that need to be resolved for it to be committed but we should still be able to work on tests because we will always rely on mocked composer output for tests

tedbow’s picture

Title: [PP-3] Rely on TUF-protected resources to determine which updates are available » Rely on TUF-protected resources to determine which updates are available

phenaproxima made their first commit to this issue’s fork.

tedbow’s picture

I talked with @phenaproxima and @effulgentsia yesterday and proposed the idea of instead of altering any of the services by the update module we simply confirm the information available the drupal.org update.xml matches the same releases and secure status from Composer.

This has 2 advantages,

  1. Not altering the functionality of the Update module at all as far as available updates page and update emails. this is important because even though any one install this module in core experimental stage knows that they are taking some risk they might not expect it to alter the functionality of a non-experimental security focused module like the Update module
  2. this should probably be simpler short term solution because we can probably do all the checking of update xml release in 1 place
    either in \Drupal\package_manager\ProjectInfo::getInstallableReleases

For now we could hard-code what versions of core are supported via the info in \Drupal\update\ProjectSecurityData combined with the releases that available of core according to composer

tedbow’s picture

I talked to @phenaproxima about this recently

We had talked about making sure that information from update XML matched what available in the ProjectInfo class basically

  1. Get the update XML info
  2. Check the info from 1 matches exactly with Composer metadata, for release and which releases are secure

The problem I realize now is that we really can't do that in \Drupal\package_manager\ProjectInfo::getInstallableReleases() for few reasons:

  1. There could be time lag our processor and the update module will cache the results for the Update XML but then we call composer info and composer audit at the time we processing the releases. So if there is new release in the last hour composer info would return it and the cached update XML might not have it yet. So if we throw an exception because they don't match that will be problem
  2. \Drupal\package_manager\ProjectInfo::getProjectInfo which is called at the beginning of getInstallableReleases() is going to call update_calculate_project_data() which depending on which releases are marked as supported or unpublished could mark the project itself as unpublished. So we have to do the check against composer before this point
  3. getProjectInfo() and getInstallableReleases() both call getAvailableProjects() so we could check composer info there but we have the problem that for projects that are in the active codebase we rely on update_get_available() but if we do that then we run into the timing problem again. That data could be hours old and a current call to composer info might not match

For that reason it is probably best to compare against Composer info in \Drupal\package_manager\PackageManagerUpdateProcessor::processFetchTask() we should probably not rely on the Update modle process at all since this data would not be compared against composer. So this would probably mean doing #3308235: Do not use the Update modules cache of UPdate XML so we can control how long it is cached but that should be a small change.

drumm’s picture

The data source for advisories and insecure releases for packages.drupal.org is different from the one used for updates.drupal.org. So there will be mismatches from time to time. This may be due to human error, as both are maintained somewhat manually as advisories are published & updated.

#3383066: Use SA Affected Version to determine whether old versions are insecure could unify these, although I was hoping to wait until 7.x support was dropped, as this could only happen for modern Drupal releases.

Notably, there are a handful of unstable releases, beta & RCs, marked as insecure for update status XML as they address security issues. But there was no advisory, so packages.drupal.org does not have that data. We could remove the insecure flag from those releases, as it was never the security team’s policy.

tedbow’s picture

Status: Needs work » Postponed (maintainer needs more info)

Postponing this on #3394754: [policy, no patch] Use Update XML in Package manager to determine release support status and updated #3319030: Drupal Core Roadmap for Package Manager and Update Manager to reflect this. I just want it to very clear that the core issue blocking work on one of our last Alpha core blockers

catch’s picture

Title: Rely on TUF-protected resources to determine which updates are available » Use the packagist endpoint instead of update XML to determine which updates are supported/installable
Category: Bug report » Task
Priority: Minor » Normal
Status: Postponed (maintainer needs more info) » Postponed

With a narrower scope, I think this would now be postponed on #3394754: [policy, no patch] Use Update XML in Package manager to determine release support status implementing the functionality with update XML, and then a decision whether we move all that metadata to Drupal's packagist endpoint.

catch’s picture

Tagging for issue summary update since it would be good to document what it would entail to rely 100% on composer/d.o packagist for this information.