Add support for server hostname verification

[kaie]

Related to #795

[kaie]

Depends on pyca/cryptography#4492

[reaperhulk]

In the past I believe people have used service_identity.pyopenssl.verify_hostname to handle this case, but now that OpenSSL properly includes a way to validate it makes sense to potentially expose it here. Unfortunately, X509_VERIFY_PARAM_set1_host is a 1.0.2+ feature and we still support 1.0.1 so we'll have to either have a fallback through service_identity or raise an exception if this is called when cryptography is compiled against 1.0.1.

@hynek this touches a thing you actually care about, opinions?

[tiran]

@reaperhulk You also need the fallback for LibreSSL < 2.7.1.

[reaperhulk]

Ah thanks for the reminder Christian. I'll put that note over on the cryptography PR as well, sigh.

[alex]

If LibreSSL was the only think that didn't support this, I'd drop it in a heartbeat.

…

On Tue, Oct 9, 2018 at 9:53 PM Christian Heimes ***@***.***> wrote: @reaperhulk <https://github.com/reaperhulk> You also need the fallback for LibreSSL < 2.7.1. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#796 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAADBHona7phzAefWrj6gA9Cmr6ttJDYks5ujVL9gaJpZM4XUJDf> .

-- All that is necessary for evil to succeed is for good people to do nothing.

[hynek]

Obviously, supporting native APIs is the right thing to do. Pls Sherlock service_identity. ;)

[kaie]

Obviously, supporting native APIs is the right thing to do. Pls Sherlock service_identity. ;)

Can you please explain what you mean with "Sherlock service_identity"?

[kaie]

Unfortunately, X509_VERIFY_PARAM_set1_host is a 1.0.2+ feature and we still support 1.0.1 so we'll have to either have a fallback through service_identity or raise an exception if this is called when cryptography is compiled against 1.0.1.

I've submitted code that raises an exception.

[kaie]

Now that the base changes have been added to cryptography, this might be ready for review again.

Is the approach using the exception sufficient for initial support?
Do you require me to implement a fallback to service_identity?

[alex]

No, I think an exception and leaving it up to consumers is fine.

What do you think about using the IGNORE_SUBJECT flag, if it's available? That's what browsers are doing these days, and it resolves various potential issues stemming from the CommonName being untyped.

[kaie]

What do you think about using the IGNORE_SUBJECT flag, if it's available? That's what browsers are doing these days, and it resolves various potential issues stemming from the CommonName being untyped.

Is "fallback to CN" allowed if the SAN extension is absent? Is the requirement "SAN must be present" something specific to the "CA Baseline Requirements" that browser vendors are enforcing as part of the CABForum work?
If it's specific to the browser world, but CN is still allowed as a fallback for non-browser sites, maybe forbidding the fallback in general might be too strict?
Maybe we should allow some parameterizing of the new function?

[alex]

RFC 6125 encourages moving away from CN, so it's not just the CABF.

Since this is a new API, I'd like to see if we can be aggressive about encouraging modern practices.

[kaie]

OK, sounds good. Because python offers default parameters, there will be the option to introduce an optional additional parameter at a later time, if necessary.

Regarding the function name, in order to be consistent with the existing set_tlsext_host_name, maybe the new name should be set_verify_host_name.

[kaie]

I've implemented your suggestion.

[alex]

Please take a look at the failing builds here -- you can ignore the 1.1.0 build which has some unrelated fallout from 1.1.1.

[kaie]

Thanks for the explanation. I've fixed the whitespace and style issues. It's mostly green now. Two travis configurations appear to experience infrastructure/connectivity issues.

[alex]

Yeah, I'm restarting those builds. Which is annoying, but not an issue for you.

We'll try to get you a proper review of the substance soonish.

[alex]

The core code itself here looks good, modulo my comment. Needs a rebase to get tests passing and a changelog entry + a test for this.

[alex]

What do you think about integrating this into set_tlsext_host_name, so there's only a single method call required for correct/secure usage?

[pquentin]

Certainly doable, but the main difference with set_tlsext_host_name is that it should not be called on IP addresses, per RFC6066, Section 3

[tiran]

Excellent point, you have to use X509_VERIFY_PARAM_set1_ip() for IP address validation any way. Feel free to copy my implementation from Python's ssl module. The function _ssl_configure_hostname() takes care of SNI, hostname verification, and special cases IP addresses.

https://github.com/python/cpython/blob/ba251c2ae6654bfc8abd9d886b219698ad34ac3c/Modules/_ssl.c#L862-L926