backdrop / backdrop-issues

Issue tracker for Backdrop core.
145 stars 40 forks source link

Allow using HTTP for the Update Manager / Allow SSL verification to be skipped for Project Installer #5455

Open argiepiano opened 2 years ago

argiepiano commented 2 years ago

Description of the need

We are seeing a new frustrated Forum post regarding the failure to get update data (https://forum.backdropcms.org/forum/not-possible-check-upgrades). This seems to come up regularly.

I experienced this issue and the problem was that Backdrop uses SSL to check for updates, and my local certificate used by stream_socket_client to make this socket connection with the server was expired. See https://forum.backdropcms.org/forum/issues-checking-updates-localhost

Other posters found similar solutions related to expired certificates. I think this is not uncommon.

Proposed solution

Drupal 7, to this date, uses an HTTP connection to obtain the same module update information. Backdrop's defines the constant UPDATE_DEFAULT_URL as https://updates.backdropcms.org/release-history. Drupal 7 http://updates.drupal.org/release-history.

It seems like D8/9 does use SSL to fetch update data, but provides a update_fetch_with_http_fallback fallback option in settings.php.

It'd be great to have this feature added to Backdrop.

This is the D8/9 doc explanation for this setting:

/**
 * Fallback to HTTP for Update Manager and for fetching security advisories.
 *
 * If your site fails to connect to updates.drupal.org over HTTPS (either when
 * fetching data on available updates, or when fetching the feed of critical
 * security announcements), you may uncomment this setting and set it to TRUE to
 * allow an insecure fallback to HTTP. Note that doing so will open your site up
 * to a potential man-in-the-middle attack. You should instead attempt to
 * resolve the issues before enabling this option.
 */
indigoxela commented 2 years ago

I think, the least we should do is to provide better help, in case the update check fails. It's usually a broken setup or missing cert update on the client side (dev environment), but figuring out the right solution can be a challenge.

Providing a http fallback as a last resort for totally quirky environments may make sense, but that also requires a server-side change, because http currently wouldn't work (leads to 404).

https://updates.backdropcms.org/release-history/backdrop/1.x
klonos commented 2 years ago

We are not the only ones dealing with similar issues. This is from a recent release of ClassicPress: https://github.com/ClassicPress/ClassicPress-release/releases/tag/1.3.1

Remove an expired Let's Encrypt certificate from the list of certificates that ClassicPress maintains (#814, thanks to WP contributors who originally submitted this change there). This helps resolve issues with ClassicPress sites being unable to communicate with the ClassicPress update server, ...

The change descriptions finishes with this though:

...but in some cases, server configuration changes are also needed.

klonos commented 2 years ago

...http currently wouldn't work (leads to 404).

FTR, we mentioned this issue during the weekly dev meeting yesterday. We discussed the possibility of redirecting http://updates.backdropcms.org/release-history/backdrop/1.x to the HTTPS equivalent instead of 404.

In the meantime, I'll work on a PR to implement the insecure fallback override in settings.php, which I think would be a good workaround (accompanied with strong warning re security of course).

klonos commented 2 years ago

...I feel that this qualifies as a task, which could get into a future bug release.

klonos commented 2 years ago

@larsdesigns thank you for enabling HTTP for http://updates.backdropcms.org/release-history/backdrop/1.x for us πŸ™πŸΌ ...it doesn't 404 any more πŸ‘πŸΌ

@echozone while we're working on a PR and because this may take time to get merged and into an official Backdrop core release, can you please try the following and report whether that works for you or not?: manually edit your update.settings.json file in your active config directory, and change the setting 'update_url' from its default blank value to the http version of 'http://updates.backdropcms.org/release-history'. Save and check for updates again. Thanks.

echozone commented 2 years ago

@klonos It WORKS!!!

klonos commented 2 years ago

Thank you for testing and confirming @echozone πŸ™πŸΌ ...pull request coming up shortly.

echozone commented 2 years ago

Error also occurs on /admin/modules/install Install New Modules. "Encountered an error when trying to fetch results from Backdrop. Error 0 : Error opening socket ssl://projects.backdropcms.org:443"

klonos commented 2 years ago

Initial PR up for review: https://github.com/backdrop/backdrop/pull/3972 (only adds a setting in settings.php and handles the Update Manager bit for now)

klonos commented 2 years ago

Error also occurs on /admin/modules/install Install New Modules. "Encountered an error when trying to fetch results from Backdrop. Error 0 : Error opening socket ssl://projects.backdropcms.org:443"

That may or may not be related to:

indigoxela commented 2 years ago

Both of these D issues look interesting.

I'm a bit hesitant to confirm the current approach. HTTP is second best. At least, a warning on the status page might make sense. Otherwise people just set it and forget about it, and the site does something not ideal without any warning.

The proper way would be to fix the ssl problem on hosting. The fallback should really be the last resort, if everything else failed.

@echozone what's your SSL version? (/admin/reports/status/php)

echozone commented 2 years ago

Agreed best to have it working on SSL. SSL Version OpenSSL/1.0.2k-fips

klonos commented 2 years ago

At least, a warning on the status page might make sense.

Yes πŸ‘πŸΌ ...I'll update the PR to implement that.

The proper way would be to fix the ssl problem on hosting.

Agreed. I'll add that in the wording of the status page warning.

klonos commented 2 years ago

@echozone thank you for getting your hosting provider to set up a test server for us. I was able to figure out solutions for both issues.

I was thinking to split the issue with the Project Installer to a separate PR/issue, but the actual root cause is the same for both (misconfigured/problematic SSL certificates on the hosting provider end), and the solution is similar for both issues (adding an option in settings.php).

Now, the question is: do we add a single setting that controls both things, or 2 separate settings? Also, what do we call these settings?

echozone commented 2 years ago

I think they should be together since if you have one issue, you'd have the other (so far, everywhere I saw it, I saw both). That might make naming harder, how about something with "ssl bypass"?

klonos commented 2 years ago

I've updated the PR. There are now 2 options in the settings.php file (commended-out by default):

$settings['update_fetch_with_http'] = TRUE;
$settings['installer_disable_ssl_verification'] = TRUE;

The first is used in _update_build_fetch_url(); the latter in installer_browser_fetch_results().

The question now is: is there a case where only one of these options needs to be enabled?

klonos commented 2 years ago

@echozone thanks for the suggestion. Although naming the option(s) something as simple as ssl_bypass may seem like a good idea, it is too generic, and is not self-explanatory as per where it's being used (by which module). As an example, settings.php also has an option called update_free_access, which is used by the Update Manager module (which has a machine name of update, so the update_ prefix of the option makes sense and is useful).

Naming things is hard πŸ˜…

klonos commented 2 years ago

The question now is: is there a case where only one of these options needs to be enabled?

...I've done some testing on the problematic server instance provided by the hosting company used by @echozone:

  1. I've enabled both settings:

    $settings['update_fetch_with_http'] = TRUE;
    $settings['installer_disable_ssl_verification'] = TRUE;

    Everything worked without any issue πŸ‘πŸΌ ...I was able to retrieve updates, and list/install modules via the Project Browser/Installer.

  2. Then I've set the 2 settings as follows:

    $settings['update_fetch_with_http'] = FALSE;
    $settings['installer_disable_ssl_verification'] = TRUE;

    This had the following result (besides not being able to retrieve update information, which was expected):

    • in admin/modules/install the module listing and search/filtering worked without any issue πŸ‘πŸΌ
    • adding any module in the installation queue and clicking "Install" didn't work πŸ‘ŽπŸΌ

So it does seem that we need to merge the 2 settings into one πŸ€” ...thoughts?

indigoxela commented 2 years ago

So it does seem that we need to merge the 2 settings into one...

These settings do pretty different things, though.

One disables the verification, but still fetches via HTTPS, the other one does an insecure fetch. Both at a time... does that make sense, anyway?

Actually, these two settings do different things in two places (install / update fetch).

klonos commented 2 years ago

@indigoxela yeah, and if we were to combine them, then I wouldn't know a suitable prefix to use for the name 🀷🏼 ...do you reckon we keep the settings separate, but group them together in the settings.php file? (the docblocks and security warnings are pretty much the same for both settings)

indigoxela commented 2 years ago

if we were to combine them, then I wouldn't know a suitable prefix to use for the name

Same here.

klonos commented 2 years ago

PR updated again:

findlabnet commented 2 years ago

Maybe just cancel and close without any further action?

klonos commented 2 years ago

Maybe just cancel and close without any further action?

@findlabnet I understand where you are coming from. The actual issue here is misconfigured SSL on the hosting provider's end, so that's where this should be fixed. We all agree on that πŸ‘πŸΌ ...however, hosting providers are usually "blaming" the application, so people come to the Backdrop forum and/or issue queue saying that "Backdrop doesn't work properly". These settings will help with troubleshooting, and will provide a way to demonstrate that it is not Backdrop that is "broken".

Even if the hosting provider is willing to look into things, it might take them some time to properly fix the issue. In the meantime, by providing these settings we allow the site owners to temporarily allow updating (provided they understand the security risks involved). So if there is a security release for example, instead of wasting time on discussions of "who's fault it is", the site owner can:

  1. edit their settings.php and allow insecure connections
  2. update their site
  3. disable the settings in settings.php

I think that this is worth the effort here.

PS: we have a pretty similar use case where we provide allow_authorize_operations in settings.php, which is also insecure (hence disabled by default).

findlabnet commented 2 years ago

I guess less technical people won't go into that kind of detail, but we still have a problem if the server software is 5 years out of date. But this and many other vulnerabilities are beyond our very limited resources.

klonos commented 2 years ago

Agreed @findlabnet ...but I'm sure that @echozone has appreciated our help to get to the bottom of this, and that is what we're here for, right?

findlabnet commented 2 years ago

I'm sure too, but in this case we can/should expand the troubleshooting documentation / FAQ, but not the software itself (IMHO).

argiepiano commented 2 years ago

Even if the hosting provider is willing to look into things, it might take them some time to properly fix the issue.

This is probably the best reason to provide this functionality. Some of us have been in this situation. D9 provides this option for this reason. And yes, it would be helpful to also have better documentation.

I agree that in a perfect world this would not be needed.

indigoxela commented 2 years ago

Currently, with the (usual) problem (ca-certificates didn't get updated on the web-host :roll_eyes: ), always both settings are needed.

I see a chance to provide a way to still update via https (only suppress cert verification), by adapting _update_process_fetch_task().

Thoughts?

klonos commented 2 years ago

@indigoxela interesting. Would it be a completely different approach, or supplementing my PR? Depending on the answer, please either file an alternative PR, or a PR against mine. We can discuss/decide then.

indigoxela commented 2 years ago

Would it be a completely different approach, or supplementing my PR?

I'm assuming, it would be supplementing, but have to admit, that I didn't try anything out yet.

Function _update_process_fetch_task() also does a backdrop_http_request, which could take the same options - based on settings.

This enables us to still provide https fetch (without verification, though), which seems slightly better than using http right away.

I think, then only using installer_disable_ssl_verification would already workaround the problem with outdated (expired) CA info on webservers.

And possibly we might not even need the second setting (update_fetch_with_http), as the url is already configurable (in update.settings.json). Again, that's all just theory. :wink:

klonos commented 2 years ago

Sounds plausible πŸ™‚ ...I'd say please go ahead and give it a go πŸ‘πŸΌ

indigoxela commented 2 years ago

I'd say please go ahead and give it a go

My problem is, that I'm currently very short on time. @klonos you already started working, so you'd be quick in also adding the verification bypass in function _update_process_fetch_task. (And also: if I'm not contributing to it, I can review it :wink: )

larsdesigns commented 2 years ago

I have a feeling this is because to harden the server, I removed older tls and ssl cyphers. I suspect that mamp is configured to use one of those.

What is mamp configured to use?

It is possible to restore these older ssl and tls options for a specific url.

indigoxela commented 2 years ago

I have a feeling this is because to harden the server, I removed older tls and ssl cyphers

All of our research (also in the chat) suggest, that it's just yet another problem with the expired Letsecrypt root (CA) cert (back in 2021).

Some more details: the server which caused the initial report here, is a CentOS 7 which has been updated last time (created?) in 2020. Hence the old CA is still around.

larsdesigns commented 2 years ago

Oh, this is news to me. Which domain/subdomain is this certificate for?

indigoxela commented 2 years ago

@larsdesigns to not derail this issue too much, mind to read the issue description, and possibly the related Zulip thread? It's about updates.backdropcms.org - which has a valid and properly working LE cert. Just some (few) webservers fail to connect.

larsdesigns commented 2 years ago

Okay, fair enough. Let me know if I can help out on the server side.

klonos commented 2 years ago

Related: there was a recent commit in the 7.x version of the Stage File Proxy module, which also provided a way to bypass SSL verification (although they provided that via a checkbox in the UI). See: https://www.drupal.org/project/stage_file_proxy/issues/3008351

So if there are use cases where contrib projects may need this, perhaps it does make sense to make it a single, generic setting πŸ€”

indigoxela commented 2 years ago

So if there are use cases where contrib projects may need this, perhaps it does make sense to make it a single, generic setting

Do you mean a UI form instead of a (sort of hidden) option in settings.php? Maybe... but the whole certificate verification problem is rather an edge-case. Very annoying for people who have to deal with it, but not an 80% problem.

In the meantime a contrib module exists, which uses a different approach. Instead of bypassing verification, it enables proper verification even on problematic hosting.

This doesn't mean that the current approach here (allow to bypass) isn't needed anymore. There still can be other problems with ssl/tls, that a proper cafile can't fix.

klonos commented 2 years ago

Do you mean a UI form instead of a (sort of hidden) option in settings.php?

Nope, if in core, this would still be in settings.php. Unless in the future we decide to expose it in the UI somewhere like a dedicated security section (#3624).

izmeez commented 1 year ago

Is it time to get this in or is there a strong desire to combine the two settings into one?