tls: remove 1.0 and 1.1 from client defaults #8755
Conversation
Signed-off-by: Derek Argueta <[email protected]>
Signed-off-by: Derek Argueta <[email protected]>
There was a problem hiding this comment.
LGTM, would be nice to have @PiotrSikora provide an additional sanity check on whether we've missed anything.
| @@ -35,7 +35,8 @@ message TlsParameters { | |||
| TLSv1_3 = 4; | |||
| } | |||
|
|
|||
| // Minimum TLS protocol version. By default, it's ``TLSv1_0``. | |||
| // Minimum TLS protocol version. By default, it's ``TLSv1_2`` for clients and ``TLSv1_0`` for | |||
There was a problem hiding this comment.
FWIW, this is cool by the Envoy stable API versioning guidelines, since defaults are client specific; control planes that need stability must set explicit values.
There was a problem hiding this comment.
The change looks good, thanks! However, we might want to hold off with merging it post 1.12.0 release, so that there is some bake-in time, since it's a potentially disruptive feature.
What do you think @lizan?
/wait
|
@PiotrSikora bake-in for? isn't it just push back the disruptive part to 1.13? |
|
@lizan I meant that if we make this change now, days(?) before the release is cut, then it doesn't give enough time for it to be tested in production in various deployments by people running But making those changes right at the beginning of each release cycle was my plan for each of the changes described in #5401, so I'm obviously biased, and you might want to ignore it... It's just something to think about. |
|
I'm mildly +1 on waiting to ship this until after we snap 1.12.0 which is probably going to be Monday or Tuesday. |
|
I don't have strong opinion, so it's fine either way. My thought was people running master will see them after PR merge anyway, and people running stable will see during new release. There's no much case that master testing can help on stable side and it is easy to override by config. |
|
Can you merge master and move the release note to 1.13 and we can ship? /wait |
Signed-off-by: Derek Argueta <[email protected]>
Signed-off-by: Derek Argueta <[email protected]>
derekargueta
dismissed stale reviews from PiotrSikora, lizan, and htuch
via
d3e465d
Signed-off-by: Derek Argueta <[email protected]>
Description: Remove TLS 1.0 and 1.1 from the default client TLS versions. Users can still explicitly opt-in to 1.0 and 1.1 using
tls_minimum_protocol_version.Risk Level: Low (as #5401 explains, client-side is generally safer, but still a possible risk that it breaks someone somewhere)
Testing: updated
Docs Changes: updated
Release Notes: added
Fixes: #5395 and checks off one box for #5401
cc @PiotrSikora
Signed-off-by: Derek Argueta [email protected]