Closed (fixed)
Project:
Provision
Version:
7.x-3.x-dev
Component:
Code
Priority:
Major
Category:
Feature request
Assigned:
Issue tags:
Reporter:
Created:
13 Apr 2018 at 19:42 UTC
Updated:
14 Nov 2021 at 19:39 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #3
jon pughComment #4
jon pughComment #6
helmo commentedAfter moving it to the vhost config this also needs a patch for hosting_https ...
Comment #7
colanLooks reasonable, but I'm not sure of a good way to test. Would it be hard to add CI tests for both servers?
Comment #8
colanComment #9
helmo commentedI'm open to suggestions for a CI test but don't have one ready.
Here's an extra patch for hosting module to re-verify all sites to update their vhost config.
Comment #10
memtkmcc commentedNginx config has to be moved to the default config include, which is included in every vhost, since "set" directive is not allowed in this context:
Comment #11
memtkmcc commentedHere is a working patch for Nginx (attached).
Comment #12
memtkmcc commentedNote that there is no need to re-verify sites hosted in Nginx based Aegir, since the updated (single) config is dynamically included in all vhosts.
Comment #13
helmo commentedthanks @memtkmcc, I've committed that to the 2960825-mitigation branch.
For easy review, here's the full patch.
Comment #14
helmo commented@memtkmcc so instead of the sites we should verify the server nodes?
Comment #15
helmo commentedI somehow managed to delete the patch from #13 .. here's that again.
And an updated re-verify patch to also do the server.
Comment #16
jon pughCorrect, Server verify saves the vhost include:
From http\Provision\Service\http\nginx.php:
Comment #18
helmo commentedThis was released with 3.14.6 ... it would be nice to improve this (or verify that it already does) cover SA-CORE-2018-004. But lets create a new issue when there is someone available to work on it.
Comment #20
jon pughThere's one more change still in this branch.
Can someone confirm this is not needed? https://git.drupalcode.org/project/provision/compare/7.x-3.x...2960825-m...
Comment #21
kienan commentedThe rules are still present in the nginx templates, so I think the trailing commit for apache should probably be merged in so the configurations are coherent between the two possible webservers. Tested w/ apache2.
Comment #23
helmo commentedI've now committed the changed Jon pointed at.