Added support for proxy configuration using environment variables

[clickkiddie]

Added environment variables NEXTCLOUD_OVERWRITEPROTOCOL, NEXTCLOUD_OVERWRITEHOST, NEXTCLOUD_OVERWRITEWEBROOT according to https://docs.nextcloud.com/server/13/admin_manual/configuration_server/reverse_proxy_configuration.html

[icasdri]

Is overwrite.cli.url needed as well?
Reference: https://help.nextcloud.com/t/stuck-in-a-login-loop-when-logging-in-from-sync-client-on-linux/47549

[clickkiddie]

Is overwrite.cli.url needed as well?
Reference: https://help.nextcloud.com/t/stuck-in-a-login-loop-when-logging-in-from-sync-client-on-linux/47549

My setup runs with NEXTCLOUD_TRUSTED_DOMAINS, NEXTCLOUD_OVERWRITEPROTOCOL and NEXTCLOUD_OVERWRITEWEBROOT and I can access it using webfrontend, windows desktoop app and andoid app without issues. I don't think overwrite.cli.url is required. It isn't even mentioned in the nextcloud manual.

[lukasmrtvy]

Fixes #792

[lukasmrtvy]

You should also consider to implement support for trusted_proxies NEXTCLOUD_TRUSTED_PROXIES
https://docs.nextcloud.com/server/16/admin_manual/configuration_server/reverse_proxy_configuration.html#defining-trusted-proxies

Without this, You will get a message from nextcloud:

The reverse proxy header configuration is incorrect, or you are accessing Nextcloud from a trusted proxy. If not, this is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud. Further information can be found in the documentation.

[clickkiddie]

You should also consider to implement support for trusted_proxies NEXTCLOUD_TRUSTED_PROXIES

NEXTCLOUD_TRUSTED_PROXIES is already included in master branch

[lukasmrtvy]

where ?
https://github.com/nextcloud/docker/blob/master/16.0/apache/entrypoint.sh

[clickkiddie]

where ?
https://github.com/nextcloud/docker/blob/master/16.0/apache/entrypoint.sh

you are correct. It is missing. I mixed it up with 'trusted_domains'. 'trusted_proxies' should be set for a sane configuration. I'll add it when I have time to test it

[clickkiddie]

You should also consider to implement support for trusted_proxies NEXTCLOUD_TRUSTED_PROXIES

NEXTCLOUD_TRUSTED_PROXIES is already included in master branch

NEXTCLOUD_TRUSTED_PROXIES has been added
additionally the config setup has been moved to a region where it is also done after install (so you can update container configuration while keeping the volumes)

following tests have been performed (updated):

1. setting NEXTCLOUD_TRUSTED_PROXIES disables security warning in admin panel -> PASS
2. setting reverse proxy config before install -> PASS
3. setting reverse proxy config after install -> PASS
4. setting incorrect trusted proxy triggers security warning-> FAIL (it seems that the config-check only checks if trusted_proxies is present, but not if it actually matches the proxy used)
5. checking IP-addresses in access log (trusted proxy correct) -> PASS (real client IPs are shown)
6. checking IP-addresses in access log (trusted proxy incorrect) -> FAIL (real client IPs are shown; expected REAL-IP-HEADER is ignored)

Can somebody explain 6? is that intended behavior? I hope the check is stricter for other header fields.

arm64-apache image for testing will be available on Dockhub (clickkiddie/nextcloud:latest) in about 30 min (upload takes time)

[gobengo]

@clickkiddie Thanks for making this. I can't wait to use it! Will you rebase/merge from master so this PR is mergeable without write conflicts?

[clickkiddie]

@clickkiddie Thanks for making this. I can't wait to use it! Will you rebase/merge from master so this PR is mergeable without write conflicts?

I'll have a look on that during the coming week

[clickkiddie]

sorry, I recently messed things up during merge. Now it should be in sync again.
@gobengo please provide some feedback if that works for you, maybe it increases our chances to get this PR integrated by the maintainers

[gobengo]

@clickkiddie way to go! Thank you.

@J0WI@tilosp y'all have been merging PRs recently. Will you consider merging this or providing feedback? It would really help with using the docker container behind reverse proxies, which I think is pretty common.

Thank you :) <3

[kesselb]

additionally the config setup has been moved to a region where it is also done after install (so you can update container configuration while keeping the volumes)

Thanks 👍

4. setting incorrect trusted proxy triggers security warning-> FAIL (it seems that the config-check only checks if `trusted_proxies` is present, but not if it actually matches the proxy used)

Yes: nextcloud/server#14606

6. checking IP-addresses in access log (trusted proxy incorrect) -> FAIL (real client IPs are shown; expected REAL-IP-HEADER is ignored)

https://github.com/nextcloud/server/blob/fdf4e1ebb2a458f767b57abddac337d1b92f45b6/lib/private/AppFramework/Http/Request.php#L642-L665

is that intended behavior?

I don't think so. You should see the ip of the reverse proxy in this case.

I don't think overwrite.cli.url is required. It isn't even mentioned in the nextcloud manual.

It's good to set it but not required if overwriteprotocol and overwritehost are present. Nextcloud uses the value to generate absolute urls from cli (e.g cron, background job). https://github.com/nextcloud/server/blob/b8fcf6e9b3d492d19bbbdcd144428fef9bbf2324/lib/private/URLGenerator.php#L242-L250getBaseUrl will do the fallback to overwriteprotocl and overwritehost.

NEXTCLOUD_TRUSTED_PROXIES

Could you consider to set a default value? I prepared a pull request for it #891 but with a different approach. This one (setting the values on container start) is better. Nginx and Traefik set X-Forwarded-Host and X-Forwarded-Protocol by default. If the request is coming from a trusted proxy overwriteprotocol and overwritehost are superfluous. Not sure if overwrite.cli.url is required then 🤔 (becaue the fallback to overwrite* is missing).

wants to merge 39 commits into

Could you try to cleanup the git history a bit? You can use rebase for it.

[clickkiddie]

NEXTCLOUD_TRUSTED_PROXIES
Could you consider to set a default value?

I don't think a default should be set. It might introduce security issues when used without reverse proxy.

6. checking IP-addresses in access log (trusted proxy incorrect) -> FAIL (real client IPs are shown; expected REAL-IP-HEADER is ignored)

https://github.com/nextcloud/server/blob/fdf4e1ebb2a458f767b57abddac337d1b92f45b6/lib/private/AppFramework/Http/Request.php#L642-L665

I don't think so. You should see the ip of the reverse proxy in this case.

I am not able to reproduce this one anymore. Maybe I did something wrong during my tests. Code looks good on first glance, despite scenarios where multiple forwarded-for-headers are defined in config.

Could you try to cleanup the git history a bit? You can use rebase for it.

done

[kesselb]

I don't think a default should be set. It might introduce security issues when used without reverse proxy.

With private IPv4 addresses?

[clickkiddie]

I don't think a default should be set. It might introduce security issues when used without reverse proxy.

With private IPv4 addresses?

Sure. Many larger networks in Universities and Companies have their whole infrastructure - including workstations and wifi dhcp range- in these ranges. Having e.g. 10.x.x.x trusted per default would allow a huge number of clients to spoof being a proxy.

[kesselb]

Convinced ;) overwrite.cli.url would be nice 🙈

[clickkiddie]

@clickkiddie can you complete the signoff?

it looks like thats what is missing for this PR to be completed!?

did it. sorry for latency, i'm quite busy currently.

can somebody comment / verify the latest test findings from @kesselb ? Which configs work, which not? Do we need to improve the config script or is a doc update sufficient? Currently I have no test system available to work on it.

[JonasBVS]

@clickkiddie can you complete the signoff?
it looks like thats what is missing for this PR to be completed!?

did it. sorry for latency, i'm quite busy currently.

can somebody comment / verify the latest test findings from @kesselb ? Which configs work, which not? Do we need to improve the config script or is a doc update sufficient? Currently I have no test system available to work on it.

@clickkiddie i can try verifying or commenting, but is there a procedure for this? which image should i use for testing?

besides the travis-ci builds seems to fail at this point!?

[mschoettle]

I have exactly this problem (fpm-alpine, nginx & traefik).

How can this PR be tested?

[J0WI]

How can this PR be tested?

• clone this repo
• apply the patch locally
• run docker build -t nextcloud:patch-819 17.0/fpm-alpine
• run the specified version (nextcloud:patch-819) of the container

[mschoettle]

Thanks!

Setting these environment variables on a new instance causes the following problem:

nextcloud-test-app | setting trusted proxies…
nextcloud-test-app | Nextcloud is not installed - only a limited number of commands are available
nextcloud-test-app | nextcloud-test-app | nextcloud-test-app | There are no commands defined in the "config:system" namespace. nextcloud-test-app | nextcloud-test-app | nextcloud-test-app exited with code 1

Without the environment variables (to install it first) I first get "Access through untrusted domain" error when accessing the web interface. For some reason right now even if I add the domain manually in config.php it still tells me it is an untrusted domain.

Edit: With the current nextcloud:fpm-alpine image using an environment variable NEXTCLOUD_TRUSTED_DOMAINS works in seeing the install page.

[mschoettle]

Setting NEXTCLOUD_TRUSTED_DOMAINS works in master.

There should probably be a check before setting the configs based on the environment variables to ensure Nextcloud is installed. However, at the end of the script "$installed_version" != "0.0.0.0" so might have to check with occ status (which will contain - installed: false in the output) but maybe there's an easier way.

[kesselb]

maybe there's an easier way.

Not messing around with occ ;) I was thinking about putting all those reverse proxy configurations into a separate config file similar to https://github.com/nextcloud/docker/blob/master/.config/smtp.config.php. That's probably less performant but much easier. And works better with the weird way nextcloud handles configuration. But time ... 😞

[mschoettle]

maybe there's an easier way.

Not messing around with occ ;)

I meant to just "parse" the output of the status command to see whether Nextcloud is installed.

[schildbach]

What's the state of this PR? Due to Nextcloud not honoring the X-Forwarded-Proto proxy header and also the NEXTCLOUD_OVERWRITEPROTOCOL variable not yet present, Nextcloud currently cannot be used within Docker and behind an SSL proxy. It throws around URLs that link back to unsecure HTTP.

[kesselb]

What's the state of this PR? Due to Nextcloud not honoring the X-Forwarded-Proto proxy header and also the NEXTCLOUD_OVERWRITEPROTOCOL variable not yet present, Nextcloud currently cannot be used within Docker and behind an SSL proxy. It throws around URLs that link back to unsecure HTTP.

You can set most configuration variables via ENV by prefixing NC_. NC_overwriteprotocol for that case. Hopefully it will replaced by NEXTCLOUD_OVERWRITEPROTOCOL sometime but for the time beeing.

[schildbach]

NC_overwriteprotocol works. Thanks!

[ghostkernel-core]

Can anyone please tell me the ENV variables to required to set for the official Nextcloud:latest image so that my config.php have these two settings from the fresh install (initial run)

'overwriteprotocol' => 'https',
'filelocking.enabled' => false,

[J0WI]

Can anyone please tell me the ENV variables to required to set for the official Nextcloud:latest image so that my config.php have these two settings from the fresh install (initial run)

As you can see this PR is not yet merged, so there are currently no ENVs for these settings. You can mount additional config files in the config folders.

[thannaske]

Is there any plan to complete this (imho very important PR)? Without those environment variables you are not able to properly deploy this image in a high-scale k8s-environment.

[ghostkernel-core]

Is there any plan to complete this (imho very important PR)? Without those environment variables you are not able to properly deploy this image in a high-scale k8s-environment.

I don't see any movements for the time being. But on the other hand I developed a custom php to add any variables required just pass through the ENV file of the stack. Knock me stating your required variables, if you need one.

[J0WI]

Recent progress have been made in #1048.

[J0WI]

Fixed in #1048.

[Kr4cker]

Your domain is reachable at https://cloud.domain.com via reverse proxy (192.168.10.2)

Add 192.168.10.2 to trusted_proxies
Set overwriteprotcol to https
Set overwritehost to cloud.domain.com
Set overwrite.url.cli to https://cloud.domain.com
I hope this helps a little to understand this topic.

Just a small hint for the future people coming to this thread.
The entry of @kesselb helped me to solve my login issue.
I had a hard time at the beginning cause I just copy pasted the attribute names from here.
But the overwriteprotocol is missing an o. After fixing this typo, my Nextcloud login worked =)

[yassinwallace]

@Kr4cker Thank you for your input. It helped me.
I think it is overwrite.cli.url and not overwrite.url.cli.
I'm a total newbie on the subject and this is my very first comment on Git. So I might be wrong.

[Kr4cker]

@yassinwallace yes you are right. I checked the Nextcloud documentation.