Adjust the x509 signing API to return text instead of bytes

Instead of returning ascii-encoded bytes, return the signature and certificate as text so it can be JSON-serialized in Python 3. fixes #495 Signed-off-by: Jeremy Cline <[email protected]>
fedora-infra · Dec 8, 2017 · 78c0ffa · 78c0ffa
1 parent b8214e7
commit 78c0ffa
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 60 deletions.
diff --git a/fedmsg/crypto/x509.py b/fedmsg/crypto/x509.py
@@ -86,8 +86,8 @@ def _m2crypto_sign(message, ssldir=None, certname=None, **config):
     # Return a new dict containing the pairs in the original message as well
     # as the new authn fields.
     return dict(message.items() + [
-        ('signature', signature.encode('base64')),
-        ('certificate', certificate.encode('base64')),
+        ('signature', signature.encode('base64').decode('ascii')),
+        ('certificate', certificate.encode('base64').decode('ascii')),
     ])
 
 

diff --git a/fedmsg/crypto/x509_ng.py b/fedmsg/crypto/x509_ng.py
@@ -92,8 +92,8 @@ def sign(message, ssldir=None, certname=None, **config):
         cert_pem = cert.public_bytes(serialization.Encoding.PEM)
 
     return _prep_crypto_msg(dict(list(message.items()) + [
-        ('signature', base64.b64encode(signature)),
-        ('certificate', base64.b64encode(cert_pem)),
+        ('signature', base64.b64encode(signature).decode('ascii')),
+        ('certificate', base64.b64encode(cert_pem).decode('ascii')),
     ]))
 
 
@@ -119,8 +119,8 @@ def _prep_crypto_msg(message):
     for x in range(0, len(certificate), 76):
         sliced_certificate.append(certificate[x:x+76])
 
-    message['signature'] = b'\n'.join(sliced_signature) + b'\n'
-    message['certificate'] = b'\n'.join(sliced_certificate) + b'\n'
+    message['signature'] = u'\n'.join(sliced_signature) + u'\n'
+    message['certificate'] = u'\n'.join(sliced_certificate) + u'\n'
     return message
 
 

diff --git a/fedmsg/tests/consumers/test_relay.py b/fedmsg/tests/consumers/test_relay.py
@@ -12,34 +12,34 @@ class TestSigningRelayConsumer(unittest.TestCase):
     def setUp(self):
         self.hub = mock.Mock()
         self.signing_cert = (
-            b'LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVVekNDQTd5Z0F3SUJBZ0lCRHpBTkJna3Fo\n'
-            b'a2lHOXcwQkFRVUZBRENCb0RFTE1Ba0dBMVVFQmhNQ1ZWTXgKQ3pBSkJnTlZCQWdUQWs1RE1SQXdE\n'
-            b'Z1lEVlFRSEV3ZFNZV3hsYVdkb01SY3dGUVlEVlFRS0V3NUdaV1J2Y21FZwpVSEp2YW1WamRERVBN\n'
-            b'QTBHQTFVRUN4TUdabVZrYlhObk1ROHdEUVlEVlFRREV3Wm1aV1J0YzJjeER6QU5CZ05WCkJDa1RC\n'
-            b'bVpsWkcxelp6RW1NQ1FHQ1NxR1NJYjNEUUVKQVJZWFlXUnRhVzVBWm1Wa2IzSmhjSEp2YW1WamRD\n'
-            b'NXYKY21jd0hoY05NVEl3TnpFMU1qRXhPRFV5V2hjTk1qSXdOekV6TWpFeE9EVXlXakNCNGpFTE1B\n'
-            b'a0dBMVVFQmhNQwpWVk14Q3pBSkJnTlZCQWdUQWs1RE1SQXdEZ1lEVlFRSEV3ZFNZV3hsYVdkb01S\n'
-            b'Y3dGUVlEVlFRS0V3NUdaV1J2CmNtRWdVSEp2YW1WamRERVBNQTBHQTFVRUN4TUdabVZrYlhObk1U\n'
-            b'QXdMZ1lEVlFRREV5ZHphR1ZzYkMxd1lXTnIKWVdkbGN6QXhMbkJvZURJdVptVmtiM0poY0hKdmFt\n'
-            b'VmpkQzV2Y21jeE1EQXVCZ05WQkNrVEozTm9aV3hzTFhCaApZMnRoWjJWek1ERXVjR2g0TWk1bVpX\n'
-            b'UnZjbUZ3Y205cVpXTjBMbTl5WnpFbU1DUUdDU3FHU0liM0RRRUpBUllYCllXUnRhVzVBWm1Wa2Iz\n'
-            b'SmhjSEp2YW1WamRDNXZjbWN3Z1o4d0RRWUpLb1pJaHZjTkFRRUJCUUFEZ1kwQU1JR0oKQW9HQkFN\n'
-            b'RUlKNURzZ0VsaG5XMENLcnNpc1UvV0svUFBrSkNST0N0WnBwQXZha0dDVHhVU1RoWDhpZmVsVjVa\n'
-            b'dwp1T1dCWDlxTHg2cGJzNHhodnVrVDkwUHphYUlKR24xeUpjVnZLTDYzS1I1SCtZNXdOamJLREhY\n'
-            b'ZlBuM0J1Z0hSCmRzdnV0Yi9Fa3hNM3NYbnRpZWY0K2ZWVGsyanZiTXFsYmEvWHc4cXBsRWxqMXFm\n'
-            b'aEFnTUJBQUdqZ2dGWE1JSUIKVXpBSkJnTlZIUk1FQWpBQU1DMEdDV0NHU0FHRytFSUJEUVFnRmg1\n'
-            b'RllYTjVMVkpUUVNCSFpXNWxjbUYwWldRZwpRMlZ5ZEdsbWFXTmhkR1V3SFFZRFZSME9CQllFRkUw\n'
-            b'Zmh6czZhWjViVDJVNjZzUjNrUG1LdzBGYk1JSFZCZ05WCkhTTUVnYzB3Z2NxQUZBQ1lwZFhueEZV\n'
-            b'T2hLTm4vbVpLRnVBRUZkMGhvWUdtcElHak1JR2dNUXN3Q1FZRFZRUUcKRXdKVlV6RUxNQWtHQTFV\n'
-            b'RUNCTUNUa014RURBT0JnTlZCQWNUQjFKaGJHVnBaMmd4RnpBVkJnTlZCQW9URGtabApaRzl5WVNC\n'
-            b'UWNtOXFaV04wTVE4d0RRWURWUVFMRXdabVpXUnRjMmN4RHpBTkJnTlZCQU1UQm1abFpHMXpaekVQ\n'
-            b'Ck1BMEdBMVVFS1JNR1ptVmtiWE5uTVNZd0pBWUpLb1pJaHZjTkFRa0JGaGRoWkcxcGJrQm1aV1J2\n'
-            b'Y21Gd2NtOXEKWldOMExtOXlaNElKQUk3cktOaXBFNTE4TUJNR0ExVWRKUVFNTUFvR0NDc0dBUVVG\n'
-            b'QndNQ01Bc0dBMVVkRHdRRQpBd0lIZ0RBTkJna3Foa2lHOXcwQkFRVUZBQU9CZ1FCK3RlWFNCV0pQ\n'
-            b'VWlLMDBEYWl4RmF6ZThSUW01S1ZBQjBRCkRSdDdqcDdRcVViZHd2ZWhvU3NKODVDYnZLazhYZ0Ey\n'
-            b'UW16RFdhRzRhcklrQUVCWGFkNjlyMkZmMTMzTmQxQlEKeGZGRGRWdXFyeE9HeXJwazhyOFAxYmJJ\n'
-            b'YjRNb09aUVQxbGFGTFUzZjNJUVNIYW93RkRuZ0V0azlZUzRpSEhrWQora3FlRnczYmhRPT0KLS0t\n'
-            b'LS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=\n'
+            u'LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVVekNDQTd5Z0F3SUJBZ0lCRHpBTkJna3Fo\n'
+            u'a2lHOXcwQkFRVUZBRENCb0RFTE1Ba0dBMVVFQmhNQ1ZWTXgKQ3pBSkJnTlZCQWdUQWs1RE1SQXdE\n'
+            u'Z1lEVlFRSEV3ZFNZV3hsYVdkb01SY3dGUVlEVlFRS0V3NUdaV1J2Y21FZwpVSEp2YW1WamRERVBN\n'
+            u'QTBHQTFVRUN4TUdabVZrYlhObk1ROHdEUVlEVlFRREV3Wm1aV1J0YzJjeER6QU5CZ05WCkJDa1RC\n'
+            u'bVpsWkcxelp6RW1NQ1FHQ1NxR1NJYjNEUUVKQVJZWFlXUnRhVzVBWm1Wa2IzSmhjSEp2YW1WamRD\n'
+            u'NXYKY21jd0hoY05NVEl3TnpFMU1qRXhPRFV5V2hjTk1qSXdOekV6TWpFeE9EVXlXakNCNGpFTE1B\n'
+            u'a0dBMVVFQmhNQwpWVk14Q3pBSkJnTlZCQWdUQWs1RE1SQXdEZ1lEVlFRSEV3ZFNZV3hsYVdkb01S\n'
+            u'Y3dGUVlEVlFRS0V3NUdaV1J2CmNtRWdVSEp2YW1WamRERVBNQTBHQTFVRUN4TUdabVZrYlhObk1U\n'
+            u'QXdMZ1lEVlFRREV5ZHphR1ZzYkMxd1lXTnIKWVdkbGN6QXhMbkJvZURJdVptVmtiM0poY0hKdmFt\n'
+            u'VmpkQzV2Y21jeE1EQXVCZ05WQkNrVEozTm9aV3hzTFhCaApZMnRoWjJWek1ERXVjR2g0TWk1bVpX\n'
+            u'UnZjbUZ3Y205cVpXTjBMbTl5WnpFbU1DUUdDU3FHU0liM0RRRUpBUllYCllXUnRhVzVBWm1Wa2Iz\n'
+            u'SmhjSEp2YW1WamRDNXZjbWN3Z1o4d0RRWUpLb1pJaHZjTkFRRUJCUUFEZ1kwQU1JR0oKQW9HQkFN\n'
+            u'RUlKNURzZ0VsaG5XMENLcnNpc1UvV0svUFBrSkNST0N0WnBwQXZha0dDVHhVU1RoWDhpZmVsVjVa\n'
+            u'dwp1T1dCWDlxTHg2cGJzNHhodnVrVDkwUHphYUlKR24xeUpjVnZLTDYzS1I1SCtZNXdOamJLREhY\n'
+            u'ZlBuM0J1Z0hSCmRzdnV0Yi9Fa3hNM3NYbnRpZWY0K2ZWVGsyanZiTXFsYmEvWHc4cXBsRWxqMXFm\n'
+            u'aEFnTUJBQUdqZ2dGWE1JSUIKVXpBSkJnTlZIUk1FQWpBQU1DMEdDV0NHU0FHRytFSUJEUVFnRmg1\n'
+            u'RllYTjVMVkpUUVNCSFpXNWxjbUYwWldRZwpRMlZ5ZEdsbWFXTmhkR1V3SFFZRFZSME9CQllFRkUw\n'
+            u'Zmh6czZhWjViVDJVNjZzUjNrUG1LdzBGYk1JSFZCZ05WCkhTTUVnYzB3Z2NxQUZBQ1lwZFhueEZV\n'
+            u'T2hLTm4vbVpLRnVBRUZkMGhvWUdtcElHak1JR2dNUXN3Q1FZRFZRUUcKRXdKVlV6RUxNQWtHQTFV\n'
+            u'RUNCTUNUa014RURBT0JnTlZCQWNUQjFKaGJHVnBaMmd4RnpBVkJnTlZCQW9URGtabApaRzl5WVNC\n'
+            u'UWNtOXFaV04wTVE4d0RRWURWUVFMRXdabVpXUnRjMmN4RHpBTkJnTlZCQU1UQm1abFpHMXpaekVQ\n'
+            u'Ck1BMEdBMVVFS1JNR1ptVmtiWE5uTVNZd0pBWUpLb1pJaHZjTkFRa0JGaGRoWkcxcGJrQm1aV1J2\n'
+            u'Y21Gd2NtOXEKWldOMExtOXlaNElKQUk3cktOaXBFNTE4TUJNR0ExVWRKUVFNTUFvR0NDc0dBUVVG\n'
+            u'QndNQ01Bc0dBMVVkRHdRRQpBd0lIZ0RBTkJna3Foa2lHOXcwQkFRVUZBQU9CZ1FCK3RlWFNCV0pQ\n'
+            u'VWlLMDBEYWl4RmF6ZThSUW01S1ZBQjBRCkRSdDdqcDdRcVViZHd2ZWhvU3NKODVDYnZLazhYZ0Ey\n'
+            u'UW16RFdhRzRhcklrQUVCWGFkNjlyMkZmMTMzTmQxQlEKeGZGRGRWdXFyeE9HeXJwazhyOFAxYmJJ\n'
+            u'YjRNb09aUVQxbGFGTFUzZjNJUVNIYW93RkRuZ0V0azlZUzRpSEhrWQora3FlRnczYmhRPT0KLS0t\n'
+            u'LS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=\n'
         )
         self.hub.config = {
             'fedmsg.consumers.relay.enabled': True,
@@ -63,9 +63,9 @@ def test_message_signed(self):
         expected_msg = {
             'my': 'msg',
             'crypto': 'x509',
-            'signature': (b'kyZ496SD+qgufonX9lqV/4L/o3s0+j4j5RaeMzhRIIGhfk6/RIEtl1DW73xbo+'
-                          b'Xs2STbidFyz7Yt\n6IUb3/U+8Io0CTTbIyQvcvtof/a3EdmbnZtOQ93VfnXXkn'
-                          b'6m76yVcFnQDicagY/600KmfNCDAwve\nI6+B9va/q10CBloMLkE=\n'),
+            'signature': (u'kyZ496SD+qgufonX9lqV/4L/o3s0+j4j5RaeMzhRIIGhfk6/RIEtl1DW73xbo+'
+                          u'Xs2STbidFyz7Yt\n6IUb3/U+8Io0CTTbIyQvcvtof/a3EdmbnZtOQ93VfnXXkn'
+                          u'6m76yVcFnQDicagY/600KmfNCDAwve\nI6+B9va/q10CBloMLkE=\n'),
             'certificate': self.signing_cert,
         }
         consumer = relay.SigningRelayConsumer(self.hub)

diff --git a/fedmsg/tests/crypto/test_x509.py b/fedmsg/tests/crypto/test_x509.py
@@ -29,7 +29,7 @@
 except ImportError:
     from unittest2 import skipIf, expectedFailure
 
-from fedmsg import crypto  # noqa: E402
+from fedmsg import crypto, encoding as fedmsg_encoding  # noqa: E402
 from fedmsg.crypto.x509 import _m2crypto
 from fedmsg.crypto.x509_ng import _cryptography
 from fedmsg.tests.base import SSLDIR, FedmsgTestCase
@@ -69,13 +69,11 @@ def test_sign(self):
         self.assertTrue('signature' in signed)
         self.assertTrue('certificate' in signed)
 
-    @expectedFailure
     def test_signature_text(self):
         """Assert signature fields are unicode text."""
         signed = self.sign({'my': 'message'}, **self.config)
         self.assertTrue(isinstance(signed['signature'], six.text_type))
 
-    @expectedFailure
     def test_certificate_text(self):
         """Assert signing a message inserts the certificate and a signature."""
         signed = self.sign({'my': 'message'}, **self.config)
@@ -212,11 +210,6 @@ def test_bytes_undecodable(self):
         """Assert un-decodable signatures/certificates fails validation."""
         signed = self.sign({'topic': 'mytopic', 'message': 'so secure'}, **self.config)
 
-        # This assertion will fail when the sign API changes to return text types, at
-        # which point this test should encode the signature to bytes.
-        self.assertTrue(isinstance(signed['signature'], six.binary_type))
-        self.assertTrue(isinstance(signed['certificate'], six.binary_type))
-
         # This is a non-ascii character that encodes to a bytestring in latin-1
         # that won't decode in UTF-8
         signed['signature'] = u'Ö'.encode('latin-1')
@@ -228,15 +221,16 @@ def test_text_validate(self):
         """Assert unicode-type signatures/certificates work."""
         signed = self.sign({'topic': 'mytopic', 'message': 'so secure'}, **self.config)
 
-        # This assertion will fail when the sign API changes to return text types, at
-        # which point this test should drop the decode step.
-        self.assertTrue(isinstance(signed['signature'], six.binary_type))
-        self.assertTrue(isinstance(signed['certificate'], six.binary_type))
-        signed['signature'] = signed['signature'].decode('utf-8')
-        signed['certificate'] = signed['certificate'].decode('utf-8')
-
+        self.assertTrue(isinstance(signed['signature'], six.text_type))
+        self.assertTrue(isinstance(signed['certificate'], six.text_type))
         self.assertTrue(self.validate(signed, **self.config))
 
+    def test_serializeable(self):
+        """Assert signed messages are serializable."""
+        signed = self.sign({'topic': 'mytopic', 'message': 'so secure'}, **self.config)
+
+        fedmsg_encoding.dumps(signed)
+
 
 @skipIf(not _cryptography, "cryptography/pyOpenSSL are missing.")
 class X509CryptographyTests(X509BaseTests):
@@ -271,10 +265,10 @@ def test_bytes_logs_error(self, mock_log):
         """Assert calling validate with byte signature/certificate logs an error."""
         signed = self.sign({'topic': 'mytopic', 'message': 'so secure'}, **self.config)
 
-        # This assertion will fail when the sign API changes to return text types, at
-        # which point this test should encode the signature to bytes.
-        self.assertTrue(isinstance(signed['signature'], six.binary_type))
-        self.assertTrue(isinstance(signed['certificate'], six.binary_type))
+        self.assertTrue(isinstance(signed['signature'], six.text_type))
+        self.assertTrue(isinstance(signed['certificate'], six.text_type))
+        signed['signature'] = signed['signature'].encode('utf-8')
+        signed['certificate'] = signed['certificate'].encode('utf-8')
 
         self.assertTrue(self.validate(signed, **self.config))
         mock_log.error.assert_any_call("msg['signature'] is not a unicode string")
@@ -316,10 +310,10 @@ def test_bytes_logs_error(self, mock_log):
         """Assert calling validate with byte signature/certificate logs an error."""
         signed = self.sign({'topic': 'mytopic', 'message': 'so secure'}, **self.config)
 
-        # This assertion will fail when the sign API changes to return text types, at
-        # which point this test should encode the signature to bytes.
-        self.assertTrue(isinstance(signed['signature'], six.binary_type))
-        self.assertTrue(isinstance(signed['certificate'], six.binary_type))
+        self.assertTrue(isinstance(signed['signature'], six.text_type))
+        self.assertTrue(isinstance(signed['certificate'], six.text_type))
+        signed['signature'] = signed['signature'].encode('utf-8')
+        signed['certificate'] = signed['certificate'].encode('utf-8')
 
         self.assertTrue(self.validate(signed, **self.config))
         mock_log.error.assert_any_call("msg['signature'] is not a unicode string")