SSLContext.load_cert_chain() should accept a password argument

[simpkins]

BPO	12803
Nosy	@pitrou, @giampaolo, @merwok
Files	ssl-password.patch: Patch to add a password argument to load_cert_chain() ssl-password.2.patch: Updated patch to accept any callable ssl-password.3.patch: Updated patch to fix missing decref ssl-password.4.patch: Patch with updated documentation

^{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 2011-08-25.12:46:34.543>
created_at = <Date 2011-08-21.07:28:26.306>
labels = ['type-feature', 'library']
title = 'SSLContext.load_cert_chain() should accept a password argument'
updated_at = <Date 2011-08-25.12:46:34.542>
user = 'https://bugs.python.org/simpkins'

bugs.python.org fields:

activity = <Date 2011-08-25.12:46:34.542>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2011-08-25.12:46:34.543>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2011-08-21.07:28:26.306>
creator = 'simpkins'
dependencies = []
files = ['22971', '22975', '23029', '23043']
hgrepos = []
issue_num = 12803
keywords = ['patch']
message_count = 11.0
messages = ['142595', '142611', '142637', '142640', '142884', '142893', '142924', '142955', '142956', '142967', '142968']
nosy_count = 6.0
nosy_names = ['janssen', 'pitrou', 'giampaolo.rodola', 'eric.araujo', 'python-dev', 'simpkins']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12803'
versions = ['Python 3.3']

[simpkins]

The SSLContext.load_cert_chain() method should accept a password argument to use if the private key is encrypted. Currently it always uses OpenSSL's default password callback, which prompts the user interactively for a password.

I've attached a patch that adds an extra password argument, which can be either a string, bytes, bytearray, or a function to call to get the password.

[merwok]

I've attached a patch that adds an extra password argument, which can be
either a string, bytes, bytearray, or a function to call to get the password.

It seems a bit strange to me to accept string types or callable in the same argument. If it just supported strings, people could still write password=somefunction(), right?

[simpkins]

It seems a bit strange to me to accept string types or callable in the
same argument. If it just supported strings, people could still write
password=somefunction(), right?

The function is only called if the private key is encrypted and a
password is necessary. This allows the code to only prompt for a
password if it is actually needed.

This also parallels the OpenSSL C API, which only accepts a function to
get the password. I added support for plain strings as a convenience.
The string argument will be ignored if the file is not encrypted.

[simpkins]

Here's a new patch that accepts any callable. The old patch only accepted
actual function objects.

[pitrou]

Thanks for the patch. This is a generally useful functionality and the patch looks mostly good.
I have a couple comments:

• in _pwinfo_set(), you need to decref password_bytes when you're finished
• you check the password size in _password_callback() but not in _pwinfo_set(), is it expected? is the size limit only meaningful with a callback rather than a predefined password?

[simpkins]

Good catch. Here's an updated patch to fix the missing decref in
_pwinfo_set()

The length check in _password_callback() applies to both callback
functions and predefined strings. The C API always uses a callback,
so _password_callback() is used even when the python caller has passed
in a string. We can't check the length in _pwinfo_set() since it depends
on the buffer length argument that openssl will pass to
_password_callback().

[pitrou]

I have one last concern: what is the character set of an OpenSSL password? I see you are using PyUnicode_AsEncodedString(x, NULL, NULL), which basically returns a utf8-encoded bytestring. Since the OpenSSL doc don't specify anything, we could accept it as a best-effort thing.
Or perhaps we should use PyUnicode_EncodeFSDefault(), which uses the "filesystem encoding", which is usually the same as the current terminal (assuming the password is typed on a terminal).

[simpkins]

OpenSSL doesn't appear to do any special handling for i18n, and just
treats the strings as binary data. It uses fgets() to read the password
from the terminal, so it will receive it however the terminal encodes
it.

It's not clear to me that PyUnicode_EncodeFSDefault() is quite the right
thing to do. The initfsencoding() routine seems different from how
initstdio() works (which checks the PYTHONIOENCODING environment
variable, and then appears to fall back to os.device_encoding()).

I'm leaning towards just updating the docs to specify that if a string
is supplied it will be encoded using UTF-8. I'm happy to do either way
you recommend, though.

[simpkins]

Here's a patch with updates to the documentation to more fully specify the
behavior of the password field, including specifying that strings will be
encoded using UTF-8.

[python-dev]

New changeset cdc6c1b072a5 by Antoine Pitrou in branch 'default':
Issue bpo-12803: SSLContext.load_cert_chain() now accepts a password argument
http://hg.python.org/cpython/rev/cdc6c1b072a5

[pitrou]

Your latest patch was committed, thank you!