Merge lp:~shanemhansen/pyopenssl/dump_publickey into lp:~exarkun/pyopenssl/trunk

Proposed by Shane Hansen
Status: Needs review
Proposed branch: lp:~shanemhansen/pyopenssl/dump_publickey
Merge into: lp:~exarkun/pyopenssl/trunk
Diff against target: 180 lines (+119/-1)
3 files modified
OpenSSL/crypto/crypto.c (+56/-0)
OpenSSL/test/test_crypto.py (+54/-1)
doc/api/crypto.rst (+9/-0)
To merge this branch: bzr merge lp:~shanemhansen/pyopenssl/dump_publickey
Reviewer Review Type Date Requested Status
Jean-Paul Calderone Needs Fixing
Review via email: mp+125370@code.launchpad.net

Description of the change

This change implements the crypto.dump_publickey API. There are tests for ASN1 and PEM formatting.
There seem to be a number of api's for dumping public keys but I was able to match the PEM and ASN1 output with the openssl pkey tool which uses the PEM_write_bio_PUBKEY and i2d_PUBKEY_bio apis.

This is my first attempt at submitting something like this, so let me know on #pyopenssl if there's anything I can add. Thanks!

To post a comment you must log in.
Revision history for this message
Jean-Paul Calderone (exarkun) wrote :

Thanks. Some observations:

  1. The switch in crypto_dump_publickey has a default case that raises an exception with a message talking about FILETYPE_TEXT being a valid input, but there is no FILETYPE_TEXT case in the switch statement.
  2. There's a `print` statement left in test_dump_publickey_valid_filetype
  3. I'd like to see test_dump_publickey_valid_filetype split into two tests, one for FILETYPE_ASN1 and one for FILETYPE_PEM.
  4. There's a bit of trailing whitespace after test_dump_publickey_valid_filetype which should be deleted.
  5. All APIs should be documented in doc/api/ as well.

Otherwise this looks good. Thanks!

review: Needs Fixing
Revision history for this message
Shane Hansen (shanemhansen) wrote :

Thanks for the feedback, I've made the requested changes and repushed.
Let me know if there are any steps I've missed.

On Sat, Oct 20, 2012 at 6:25 PM, Jean-Paul Calderone <
<email address hidden>> wrote:

> The proposal to merge lp:~shanemhansen/pyopenssl/dump_publickey into
> lp:pyopenssl has been updated.
>
> Status: Needs review => Work in progress
>
> For more details, see:
>
> https://code.launchpad.net/~shanemhansen/pyopenssl/dump_publickey/+merge/125370
> --
>
> https://code.launchpad.net/~shanemhansen/pyopenssl/dump_publickey/+merge/125370
> You are the owner of lp:~shanemhansen/pyopenssl/dump_publickey.
>

Unmerged revisions

170. By Shane Hansen

Remove spurious print statement, incorrect exception statement,
and split up tests for dump_publickey.

169. By Space Monkey

Implement crypto.dump_publickey API (with tests)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'OpenSSL/crypto/crypto.c'
--- OpenSSL/crypto/crypto.c 2009-12-22 16:02:04 +0000
+++ OpenSSL/crypto/crypto.c 2012-10-21 20:06:21 +0000
@@ -171,6 +171,60 @@
171 return (PyObject *)crypto_PKey_New(pkey, 1);171 return (PyObject *)crypto_PKey_New(pkey, 1);
172}172}
173173
174static char crypto_dump_publickey_doc[] = "\n\
175Dump a public key to a buffer\n\
176\n\
177:param type: The file type (one of FILETYPE_PEM, FILETYPE_ASN1)\n\
178:param pkey: The PKey to dump\n\
179:return: The buffer with the dumped key in\n\
180:rtype: :py:data:`str`\n\
181";
182
183static PyObject *
184crypto_dump_publickey(PyObject *spam, PyObject *args)
185{
186 int type, ret, buf_len;
187 char *temp;
188 PyObject *buffer;
189 BIO *bio;
190 crypto_PKeyObj *pkey;
191
192 if (!PyArg_ParseTuple(args, "iO!:dump_publickey", &type,
193 &crypto_PKey_Type, &pkey)) {
194 return NULL;
195 }
196 bio = BIO_new(BIO_s_mem());
197 if (bio == NULL) {
198 exception_from_error_queue(crypto_Error);
199 return NULL;
200 }
201 switch (type) {
202 case X509_FILETYPE_PEM:
203 ret = PEM_write_bio_PUBKEY(bio, pkey->pkey);
204 break;
205
206 case X509_FILETYPE_ASN1:
207 ret = i2d_PUBKEY_bio(bio, pkey->pkey);
208 break;
209
210 default:
211 PyErr_SetString(PyExc_ValueError, "type argument must be FILETYPE_PEM, FILETYPE_ASN1");
212 BIO_free(bio);
213 return NULL;
214 }
215
216 if (ret == 0) {
217 BIO_free(bio);
218 return raise_current_error();
219 }
220
221 buf_len = BIO_get_mem_data(bio, &temp);
222 buffer = PyBytes_FromStringAndSize(temp, buf_len);
223 BIO_free(bio);
224
225 return buffer;
226}
227
174static char crypto_dump_privatekey_doc[] = "\n\228static char crypto_dump_privatekey_doc[] = "\n\
175Dump a private key to a buffer\n\229Dump a private key to a buffer\n\
176\n\230\n\
@@ -185,6 +239,7 @@
185:rtype: :py:data:`str`\n\239:rtype: :py:data:`str`\n\
186";240";
187241
242
188static PyObject *243static PyObject *
189crypto_dump_privatekey(PyObject *spam, PyObject *args)244crypto_dump_privatekey(PyObject *spam, PyObject *args)
190{245{
@@ -733,6 +788,7 @@
733 /* Module functions */788 /* Module functions */
734 { "load_privatekey", (PyCFunction)crypto_load_privatekey, METH_VARARGS, crypto_load_privatekey_doc },789 { "load_privatekey", (PyCFunction)crypto_load_privatekey, METH_VARARGS, crypto_load_privatekey_doc },
735 { "dump_privatekey", (PyCFunction)crypto_dump_privatekey, METH_VARARGS, crypto_dump_privatekey_doc },790 { "dump_privatekey", (PyCFunction)crypto_dump_privatekey, METH_VARARGS, crypto_dump_privatekey_doc },
791 { "dump_publickey", (PyCFunction)crypto_dump_publickey, METH_VARARGS, crypto_dump_publickey_doc },
736 { "load_certificate", (PyCFunction)crypto_load_certificate, METH_VARARGS, crypto_load_certificate_doc },792 { "load_certificate", (PyCFunction)crypto_load_certificate, METH_VARARGS, crypto_load_certificate_doc },
737 { "dump_certificate", (PyCFunction)crypto_dump_certificate, METH_VARARGS, crypto_dump_certificate_doc },793 { "dump_certificate", (PyCFunction)crypto_dump_certificate, METH_VARARGS, crypto_dump_certificate_doc },
738 { "load_certificate_request", (PyCFunction)crypto_load_certificate_request, METH_VARARGS, crypto_load_certificate_request_doc },794 { "load_certificate_request", (PyCFunction)crypto_load_certificate_request, METH_VARARGS, crypto_load_certificate_request_doc },
739795
=== modified file 'OpenSSL/test/test_crypto.py'
--- OpenSSL/test/test_crypto.py 2012-03-09 22:58:00 +0000
+++ OpenSSL/test/test_crypto.py 2012-10-21 20:06:21 +0000
@@ -18,7 +18,8 @@
18from OpenSSL.crypto import load_certificate, load_privatekey18from OpenSSL.crypto import load_certificate, load_privatekey
19from OpenSSL.crypto import FILETYPE_PEM, FILETYPE_ASN1, FILETYPE_TEXT19from OpenSSL.crypto import FILETYPE_PEM, FILETYPE_ASN1, FILETYPE_TEXT
20from OpenSSL.crypto import dump_certificate, load_certificate_request20from OpenSSL.crypto import dump_certificate, load_certificate_request
21from OpenSSL.crypto import dump_certificate_request, dump_privatekey21from OpenSSL.crypto import dump_certificate_request, dump_privatekey, dump_publickey
22from OpenSSL.crypto import dump_publickey
22from OpenSSL.crypto import PKCS7Type, load_pkcs7_data23from OpenSSL.crypto import PKCS7Type, load_pkcs7_data
23from OpenSSL.crypto import PKCS12, PKCS12Type, load_pkcs1224from OpenSSL.crypto import PKCS12, PKCS12Type, load_pkcs12
24from OpenSSL.crypto import CRL, Revoked, load_crl25from OpenSSL.crypto import CRL, Revoked, load_crl
@@ -71,6 +72,24 @@
71-----END RSA PRIVATE KEY-----72-----END RSA PRIVATE KEY-----
72""")73""")
7374
75#Generated from `openssl pkey -pubout` on root_key_pem
76root_public_key_pem = b("""-----BEGIN PUBLIC KEY-----
77MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQD5mkLpi7q6ROdu7khB3S9aanA0
78Zls7vvfGOmB80/yeylhGpsjAjWen0VtSQke/NlEPGtO38tsV7CsuFnSmschvAnGr
79cJl76b0UOOHUgDTIoRxC6QDU3claegwsrBA+sJEBbqx5RdXbIRGicPG/8qQ4Zm1S
80KOgotcbwiaor2yxZ2wIDAQAB
81-----END PUBLIC KEY-----
82""")
83#Generated from `openssl pkey -pubout -outform DER` on root_key_pem
84root_public_key_asn1 = b("""0\x81\x9f0\r\x06\t*\x86H\x86\xf7\r\x01\
85\x01\x01\x05\x00\x03\x81\x8d\x000\x81\x89\x02\x81\x81\x00\xf9\x9aB\
86\xe9\x8b\xba\xbaD\xe7n\xeeHA\xdd/Zjp4f[;\xbe\xf7\xc6:`|\xd3\xfc\x9e\
87\xcaXF\xa6\xc8\xc0\x8dg\xa7\xd1[RBG\xbf6Q\x0f\x1a\xd3\xb7\xf2\xdb\
88\x15\xec+.\x16t\xa6\xb1\xc8o\x02q\xabp\x99{\xe9\xbd\x148\xe1\xd4\
89\x804\xc8\xa1\x1cB\xe9\x00\xd4\xdd\xc9Zz\x0c,\xac\x10>\xb0\x91\x01n\
90\xacyE\xd5\xdb!\x11\xa2p\xf1\xbf\xf2\xa48fmR(\xe8(\xb5\xc6\xf0\x89\
91\xaa+\xdb,Y\xdb\x02\x03\x01\x00\x01""")
92
74server_cert_pem = b("""-----BEGIN CERTIFICATE-----93server_cert_pem = b("""-----BEGIN CERTIFICATE-----
75MIICKDCCAZGgAwIBAgIJAJn/HpR21r/8MA0GCSqGSIb3DQEBBQUAMFgxCzAJBgNV94MIICKDCCAZGgAwIBAgIJAJn/HpR21r/8MA0GCSqGSIb3DQEBBQUAMFgxCzAJBgNV
76BAYTAlVTMQswCQYDVQQIEwJJTDEQMA4GA1UEBxMHQ2hpY2FnbzEQMA4GA1UEChMH95BAYTAlVTMQswCQYDVQQIEwJJTDEQMA4GA1UEBxMHQ2hpY2FnbzEQMA4GA1UEChMH
@@ -2069,6 +2088,40 @@
2069 load_privatekey,2088 load_privatekey,
2070 FILETYPE_PEM, encryptedPrivateKeyPEM, lambda *args: 3)2089 FILETYPE_PEM, encryptedPrivateKeyPEM, lambda *args: 3)
20712090
2091 def test_dump_publickey_wrong_args(self):
2092 """
2093 :py:obj:`dump_publickey` raises :py:obj:`TypeError` if called with the wrong number
2094 of arguments.
2095 """
2096 self.assertRaises(TypeError, dump_publickey)
2097
2098 def test_dump_publickey_invalid_filetype(self):
2099 """
2100 :py:obj:`dump_publickey` raises :py:obj:`ValueError` if called with an unrecognized
2101 filetype.
2102 """
2103 key = PKey()
2104 key.generate_key(TYPE_RSA, 512)
2105 self.assertRaises(ValueError, dump_publickey, 100, key)
2106
2107 def test_dump_publickey_valid_filetype_asn1(self):
2108 """
2109 :py:obj:`dump_publickey` can successfully dump an RSA public key
2110 in ASN1 format
2111 """
2112 key = load_privatekey(FILETYPE_PEM, root_key_pem)
2113 buf = dump_publickey(FILETYPE_ASN1, key)
2114 self.assertEqual(buf, root_public_key_asn1,
2115 "Public key matches that from openssl DER")
2116
2117 def test_dump_publickey_valid_filetype_PEM(self):
2118 """
2119 :py:obj:`dump_publickey` can successfully dump an RSA public key.
2120 """
2121 key = load_privatekey(FILETYPE_PEM, root_key_pem)
2122 buf = dump_publickey(FILETYPE_PEM, key)
2123 self.assertEqual(buf, root_public_key_pem,
2124 "Public key matches that from openssl PEM")
20722125
2073 def test_dump_privatekey_wrong_args(self):2126 def test_dump_privatekey_wrong_args(self):
2074 """2127 """
20752128
=== modified file 'doc/api/crypto.rst'
--- doc/api/crypto.rst 2012-09-12 14:49:07 +0000
+++ doc/api/crypto.rst 2012-10-21 20:06:21 +0000
@@ -141,6 +141,15 @@
141 pass phrase.141 pass phrase.
142142
143143
144.. py:function:: dump_publickey(type, pkey)
145
146 Dump the publickey key *pkey* into a buffer string encoded with the type
147 *type*.
148
149 *type* is one of (:py:const:`FILETYPE_PEM`, :py:const:`FILETYPE_ASN1`).
150 *pkey* is a :py:class:`PKey` instance.
151
152
144.. py:function:: load_certificate(type, buffer)153.. py:function:: load_certificate(type, buffer)
145154
146 Load a certificate (X509) from the string *buffer* encoded with the155 Load a certificate (X509) from the string *buffer* encoded with the

Subscribers

People subscribed via source and target branches

to status/vote changes: