Add support for specifying subject and issuer information when creating X509 extensions

6 files changed, 192 insertions, 17 deletions

diff --git a/ChangeLog b/ChangeLog
index 98ae8ff..d9d45f8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2009-07-17  Rick Dean  <rick@fdd.com>
+            Jean-Paul Calderone  <exarkun@twistedmatrix.com>
+
+	* src/crypto/x509ext.c: Add subject and issuer parameters to
+	  X509Extension, allowing creation of extensions which require that
+	  information.  Fixes LP#322813.
+
 2009-07-16  Jean-Paul Calderone  <exarkun@twistedmatrix.com>
 
 	* test/util.py: Changed the base TestCase's tearDown to assert that
diff --git a/doc/pyOpenSSL.tex b/doc/pyOpenSSL.tex
index 1a05e64..bd83d6d 100644
--- a/doc/pyOpenSSL.tex
+++ b/doc/pyOpenSSL.tex
@@ -188,8 +188,11 @@ A Python type object representing the PKCS12 object type.
 See \class{X509Extension}.
 \end{datadesc}
 
-\begin{classdesc}{X509Extension}{typename, critical, value}
-A class representing an X.509 v3 certificate extensions.
+\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} 
+for \var{typename} strings and their options.
+Optional parameters \var{subject} and \var{issuer} must be X509 objects.
 \end{classdesc}
 
 \begin{datadesc}{NetscapeSPKIType}
diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h
index 5d0bf27..b5e6b65 100644
--- a/src/crypto/crypto.h
+++ b/src/crypto/crypto.h
@@ -49,7 +49,7 @@ extern PyObject *crypto_Error;
 
 #define crypto_X509Extension_New_NUM    5
 #define crypto_X509Extension_New_RETURN crypto_X509ExtensionObj *
-#define crypto_X509Extension_New_PROTO  (char *, int, char *)
+#define crypto_X509Extension_New_PROTO  (char *, int, char *, crypto_X509Obj *, crypto_X509Obj *)
 
 #define crypto_PKCS7_New_NUM            6
 #define crypto_PKCS7_New_RETURN         crypto_PKCS7Obj *
diff --git a/src/crypto/x509ext.c b/src/crypto/x509ext.c
index e7ab5e1..90ef543 100644
--- a/src/crypto/x509ext.c
+++ b/src/crypto/x509ext.c
@@ -72,20 +72,44 @@ static PyMethodDef crypto_X509Extension_methods[] =
  * Arguments: type_name - ???
  *            critical  - ???
  *            value     - ???
+ *            subject   - An x509v3 certificate which is the subject for this extension.
+ *            issuer    - An x509v3 certificate which is the issuer for this extension.
  * Returns:   The newly created X509Extension object
  */
 crypto_X509ExtensionObj *
-crypto_X509Extension_New(char *type_name, int critical, char *value)
-{
+crypto_X509Extension_New(char *type_name, int critical, char *value,
+                         crypto_X509Obj *subject, crypto_X509Obj  *issuer) {
     X509V3_CTX ctx;
     crypto_X509ExtensionObj *self;
     char* value_with_critical = NULL;
 
-    /* We have no configuration database - but perhaps we should.  Anyhow, the
-     * context is necessary for any extension which uses the r2i conversion
-     * method.  That is, X509V3_EXT_nconf may segfault if passed a NULL ctx. */
+
+    /*
+     * A context is necessary for any extension which uses the r2i conversion
+     * method.  That is, X509V3_EXT_nconf may segfault if passed a NULL ctx.
+     * Start off by initializing most of the fields to NULL.
+     */
+    X509V3_set_ctx(&ctx, NULL, NULL, NULL, NULL, 0);
+
+    /*
+     * We have no configuration database - but perhaps we should (some
+     * extensions may require it).
+     */
     X509V3_set_ctx_nodb(&ctx);
 
+    /*
+     * Initialize the subject and issuer, if appropriate.  ctx is a local, and
+     * as far as I can tell none of the X509V3_* APIs invoked here steal any
+     * references, so no need to incref subject or issuer.
+     */
+    if (subject) {
+            ctx.subject_cert = subject->x509;
+    }
+
+    if (issuer) {
+            ctx.issuer_cert = issuer->x509;
+    }
+
     self = PyObject_New(crypto_X509ExtensionObj, &crypto_X509Extension_Type);
 
     if (self == NULL) {
@@ -137,27 +161,40 @@ crypto_X509Extension_New(char *type_name, int critical, char *value)
 }
 
 static char crypto_X509Extension_doc[] = "\n\
-X509Extension(typename, critical, value) -> X509Extension instance\n\
+X509Extension(typename, critical, value[, subject][, issuer]) -> \n\
+                X509Extension instance\n\
 \n\
 @param typename: The name of the extension to create.\n\
 @type typename: C{str}\n\
 @param critical: A flag indicating whether this is a critical extension.\n\
 @param value: The value of the extension.\n\
 @type value: C{str}\n\
+@param subject: Optional X509 cert to use as subject.\n\
+@type subject: C{X509}\n\
+@param issuer: Optional X509 cert to use as issuer.\n\
+@type issuer: C{X509}\n\
 @return: The X509Extension object\n\
 ";
 
 static PyObject *
-crypto_X509Extension_new(PyTypeObject *subtype, PyObject *args, PyObject *kwargs) {
+crypto_X509Extension_new(PyTypeObject *subtype, PyObject *args,
+                         PyObject *kwargs) {
     char *type_name, *value;
-    int critical;
-
-    if (!PyArg_ParseTuple(args, "sis:X509Extension", &type_name, &critical,
-                          &value)) {
+    int critical = 0;
+    crypto_X509Obj * subject = NULL;
+    crypto_X509Obj * issuer = NULL;
+    static char *kwlist[] = {"type_name", "critical", "value", "subject",
+                             "issuer", NULL};
+
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "sis|O!O!:X509Extension",
+                                     kwlist, &type_name, &critical, &value,
+                                     &crypto_X509_Type, &subject,
+                                     &crypto_X509_Type, &issuer )) {
         return NULL;
     }
 
-    return (PyObject *)crypto_X509Extension_New(type_name, critical, value);
+    return (PyObject *)crypto_X509Extension_New(type_name, critical, value,
+                                                subject, issuer);
 }
 
 /*
diff --git a/test/test_crypto.py b/test/test_crypto.py
index aa22cd0..9500e41 100644
--- a/test/test_crypto.py
+++ b/test/test_crypto.py
@@ -7,6 +7,7 @@ Unit tests for L{OpenSSL.crypto}.
 from unittest import main
 
 from os import popen2
+from datetime import datetime, timedelta
 
 from OpenSSL.crypto import TYPE_RSA, TYPE_DSA, Error, PKey, PKeyType
 from OpenSSL.crypto import X509, X509Type, X509Name, X509NameType
@@ -175,6 +176,33 @@ Ho4EzbYCOaEAMQA=
 
 
 class X509ExtTests(TestCase):
+    """
+    Tests for L{OpenSSL.crypto.X509Extension}.
+    """
+
+    def setUp(self):
+        """
+        Create a new private key and start a certificate request (for a test
+        method to finish in one way or another).
+        """
+        # Basic setup stuff to generate a certificate
+        self.pkey = PKey()
+        self.pkey.generate_key(TYPE_RSA, 384)
+        self.req = X509Req()
+        self.req.set_pubkey(self.pkey)
+        # Authority good you have.
+        self.req.get_subject().commonName = "Yoda root CA"
+        self.x509 = X509()
+        self.subject = self.x509.get_subject()
+        self.subject.commonName = self.req.get_subject().commonName
+        self.x509.set_issuer(self.subject)
+        self.x509.set_pubkey(self.pkey)
+        now = datetime.now().strftime("%Y%m%d%H%M%SZ")
+        expire  = (datetime.now() + timedelta(days=100)).strftime("%Y%m%d%H%M%SZ")
+        self.x509.set_notBefore(now)
+        self.x509.set_notAfter(expire)
+
+
     def test_construction(self):
         """
         L{X509Extension} accepts an extension type name, a critical flag,
@@ -234,6 +262,105 @@ class X509ExtTests(TestCase):
         self.assertEqual(ext.get_short_name(), 'nsComment')
 
 
+    def test_unused_subject(self):
+        """
+        The C{subject} parameter to L{X509Extension} may be provided for an
+        extension which does not use it and is ignored in this case.
+        """
+        ext1 = X509Extension('basicConstraints', False, 'CA:TRUE', subject=self.x509)
+        self.x509.add_extensions([ext1])
+        self.x509.sign(self.pkey, 'sha1')
+        # This is a little lame.  Can we think of a better way?
+        text = dump_certificate(FILETYPE_TEXT, self.x509)
+        self.assertTrue('X509v3 Basic Constraints:' in text)
+        self.assertTrue('CA:TRUE' in text)
+
+
+    def test_subject(self):
+        """
+        If an extension requires a subject, the C{subject} parameter to
+        L{X509Extension} provides its value.
+        """
+        ext3 = X509Extension('subjectKeyIdentifier', False, 'hash', subject=self.x509)
+        self.x509.add_extensions([ext3])
+        self.x509.sign(self.pkey, 'sha1')
+        text = dump_certificate(FILETYPE_TEXT, self.x509)
+        self.assertTrue('X509v3 Subject Key Identifier:' in text)
+
+
+    def test_missing_subject(self):
+        """
+        If an extension requires a subject and the C{subject} parameter is
+        given no value, something happens.
+        """
+        self.assertRaises(
+            Error, X509Extension, 'subjectKeyIdentifier', False, 'hash')
+
+
+    def test_invalid_subject(self):
+        """
+        If the C{subject} parameter is given a value which is not an L{X509}
+        instance, L{TypeError} is raised.
+        """
+        for badObj in [True, object(), "hello", [], self]:
+            self.assertRaises(
+                TypeError,
+                X509Extension,
+                'basicConstraints', False, 'CA:TRUE', subject=badObj)
+
+
+    def test_unused_issuer(self):
+        """
+        The C{issuer} parameter to L{X509Extension} may be provided for an
+        extension which does not use it and is ignored in this case.
+        """
+        ext1 = X509Extension('basicConstraints', False, 'CA:TRUE', issuer=self.x509)
+        self.x509.add_extensions([ext1])
+        self.x509.sign(self.pkey, 'sha1')
+        text = dump_certificate(FILETYPE_TEXT, self.x509)
+        self.assertTrue('X509v3 Basic Constraints:' in text)
+        self.assertTrue('CA:TRUE' in text)
+
+
+    def test_issuer(self):
+        """
+        If an extension requires a issuer, the C{issuer} parameter to
+        L{X509Extension} provides its value.
+        """
+        ext2 = X509Extension(
+            'authorityKeyIdentifier', False, 'issuer:always',
+            issuer=self.x509)
+        self.x509.add_extensions([ext2])
+        self.x509.sign(self.pkey, 'sha1')
+        text = dump_certificate(FILETYPE_TEXT, self.x509)
+        self.assertTrue('X509v3 Authority Key Identifier:' in text)
+        self.assertTrue('DirName:/CN=Yoda root CA' in text)
+
+
+    def test_missing_issuer(self):
+        """
+        If an extension requires an issue and the C{issuer} parameter is given
+        no value, something happens.
+        """
+        self.assertRaises(
+            Error,
+            X509Extension,
+            'authorityKeyIdentifier', False, 'keyid:always,issuer:always')
+
+
+    def test_invalid_issuer(self):
+        """
+        If the C{issuer} parameter is given a value which is not an L{X509}
+        instance, L{TypeError} is raised.
+        """
+        for badObj in [True, object(), "hello", [], self]:
+            self.assertRaises(
+                TypeError,
+                X509Extension,
+                'authorityKeyIdentifier', False, 'keyid:always,issuer:always',
+                issuer=badObj)
+
+
 
 class PKeyTests(TestCase):
     """
diff --git a/test/util.py b/test/util.py
index d3572db..e0e25f9 100644
--- a/test/util.py
+++ b/test/util.py
@@ -11,6 +11,7 @@ import shutil
 import os, os.path
 from tempfile import mktemp
 from unittest import TestCase
+import sys
 
 from OpenSSL.crypto import Error, _exception_from_error_queue
 
@@ -88,10 +89,10 @@ class TestCase(TestCase):
         except exception, inst:
             return inst
         except:
-            raise self.failureException('%s raised instead of %s:\n %s'
+            raise self.failureException('%s raised instead of %s'
                                         % (sys.exc_info()[0],
                                            exception.__name__,
-                                           failure.Failure().getTraceback()))
+                                          ))
         else:
             raise self.failureException('%s not raised (%r returned)'
                                         % (exception.__name__, result))