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

Move PHP settings to PHP config files + Split Nginx config into SSL/non-SSL #1191

Merged
merged 5 commits into from
Oct 26, 2017
Merged

Conversation

MichaIng
Copy link
Owner

...as e.g. enabling caching for CLI has no effect inside webserver config, which is not asked by CLI requests, e.g. by cron.
We just leave the critical opcache size inside webserver config, in case hard coding 128M, so this will not allow other web services to full the cache and cause swapping.

...as e.g. enabling caching for CLI has no effect inside webserver config, which is not asked by CLI requests, e.g. by cron.
We just leave the critical opcache size inside webserver config, in case hard coding 128M, so this will not allow other web services to full the cache and cause swapping.
";" at end of directive
grep -q 'opcache.interned_strings_buffer=' $FP_PHP_BASE_DIR/mods-available/opcache.ini && sed -i '/opcache.interned_strings_buffer=/c\opcache.interned_strings_buffer=8' $FP_PHP_BASE_DIR/mods-available/opcache.ini || echo 'opcache.interned_strings_buffer=8' >> $FP_PHP_BASE_DIR/mods-available/opcache.ini
grep -q 'opcache.max_accelerated_files=' $FP_PHP_BASE_DIR/mods-available/opcache.ini && sed -i '/opcache.max_accelerated_files=/c\opcache.max_accelerated_files=10000' $FP_PHP_BASE_DIR/mods-available/opcache.ini || echo 'opcache.max_accelerated_files=10000' >> $FP_PHP_BASE_DIR/mods-available/opcache.ini
grep -q 'opcache.save_comments=' $FP_PHP_BASE_DIR/mods-available/opcache.ini && sed -i '/opcache.save_comments=/c\opcache.save_comments=1' $FP_PHP_BASE_DIR/mods-available/opcache.ini || echo 'opcache.save_comments=1' >> $FP_PHP_BASE_DIR/mods-available/opcache.ini
grep -q 'opcache.revalidate_freq=' $FP_PHP_BASE_DIR/mods-available/opcache.ini && sed -i '/opcache.revalidate_freq=/c\opcache.revalidate_freq=1' $FP_PHP_BASE_DIR/mods-available/opcache.ini || echo 'opcache.revalidate_freq=1' >> $FP_PHP_BASE_DIR/mods-available/opcache.ini
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should we also do that just inside webserver config, as it is set to "60" during PHP installation, which is also totally fine for CLI requests.

@MichaIng
Copy link
Owner Author

What do we do on removing Nextcloud with the PHP settings? For now these settings inside these files are not changed on any ofter dietpi-software installation, so we can simply remove the related lines.

@MichaIng MichaIng changed the title WORK IN PROCESS: Move PHP settings from webserver to PHP config files Move PHP settings from webserver to PHP config files Oct 13, 2017
@MichaIng
Copy link
Owner Author

About https://github.com/Fourdee/DietPi/issues/1067#issuecomment-337935644:

Added nginx conf again by this PR. Indeed the nginx nextcloud.config contains settings (fastcgi_param HTTPS on; and HSTS) that are just relevant/valid for SSL enabled systems.

If I understand right, fastcgi_param needs to be in case set for every sub location, so we can't set all of this in the default nginx ssl config, as it is possible with apache. So we need 2 versions of the nextcloud config file, one for ssl enabled systems (how do we test?) and one for systems without ssl. Or maybe we can create a delta config that just adds the ssl relevant parts to to the nextcloud location directives, which can be then enabled and disabled, without the need to directly rewrite the basic (non-ssl) nginx nextcloud.config?

@Fourdee Fourdee added this to the v158 milestone Oct 19, 2017
@Fourdee
Copy link
Collaborator

Fourdee commented Oct 19, 2017

@MichaIng

Added nginx conf again by this PR

Legend, thanks 👍

one for systems without ssl

This should be the default item. The only instance of SSL enabled (in DietPi at least), is when the user uses http://dietpi.com/phpbb/viewtopic.php?f=8&t=5&p=1061#p1062. Although, not sure if Nginx is implemented yet, will need to check.

I'd stick with non-SSL for now. We can always sed the changes at a later date?

@MichaIng
Copy link
Owner Author

MichaIng commented Oct 19, 2017

@Fourdee
By https://github.com/Fourdee/DietPi/pull/1166 nginx support for certbot/ssl was added. And I bet/hope that every user, that installs some web service opened to the internet, manually enabled SSL, if not automatically done by DietPi 😉.

So I would directly prepare and additional SSL conf, but yeah non-SSL by default of course. Need to find out a good check, if SSL is enabled or not, for related webservers and then sed the config file, use an additional one, or include one, that just adds all ssl relevant directives:
ToDo:
[ ] Add check (also in case dietpi-letsencrypt was not used) whether SSL is used or not
[ ] Remove SSL-related directives from nginx nextcloud.config
[ ] Include (just-)SSL config file, if possible, else...
[ ] ...use existing nextcloud.config as SSL-enabled alternative or...
[ ] ...sed SSL directives, which could be hard, as they seem to be located inside different location directives.

@MichaIng MichaIng changed the title Move PHP settings from webserver to PHP config files Move PHP settings from webserver to PHP config files + splitting nginx config into SSL/non-SSL Oct 19, 2017
@MichaIng MichaIng changed the title Move PHP settings from webserver to PHP config files + splitting nginx config into SSL/non-SSL Move PHP settings to PHP config files + Split Nginx config into SSL/non-SSL Oct 19, 2017
@Fourdee
Copy link
Collaborator

Fourdee commented Oct 19, 2017

@MichaIng

And I bet/hope that every user, that installs some web service opened to the internet, manually enabled SSL, if not automatically done by DietP

Would be great if we could make this possible, however, 99.9% of routers these days have port 80 disabled to internal devices, unless port forwarded by the user in router settings, in which case its their responsibility :)

@MichaIng
Copy link
Owner Author

Hehe yeah of course opening/forwarding the ports of course still needs to be done manually. I just mean the SSL configuration by e.g. python-certbot-nginx/apache package and things like this 😉.

Btw. NextcloudPi uses some UPnP method to automatically forward the ports, but it does not work on every system/with every router of course.

@MichaIng
Copy link
Owner Author

@Bridouz @Fourdee

Do you know, which directives depend on SSL? fastcgi_param HTTPS on and HSTS are obvious, but I don't know e.g. about the add_header X- X-Headers?
Also I wonder, if all of this is really still necessary, if it is already set on parent nginx config e.g. by certbot auto configuration or manually by admin.

And how do we check reliable, if SSL is active? Check for some active nginx config with "listen 443" inside seems not too reliable for me? Better, if we could do something like a curl request on you own 443 port or something, but I don't know how to realize this 😅.

@Fourdee Fourdee modified the milestone: v158 Oct 25, 2017
@Fourdee
Copy link
Collaborator

Fourdee commented Oct 26, 2017

@MichaIng

Do you know, which directives depend on SSL?

I have no idea, sorry. My SSL knowledge and site config knowledge is minimal.

@Fourdee Fourdee merged commit bf56f98 into MichaIng:testing Oct 26, 2017
@Fourdee
Copy link
Collaborator

Fourdee commented Oct 26, 2017

@MichaIng

Thanks for cleaning this up 👍
Reference, testing required : https://github.com/Fourdee/DietPi/issues/1067

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

Successfully merging this pull request may close these issues.

2 participants