Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UX] Auto Updates for security/modules #414

Open
timodwhit opened this issue Nov 10, 2014 · 36 comments · May be fixed by backdrop/backdrop#3753
Open

[UX] Auto Updates for security/modules #414

timodwhit opened this issue Nov 10, 2014 · 36 comments · May be fixed by backdrop/backdrop#3753

Comments

@timodwhit
Copy link

timodwhit commented Nov 10, 2014

Not sure if there is an issue about this, but I think something strong that backdrop-issue could offer is auto updates for security issues etc.

Since there is semantic versioning, this could be used as a check.

There are obviously a lot of issues that are needing to be addressed here, but thought I would start the issue.


screen shot 2018-12-06 at 4 43 32 pm


PR by @serundeputy: backdrop/backdrop#2307
Revised by @laryn: backdrop/backdrop#3753


Part of META issue: #2018

@quicksketch
Copy link
Member

I'm interested in this functionality also, but yes, it's going to take a lot of effort and planning to do properly. After "Drupalgeddon" with the SQL injection attack hijacking sites within 7 hours of release, it's a great example of how manual updates just don't cut it any more with large pieces of web software, any more than attacks against popular browsers or operating systems.

There's a bit of relevant discussion over in #395, but only loosely.

I'd love to take a look at the Wordpress auto-update model and how well that is working for them.

@jenlampton
Copy link
Member

I'm conflicted about this. I like the idea of auto-updates, but also worry about 1) php being able to write it's own files (isn't that a huge risk?) and 2) breaking things in contrib by updating core.

2 will be a bigger risk for us than with Drupal 7 since we may be doing things like adding modules to core. If you had link module from contrib and are running 1.0.0 and in 1.2.0 we added link module to core, and in 1.2.2 we had a major security fix, auto updating you to 1.2.2 might cause a ton of problems. Would we also need to maintain a 1.0.x with the same security fix as 1.2.2 to prevent this from happening? Possible, but more work.

@timodwhit
Copy link
Author

I'm conflicted as well about it, but at the same time, it is helpful for those smaller sites that may not be maintained as regularly as other large sites. Over at d.o there are different thoughts and ideas being kicked around: https://www.drupal.org/node/2367319

As far as the core bringing in modules, I could foreseeably see us saying that we support up to three minor versions pack with security patches or something similar to that. And then each site would have the ability to chose how they update like gems and bundler. You can select whether to update to major, minor, or patch version through a module_version and module_version.lock file type approach. By default we could say modules update to patch versions, but minor we leave alone unless you decide to do that. This could easily be done with a UI of check boxes, like the permissions tab or through files.

I think the level of security issue something like the "drupalgeddon" bug would warrant a patch to minor versions, lets say 3 minor versions back, which seems like it would be quite significant.

As far as PHP writing files, I think it depends on the host you are on, doesn't the update module allow for this currently?

@quicksketch quicksketch added this to the 2.x-future milestone Dec 5, 2014
@jenlampton jenlampton changed the title Auto Updates for security/modules [UX] Auto Updates for security/modules Jan 3, 2016
@mbaynton
Copy link

@quicksketch, @jenlampton and I talked for a while about this today at Twin Cities Drupal Camp sprints. Its a problem I've been working on, on and off, since I was introduced to Backdrop's philosophy at last year's camp. Especially when coupled with security-only releases of older Backdrop versions, making the technology seemed worthwhile.

My approach has been to create a mostly-standalone web application that updates other web applications, vs. simply modifying/extending similar functionality already in Backdrop that installs and updates modules. The advantages of this are that it avoids potential complications with the updater being dependent on and loading things while they're getting updated, that if a given update totally broke a given site, the updater application itself is still usable, and that it becomes simpler to make the work applicable to other php applications.

The work so far can be found at https://github.com/curator-wik. I call the app Curator.

To clarify, the first problem I'm trying to solve is in-browser, user attended core updates, with automatic updates being a progression from there. But, some takeaways from today's discussion that ought to get noted on here:

  • @quicksketch felt pretty strongly that the capacity to perform core updates, automatically or with a button in the UI, shouldn't be an optional feature/add-on. He hasn't looked over my code in-depth or anything, but was very open to a model where a second app ships with Backdrop core in order to make it standard functionality.
  • We discussed the interface between Backdrop and Curator a bit, and some changes to improve UX:
    1. To make at least a cursory effort at maintaining a similar look/feel when the user is sent away from Backdrop and to the Curator UI, support having Curator load a custom stylesheet that Backdrop provides. Ostensibly Backdrop would pass a stylesheet from the active admin theme.
    2. Since Curator would be used for core updates but not replace the module installer for that task, provide a means to forward file writing configuration (protocol, username, password etc) from Backdrop to Curator so that Curator can operate with zero extra configuration of its own.

Nate also brought up a bit of a thought experiment about what it would look like for Curator to self-update, especially if it was shipped as part of core and thus in some releases would be responsible for updating itself. Since Curator will be distributed as a single .phar file, it should be fairly simple to rapidly and cleanly switch from an old version to a new one. However, there was consensus that there should probably be a little special handling when a core update includes a Curator update, to ensure that the updater updates itself first, before attempting to apply the update to Backdrop.

We talked quite a bit about what the input to Curator should be, e.g. a full .tgz of the new release vs some paired-down format that only contains the things that changed from one Backdrop release to another. The advantage of updating from a full .tgz is that there is less new infrastructure/release generation work necessary. The advantage of a paired-down format are that applying an update can be done more quickly by each site, by executing just the file overwrites/deletes necessary to make the old release source tree match the new release source tree rather than deleting the entire old core and then installing an entirely new one. Curator supports a paired-down format. I think I was at least reasonably successful at bringing Jen and Nate around to my way of thinking that incorporating some automation to their release process that generates these paired-down packages might be worthwhile :)

@docwilmot
Copy link
Contributor

The concept of self or auto updates is obviously important and the approach sounds practical. Apart from that though I cannot comment on the plan or code: a bit high level for me. Will be happy to test when it is ready, and if it is already testable, will need instructions.

@klonos
Copy link
Member

klonos commented Jul 3, 2017

Thank you @mbaynton for all the time and energy spent on this 👍 Basically what @docwilmot said: way beyond me, but do let me know when you need help with testing.

@laryn
Copy link
Contributor

laryn commented Feb 21, 2018

For reference, Drupal is beginning to discuss a similar idea:
https://www.drupal.org/project/ideas/issues/2940731

The goal is to implement a secure system for automatically installing updates in Drupal, lowering the total cost of ownership of maintaining a Drupal site, improving the security of Drupal sites in the wild, and lowering the barrier to entry to using Drupal.

@findlabnet
Copy link

Moreover, content of such sites with low cost of ownership and maintaining also can be added automatically, or at least this server can generate(mining) some (any buzzwords can be placed here)coins.

@klonos
Copy link
Member

klonos commented Jun 17, 2018

I wonder how this issue here fits into #2018 and its child issues. Is this also a sub-issue of the META, or should it be closed as duplicate?

@serundeputy
Copy link
Member

@klonos I think this is the automatic part of #2018 and therefore should be the last part of that meta.

@serundeputy
Copy link
Member

Could use feedback on UX and language:

screen shot 2018-07-12 at 3 56 57 pm

in the top right I would like to put date pickers so people can restrict the updates to a time window (like between 2AM and 5AM).

current language:

A Security release is a release that pathes a security vulnerability.
A Patch release is a security and/or bug fix releases, i.e. 1.10.1 to 1.10.2 is a patch release.
A Minor release is feature release (new functionality no API changes), i.e. 1.10.2 to 1.11.0 is a minor release.

@klonos klonos mentioned this issue Jul 12, 2018
10 tasks
@olafgrabienski
Copy link

  • A Security release removes security vulnerabilities.
  • A Patch release removes bugs and contains improvements of the user interface. It may also contain security fixes.
  • A Minor release adds new features (without API changes).

@klonos
Copy link
Member

klonos commented Jul 17, 2018

I like @olafgrabienski's proposal. My suggestion based on that:

  • A Security release fixes security problems.
  • A Patch release fixes functionality flaws and improves the user interface. It may also contain security fixes.
  • A Minor release adds new features (without API changes).

I kinda liked the "i.e." examples, but we could omit them if people find them too "technical". ...although this part of the admin UI is targeted towards technically-savvy people I guess.

@serundeputy do you have a PR for this?

@serundeputy
Copy link
Member

@klonos PR is fortcoming. I have code but needs attention and <3.

@serundeputy
Copy link
Member

serundeputy commented Sep 22, 2018

PR: backdrop/backdrop#2307

TODO:

  • check current version against version requirements (sec only, patch, minor)
    • need to get current version number and proposed version in installer_cron
  • run dbupdate
  • right now the updater copies the core dir from backdrop.zip over the currently installed core, but for example we moved node_path_info() to a new file, but with the copy it is in both files and backdrop explodes; ( i think this would fail on updating via UI too on an update from 1.10.2 to 1.11.0 )
    • need to remove core? and still let the ops finish?????

@herbdool
Copy link

So it leaves files that are no longer in the new version? If that's true then I guess we do need to remove core first somehow.

I wonder how WP does it? I also seem to recall it has ability to roll back at some point (before db updates i imagine).

@mbaynton
Copy link

I know lots of work has gone into this direction for doing your updates already, but this talk about deleting files and renaming files and needing to remove core have sucked me in :) You'd have to spin up a Drupal 7 site 😱 but you might want to take a gander at

  • https://youtu.be/r3H-KzfV5sk
  • https://www.drupal.org/sandbox/mbaynton/2997009
    which sidesteps the need to remove core (how would you even do that with a web-based updater that's integrated into your core?) while handling file deletes and renames by downloading a release artifact that's basically the deltas from your n-1 release and applying deletions and renames per those delta's instructions. The whole thing also runs awful fast on account of the much-reduced amount of work necessary to do the update versus writing out a whole release, which is nice since it's presumably happening on somebody's real-world site.

Although the thing you can actually test drive today is Drupal 7, the entirety of the D7 specific code is like 150 lines; the rest lives in a .phar file that is application agnostic. It should be pretty easy to port to Backdrop. I'd do it myself but I was actually working on adding rollback capability as I saw these comments, so I'm otherwise engaged at the moment 😉

@mbaynton
Copy link

Oh! I also has docs and resources and goodies:

@serundeputy
Copy link
Member

@mbaynton thanks; I took a brief glance at the things you have going on there, but I could not get quickly up to speed on it (a lot to digest)

My current thinking is that i'm close with the api that already exists in Backdrop ... so it seems simpler to me to implement that way. I'm going to continue on that path ... others should give their opinions as well.

The current PR is not ready for general testing (it doesn't quite work yet), but dropping this script here that eases some of the steps needed to get to a place that one can test:

#!/usr/bin/env.bash¬
set -ex¬

git clone git@github.com:backdrop/backdrop.git 414automaticUpdates
cp 2289.patch 414automaticUpdates/
cp 2307.patch 414automaticUpdates/
cp .lando.yml 414automaticUpdates/
cd 414automaticUpdates/
git checkout 1.10.2
git apply 2289.patch
git apply 2307.patch
lando.dev start && lando.dev drush si --db-url=mysql://backdrop:backdrop@database/backdrop
lando.dev drush uli

Run this in a clean directory and it will set up a testable backdrop environment that has the right patches and is ready to upgrade from 1.10.2 to 1.11.0.

When you are done with that environment (like maybe you want to test again) do a lando destroy -y and rm -rf 414automaticUpdates, then you can run the script again and start over.

@klonos
Copy link
Member

klonos commented Oct 15, 2018

I have just tested upgrading 1.11.0 to 1.11.1 and it run smoothly 👍. Notes:

Settings page (/admin/reports/updates/settings)

  • "Check if you want the ability...": I usually prefer the words "enable" (formal) or "tick" (informal). It helps avoid ambiguations (check = test/verify).

  • I would like to propose to remove the checkbox for core updates altogether. We can then change the label of the dropdown to "Core updates method", and also change its options to these:

    • Disabled
    • Manual
    • Automatic - Security releases only
    • Automatic - Minor releases
    • Automatic - Patch releases

    ...I would in fact argue that the "Disabled" option should not exist at all (since we have checkboxes which allow people to include/exclude individual projects during the update wizard). This, combined with changing the dropdown to a set of radio options would allow to place the respective help text under each method. Here's what it would look like:

    screen shot 2018-10-16 at 7 19 40 am

    ...so no need for custom #states/css magic to show/hide any options + single click to make a choice = UX+

    Also... notice how in the help text for the manual option in my mockup I have a ??? placeholder for the update wizard page? Well, how weird is it now that the core updates are available under the "Update modules" page (/admin/modules/update) & the "Update themes" page (/admin/themes/update) & the "Update layouts" page (/admin/layouts/update), huh? 😄 ...this is me shamelessly plugging [UX] Consolidate the places where we list available updates... #3039

Update wizard

  • Perhaps "Back up your database and site before you continue. Learn how." (/admin/update/ready) should be a warning message instead of plain markup text. Before:

    screen shot 2018-10-16 at 6 24 40 am

    ...after (mockup):

    screen shot 2018-10-16 at 8 58 08 am

    ...not sure about "Updating modules, themes, and layouts requires FTP access to your server. ...". It feels like an info message.

  • "Updating modules, themes, and layouts, as well as Backdrop core requires..."

  • During next step, if ftp is the only method, then why offer a dropdown menu with options?

    screen shot 2018-10-16 at 6 28 33 am

    ...or are we removing that altogether (Deprecate & Remove authorize.php entirely #3208)?

@klonos
Copy link
Member

klonos commented Oct 15, 2018

...another question: I know that this ticket is about the backend work required to get automatic updates working, but why restrict the security updates only to core? A security update is a security update no matter what.

Anyway, in the case we do want to enable automatic security updates for contrib projects too, then I think that we should:

  • check daily
  • remove the "Check for updates" options
  • add new options for "Automatic updates window" (days/hrs)
  • hide the "Automatic updates window" options if the method of updates is set to "Manual only"

@olafgrabienski
Copy link

olafgrabienski commented Oct 21, 2018

@klonos Above, you mentioned the page admin/update/ready with the message "Back up your database and site before you continue. Learn how."

I just noticed that the "Learn how" link points to the page "Backing up a site" on drupal.org. Indeed, I didn't find a similar page in the Backdrop documentation. Are you aware of such a page? (If not, I'd open a ticket with the suggestion to add one.)

@klonos
Copy link
Member

klonos commented Oct 21, 2018

@olafgrabienski I could not find anything in https://backdropcms.org/user-guide, so by all means create a separate documentation issue. We will need to link to it from other pages too, like in update.php, where we say:

...
Create backups. This update utility will alter your database and config files. In case of an emergency you may need to revert to a recent backup; make sure you have one.
...

...in fact there are still many places in our codebase where we still link to d.org. We cannot possibly match the accumulated decades of documentation that's there.

@jenlampton
Copy link
Member

jenlampton commented Dec 7, 2018

I added a screenshot of the proposed UI from the PR into the parent issue here. It looks like there are a bunch of assumptions there (like the ability to update to not-the-latest-version) that we haven't yet determined are possible.

I also have some suggestions for language improvements, but I'm going to start on the single checkbox for "Enable core updates" and put those language changes in the PR for #3271.

While working on #3271 It occurred to me that we should add a few other settings for the automatic updates (and these may qualify as feature additions on their own)

  • allow people to schedule the updates for a particular day and/or time?
  • allow the site to send emails before & after the update?

@klonos
Copy link
Member

klonos commented Dec 9, 2018

cross-posting from #3271 (comment)

...wondering if it would be a safer option to be renaming the old core and then extracting the update in its place. We'd then let the end user manually delete the backup of the old core.

If they don't delete the old backups, worse that can happen is to run out of disk space (and given the size of core and the space offerings by hosting providers nowadays, that'd be rare). That's way safer than destructively deleting things.

@herbdool
Copy link

I believe it relies on the updater classes https://github.com/backdrop/backdrop/blob/1.x/core/includes/updater.inc. There is a backup function makeBackup but it currently does nothing.

@quicksketch
Copy link
Member

This PR probably needs to be updated to work with the code that was merged in #3271. We added the UI to enable manual updates, so this issue can focus just on adding the automatic part.

@alanmels
Copy link

Just subscribing to this nice discussion to include it under my radar.

@laryn
Copy link
Contributor

laryn commented Sep 24, 2021

It still needs work and further feedback but I started reworking @serundeputy's PR from ~3 years ago here:
backdrop/backdrop#3753

Please help if interested 😅

@laryn
Copy link
Contributor

laryn commented Sep 28, 2021

An example of an email that Wordpress sends after an auto-update, for reference:

SUBJECT:

[Website Name] Some plugins were automatically updated

BODY:

Howdy! Some plugins have automatically updated to their latest versions on your site at https://example.org. No further action is needed on your part.

These plugins are now up to date:

  • Yoast SEO (from version 17.2 to 17.2.1)

If you experience any issues or need support, the volunteers in the WordPress.org support forums may be able to help.
https://wordpress.org/support/forums/

The WordPress Team

@klonos
Copy link
Member

klonos commented Oct 5, 2021

If we're doing auto-updates for sites, then I would be more comfortable if we also auto-backed up their db as well as part of the process. So I would like this email to also mention something along the lines of:

A backup of your database has been created under [path-to-private-dir]

@klonos
Copy link
Member

klonos commented Oct 5, 2021

@laryn how do we test this? Is it for core only, or contrib projects as well?

If would expect this to be a scenario to test:

  • setup Backdrop on local
  • patch with https://patch-diff.githubusercontent.com/raw/backdrop/backdrop/pull/3753.patch
  • install outdated versions of various modules/themes/layouts, with some of them having security releases available
  • run cron -> confirm that no autoupdates occurred
  • enable security updates
  • run cron -> confirm that only projects with security updates have been updated (expect a summary email about it too?)
  • set auto-updates to patch/bug fix release level
  • run cron -> confirm that only projects with bug fix updates have been updated (expect a summary email about it too?)
  • set auto-updates to minor fix release level
  • run cron -> confirm that all remaining projects have been updated (expect a summary email about it too?)

What about core? How do we test that? Is this a scenario that should work?:

  • Install Backdrop 1.19.2 which has a security update
  • enable core auto-updates
  • run cron
    ...is the expectation in this scenario to have core updated to 1.19.3 (security update), or 1.19.4 (latest 1.19.x), or whatever is the latest 1.20.x?

@herbdool
Copy link

herbdool commented Oct 5, 2021

We've got a whole meta issue on things that we need before we can turn this on, including digital signatures on packages: #2018. (I think the signatures is getting close but it has stalled as well.)

@laryn
Copy link
Contributor

laryn commented Oct 7, 2021

Yes, I didn't mean to imply that this was ready to go (or to take it over), just trying to keep it up to speed with core changes and hopefully keep it on the radar.

@klonos I haven't changed any of the functionality from the initial PR and I think a lot of those questions are still to be answered. Right now it's in a very early form with few options or checks in place. As far as testing what is there now, here's a quick way:

  • Install Backdrop from that PR directly, including the auto-update code
  • Verify version is 1.21.x-dev
  • Make sure the config setting for Automatic self-update is not checked (if trying this multiple times the setting can unexpectedly be on after replacing the core directory but leaving config as is).
  • Edit core/includes/bootstrap.inc and set the version number to 1.19.2 (or whatever earlier version)
  • Go to Status report and verify new version shows
  • Run cron
  • Verify no version change
  • Change setting to run auto-updates
  • Run cron
  • Verify core has updated to latest official release (1.20.0 currently)

I think doing this on an actual 1.19.2 install will raise the issue of database updates required -- which the PR still marks as a @todo.

@klonos
Copy link
Member

klonos commented Oct 13, 2021

  • I would like to propose to remove the checkbox for core updates altogether. We can then change the label of the dropdown to "Core updates method", and also change its options to these:

    • Disabled
    • Manual
    • Automatic - Security releases only
    • Automatic - Minor releases
    • Automatic - Patch releases

FTR, this is what the respective UI looks like in D9:

@philsward
Copy link

philsward commented Nov 18, 2021

To me, "Disabled" implies "Manual" so that could probably be consolidated.

I would like to see 3 options personally, 4 at the most.

"Automatic - Patch releases" doesn't mean anything to me so I would never use it. I can only guess a patch is a major bug fix that can't wait for the next release (a bug got through review and broke stuff for example), which I would personally lump in to run with security updates which are technically patches if I'm thinking about it right.

One thought I've pondered for a while is if it's worth doing a diff on core files before an update?

A long time ago, I played around with PHPBB and when updating their platform, it would scan all of the core files and tell you if any core files had manual changes performed and told you exactly which files did not match the old or new meaning they were modified by the user. It would even display the line mismatches in the GUI. There were several options to deal with those changes, but it's made me wonder if it would be worth it to have the updater (whether auto or manual) do a diff check to see if a core file has been modified by the end-user and then ask what to do about it before proceeding. This would mainly benefit those who add patches to core files but it adds another layer of break-proofing things. For what it's worth.

Finally, what would it take to push updates instead of each site pinging for an update? I would think a site should still ping occasionally if it hasn't received anything from the server in a while, but if there is a major security update that rolls through (like the Drupal SQL one), sites will be notified of the update and instructed to updated instead of requesting if there is an update at pre-defined times that might open the window for exploitation. It would take some thought on the load-balancing side of things to offset how much is being pushed to prevent server load etc but I feel this would be a better solution long term. I would be OK if this route was only applied to security and "patch" updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.