Re: [Python-Dev] Let's make the SSL module sane

[Antoine Pitrou]

On Sat, 10 Sep 2016 20:23:13 +0200
Christian Heimes  wrote:
> 
> It's a bit too clever and tricky for my taste. I prefer 'explicit is
> better than implicit' for trust anchors. My main concern are secure
> default settings. A SSLContext should be secure w/o further settings in
> order to prevent developers to shoot themselves in the knee.
> 
> Missing root certs are not a direct security issue with CERT_REQUIRED.
> The connection will simply fail. I'd rather improve the error message
> than to auto-load certs.

Agreed with all this.  You don't want to have "magic" behaviour in a
security-oriented module.  Let people configure their contexts
explicitly.

As a reminder, people who don't want to configure TLS themselves should
use an intermediate layer instead, such as ssl.create_default_context()
or an application protocol implementation (httplib, etc.).

Regards

Antoine.


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

Re: [Python-Dev] Let's make the SSL module sane

[Antoine Pitrou]

On Sat, 10 Sep 2016 16:22:57 +0200
Christian Heimes  wrote:
> 
> For 3.6 I like to make the SSL more sane and more secure by default.
> Yes, I'm a bit late but all my proposals are implemented, documented,
> partly tested and  existing tests are passing.

I don't have time nor motivation to review most of them, but I trust
you that the implementations are sane :-)

> First I like to deprecated some old APIs and favor of SSLCotext.

This has always been the plan (to me), so a big +1.

> The patch
> also deprecates certfile, keyfile an similar arguments in network
> protocol libraries.

+1.

> I also considered to make cert validation enabled by default for all
> protocol in 3.6, Victor has rising some concerns.

I assume you mean "in client mode". I think that sounds fine nowadays.
If people haven't configured a set of trusted CAs properly, this should
error out immediately, so they would notice it quickly IMHO.

(in other words, +0.5)

> How about we change
> the behavior in 3.7 and just add a warning to 3.6?

As you (or others) prefer :-)

> Next up SSLContext default configuration. A bare SSLContext comes with
> insecure default settings. I'd like to make SSLContext(PROTOCOL_SSLv23)
> secure bu default. Changelog: The context is created with more secure
> default values. The options OP_NO_COMPRESSION,
> OP_CIPHER_SERVER_PREFERENCE, OP_SINGLE_DH_USE, OP_SINGLE_ECDH_USE,
> OP_NO_SSLv2 (except for PROTOCOL_SSLv2), and OP_NO_SSLv3 (except for
> PROTOCOL_SSLv3) are set by default.
> The initial cipher suite list
> contains only HIGH ciphers, no NULL ciphers and MD5 ciphers (except for
> PROTOCOL_SSLv2).

+1 to all this from me.  The ship has sailed on most of this stuff
already.

> Finally (and this is the biggest) I like to change how the protocols
> work. OpenSSL 1.1.0 has deprecated all version specific protocols. Soon
> OpenSSL will only support auto-negotiation (formerly known as
> PROTOCOL_SSLv23). My patch #26470 added PROTOCOL_TLS as alias for
> PROTOCOL_SSLv23. If the last idea is accepted I will remove PROTOCOL_TLS
> again. It hasn't been released yet. Instead I'm going to add
> PROTOCOL_TLS_CLIENT and PROTOCOL_TLS_SERVER (see
> https://www.openssl.org/docs/manmaster/ssl/SSL_CTX_new.html
> TLS_server_method(), TLS_client_method()). PROTOCOL_TLS_CLIENT is like
> PROTOCOL_SSLv23 but only supports client-side sockets and
> PROTOCOL_TLS_SERVER just server-side sockets. In my experience we can't
> have a SSLContext with sensible and secure settings for client and
> server at the same time. Hostname checking and cert validation is only
> sensible for client-side sockets.

This sounds reasonable.  No strong opinion from me but +0.5 as well.

> Starting in 3.8 (or 3.7?) there will be only PROTOCOL_TLS_CLIENT and
> PROTOCOL_TLS_SERVER.

You *may* provide the old constants for compatibility, though (meaning
"PROTOCOL_TLS", roughly).

> How will my proposals change TLS/SSL code?
> 
> Application must create a SSLContext object. Applications are
> recommended to keep the context around to benefit from session reusage
> and reduce overload of cert parsing.

(well, most applications are advised to use an intermediate layer such
as httplib ;-))

> I hope this mail makes sense.

It does to me!

Regards

Antoine.


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

Re: [Python-Dev] Let's make the SSL module sane

[Nick Coghlan]

On 11 September 2016 at 05:20, Christian Heimes  wrote:
> On 2016-09-10 17:24, Nick Coghlan wrote:
>> On 11 September 2016 at 00:22, Christian Heimes  wrote:
>>> First I like to deprecated some old APIs and favor of SSLCotext. We have
>>> multiple ways to create a SSL socket or to configure libraries like
>>> urllib. The general idea is to make SSLContext the central object for
>>> TLS/SSL configuration. My patch deprecates ssl.wrap_socket()
>>
>> I'll bring over my question from the tracker issue to here: there's a
>> subset of ssl.wrap_socket() arguments which actually make sense as
>> arguments to ssl.get_default_context().wrap_socket().
>>
>> Accordingly, we can pick a subset of code (e.g. SSL/TLS clients) that
>> we bless with not needing to change, leaving only code using
>> deprecated parameters or creating server sockets that needs to be
>> updated.
>
> Do you consider ssl.wrap_socket() relevant for so many projects? The
> function hurts performance and is no longer best practice. The
> deprecation of ssl.wrap_socket() is a friendly nudge. I don't mind to
> keep it around for another four or six years.

I have no problem with ripping out and replacing the internals of
ssl.wrap_socket(), and doing whatever is needed to improve its
performance. What I'm mainly looking for is a decision tree in the
overall API design that minimises the amount of fresh information a
developer needs to supply, and that makes the purpose of their code
relatively self-evident to someone that is reading low(ish) level
Python SSL/TLS code for the first time.

For example, I think this would be a desirably simple design from a
usage perspective:

# Client sockets as default, settings may change in maintenance releases
my_context = ssl.get_default_context()
my_tls_socket = ssl.wrap_socket(my_uncovered_socket)

# Server sockets by request, settings may change in maintenance releases
my_context = ssl.get_default_server_context()
my_tls_socket = ssl.wrap_server_socket(my_uncovered_socket)

# More control with more responsibility, defaults only change in
feature releases
my_context = ssl.SSLContext()
my_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)

With that approach, an API user only has to make two forced decisions:

- am I securing a client connection or a server connection?
- do I want to implicitly pick up modernised defaults in maintenance releases?

And we can make the second one a non-decision in most cases by
presenting the higher level convenience API as the preferred approach.

There would be a third hidden decision implied by the convenience APIs
(using the default system certificate store rather than loading a
custom one), but most users wouldn't need to worry about that.

Cheers,
Nick.

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

Re: [Python-Dev] Let's make the SSL module sane

[Christian Heimes]

On 2016-09-10 17:24, Nick Coghlan wrote:
> On 11 September 2016 at 00:22, Christian Heimes  wrote:
>> First I like to deprecated some old APIs and favor of SSLCotext. We have
>> multiple ways to create a SSL socket or to configure libraries like
>> urllib. The general idea is to make SSLContext the central object for
>> TLS/SSL configuration. My patch deprecates ssl.wrap_socket()
> 
> I'll bring over my question from the tracker issue to here: there's a
> subset of ssl.wrap_socket() arguments which actually make sense as
> arguments to ssl.get_default_context().wrap_socket().
> 
> Accordingly, we can pick a subset of code (e.g. SSL/TLS clients) that
> we bless with not needing to change, leaving only code using
> deprecated parameters or creating server sockets that needs to be
> updated.

Do you consider ssl.wrap_socket() relevant for so many projects? The
function hurts performance and is no longer best practice. The
deprecation of ssl.wrap_socket() is a friendly nudge. I don't mind to
keep it around for another four or six years.

There is one other use case not covered by SSLContext.wrap_socket() but
by SSLSocket.__init__(). The SSLSocket constructor takes a fileno
argument. But it's an undocumented feature and it's broken since at
least 3.3. https://bugs.python.org/issue27629


> As with past network security changes, a major factor we need to
> account for is that no matter how valuable a particular goal is from a
> broader industry perspective, people don't tend to react to API breaks
> by fixing their code - they react by not upgrading at all.

I totally agree and have been verify careful to keep backwards
compatibility. My third patch breaks just one scenario:
ssl.create_default_context(purpose=Purpose.SERVER_AUTH) no longer
supports server-side connections and CLIENT_AUTH no longer client-side
connections. It's the good kind of incompatibility because it reveals
API misuse. Application should never have used SERVER_AUTH context to
create server sockets.



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

Re: [Python-Dev] Let's make the SSL module sane

[Christian Heimes]

On 2016-09-10 18:24, Donald Stufft wrote:
> 
>> On Sep 10, 2016, at 10:22 AM, Christian Heimes  wrote:
>>
>> I don't load any certs because it is not possible to remove a cert or
>> X509 lookup once it is loaded. create_default_context() just have to
>> load the certs and set more secure ciper suites.
> 
> 
> This part is the most concerning to me, though I understand why it’s the 
> case. Perhaps we can do something a little tricky to allow both things to 
> happen? IOW do sort of a late binding of a call to loading the default 
> certificates if no other certificates has been loaded when the call to 
> SSLContext().wrap_socket() has been made.
> 
> So we’d do something like:
> 
> 
> class SSLContext:
> def __init__(self, …):
> self._loaded_certificates = False
> …  # Do Other Stuff
> 
> def load_default_certs(self, …):
> self._loaded_certificates = True
> …  # Do Other Stuff
> 
> def load_verify_locations(self, …):
> self._loaded_certificates = True
> …  # Do Other Stuff
> 
> def wrap_socket(self, …):
> if not self._loaded_certificates:
> self.load_default_certs()
> 
> …  # Do Other Stuff
> 
> 
> That way if someone does something like:
> 
> ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
> ctx.load_verify_locations(cafile=“…”)
> ctx.wrap_socket(…)
> 
> Then they don’t get any default certificates added, HOWEVER if they do:
> 
> ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
> ctx.wrap_socket(…)
> 
> Then they do.
> 
> The main draw back I can see with this is that you can’t wrap a socket and 
> then add certificates after the fact… but I don’t even know if that makes 
> sense to do?

It's a bit too clever and tricky for my taste. I prefer 'explicit is
better than implicit' for trust anchors. My main concern are secure
default settings. A SSLContext should be secure w/o further settings in
order to prevent developers to shoot themselves in the knee.

Missing root certs are not a direct security issue with CERT_REQUIRED.
The connection will simply fail. I'd rather improve the error message
than to auto-load certs.

Christian

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

Re: [Python-Dev] Let's make the SSL module sane

[Donald Stufft]

> On Sep 10, 2016, at 10:22 AM, Christian Heimes  wrote:
> 
> I don't load any certs because it is not possible to remove a cert or
> X509 lookup once it is loaded. create_default_context() just have to
> load the certs and set more secure ciper suites.


This part is the most concerning to me, though I understand why it’s the case. 
Perhaps we can do something a little tricky to allow both things to happen? IOW 
do sort of a late binding of a call to loading the default certificates if no 
other certificates has been loaded when the call to SSLContext().wrap_socket() 
has been made.

So we’d do something like:


class SSLContext:
def __init__(self, …):
self._loaded_certificates = False
…  # Do Other Stuff

def load_default_certs(self, …):
self._loaded_certificates = True
…  # Do Other Stuff

def load_verify_locations(self, …):
self._loaded_certificates = True
…  # Do Other Stuff

def wrap_socket(self, …):
if not self._loaded_certificates:
self.load_default_certs()

…  # Do Other Stuff


That way if someone does something like:

ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
ctx.load_verify_locations(cafile=“…”)
ctx.wrap_socket(…)

Then they don’t get any default certificates added, HOWEVER if they do:

ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
ctx.wrap_socket(…)

Then they do.

The main draw back I can see with this is that you can’t wrap a socket and then 
add certificates after the fact… but I don’t even know if that makes sense to 
do?

—
Donald Stufft



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

Re: [Python-Dev] Let's make the SSL module sane

[Nick Coghlan]

On 11 September 2016 at 00:22, Christian Heimes  wrote:
> First I like to deprecated some old APIs and favor of SSLCotext. We have
> multiple ways to create a SSL socket or to configure libraries like
> urllib. The general idea is to make SSLContext the central object for
> TLS/SSL configuration. My patch deprecates ssl.wrap_socket()

I'll bring over my question from the tracker issue to here: there's a
subset of ssl.wrap_socket() arguments which actually make sense as
arguments to ssl.get_default_context().wrap_socket().

Accordingly, we can pick a subset of code (e.g. SSL/TLS clients) that
we bless with not needing to change, leaving only code using
deprecated parameters or creating server sockets that needs to be
updated.

As with past network security changes, a major factor we need to
account for is that no matter how valuable a particular goal is from a
broader industry perspective, people don't tend to react to API breaks
by fixing their code - they react by not upgrading at all.

Regards,
Nick.

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