tls: allow renegotiation when acting as a client. #3551
Conversation
*Risk Level*: Low *Testing*: Manual (BoringSSL doesn't support renegotiation when acting as a server) *Docs Changes*: Added *Release Notes*: Added Signed-off-by: Piotr Sikora <piotrsikora@google.com>
|
cc @ggreenway |
|
What's the motivation here? Renegotiation has serious security consequences, makes the I/O pattern weird, is incompatible with HTTP/2, and blocks a BoringSSL optimization to make TLS connections use less memory ( |
|
@davidben I'm well aware of that, and like you, I'd prefer to avoid renegotiation, but there were few reports (both on Envoy and Istio mailing lists) of people being unable to connect to external services due to Note: This change keeps renegotiation off by default,, so I don't expect people to use it unless they hit the aforementioned issue. |
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
|
LGTM. I hate renegotiation, but I will begrudgingly concede that sometimes you might need a toggle for it. :-) |
That's my feeling as well. |
There was a problem hiding this comment.
@PiotrSikora can we get a test for this somehow? Not sure how hard that would be... At the very least can we have a test that at least turns on this flag to make sure it doesn't do anything obviously bad?
…renegotiate Signed-off-by: Piotr Sikora <piotrsikora@google.com>
|
@mattklein123 yeah, I'll add tests with this flag turned on shortly, to make sure it doesn't break other stuff, but I don't think we can properly test renegotiation without pulling another SSL library, which might be an overkill... I'll eventually fix this with #186. |
Yup, agreed. sgtm. |
…tion. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Risk Level: Low
Testing: Manual (BoringSSL doesn't support renegotiation when acting as a server)
Docs Changes: Added
Release Notes: Added
Signed-off-by: Piotr Sikora piotrsikora@google.com