From 07a7c3102c684baa8b364f419c9e9b6c18360f5e Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Thu, 20 Jun 2013 18:49:26 +0200 Subject: [PATCH] Add workaround for OSError raised by Popen.communicate() Python 2.6 can raise OSError when too much data is written to STDIN and the process died prematurely. In the case of keystoneclient this happens during the first cms_verify() call of a process. The calling logic expects a useful error message in order to refetch the CA or singing CERT, which is missing in the case of an OSError. So just fake it instead. Add basic unit tests to cover all of the public methods from keystone.common.cms, raising test coverage to 77%. Add unit test for this specific bug (test_cms_verify_token_no_oserror). Closes-Bug: LP Bug#1235252 Change-Id: I6e650ab9494c605b4e41c78c87a9505e09d5fc29 --- keystoneclient/common/cms.py | 51 +++++++++++++-- keystoneclient/tests/client_fixtures.py | 14 +++- keystoneclient/tests/test_cms.py | 85 +++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 7 deletions(-) diff --git a/keystoneclient/common/cms.py b/keystoneclient/common/cms.py index da0402941..0e6a5b754 100644 --- a/keystoneclient/common/cms.py +++ b/keystoneclient/common/cms.py @@ -21,6 +21,7 @@ If set_subprocess() is not called, this module will pick Python's subprocess or eventlet.green.subprocess based on if os module is patched by eventlet. """ +import errno import hashlib import logging @@ -57,6 +58,46 @@ def set_subprocess(_subprocess=None): subprocess = _subprocess +def _check_files_accessible(files): + err = None + try: + for try_file in files: + with open(try_file, 'r'): + pass + except IOError as e: + # Catching IOError means there is an issue with + # the given file. + err = ('Hit OSError in _process_communicate_handle_oserror()\n' + 'Likely due to %s: %s') % (try_file, e.strerror) + + return err + + +def _process_communicate_handle_oserror(process, text, files): + """Wrapper around process.communicate that checks for OSError.""" + + try: + output, err = process.communicate(text) + except OSError as e: + if e.errno != errno.EPIPE: + raise + # OSError with EPIPE only occurs with Python 2.6.x/old 2.7.x + # http://bugs.python.org/issue10963 + + # The quick exit is typically caused by the openssl command not being + # able to read an input file, so check ourselves if can't read a file. + err = _check_files_accessible(files) + if process.stderr: + err += process.stderr.read() + + output = "" + retcode = -1 + else: + retcode = process.poll() + + return output, err, retcode + + def cms_verify(formatted, signing_cert_file_name, ca_file_name): """Verifies the signature of the contents IAW CMS syntax. @@ -73,8 +114,8 @@ def cms_verify(formatted, signing_cert_file_name, ca_file_name): stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - output, err = process.communicate(formatted) - retcode = process.poll() + output, err, retcode = _process_communicate_handle_oserror( + process, formatted, (signing_cert_file_name, ca_file_name)) # Do not log errors, as some happen in the positive thread # instead, catch them in the calling code and log them there. @@ -184,8 +225,10 @@ def cms_sign_text(text, signing_cert_file_name, signing_key_file_name): stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - output, err = process.communicate(text) - retcode = process.poll() + + output, err, retcode = _process_communicate_handle_oserror( + process, text, (signing_cert_file_name, signing_key_file_name)) + if retcode or "Error" in err: LOG.error('Signing error: %s' % err) raise subprocess.CalledProcessError(retcode, "openssl") diff --git a/keystoneclient/tests/client_fixtures.py b/keystoneclient/tests/client_fixtures.py index dfb6e8a84..1cdf36de9 100644 --- a/keystoneclient/tests/client_fixtures.py +++ b/keystoneclient/tests/client_fixtures.py @@ -28,7 +28,7 @@ TESTDIR = os.path.dirname(os.path.abspath(__file__)) ROOTDIR = os.path.normpath(os.path.join(TESTDIR, '..', '..')) CERTDIR = os.path.join(ROOTDIR, 'examples', 'pki', 'certs') CMSDIR = os.path.join(ROOTDIR, 'examples', 'pki', 'cms') - +KEYDIR = os.path.join(ROOTDIR, 'examples', 'pki', 'private') # @TODO(mordred) This should become a testresources resource attached to the # class @@ -51,9 +51,17 @@ with open(os.path.join(CMSDIR, 'revocation_list.json')) as f: REVOCATION_LIST = jsonutils.loads(f.read()) with open(os.path.join(CMSDIR, 'revocation_list.pem')) as f: SIGNED_REVOCATION_LIST = jsonutils.dumps({'signed': f.read()}) -with open(os.path.join(CERTDIR, 'signing_cert.pem')) as f: + +SIGNING_CERT_FILE = os.path.join(CERTDIR, 'signing_cert.pem') +with open(SIGNING_CERT_FILE) as f: SIGNING_CERT = f.read() -with open(os.path.join(CERTDIR, 'cacert.pem')) as f: + +SIGNING_KEY_FILE = os.path.join(KEYDIR, 'signing_key.pem') +with open(SIGNING_KEY_FILE) as f: + SIGNING_KEY = f.read() + +SIGNING_CA_FILE = os.path.join(CERTDIR, 'cacert.pem') +with open(SIGNING_CA_FILE) as f: SIGNING_CA = f.read() UUID_TOKEN_DEFAULT = "ec6c0710ec2f471498484c1b53ab4f9d" diff --git a/keystoneclient/tests/test_cms.py b/keystoneclient/tests/test_cms.py index 8f9cb73bf..2c9eb7aa9 100644 --- a/keystoneclient/tests/test_cms.py +++ b/keystoneclient/tests/test_cms.py @@ -12,15 +12,100 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import subprocess + +import mock + from keystoneclient.common import cms from keystoneclient import exceptions +from keystoneclient.tests import client_fixtures from keystoneclient.tests import utils class CMSTest(utils.TestCase): + + """Unit tests for the keystoneclient.common.cms module.""" + def test_cms_verify(self): self.assertRaises(exceptions.CertificateConfigError, cms.cms_verify, 'data', 'no_exist_cert_file', 'no_exist_ca_file') + + def test_token_to_cms_to_token(self): + with open(os.path.join(client_fixtures.CMSDIR, + 'auth_token_scoped.pem')) as f: + AUTH_TOKEN_SCOPED_CMS = f.read() + + self.assertEqual(cms.token_to_cms(client_fixtures.SIGNED_TOKEN_SCOPED), + AUTH_TOKEN_SCOPED_CMS) + + tok = cms.cms_to_token(cms.token_to_cms( + client_fixtures.SIGNED_TOKEN_SCOPED)) + self.assertEqual(tok, client_fixtures.SIGNED_TOKEN_SCOPED) + + def test_ans1_token(self): + self.assertTrue(cms.is_ans1_token(client_fixtures.SIGNED_TOKEN_SCOPED)) + self.assertFalse(cms.is_ans1_token('FOOBAR')) + + def test_cms_sign_token_no_files(self): + self.assertRaises(subprocess.CalledProcessError, + cms.cms_sign_token, + client_fixtures.SIGNED_TOKEN_SCOPED, + '/no/such/file', '/no/such/key') + + def test_cms_sign_token_success(self): + self.assertTrue( + cms.cms_sign_token(client_fixtures.SIGNED_TOKEN_SCOPED, + client_fixtures.SIGNING_CERT_FILE, + client_fixtures.SIGNING_KEY_FILE)) + + def test_cms_verify_token_no_files(self): + self.assertRaises(exceptions.CertificateConfigError, + cms.cms_verify, + client_fixtures.SIGNED_TOKEN_SCOPED, + '/no/such/file', '/no/such/key') + + def test_cms_verify_token_no_oserror(self): + import errno + + def raise_OSError(*args): + e = OSError() + e.errno = errno.EPIPE + raise e + + with mock.patch('subprocess.Popen.communicate', new=raise_OSError): + try: + cms.cms_verify("x", '/no/such/file', '/no/such/key') + except subprocess.CalledProcessError as e: + self.assertIn('/no/such/file', e.output) + self.assertIn('Hit OSError ', e.output) + else: + self.fail('Expected subprocess.CalledProcessError') + + def test_cms_verify_token_scoped(self): + cms_content = cms.token_to_cms(client_fixtures.SIGNED_TOKEN_SCOPED) + self.assertTrue(cms.cms_verify(cms_content, + client_fixtures.SIGNING_CERT_FILE, + client_fixtures.SIGNING_CA_FILE)) + + def test_cms_verify_token_scoped_expired(self): + cms_content = cms.token_to_cms( + client_fixtures.SIGNED_TOKEN_SCOPED_EXPIRED) + self.assertTrue(cms.cms_verify(cms_content, + client_fixtures.SIGNING_CERT_FILE, + client_fixtures.SIGNING_CA_FILE)) + + def test_cms_verify_token_unscoped(self): + cms_content = cms.token_to_cms(client_fixtures.SIGNED_TOKEN_UNSCOPED) + self.assertTrue(cms.cms_verify(cms_content, + client_fixtures.SIGNING_CERT_FILE, + client_fixtures.SIGNING_CA_FILE)) + + def test_cms_verify_token_v3_scoped(self): + cms_content = cms.token_to_cms(client_fixtures.SIGNED_v3_TOKEN_SCOPED) + self.assertTrue(cms.cms_verify(cms_content, + client_fixtures.SIGNING_CERT_FILE, + client_fixtures.SIGNING_CA_FILE))