mirror of
https://salsa.debian.org/freeipa-team/freeipa.git
synced 2025-02-25 18:55:28 -06:00
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. https://pagure.io/freeipa/issue/6787 Signed-off-by: Christian Heimes <cheimes@redhat.com> Reviewed-By: Jan Cholasta <jcholast@redhat.com>
This commit is contained in:
committed by
Jan Cholasta
parent
e934da09d5
commit
abefb64bea
@@ -20,7 +20,6 @@
|
||||
from __future__ import print_function
|
||||
|
||||
import base64
|
||||
import collections
|
||||
import errno
|
||||
import getpass
|
||||
import io
|
||||
@@ -558,74 +557,79 @@ class vault_mod(Local):
|
||||
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)
|
||||
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 transport_cert
|
||||
|
||||
def __setitem__(self, domain, transport_cert):
|
||||
:param domain: IPA domain
|
||||
:return: cryptography.x509.Certificate or None
|
||||
"""
|
||||
filename = self._get_filename(domain)
|
||||
transport_cert_der = (
|
||||
transport_cert.public_bytes(serialization.Encoding.DER))
|
||||
try:
|
||||
try:
|
||||
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)
|
||||
|
||||
def store_cert(self, domain, transport_cert):
|
||||
"""Store a new cert or override existing cert
|
||||
|
||||
:param domain: IPA domain
|
||||
:param transport_cert: cryptography.x509.Certificate
|
||||
:return: True if cert was stored successfully
|
||||
"""
|
||||
filename = self._get_filename(domain)
|
||||
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
|
||||
else:
|
||||
return True
|
||||
|
||||
|
||||
_transport_cert_cache = _TransportCertCache()
|
||||
@@ -646,7 +650,10 @@ class vaultconfig_show(MethodOverride):
|
||||
# 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 +687,7 @@ class ModVaultData(Local):
|
||||
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 +698,17 @@ class ModVaultData(Local):
|
||||
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 (cached by vaultconfig_show)
|
||||
response = self.api.Command.vaultconfig_show()
|
||||
transport_cert = x509.load_certificate(
|
||||
response['result']['transport_cert'], x509.DER)
|
||||
# call with the retrieved transport certificate
|
||||
return self._do_internal(algo, transport_cert, True,
|
||||
*args, **options)
|
||||
|
||||
Reference in New Issue
Block a user