From 720859efb6f339024d7889b7bb12511b686ebf10 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 4 Mar 2019 10:59:20 -0500 Subject: [PATCH 01/10] Ugly implementation for CRLInvalidException --- atst/domain/authnid/crl/__init__.py | 12 +++++++++++- tests/domain/authnid/test_crl.py | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/atst/domain/authnid/crl/__init__.py b/atst/domain/authnid/crl/__init__.py index 937a2db1..82dc4896 100644 --- a/atst/domain/authnid/crl/__init__.py +++ b/atst/domain/authnid/crl/__init__.py @@ -16,6 +16,12 @@ class CRLRevocationException(Exception): pass +class CRLInvalidException(Exception): + # CRL expired + # CRL missing + pass + + class CRLInterface: def __init__(self, *args, logger=None, **kwargs): self.logger = logger @@ -111,7 +117,7 @@ class CRLCache(CRLInterface): issuer_name = get_common_name(issuer) if not crl_info: - raise CRLRevocationException( + raise CRLInvalidException( "Could not find matching CRL for issuer with Common Name {}".format( issuer_name ) @@ -170,6 +176,10 @@ class CRLCache(CRLInterface): return True except crypto.X509StoreContextError as err: + if ( + err.args[0][2] == "CRL has expired" + ): # there has to be a better way than this + raise CRLInvalidException("CRL expired. Args: {}".format(err.args)) raise CRLRevocationException( "Certificate revoked or errored. Error: {}. Args: {}".format( type(err), err.args diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index c385c4bb..c51e9a2a 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -12,7 +12,12 @@ from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.hazmat.primitives.serialization import Encoding from cryptography.x509.oid import NameOID -from atst.domain.authnid.crl import CRLCache, CRLRevocationException, NoOpCRLCache +from atst.domain.authnid.crl import ( + CRLCache, + CRLRevocationException, + CRLInvalidException, + NoOpCRLCache, +) from tests.mocks import FIXTURE_EMAIL_ADDRESS, DOD_CN @@ -233,6 +238,16 @@ def test_no_op_crl_cache_logs_common_name(): assert "ART.GARFUNKEL.1234567890" in logger.messages[-1] +def test_expired_crl_raises_CRLInvalidException( + ca_file, expired_crl_file, ca_key, make_x509 +): + client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") + client_pem = client_cert.public_bytes(Encoding.PEM) + crl_cache = CRLCache(ca_file, crl_locations=[expired_crl_file]) + with pytest.raises(CRLInvalidException): + crl_cache.crl_check(client_pem) + + def test_updates_expired_certs(ca_file, expired_crl_file, crl_file, ca_key, make_x509): """ Given a CRLCache object with an expired CRL and a function for updating the From 30cd77ff98ee8409f4eaf095ae01024a62ff2a2f Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 4 Mar 2019 13:47:49 -0500 Subject: [PATCH 02/10] Test AuthenticationContext --- atst/domain/authnid/__init__.py | 4 +++- .../authnid/test_authentication_context.py | 22 +++++++++++++++++-- tests/domain/authnid/test_crl.py | 4 ++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/atst/domain/authnid/__init__.py b/atst/domain/authnid/__init__.py index fab6e722..25c04a5d 100644 --- a/atst/domain/authnid/__init__.py +++ b/atst/domain/authnid/__init__.py @@ -1,7 +1,7 @@ from atst.domain.exceptions import UnauthenticatedError, NotFoundError from atst.domain.users import Users from .utils import parse_sdn, email_from_certificate -from .crl import CRLRevocationException +from .crl import CRLRevocationException, CRLInvalidException class AuthenticationContext: @@ -47,6 +47,8 @@ class AuthenticationContext: def _crl_check(self): try: self.crl_cache.crl_check(self.cert) + except CRLInvalidException as exc: + raise UnauthenticatedError("CRL expired. " + str(exc)) except CRLRevocationException as exc: raise UnauthenticatedError("CRL check failed. " + str(exc)) diff --git a/tests/domain/authnid/test_authentication_context.py b/tests/domain/authnid/test_authentication_context.py index 1ca967c3..820f7028 100644 --- a/tests/domain/authnid/test_authentication_context.py +++ b/tests/domain/authnid/test_authentication_context.py @@ -1,7 +1,11 @@ import pytest from atst.domain.authnid import AuthenticationContext -from atst.domain.authnid.crl import CRLCache, CRLRevocationException +from atst.domain.authnid.crl import ( + CRLCache, + CRLRevocationException, + CRLInvalidException, +) from atst.domain.exceptions import UnauthenticatedError, NotFoundError from atst.domain.users import Users @@ -12,12 +16,15 @@ CERT = open("tests/fixtures/{}.crt".format(FIXTURE_EMAIL_ADDRESS)).read() class MockCRLCache: - def __init__(self, valid=True): + def __init__(self, valid=True, expired=False): self.valid = valid + self.expired = expired def crl_check(self, cert): if self.valid: return True + elif self.expired == True: + raise CRLInvalidException() raise CRLRevocationException() @@ -45,6 +52,17 @@ def test_crl_check_fails(): assert "CRL check" in message +def test_expired_crl_check_fails(): + auth_context = AuthenticationContext( + MockCRLCache(valid=False, expired=True), "SUCCESS", DOD_SDN, CERT + ) + with pytest.raises(UnauthenticatedError) as excinfo: + assert auth_context.authenticate() + + (message,) = excinfo.value.args + assert "CRL expired" in message + + def test_bad_sdn(): auth_context = AuthenticationContext(MockCRLCache(), "SUCCESS", "abc123", CERT) with pytest.raises(UnauthenticatedError) as excinfo: diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index c51e9a2a..6e9800a5 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -188,7 +188,7 @@ def test_can_dynamically_update_crls(tmpdir): assert cache.crl_check(cert) # override the original CRL with one that revokes atat.mil.crt shutil.copyfile("tests/fixtures/test.der.crl", crl_file) - with pytest.raises(CRLRevocationException): + with pytest.raises(CRLInvalidException): assert cache.crl_check(cert) @@ -197,7 +197,7 @@ def test_throws_error_for_missing_issuer(): # this cert is self-signed, and so the application does not have a # corresponding CRL for it cert = open("tests/fixtures/{}.crt".format(FIXTURE_EMAIL_ADDRESS), "rb").read() - with pytest.raises(CRLRevocationException) as exc: + with pytest.raises(CRLInvalidException) as exc: assert cache.crl_check(cert) (message,) = exc.value.args # objects that the issuer is missing From 2eeb54845803dd9c243a49f7223e2438e8f1e50e Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 12 Mar 2019 10:29:59 -0400 Subject: [PATCH 03/10] Move crl fixtures to conftest --- tests/conftest.py | 111 +++++++++++++++++++++++++++++ tests/domain/authnid/test_crl.py | 115 ++----------------------------- 2 files changed, 116 insertions(+), 110 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4bc5dda8..b728a9f2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,6 +14,15 @@ from atst.queue import queue as atst_queue import tests.factories as factories from tests.mocks import PDF_FILENAME, PDF_FILENAME2 +from datetime import datetime, timezone, timedelta +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography import x509 +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.serialization import Encoding +from cryptography.x509.oid import NameOID + + dictConfig({"version": 1, "handlers": {"wsgi": {"class": "logging.NullHandler"}}}) @@ -153,3 +162,105 @@ def extended_financial_verification_data(pdf_upload): def queue(): yield atst_queue atst_queue.get_queue().empty() + + +@pytest.fixture +def rsa_key(): + def _rsa_key(): + return rsa.generate_private_key( + public_exponent=65537, key_size=2048, backend=default_backend() + ) + + return _rsa_key + + +@pytest.fixture +def ca_key(rsa_key): + return rsa_key() + + +@pytest.fixture +def make_x509(): + def _make_x509(private_key, signer_key=None, cn="ATAT", signer_cn="ATAT"): + if signer_key is None: + signer_key = private_key + + one_day = timedelta(1, 0, 0) + public_key = private_key.public_key() + builder = x509.CertificateBuilder() + builder = builder.subject_name( + x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, cn)]) + ) + builder = builder.issuer_name( + x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, signer_cn)]) + ) + if signer_key == private_key: + builder = builder.add_extension( + x509.BasicConstraints(ca=True, path_length=None), critical=True + ) + builder = builder.not_valid_before(datetime.today() - (one_day * 2)) + builder = builder.not_valid_after(datetime.today() + (one_day * 30)) + builder = builder.serial_number(x509.random_serial_number()) + builder = builder.public_key(public_key) + certificate = builder.sign( + private_key=signer_key, algorithm=hashes.SHA256(), backend=default_backend() + ) + + return certificate + + return _make_x509 + + +@pytest.fixture +def make_crl(): + def _make_crl(private_key, last_update_days=-1, next_update_days=30, cn="ATAT"): + one_day = timedelta(1, 0, 0) + builder = x509.CertificateRevocationListBuilder() + builder = builder.issuer_name( + x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, cn)]) + ) + builder = builder.last_update(datetime.today() + (one_day * last_update_days)) + builder = builder.next_update(datetime.today() + (one_day * next_update_days)) + crl = builder.sign( + private_key=private_key, + algorithm=hashes.SHA256(), + backend=default_backend(), + ) + + return crl + + return _make_crl + + +def serialize_pki_object_to_disk(obj, name, encoding=Encoding.PEM): + with open(name, "wb") as file_: + file_.write(obj.public_bytes(encoding)) + + return name + + +@pytest.fixture +def ca_file(make_x509, ca_key, tmpdir): + ca = make_x509(ca_key) + ca_out = tmpdir.join("atat-ca.crt") + serialize_pki_object_to_disk(ca, ca_out) + + return ca_out + + +@pytest.fixture +def expired_crl_file(make_crl, ca_key, tmpdir): + crl = make_crl(ca_key, last_update_days=-7, next_update_days=-1) + crl_out = tmpdir.join("atat-expired.crl") + serialize_pki_object_to_disk(crl, crl_out, encoding=Encoding.DER) + + return crl_out + + +@pytest.fixture +def crl_file(make_crl, ca_key, tmpdir): + crl = make_crl(ca_key) + crl_out = tmpdir.join("atat-valid.crl") + serialize_pki_object_to_disk(crl, crl_out, encoding=Encoding.DER) + + return crl_out diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index 6e9800a5..6bb27bb4 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -3,14 +3,8 @@ import pytest import re import os import shutil -from datetime import datetime, timezone, timedelta -from OpenSSL import crypto, SSL -from cryptography import x509 from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives import hashes -from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.hazmat.primitives.serialization import Encoding -from cryptography.x509.oid import NameOID from atst.domain.authnid.crl import ( CRLCache, @@ -22,104 +16,6 @@ from atst.domain.authnid.crl import ( from tests.mocks import FIXTURE_EMAIL_ADDRESS, DOD_CN -def rsa_key(): - return rsa.generate_private_key( - public_exponent=65537, key_size=2048, backend=default_backend() - ) - - -@pytest.fixture -def ca_key(): - return rsa_key() - - -@pytest.fixture -def make_x509(): - def _make_x509(private_key, signer_key=None, cn="ATAT", signer_cn="ATAT"): - if signer_key is None: - signer_key = private_key - - one_day = timedelta(1, 0, 0) - public_key = private_key.public_key() - builder = x509.CertificateBuilder() - builder = builder.subject_name( - x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, cn)]) - ) - builder = builder.issuer_name( - x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, signer_cn)]) - ) - if signer_key == private_key: - builder = builder.add_extension( - x509.BasicConstraints(ca=True, path_length=None), critical=True - ) - builder = builder.not_valid_before(datetime.today() - (one_day * 2)) - builder = builder.not_valid_after(datetime.today() + (one_day * 30)) - builder = builder.serial_number(x509.random_serial_number()) - builder = builder.public_key(public_key) - certificate = builder.sign( - private_key=signer_key, algorithm=hashes.SHA256(), backend=default_backend() - ) - - return certificate - - return _make_x509 - - -@pytest.fixture -def make_crl(): - def _make_crl(private_key, last_update_days=-1, next_update_days=30, cn="ATAT"): - one_day = timedelta(1, 0, 0) - builder = x509.CertificateRevocationListBuilder() - builder = builder.issuer_name( - x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, cn)]) - ) - builder = builder.last_update(datetime.today() + (one_day * last_update_days)) - builder = builder.next_update(datetime.today() + (one_day * next_update_days)) - crl = builder.sign( - private_key=private_key, - algorithm=hashes.SHA256(), - backend=default_backend(), - ) - - return crl - - return _make_crl - - -def serialize_pki_object_to_disk(obj, name, encoding=Encoding.PEM): - with open(name, "wb") as file_: - file_.write(obj.public_bytes(encoding)) - - return name - - -@pytest.fixture -def ca_file(make_x509, ca_key, tmpdir): - ca = make_x509(ca_key) - ca_out = tmpdir.join("atat-ca.crt") - serialize_pki_object_to_disk(ca, ca_out) - - return ca_out - - -@pytest.fixture -def expired_crl_file(make_crl, ca_key, tmpdir): - crl = make_crl(ca_key, last_update_days=-7, next_update_days=-1) - crl_out = tmpdir.join("atat-expired.crl") - serialize_pki_object_to_disk(crl, crl_out, encoding=Encoding.DER) - - return crl_out - - -@pytest.fixture -def crl_file(make_crl, ca_key, tmpdir): - crl = make_crl(ca_key) - crl_out = tmpdir.join("atat-valid.crl") - serialize_pki_object_to_disk(crl, crl_out, encoding=Encoding.DER) - - return crl_out - - class MockX509Store: def __init__(self): self.crls = [] @@ -135,11 +31,8 @@ class MockX509Store: pass -def test_can_build_crl_list(ca_file, ca_key, make_crl, tmpdir): +def test_can_build_crl_list(crl_file, ca_key, ca_file, make_crl, tmpdir): crl = make_crl(ca_key) - crl_file = tmpdir.join("atat.crl") - serialize_pki_object_to_disk(crl, crl_file, encoding=Encoding.DER) - cache = CRLCache(ca_file, crl_locations=[crl_file], store_class=MockX509Store) issuer_der = crl.issuer.public_bytes(default_backend()) assert len(cache.crl_cache.keys()) == 1 @@ -239,7 +132,7 @@ def test_no_op_crl_cache_logs_common_name(): def test_expired_crl_raises_CRLInvalidException( - ca_file, expired_crl_file, ca_key, make_x509 + ca_file, expired_crl_file, ca_key, make_x509, rsa_key ): client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") client_pem = client_cert.public_bytes(Encoding.PEM) @@ -248,7 +141,9 @@ def test_expired_crl_raises_CRLInvalidException( crl_cache.crl_check(client_pem) -def test_updates_expired_certs(ca_file, expired_crl_file, crl_file, ca_key, make_x509): +def test_updates_expired_certs( + rsa_key, ca_file, expired_crl_file, crl_file, ca_key, make_x509 +): """ Given a CRLCache object with an expired CRL and a function for updating the CRLs, the CRLCache should run the update function before checking a From d6906c850416876fef0b112ec41c14e079416f21 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 12 Mar 2019 16:42:08 -0400 Subject: [PATCH 04/10] Add config for CRL_FAIL_OPEN --- atst/app.py | 1 + config/base.ini | 1 + 2 files changed, 2 insertions(+) diff --git a/atst/app.py b/atst/app.py index fcca8f44..4413a55f 100644 --- a/atst/app.py +++ b/atst/app.py @@ -142,6 +142,7 @@ def map_config(config): "RQ_REDIS_URL": config["default"]["REDIS_URI"], "RQ_QUEUES": [config["default"]["RQ_QUEUES"]], "DISABLE_CRL_CHECK": config.getboolean("default", "DISABLE_CRL_CHECK"), + "CRL_FAIL_OPEN": config.getboolean("default", "CRL_FAIL_OPEN"), } diff --git a/config/base.ini b/config/base.ini index 0629ec29..37f5df0a 100644 --- a/config/base.ini +++ b/config/base.ini @@ -7,6 +7,7 @@ CRL_STORAGE_CONTAINER = crls CRL_STORAGE_PROVIDER = LOCAL CRL_STORAGE_REGION = iad DISABLE_CRL_CHECK = false +CRL_FAIL_OPEN = false DEBUG = true ENVIRONMENT = dev PERMANENT_SESSION_LIFETIME = 600 From effec85cf9707df65ca1de9f3f21d5c8a5ffb999 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 12 Mar 2019 16:42:58 -0400 Subject: [PATCH 05/10] Raise Error Code 008 for invalid CRLs --- atst/domain/authnid/__init__.py | 2 -- atst/domain/authnid/crl/__init__.py | 13 +++++++++---- atst/routes/errors.py | 6 ++++++ tests/domain/authnid/test_authentication_context.py | 5 +---- tests/test_auth.py | 13 +++++++++++++ 5 files changed, 29 insertions(+), 10 deletions(-) diff --git a/atst/domain/authnid/__init__.py b/atst/domain/authnid/__init__.py index 25c04a5d..b644020e 100644 --- a/atst/domain/authnid/__init__.py +++ b/atst/domain/authnid/__init__.py @@ -47,8 +47,6 @@ class AuthenticationContext: def _crl_check(self): try: self.crl_cache.crl_check(self.cert) - except CRLInvalidException as exc: - raise UnauthenticatedError("CRL expired. " + str(exc)) except CRLRevocationException as exc: raise UnauthenticatedError("CRL check failed. " + str(exc)) diff --git a/atst/domain/authnid/crl/__init__.py b/atst/domain/authnid/crl/__init__.py index 82dc4896..26956f09 100644 --- a/atst/domain/authnid/crl/__init__.py +++ b/atst/domain/authnid/crl/__init__.py @@ -2,9 +2,13 @@ import sys import os import re import hashlib +from flask import current_app as app from datetime import datetime from OpenSSL import crypto, SSL +# error codes from OpenSSL: https://github.com/openssl/openssl/blob/2c75f03b39de2fa7d006bc0f0d7c58235a54d9bb/include/openssl/x509_vfy.h#L111 +CRL_EXPIRED_ERROR_CODE = 12 + def get_common_name(x509_name_object): for comp in x509_name_object.get_components(): @@ -176,10 +180,11 @@ class CRLCache(CRLInterface): return True except crypto.X509StoreContextError as err: - if ( - err.args[0][2] == "CRL has expired" - ): # there has to be a better way than this - raise CRLInvalidException("CRL expired. Args: {}".format(err.args)) + if err.args[0][0] == CRL_EXPIRED_ERROR_CODE: + if app.config.get("CRL_FAIL_OPEN"): + return True + else: + raise CRLInvalidException("CRL expired. Args: {}".format(err.args)) raise CRLRevocationException( "Certificate revoked or errored. Error: {}. Args: {}".format( type(err), err.args diff --git a/atst/routes/errors.py b/atst/routes/errors.py index 279d6c8f..d00c7f0a 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -8,6 +8,7 @@ from atst.domain.invitations import ( ExpiredError as InvitationExpiredError, WrongUserError as InvitationWrongUserError, ) +from atst.domain.authnid.crl import CRLInvalidException from atst.domain.portfolios import PortfolioError from atst.utils.flash import formatted_flash as flash @@ -32,6 +33,11 @@ def make_error_pages(app): def not_found(e): return handle_error(e) + @app.errorhandler(CRLInvalidException) + # pylint: disable=unused-variable + def missing_crl(e): + return handle_error(e, message="Error Code 008", code=401) + @app.errorhandler(exceptions.UnauthenticatedError) # pylint: disable=unused-variable def unauthorized(e): diff --git a/tests/domain/authnid/test_authentication_context.py b/tests/domain/authnid/test_authentication_context.py index 820f7028..f83cea64 100644 --- a/tests/domain/authnid/test_authentication_context.py +++ b/tests/domain/authnid/test_authentication_context.py @@ -56,12 +56,9 @@ def test_expired_crl_check_fails(): auth_context = AuthenticationContext( MockCRLCache(valid=False, expired=True), "SUCCESS", DOD_SDN, CERT ) - with pytest.raises(UnauthenticatedError) as excinfo: + with pytest.raises(CRLInvalidException) as excinfo: assert auth_context.authenticate() - (message,) = excinfo.value.args - assert "CRL expired" in message - def test_bad_sdn(): auth_context = AuthenticationContext(MockCRLCache(), "SUCCESS", "abc123", CERT) diff --git a/tests/test_auth.py b/tests/test_auth.py index 30f8c47b..6c1f9cf2 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -6,6 +6,7 @@ from .mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS from atst.domain.users import Users from atst.domain.roles import Roles from atst.domain.exceptions import NotFoundError +from atst.domain.authnid.crl import CRLInvalidException from atst.domain.auth import UNPROTECTED_ROUTES from .factories import UserFactory @@ -211,3 +212,15 @@ def test_redirected_on_login(client, monkeypatch): target_route = url_for("users.user") response = _login(client, next=target_route) assert target_route in response.headers.get("Location") + + +def test_error_on_invalid_crl(client, monkeypatch): + def _raise_crl_error(*args): + raise CRLInvalidException() + + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.authenticate", _raise_crl_error + ) + response = _login(client) + assert response.status_code == 401 + assert "Error Code 008" in response.data.decode() From a1ebedb38288befb3cd9da6e4cd2738f0f1c8769 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 13 Mar 2019 13:57:39 -0400 Subject: [PATCH 06/10] Add another CRLCache test --- tests/domain/authnid/test_crl.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index 6bb27bb4..34fc718c 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -131,9 +131,10 @@ def test_no_op_crl_cache_logs_common_name(): assert "ART.GARFUNKEL.1234567890" in logger.messages[-1] -def test_expired_crl_raises_CRLInvalidException( - ca_file, expired_crl_file, ca_key, make_x509, rsa_key +def test_expired_crl_raises_CRLInvalidException_with_failover_config_false( + app, ca_file, expired_crl_file, ca_key, make_x509, rsa_key, monkeypatch ): + app.config.update({"CRL_FAIL_OPEN": False}) client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") client_pem = client_cert.public_bytes(Encoding.PEM) crl_cache = CRLCache(ca_file, crl_locations=[expired_crl_file]) @@ -141,6 +142,18 @@ def test_expired_crl_raises_CRLInvalidException( crl_cache.crl_check(client_pem) +def test_expired_crl_passes_with_failover_config_true( + ca_file, expired_crl_file, ca_key, make_x509, rsa_key, monkeypatch, app +): + app.config.update({"CRL_FAIL_OPEN": True}) + client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") + client_pem = client_cert.public_bytes(Encoding.PEM) + crl_cache = CRLCache(ca_file, crl_locations=[expired_crl_file]) + + assert crl_cache.crl_check(client_pem) + app.config.update({"CRL_FAIL_OPEN": False}) + + def test_updates_expired_certs( rsa_key, ca_file, expired_crl_file, crl_file, ca_key, make_x509 ): From 5782c30a7d5edc5be4551ef3dd12f4ae47c557f3 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 13 Mar 2019 16:36:52 -0400 Subject: [PATCH 07/10] Use pytest fixture for app with non default configs --- tests/conftest.py | 7 +++++++ tests/domain/authnid/test_crl.py | 6 ++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index b728a9f2..c800a088 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -164,6 +164,13 @@ def queue(): atst_queue.get_queue().empty() +@pytest.fixture +def crl_failover_open_app(app): + app.config.update({"CRL_FAIL_OPEN": True}) + yield app + app.config.update({"CRL_FAIL_OPEN": False}) + + @pytest.fixture def rsa_key(): def _rsa_key(): diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index 34fc718c..9b98ed3d 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -132,7 +132,7 @@ def test_no_op_crl_cache_logs_common_name(): def test_expired_crl_raises_CRLInvalidException_with_failover_config_false( - app, ca_file, expired_crl_file, ca_key, make_x509, rsa_key, monkeypatch + app, ca_file, expired_crl_file, ca_key, make_x509, rsa_key ): app.config.update({"CRL_FAIL_OPEN": False}) client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") @@ -143,15 +143,13 @@ def test_expired_crl_raises_CRLInvalidException_with_failover_config_false( def test_expired_crl_passes_with_failover_config_true( - ca_file, expired_crl_file, ca_key, make_x509, rsa_key, monkeypatch, app + ca_file, expired_crl_file, ca_key, make_x509, rsa_key, crl_failover_open_app ): - app.config.update({"CRL_FAIL_OPEN": True}) client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") client_pem = client_cert.public_bytes(Encoding.PEM) crl_cache = CRLCache(ca_file, crl_locations=[expired_crl_file]) assert crl_cache.crl_check(client_pem) - app.config.update({"CRL_FAIL_OPEN": False}) def test_updates_expired_certs( From 280775fa66a1f672921f928a8ebe17d684485d33 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 14 Mar 2019 13:38:52 -0400 Subject: [PATCH 08/10] Fix test that produces CRLRevocationException --- tests/conftest.py | 49 ++++++++++++++++++++++++-------- tests/domain/authnid/test_crl.py | 30 ++++++++++++------- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index c800a088..01683898 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,4 @@ import os -import datetime import pytest import alembic.config import alembic.command @@ -220,14 +219,26 @@ def make_x509(): @pytest.fixture def make_crl(): - def _make_crl(private_key, last_update_days=-1, next_update_days=30, cn="ATAT"): + def _make_crl( + private_key, + last_update_days=-1, + next_update_days=30, + cn="ATAT", + expired_serials=None, + ): one_day = timedelta(1, 0, 0) builder = x509.CertificateRevocationListBuilder() builder = builder.issuer_name( x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, cn)]) ) - builder = builder.last_update(datetime.today() + (one_day * last_update_days)) - builder = builder.next_update(datetime.today() + (one_day * next_update_days)) + last_update = datetime.today() + (one_day * last_update_days) + next_update = datetime.today() + (one_day * next_update_days) + builder = builder.last_update(last_update) + builder = builder.next_update(next_update) + if expired_serials: + for serial in expired_serials: + builder = add_revoked_cert(builder, serial, last_update) + crl = builder.sign( private_key=private_key, algorithm=hashes.SHA256(), @@ -239,15 +250,29 @@ def make_crl(): return _make_crl -def serialize_pki_object_to_disk(obj, name, encoding=Encoding.PEM): - with open(name, "wb") as file_: - file_.write(obj.public_bytes(encoding)) - - return name +def add_revoked_cert(crl_builder, serial, revocation_date): + revoked_cert = ( + x509.RevokedCertificateBuilder() + .serial_number(serial) + .revocation_date(revocation_date) + .build(default_backend()) + ) + return crl_builder.add_revoked_certificate(revoked_cert) @pytest.fixture -def ca_file(make_x509, ca_key, tmpdir): +def serialize_pki_object_to_disk(): + def _serialize_pki_object_to_disk(obj, name, encoding=Encoding.PEM): + with open(name, "wb") as file_: + file_.write(obj.public_bytes(encoding)) + + return name + + return _serialize_pki_object_to_disk + + +@pytest.fixture +def ca_file(make_x509, ca_key, tmpdir, serialize_pki_object_to_disk): ca = make_x509(ca_key) ca_out = tmpdir.join("atat-ca.crt") serialize_pki_object_to_disk(ca, ca_out) @@ -256,7 +281,7 @@ def ca_file(make_x509, ca_key, tmpdir): @pytest.fixture -def expired_crl_file(make_crl, ca_key, tmpdir): +def expired_crl_file(make_crl, ca_key, tmpdir, serialize_pki_object_to_disk): crl = make_crl(ca_key, last_update_days=-7, next_update_days=-1) crl_out = tmpdir.join("atat-expired.crl") serialize_pki_object_to_disk(crl, crl_out, encoding=Encoding.DER) @@ -265,7 +290,7 @@ def expired_crl_file(make_crl, ca_key, tmpdir): @pytest.fixture -def crl_file(make_crl, ca_key, tmpdir): +def crl_file(make_crl, ca_key, tmpdir, serialize_pki_object_to_disk): crl = make_crl(ca_key) crl_out = tmpdir.join("atat-valid.crl") serialize_pki_object_to_disk(crl, crl_out, encoding=Encoding.DER) diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index 9b98ed3d..0b1c88fa 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -73,16 +73,26 @@ def test_can_validate_certificate(): cache.crl_check(bad_cert) -def test_can_dynamically_update_crls(tmpdir): - crl_file = tmpdir.join("test.crl") - shutil.copyfile("ssl/client-certs/client-ca.der.crl", crl_file) - cache = CRLCache("ssl/server-certs/ca-chain.pem", crl_locations=[crl_file]) - cert = open("ssl/client-certs/atat.mil.crt", "rb").read() - assert cache.crl_check(cert) - # override the original CRL with one that revokes atat.mil.crt - shutil.copyfile("tests/fixtures/test.der.crl", crl_file) - with pytest.raises(CRLInvalidException): - assert cache.crl_check(cert) +def test_can_dynamically_update_crls( + ca_key, + ca_file, + crl_file, + rsa_key, + make_x509, + make_crl, + serialize_pki_object_to_disk, +): + cache = CRLCache(ca_file, crl_locations=[crl_file]) + client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") + client_pem = client_cert.public_bytes(Encoding.PEM) + assert cache.crl_check(client_pem) + + revoked_crl = make_crl(ca_key, expired_serials=[client_cert.serial_number]) + # override the original CRL with one that revokes client_cert + serialize_pki_object_to_disk(revoked_crl, crl_file, encoding=Encoding.DER) + + with pytest.raises(CRLRevocationException): + assert cache.crl_check(client_pem) def test_throws_error_for_missing_issuer(): From ceee1f69d228fd238828f277e105efa83c42e588 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 14 Mar 2019 13:46:52 -0400 Subject: [PATCH 09/10] Rely on fixture to change app config --- tests/domain/authnid/test_crl.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index 0b1c88fa..779e3c66 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -144,7 +144,6 @@ def test_no_op_crl_cache_logs_common_name(): def test_expired_crl_raises_CRLInvalidException_with_failover_config_false( app, ca_file, expired_crl_file, ca_key, make_x509, rsa_key ): - app.config.update({"CRL_FAIL_OPEN": False}) client_cert = make_x509(rsa_key(), signer_key=ca_key, cn="chewbacca") client_pem = client_cert.public_bytes(Encoding.PEM) crl_cache = CRLCache(ca_file, crl_locations=[expired_crl_file]) From 4ec9ead1ac994f1759e7fe4392234767a3b14310 Mon Sep 17 00:00:00 2001 From: dandds <38955503+dandds@users.noreply.github.com> Date: Fri, 15 Mar 2019 14:22:13 -0400 Subject: [PATCH 10/10] Update atst/domain/authnid/crl/__init__.py Co-Authored-By: montana-mil <42577527+montana-mil@users.noreply.github.com> --- atst/domain/authnid/crl/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/atst/domain/authnid/crl/__init__.py b/atst/domain/authnid/crl/__init__.py index 26956f09..3451e30e 100644 --- a/atst/domain/authnid/crl/__init__.py +++ b/atst/domain/authnid/crl/__init__.py @@ -182,6 +182,11 @@ class CRLCache(CRLInterface): except crypto.X509StoreContextError as err: if err.args[0][0] == CRL_EXPIRED_ERROR_CODE: if app.config.get("CRL_FAIL_OPEN"): + self._log_info( + "Encountered expired CRL for certificate with CN {} and issuer CN {}, failing open.".format( + parsed.get_subject().CN, parsed.get_issuer().CN + ) + ) return True else: raise CRLInvalidException("CRL expired. Args: {}".format(err.args))