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
Feature HA Proxy protocol support #2300
Conversation
Signed-off-by: hitech95 <nicveronese@gmail.com>
Thanks for submitting this pull request. bors try Note: if this build fails, read this. |
tryBuild failed: |
Signed-off-by: hitech95 <nicveronese@gmail.com>
Signed-off-by: hitech95 <nicveronese@gmail.com>
3f048e9
to
f088742
Compare
Why do you create new endpoints instead of disabling STARTTLS in nginx when EMAIL_PROXYED is true? Shouldn't it imply TLS = false ? |
Some proxies dont play nicely with STARTTLS. That port set allow to have the front container to handle StartTLS and the proxy to handle SSL termination. What I want to have only SSL but allow STATTLS on port 25? I cannot SSL terminate to a STARTTLS endpoint. |
I don't understand what you mean. Can you clarify the usecase for this (why do you need a proxy in the first place, why does it need to do the SSL termination, ... )?
That's the worst of both worlds: you end up with SSL termination in two different places -> you need to deploy valid certificates in both. |
What if you want to have SSL provided by front but non encrypted local/lan endpoints for internal services? This allow to have SSL, StartTLS, and not encrypted endpoints available and also support for PROXY protocol. |
You can use the proxy to export a port without SSL (where the client would connect without SSL to the proxy and the proxy would establish an SSL connection to Mailu). In Mailu we specifically discourage people to run a mailserver without TLS (except for testing, see https://mailu.io/master/compose/setup.html#tls-flavor).
I think that allowing for the proxy protocol to be supported is an enhancement of what we already do (by allowing HTTP reverse proxies); I do not think that exposing non encrypted endpoints brings anything useful to the table (and I do think that it's a bad idea, going against what we are attempting to do elsewhere: ensure that clients connect in a secure way). |
I think that only nginx would allow somehting like that. Most proxies are not able to do so. This reduce versability.
You can always be enabled and it is independent if you need the not secure port set.
Right now ther are 2 endpoints that are not encryped and not exposed: the two used by the webmail. Let's look to some examples:
If you think that this is not necessary I can drop that section. |
I'm using in my docker stack with Traefik in front of front. Traefik is configured with HA proxy protocol so I can forward the source IP address. Nginx and Ha proxy can be used as a proxy too! So k8s Ha might be one of the applications. |
@abebeos if what you are looking for is images with the patch, you can get them from https://hub.docker.com/r/mailuci/ https://hub.docker.com/r/mailuci/nginx/ for pr2300 We're definitely not merging code that hasn't been tested and to me it's very far from clear that this additional functionality does not break anything. |
bors try |
tryBuild succeeded: |
(Only talking about PROXY_PROTOCOL) This change is mandatory when running Mailu behind a reverse proxy. This scenario might occur when using a single floating IP for incoming mail and web traffic in a Kubernetes environment. (My use case. Details can be found here.) Please consider merging those changes. I think it would be a nice addition when having clustered setups in mind. I cant speak about EMAIL_PROXYED, since I don't really know when it would be used. |
Since Edit: When it comes to testing it in an environment that uses the |
We've discussed this at the last dev-meeting (sorry this isn't in the minutes)... To reach a mergeable state we need:
|
reformulating the above about EMAIL_PROXYED, I see two cases: we either have:
The PR as currently proposed adds new ports in the case where we have a compatible proxy and I'm arguing that it's not useful since the compatible proxy can export its own "listeners" with or without SSL on its own. |
I can provide my use case. (see below)
See my example below.
Yes this is a must, I dont remember exactly but without it NGINX don't route the traffic (need testing).
The email TCP proxy and web http(s) proxy could be different this meas that both protocols should be different. One could use nginx for email end apache for http(s). My docker stacks:
Mailu stack:
An example of my stack that use local relay:
Local relay settings are: |
He's not needed to map mail ports to traefik ? Like 25 587 etc... |
any update on this? would be good to have resolved and merged in 👍 |
You'll have to wait for the next release or start using master (our development branch) to use it. |
Original patch author here. I didnt had any new feedback. Example network is composed as:
How would you handle this case with the proxy protocol enabled?
This expose another issue/problem:
While developing the patch I've also discovered the following user case that I tried to mitigate using
|
I also have a use case that requires this if I want to run it in k8s: HAProxy (terminates TLS, but also IPv4) -> Nginx Ingress Controller (IPv6 only) -> Mailu (IPv6 only). At the moment I have to use a dedicated VM running a docker setup. If this gets merged I can migrate it to K8s and use HA, Distributed Storage etc. |
I'm running with a DO LB (in proxy protocol mode), to Traefik, then to Mailu via Docker Swarm.
Having run my setup for over a year, this is the final hurdle, and here is some research I have conducted. Now its a longshot, but it appears theres an issue with the Docker overlay network (when you create a swarm), which it seems does not allow for ip forwarding - https://community.traefik.io/t/whitelist-swarm-cant-get-real-source-ip/3897/2 I've listed a couple of options here to try, which may help with the Proxy Protocol support - i'm not sure if PP may need to be turned off in these test cases below but its worth trying! There is also an indepth post about https://stackoverflow.com/a/44648488 - details that you would run Traefik on each server in your swarm as global deployment to map to the ports on each server
There is also the docker-ingress-routing-daemon - https://github.com/newsnowlabs/docker-ingress-routing-daemon which looks promising and may be worth a try, which reads:
This may be worth trying as it seems to tackle the common issue we are having I've also seen some posts around enabling IPV6 for Docker Swarm due to overlay issues, which means you need to recreate this network bridge for the swarm - https://github.com/robbertkl/docker-ipv6nat - although with latest version of docker this container may no longer be required - robbertkl/docker-ipv6nat#65 - but the recreation to allow for ip forwarding whilst keeping the overlay network is interesting! With this in mind, I've modified this gist with the ipv6 swarm mode config - with the additional ip forwarding - i've not tested this yet so please text on a non-production environment if you wish to try https://gist.github.com/hgross/26042d052f58feeb6d1b329e8dd2dfcc (this was to change subnet address) You can check existing network setup here on your server node:
Heres the bash steps, which will vary if your running docker as a service for your swarm, etc.
Hope this helps so we can finally bring the client ips into our containers! |
@@ -68,8 +70,8 @@ Because the admin interface is served as ``/admin``, the Webmail as ``/webmail`` | |||
|
|||
location ~* ^/(admin|sso|static|webdav|webmail|(apple\.)?mobileconfig|(\.well\-known/autoconfig/)?mail/|Autodiscover/Autodiscover) { | |||
proxy_set_header Host $host; | |||
proxy_set_header X-Real-IP $remote_addr; | |||
proxy_pass https://localhost:8443; | |||
proxy_set_header X-Real-IP $remote_addr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is clearly introducing a bug; the trailing semicolon is mandatory.
See Mailu#2300 for the initial proposal
See Mailu#2300 for the initial proposal
See Mailu#2300 for the initial proposal
See Mailu#2300 for the initial proposal
See Mailu#2300 for the initial proposal
See Mailu#2300 for the initial proposal
See Mailu#2300 for the initial proposal
* Update tests/compose/test.py Co-authored-by: Alexander Graf <ghostwheel42@users.noreply.github.com> * Update docs/cli.rst Co-authored-by: Alexander Graf <ghostwheel42@users.noreply.github.com> * Update tests/compose/test.py Co-authored-by: Alexander Graf <ghostwheel42@users.noreply.github.com> * Update docs/cli.rst Co-authored-by: Alexander Graf <ghostwheel42@users.noreply.github.com> * Update docs/cli.rst Co-authored-by: Alexander Graf <ghostwheel42@users.noreply.github.com> * Czech translation Czech translation * Update messages.po * Apply DEFAULT_QUOTA to user creation admin ui page * Fix UserForm of Admin UI * Fix updating the default quota_bytes in the form * Fix user create form * Change rspamd override system to use include with lowest priority. All override files are used as if they were placed in the rspamd local.d folder. From the newsfragment: New override system for Rspamd. In the old system, all files were placed in the Rspamd overrides folder. These overrides would override everything, including the Mailu Rspamd config. Now overrides are placed in /overrides. If you use your own map files, change the location to /override/myMapFile.map in the corresponding conf file. It works as following. * If the override file overrides a Mailu defined config file, it will be included in the Mailu config file with lowest priority. It will merge with existing sections. * If the override file does not override a Mailu defined config file, then the file will be placed in the rspamd local.d folder. It will merge with existing sections. For more information, see the description of the local.d folder on the rspamd website: https://www.rspamd.com/doc/faq.html#what-are-the-locald-and-overrided-directories * Fix some small errors * bring back removed blank lines * fix Mailu#2693 * fixes suggested by diman0 * Maybe fix the tests * the space may or may not exist * Renumber and clarify * Add changelog entry for PR2676 * Introduce AUTH_PROXY_LOGOUT_URL * Make the login page guess where to redirect * Fix 2692: make the external auth proxy usable * doh * Make it work for /admin/antispam too * Handle WEBROOT_REDIRECT better * Set snappymail autologout time according to PERMANENT_SESSION_LIFETIME closes Mailu#2680 * Upgrade snappymail to v2.26.4 * Fix broken link. Add extra clarification for login targets. * Paranoia: drop the headers we don't use * Check https://attackshipsonfi.re/p/exploiting-cors-misconfigurations out * Switch the container registry used for deploying images from docker to ghcr.io (github). Images are now first build with '-build' appended to the tag. E.g. ghcr.io/mailu/admin:master-build. This is to prevent the image being available before automatic testing has completed. In the deploy job, the final image is pushed (this still works the same). Update setup & documentation for switch to ghcr.io * Add changelog entry. * Add missing () * Extend roundcube's session lifetime * Fix typo and wording in faq.rst * Update changelog with extra info. * Fix build.hcl / CI.yml regarding labels The version label and versions passed to docs image were based on the tag. Now we first build the images with -build appended to the tag, we cannot use the tag as version label. A new env var is introduced to pass the version to the build.hcl file. This will be used to set the VERSION label in the image, and pass as build arguments to the docs image. * Proxy endpoint was checking real client ip instead of proxy ip for validating PROXY_AUTH_WHITELIST * Add fallback just in case X-Forwarded-By is empty. * Add fix for wrong redirect in proxy scenario and accessing WEBROOT_REDIRECT * Fix a typo. * Fix error in check for proxy scenario * Don't use the header when we don't need it. * Provide a changelog for minor releases. The github release will now: * Provide the changelog message from the newsfragment of the PR that triggered the backport. * Provide a github link to the PR/issue of the PR that was backported. Switch to building multi-arch images. The images build for pull requests, master and production are now multi-arch images for the architectures: * linux/amd64 * linux/arm64/v8 * linux/arm/v7 Enhance CI/CD workflow with retry functionality. All steps for building images are now automatically retried. If a build temporarily fails due to a network error, the retried step will still succeed. * Forgot to change the target. * Also forgot the --push argument. * Prevent creation of unknown/unknown arch. Set more forgiving timeouts for scenario where image is build without cache. Set better readable tags. * Update docs/compose/requirements.rst * Update docs/compose/requirements.rst * Update docs/setup.rst * Update docs/setup.rst * Update setup.rst * Introduce connection string (database url) for roundcube. Remove database choice from setup. Remove the old *DB_* database env variables from the documentation. The env vars are deprecated now. They will be removed after the upcoming Mailu release. * Sigh. Forgot to actually save the modified requirements-dev.txt file. Remove the pinned version for requirements for dev. The blocking issue is resolved, so no need to pin the old version. * Rephrase the doc * Make sure that the arm build also uses build-cache. Remove the step of building the base image. This is not required. when it is build for the first time for an image, it will be part of the build cache of that image. * Fix a later/latter typo * nginx: Allow http and/or mail servers to accept the PROXY protocol See Mailu#2300 for the initial proposal * nginx: fix proxy settings when PROXY protocol is used Tested-By: Didier Raboud <odyx@raksha.ch> * nginx with PROXY protocol; much stronger wording * nginx behind proxy: attackers are not only men * nginx with PROXY protocol for mail; only set_real_ip_from in 'all' and 'mail' alternatives * l10n fr: fix Relayed domains' plural * l10n fr: add DNS TLS and autoconfig translations * l10n fr: uppercase accented 'status' * nginx behind proxy: provide a healthcheck for localhost over port 10204 * nginx with proxy protocol: clarify documentation * Mirror alpine image to ghcr.io/mailu docker org to prevent docker pull rate limit. Use mirrored ghcr.io/mailu/alpine image as base image. * Adapt mirror.yml that it can only be run manually When starting it manually, you can provide the tag that must be synchronised * Update instructions for syncing alpine image * Fix access to radicale * Remove not needed mailu.env file. * Only account for distinct attempts in rate limits * should never happen but heh * Ensure we always ask for the existing password before allowing a change * resets don't need the current password * This won't work * LOG_DRIVER just doesn't work * Update core/admin/mailu/limiter.py Co-authored-by: Dimitri Huisman <52963853+Diman0@users.noreply.github.com> * s/docker-/mailu-/g * No need for that * Initial changes for Mailu 2.0 release * Update releases.rst * Update releases.rst * Make the journald container tag changes consistent * Fix doc * Process latest towncrier entries into changelog.md * doh * fix bug * Clarify * Clarify * Don't rate-limit port 25, ever. * Update dependencies with CVEs * Fix unmet dependency * Further improve releases.rst * Newsfragment for releasing Mailu 2.0 * Fix tag-release step in workflow which prevented github releases from being created automatically. Cause was that a specific method is required for assigning multi-line strings in github workflow files: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings * Improve releases.rst. Add extra links to relevant sections in documentation. Add example of using the new override location for rspamd. Add clarification in rspamd section for rspamd override change and new autoconfig.* endpoint * Update releases.rst * Add reminder to configure mta-sts * Use intermediate images for CI workflow First the base and assets images are build and pushed to ghcr.io. After that all main images are build. These images use the previously build base/assets image by pulling it from ghcr.io. * Forgot to update the hcl file to the new build-ci.hcl file * Add release note for PR 2748 * Fix config-import. Config with dkim key could not be imported. * Add test to show it's broken * Fix it * maybe fix eval * fix 2757 * review * Unique exit codes * Fix Mailu#2720 * Fix Mailu#2766 * Sanitize logs as appropriate * fix Mailu#2764 * Always exempt app-tokens from rate limits * doh * ratelimit: ensure we hit the ip-ratelimit on unsuccesful attempts against a valid account * Make it happen post-deduplication * Whitelist all mailso* stream types in snuffleupagus for snappymail For attachment download in snappymail to work, at least mailsoliteral is needed. The additionally used stream types (from looking at the snappymail source) have also been added, to ensure compatability with whatever feature might rely on them …. * adapted to v2 release and a docker change - v2 changed the path - docker deprecated/removed the scale command, you have to do it like this now * Update antispam.rst shorter variant (scale isn't needed as there's only 1 at a time anyway) * Implement managesieve support * add tests * LD_PRELOAD may not be in ENV * Try to do the same for ARM64, log a message if we do * warning is enough * Remove another useless message * Add health-check * tweak-logs * Make it generic. Should we implement TARPIT? * maybe fix healthcheck * Ensure we log rport * Send rport too * Fix logs in the SMTP container * dovecot is creating zombies * Simplify the health-check * fix Mailu#2139 * COMPRESSION_LEVEL too * as requested in review * noticket * Deal with certwatcher too * Fix roundcube's spellchecker * Document in the FAQ * typo * Another typo * doh * grmll. * Fix typo * Fix2805 * Improve auth-related logging * change healtcheck again * Add this endpoint back too * Make webmails use a different port without proxy protocol * Need this too * Update version to 2.+ in release template * Rename as requested by reviewer * review * add token.comment too * Update nginx.py Doh * Update nginx.py Fix typo * quote the comments * Don't send ooo messages to noreply@ * update docs * Note ports that need to be open in the firewall The primary purpose of this change is to include the keyword "firwall" because when I went to open up ports in my network security group I expected a search for "firewall" in the docs to instantly bring this information up, but it didn't. * Authentication failed for email clients when the password contained a non latin-1 character. * Also url encode the password when authentication fails * Get the password from the source. Remove password from response (not needed) * Retrieve raw password on the correct location * Update 05_connectivity test to use UTF8 password. * Update core/admin/mailu/internal/views/auth.py * Ensure we log which account is invalid * Fix the bug @ghost has reported * Use dovecot-proxy where appropriate * Add doc for DEFAULT_QUOTA * Allow multiple IP addresses/networks to be set for tokens * add migration * bugfix for dovecot-proxy * bugfix for dovecot-proxy * newsfragment * increase the number of postfix workers * Document that the default config for netplan is broken * Add a clue * Fix issue Mailu#2811. Clamav Healthcheck created zombie processes --------- Co-authored-by: Florent Daigniere <nextgens@users.noreply.github.com> Co-authored-by: Alexander Graf <ghostwheel42@users.noreply.github.com> Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com> Co-authored-by: score <seejay.11@gmail.com> Co-authored-by: Florent Daigniere <nextgens@freenetproject.org> Co-authored-by: S474N <S474N@users.noreply.github.com> Co-authored-by: PM Extra <pm@jubeat.net> Co-authored-by: Dimitri Huisman <diman@huisman.xyz> Co-authored-by: Dario Ernst <dario@kanojo.de> Co-authored-by: Dimitri Huisman <52963853+Diman0@users.noreply.github.com> Co-authored-by: Didier 'OdyX' Raboud <odyx@raksha.ch> Co-authored-by: Didier Raboud <odyx@debian.org> Co-authored-by: Dario Ernst <dario.ernst@rommelag.com> Co-authored-by: elandorr <56206745+elandorr@users.noreply.github.com> Co-authored-by: AJ Jordan <alex@strugee.net>
What type of PR?
Enhancement
What does this PR do?
This PR implements PROXY protocol for email endpoint on
front
. This allow to have thefront
container behind another email proxy. WithPROXY
protocol we can forward theOrigin-IP/Client-IP
wich allowsadmin
to properly apply throttling rules also behind a proxy. Enabling thePROXY_PROTOCOL
all the mail endpoints expect it.Custom endpoints have also been added to provide auth only endpoints. this allow to have SSL termination in the proxy and STARTLS available on port 25 without having to forward TLS traffic with the proxy.
Those endpoins are not expected to be mapped to the host so they are not been added to the
Dockerfile
EXPOSE
list.Related issue(s)
front
behind a mail proxy (like another nginx instance) : closes PROXY Protocol support #1472