[Python-Dev] Verification of SSL cert and hostname made easy

[Nick Coghlan]

On 1 December 2013 20:37, Antoine Pitrou <solipsis at pitrou.net> wrote:
> On Sun, 01 Dec 2013 02:53:32 +0100
> Christian Heimes <christian at python.org> wrote:
>> Am 30.11.2013 23:51, schrieb Antoine Pitrou:
>> > Small nit: what happens if the server_hostname is None (i.e. wasn't
>> > passed to context.wrap_socket())?
>>
>> The code will raise an exception. My patch already implements a more
>> verbose ValueError that explains the cause of the problem. It's flaw in
>> code, that calls context.wrap_socket. Erroneous code will no longer pass
>> silently.
>>
>> The patch also ensures a valid combination of verify_mode and
>> check_hostname:
>>
>> >>> context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
>> >>> context.check_hostname = True
>> Traceback (most recent call last):
>>   File "<stdin>", line 1, in <module>
>> ValueError: check_hostname needs a SSL context with either CERT_OPTIONAL
>> or CERT_REQUIRED
>> >>> context.verify_mode = ssl.CERT_REQUIRED
>> >>> context.check_hostname = True
>> >>> context.verify_mode = ssl.CERT_NONE
>> Traceback (most recent call last):
>>   File "<stdin>", line 1, in <module>
>> ValueError: Cannot set verify_mode to CERT_NONE when check_hostname is
>> enabled.
>
> So I have to set attributes in a given order? I find this silly.

Perhaps a cleaner option would be to make check_hostname read only,
and add a secure-by-default method that allows all verification
related settings to be adjusted at once:

    def set_verify_mode(mode=ssl.CERT_REQUIRED, check_hostname=True):
        ...

That way the consistency check would be directly on the method
arguments, rather than involving the current context state. Any future
settings that required a verified cert could then use a similar model.

If we don't do that, then I think Christian's approach is a reasonable
compromise given the late stage of the release cycle - it ensures the
context can't get into the inconsistent verify_mode=CERT_NONE and
check_hostname=True state, and leaves our options completely open for
3.5:

- add a "configure all security related settings at once"
set_verify_mode method as suggested above
- allow the context to get into an inconsistent state, check it in
wrap_socket (bad due to point of exception != point of error)
- have setting check_hostname potentially affect verify_mode and
vice-versa (bad due to being implicit magic)

We wouldn't be able to make check_hostname read-only the way we would
if we added the configuration method now, but that doesn't bother me
that much as long as the setting consistency invariant is enforced.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia