From ebfaf10357a0ff5f635e9ff9092adae1338f7415 Mon Sep 17 00:00:00 2001 From: Dolph Mathews Date: Wed, 9 Apr 2014 08:09:09 -0500 Subject: [PATCH] eliminate race condition fetching certs There's a race between the time that a CertificateConfigError is raised, and when we check to see what caused it. Eliminating the checks and unconditionally fetching certificates eliminates the race. Giant thanks to Jamie Lennox for identifying the root cause described above! Co-Authored-By: David Stanek Change-Id: I19113496ceaecdc03e209d550e0db156df95f9b8 Closes-Bug: 1285833 --- keystoneclient/middleware/auth_token.py | 39 ++++++++++--------- .../tests/test_auth_token_middleware.py | 8 ---- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/keystoneclient/middleware/auth_token.py b/keystoneclient/middleware/auth_token.py index b1a8515e9..9c5a709eb 100644 --- a/keystoneclient/middleware/auth_token.py +++ b/keystoneclient/middleware/auth_token.py @@ -1125,9 +1125,6 @@ class AuthProtocol(object): self.LOG.debug('Marking token as unauthorized in cache') self._cache_store(token_id, 'invalid') - def cert_file_missing(self, proc_output, file_name): - return (file_name in proc_output and not os.path.exists(file_name)) - def verify_uuid_token(self, user_token, retry=True): """Authenticate user token with keystone. @@ -1201,28 +1198,32 @@ class AuthProtocol(object): def cms_verify(self, data): """Verifies the signature of the provided data's IAW CMS syntax. - If either of the certificate files are missing, fetch them and + If either of the certificate files might be missing, fetch them and retry. """ - while True: + def verify(): try: - output = cms.cms_verify(data, self.signing_cert_file_name, - self.signing_ca_file_name) - except exceptions.CertificateConfigError as err: - if self.cert_file_missing(err.output, - self.signing_cert_file_name): - self.fetch_signing_cert() - continue - if self.cert_file_missing(err.output, - self.signing_ca_file_name): - self.fetch_ca_cert() - continue - self.LOG.error('CMS Verify output: %s', err.output) - raise + return cms.cms_verify(data, self.signing_cert_file_name, + self.signing_ca_file_name) except cms.subprocess.CalledProcessError as err: self.LOG.warning('Verify error: %s', err) raise - return output + + try: + return verify() + except exceptions.CertificateConfigError: + # the certs might be missing; unconditionally fetch to avoid racing + self.fetch_signing_cert() + self.fetch_ca_cert() + + try: + # retry with certs in place + return verify() + except exceptions.CertificateConfigError as err: + # if this is still occurring, something else is wrong and we + # need err.output to identify the problem + self.LOG.error('CMS Verify output: %s', err.output) + raise def verify_signed_token(self, signed_text): """Check that the token is unrevoked and has a valid signature.""" diff --git a/keystoneclient/tests/test_auth_token_middleware.py b/keystoneclient/tests/test_auth_token_middleware.py index 5e52268cc..7dc1d388a 100644 --- a/keystoneclient/tests/test_auth_token_middleware.py +++ b/keystoneclient/tests/test_auth_token_middleware.py @@ -594,14 +594,6 @@ class CommonAuthTokenMiddlewareTest(object): stat.S_IRWXU) shutil.rmtree(test_parent_signing_dir) - def test_cert_file_missing(self): - self.assertFalse(self.middleware.cert_file_missing( - "openstack: /tmp/haystack: No such file or directory", - "/tmp/needle")) - self.assertTrue(self.middleware.cert_file_missing( - "openstack: /not/exist: No such file or directory", - "/not/exist")) - def test_get_token_revocation_list_fetched_time_returns_min(self): self.middleware.token_revocation_list_fetched_time = None self.middleware.revoked_file_name = ''