- Issue created by @tedbow
- 🇺🇸United States drumm NY, US
and a special Rugged instance set up to provide a TUF protected source for Drupal core itself(and other drupal/* packages that are not hosted on drupal.org?).
Clarifying this in the issue summary. The Rugged instance for Drupal Core will cover the core components / subtree splits.
- 🇺🇸United States drumm NY, US
Need to get confirmation that Drupal security releases of core and contrib will also be show up in composer audit
This is mostly correct, and I’d say good to rely on.
There is an "insecure" label for releases used in update status XML. This is a separate source of data that can diverge from the affected versions in Composer audit. ✨ Use SA Affected Version to determine whether old versions are insecure Active has some more details. Practically, these should match up. Composer audit may be fully updated up to a couple minutes earlier than update status XML. There are a handful of releases, mostly older, probably less than 100, marked as insecure without an advisory for Composer audit. These would be non-stable contrib releases, like a 8.x-1.0-beta1, which got marked as insecure outside of the normal security team process. We could audit these to reconcile the data if needed.
- 🇺🇸United States drumm NY, US
Need to confirm that getting all relevant the metadata that is only in Update XML into composer metadata or other TUF protect targets is not on the short term road map for the DA.
If needed, we can add a “release is on a supported branch” flag to the
extra
metadata section. Or we could have a “supported branch version constraint” added to the package’sextra
metadata section. Either would not take a huge amount of effort and could be done short term.We could also fabricate security advisory metadata to flag unsupported releases with Composer audit. This is somewhat of an abuse of the API, unsupported without a security advisory isn’t the same as having a security advisory. In particular, Drupal projects with future versions that are not yet supported would be especially weird being flagged in Composer audit; Drupal maintainers should probably not do this anyway, the release stability is the way to show how stable the releases on a new branch are. If this is the way to go, it would not take a huge amount of effort and could be done short term.
In an ideal world, this would be natively supported in Composer via https://github.com/composer/composer/issues/8272 or equivalent. So we aren’t implementing something too Drupal-specific.
- 🇺🇸United States drumm NY, US
While we can quickly metadata about supported branches on packages.drupal.org, I agree with tedbow that relying on update status XML for this looks best right now. When/if a resolution for https://github.com/composer/composer/issues/8272 lands, that would be ideal as a permanent home of the metadata. Anything we add to the metadata is effectively permanent, we don’t want to add anything too hastily.
- 🇺🇸United States tedbow Ithaca, NY, USA
Bumping this up to critical.
I generally don't do this on core issues but @xjm set 🐛 Exceptions in batch no longer are shown on the page when Javascript is disabled Needs work to critical because
This blocks AU in core and is therefore critical.
This also applies to this issue as depending on what decision is made here it could lead to major work in the module and on the drupal.org infrastructure, though I don't think it needs to as I agree with @drumm on #5
I have postponed 🐛 Rely on TUF-protected resources to determine which updates are available Postponed: needs info which is last known major core alpha security issue in the module itself on 🌱 Drupal 10 Core Roadmap for Automatic Updates Active
- 🇬🇧United Kingdom catch
There appears to be only one proposed solution, with sub-items underneath, and no #2 - is that correct or is one of the sub-items at the wrong level?
- Assigned to tedbow
- 🇺🇸United States tedbow Ithaca, NY, USA
@catch I probably got the formatting, wrong will fix
- Issue was unassigned.
- 🇬🇧United Kingdom catch
This would mean that even if the Update XML was compromised you could not “hide”(freeze attack) a newer version of a modul
Say we got into this actual situation, what would happen. Composer would know about the new release, but update XML wouldn't, would we then assume that 'missing' releases from update XML are because it's compromised, and allow an update?
Overall #1 sounds like the much better option, if we're slowly able to move more metadata into composer later, then we can do that transparently over time.
- 🇺🇸United States tedbow Ithaca, NY, USA
@catch we would have to figure that out.
For Drupal core I think we could use the Drupal Core’s special case reasoning in the issue summary to only rely Composer metadata
For package manager in general where we want to expose an API to determine what is installable, basically Project Browsers use case, I think we could not allow the install and when we support contrib updates I don't think we could not allow the update.
This is because for all we know the reason someone compromised the XML and removed newest releases is that these releases are unsupported. So they are trying to trick the site from moving from a current installed version, 1.0.1 where 1.0.x is supported and secure, to 1.2.1, where 1.2.x is not supported(and therefore can't be considered secure because the security team would not put an SA). Or in Project Browsers case they are trying to trick the site into installing 1.2.x instead of 1.0.1
I think in this case we just have show the user an error and say the system is not usable right now.
- 🇬🇧United Kingdom catch
I think in this case we just have show the user an error and say the system is not usable right now.
That makes sense, then we assume that if it's really compromised somewhere, it'll be recovered and start working again, but default to not making any changes.
Also agree with making use of core's hard-coding of minor release cycles.
- 🇬🇧United Kingdom catch
Switching this from release manager review to framework manager review, seems like a security/functionality question not really about bc (except for d.o packagist data which it will help us stuffing extra things into in a rush, but core doesn't produce that).
- 🇬🇧United Kingdom catch
My general feeling here is:
1. We should start with project XML data, since prematurely adding something to packagist data could cause us problems down the line.
If the worst thing someone can do is a freeze attack (and if we can show an error that the system is broken, maybe even prompt them to check the announcements feed?) that to me is in the category of lesser risks compared to everything else that could go wrong. Would be good to e-mail the site admin if that error state happens similar to update status e-mails in case the site is infrequently visited by admins.
2. I think that automatic updates forces this to use the https URL already independent of the update status configuration which still allows http? If so could we document that in the issue summary?
3. We should have a follow-up issue for moving/duplicating the data to the packagist endpoint. I don't think this issue should be alpha or beta blocking, I think we should defer deciding whether it's stable blocking until that issue is open and in progress. For me, I think if it turns out to be easy, it's worth doing asap, but if it's not, it's better to do it without a 'deadline' to get it right - unless we come up with new risk vectors from the update XML feed in the process.
Would be great to get more opinions here.
- 🇬🇧United Kingdom catch
To summarise #10 and #11, hopefully correctly:
1. Update XML, assuming we rely on it, would be relied upon for installability of a version, not its existence. So for example say a module has supported 1 and 2 branches, and an unsupported an abandoned dead end 3.x branch, that kind of information is only currently available from update XML. If we don't check installability, we could update people to dead end branches or paper bag releases that have been withdrawn.
There are two weakness to relying on update XML:
A. Because we have the packagist data to compare to for the existence of releases, it would be impossible for someone to use compromised update XML to get someone to install a release that doesn't exist - because we would only install something we can see from packagist. However, they could fake that a previously uninstallable release (unsupported newer branch) is installable, and get someone to install to that, and we wouldn't be able to detect that. However if there's a security advisory that's recognisable for composer audit, we could prevent updates to known insecure versions.
We can mitigate this for core because we know what core's release cycle is within core itself, but for contrib it would be a known weakness -however no worse than not using the update XML metadata in the first place and relying on the existing packagist APIs.
B. Someone could hide a release entirely from update.xml or mark it as uninstallable and we'd still be able to see it in packagist, but we wouldn't know whether it was installable or not.
If the release is hidden in update xml and visible in packagist, there are two options:
1. Update anyway - this could lead to the 'dead end' situation similar to A.
2. Don't update, but because we know there's a mismatch, we can show a warning that something isn't right and the site owner should check manually - this could be in the UI and/or maybe an e-mail notification.
So the two risks from the current proposed solution are:
1. You might get updated from an installable version of a module to an uninstallable version (but not a known insecure one as far as composer audit knows).
2. You might get stuck on your existing version, but with a loud warning.
What we need to decide:
1. Is the above correct, or is there a mistake in the logic?
2. If it's not correct, are there different mitigations that would be OK, or none?
3. If the problem is correct and the mitigations are solid, are mitigations good enough for an alpha, beta, or stable release?
4. At whatever point we decide it's not OK to rely on update XML, we would need to replicate the same information into d.o's packagist endpoint, but once we do that, we will need to decide on, and handle changes to, the format over time since there's not a single standard for this additional metadata.
- 🇺🇸United States tedbow Ithaca, NY, USA
updated the summary with the selected solution
- 🇫🇮Finland lauriii Finland
Thanks for explaining this in Slack and updating the issue summary based on that. I think the proposal makes sense now. At the moment, I think that the proposed approach is fine for AU stable. However, I'd like to review the actual implementation and messages before stable.
- Status changed to Needs work
12 months ago 8:15pm 5 December 2023 - 🇺🇸United States tedbow Ithaca, NY, USA
@lauriii @larowlan thanks for the sign-off
So can we mark this issue as fixed?. I am not going to start work on this until it is marked as fixed because to me that is clearest indication the decision has been made by the committer team
However, I'd like to review the actual implementation and messages before stable.
Of course we will first implement this is in the contrib module unless getting this done is not an Alpha core blocker.
Setting it "Needs work" since it is not fixed. I don't think as a non-core committer I can mark these types of policy issues as fixed
- Status changed to RTBC
12 months ago 8:29pm 5 December 2023 - Status changed to Fixed
11 months ago 6:16pm 12 December 2023 - 🇬🇧United Kingdom catch
I was going to open a follow-up issue so we had something to document the idea of moving to extended Drupal.org packagist info later on (even if we decided not to do that ever, we could still explicitly rule it out), but then realised 🐛 Rely on TUF-protected resources to determine which updates are available Postponed: needs info is that issue which this was spun out of, so just moved that one to postponed/task.
Going to go ahead and mark this fixed.
- 🇬🇧United Kingdom catch
After thinking about about #21 and discussing with @tedbow, we realised it would be better to have a core policy issue about the packagist endpoint changes, because after a decision, it'll need both infra and AU changes if that's what we do, so I've opened 🌱 [policy, no patch] Decide if and when automatic updates should rely only on packagist data to determine installability of modules Active .
Also adding commit (or 'commit') credit because I forgot when marking this fixed.
Automatically closed - issue fixed for 2 weeks with no activity.