Skip to content

Fix mod_auth_mellon failures behind SSL terminating reverse proxy server#128

Closed
nathanjrobertson wants to merge 1 commit into
latchset:mainfrom
nathanjrobertson:force_https
Closed

Fix mod_auth_mellon failures behind SSL terminating reverse proxy server#128
nathanjrobertson wants to merge 1 commit into
latchset:mainfrom
nathanjrobertson:force_https

Conversation

@nathanjrobertson

@nathanjrobertson nathanjrobertson commented Aug 11, 2023

Copy link
Copy Markdown

This PR aims to solve the same problem posed in PR #33, where the Apache server that mod_auth_mellon is running on is behind a reverse proxy that terminates the SSL connection (meaning it's plain HTTP between the reverse proxy and the server mod_auth_mellon is running on). This causes Apache to believe it is running plain HTTP, hence mod_auth_mellon (using Apache APIs) generates URLs for things like ReturnTo and RelayState set with the wrong protocol / scheme.

The main objection to PR #33 was the security considerations of relying on an HTTP header to determine whether the server should alter this or not. The patch @smartalock put forward relies on the reverse proxy injecting a header in the request, which works, but perhaps isn't the best way.

Instead, this PR adds the MellonForceHttpsUrlRewrites configuration directive. This makes the behaviour a configuration file element, set by the admin, fixing the security issue above.

This works for cases where you can't set ServerName (ie. it has several reverse proxies in front of it). Edge case, I know. But in SimpleSAMLphp land (which I know @thijskh is familiar with) we've done this by setting the baseUrl dynamically in config.php (in PHP code) to cover up for this case, which of course you can't do here (ServerName can't be set per-request). So, this PR would be the next best thing.

…ion of https URLs when you're behind a reverse proxy that terminates the SSL connection.
@nathanjrobertson

Copy link
Copy Markdown
Author

Actually, I just figured out another way to solve my particular problem, and I think most other cases will be able to use the solution suggested in the comment in PR #33 to use the ServerName directive. I'll close this PR - sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant