Repository navigation
ssl.wrap_socket() is incompatible with servers that drop privileges, due to keyfile requirement #48073
Description
| BPO | 3823 |
|---|---|
| Nosy | @jcea, @pitrou, @giampaolo, @anacrolix |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2010-06-29.16:18:08.137>
created_at = <Date 2008-09-09.18:25:18.562>
labels = ['type-security', 'library']
title = 'ssl.wrap_socket() is incompatible with servers that drop privileges, due to keyfile requirement'
updated_at = <Date 2010-06-29.16:18:08.136>
user = 'https://bugs.python.org/forest'bugs.python.org fields:
activity = <Date 2010-06-29.16:18:08.136>
actor = 'pitrou'
assignee = 'janssen'
closed = True
closed_date = <Date 2010-06-29.16:18:08.137>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2008-09-09.18:25:18.562>
creator = 'forest'
dependencies = []
files = []
hgrepos = []
issue_num = 3823
keywords = []
message_count = 13.0
messages = ['72891', '72905', '72955', '72976', '72977', '72982', '72983', '72984', '72986', '103242', '104938', '108901', '108916']
nosy_count = 7.0
nosy_names = ['jcea', 'janssen', 'pitrou', 'forest', 'giampaolo.rodola', 'hodgestar', 'anacrolix']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue3823'
versions = ['Python 3.2']
Activity
forest commentedon Sep 9, 2008
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.
[-]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[/+]forest commentedon Sep 9, 2008
This problem also exists in the add-on ssl module for python < 2.6:
http://pypi.python.org/pypi/ssl/
hodgestar commentedon Sep 10, 2008
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.
janssen commentedon Sep 10, 2008
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.
exarkun commentedon Sep 10, 2008
Both M2Crypto and pyOpenSSL expose certificate and key objects and have
seen lots of real-world use. Following their lead would be sensible.
janssen commentedon Sep 10, 2008
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...
exarkun commentedon Sep 10, 2008
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.
forest commentedon Sep 10, 2008
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.
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?
exarkun commentedon Sep 10, 2008
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
giampaolo commentedon Apr 15, 2010
Any update about this issue?
This should be marked as high priority, imho.
pitrou commentedon May 4, 2010
There's a patch in bpo-8550 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.
anacrolix commentedon Jun 29, 2010
I take it this is out for 2.7, will it be in 3.2?
pitrou commentedon Jun 29, 2010
The patch for bpo-8550 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.