Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Enable the proxy_ssl_server_name option #32

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

ankon
Copy link
Contributor

@ankon ankon commented Mar 24, 2016

This fixes #31

Note that this is currently on top of #30, but that isn't required.

@ankon ankon mentioned this pull request Mar 24, 2016
@hone
Copy link
Member

hone commented Mar 24, 2016

Thanks for the patch. the proxy stuff definitely needs some more work/cleanup. Can you split this from #30? Also, is there cases where you wouldn't want this option enabled?

@ankon
Copy link
Contributor Author

ankon commented Mar 24, 2016

As far as I understand the option: it would now send some additional data in the first request, which the origin can use to select the correct certificate, or ignore it.

Re splitting: sure, will rebase this onto master then.

@jmervine
Copy link

👍 for making it on by default... perhaps support disabling via config?

@hone
Copy link
Member

hone commented Jun 10, 2016

@jmervine @ankon are there cases where you wouldn't want this on?

@jmervine
Copy link

@hone I can't specifically think of one, but I feel like Nginx would make it on by default if there weren't some obvious reason not to. If nothing else, performance perhaps, when it's not needed, but that's probably marginal.

@jmervine
Copy link

@rdsharma

@wcurtis
Copy link

wcurtis commented Aug 14, 2016

Bump - any timeline on getting this in? We could sure use it.

@ojacobson
Copy link
Contributor

As I understand it, this PR enables the presentation of SNI information when Nginx connects to proxy backends. This is a good thing, in 2016: SNI is widespread. So long as the hostname included in the presented SNI handshake comes from the proxy backend URL, and not from the client's Host: header - that is, so long as backends perceive requests as being meant for the host in the backend URL, and not the host of the proxy itself - then this is the right fix.

I believe that's exactly what the named option does. Merging.

@ojacobson ojacobson merged commit f13104e into heroku:master Aug 24, 2016
@ankon ankon deleted the fix/sni-support branch August 25, 2016 08:40
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.

Cannot proxy to servers that use HTTPS with SNI
5 participants