Re: www/nextcloud: possible improvements to provided httpd.conf example

2024-02-11 Thread Clemens Gößnitzer
On Thu, 2024-02-01 at 20:53 +0100, Clemens Gößnitzer wrote:
> On Thu, 2024-02-01 at 16:48 +0100, Gonzalo L. Rodriguez wrote:
> > Hi, it was discussed many times about put a super tweaked
> > httpd.conf
> > but the
> > idea of the example it's just that, an example, if you think your
> > version could
> > be better or more secure and you tested it, please send a diff and
> > I
> > will try
> > it.
> > 
> > Thanks!
> > 
> 
> Attached you find a diff.  Rationale for the changes:
> 
> -) I only need to set max request body to 10M for even very large
> files
> (>500M) to be uploaded successfully in the browser.
> 
> -) Only pass needed *.php* files to the fastcgi interpreter.  This
> change requires careful testing.  The below setting works for me, but
> I
> don't use all features of nextcloud.
> 
> -) In the nextcloud root directory, there is already an index.html
> which takes care of the redirect.  Use that instead of a return 301. 
> If it ever required to redirect to a different location, the chances
> are high that nextcloud takes care of that in index.html, thus,
> changing the httpd configuration wouldn't be required.
> 
> -) the 'location match "/nextcloud/oc[ms]%-provider/*"' had some
> issues
> and seems to be not necessary anymore, since ocm-provider got deleted
> some time ago:
> * no root set, thus, no access to the nextcloud directory
> * no fastcgi set for php parsing
> * no request strip 1 set
>  This was not immediately visible due to the "catch-all" "*.php*"
> glob.
> 
> -) according to the documentation for nginx [1], all .well-known
> except
> carddav and caldav should just be handed over to index.php for
> correct
> handling.  This could potentially break setups that also use ACME for
> getting TLS certificates if the .well-known/acme-challenge
> configuration comes after this one (in httpd first location statement
> wins).
> 
> Potential further changes, not shown in this diff:
> -) change from example.com/nextcloud to cloud.example.com.  This
> would
> simplify some settings.  I don't know how many people run the former
> vs. the latter, and it's subject to personal taste.
> -) Setting up an err.html with a redirect to /nextcloud/index.php,
> the
> same way index.html in the nextcloud root directory works.  Thus, all
> invalid links would be forwarded to index.php.  This wouldn't work if
> there are other services than nextcloud in the same server / domain.
> 
> [1]
> https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html
> 

Is there any interest in this changes?  I successfully deployed them,
and I did not see any problems.  Did anybody else try them? 
Additionally, the warnings with the current example are gone, and it
would not recommend this IMHO wrong entry (no root set, no fastcgi set,
ocm-provider deleted):

> location match "/nextcloud/oc[ms]%-provider/*" { 
> directory index index.php
> pass 
>} 


> 
> Index: pkg/README
> ===
> RCS file: /cvs/ports/www/nextcloud/pkg/README,v
> retrieving revision 1.23
> diff -u -p -u -r1.23 README
> --- pkg/README  5 Oct 2023 14:18:07 -   1.23
> +++ pkg/README  1 Feb 2024 19:38:49 -
> @@ -43,25 +43,54 @@ server "domain.tld" {
> key "/etc/ssl/private/domain.tld_private.pem"
> }
>  
> -   # Set max upload size to 513M (in bytes)
> -   connection max request body 537919488
> +   # Set max upload size to 10M (in bytes)
> +   connection max request body 10485760
> connection max requests 1000
> connection request timeout 3600
> connection timeout 3600
>  
> block drop
>  
> -   # Ensure that no '*.php*' files can be fetched from these
> directories
> -   location "/nextcloud/config/*" {
> -   block drop
> +   # only allow well-known and required php files for fastcgi
> +   # required for the webpage to work
> +   location "/nextcloud/index.php*" {
> +   root "/nextcloud"
> +   request strip 1
> +   fastcgi socket "/run/php-fpm.sock"
> +   pass
> +   }
> +
> +   location "/nextcloud/public.php*" {
> +   root "/nextcloud"
> +   request strip 1
> +   fastcgi socket "/run/php-fpm.sock"
> +   pass
> }
>  
> -   location "/nextcloud/data/*" {
> -   block drop
> +   # required for caldav and carddav
> +   location "/nextcloud/remote.php*" {
> +   root "/nextcloud"
> +   request strip 1
> +   fastcgi socket "/run/php-fpm.sock"
> +   pass
> }
>  
> -   # Note that this matches "*.php*" anywhere in the request
> path.
> -   location "/nextcloud/*.php*" {
> +   # required for the sync app
> +   location "/nextcloud/status.php"

Re: www/nextcloud: possible improvements to provided httpd.conf example

2024-02-01 Thread Clemens Gößnitzer
On Thu, 2024-02-01 at 16:48 +0100, Gonzalo L. Rodriguez wrote:
> On Thu, 01 Feb 2024 at 13:57:35 +0100, Clemens Gößnitzer wrote:
> > I value the amazing work you are doing to keep www/nextcloud in a
> > really good shape.  Thank you for that!
> > 
> > I have three suggestions to possibly improve the provided
> > httpd.conf
> > example:
> > 
> > 1.) block more locations
> > 2.) use block return 403 together with errdocs redirect
> > 3.) only allow needed .php files
> > 
> > 1.)
> > According to
> > https://github.com/nextcloud/server/blob/master/.htaccess#L91,
> > 3rdparty
> > and lib should be blocked as well.  The example already does that
> > for
> > config and data.
> > 
> > 2.)
> > Instead of doing "block drop", one could alternatively use "block
> > return 403".  With an error document similar to the provided
> > index.html
> > from nextcloud, a simple redirect to index.php could be provided,
> > too,
> > when returning 403.  This could also be done for the global server
> > configuration.  Thus, the user would always get redirected to
> > index.php
> > if he requests an invalid URL.
> > 
> > 3.)
> > It seems like index.php, remote.php and status.php (potentially
> > cron.php for web-based cron) are the only php files which need to
> > be
> > parsed by php-fpm socket via fcgi.  Thus, instead of matching all
> > php
> > files in the path (with "*.php*"), one could restrict php fcgi
> > functionality to just these three (four) files.
> > 
> > Change 3.) would even make change 1) unnecessary, since then there
> > is
> > no danger of fetching random php files with the glob, and these
> > block
> > drop could be replaced by a block drop/return 403 in the global
> > server
> > configuration.
> > 
> > If you like these ideas, I will provide a patch.
> > 
> > Thanks.
> > 
> 
> Hi, it was discussed many times about put a super tweaked httpd.conf
> but the
> idea of the example it's just that, an example, if you think your
> version could
> be better or more secure and you tested it, please send a diff and I
> will try
> it.
> 
> Thanks!
> 

Attached you find a diff.  Rationale for the changes:

-) I only need to set max request body to 10M for even very large files
(>500M) to be uploaded successfully in the browser.

-) Only pass needed *.php* files to the fastcgi interpreter.  This
change requires careful testing.  The below setting works for me, but I
don't use all features of nextcloud.

-) In the nextcloud root directory, there is already an index.html
which takes care of the redirect.  Use that instead of a return 301. 
If it ever required to redirect to a different location, the chances
are high that nextcloud takes care of that in index.html, thus,
changing the httpd configuration wouldn't be required.

-) the 'location match "/nextcloud/oc[ms]%-provider/*"' had some issues
and seems to be not necessary anymore, since ocm-provider got deleted
some time ago:
* no root set, thus, no access to the nextcloud directory
* no fastcgi set for php parsing
* no request strip 1 set
 This was not immediately visible due to the "catch-all" "*.php*" glob.

-) according to the documentation for nginx [1], all .well-known except
carddav and caldav should just be handed over to index.php for correct
handling.  This could potentially break setups that also use ACME for
getting TLS certificates if the .well-known/acme-challenge
configuration comes after this one (in httpd first location statement
wins).

Potential further changes, not shown in this diff:
-) change from example.com/nextcloud to cloud.example.com.  This would
simplify some settings.  I don't know how many people run the former
vs. the latter, and it's subject to personal taste.
-) Setting up an err.html with a redirect to /nextcloud/index.php, the
same way index.html in the nextcloud root directory works.  Thus, all
invalid links would be forwarded to index.php.  This wouldn't work if
there are other services than nextcloud in the same server / domain.

[1]
https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html


Index: pkg/README
===
RCS file: /cvs/ports/www/nextcloud/pkg/README,v
retrieving revision 1.23
diff -u -p -u -r1.23 README
--- pkg/README  5 Oct 2023 14:18:07 -   1.23
+++ pkg/README  1 Feb 2024 19:38:49 -
@@ -43,25 +43,54 @@ server "domain.tld" {
key "/etc/ssl/private/domain.tld_private.pem"
}
 
-   # Set max upload size to 513M (in bytes)
-   connection max request body 537919488
+   # Set max upload size to 10M (in bytes)
+   connection max request body 10485760
connection max requests 1000
connection request timeout 3600
connection timeout 3600
 
block drop
 
-   # Ensure that no '*.php*' files can be fetched from these directories
-   location "/nextcloud/config/*" {
-   block drop
+   # only allow well-known and required php fi

Re: www/nextcloud: possible improvements to provided httpd.conf example

2024-02-01 Thread Gonzalo L. Rodriguez
On Thu, 01 Feb 2024 at 13:57:35 +0100, Clemens Gößnitzer wrote:
> I value the amazing work you are doing to keep www/nextcloud in a
> really good shape.  Thank you for that!
> 
> I have three suggestions to possibly improve the provided httpd.conf
> example:
> 
> 1.) block more locations
> 2.) use block return 403 together with errdocs redirect
> 3.) only allow needed .php files
> 
> 1.)
> According to
> https://github.com/nextcloud/server/blob/master/.htaccess#L91, 3rdparty
> and lib should be blocked as well.  The example already does that for
> config and data.
> 
> 2.)
> Instead of doing "block drop", one could alternatively use "block
> return 403".  With an error document similar to the provided index.html
> from nextcloud, a simple redirect to index.php could be provided, too,
> when returning 403.  This could also be done for the global server
> configuration.  Thus, the user would always get redirected to index.php
> if he requests an invalid URL.
> 
> 3.)
> It seems like index.php, remote.php and status.php (potentially
> cron.php for web-based cron) are the only php files which need to be
> parsed by php-fpm socket via fcgi.  Thus, instead of matching all php
> files in the path (with "*.php*"), one could restrict php fcgi
> functionality to just these three (four) files.
> 
> Change 3.) would even make change 1) unnecessary, since then there is
> no danger of fetching random php files with the glob, and these block
> drop could be replaced by a block drop/return 403 in the global server
> configuration.
> 
> If you like these ideas, I will provide a patch.
> 
> Thanks.
> 

Hi, it was discussed many times about put a super tweaked httpd.conf but the
idea of the example it's just that, an example, if you think your version could
be better or more secure and you tested it, please send a diff and I will try
it.

Thanks!

-- 

 %gonzalo



www/nextcloud: possible improvements to provided httpd.conf example

2024-02-01 Thread Clemens Gößnitzer
I value the amazing work you are doing to keep www/nextcloud in a
really good shape.  Thank you for that!

I have three suggestions to possibly improve the provided httpd.conf
example:

1.) block more locations
2.) use block return 403 together with errdocs redirect
3.) only allow needed .php files

1.)
According to
https://github.com/nextcloud/server/blob/master/.htaccess#L91, 3rdparty
and lib should be blocked as well.  The example already does that for
config and data.

2.)
Instead of doing "block drop", one could alternatively use "block
return 403".  With an error document similar to the provided index.html
from nextcloud, a simple redirect to index.php could be provided, too,
when returning 403.  This could also be done for the global server
configuration.  Thus, the user would always get redirected to index.php
if he requests an invalid URL.

3.)
It seems like index.php, remote.php and status.php (potentially
cron.php for web-based cron) are the only php files which need to be
parsed by php-fpm socket via fcgi.  Thus, instead of matching all php
files in the path (with "*.php*"), one could restrict php fcgi
functionality to just these three (four) files.

Change 3.) would even make change 1) unnecessary, since then there is
no danger of fetching random php files with the glob, and these block
drop could be replaced by a block drop/return 403 in the global server
configuration.

If you like these ideas, I will provide a patch.

Thanks.