classification
Title: ssl.wrap_socket() is incompatible with servers that drop privileges, due to keyfile requirement
Type: security Stage:
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: janssen Nosy List: anacrolix, forest, giampaolo.rodola, hodgestar, janssen, jcea, pitrou
Priority: normal Keywords:

Created on 2008-09-09 18:25 by forest, last changed 2010-06-29 16:18 by pitrou. This issue is now closed.

Messages (13)
msg72891 - (view) Author: Forest (forest) Date: 2008-09-09 18:25
SSLSocket() and ssl.wrap_socket() accept private keys only as paths to
their location on the file system.  This means that a server can only
support SSL if it has read access to its private key file at the time
when client connections arrive, which is a problem for servers that bind
to their socket and drop privileges as soon as they start up.

In other words, the new ssl module's API prevents its use in servers
that follow best practices that are prevalent in the unix world.

If SSLSocket() and ssl.wrap_socket() were updated to accept private keys
as strings (or open file-like objects or some such), the problem would
go away.  Moreover, it would allow a program to keep private keys cached
in memory, which might slightly improve performance over reading them
from the file system on every new connection.
msg72905 - (view) Author: Forest (forest) Date: 2008-09-09 20:29
This problem also exists in the add-on ssl module for python < 2.6:
http://pypi.python.org/pypi/ssl/
msg72955 - (view) Author: Simon Cross (hodgestar) Date: 2008-09-10 11:34
I've dug around in the code a bit and the keyfile, certfile and ca_certs
filename arguments to SSLSocket.__init__ are passed down into
newPySSLObject in _ssl.c and from there directly to SSL_CTX_* function
from OpenSSL so making these arguments allow file-like objects is going
to be non-trivial.

The options I see are:

* Write the file-like objects out to named temporary files and pass
those through to OpenSSL (seems like a nasty hack and prone to all sorts
of problems).

* Change the which OpenSSL functions are used to setup the certificate
(I definitely don't think this could go into 2.6 or 3.0 at this stage;
also see analysis of current OpenSSL usage below for more difficulties)

* Add an SSL CTX wrapper object and allow that to be passed down to
newPySSLObject instead of the filenames. Then the CTX object could be
created before dropping privileges (I think this is probably also too
big a change to be considered for 2.6 or 3.0 at this point, but it's
what looks best to me at the moment).

The current situation in _ssl.c:

* keyfile is loaded using SSL_CTX_use_PrivateKey_file(...) which loads
the first certificate from keyfile into ctx. We could replace this with
SSL_CTX_use_PrivateKey(SSL_CTX *ctx, EVP_PKEY *pkey) but we'd have to
load the key ourselves and make sure to follow what OpenSSL does to
maintain compatibility.

* certfile is loaded with SSL_CTX_use_certificate_chain_file(...) which
reads in all the certificates from certfile into ctx. We could read the
certificates in ourselves and them load them one by one using
SSL_CTX_use_certificate(...) and then SSL_CTX_add_extra_chain_cert(...).

* ca_certs is loaded using SSL_CTX_load_verify_locations(...). As fasr
as I can see there is no convenient replacement function for this in
OpenSSL. SSL_CTX_set_client_CA_list(...) will load a list of certificate
names but doesn't load the certificates themselves (so verification
won't be done with them) and SSL_CTX_add_client_CA(...) has the same
issue.      

One could use SSL_CTX_set_cert_store(...) to register callbacks (and
then presumably one can do whatever one wants and can get around the
ca_certs issue) but the man page for SSL_CTX_set_cert_store has the
rather disheartening "Currently no detailed documentation on how to use
the X509_STORE object is available."

All this comes with the proviso that I just started digging into the
OpenSSL manpages today so I'm a long way from being an expert. :)

I can probably find time to create a patch with tests once we have a
clear direction to go in.

@Forest: If you have an details on how non-Python servers go about
loading certificates and then dropping privileges using OpenSSL, that
would be extremely useful.
msg72976 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-09-10 16:47
Thanks, Simon.

I remember digging through all this last year, and finally deciding
to keep things simple and use the strategy the current codebase uses.

It almost sounds like we'd need to create Key and Certificate objects
in the _ssl module, which could be used to load up all the keys and/or
certificates the server uses, before it changes UID (and presumbably
loses access to the files the data is kept in).  I was resisting
going down that path; there's a lot of complexity there I want to avoid.
But much of the mechanism of a Certificate object is already there;
perhaps adding an opaque Key object wouldn't be too bad.
msg72977 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2008-09-10 16:54
Both M2Crypto and pyOpenSSL expose certificate and key objects and have
seen lots of real-world use.  Following their lead would be sensible.
msg72982 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2008-09-10 20:36
Sure -- but the point of the SSL module was to do something
small and compatible with the existing socket.ssl module; I really don't
want to get into a full-fledged Python interface to OpenSSL.  Perhaps
incremental progress would be OK...
msg72983 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2008-09-10 20:47
I'm just suggesting that if the ssl module *is* going to gain
certificate and key objects, looking at existing APIs and perhaps
emulating them, to the extent that they provide functionality which the
ssl module is also going to provide, is a good idea.  Perhaps we could
actually get the various APIs to begin to converge (where they overlap)
instead of producing further divergence.
msg72984 - (view) Author: Forest (forest) Date: 2008-09-10 21:02
Simon:  I wish I could offer guidance here, but I'm afraid that I too am
reading some of these openssl man pages for the first time.

I agree that writing to a temporary file would be bad.

Accepting file-like objects from python code would be nice, but isn't
really necessary.  Simple strings would suffice.  It's easy enough for
application code to read the strings from the appropriate files.  Of
course, the ssl module (or an underlying library) would need a way to
determine the data format within the strings.  Unfortunately, I didn't
notice an equivalent of SSL_CTX_use_PrivateKey_file() that accepts a
file's contents instead of its path.  SSL_CTX_use_RSAPrivateKey() looks
like the next best thing.

>much of the mechanism of a Certificate object is already there;
>perhaps adding an opaque Key object wouldn't be too bad."

That's encouraging.

From the openssl docs I've read so far, it looks like certificates and
keys have several formats and use patterns.  That seems to me like
another argument in favor of supporting separate Certificate and Key
objects, even if they're only minimally implemented to begin with.  In
other words, I don't imagine the presence of Certificate and Key objects
would muck up the ssl module's api.

In order to keep compatibility with socket.ssl, perhaps any new
certificate and key objects should be accepted as alternatives to the
existing certfile and keyfile args, but the latter should still be allowed?
msg72986 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2008-09-10 21:08
You can load a private key from a string by creating a memory BIO and
using PEM_read_bio_PrivateKey or d2i_PrivateKey_bio.

This is how pyOpenSSL implements its load_privatekey API.  You can see
the code here:

http://bazaar.launchpad.net/~exarkun/pyopenssl/trunk/annotate/70?file_id=crypto.c-20080219014912-qyb7kjf196jhzlyv-128
msg103242 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-04-15 18:16
Any update about this issue?
This should be marked as high priority, imho.
msg104938 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-04 14:07
There's a patch in issue8550 to expose SSL contexts as first-class objects. It allows you to create first your context object(s) and load certificates, then drop privileges, then create sockets using this/these contexts.
In any case, resolution can't happen before 3.2. 2.7 is in feature freeze now.
msg108901 - (view) Author: Matt Joiner (anacrolix) Date: 2010-06-29 11:09
I take it this is out for 2.7, will it be in 3.2?
msg108916 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-06-29 16:18
> I take it this is out for 2.7, will it be in 3.2?

The patch for issue8550 has been committed. The SSLContext API should solve this issue:
http://docs.python.org/dev/py3k/library/ssl.html#ssl.SSLContext.wrap_socket

If it doesn't, please explain why.
History
Date User Action Args
2010-06-29 16:18:08pitrousetstatus: open -> closed
resolution: accepted -> fixed
messages: + msg108916
2010-06-29 11:09:04anacrolixsetnosy: + anacrolix
messages: + msg108901
2010-05-04 14:07:44pitrousetnosy: + pitrou

messages: + msg104938
versions: + Python 3.2, - Python 2.6
2010-04-15 18:16:28giampaolo.rodolasetmessages: + msg103242
2009-01-29 15:01:17exarkunsetnosy: - exarkun
2009-01-29 14:51:27jceasetnosy: + jcea
2009-01-07 04:06:09giampaolo.rodolasetnosy: + giampaolo.rodola
2008-09-10 21:08:10exarkunsetmessages: + msg72986
2008-09-10 21:02:37forestsetmessages: + msg72984
2008-09-10 20:47:07exarkunsetmessages: + msg72983
2008-09-10 20:36:57janssensetmessages: + msg72982
2008-09-10 16:54:31exarkunsetnosy: + exarkun
messages: + msg72977
2008-09-10 16:47:18janssensetmessages: + msg72976
2008-09-10 11:34:18hodgestarsetnosy: + hodgestar
messages: + msg72955
2008-09-10 01:32:12janssensetresolution: accepted
2008-09-09 20:57:06benjamin.petersonsetassignee: janssen
2008-09-09 20:29:16forestsetmessages: + msg72905
2008-09-09 20:23:59forestsetnosy: + janssen
2008-09-09 18:48:42forestsettitle: ssl.wrap_socket() is incompatible with unprivileged servers, due to keyfile requirement -> ssl.wrap_socket() is incompatible with servers that drop privileges, due to keyfile requirement
2008-09-09 18:25:18forestcreate