Merge lp:~zseil/pyopenssl/client_CA into lp:~exarkun/pyopenssl/trunk

[Ziga Seilnacht]

[Ziga Seilnacht (zseil)]

Hello Jean-Paul,

This branch adds finer control over the list of preferred certificate authorities sent by the server when requesting a client certificate.

This functionality was requested in bug #364185 and in Twisted's bug #2061. The branch adds two new methods to SSL.Context, set_client_CA_list and add_client_CA and one new method to SSL.Connection, get_client_CA_list. The new methods are documented and tested.

More information can be found in commit messages and documentation. There are some unrelated changes in this branch, let me know if I should create a separate branch for those.

Regards,
Ziga

[Jean-Paul Calderone (exarkun)]

Hi Žiga,

First, thanks for working on this. :)

It would be great to see the CA management API additions separated out from the other bug fixes. I think the new methods look like they should go in (I need to spend a little more time looking at the code and reading about the relevant OpenSSL APIs to be sure). Some of the other changes are a bit trickier though. It'd be great to get the CA stuff in without having to think about all the consequences of changing the type names of all the types in pyOpenSSL, for example. :) Similarly, I think some of the unrelated fixes (eg, for NULL return checks) should probably have test coverage added, but figuring out exactly how to test those may take some consideration.

Thanks again. Looking forward to the next version of the branch.

review: Needs Fixing

[Ziga Seilnacht (zseil)]

> Hi Žiga,
>
> First, thanks for working on this. :)
>
> It would be great to see the CA management API additions separated out from
> the other bug fixes. I think the new methods look like they should go in (I
> need to spend a little more time looking at the code and reading about the
> relevant OpenSSL APIs to be sure). Some of the other changes are a bit
> trickier though. It'd be great to get the CA stuff in without having to think
> about all the consequences of changing the type names of all the types in
> pyOpenSSL, for example. :) Similarly, I think some of the unrelated fixes
> (eg, for NULL return checks) should probably have test coverage added, but
> figuring out exactly how to test those may take some consideration.
>
> Thanks again. Looking forward to the next version of the branch.

Hello Jean-Paul,

Thank you for reviewing :)

I reverted the unrelated changes now, I'll submit them as separate issues in the next few days.

review: Needs Resubmitting

[Jean-Paul Calderone (exarkun)]

  1. In the tex docs for add_client_CA, two lines are added which each begin with "%" and repeat the word "Certificate" a number of times. What's this?
  2. From the tex docs, I can't tell what the argument to set_client_CA_list should be a list of. I would have guessed X509Names, but since add_client_CA takes an X509, I'm not sure (the Python docstring clarifies this; it'd be good to have the information in both places).
  3. Why does add_client_CA take an X509 instead of an X509Name? (I suppose it's because that's the how the OpenSSL API works - lame)
  4. I've been postponing looking at the cross-module type checking code that you replaced with "import_crypto_type" because it wasn't causing problems and I didn't want to think about it. However... now I've thought about it a little, and I think it should be simpler. If crypto_X509NameObj is defined, then crypto_X509Name_Type should be defined too, since they're defined in the same place. So the code should just use crypto_X509Name_Type (or crypto_PKey_Type or crypto_X509_Type). It doesn't need to do the import dance or check sizes or anything. There was probably a reason not to do it this way long, long ago, but I'm not very interested in it now.
  5. Likewise, crypto_X509Name_Check should be defined for ssl_Context_set_client_CA_list to use.

Thanks! I generally like the code and it doesn't seem to have any major problems. I'm looking forward to the next version of this.

review: Needs Fixing

[Ziga Seilnacht (zseil)]

Jean-Paul Calderone wrote:
> Review: Needs Fixing
> 1. In the tex docs for add_client_CA, two lines are added which each begin with "%" and repeat the word "Certificate" a number of times. What's this?

That was supposed to be a humorous reminder for myself that the text
should be clarified. It seems it failed in its purpose, I'll remove it
and try to clarify the text.

> 2. From the tex docs, I can't tell what the argument to set_client_CA_list should be a list of. I would have guessed X509Names, but since add_client_CA takes an X509, I'm not sure (the Python docstring clarifies this; it'd be good to have the information in both places).

Ok.

> 3. Why does add_client_CA take an X509 instead of an X509Name? (I suppose it's because that's the how the OpenSSL API works - lame)

Yes, the underlying OpenSSL API accepts certificates instead of
X509Names. Should I drop the whole add_client_CA method? It is
not strictly necessary.

Also, are the current method names ok? The preexisting method
load_client_ca_list has lowercase CA, should the new methods be
consistent with it rather than with the OpenSSL API?

> 4. I've been postponing looking at the cross-module type checking code that you replaced with "import_crypto_type" because it wasn't causing problems and I didn't want to think about it. However... now I've thought about it a little, and I think it should be simpler. If crypto_X509NameObj is defined, then crypto_X509Name_Type should be defined too, since they're defined in the same place. So the code should just use crypto_X509Name_Type (or crypto_PKey_Type or crypto_X509_Type). It doesn't need to do the import dance or check sizes or anything. There was probably a reason not to do it this way long, long ago, but I'm not very interested in it now.

crypto_XXX_Type's can't be used in the same way as crypto_XXXobj's. The
former are true runtime variables, while the latter are just struct
definitions. To use crypto_XXX_Types directly, the SSL module would have
  to be linked against the crypto module.

Other extension modules (Python's datetime for example) usually solve
this by providing the types in the CAPI struct pointer, and then using
some fairly ugly macros to make them look as normal variables. I can do
that, but we would loose ABI compatibility if we would reuse the current
crypto_XXX_Type names. I guess exposing them just with crypto_XXX_Check
macros would be enough?

> 5. Likewise, crypto_X509Name_Check should be defined for ssl_Context_set_client_CA_list to use.

I did try to use it, but that lead to linking errors and a few hours of
headscratching before I figured out what the difference between *Objs
and *Types was :)

>
> Thanks! I generally like the code and it doesn't seem to have any major problems. I'm looking forward to the next version of this.
>

Thank you for the review. I hope I got the workflow right, the status of
the merge request should be changed to resubmit when the fixes are made?
  Launchpad's interface is a bit confusing in this regard, I guess I
should report a bug...

Regards,
Ziga

[Jean-Paul Calderone (exarkun)]

>
> Thank you for the review. I hope I got the workflow right, the status of
> the merge request should be changed to resubmit when the fixes are made?
> Launchpad's interface is a bit confusing in this regard, I guess I
> should report a bug...
>

I respond to the rest later on, but for now I just wanted to comment on the workflow. I'm basically making this up as I go. pyOpenSSL is the only project I really seriously use Launchpad for. I'm sort of trying to implement Twisted's development workflow using Launchpad, but that doesn't tell me what state to put merge proposals into or other Launchpad-specific details like that. So, so far so good, I think, but if you have any suggestions (or just any feedback - what parts of the workflow are helpful, what parts are not), please let me know. :)

Thanks again for your work on this ticket.

Jean-Paul

[Jean-Paul Calderone (exarkun)]

Just wanted to ping you on this. Should I be re-reviewing this branch (it doesn't look like there have been any changes since last time), or providing more feedback? 0.10 might be coming up, and if this can be ready it'd be great to include it.

[lp:~zseil/pyopenssl/client_CA]

132. By Ziga Seilnacht on 2009-10-23

Clarify documentation for SSL.Context.set_client_CA_list and remove silly comments.

133. By Ziga Seilnacht on 2009-10-23

Lowercase *client_CA* methods for consistency with the rest of PyOpenSSL.

[Ziga Seilnacht (zseil)]

> Just wanted to ping you on this. Should I be re-reviewing this branch (it
> doesn't look like there have been any changes since last time), or providing
> more feedback? 0.10 might be coming up, and if this can be ready it'd be
> great to include it.

Sorry for the long silence, I've been terribly busy at work lately.

I've updated the branch now with doc fixes, this should address 1) and 2).

Regarding 3), I can drop the whole method, but I think that it is useful
as it is, i.e. I expect that it would be used like this:

    store = context.get_cert_store()
    for cacert in cacerts:
        store.add_cert(cacert)
        context.add_client_ca(cacert)

I also lowercased the new methods, for consistency with the rest of
PyOpenSSL (e.g. SSL.Context.load_client_ca,
crypto.PKCS12.get_ca_certificates). This change was made in a single
revision, so it can be dropped if you think the old names are better.

Regarding 4) and 5), the needed changes would be quite intrusive and
I'm not sure that we could preserve binary compatibility. I'll try to
work on that over the weekend, but I would be more comfortable if that
was a separate branch, possibly merged after the release.

review: Needs Resubmitting

[Jean-Paul Calderone (exarkun)]

> > Just wanted to ping you on this. Should I be re-reviewing this branch (it
> > doesn't look like there have been any changes since last time), or providing
> > more feedback? 0.10 might be coming up, and if this can be ready it'd be
> > great to include it.
>
> Sorry for the long silence, I've been terribly busy at work lately.

No worries, I can understand that. :)

> Regarding 4) and 5), the needed changes would be quite intrusive and
> I'm not sure that we could preserve binary compatibility. I'll try to
> work on that over the weekend, but I would be more comfortable if that
> was a separate branch, possibly merged after the release.

I think you've probably put more thought into this area so far than I have. So I think I'm happy to worry about these points later on, if at all. I'm going to take a look at the branch again now.

[lp:~zseil/pyopenssl/client_CA]

134. By Jean-Paul Calderone on 2009-10-24

Break up big set_client_ca_list test into a bunch of smaller ones

135. By Jean-Paul Calderone on 2009-10-24

Break up big add_client_ca test into a bunch of smaller ones

136. By Jean-Paul Calderone on 2009-10-24

doc formatting and wording tweak

137. By Jean-Paul Calderone on 2009-10-24

remove pre-2.5 warning with PyObject_GetAttrString; reformat some braces

138. By Jean-Paul Calderone on 2009-10-24

a few more brace re-arrangements

[Ziga Seilnacht (zseil)]

>> Regarding 4) and 5), the needed changes would be quite intrusive and
>> I'm not sure that we could preserve binary compatibility. I'll try to
>> work on that over the weekend, but I would be more comfortable if that
>> was a separate branch, possibly merged after the release.
>
> I think you've probably put more thought into this area so far than I have.
> So I think I'm happy to worry about these points later on, if at all. I'm
> going to take a look at the branch again now.

Thanks for your work, I've seen your recent changes, it looks like I should
look deeper into this "unit" thing in unit testing :).

I've looked into C API issues some more and have found a way that preserves
API as well as ABI compatibility, you can see the current progress at:

lp:~ziga-seilnacht/pyopenssl/bwcompat-c-api

The branch is based on client_CA branch and contains needed changes to the
crypto C API and the resulting simplification in SSL module.

Let me know if you think this is worth pursuing, in that case I'll open a
ticket and try to find some time to write tests and do the same for SSL
C API.

[Jean-Paul Calderone (exarkun)]

> >> Regarding 4) and 5), the needed changes would be quite intrusive and
> >> I'm not sure that we could preserve binary compatibility. I'll try to
> >> work on that over the weekend, but I would be more comfortable if that
> >> was a separate branch, possibly merged after the release.
> >
> > I think you've probably put more thought into this area so far than I have.
> > So I think I'm happy to worry about these points later on, if at all. I'm
> > going to take a look at the branch again now.
>
> Thanks for your work, I've seen your recent changes, it looks like I should
> look deeper into this "unit" thing in unit testing :).

It can definitely be helpful to have the smallest possible test method for each individual piece of behavior, since a single test method is the smallest unit of testing one can work with (for example, I can run one test method at a time with my test runner, but not less than that :).

Of course, with pyOpenSSL, I find that it becomes a bit challenging to decide what a sensible unit really is. Can't really test add_client_ca without also testing get_client_ca_list, for example. And writing a unit test that verifies some code doesn't leak memory is pretty hard. I'm just applying what I've learned about Python unit testing, hoping that at least some of it makes sense, and trying to learn from the experience. :)

> I've looked into C API issues some more and have found a way that preserves
> API as well as ABI compatibility, you can see the current progress at:
>
> lp:~ziga-seilnacht/pyopenssl/bwcompat-c-api
>
> The branch is based on client_CA branch and contains needed changes to the
> crypto C API and the resulting simplification in SSL module.
>
> Let me know if you think this is worth pursuing, in that case I'll open a
> ticket and try to find some time to write tests and do the same for SSL
> C API.

Cool! I think I'll have a chance to look at this tomorrow. Thanks.

[lp:~zseil/pyopenssl/client_CA]

139. By Jean-Paul Calderone on 2009-11-07

Minimal tests for Context.add_extra_chain_cert

=== modified file 'doc/pyOpenSSL.tex'
--- doc/pyOpenSSL.tex	2009-07-25 16:32:46 +0000
+++ doc/pyOpenSSL.tex	2009-10-23 08:09:11 +0000
@@ -189,8 +189,8 @@
 \end{datadesc}
 
 \begin{classdesc}{X509Extension}{typename, critical, value\optional{, subject}\optional{, issuer}}
-A class representing an X.509 v3 certificate extensions.  
-See \url{http://openssl.org/docs/apps/x509v3_config.html\#STANDARD_EXTENSIONS} 
+A class representing an X.509 v3 certificate extensions.
+See \url{http://openssl.org/docs/apps/x509v3_config.html\#STANDARD_EXTENSIONS}
 for \var{typename} strings and their options.
 Optional parameters \var{subject} and \var{issuer} must be X509 objects.
 \end{classdesc}
@@ -673,7 +673,7 @@
 \end{funcdesc}
 
 \begin{excdesc}{Error}
-If the current RAND method supports any errors, this is raised when needed.  
+If the current RAND method supports any errors, this is raised when needed.
 The default method does not raise this when the entropy pool is depleted.
 
 Whenever this exception is raised directly, it has a list of error messages
@@ -853,6 +853,22 @@
 when requesting a client certificate.
 \end{methoddesc}
 
+\begin{methoddesc}[Context]{set_client_ca_list}{certificate_authorities}
+Replace the current list of preferred certificate signers that would be
+sent to the client when requesting a client certificate with the
+\var{certificate_authorities} sequence of \class{OpenSSL.crypto.X509Name}s.
+
+\versionadded{0.10}
+\end{methoddesc}
+
+\begin{methoddesc}[Context]{add_client_ca}{certificate_authority}
+Extract a \class{OpenSSL.crypto.X509Name} from the \var{certificate_authority}
+\class{OpenSSL.crypto.X509} certificate and add it to the list of preferred
+certificate signers sent to the client when requesting a client certificate.
+
+\versionadded{0.10}
+\end{methoddesc}
+
 \begin{methoddesc}[Context]{load_verify_locations}{pemfile, capath}
 Specify where CA certificates for verification purposes are located. These
 are trusted certificates. Note that the certificates have to be in PEM
@@ -1026,6 +1042,20 @@
 but not it returns the entire list in one go.
 \end{methoddesc}
 
+\begin{methoddesc}[Connection]{get_client_ca_list}{}
+Retrieve the list of preferred client certificate issuers sent by the server
+as \class{OpenSSL.crypto.X509Name} objects.
+
+If this is a client \class{Connection}, the list will be empty until the
+connection with the server is established.
+
+If this is a server \class{Connection}, return the list of certificate
+authorities that will be sent or has been sent to the client, as controlled
+by this \class{Connection}'s \class{Context}.
+
+\versionadded{0.10}
+\end{methoddesc}
+
 \begin{methoddesc}[Connection]{get_context}{}
 Retrieve the Context object associated with this Connection.
 \end{methoddesc}

=== modified file 'src/ssl/connection.c'
--- src/ssl/connection.c	2009-07-08 16:48:33 +0000
+++ src/ssl/connection.c	2009-10-23 08:09:11 +0000
@@ -829,6 +829,58 @@
     return lst;
 }
 
+static char ssl_Connection_get_client_ca_list_doc[] = "\n\
+Get CAs whose certificates are suggested for client authentication.\n\
+\n\
+@return: A list of X509Names representing the acceptable CAs as set by\n\
+         ssl.Context.{set, add}_client_ca* if this is a server connection\n\
+         or as sent by the server if this is a client connection.\n\
+";
+
+static PyObject *
+ssl_Connection_get_client_ca_list(ssl_ConnectionObj *self, PyObject *args)
+{
+    STACK_OF(X509_NAME) *CANames;
+    PyObject *CAList;
+    int i, n;
+
+    if (!PyArg_ParseTuple(args, ":get_client_ca_list")) {
+        return NULL;
+    }
+    CANames = SSL_get_client_CA_list(self->ssl);
+    if (CANames == NULL) {
+        return PyList_New(0);
+    }
+    n = sk_X509_NAME_num(CANames);
+    CAList = PyList_New(n);
+    if (CAList == NULL) {
+        return NULL;
+    }
+    for (i = 0; i < n; i++) {
+        X509_NAME *CAName;
+        PyObject *CA;
+
+        CAName = X509_NAME_dup(sk_X509_NAME_value(CANames, i));
+        if (CAName == NULL) {
+            Py_DECREF(CAList);
+            exception_from_error_queue(ssl_Error);
+            return NULL;
+        }
+        CA = (PyObject *)crypto_X509Name_New(CAName, 1);
+        if (CA == NULL) {
+            X509_NAME_free(CAName);
+            Py_DECREF(CAList);
+            return NULL;
+        }
+        if (PyList_SetItem(CAList, i, CA)) {
+            Py_DECREF(CA);
+            Py_DECREF(CAList);
+            return NULL;
+        }
+    }
+    return CAList;
+}
+
 static char ssl_Connection_makefile_doc[] = "\n\
 The makefile() method is not implemented, since there is no dup semantics\n\
 for SSL connections\n\
@@ -1087,6 +1139,7 @@
     ADD_METHOD(bio_shutdown),
     ADD_METHOD(shutdown),
     ADD_METHOD(get_cipher_list),
+    ADD_METHOD(get_client_ca_list),
     ADD_METHOD(makefile),
     ADD_METHOD(get_app_data),
     ADD_METHOD(set_app_data),

=== modified file 'src/ssl/context.c'
--- src/ssl/context.c	2009-07-08 16:48:33 +0000
+++ src/ssl/context.c	2009-10-23 08:09:11 +0000
@@ -335,36 +335,65 @@
     return Py_None;
 }
 
+static PyTypeObject *
+type_modified_error(const char *name)
+{
+    PyErr_Format(PyExc_RuntimeError,
+                 "OpenSSL.crypto's '%s' attribute has been modified",
+                 name);
+    return NULL;
+}
+
+static PyTypeObject *
+import_crypto_type(const char *name, size_t objsize)
+{
+    PyObject *module, *type, *name_attr;
+    PyTypeObject *res;
+    int right_name;
+
+    module = PyImport_ImportModule("OpenSSL.crypto");
+    if (module == NULL) {
+        return NULL;
+    }
+    type = PyObject_GetAttrString(module, name);
+    Py_DECREF(module);
+    if (type == NULL) {
+        return NULL;
+    }
+    if (!(PyType_Check(type))) {
+        Py_DECREF(type);
+        return type_modified_error(name);
+    }
+    name_attr = PyObject_GetAttrString(type, "__name__");
+    if (name_attr == NULL) {
+        Py_DECREF(type);
+        return NULL;
+    }
+    right_name = (PyString_CheckExact(name_attr) &&
+                  strcmp(name, PyString_AsString(name_attr)) == 0);
+    Py_DECREF(name_attr);
+    res = (PyTypeObject *)type;
+    if (!right_name || res->tp_basicsize != objsize) {
+        Py_DECREF(type);
+        return type_modified_error(name);
+    }
+    return res;
+}
+
 static crypto_X509Obj *
-parse_certificate_argument(const char* format1, const char* format2, PyObject* args)
+parse_certificate_argument(const char* format, PyObject* args)
 {
     static PyTypeObject *crypto_X509_type = NULL;
     crypto_X509Obj *cert;
 
-    /* We need to check that cert really is an X509 object before
-       we deal with it. The problem is we can't just quickly verify
-       the type (since that comes from another module). This should
-       do the trick (reasonably well at least): Once we have one
-       verified object, we use it's type object for future
-       comparisons. */
-
     if (!crypto_X509_type)
     {
-	if (!PyArg_ParseTuple(args, (PYARG_PARSETUPLE_FORMAT *)format1, &cert))
-	    return NULL;
-
-	if (strcmp(cert->ob_type->tp_name, "X509") != 0 || 
-	    cert->ob_type->tp_basicsize != sizeof(crypto_X509Obj))
-	{
-	    PyErr_SetString(PyExc_TypeError, "Expected an X509 object");
-	    return NULL;
-	}
-
-	crypto_X509_type = cert->ob_type;
+        crypto_X509_type = import_crypto_type("X509", sizeof(crypto_X509Obj));
+        if (!crypto_X509_type)
+            return NULL;
     }
-    else
-	if (!PyArg_ParseTuple(args, (PYARG_PARSETUPLE_FORMAT *)format2, crypto_X509_type,
-			      &cert))
+    if (!PyArg_ParseTuple(args, (PYARG_PARSETUPLE_FORMAT *)format,
+                          crypto_X509_type, &cert))
 	    return NULL;
     return cert;
 }
@@ -381,7 +410,7 @@
 {
     X509* cert_original;
     crypto_X509Obj *cert = parse_certificate_argument(
-        "O:add_extra_chain_cert", "O!:add_extra_chain_cert", args);
+        "O!:add_extra_chain_cert", args);
     if (cert == NULL)
     {
         return NULL;
@@ -471,7 +500,7 @@
 ssl_Context_use_certificate(ssl_ContextObj *self, PyObject *args)
 {
     crypto_X509Obj *cert = parse_certificate_argument(
-        "O:use_certificate", "O!:use_certificate", args);
+        "O!:use_certificate", args);
     if (cert == NULL) {
         return NULL;
     }
@@ -538,28 +567,12 @@
     static PyTypeObject *crypto_PKey_type = NULL;
     crypto_PKeyObj *pkey;
 
-    /* We need to check that cert really is a PKey object before
-       we deal with it. The problem is we can't just quickly verify
-       the type (since that comes from another module). This should
-       do the trick (reasonably well at least): Once we have one
-       verified object, we use it's type object for future
-       comparisons. */
-
     if (!crypto_PKey_type)
     {
-	if (!PyArg_ParseTuple(args, "O:use_privatekey", &pkey))
-	    return NULL;
-
-	if (strcmp(pkey->ob_type->tp_name, "OpenSSL.crypto.PKey") != 0 ||
-	    pkey->ob_type->tp_basicsize != sizeof(crypto_PKeyObj))
-	{
-	    PyErr_SetString(PyExc_TypeError, "Expected a PKey object");
-	    return NULL;
-	}
-
-	crypto_PKey_type = pkey->ob_type;
+        crypto_PKey_type = import_crypto_type("PKey", sizeof(crypto_PKeyObj));
+        if (!crypto_PKey_type)
+            return NULL;
     }
-    else
     if (!PyArg_ParseTuple(args, "O!:use_privatekey", crypto_PKey_type, &pkey))
         return NULL;
 
@@ -789,6 +802,111 @@
     }
 }
 
+static char ssl_Context_set_client_ca_list_doc[] = "\n\
+Set the list of preferred client certificate signers for this server context.\n\
+\n\
+This list of certificate authorities will be sent to the client when the\n\
+server requests a client certificate.\n\
+\n\
+@param certificate_authorities: a sequence of X509Names.\n\
+@return: None\n\
+";
+
+static PyObject *
+ssl_Context_set_client_ca_list(ssl_ContextObj *self, PyObject *args)
+{
+    static PyTypeObject *X509NameType;
+    PyObject *sequence, *tuple, *item;
+    crypto_X509NameObj *name;
+    X509_NAME *sslname;
+    STACK_OF(X509_NAME) *CANames;
+    Py_ssize_t length;
+    int i;
+
+    if (X509NameType == NULL) {
+        X509NameType = import_crypto_type("X509Name", sizeof(crypto_X509NameObj));
+        if (X509NameType == NULL) {
+            return NULL;
+        }
+    }
+    if (!PyArg_ParseTuple(args, "O:set_client_ca_list", &sequence)) {
+        return NULL;
+    }
+    tuple = PySequence_Tuple(sequence);
+    if (tuple == NULL) {
+        return NULL;
+    }
+    length = PyTuple_Size(tuple);
+    if (length >= INT_MAX) {
+        PyErr_SetString(PyExc_ValueError, "client CA list is too long");
+        Py_DECREF(tuple);
+        return NULL;
+    }
+    CANames = sk_X509_NAME_new_null();
+    if (CANames == NULL) {
+        Py_DECREF(tuple);
+        exception_from_error_queue(ssl_Error);
+        return NULL;
+    }
+    for (i = 0; i < length; i++) {
+        item = PyTuple_GetItem(tuple, i);
+        if (item->ob_type != X509NameType) {
+            PyErr_Format(PyExc_TypeError,
+                         "client CAs must be X509Name objects, not %s objects",
+                         item->ob_type->tp_name);
+            sk_X509_NAME_free(CANames);
+            Py_DECREF(tuple);
+            return NULL;
+        }
+        name = (crypto_X509NameObj *)item;
+        sslname = X509_NAME_dup(name->x509_name);
+        if (sslname == NULL) {
+            sk_X509_NAME_free(CANames);
+            Py_DECREF(tuple);
+            exception_from_error_queue(ssl_Error);
+            return NULL;
+        }
+        if (!sk_X509_NAME_push(CANames, sslname)) {
+            X509_NAME_free(sslname);
+            sk_X509_NAME_free(CANames);
+            Py_DECREF(tuple);
+            exception_from_error_queue(ssl_Error);
+            return NULL;
+        }
+    }
+    Py_DECREF(tuple);
+    SSL_CTX_set_client_CA_list(self->ctx, CANames);
+    Py_INCREF(Py_None);
+    return Py_None;
+}
+
+static char ssl_Context_add_client_ca_doc[] = "\n\
+Add the CA certificate to the list of preferred signers for this context.\n\
+\n\
+The list of certificate authorities will be sent to the client when the\n\
+server requests a client certificate.\n\
+\n\
+@param certificate_authority: certificate authority's X509 certificate.\n\
+@return: None\n\
+";
+
+static PyObject *
+ssl_Context_add_client_ca(ssl_ContextObj *self, PyObject *args)
+{
+    crypto_X509Obj *cert;
+
+    cert = parse_certificate_argument("O!:add_client_ca", args);
+    if (cert == NULL) {
+        return NULL;
+    }
+    if (!SSL_CTX_add_client_CA(self->ctx, cert->x509)) {
+        exception_from_error_queue(ssl_Error);
+        return NULL;
+    }
+    Py_INCREF(Py_None);
+    return Py_None;
+}
+
 static char ssl_Context_set_timeout_doc[] = "\n\
 Set session timeout\n\
 \n\
@@ -960,6 +1078,8 @@
     ADD_METHOD(get_verify_depth),
     ADD_METHOD(load_tmp_dh),
     ADD_METHOD(set_cipher_list),
+    ADD_METHOD(set_client_ca_list),
+    ADD_METHOD(add_client_ca),
     ADD_METHOD(set_timeout),
     ADD_METHOD(get_timeout),
     ADD_METHOD(set_info_callback),

=== modified file 'src/util.h'
--- src/util.h	2009-07-08 16:48:33 +0000
+++ src/util.h	2009-10-23 08:09:11 +0000
@@ -119,6 +119,10 @@
 }
 #endif
 
-
+#if !defined(PY_SSIZE_T_MIN)
+typedef int Py_ssize_t;
+#define PY_SSIZE_T_MAX INT_MAX
+#define PY_SSIZE_T_MIN INT_MIN
+#endif
 
 #endif

=== modified file 'test/test_ssl.py'
--- test/test_ssl.py	2009-07-25 16:32:46 +0000
+++ test/test_ssl.py	2009-10-23 08:09:11 +0000
@@ -255,7 +255,7 @@
             context = Context(SSLv3_METHOD)
             context.set_default_verify_paths()
             context.set_verify(
-                VERIFY_PEER, 
+                VERIFY_PEER,
                 lambda conn, cert, errno, depth, preverify_ok: preverify_ok)
 
             client = socket()
@@ -511,8 +511,8 @@
             established = True  # assume the best
             for ssl in client_conn, server_conn:
                 try:
-                    # Generally a recv() or send() could also work instead 
-                    # of do_handshake(), and we would stop on the first 
+                    # Generally a recv() or send() could also work instead
+                    # of do_handshake(), and we would stop on the first
                     # non-exception.
                     ssl.do_handshake()
                 except WantReadError:
@@ -583,6 +583,125 @@
         self.assertEquals(e.__class__, Error)
 
 
+    def _check_client_ca_list(self, func):
+        server = self._server(None)
+        client = self._client(None)
+        self.assertEqual(client.get_client_ca_list(), [])
+        self.assertEqual(server.get_client_ca_list(), [])
+        ctx = server.get_context()
+        expected = func(ctx)
+        self.assertEqual(client.get_client_ca_list(), [])
+        self.assertEqual(server.get_client_ca_list(), expected)
+        self._loopback(client, server)
+        self.assertEqual(client.get_client_ca_list(), expected)
+        self.assertEqual(server.get_client_ca_list(), expected)
+
+
+    def test_set_client_ca_list_basic(self):
+        """
+        Test paramater validation and return value for
+        L{Context.set_client_ca_list}.
+        """
+        ctx = Context(TLSv1_METHOD)
+        self.assertRaises(TypeError, ctx.set_client_ca_list, "spam")
+        self.assertRaises(TypeError, ctx.set_client_ca_list, ["spam"])
+        self.assertIdentical(ctx.set_client_ca_list([]), None)
+
+
+    def test_set_client_ca_list_functional(self):
+        """
+        The list of CAs set by L{Context.set_client_ca_list} and read by
+        L{Connection.get_client_ca_list} should match on server and
+        client side.
+        """
+        cacert = load_certificate(FILETYPE_PEM, root_cert_pem)
+        secert = load_certificate(FILETYPE_PEM, server_cert_pem)
+        clcert = load_certificate(FILETYPE_PEM, server_cert_pem)
+
+        cadesc = cacert.get_subject()
+        sedesc = secert.get_subject()
+        cldesc = clcert.get_subject()
+
+        def single_ca(ctx):
+            ctx.set_client_ca_list([cadesc])
+            return [cadesc]
+        self._check_client_ca_list(single_ca)
+
+        def no_ca(ctx):
+            ctx.set_client_ca_list([])
+            return []
+        self._check_client_ca_list(no_ca)
+
+        def multiple_ca(ctx):
+            L = [cadesc, sedesc, cldesc]
+            ctx.set_client_ca_list(L)
+            return L
+        self._check_client_ca_list(multiple_ca)
+
+        def changed_ca(ctx):
+            ctx.set_client_ca_list([sedesc, cldesc])
+            ctx.set_client_ca_list([cadesc])
+            return [cadesc]
+        self._check_client_ca_list(changed_ca)
+
+        def mutated_ca(ctx):
+            L = [cadesc]
+            ctx.set_client_ca_list([cadesc])
+            L.append(sedesc)
+            return [cadesc]
+        self._check_client_ca_list(mutated_ca)
+
+
+    def test_add_client_ca_basic(self):
+        """
+        Test paramater validation and return value for
+        L{Context.add_client_ca}.
+        """
+        ctx = Context(TLSv1_METHOD)
+        cacert = load_certificate(FILETYPE_PEM, root_cert_pem)
+        self.assertRaises(TypeError, ctx.add_client_ca, "spam")
+        self.assertIdentical(ctx.add_client_ca(cacert), None)
+
+
+    def test_add_client_ca_functional(self):
+        """
+        The list of CAs set by L{Context.add_client_ca} and read by
+        L{Connection.get_client_ca_list} should match on server and
+        client side.
+        """
+        cacert = load_certificate(FILETYPE_PEM, root_cert_pem)
+        secert = load_certificate(FILETYPE_PEM, server_cert_pem)
+        clcert = load_certificate(FILETYPE_PEM, server_cert_pem)
+
+        cadesc = cacert.get_subject()
+        sedesc = secert.get_subject()
+        cldesc = clcert.get_subject()
+
+        def single_ca(ctx):
+            ctx.add_client_ca(cacert)
+            return [cadesc]
+        self._check_client_ca_list(single_ca)
+
+        def multiple_ca(ctx):
+            ctx.add_client_ca(cacert)
+            ctx.add_client_ca(secert)
+            return [cadesc, sedesc]
+        self._check_client_ca_list(multiple_ca)
+
+        def mixed_set_add_ca(ctx):
+            ctx.set_client_ca_list([cadesc, sedesc])
+            ctx.add_client_ca(clcert)
+            return [cadesc, sedesc, cldesc]
+        self._check_client_ca_list(mixed_set_add_ca)
+
+        def set_replaces_add_ca(ctx):
+            ctx.add_client_ca(clcert)
+            ctx.set_client_ca_list([cadesc])
+            ctx.add_client_ca(secert)
+            return [cadesc, sedesc]
+        self._check_client_ca_list(set_replaces_add_ca)
+
+
 
 if __name__ == '__main__':
     main()