openstack/python-keystoneclient
Code Issues Proposed changes

Add workaround for OSError raised by Popen.communicate()

Browse Source 
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
This commit is contained in:
Dirk Mueller
2013-06-20 18:49:26 +02:00
parent 30ab23854c
commit 07a7c3102c
3 changed files with 143 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
keystoneclient/common/cms.py
View File

@@ -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")

14
keystoneclient/tests/client_fixtures.py
View File

@@ -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"

85
keystoneclient/tests/test_cms.py
View File

@@ -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))