-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
TLS NPN support #1524
TLS NPN support #1524
Conversation
@PiotrSikora do you mind taking a look? :-) |
I'll test this in a bit, but at the very least, it looks that the NPN support is added only on the downstream side and not the upstream. I'm not sure if we want to have this asymmetry. |
You are absolutely right - I've only tested downstream part, since my infrastructure does not use TLS to upstreams. If you suggest some code point where it should be implemented for upstreams - I will try to fix it tomorrow. |
After some investigation I think that process should be "mirrored" in |
@crazyproger @PiotrSikora no objection from me for supporting NPN (though it's been deprecated). But we should support both server and client. Also, you will need to add tests. Testing will be a little complicated insofar as without a config option, it will be difficult to differentiate NPN from ALPN. @PiotrSikora thoughts on what to do about tests? |
@crazyproger for the client side, you need to install callback using As for the choice between ALPN & NPN - BoringSSL will ignore NPN if ALPN was also announced by the client, and client will reject connection if server incorrectly negotiated both, so we should be covered there. @mattklein123 We can't really test it without #186 (soon...), not without adding knobs to the config, which we shouldn't do. |
OK fair enough, I'm willing to let tests slide on this one once we add the client side code. |
Going to close this for now. We can reopen when ready to finish it up. |
On my production system found that clients with TLS NPN can not connect to envoy. This patch fixes that, tested on my production system.