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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we set
RACK_URL_SCHEME
anywhere else or did I just completely miss it?Also, is this checked by rack linter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RACK_URL_SCHEME
defaults torequest.scheme
, however that's not accurate in cases where we're listening on HTTP but have SSL terminated upstream.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user sent the
X-Forwarded-Proto: https
could that break the application that's running onhttp
? Just wondering if there are security implications to this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context, here's how puma sets
RACK_URL_SCHEME
:Here's
default_server_port
:In theory if the application was exposed directly, you could bypass middleware to force ssl by sending
X-Forwarded-Proto: https
. Whether or not that has security implications I don't know, since it could only affect the current user's session.I'm not certain on this, but I would think it's up to nginx, haproxy, or whatever is used in front of the application to set or reject these headers. Here's how Heroku handles it:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ioquatix the purpose of the scheme check is to protect the user from sending information over unencrypted connections. If the user provides the
X-Forwarded-Proto: https
it will only impact them. I don't think this is a security concern.