perekipchenko / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

Support for HTTP_X_FORWARDED_ headers #586

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
When running Simplesamlphp from behind an SSL terminated load balancer, the 
apache server may be receiving requests over port 80 and non-SSL but the end 
user's connection is over port 443 and SSL.

1. End Use connects to load balancer using https://www.example.com/saml on port 
443.
2. Load Balancer terminates the SSL and proxies the request to the app server 
on port 80 in a private network.
3. SimpleSAML_Utilities::getServerHTTPS() method returns FALSE
4. SimpleSAML_Utilities::getServerPort() method returns ":80".
5. RelayState=https://www.example.com:80/saml
6. The IDP doesn't recognize the SP "https://www.example.com:80/saml" and 
returns a metadata not found error.

The load balancer sends $_SERVER['HTTP_X_FORWARDED_PROTO'] and 
$_SERVER['HTTP_X_FORWARDED_PORT'] headers.

The application has to detect these and behave accordingly.  (There is the 
added complication of trust.  Ideally you only trust the special headers if the 
request is coming from your load balancer.  This isn't always possible in a 
cloud hosted environment like EC2, where your load balancer's IP address is not 
a known constant.)

If SimpleSAML_Utilities::getServerHTTPS() and 
SimpleSAML_Utilities::getServerPort() are patched to look for the Forwarded-For 
headers, then the exchange works properly and the RelayState URL is set 
correctly.

The attached patch fixed the issue for me.

Original issue reported on code.google.com by nathan.l...@gmail.com on 29 Oct 2013 at 1:26

Attachments:

GoogleCodeExporter commented 8 years ago
Hi Nathan,

According to the SAML standard, the RelayState parameter should be used for 
preserving state information on the SP. Almost all bindings (if not all) limit 
its length to 80 bytes. That means including URLs in the RelayState is wrong 
and against the standard. But as you can see, most software (including SSP) is 
breaking the standard and using the RelayState to send a URL that's used to 
know where to return the user back after authentication. Just because it is 
convenient.

Therefore, the use we are doing might be right or wrong, but there's a 
consequence derived from the text in the standard: an IdP must never use the 
RelayState to identify the SP, for two very good reasons. One, RelayState data 
is intended for the SP, not the IdP. Two, the kind of data conveyed in 
RelayState would not be suitable to identify the SP in any way, and if it is, 
that's probably due to a misuse of the standard.

So if you have an IdP that triggers a "metadata not found" error due to an 
unrecognized RelayState, that IdP is wrong and should be fixed. What I suspect 
could be happening, on the other hand, is that the IdP is failing due to an 
unknown entityID set as the Issuer of the AuthnRequest. That's normal, and 
could happen with the default configuration of SSP, since the entityID is then 
dynamically generated.

The usual way to solve an issue like this is to properly set the 'baseurlpath' 
option to point to the external URL of the load balancer. The comment on the 
config file is pretty self explanatory:

/**
     * Setup the following parameters to match the directory of your installation.
     * See the user manual for more details.
     *
     * Valid format for baseurlpath is:
     * [(http|https)://(hostname|fqdn)[:port]]/[path/to/simplesaml/]
     * (note that it must end with a '/')
     *
     * The full url format is useful if your simpleSAMLphp setup is hosted behind
     * a reverse proxy. In that case you can specify the external url here.
     *
     * Please note that simpleSAMLphp will then redirect all queries to the
     * external url, no matter where you come from (direct access or via the
     * reverse proxy).
     */

That said, I recognize that this is a common scenario and support for 
X-Forwarded-* headers could be nice. But at the same time I'm a bit concerned 
about its implications. A simple patch as yours would solve the issue even with 
the default configuration of SSP, but there's side effects like the ones you 
are commenting, and there might be others.

There's also other ways to solve the issue that might be more convenient. 
Usually, web apps don't support this set of headers, so if you are running 
behind load balancers, all of them will have the same problem. A possible 
workaround that could also add more security to the deployment consists on 
creating a script that checks the headers and sets the global variables 
accordingly, and run it for every request by means of the auto_prepend_file.

Original comment by jaim...@gmail.com on 30 Oct 2013 at 9:34

GoogleCodeExporter commented 8 years ago

Original comment by jaim...@gmail.com on 20 Feb 2014 at 11:24