classification
Title: ctx.load_verify_locations(cadata)
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: christian.heimes, ezio.melotti, jcea, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2013-06-05 01:50 by christian.heimes, last changed 2014-06-19 22:43 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
sslctx_add_cert.patch christian.heimes, 2013-06-05 01:50 review
sslctx_add_cert2.patch christian.heimes, 2013-06-09 20:23 review
sslctx_add_cert4.patch christian.heimes, 2013-06-18 13:52 review
sslctx_add_cert5.patch christian.heimes, 2013-06-18 17:56 review
ssl_cadata.patch christian.heimes, 2013-11-20 19:24 review
ssl_cadata2.patch christian.heimes, 2013-11-20 22:25 review
Messages (12)
msg190637 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-05 01:50
The patch implements an add_cert(pem_or_der_data) method for the ssl.SSLContext() object. On success the method adds a trusted CA cert to the context's internal cert store. The CA certificate can either be an ASCII unicode string (PEM format) or buffer object (DER / ASN1 format).

The patch also implements a get_cert_count() method for debugging. I'm going to remove that function eventually as it doesn't give correct answers when the object table contains CRLs, too. A correct implementation might be useful to verify set_default_verify_paths().

I've split up the functions so I can re-use _add_cert() in my upcoming patch for an interface to crypt32.dll on Windows.
msg190872 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-09 20:23
New patch:

* rename function to add_ca_cert()
* only accept CA certs, no other certs
* raise an error if extra data is found after cert (e.g. two certs). PEM_read_bio_X509() silently ignores extra data
* fixes from Ezio's code review
* documentation
msg191409 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-18 13:52
Here is a simplified version of the C function. It uses y* or es# "ascii" to parse the argument.

The check for trailing data ensures that the user gets an error message if she tries to load a PEM string with multiple certs. She might expect that add_ca_cert(pem) loads all PEM certs from the string while in fact PEM_read_bio_X509() only loads the first cert. The new patch make the check optional.

I still need to find a good name for the option, though...
msg191411 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-06-18 13:59
> The check for trailing data ensures that the user gets an error
> message if she tries to load a PEM string with multiple certs. She
> might expect that add_ca_cert(pem) loads all PEM certs from the
> string while in fact PEM_read_bio_X509() only loads the first cert.

I don't think it is useful. Just make the behaviour well-documented.
(there is no security risk in loading too few CA certs)
msg191419 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-18 17:30
I'm pondering about the error case "cert already in hash table". There should be a way to distinguish the error from other errors. I see three ways to handle the case:

1) introduce SSLCertInStoreError exeption
2) ignore the error and do nothing
3) ignore the error and return True if a cert was added or False if the cert is already in the store

I like 3).
msg191420 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-06-18 17:35
Le mardi 18 juin 2013 à 17:30 +0000, Christian Heimes a écrit :
> Christian Heimes added the comment:
> 
> I'm pondering about the error case "cert already in hash table". There
> should be a way to distinguish the error from other errors.

I don't know if you've seen it, but SSLError has "library" and "reason"
attributes (they are little known). See SSLErrorTests.

>  I see three ways to handle the case:
> 
> 1) introduce SSLCertInStoreError exeption
> 2) ignore the error and do nothing
> 3) ignore the error and return True if a cert was added or False if
> the cert is already in the store
> 
> I like 3).

Yes, sounds reasonable.
msg191421 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-18 17:40
Yes, I have seen them. In fact OpenSSL has library, function and reason.

if ((ERR_GET_LIB(errcode) == ERR_LIB_X509) && 
    (ERR_GET_REASON(errcode) == X509_R_CERT_ALREADY_IN_HASH_TABLE)) {}

I'm going for 3)
msg203522 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-20 19:24
I think the patch in #16487 does too many things at once. The new patch is a draft for a new patch that adds SSLContext.load_verify_locations(cadata) to the SSL module. cadata can be a bunch of PEM encoded certs (ASCII) or DER encoded certs (bytes-like). The patch may contain bugs as I haven't verified all error paths yet.
msg203541 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-20 22:25
Final patch
msg203564 - (view) Author: Roundup Robot (python-dev) Date: 2013-11-21 02:35
New changeset 234e3c8dc52f by Christian Heimes in branch 'default':
Issue #18138: Implement cadata argument of SSLContext.load_verify_location()
http://hg.python.org/cpython/rev/234e3c8dc52f
msg203565 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-21 02:36
Memo to me: update whatsnew
msg212973 - (view) Author: Roundup Robot (python-dev) Date: 2014-03-09 19:17
New changeset 8e3b3b4a90fb by R David Murray in branch 'default':
whatsnew: SSLcontext.load_verify_locations cadata argument (#18138)
http://hg.python.org/cpython/rev/8e3b3b4a90fb
History
Date User Action Args
2014-06-19 22:43:31ezio.melottisetstatus: open -> closed
2014-03-09 19:17:41python-devsetstatus: pending -> open

messages: + msg212973
2013-11-21 02:36:13christian.heimessetstatus: open -> pending
messages: + msg203565

assignee: christian.heimes
resolution: fixed
stage: patch review -> resolved
2013-11-21 02:35:11python-devsetnosy: + python-dev
messages: + msg203564
2013-11-20 22:25:22christian.heimessetfiles: + ssl_cadata2.patch

messages: + msg203541
title: ssl.SSLContext.add_cert() -> ctx.load_verify_locations(cadata)
2013-11-20 19:24:21christian.heimessetfiles: + ssl_cadata.patch

messages: + msg203522
2013-06-25 22:55:07jceasetnosy: + jcea
2013-06-18 17:56:16christian.heimessetfiles: + sslctx_add_cert5.patch
2013-06-18 17:40:56christian.heimessetmessages: + msg191421
2013-06-18 17:35:48pitrousetmessages: + msg191420
2013-06-18 17:30:00christian.heimessetmessages: + msg191419
2013-06-18 13:59:04pitrousetmessages: + msg191411
2013-06-18 13:52:46christian.heimessetfiles: + sslctx_add_cert4.patch

messages: + msg191409
2013-06-09 20:23:35christian.heimessetfiles: + sslctx_add_cert2.patch
nosy: + pitrou, ezio.melotti
messages: + msg190872

2013-06-05 01:50:33christian.heimescreate