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

2013-12-01 Thread Antoine Pitrou
On Sun, 01 Dec 2013 02:53:32 +0100
Christian Heimes  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 "", line 1, in 
> 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 "", line 1, in 
> 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.

Regards

Antoine.
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2013-12-01 Thread Nick Coghlan
On 1 December 2013 20:37, Antoine Pitrou  wrote:
> On Sun, 01 Dec 2013 02:53:32 +0100
> Christian Heimes  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 "", line 1, in 
>> 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 "", line 1, in 
>> 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   |   [email protected]   |   Brisbane, Australia
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2013-12-01 Thread Antoine Pitrou
On Sun, 1 Dec 2013 21:33:06 +1000
Nick Coghlan  wrote:
> 
> 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:

I would prefer the check to be made when the the socket is created,
i.e. the wrap_socket() call.

Regards

Antoine.
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2013-12-01 Thread Nick Coghlan
On 1 December 2013 21:40, Antoine Pitrou  wrote:
> On Sun, 1 Dec 2013 21:33:06 +1000
> Nick Coghlan  wrote:
>>
>> 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:
>
> I would prefer the check to be made when the the socket is created,
> i.e. the wrap_socket() call.

That was my initial reaction, but then I realised it creates a
situation where the exception is raised at a point that differs from
the source of the error (the bug is in the way the context was
configured, but the exception won't be thrown until you actually try
to wrap a socket). So I now agree with Christian that it's better to
prevent the creation of the internally inconsistent SSL context in the
first place, rather than delaying the detection of the inconsistency
until the context is actually used.

I think a read-only attribute and a combined setter method is a better
way to achieve that (since it avoids the quirky "order of setting
attributes matters" behaviour), but I'm also OK with  read/write
properties that internally enforce of the state invariant.

If we decide the invariant enforcement in 3.4 is too strict, we can
change our minds and relax things in 3.5. By contrast, if we allow the
invariant to be broken in 3.4, we're locked in by backwards
compatibility concerns and can't change our minds in the future.

Cheers,
Nick.

-- 
Nick Coghlan   |   [email protected]   |   Brisbane, Australia
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2013-12-01 Thread Christian Heimes
Am 01.12.2013 12:33, schrieb Nick Coghlan:
> 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.

I don't like to add yet another API element to SSLContext. I'd rather
extend SSLContext.__init__ to take two additional parameters:

class SSLContext(_SSLContext):
def __init__(self, protocol, verify_mode=ssl.CERT_NONE,
check_hostname=False):
self.protocol = protocol
self.verify_mode = verify_mode
self.check_hostname = check_hostname


I also like to point out (again) that the requirement is only a
limitation of our API. OpenSSL is able to acquire and return the peer's
certificate with any mode. In the past somebody decided to map 'no
certificate' to None and 'no verification' to empty dict.


> 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:

You are right, I'm trying to aim for the simplest and smallest solution
possible. I'm well aware that the order of API calls is a limitation and
that it feels a bit awkward, too. In my opinion there is no way this API
can be screwed up. :) Any limitation can be lifted for 3.5 but we can't
make it more restrict in future versions.

And there is ssl.create_default_context(), too. It creates a context
with all security-related bits flipped on.

Christian


___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2013-12-01 Thread Christian Heimes
Am 30.11.2013 23:16, schrieb Guido van Rossum:
> Sounds good.
> 
> Is another change for asyncio needed?

Yes, but just a small one. The match_hostname() call in
selector_events is no longer required in 3.4.

Christian
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


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

2013-12-01 Thread Ronald Oussoren


> On 30 nov. 2013, at 19:29, Christian Heimes  wrote:
> 
> With CERT_REQUIRED OpenSSL verifies that the peer's certificate is
> directly or indirectly signed by a trusted root certification authority.
> With Python 3.4 the ssl module is able to use/load the system's trusted
> root certs on all major systems (Linux, Mac, BSD, Windows). On Linux and
> BSD it requires a properly configured system openssl to locate the root
> certs. This usually works out of the box. On Mac Apple's openssl build
> is able to use the keychain API of OSX. I have added code for Windows'
> system store.

Note that only Apple's build of OpenSSL integrates with keychain, other builds 
don't. The patch for keychain integration is on Apple's open source site but 
that isn't very helpful because that code uses a private API to do most of the 
work.   

This almost certainly means that users of fink, macports and the like cannot 
use the system keystore. 

It is probably possible to use the Keychain API to verify certificates, I 
haven't seriously looked into that yet and there is a risk of using higher 
level APIs: those tend to not like calling fork without calling execv soon 
after and that could break existing scripts. 

Ronald
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


[Python-Dev] SSL sockets and settimeout

2013-12-01 Thread Jon Ribbens
Am I correct in thinking that Python's newfangled socket.settimeout()
feature does not play well with SSL wrapped sockets? Would there be
any interest in making it so that it did?
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com