Skip to content

Commit

Permalink
Allow the CA and CRL to be file paths
Browse files Browse the repository at this point in the history
This also changes how they are cached in order to better handle expired
or rotated CRLs/CAs.

If the CA/CRL is a file, it is read into a cache and used until a
message fails validation. At that point, the cache is invalidated and
the CA/CRL is reloaded. If the message still fails validation, we mark
it as invalid and continue.

If the CA/CRL is a URL, the file is downloaded and cached in memory just
like the file approach.

It would be nice if the process halted when a fatal error was
encountered (like the CRL being expired), but unfortunately there's no
way to communicate that to moksha. Once we drop moksha we can do that
with a set of fedmsg exceptions, but for now logging at the error level
is the only thing we can do.

fixes #481
fixes #365

Signed-off-by: Jeremy Cline <jeremy@jcline.org>
  • Loading branch information
jeremycline committed Sep 29, 2017
1 parent 73425a9 commit fb3185d
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 75 deletions.
72 changes: 72 additions & 0 deletions fedmsg/crypto/utils.py
Expand Up @@ -4,6 +4,9 @@
import time


# A simple dictionary to cache certificates in
_cached_certificates = dict()

_log = logging.getLogger(__name__)


Expand Down Expand Up @@ -98,6 +101,75 @@ def validate_policy(topic, signer, routing_policy, nitpicky=False):
return True


def load_certificates(ca_location, crl_location=None, invalidate_cache=False):
"""
Load the CA certificate and CRL, caching it for future use.
Args:
ca_location (str): The location of the Certificate Authority certificate. This should
be the absolute path to a PEM-encoded file. It can also be an HTTPS url, but this
is deprecated and will be removed in a future release.
ca_location (str): The location of the Certificate Revocation List. This should
be the absolute path to a PEM-encoded file. It can also be an HTTPS url, but
this is deprecated and will be removed in a future release.
invalidate_cache (bool): Whether or not to invalidate the certificate cache.
Returns:
tuple: A tuple of the (CA certificate, CRL) as unicode strings.
Raises:
requests.exception.RequestException: Any exception requests could raise.
IOError: If the location provided could not be opened and read.
"""
if crl_location is None:
crl_location = ''

try:
if invalidate_cache:
del _cached_certificates[ca_location + crl_location]
else:
return _cached_certificates[ca_location + crl_location]
except KeyError:
pass

ca, crl = None, None
if ca_location:
ca = _load_certificate(ca_location)
if crl_location:
crl = _load_certificate(crl_location)

_cached_certificates[ca_location + crl_location] = ca, crl
return ca, crl


def _load_certificate(location):
"""
Load a certificate from the given location.
Args:
location (str): The location to load. This can either be an HTTPS URL or an absolute file
path. This is intended to be used with PEM-encoded certificates and therefore assumes
ASCII encoding.
Returns:
str:
Raises:
requests.exception.RequestException: Any exception requests could raise.
IOError: If the location provided could not be opened and read.
"""
if location.startswith('https://'):
_log.info('Downloading x509 certificate from %s', location)
with requests.Session() as session:
session.mount('https://', requests.adapters.HTTPAdapter(max_retries=3))
response = session.get(location, timeout=30)
return response.text
else:
_log.info('Loading local x509 certificate from %s', location)
with open(location, 'rb') as fd:
return fd.read().decode('ascii')


def _load_remote_cert(location, cache, cache_expiry, tries=3, **config):
"""Get a fresh copy from fp.o/fedmsg/crl.pem if ours is getting stale.
Expand Down
52 changes: 32 additions & 20 deletions fedmsg/crypto/x509.py
Expand Up @@ -20,6 +20,8 @@
""" ``fedmsg.crypto.x509`` - X.509 backend for :mod:`fedmsg.crypto`. """

import logging
import os
import tempfile
import warnings

import six
Expand All @@ -31,7 +33,7 @@
except ImportError:
_m2crypto = False

from .utils import _load_remote_cert, validate_policy
from .utils import validate_policy, load_certificates
from .x509_ng import _cryptography, sign as _crypto_sign, validate as _crypto_validate
import fedmsg.crypto # noqa: E402
import fedmsg.encoding # noqa: E402
Expand Down Expand Up @@ -136,28 +138,38 @@ def fail(reason):
# validate_certificate will one day be a part of M2Crypto.SSL.Context
# https://bugzilla.osafoundation.org/show_bug.cgi?id=11690

default_ca_cert_loc = 'https://fedoraproject.org/fedmsg/ca.crt'
cafile = _load_remote_cert(
config.get('ca_cert_location', default_ca_cert_loc),
config.get('ca_cert_cache', '/etc/pki/fedmsg/ca.crt'),
config.get('ca_cert_cache_expiry', 0),
**config)
ca_location = config.get('ca_cert_location', 'https://fedoraproject.org/fedmsg/ca.crt')
crl_location = config.get('crl_location', 'https://fedoraproject.org/fedmsg/crl.pem')
ca_certificate, crl = load_certificates(ca_location, crl_location)

ctx = m2ext.SSL.Context()
ctx.load_verify_locations(cafile=cafile)
if not ctx.validate_certificate(cert):
return fail("X509 certificate is not valid.")

# Load and check against the CRL
crl = None
if 'crl_location' in config and 'crl_cache' in config:
crl = _load_remote_cert(
config.get('crl_location', 'https://fedoraproject.org/fedmsg/crl.pem'),
config.get('crl_cache', '/var/cache/fedmsg/crl.pem'),
config.get('crl_cache_expiry', 1800),
**config)
fd, cafile = tempfile.mkstemp(text=True)
os.close(fd)
try:
with open(cafile) as fd:
fd.write(ca_certificate)
ctx.load_verify_locations(cafile=cafile)
if not ctx.validate_certificate(cert):
ca_certificate, crl = load_certificates(
ca_location, crl_location, invalidate_cache=True)
with open(cafile) as fd:
fd.write(ca_certificate)
ctx = m2ext.SSL.Context()
ctx.load_verify_locations(cafile=cafile)
if not ctx.validate_certificate(cert):
return fail("X509 certificate is not valid.")
finally:
os.remove(cafile)

if crl:
crl = M2Crypto.X509.load_crl(crl)
fd, crlfile = tempfile.mkstemp(text=True)
os.close(fd)
try:
with open(crlfile) as fd:
fd.write(crl)
crl = M2Crypto.X509.load_crl(crlfile)
finally:
os.remove(crlfile)
# FIXME -- We need to check that the CRL is signed by our own CA.
# See https://bugzilla.osafoundation.org/show_bug.cgi?id=12954#c2
# if not ctx.validate_certificate(crl):
Expand Down
42 changes: 15 additions & 27 deletions fedmsg/crypto/x509_ng.py
Expand Up @@ -39,6 +39,7 @@
_cryptography = True
except ImportError: # pragma: no cover
_cryptography = False
from requests.exceptions import RequestException
import six

from . import utils
Expand Down Expand Up @@ -164,35 +165,22 @@ def validate(message, ssldir=None, **config):
certificate = base64.b64decode(message['certificate'])
message = fedmsg.crypto.strip_credentials(message)

crl_file = None
if 'crl_location' in config and 'crl_cache' in config:
crl_file = utils._load_remote_cert(
config.get('crl_location', 'https://fedoraproject.org/fedmsg/crl.pem'),
config.get('crl_cache', '/var/cache/fedmsg/crl.pem'),
config.get('crl_cache_expiry', 1800),
**config
)

ca_file = utils._load_remote_cert(
config.get('ca_cert_location', 'https://fedoraproject.org/fedmsg/ca.crt'),
config.get('ca_cert_cache', '/etc/pki/fedmsg/ca.crt'),
config.get('ca_cert_cache_expiry', 0),
**config
)

with open(ca_file, 'rb') as fd:
ca_certificate = fd.read()

crl = None
if crl_file:
with open(crl_file, 'rb') as fd:
crl = fd.read()

# Unfortunately we can't change this defaulting to Fedora behavior until
# fedmsg-2.0
ca_location = config.get('ca_cert_location', 'https://fedoraproject.org/fedmsg/ca.crt')
crl_location = config.get('crl_location', 'https://fedoraproject.org/fedmsg/crl.pem')
try:
ca_certificate, crl = utils.load_certificates(ca_location, crl_location)
_validate_signing_cert(ca_certificate, certificate, crl)
except X509StoreContextError as e:
_log.error(str(e))
return False
except (IOError, RequestException, X509StoreContextError) as e:
# Maybe the CA/CRL is expired or just rotated, so invalidate the cache and try again
try:
ca_certificate, crl = utils.load_certificates(
ca_location, crl_location, invalidate_cache=True)
_validate_signing_cert(ca_certificate, certificate, crl)
except (IOError, RequestException, X509StoreContextError) as e:
_log.error(str(e))
return False

# Validate the signature of the message itself
try:
Expand Down
8 changes: 2 additions & 6 deletions fedmsg/tests/consumers/test_consumers.py
Expand Up @@ -76,12 +76,8 @@ def setUp(self):
'dummy': True,
'ssldir': SSLDIR,
'certname': 'shell-app01.phx2.fedoraproject.org',
'ca_cert_cache': os.path.join(SSLDIR, 'ca.crt'),
'ca_cert_cache_expiry': 1497618475, # Stop fedmsg overwriting my CA, See Issue 420

'crl_location': "http://threebean.org/fedmsg-tests/crl.pem",
'crl_cache': os.path.join(SSLDIR, 'crl.pem'),
'crl_cache_expiry': 1497618475,
'ca_cert_location': os.path.join(SSLDIR, 'ca.crt'),
'crl_location': os.path.join(SSLDIR, 'crl.pem'),
'crypto_validate_backends': ['x509'],
}
self.hub = mock.Mock(config=self.config)
Expand Down
32 changes: 10 additions & 22 deletions fedmsg/tests/crypto/test_x509.py
Expand Up @@ -42,12 +42,8 @@ def setUp(self):
self.config = {
'ssldir': SSLDIR,
'certname': 'shell-app01.phx2.fedoraproject.org',
'ca_cert_cache': os.path.join(SSLDIR, 'ca.crt'),
'ca_cert_cache_expiry': 1497618475, # Stop fedmsg overwriting my CA, See Issue 420

'crl_location': "http://threebean.org/fedmsg-tests/crl.pem",
'crl_cache': os.path.join(SSLDIR, 'crl.pem'),
'crl_cache_expiry': 1497618475,
'ca_cert_location': os.path.join(SSLDIR, 'ca.crt'),
'crl_location': os.path.join(SSLDIR, 'crl.pem'),
'crypto_validate_backends': ['x509'],
}
self.sign = crypto.sign
Expand Down Expand Up @@ -95,7 +91,7 @@ def test_unsigned(self):

def test_invalid_ca(self):
"""Assert when the CA didn't sign the certificate, validation fails."""
self.config['ca_cert_cache'] = os.path.join(SSLDIR, 'badca.crt')
self.config['ca_cert_location'] = os.path.join(SSLDIR, 'badca.crt')

signed = self.sign({'my': 'message'}, **self.config)
self.assertFalse(self.validate(signed, **self.config))
Expand Down Expand Up @@ -125,21 +121,17 @@ def test_invalid_message_signature(self):

def test_no_crl(self):
"""Assert that it's okay to not use a CRL."""
del self.config['crl_location']
del self.config['crl_cache']
del self.config['crl_cache_expiry']
self.config['crl_location'] = None

signed = self.sign({'message': 'so secure'}, **self.config)
self.assertTrue(self.validate(signed, **self.config))

def test_signed_by_expired_ca(self):
"""Assert certs signed by an expired CA fail validation."""
self.config['certname'] = 'signed_by_expired_ca'
self.config['ca_cert_cache'] = os.path.join(SSLDIR, 'expired_ca.crt')
self.config['ca_cert_location'] = os.path.join(SSLDIR, 'expired_ca.crt')
# There's no CRL for this CA.
del self.config['crl_location']
del self.config['crl_cache']
del self.config['crl_cache_expiry']
self.config['crl_location'] = None

signed = self.sign({'message': 'so secure'}, **self.config)
self.assertFalse(self.validate(signed, **self.config))
Expand Down Expand Up @@ -200,7 +192,7 @@ def setUp(self):
def test_old_crl(self):
"""Assert when an old CRL is used, validation fails."""
signed = self.sign({'my': 'message'}, **self.config)
self.config['crl_cache'] = os.path.join(SSLDIR, 'expired_crl.pem')
self.config['crl_location'] = os.path.join(SSLDIR, 'expired_crl.pem')

self.assertFalse(self.validate(signed, **self.config))

Expand Down Expand Up @@ -232,7 +224,7 @@ def setUp(self):
def test_old_crl(self):
"""Assert when an old CRL is used, validation fails."""
signed = self.sign({'my': 'message'}, **self.config)
self.config['crl_cache'] = os.path.join(SSLDIR, 'expired_crl.pem')
self.config['crl_location'] = os.path.join(SSLDIR, 'expired_crl.pem')

self.assertFalse(self.validate(signed, **self.config))

Expand Down Expand Up @@ -279,12 +271,8 @@ def setUp(self):
self.config = {
'ssldir': SSLDIR,
'certname': 'shell-app01.phx2.fedoraproject.org',
'ca_cert_cache': os.path.join(SSLDIR, 'ca.crt'),
'ca_cert_cache_expiry': 1497618475, # Stop fedmsg overwriting my CA, See Issue 420

'crl_location': "http://threebean.org/fedmsg-tests/crl.pem",
'crl_cache': os.path.join(SSLDIR, 'crl.pem'),
'crl_cache_expiry': 1497618475,
'ca_cert_location': os.path.join(SSLDIR, 'ca.crt'),
'crl_location': os.path.join(SSLDIR, 'crl.pem'),
'crypto_validate_backends': ['x509'],
}

Expand Down

0 comments on commit fb3185d

Please sign in to comment.