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

Fix mod_auth_mellon failures behind SSL terminating reverse proxy server #128

Closed
wants to merge 1 commit into from

Conversation

nathanjrobertson
Copy link

@nathanjrobertson nathanjrobertson commented Aug 11, 2023

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
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