Skip to content
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

add multiple domains reverse SSL proxy support #1099

Merged
merged 2 commits into from
Jan 31, 2013

Conversation

herbrechtsmeier
Copy link
Contributor

Add support for a reverse proxy that handles multiple domains via different
web roots (https://proxy.tld/domain.tld/owncloud) and only forwards SSL
connections unencrypted to the web server.

As the reverse proxy is transparent for the web server the REQUEST_URI and
SCRIPT_NAME need manual adjustments. This patch replace the direct use
of this _SERVER variables with function calls and extend this functions
to overwrite the webroot. Additionally it adds a Sabre request backend
that extends the Sabre_HTTP_Request to use the same functions. It allows
to detect the reverse proxy via regular expression for the remote IP
address and conditional overwrite the host name, protocol and web root.

This was referenced Jan 6, 2013
@karlitschek
Copy link
Contributor

👍 Looks good

@thessy
Copy link

thessy commented Jan 11, 2013

Great news! Thank you both for enabling this feature getting into core of ownlcoud! I can't await it, as I wrote the first 'bad' und 'ugly' merge request for OC3 (https://gitorious.org/owncloud/owncloud/merge_requests/74).

This feature will be in OC5? Will test it on my shared webspace...

@karlitschek
Copy link
Contributor

Can we have one more reviewer? @schiesbn @blizzz @icewind1991 @bartv2 ?

@herbrechtsmeier
Copy link
Contributor Author

I need to patch some application code and 3rdparty setup to activate the new feature. Should I already post a pull request or wait until this is merged?

Is somebody interested in a patch for owncloud stable to test the feature?

@karlitschek
Copy link
Contributor

@herbrechtsmeier O.K. I suggest to do do pull requests in other repositories too so that we can merge them all at the same time.

@LukasReschke
Copy link
Member

I need to patch some application code and 3rdparty setup to activate the new feature.

Did I understand you right that you need to patch 3rdparty code in the 3rdparty repository? - Please don't do this, this is worse. The next time someone bumps a 3rdparty library your changes are probably gone by mistake!

@karlitschek
Copy link
Contributor

@herbrechtsmeier Lukas is right. what changes are needed here?

@herbrechtsmeier
Copy link
Contributor Author

I need to patch some application code and 3rdparty setup to activate the new feature.

Did I understand you right that you need to patch 3rdparty code in the 3rdparty repository? - Please don't do this, this is worse. The next time someone bumps a 3rdparty library your changes are probably gone by mistake!

That's the reason this patch extend a Sabre class. I only patch code that already use a owncloud functions:

diff --git a/openid/phpmyid.php b/openid/phpmyid.php
index 13fd31c..6240fec 100644
--- a/openid/phpmyid.php
+++ b/openid/phpmyid.php
@@ -1626,7 +1626,7 @@ $profile['req_url'] = sprintf("%s://%s%s",
                      $proto,
                      OCP\Util::getServerHost(),
 //                   $port,//host  already includes the path
-                     $_SERVER["REQUEST_URI"]);
+                     OCP\Util::getRequestUri());

Because I haven't test the openid function I can leave them as they are to not break anything.

@herbrechtsmeier
Copy link
Contributor Author

@thessy Have you succeed to test this pull request together with owncloud-archive/apps#449?

@karlitschek How should I process with the openid changes?

@karlitschek
Copy link
Contributor

Yes. Works for me as long as no 3rd party code needs to be patched.

@herbrechtsmeier
Copy link
Contributor Author

@karlitschek Are the pull request okay for you or should I change all appearance of $_SERVER variables REQUEST_URI and SCRIPT_NAME outside of 3rd party code even if I can not test the code?

Will this feature be included in the OC5 release?

@farson2003
Copy link

YES, good question. In my and others interest as well. Also discussed here:
http://forum.owncloud.org/viewtopic.php?f=9&t=7776

Would appreciate response!
Thank you!

@karlitschek
Copy link
Contributor

@herbrechtsmeier Hmmm. Please be careful. ;-) But the changes shouldn't be that difficult so please go on.
The feature will be in ownCloud 5 if we manage to merge it before feature freeze Friday Feb. 1st

@LukasReschke
Copy link
Member

@herbrechtsmeier Could you please rebase? - THX

@farson2003
Copy link

GREAT! Looking forward to seeing this implementation as soon as possible!

@herbrechtsmeier
Copy link
Contributor Author

@LukasReschke Rebase is done

@farson2003
Copy link

@Herbrechtsmeyer:
Many users –I am sure- want to sync their data (e.g. Kontakts/Kalendar/Files) via the Proxy Server,
e.g. using CalDAVSync/CardDAV-Sync or FolderSync for Android or one of Owncloud.org Desktop Sync Clients.

Will these Apps be able to connect with their Owncloud Server via SSL Proxy, e.g. via
https://ssl-account.com/Subdomain.domain.com/owncloud/remote.php/webdav/
or
https://ssl-account.com/Subdomain.domain.com/owncloud/remote.php/carddav/
??

Or are there known limitations to the use of the Proxy Server you would think of?

@karlitschek
Copy link
Contributor

Just a brief reminder. Feature Freeze is on Friday. Do we have another reviewer?

@farson2003
Copy link

I cannot help. I am an amateur only.

@herbrechtsmeier
Copy link
Contributor Author

@farson2003 Together with my other pull request owncloud-archive/apps#449 CalDAV, CardDAV and WebDAV are available via proxy.

@farson2003
Copy link

We would love to see SSL Proxy support in OC5. Same for many other users on hosted servers, who have been waiting forever for this major imrpovement.

Is there anything that can be done for inclusion in OC5?

@LukasReschke
Copy link
Member

Is there anything that can be done for inclusion in OC5?

Convince a core dev to 👍 this ;-)

@DeepDiver1975
Copy link
Member

@owncloud-bot retest this please

@DeepDiver1975
Copy link
Member

@herbrechtsmeier rebase please - THX

@karlitschek
Copy link
Contributor

👍 from me too. Let's rebase and merge today.

Add support for a reverse proxy that handles multiple domains via different
web roots (http[s]://proxy.tld/domain.tld/owncloud).

As the reverse proxy web root is transparent for the web server the
REQUEST_URI and SCRIPT_NAME need manual adjustments. This patch replace
the direct use of this _SERVER variables with function calls and extend
this functions to overwrite the web root. Additionally it adds a Sabre
request backend that extends the Sabre_HTTP_Request to use the same
functions.
Add support for a reverse proxy that only forwards SSL connections
unencrypted to the web server.

This patch allows to detect the reverse proxy via regular expression for
the remote IP address and conditional overwrite the host name, protocol
and web root.
DeepDiver1975 added a commit that referenced this pull request Jan 31, 2013
add multiple domains reverse SSL proxy support
@DeepDiver1975 DeepDiver1975 merged commit 317261d into owncloud:master Jan 31, 2013
@farson2003
Copy link

I installed OC5. I cannot see any options to enter for SSL Proxy support, also here in the forum: http://forum.owncloud.org/viewtopic.php?f=3&t=8453

Can anyone or herbrechtmeyer comment on this?
Thank you

@herbrechtsmeier
Copy link
Contributor Author

@farson2003 The option names have the prefix overwrite. You can find them in the config.sample.php. Normally it is enough to set overwritewebroot to the complete webroot and overwritecondaddr to the regular expression for the ip addresses of you proxies as you would do it for a conditional redirect in the .htaccess file.

@herbrechtsmeier herbrechtsmeier deleted the ssl-proxy branch February 23, 2013 11:59
@farson2003
Copy link

@herbrechtsmeier
I appreciate. I am sorry but I ask you or someone else for a few details, as I am a beginner, and many others are no code experts a swell.
=>
Lets assume we want to reach our OC installation via: https://ssl-account.com/domain.tld/owncloud
The IP adress of the Proxy server shall be: 85.13.128.137 (ssl-account.com).

Which files of the OC5 Installation have to be edited, which exact code-lines must be altered to make the installation work with above configuration?

Could you give a step-by-step instrucion?
I will post your instructions with an OC5 installation manual on a AllInkl hosted server in the OC5-forum.

Thank yoU!

@karlitschek
Copy link
Contributor

@herbrechtsmeier It would be awesome if you could also document this in the new git documentation system :-)

@herbrechtsmeier
Copy link
Contributor Author

@karlitschek I have add a documentation in owncloud-archive/documentation#57
@farson2003 Please check the documentation under owncloud-archive/documentation#57 and let me know (in the other thread) if it is okay for you. Maybe it makes sense to collect the configuration for different providers also.

@farson2003
Copy link

Good I will reply in the other thread here herbrechtsmeier/documentation@fe4ac73#commitcomment-2680486

. I agree it makes sense to collect the configuration for different providers also..

@karlitschek
Copy link
Contributor

@herbrechtsmeier Thanks a lot

herbrechtsmeier added a commit to herbrechtsmeier/core that referenced this pull request Mar 9, 2013
This patch enables the use of forcessl together with a multiple domains
reverse SSL proxy (owncloud#1099) which have different hostname
and webroot for http and https access. The code assumes that the ssl
proxy (https) hostname and webroot is configured via overwritehost and
overwritewebroot.
herbrechtsmeier added a commit that referenced this pull request Jun 28, 2013
This patch enables the use of forcessl together with a multiple domains
reverse SSL proxy (#1099) which have different hostname
and webroot for http and https access. The code assumes that the ssl
proxy (https) hostname and webroot is configured via overwritehost and
overwritewebroot.
@lock lock bot locked as resolved and limited conversation to collaborators Aug 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants