fix: issue the DAV discovery redirects with the client's real scheme - #138
Merged
Conversation
… 34.0.3:1 -> 34.0.3:2 Apache builds the Location of its .htaccess redirects from its own connection scheme, which behind the TLS-terminating StartOS proxy is always http. So /.well-known/carddav, /.well-known/caldav, and the DavClnt user-agent redirect on / all sent clients to an http:// URL, which on an https-only address (the normal 0.4 LAN binding) nothing answers. macOS/iOS account setup walks exactly that endpoint: the account verified but never synced (#137). The rest of the chain already worked: SDK 2.0.9 defaults addXForwardedHeaders on for http bindings, StartOS 0.4.0.1 injects X-Forwarded-Proto/-For on TLS vhosts, and config.php's trusted_proxies (10.0.3.0/24) makes Nextcloud honor them — PHP-generated URLs were already per-request correct. And the issue's proposed htaccess neutralization cannot work: Nextcloud has no PHP well-known handler for carddav/caldav (only webfinger/nodeinfo), so falling through to index.php returns 404. So the fix duplicates the three redirects scheme-aware in an Apache conf baked into the image: <Directory /var/www/html> rules under RewriteOptions InheritDownBefore run ahead of the volume's stock .htaccess, issue https:// redirects when the proxy's X-Forwarded-Proto says https, and fall through to stock behavior on plain-http (onion) bindings. A conf rather than an .htaccess patch is what reaches installs whose volume already carries this Nextcloud version — the entrypoint only rsyncs on a version change. mod_headers cannot do it: a 3xx's Location lives in the one header table Header edit never processes (verified empirically). APACHE_DISABLE_REWRITE_IP=1 pins the image's mod_remoteip off. It is inert today only because its conf reads X-Real-IP, which nothing sends; were a future image to read X-Forwarded-For, it would rewrite REMOTE_ADDR out of trusted_proxies range and silently break the whole scheme/client-IP chain. Verified on StartOS 0.4.0.1: carddav/caldav/DavClnt 301/302 to https on both the LAN wildcard vhost (nonstandard port) and a tunnel domain vhost; plain-http leg unchanged; discovery-then-PROPFIND 207; occ setupchecks well-known success; update-in-place from 34.0.3:1. Closes #137 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #137.
What was actually broken
Only one of the three legs #137 diagnosed. Investigated against the OS source at
start-os/v0.4.0.1and verified live on a 0.4.0.1 box:addXForwardedHeadersis already enabled. SDK 2.0.9'sknownProtocolsdefaults it on forhttpbindings, both released package versions shipped that SDK, and the live binding record carries"addXForwardedHeaders": true. The OS honors it on both the wildcard (LAN nonstandard-port) and domain vhosts.trusted_proxiesis already set and effective. The proxy dials the container from10.0.3.1, inside the modeled10.0.3.0/24; the image'smod_remoteipis inert (its conf readsX-Real-IP, which nothing sends). PHP-generated URLs are already per-request correct —/redirects tohttps://…/loginover TLS with nooverwriteprotocolset..htaccessredirects, which absolutize from Apache's connection scheme — always http behind the TLS-terminating proxy:/.well-known/carddav,/.well-known/caldav, and (missed in DAV autodiscovery broken behind the 0.4 proxy: Apache emits an http:// well-known redirect (macOS/iOS "account adds but never syncs" / "unable to verify") #137) theDavClntuser-agent redirect on/. On an https-only address the redirect target answers nothing, so Apple's Manual-mode account setup verified credentials and then dead-ended — "adds fine, never syncs."The issue's proposed part 3 (neutralize the htaccess rules, fall through to PHP) cannot work: Nextcloud registers PHP well-known handlers only for
webfinger/nodeinfo. There is no CardDAV/CalDAV handler —/index.php/.well-known/carddavreturns 404 — so removing the rules trades a wrong-scheme redirect for a hard 404.The fix
startos-dav-redirects.conf, baked into the image: scheme-aware duplicates of the three redirects in<Directory /var/www/html>underRewriteOptions InheritDownBefore, so they run ahead of the volume's stock.htaccess. With the proxy'sX-Forwarded-Proto: httpspresent they issuehttps://redirects; without it (a plain-http/onion binding) they skip and stock behavior applies. The header is trustworthy on TLS bindings because the proxy strips any client-supplied value before injecting its own; on a plain binding a client forging it only mis-redirects itself, with no cache in the path..htaccesspatch is load-bearing: the entrypoint rsyncs/usr/src/nextcloud/onto the volume only on an image version change, so a patched.htaccesswould never reach installs already on this Nextcloud version. The conf lives in the image and applies on a plain package update, no migration.mod_headerswas tried and cannot do this: a 3xx'sLocationsits inheaders_out, and on non-2xx responses only thealways(err_headers_out) directives run —Header [always] edit Locationprovably never touches it.APACHE_DISABLE_REWRITE_IP=1: pinsmod_remoteipoff. Today it is harmless by accident (wrong header name); if a future image readX-Forwarded-Forit would rewriteREMOTE_ADDRto the client IP,trusted_proxieswould stop matching, and the entire scheme/client-IP chain would silently regress.overwriteprotocolstays modeled as null, now with the rationale in a comment: scheme truth is per-request via the forwarded header, and a blankethttpswould corrupt URLs on plain-http bindings.34.0.3:1 → 34.0.3:2(:1is permanent on beta and prod).Verified on StartOS 0.4.0.1 (34.0.3:2 built and installed as an in-place update from :1)
/.well-known/{carddav,caldav}— LAN https, nonstandard port301 https://…/remote.php/dav/*.start9.dev)301 https://…/remote.php/dav//withDavClntUA, both TLS legs302 https://…/remote.php/webdav/301 http://…— unchangedPROPFIND207occ setupchecks→ WellKnownUrlsX-Forwarded-Protoon the plain portAnswers to #137's open questions:
trusted_proxiesneeds no change (10.0.3.0/24matches the proxy's bridge source, and only the proxy or sibling containers can occupy it); no keep-alive interplay with #3731 arises because the binding is untouched — the HTTP-aware proxy path was already active; onion/plain bindings keep correcthttpURLs (verified); no OS/SDK version gate is needed — on an OS that didn't inject the header the new rules simply never fire and behavior is today's.🤖 Generated with Claude Code