Fix Nginx .well-known redirects

[MichaIng]

Follow the .htaccess and exclude .well-known redirects for ACME challenges and PKI validation, which are not handled by Nextcloud.
Preserve the query string when redirecting .well-known URIs.

Related to discussion here: #5825 (comment)

[MichaIng]

Two more questions:

Line 333 - 335:

          location /nextcloud {
              try_files $uri $uri/ /nextcloud/index.php$request_uri;
          }

This is inside the location ^~ /nextcloud block, hence looks like it would match in every case. But practically it is only matched when no previous location block is matched, since only the longest match is applied? And this is what is wanted here since previous location blocks contain each intended final handler/redirect/denial, right?

location ~ \.(?:css|js|svg|gif)$ {:
This matches the .htaccess, but is there actually any reason why other asset/image file types are not included, like PNG, map and JPEG? Nextcloud serves plenty of those (and likely others) as well.

[MorrisJobke]

cc @josh4trunks

[josh4trunks]

I think this is a good idea and should work as expected. But I don't currently have time/brain power to test myself (got a newborn and crazy 2 year old at home), so if someone else could test that would be appreciated.

Also CC'ing @jivanpal since I know he started the major effort in aligning with .htaccess

[jivanpal]

I'll try to take a look at this by the weekend, but things are hectic here, too, so it might take longer.

[jivanpal]

@MichaIng

Apologies for the delay in looking at this. Your changes to the .well-known sections look good — I don't know who/what PR introduced the problematic catch-all rule that you've fixed. In any case, I have:

• optimised the config by removing regex where possible (explicitly defining what to do for acme-challenge and pki-validation);
• included another change (handling /remote); and
• separated the configs from the RST source.

I can't commit these changes here as I don't have permissions to write to any branch on this repo, so here's a patch file you can download; please use git apply <file> to apply the changes, then commit them to your branch. Alternatively, if someone can give me PR edit permissions, that'd be appreciated.

---

Lines 333 - 335: [...]
This is inside the location ^~ /nextcloud block, hence looks like it would match in every case. But practically it is only matched when no previous location block is matched, ...

That's correct.

... since only the longest match is applied?

No; longest match is applied on a per-matching-type basis, with order of preference for the matching types being:

1. precise matches (=);
2. regex matches (~ or ~*), which are only considered if the longest prefix match is not regex-forbidding (^~);
3. prefix matches (regex-forbidding ^~ or regular, e.g. location /x).

Thus, for example, we can have a short regex rule that takes precedence over a longer regular prefix rule. See Nginx's algorithm for full details.

The purpose of the encapsulating location ^~ /nextcloud block is to allow admins to have non-conflicting configs for other sub-paths of their domain, e.g. location ^~ /nextcloud covers example.com/nextcloud, and the rest of the config covers the rest of the domain. Within the block, which rule to use is determined by Nginx's algorithm. For ease of reading, the nested blocks are written in the order from highest priority to lowest priority. The process within the location ^~ /nextcloud block is thus:

Is the request for:

1. /nextcloud precisely? If so, then:
   1. if the client is a DAV client, redirect them to /nextcloud/remote.php/webdav/; else
   2. the index directive takes over and /nextcloud/index.php (front-end-controller) does the work.
2. /nextcloud/remote or a subpath thereof? If so, let /nextcloud/remote.php handle the URI.
3. a forbidden path? If so, return 404.
4. a .php file? If so, PHP-FPM takes over.
5. a .css, .js, .svg, .gif, .woff, or .woff2 file? If so, then:
   1. if the URI corresponds to a filepath which corresponds to a file which exists on disk, serve that file; else
   2. let the front-end controller handle the URI.
6. none of the above? Then:
   1. if the URI corresponds to a filepath that corresponds to a file that exists, serve it; else
   2. if the URI corresponds to a filepath that corresponds to a directory that exists, the index directive takes over; else
   3. let the front-end controller handle the URI.

Step (6) above is what the nested location /nextcloud block handles. You could leave the enclosed try_files outside of that nested block, and just directly within the parent location ^~ /nextcloud block, but I believe @josh4trunks suggested enclosing it within location /nextcloud for the sake of clarity.

---

location ~ \.(?:css|js|svg|gif)$ {:
This matches the .htaccess, but is there actually any reason why other asset/image file types are not included [...] ?

The goal is to have consistent behaviour regardless of what web server is being used, so the approach I took in #2197 in rewriting the Nginx configs was to simply replicate Apache's behaviour, which I tried to do by replicating the .htaccess rules in Nginx style. As such, I would say to discuss this matter upstream with the Apache config maintainers (I don't know who generally manages that, if anyone, nor whether there is any rationale for only caching the current filetypes), and if changes are made there, I'll happily accept/create a PR to make the corresponding changes in the Nginx sample files.

[MichaIng]

Looks good, I removed trailing white spaces from the files but applied it otherwise without changes.

The goal is to have consistent behaviour regardless of what web server is being used

Yes I got that. This was more a general question why cache TTL is not applied to other image assets, but it makes sense to handle this in an own issue/PR that includes changed the .htaccess files.

[josh4trunks]

Sorry, as I said earlier in this thread, I don't have much time to review. It looks like changes here are minimal and non-controversial, so I trust @jivanpal @MorrisJobke or others to review and approve.

[jivanpal]

I don't have review/approval permissions, unfortunately — Paging @ChristophWurst just in case.

[jivanpal]

Christoph has given me permissions; approving.

[moviuro]

Hi, .well-known should be the place to drop some random files. (See e.g. https://en.wikipedia.org/wiki/List_of_/.well-known/_services_offered_by_webservers )

ATM, the latest nginx docs redirect .well-known/security.txt to index.php/.well-known/security.txt (which fails)

Is there a good reason for the last §? Shouldn't the last .well-known just be a try_files statement?

# Let Nextcloud's API for `/.well-known` URIs handle all other
# requests by passing them to the front-end controller.
return 301 /index.php$request_uri;

Managing a whitelist of known services in .well-known is bound to be hard work.

[MichaIng]

Redirecting the list of URLs which are known to be handled correctly by Nextcloud IMO makes much more sense than redirecting everything and leaving it to admins to report one after the other errors to fill a growing list of excludes.

But try_files is possibly a great compromise, to let existing paths untouched and only redirect those to Nextcloud which do not exist. This would make the excludes obsolete as well, assuming that ACME and PKI clients do create those directories. I'm just not sure which one takes procedure when a rewrite or another redirect/return is used by those clients (or other handlers). Also the error message/answer of the ACME/PKI server could be confusing when the client indeed failed to create the validation file and the server is then redirected to Nextcloud 🤔. Probably both is good, keeping the list of excludes and additionally replace the last return with a try_files, to leave existing paths untouched.

@moviuro
Could you create a PR? It's more productive to discuss among an actual commit, I think.

[josh4trunks]

I think having our example nginx config serve any file from a .well-known folder, that doesn't normally exist in Nextcloud, is making too many assumptions. I think if an admin is able to create the hidden folder, and add the text file, they can also do the "hard work" of editing their Nginx config

But I am open to having my mind changed on a seperate PR. Maybe the ideal solution is documentation to create a .well-known/security.txt and properly serve it with Apache and Nginx.

[MichaIng]

I think having our example nginx config serve any file from a .well-known folder, that doesn't normally exist in Nextcloud, is making too many assumptions. I think if an admin is able to create the hidden folder, and add the text file, they can also do the "hard work" of editing their Nginx config

I see it exactly the opposite way 😄: Too many assumptions are made when every .well-known URL is redirected to Nextcloud, considering the large number of possible uses, shown in the Wikipedia article linked above and despite the existence of the file. Creating a hidden directory in Linux is nothing more than a leading dot in its name, and in many cases those are created by other software, which is then broken (like the cases which led to this PR). So I'll always vote for redirecting less (a defined list of URLs) to Nextcloud by default and having additional redirects e.g. commented in the config and/or in the documentation to be enabled only when explicitly required. But I'm developing a distro which explicitly deals with multiple websites/applications served through the same webserver, so naturally not 100% objective in this regards 😉.

The try_files looks like a smart middle way, as I cannot imagine a case where someone explicitly creates a file like .well-known/security.txt for a different purpose than having it served, as this is the whole reason for this directory. But it makes sense to evaluate/discuss it in the context of a commit/PR ✌️.

[ChristophWurst]

For additional context please also see nextcloud/server#24702. Nextcloud has an API for apps to hook into well known handling. From a static point of view of this documentation we can't make assumption about what well known services Nextcloud will handle. It will depend on the enabled apps.

[moviuro]

https://nginx.org/en/docs/http/ngx_http_core_module.html#try_files

It is possible to use the following. It will try by order of preference 1. try the files on disk at the specified location and 2. if not found, then redirect to Nextcloud.

try_files $uri $uri/ /index.php$request_uri;

One-size-fits-all?

(Also, what about $is_args$args? this appears in other try_files statements)

[josh4trunks]

Sure, try_files can work for Nginx server admins, who create a .well-known directory and add files to be served. But I think this is not a use case we need to cover in Nextcloud's example nginx config. This really isn't related to Nextcloud specifically.

If an admin wants to serve other files not related to Nextcloud they will need to adapt their Nginx config accordingly.

[moviuro]

not related to Nextcloud

Do you really consider it that way? I have a dedicated machine (jail) that is used only for Nextcloud, and I for sure don't expect any interference from Nextcloud when querying https?://nextcloud.domain.tld/.well-known/*, espescially when it should "just work".

[josh4trunks]

You are free to use whatever Nginx configuration works for your setup.

I also run multiple services, in separate jails, all handled my a single Nginx frontend, and in my case I realize need to customize my configuration accordingly.
But this example configuration is targeted at admins hosting Nextcloud. We aren't making assumptions that they will be using other services and need to serve additional URIs from a .well-known directory.

If you really believe this is necessary, we should also make sure the Apache .htaccess configuration also handles .well-known/server.txt correctly since the nginx config should match the Apache behavior.

[josh4trunks]

I'm all for having a .well-known/server.txt, now that I know about it I will probably add one too!

But unless this is a file or URI served by Nextcloud, I do not see why we need our example Nginx config to handle it.

[MichaIng]

But unless this is a file or URI served by Nextcloud, I do not see why we need our example Nginx config to handle it.

Probably it's splitting hairs with the words, but that sentence somehow exactly supports our argument: Why should Nextcloud handle a file/URI that it is not able to serve? The change would not add but remove URIs from being handled special (via redirect) with the config. But I understand how you mean it, when seeing the excludes as what is handled.

Especially example configs, which are often copy&pasted, are IMO good when they are compatible and not shaped a way as if the own software is the only software/purpose that is ever going to run on that server. Additional optional features are great when offered in a commented way with some explanation. I remember the webfinger rewrite rule from an earlier version of the Nginx example config, where it was commented with some explanation. Now it throws an admin panel warning, when not redirected, regardless whether the admin wants webfinger to work or not, or wants a different handler for it than Nextcloud. Same with nodeinfo, IMO the wrong direction.

The question is whether try_files $uri $uri/ /index.php$request_uri; has any downsides compared to the current forceful redirect. Can you think of any case where either another software or the admin is creating a file in .well-known and then wants it to be ignored in favour of Nextcloud trying to handle it?

Yes the same is true for .htaccess as well:

RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule ^\.well-known/ /index.php [QSA,L]

The ACME/PKI excludes are then not required anymore, as those don't match the condition when the file/dir exists, but could still be kept to be failsafe and support parent config rewrite rules or such. The above of course has some performance implication, as additional checks are done, same with the Nginx try_files. But taking into account the general Nextcloud access performance, with PHP handler, all the bootstrapping etc, I'm pretty sure it is not even measurable.

[josh4trunks]

Ok, if Apache handles serving files a certain way, in general we should match the functionality. (unless there is an nginx specific difference/reason not too)
Any suggestion to change the default handling should be in context of changing the Apache .htaccess, with nginx example config changes made to match the new handling.

So if you are saying Apache handles this like the try_files example then I agree we should go with that. But we need a PR, with this explained to actually review, lol.

[MichaIng]

So if you are saying Apache handles this like the try_files example then I agree we should go with that.

Ah no sorry, I meant it should do also that way, but it does not currently. The two RewriteCond lines above are not present in the current .htaccess. But you are right that it should be always aligned for both webservers (all webservers). Best is hence to open a PR at the server repo and involve some guys from the team into the discussion/decision by this: https://github.com/nextcloud/server/blob/master/.htaccess#L68

Based on acceptance/result a companion Nginx example config PR can then be opened here.

[josh4trunks]

Ok, thanks for the explanation, that process makes sense to me. I see the .htaccess currenly 404's anything in .well-known except what is explicitly included.

An even cooler solution for security.txt would be if Nextcloud had a settings page similar to the link below, which allowed you to enter relevant details. Then it could generate the needed file, and handle using the new functionality @ChristophWurst mentioned.
https://securitytxt.org/