Log warning about TLS-SNI deprecation in Certbot #6468
Conversation
|
I was initially thinking of something like 20bcb11. We could instead do it in challb_to_achall where it logs each challenge we're doing, but I'm not sure if we want to log it for every challenge rather than just the first occurrence of it. What do you think? Do you prefer your approach? EDIT: After logging the message in my example though, I think we'd want to |
|
I really don't think it matters either way. |
There was a problem hiding this comment.
The reason I prefer doing things directly in Certbot is it should result in less code and it benefits all plugins. The latter is nice for 3rd party plugins (which perhaps I care too much about) as well as us because it's less error prone (e.g. the manual plugin also supports TLS-SNI-01).
If moved into Certbot directly, I'd also personally like us to stop assuming --preferred-challenges is set because it may not be for third party plugins. If we wanted to add a conditional check about this, the AuthHandler has self.pref_challs or a separate/additional warning about that could be moved directly into the parsing for the flag.
| @@ -2276,6 +2276,9 @@ def perform(self, achalls): | |||
| http_doer.add_chall(achall, i) | |||
| else: # tls-sni-01 | |||
| sni_doer.add_chall(achall, i) | |||
| logger.warning("TLS-SNI-01 is deprecated, and will stop working in February 2019. " | |||
There was a problem hiding this comment.
I don't have a strong preference, but I'd personally like us to remove the date here as things like that can change but a released version of Certbot cannot.
|
I'm seeing that we do check if |
There was a problem hiding this comment.
LGTM!
I'm seeing that we do check if pref_challs is set before using it, where are you talking about?
I just meant that if the warning message is in Certbot's core, it probably shouldn't be mentioning --preferred-challenges without first checking that the flag is set because some third party plugins may still be preferring TLS-SNI so their users don't have to set the flag for the challenge to be used.
I think you made the right call here though as adding this check doesn't seem worth the little value it adds.
Be sure to edit the
mastersection ofCHANGELOG.mdwith a line describing this PR before it gets merged.For #6319.