Make SSL options configurable in drupal_http_request()

Thanks for re-rolling my work from comment 68 in the parent issue joseph.olstad. I'd still like to see this make it into core and I'm happy to do the work to make that happen.

The "Remaining Tasks" that you copied over from the parent issue are obviously out of date; the D8 patch is in and this patch uses a different approach from what was being discussed in comment 25.

So can I get some guidance as to what still needs to be done? I'm happy to do it!

This patch doesn't apply to 7.64 since it changed settings.default.php slightly - here's a updated version.

Revert status - https://www.drupal.org/project/site_moderators/issues/3276540

This seems a lot less relevant now than it did several years ago when PHP (5.6) had only just started verifying SSL/TLS by default, and when self-signed certificates were a lot more common.

@Fabianx and I both +1'ed this approach in #1081192: Verify peer on HTTPS if cURL available (but be careful of built-in cert bundles in the codebase) but that was several years ago. I remember being very in favour of it back then.

2017 me mentioned a contrib. implementation in the original issue: #2774269: provide conditional configuration. I outlined a more flexible approach where the context could be customised per port and/or URL as well as just by host. It looks like I didn't implement that though. I don't recall why exactly but it does seem like it might be overly complicated.

If we were to commit this, nothing would change by default.. but sites would be able to set up new configurations.

I think that sounds okay, but I am really wondering whether this is "still a thing". Would any sites actually use this?

I'll RTBC it on the basis that I think it'd be safe to commit it, but I'd like to discuss whether we should a bit further with the other maintainers.

Oh, and I expect it might need a reroll as default.settings.php keeps getting new additions.

but I am really wondering whether this is "still a thing". Would any sites actually use this?

I think that the benefit of this change is that sites on PHP 5.6+ can disable SSL/TLS verification for specific hosts when using drupal_http_request(). By default it does verify SSL and it is not possible to change it (without replacing the function entirely, see below). So probably the issue has a bit different (opposite) purpose now, as it has years before and probably doesn't have much in common with the parent/original issue.

In overall, I think it is a good change, as it will add a flexibility and more users will be able to use this API function without need of the replacement.

On the other hand, two points come to my mind:

1. We have possibility to replace the drupal_http_request() with a different function, see:

$override_function = variable_get('drupal_http_request_function', FALSE);

So it is questionable, if there are still sites which will benefit from this - probably most of them are using this override or do not use drupal_http_request() at all.

2. The settings.php is getting bigger and bigger (but it is not necessary a bad thing, we just need to carefully consider adding new config options).

-------------------

Anyway, if we want to commit this, I think we need to add a test for the new settings. We can probably test a subdomain with expired/invalid SSL (for example something like the https://badssl.com/ offers) and check if drupal_http_request() works with the validation enabled / disabled.

I will not change the status before the final decision.

We have possibility to replace the drupal_http_request() with a different function ...

Yup, I think the self-signed SSL cert use case was the main reason I created https://www.drupal.org/project/alt_http_client which does exactly that.

I've updated the project a few times over the years to bring in updates made to core's drupal_http_request(), but I'm not certain when I last did so.

On the basis that it's perhaps not that common to need to deviate from PHP's defaults these days, there's an argument to leave this to be dealt with in contrib.

Oh, #3299146: re-sync with D7 core's drupal_http_request() is about updating alt_http_client with changes from core's drupal_http_request().

I think when I filed that I was waiting for a patch to be committed. Looks like it's all ready to go now.

If I updated that project, shall we close this issue for core with the recommendation that sites use alt_http_client if they need to tweak SSL options?

If I updated that project, shall we close this issue for core with the recommendation that sites use alt_http_client if they need to tweak SSL options?

+1 for this. There is/will be a fully functional module with the same functionality as is proposed here. Considering a number of sites which will possible use this and also other options they have, I think this will be the best solution.

There's now a 7.x-1.1 release of alt_http_client with the updates from core plus tests.

The module includes pretty much the same code as the patch in this issue:

https://git.drupalcode.org/project/alt_http_client/-/blob/7.x-1.1/alt_ht...

We should try to remember to update the module whenever core's drupal_http_request() changes.

Thanks everyone who worked on this, especially @kmcculloch for the original patch.