URL: https://github.com/freeipa/freeipa/pull/616
Author: tiran
Title: #616: Simplify KRA transport cert cache
Action: opened
PR body:
"""
In-memory cache causes problem in forking servers. A file based cache is
good enough. It's easier to understand and avoids performance regression
and synchronization issues when cert becomes out-of-date.
Signed-off-by: Christian Heimes <chei...@redhat.com>
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/616/head:pr616
git checkout pr616
From 0f0a9457ba6f2e871dedf3a1cb9491c916693c39 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 17 Mar 2017 10:44:38 +0100
Subject: [PATCH] Simplify KRA transport cert cache
In-memory cache causes problem in forking servers. A file based cache is
good enough. It's easier to understand and avoids performance regression
and synchronization issues when cert becomes out-of-date.
Signed-off-by: Christian Heimes <chei...@redhat.com>
---
ipaclient/plugins/vault.py | 106 +++++++++++++++++++++++++--------------------
1 file changed, 59 insertions(+), 47 deletions(-)
diff --git a/ipaclient/plugins/vault.py b/ipaclient/plugins/vault.py
index d677ec0..ca6d6da 100644
--- a/ipaclient/plugins/vault.py
+++ b/ipaclient/plugins/vault.py
@@ -558,74 +558,79 @@ def forward(self, *args, **options):
return response
-class _TransportCertCache(collections.MutableMapping):
+class _TransportCertCache(object):
def __init__(self):
self._dirname = os.path.join(
- USER_CACHE_PATH, 'ipa', 'kra-transport-certs')
- self._transport_certs = {}
+ USER_CACHE_PATH, 'ipa', 'kra-transport-certs'
+ )
def _get_filename(self, domain):
basename = DNSName(domain).ToASCII() + '.pem'
return os.path.join(self._dirname, basename)
- def __getitem__(self, domain):
- try:
- transport_cert = self._transport_certs[domain]
- except KeyError:
- transport_cert = None
+ def load_cert(self, domain):
+ """Load cert from cache
- filename = self._get_filename(domain)
+ :param domain: IPA domain
+ :return: cryptography.x509.Certificate or None
+ """
+ filename = self._get_filename(domain)
+ try:
try:
- try:
- transport_cert = x509.load_certificate_from_file(filename)
- except EnvironmentError as e:
- if e.errno != errno.ENOENT:
- raise
- except Exception:
- logger.warning("Failed to load %s: %s", filename,
- exc_info=True)
-
- if transport_cert is None:
- raise KeyError(domain)
-
- self._transport_certs[domain] = transport_cert
+ return x509.load_certificate_from_file(filename)
+ except EnvironmentError as e:
+ if e.errno != errno.ENOENT:
+ raise
+ except Exception:
+ logger.warning("Failed to load %s", filename, exc_info=True)
- return transport_cert
+ def store_cert(self, domain, transport_cert):
+ """Store a new cert or override existing cert
- def __setitem__(self, domain, transport_cert):
+ :param domain: IPA domain
+ :param transport_cert: cryptography.x509.Certificate
+ :return: True if cert was stored successfully
+ """
filename = self._get_filename(domain)
- transport_cert_der = (
- transport_cert.public_bytes(serialization.Encoding.DER))
+ pem = transport_cert.public_bytes(serialization.Encoding.PEM)
try:
try:
os.makedirs(self._dirname)
except EnvironmentError as e:
if e.errno != errno.EEXIST:
raise
- fd, tmpfilename = tempfile.mkstemp(dir=self._dirname)
- os.close(fd)
- x509.write_certificate(transport_cert_der, tmpfilename)
- os.rename(tmpfilename, filename)
+ with tempfile.NamedTemporaryFile(dir=self._dirname, delete=False,
+ mode='wb') as f:
+ try:
+ f.write(pem)
+ f.flush()
+ os.fdatasync(f.fileno())
+ f.close()
+ os.rename(f.name, filename)
+ except Exception:
+ os.unlink(f.name)
+ raise
except Exception:
logger.warning("Failed to save %s", filename, exc_info=True)
+ return False
+ else:
+ return True
- self._transport_certs[domain] = transport_cert
+ def remove_cert(self, domain):
+ """Remove a cert from cache, ignores errors
- def __delitem__(self, domain):
+ :param domain: IPA domain
+ :return: True if cert was found and removed
+ """
filename = self._get_filename(domain)
try:
os.unlink(filename)
except EnvironmentError as e:
if e.errno != errno.ENOENT:
logger.warning("Failed to remove %s", filename, exc_info=True)
-
- del self._transport_certs[domain]
-
- def __len__(self):
- return len(self._transport_certs)
-
- def __iter__(self):
- return iter(self._transport_certs)
+ return False
+ except:
+ return True
_transport_cert_cache = _TransportCertCache()
@@ -646,7 +651,10 @@ def forward(self, *args, **options):
# cache transport certificate
transport_cert = x509.load_certificate(
response['result']['transport_cert'], x509.DER)
- _transport_cert_cache[self.api.env.domain] = transport_cert
+
+ _transport_cert_cache.store_cert(
+ self.api.env.domain, transport_cert
+ )
if file:
with open(file, 'w') as f:
@@ -680,7 +688,7 @@ def _do_internal(self, algo, transport_cert, raise_unexpected,
except (errors.InternalError,
errors.ExecutionError,
errors.GenericError):
- _transport_cert_cache.pop(self.api.env.domain, None)
+ _transport_cert_cache.remove_cert(self.api.env.domain)
if raise_unexpected:
raise
@@ -691,17 +699,21 @@ def internal(self, algo, *args, **options):
domain = self.api.env.domain
# try call with cached transport certificate
- transport_cert = _transport_cert_cache.get(domain)
+ transport_cert = _transport_cert_cache.load_cert(domain)
if transport_cert is not None:
result = self._do_internal(algo, transport_cert, False,
*args, **options)
if result is not None:
return result
- # retrieve and cache transport certificate
- self.api.Command.vaultconfig_show()
- transport_cert = _transport_cert_cache[domain]
-
+ # retrieve transport certificate
+ response = self.api.Command.vaultconfig_show()
+ transport_cert = x509.load_certificate(
+ response['result']['transport_cert'], x509.DER)
+ # cache it again
+ _transport_cert_cache.store_cert(
+ self.api.env.domain, transport_cert
+ )
# call with the retrieved transport certificate
return self._do_internal(algo, transport_cert, True,
*args, **options)
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
