diff --git a/src/ocspdash/manager.py b/src/ocspdash/manager.py index e62b4a4..5717ae2 100644 --- a/src/ocspdash/manager.py +++ b/src/ocspdash/manager.py @@ -11,11 +11,11 @@ from dataclasses import dataclass from itertools import groupby from operator import attrgetter -from typing import Iterable, List, Mapping, Optional, Tuple +from typing import Iterable, List, Mapping, Optional, Tuple, Union from sqlalchemy import and_, create_engine, func from sqlalchemy.engine import Engine -from sqlalchemy.orm import scoped_session, sessionmaker +from sqlalchemy.orm import Session, scoped_session, sessionmaker from ocspdash.constants import OCSPDASH_DEFAULT_CONNECTION, OCSPDASH_USER_AGENT_IDENTIFIER from ocspdash.models import Authority, Base, Chain, Location, Responder, Result @@ -28,6 +28,8 @@ logger = logging.getLogger(__name__) +_SessionHint = Union[Session, scoped_session] + def _workaround_pysqlite_transaction_bug(): """Work around pysqlite transaction bug. @@ -88,12 +90,13 @@ class ResponderPayload: class Manager: """Manager for interacting with the database.""" - def __init__(self, engine: Engine, session: scoped_session, server_query: Optional[ServerQuery] = None) -> None: + def __init__(self, engine: Engine, session: _SessionHint, server_query: Optional[ServerQuery] = None) -> None: """Instantiate a Manager with instances of the objects it needs. :param engine: The database engine. :param session: The database session. - :param server_query: The server_query instance. If None, using server_query-related functionality will raise an error. + :param server_query: The server_query instance. If None, using server_query-related functionality will raise + an error. """ self.engine = engine self.session = session @@ -108,7 +111,8 @@ def from_args(cls, connection: Optional[str] = None, echo: bool = False, api_id: :param connection: An SQLAlchemy-compatible connection string. :param echo: True to echo SQL emitted by SQLAlchemy. :param api_id: The Censys API id. If None, the value will be obtained from configuration or the environment. - :param api_secret: The Censys API secret. If none, the value will be obtained from configuration or the environment. + :param api_secret: The Censys API secret. If none, the value will be obtained from configuration or the + environment. :returns: An instance of Manager configured according to the arguments provided. """ @@ -306,10 +310,7 @@ def ensure_chain(self, responder: Responder) -> Optional[Chain]: most_recent_chain = self.get_most_recent_chain_by_responder(responder) if most_recent_chain and not most_recent_chain.old: - if not most_recent_chain.expired: - return most_recent_chain - - if not responder.current: + if not most_recent_chain.expired or not responder.current: return most_recent_chain subject, issuer = self.server_query.get_certs_for_issuer_and_url(responder.authority.name, responder.url) @@ -510,12 +511,13 @@ def get_location_by_selector(self, selector: bytes) -> Optional[Location]: return self.session.query(Location).filter(Location.selector == selector).one_or_none() def process_location(self, invite_token: bytes, public_key: str) -> Optional[Location]: - """Given an invite token and public key, check for a valid invite and associate the public key with the corresponding location. + """Check for a valid invite and associate the public key with the corresponding location. :parameter invite_token: a 32-byte string corresponding to an invited Location. :parameter public_key: The public key to be associated with the Location. :returns: The Location if a valid invite was provided, otherwise None. + :raises ValueError: with an appropriate message for failures to process the location invite. """ if len(invite_token) != 32: raise ValueError('invite_token of wrong length') @@ -524,11 +526,11 @@ def process_location(self, invite_token: bytes, public_key: str) -> Optional[Loc location = self.get_location_by_selector(selector) if location is None: - raise Exception(f'location not found for selector: {selector}') + raise ValueError(f'invalid invite token') if location.pubkey: # this invite has already been used - return None + raise ValueError(f'invite expired') if not location.verify(validator): - return None + raise ValueError(f'invalid invite token') location.set_public_key(public_key) @@ -577,7 +579,10 @@ def get_most_recent_chains_for_authorities(self, n: Optional[int] = 10) -> List[ return query.all() def insert_payload(self, location: Location, results: Iterable[Mapping]): - """Take the submitted payload and insert its results into the database.""" + """Take the submitted payload and insert its results into the database. + + :raises sqlalchemy.IntegrityError: If a duplicate location is trying to be inserted. Rolls back. + """ for prepared_result_dict in results: result = Result(**prepared_result_dict) result.location = location diff --git a/src/ocspdash/models.py b/src/ocspdash/models.py index cfb532a..18c288f 100644 --- a/src/ocspdash/models.py +++ b/src/ocspdash/models.py @@ -226,6 +226,8 @@ def set_public_key(self, public_key: str): """Set the pubkey and key_id for the Location based on an input public key. :param public_key: The public key for the Location. + + :raises ValueError: if the passed-in public key is not of a valid algorithm as defined in constants.OCSPSCRAPE_PRIVATE_KEY_ALGORITHMS. """ pubkey = b64decode(public_key) loaded_pubkey = serialization.load_pem_public_key(pubkey, default_backend()) diff --git a/src/ocspdash/web/app.py b/src/ocspdash/web/app.py index 3eee694..3a4aec4 100644 --- a/src/ocspdash/web/app.py +++ b/src/ocspdash/web/app.py @@ -4,7 +4,7 @@ import logging import os -from typing import Optional +from typing import Mapping, Optional from flasgger import Swagger from flask import Flask @@ -23,22 +23,29 @@ logger = logging.getLogger('web') -def create_application(connection: Optional[str] = None, flask_debug: bool = False) -> Flask: +def create_application(connection: Optional[str] = None, flask_debug: bool = False, + db_session_options: Optional[Mapping] = None, secret_key=None) -> Flask: """Create the OCSPdash Flask application. - :param connection: Database connection string + :param connection: Database connection string, overridden by env $OCSPDASH_CONNECTION :param flask_debug: Enable Flask debug mode, overridden by env $DEBUG + :param db_session_options: Mapping of options passed to the flask-sqlalchemy constructor for eventual passage to + sqlalchemy.sessionmaker. """ app = Flask(__name__) app.config.update(dict( SQLALCHEMY_DATABASE_URI=connection or OCSPDASH_CONNECTION, SQLALCHEMY_TRACK_MODIFICATIONS=False, - SECRET_KEY=os.environ.get('SECRET_KEY', 'test key'), + SECRET_KEY=secret_key or os.environ['SECRET_KEY'], # keep it secret; keep it safe DEBUG=os.environ.get('DEBUG', flask_debug), CENSYS_API_ID=os.environ.get('CENSYS_API_ID'), CENSYS_API_SECRET=os.environ.get('CENSYS_API_SECRET'), + OCSPDASH_API_MANIFEST_DEFAULT_SIZE=int(os.environ.get('OCSPDASH_API_MANIFEST_DEFAULT_SIZE', default=10)), + OCSPDASH_API_MANIFEST_MAX_SIZE=int(os.environ.get('OCSPDASH_API_MANIFEST_MAX_SIZE', default=10)), + )) - db = OCSPSQLAlchemy(app=app) + + db = OCSPSQLAlchemy(app=app, session_options=db_session_options) Bootstrap(app) Swagger(app) # Adds Swagger UI diff --git a/src/ocspdash/web/blueprints/api.py b/src/ocspdash/web/blueprints/api.py index a63b968..1c9fc29 100644 --- a/src/ocspdash/web/blueprints/api.py +++ b/src/ocspdash/web/blueprints/api.py @@ -2,6 +2,7 @@ """The OCSPdash API blueprint.""" +import binascii import io import logging import uuid @@ -11,11 +12,13 @@ from http import HTTPStatus import jsonlines -from flask import Blueprint, abort, request +from flask import Blueprint, current_app, jsonify, request from jose import jwt from jose.exceptions import JWTError +from sqlalchemy.exc import IntegrityError from ocspdash.constants import OCSP_JWT_ALGORITHM, OCSP_RESULTS_JWT_CLAIM +from ocspdash.web.exceptions import InvalidUsage from ocspdash.web.proxies import manager jwt.decode = partial(jwt.decode, algorithms=OCSP_JWT_ALGORITHM) @@ -25,25 +28,45 @@ api = Blueprint('api', __name__) -@api.route('/register', methods=['POST']) +@api.route('/register', methods=['POST']) # noqa: C901 def register_location_key(): """Register a public key for an invited location.""" # TODO: error handling (what if no invite, what if duplicate name, etc.) - unverified_claims = jwt.get_unverified_claims(request.data) - unverified_public_key = b64decode(unverified_claims['pk']).decode('utf-8') + try: + unverified_claims = jwt.get_unverified_claims(request.data) + except jwt.JWTError as e: + raise InvalidUsage(f'failed to decode JWT: {str(e)}') + + try: + unverified_public_key = b64decode(unverified_claims['pk']).decode('utf-8') + except KeyError as e: + raise InvalidUsage(f'missing claim: {str(e)}') + except binascii.Error as e: + raise InvalidUsage(f"failed to decode 'pk' claim: {str(e)}") + except UnicodeError as e: + raise InvalidUsage(f"failed to decode 'pk' claim: {e.reason}") try: claims = jwt.decode(request.data, unverified_public_key) - except JWTError: - return abort(400) # bad input + except JWTError as e: + raise InvalidUsage(f'failed to decode JWT: {str(e)}') - public_key = claims['pk'] - invite_token = b64decode(claims['token']) + try: + public_key = claims['pk'] + except KeyError as e: + raise InvalidUsage(f'missing claim: {str(e)}') - new_location = manager.process_location(invite_token, public_key) + try: + invite_token = b64decode(claims['token']) + except KeyError as e: + raise InvalidUsage(f'missing claim: {str(e)}') + except binascii.Error as e: + raise InvalidUsage(f"failed to decode 'token' claim: {str(e)}") - if new_location is None: - return abort(400) + try: + manager.process_location(invite_token, public_key) + except ValueError as e: + raise InvalidUsage(f'failed to process invite: {str(e)}') return '', HTTPStatus.NO_CONTENT @@ -63,9 +86,9 @@ def get_manifest(): required: false type: integer """ - n = request.args.get('n', type=int, default=10) # TODO make configurable at app level - if n > 10: - abort(400, 'n too large, max is 10') # TODO get the max config value here too + n = request.args.get('n', type=int, default=current_app.config['OCSPDASH_API_MANIFEST_DEFAULT_SIZE']) + if n > current_app.config['OCSPDASH_API_MANIFEST_MAX_SIZE']: + raise InvalidUsage(f'n too large, max is {current_app.config["OCSPDASH_API_MANIFEST_MAX_SIZE"]}: {n}') manifest_lines = io.StringIO() with jsonlines.Writer(manifest_lines, sort_keys=True) as writer: writer.write_all( @@ -80,7 +103,10 @@ def get_manifest(): def _prepare_result_dictionary(result_data): certificate_chain_uuid: uuid.UUID = uuid.UUID(result_data['certificate_chain_uuid']) + chain = manager.get_chain_by_certificate_chain_uuid(certificate_chain_uuid) + if not chain: + raise ValueError(f'no chain with certificate_chain_uuid: {certificate_chain_uuid}') retrieved = datetime.strptime(result_data['time'], '%Y-%m-%dT%H:%M:%SZ') @@ -92,7 +118,7 @@ def _prepare_result_dictionary(result_data): } -@api.route('/submit', methods=['POST']) +@api.route('/submit', methods=['POST']) # noqa: C901 def submit(): """Submit scrape results. @@ -100,18 +126,53 @@ def submit(): tags: - ocsp """ - submitted_token_header = jwt.get_unverified_header(request.data) + try: + submitted_token_header = jwt.get_unverified_header(request.data) + except jwt.JWTError as e: + raise InvalidUsage(f'failed to decode JWT: {str(e)}') + + try: + key_id = uuid.UUID(submitted_token_header['kid']) + except KeyError as e: + raise InvalidUsage(f'missing header claim: {str(e)}') - key_id = uuid.UUID(submitted_token_header['kid']) submitting_location = manager.get_location_by_key_id(key_id) + if not submitting_location: + raise InvalidUsage(f'no location for key id', payload={'key_id': key_id}) try: claims = jwt.decode(request.data, submitting_location.pubkey.decode('utf-8')) - except JWTError: - return abort(400) + except JWTError as e: + raise InvalidUsage(f'failed to decode JWT: {str(e)}') - prepared_result_dicts = (_prepare_result_dictionary(result_data) - for result_data in claims[OCSP_RESULTS_JWT_CLAIM]) - manager.insert_payload(submitting_location, prepared_result_dicts) + try: + results = claims[OCSP_RESULTS_JWT_CLAIM] + except KeyError as e: + raise InvalidUsage(f'missing claim: {str(e)}') + + prepared_result_dicts = [] + for result_data in results: + try: + prepared_dict = _prepare_result_dictionary(result_data) + prepared_result_dicts.append(prepared_dict) + except (KeyError, ValueError): + raise InvalidUsage('invalid result data', payload={'result': result_data}) - return ('', HTTPStatus.NO_CONTENT) + try: + manager.insert_payload(submitting_location, prepared_result_dicts) + except IntegrityError: + manager.session.rollback() + raise + + return '', HTTPStatus.NO_CONTENT + + +@api.errorhandler(InvalidUsage) +def handle_invalid_usage(error: InvalidUsage): + """Handle InvalidUsage exceptions raised by views in the blueprint. + + :param error: The exception that caused this handler to be called. + """ + response = jsonify(error) + response.status_code = error.status_code + return response diff --git a/src/ocspdash/web/exceptions.py b/src/ocspdash/web/exceptions.py new file mode 100644 index 0000000..841c0dc --- /dev/null +++ b/src/ocspdash/web/exceptions.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- + +"""Exceptions and Errors for the OCSPdash web package.""" + +from http import HTTPStatus +from typing import Mapping + + +class InvalidUsage(Exception): + """An Exception to be raised by a view for invalid usage of the endpoint.""" + + status_code = HTTPStatus.BAD_REQUEST + + def __init__(self, message: str, status_code: HTTPStatus=None, payload: Mapping=None) -> None: + """Create an InvalidUsage exception. + + :param message: The message for the exception; will be placed in the `message` key of the JSON returned in the response. + :param status_code: An HTTP status code for the response; default is 400 BAD REQUEST. It will also be placed in the `status` key of the returned JSON. + :param payload: A mapping that can be jsonified by Flask; will be returned as JSON in the response alongside the message. + """ + super().__init__(self) + self.message = message + if status_code is not None: + self.status_code = status_code + self.payload = payload + + def to_json(self) -> Mapping: + """Return a representation of the exception suitable to passed for JSON conversion.""" + rv = dict(self.payload or ()) + rv['message'] = self.message + rv['status'] = self.status_code + return rv diff --git a/src/ocspdash/web/extension.py b/src/ocspdash/web/extension.py index 8637677..d0d01b1 100644 --- a/src/ocspdash/web/extension.py +++ b/src/ocspdash/web/extension.py @@ -2,6 +2,8 @@ """An extension of Flask-SQLAlchemy to support the OCSPdash manager.""" +from __future__ import annotations + from flask import Flask from flask_sqlalchemy import SQLAlchemy, get_state @@ -26,6 +28,14 @@ def init_app(self, app: Flask): self.manager = Manager(engine=self.engine, session=self.session) + @staticmethod + def get_db(app: Flask) -> SQLAlchemy: # noqa: F821 + """Get the db from an app. + + :param app: A Flask app + """ + return get_state(app).db + @staticmethod def get_manager(app: Flask) -> Manager: """Get the manager from an app. diff --git a/tests/conftest.py b/tests/conftest.py index 448192d..a7180b5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,13 +3,15 @@ """Test configuration module for OCSPdash.""" import logging +import os import pytest from sqlalchemy import create_engine, event -from sqlalchemy.orm import scoped_session, sessionmaker +from sqlalchemy.orm import Session, scoped_session, sessionmaker from ocspdash.manager import Manager -from ocspdash.models import Base +from ocspdash.models import Authority, Base, Chain, Responder +from ocspdash.web import create_application from .constants import TEST_CONNECTION logger = logging.getLogger(__name__) @@ -61,7 +63,7 @@ def manager_session(rfc): logger.debug('creating sessionmaker') session_maker = sessionmaker(bind=connection) logger.debug('creating scoped_session') - session = scoped_session(session_maker) + session: Session = scoped_session(session_maker) @event.listens_for(session, 'after_transaction_end') def restart_savepoint(session, transaction): @@ -131,3 +133,115 @@ def manager_function(manager_session): # is rolled back logger.debug('rolling back transaction from function') transaction.rollback() + + +@pytest.fixture(scope='session') +def client_session(rfc): + """Create a Flask test client with a temporary SQLite database for a test session. + + All DB operations will be rolled back upon the end of a test session. + See below for a fixture that rolls back after every test function. + + Note that a connection object is yielded as well, which is necessary for the function-scoped version, so you must unpack the actual test client object if using this fixture directly. + + :yields: a 2-tuple of test client and Connection + """ + logger.debug('creating engine for web client') + engine = create_engine(rfc) + + logger.debug('creating connection for web client') + connection = engine.connect() + + secret_key = os.urandom(8) + app = create_application(connection=rfc, db_session_options={'bind': connection}, secret_key=secret_key) + app.testing = True + + @event.listens_for(app.manager.session, 'after_transaction_end') + def restart_savepoint(session, transaction): + logger.debug('called restart_savepoint for web client') + if transaction.nested and not transaction._parent.nested: + logger.debug('restarting savepoint for web client') + # ensure that state is expired the way + # session.commit() normally does + logger.debug('expiring for web client') + session.expire_all() + + logger.debug('beginning nested in restart_savepoint for web client') + session.begin_nested() + logger.debug('end of restart_savepoint if statement for web client') + logger.debug('end of restart_savepoint for web client') + + logger.debug('beginning transaction in session for web client') + transaction = connection.begin() + + logger.debug('beginning nested in session for web client') + app.manager.session.begin_nested() + + logger.debug('yielding from session for web client') + yield app.test_client(), connection + + logger.debug('closing session from session for web client') + app.manager.session.close() + logger.debug('rolling back transaction from session for web client') + transaction.rollback() + + logger.debug('closing connection for web client') + connection.close() + + +@pytest.fixture(scope='function') +def client_function(client_session): + """Create a test client fixture with a temporary SQLite database for a test function. + + All DB operations will be rolled back upon the end of the test function. + + :yields: a Flask test client + """ + logger.debug('unpacking client and connection for web client') + client = client_session[0] + connection = client_session[1] + + logger.debug('beginning transaction in function for web client') + transaction = connection.begin() + logger.debug('beginning nested in function for web client') + client.application.manager.session.begin_nested() + + logger.debug('yielding client for web client') + yield client + + logger.debug('closing session from function for web client') + client.application.manager.session.close() + + # rollback - everything that happened with the + # Session above (including all calls to commit()) + # is rolled back + logger.debug('rolling back transaction from function for web client') + transaction.rollback() + + +# TODO: fixture to pre-fill DB with some stuff for the client to test on +def prefill_my_database(s: Session): + """Add some test data to the database. + + :param s: A session to the database. Could be wrapped for a manager or transaction or whatever + """ + a1 = Authority(name='a1', cardinality=5) + a2 = Authority(name='a2', cardinality=5) + a3 = Authority(name='a3', cardinality=5) + s.add_all([a1, a2, a3]) + s.commit() + + r1 = Responder(authority=a1, url='url1', cardinality=5) + r2 = Responder(authority=a1, url='url2', cardinality=5) + r3 = Responder(authority=a2, url='url3', cardinality=5) + r4 = Responder(authority=a2, url='url4', cardinality=5) + + s.add_all([r1, r2, r3, r4]) + + c1 = Chain(responder=r1, subject=b'c1s', issuer=b'c1i') + c2 = Chain(responder=r2, subject=b'c2s', issuer=b'c2i') + c3 = Chain(responder=r3, subject=b'c3s', issuer=b'c3i') + c4 = Chain(responder=r4, subject=b'c4s', issuer=b'c4i') + + s.add_all([c1, c2, c3, c4]) + s.commit() diff --git a/tests/test_api.py b/tests/test_api.py new file mode 100644 index 0000000..3c18193 --- /dev/null +++ b/tests/test_api.py @@ -0,0 +1,9 @@ +# -*- coding: utf-8 -*- + +"""Test the functionality of the Flask API.""" + + +def test_get_manifest_jsonl(client_function): + """Test that /manifest.jsonl returns a 200.""" + resp = client_function.get('/api/v0/manifest.jsonl') + assert resp.status_code == 200