From 2781434d5a051119cb73937b3fb99791ad6da524 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sun, 6 Sep 2026 21:24:08 +0100 Subject: [PATCH 1/8] Make the public key fetch asynchronous with no synchronous form Every function in owid.public_key_fetch that reaches the network is now a coroutine, so a caller awaits it, and the synchronous public_key_pem, signature_status and verify are removed rather than kept beside an async twin. public_key_url and clear_cache make no request and stay ordinary functions. Importing owid still never loads the fetch. A synchronous fetch blocks whichever thread calls it for up to the ten second timeout, which in an async application is the event loop itself, and keeping a synchronous twin beside the coroutine would leave that path in place for the next caller to pick up. Removing it is the point of the change. The transport a caller may supply is now an async callable of the same shape, taking the URL and the timeout and returning the response code and the body. The default transport runs the existing urllib code on a worker thread through asyncio.to_thread, which is blocking I/O on a worker thread rather than a non-blocking request, and the module and the README say so and point at an aiohttp or httpx based transport for a fully non-blocking one. The redirect refusal and the date parameter on the URL are unchanged and stay in the default transport. No dependency is added. The key cache is kept, and two callers who await the same key at the same moment now share one in-flight request, held as one asyncio task per URL, instead of making two. A failed request is forgotten so the next caller tries again, and cancelling one waiter does not cancel the request another waiter is still on. The fetch tests move to the standard library IsolatedAsyncioTestCase, so no test dependency is added, and gain tests for the shared in-flight request, for a failure reaching every waiter, for cancellation and for the contract that every network reaching function is a coroutine. The README example uses asyncio.run and every mention of the removed synchronous form is updated. --- README.md | 52 +++--- owid/public_key_fetch.py | 157 +++++++++++++----- tests/test_public_key_fetch.py | 294 +++++++++++++++++++++++++++------ 3 files changed, 392 insertions(+), 111 deletions(-) diff --git a/README.md b/README.md index a3dea53..d850b7f 100644 --- a/README.md +++ b/README.md @@ -24,9 +24,10 @@ format, the ECDSA signing and verification, a creator that binds a domain to a signing key, framework agnostic helpers for the well known end points, and the fetch of another creator's public key from its well known end point. The core has no network access of its own. The one module that reaches the -network is `owid.public_key_fetch`, which uses the standard library `urllib`, -is imported only by a caller that asks for it, and takes a transport of the -caller's own where `urllib` is not the right client. +network is `owid.public_key_fetch`, whose functions are coroutines a caller +awaits. It is imported only by a caller that asks for it, runs the standard +library `urllib` on a worker thread unless the caller supplies a transport of +its own, and adds no dependency. Version 3 is the current version produced for new OWIDs. Versions 1 and 2 are deprecated and are supported for reading existing data only. @@ -173,9 +174,14 @@ identifier it signed under an earlier key reads as not matching, which is why a creator that rotates its key has to honour the date. Keys already fetched are held against the URL they came from, which names the domain, the version and the minute, up to 1024 of them before the store is emptied, and -`clear_cache()` empties it on demand. Each request waits at most ten seconds. +`clear_cache()` empties it on demand. Two callers who await the same key at +the same moment share one request rather than making two. Each request waits +at most ten seconds. Every function that reaches the network is a coroutine, +so a caller awaits it, and there is no synchronous form. ```python +import asyncio + from owid import SignatureStatus, public_key_fetch # A creator whose key this example never actually asks for. The transport @@ -185,11 +191,11 @@ from owid import SignatureStatus, public_key_fetch remote_creator = Creator("creator.invalid", Crypto.new()) remote = remote_creator.create_string("from another creator") -def unreachable(url, timeout): +async def unreachable(url, timeout): raise OSError("this example makes no request") -fetched = public_key_fetch.signature_status( - remote, "https", transport=unreachable +fetched = asyncio.run( + public_key_fetch.signature_status(remote, "https", transport=unreachable) ) if fetched is SignatureStatus.KEY_UNAVAILABLE: # The key could not be obtained, so the signature was never examined. @@ -199,9 +205,16 @@ assert fetched is SignatureStatus.KEY_UNAVAILABLE ``` A caller whose environment needs its own HTTP client passes a transport as -the last argument, being a callable that takes the URL and the timeout in -seconds, returns the response code and the body as bytes, and raises -`OSError` where no response could be obtained at all. +the last argument, being an `async` callable that takes the URL and the +timeout in seconds, returns the response code and the body as bytes, and +raises `OSError` where no response could be obtained at all. The default +transport runs `urllib` on a worker thread through `asyncio.to_thread`, which +is blocking I/O on a worker thread rather than a non-blocking request, so the +event loop is free during the request but a thread is not. Supply an +`aiohttp` or `httpx` based transport for a fully non-blocking one. The +default transport never follows a redirect, so a creator whose domain answers +with a 3xx reads as a key that is unavailable rather than as a key served by +whatever host the redirect named. Where the whole published schedule is already held, `PublicKeySchedule` chooses the key without any request. The rule is the one the cloud itself @@ -437,15 +450,16 @@ opaque crypto error. - `public_key_url(owid, scheme)` builds the request, naming the version of the OWID and the minute the OWID was signed. -- `public_key_pem(owid, scheme, transport=None)` returns the key, raising - `PublicKeyFetchError`, which carries the status to report, the domain and - the response code. -- `signature_status(owid, scheme, others=None, transport=None)` answers with - the status, so a key that could not be fetched is `KEY_UNAVAILABLE`, one - that could not be read is `INVALID_KEY`, and neither is mistaken for a - signature that does not match. `verify` takes the same arguments and - answers True only for `SIGNATURE_VALID`. -- `clear_cache()` empties the keys already fetched. +- `await public_key_pem(owid, scheme, transport=None)` returns the key, + raising `PublicKeyFetchError`, which carries the status to report, the + domain and the response code. +- `await signature_status(owid, scheme, others=None, transport=None)` answers + with the status, so a key that could not be fetched is `KEY_UNAVAILABLE`, + one that could not be read is `INVALID_KEY`, and neither is mistaken for a + signature that does not match. `verify` takes the same arguments, is + awaited in the same way, and answers True only for `SIGNATURE_VALID`. +- `clear_cache()` empties the keys already fetched and forgets the requests + still in flight. It makes no request and is not awaited. `PublicKeySchedule` and `DatedPublicKey` diff --git a/owid/public_key_fetch.py b/owid/public_key_fetch.py index edeb52a..e6299df 100644 --- a/owid/public_key_fetch.py +++ b/owid/public_key_fetch.py @@ -27,11 +27,19 @@ not matching, which is why a creator that rotates its key has to honour the date. -Only the standard library is used, through urllib, so the package keeps its -promise of no dependency beyond cryptography. This module is the one place in -the package that reaches the network. It is imported only when a caller asks -for it, and every function takes a transport of the caller's own for an -environment where urllib is not the right client. +Every function in this module that reaches the network is a coroutine, so a +caller awaits it, and there is no synchronous form of any of them. The +default transport runs the standard library urllib on a worker thread through +asyncio.to_thread, which is blocking I/O on a worker thread rather than a +non-blocking request, so the event loop is free for the length of the request +but a thread is not. Supply an aiohttp or httpx based transport for a fully +non-blocking one. Only the standard library is used here, so the package +keeps its promise of no dependency beyond cryptography. This module is the +one place in the package that reaches the network, and it is imported only +when a caller asks for it. + +Keys already fetched are held, and two callers who await the same key at the +same moment share one request rather than making two. The Java port answers the same question with PublicKeyFetch, the Rust port with Owid::verify_status and the Go port with SignatureStatusFromDomain. @@ -39,12 +47,13 @@ from __future__ import annotations +import asyncio import http.client import threading import urllib.error import urllib.parse import urllib.request -from typing import Callable, Dict, Optional, Sequence, Tuple +from typing import Awaitable, Callable, Dict, Optional, Sequence, Tuple from . import endpoints, io from .error import OwidError, PublicKeyFetchError @@ -63,12 +72,14 @@ #: bytes, so a body beyond this is not a key and is not held or decoded. MAXIMUM_RESPONSE_BYTES = 65536 -#: A transport takes the URL and the timeout in seconds and returns the -#: response code and the body. It raises OSError where no response could be -#: obtained at all, which is what urllib raises for a refused connection, a -#: name that does not resolve and a timeout. Supply one where urllib is not -#: the right client, for example behind a proxy that needs its own set up. -Transport = Callable[[str, float], Tuple[int, bytes]] +#: A transport is an async callable that takes the URL and the timeout in +#: seconds and returns the response code and the body. It raises OSError +#: where no response could be obtained at all, which is what urllib raises +#: for a refused connection, a name that does not resolve and a timeout. +#: Supply one where urllib on a worker thread is not the right client, for +#: example an aiohttp or httpx based transport for a fully non-blocking +#: request, or behind a proxy that needs its own set up. +Transport = Callable[[str, float], Awaitable[Tuple[int, bytes]]] #: The schemes that make an HTTP request. A caller chooses the scheme, and one #: that reads something other than a creator, such as file, is refused rather @@ -83,6 +94,18 @@ #: the version and the minute, and the key a creator published for a minute #: in the past does not change. _cache: Dict[str, str] = {} + +#: Fetches still running, held against the URL, so that a caller who asks for +#: a key while the request for it is in flight awaits that request rather +#: than making another. An entry is removed when its fetch finishes, whatever +#: the outcome. A task belongs to the event loop that created it, so a caller +#: on a different loop, which only happens where two loops run in two +#: threads, cannot await it and starts a fetch of its own. +_in_flight: Dict[str, "asyncio.Task[str]"] = {} + +#: Guards both stores. An event loop runs one coroutine at a time, but the +#: stores are shared by every loop in the process, and the lock is only ever +#: held across a few dictionary operations and never across an await. _lock = threading.Lock() @@ -124,7 +147,7 @@ def public_key_url(owid: Owid, scheme: str) -> str: ) -def public_key_pem( +async def public_key_pem( owid: Owid, scheme: str, transport: Optional[Transport] = None ) -> str: """Returns the public key PEM of the creator of the OWID, for the date the @@ -134,12 +157,12 @@ def public_key_pem( status to report for the identifier, and OwidError if the OWID, the scheme or the domain is not usable. """ - return _public_key_pem_at_url( + return await _public_key_pem_at_url( public_key_url(owid, scheme), owid.domain, transport ) -def signature_status( +async def signature_status( owid: Owid, scheme: str, others: Optional[Sequence[Owid]] = None, @@ -161,10 +184,10 @@ def signature_status( url = public_key_url(owid, scheme) except OwidError: return SignatureStatus.KEY_UNAVAILABLE - return _signature_status_at_url(owid, url, others, transport) + return await _signature_status_at_url(owid, url, others, transport) -def verify( +async def verify( owid: Owid, scheme: str, others: Optional[Sequence[Owid]] = None, @@ -174,19 +197,21 @@ def verify( creator served for the date the OWID carries. Every other outcome, a signature that does not match included, is False, so ask signature_status where the difference changes what the caller does.""" - status = signature_status(owid, scheme, others, transport) + status = await signature_status(owid, scheme, others, transport) return status is SignatureStatus.SIGNATURE_VALID def clear_cache() -> None: - """Empties the cache of keys already fetched. Provided so that a long - running process can release the memory, and so that a test can start from - a known state.""" + """Empties the cache of keys already fetched, and forgets the fetches + still in flight so that the next caller for any key starts a request of + its own. Provided so that a long running process can release the memory, + and so that a test can start from a known state.""" with _lock: _cache.clear() + _in_flight.clear() -def _signature_status_at_url( +async def _signature_status_at_url( owid: Owid, url: str, others: Optional[Sequence[Owid]] = None, @@ -196,7 +221,7 @@ def _signature_status_at_url( that the tests drive the real fetch against a key end point the tests can stand up locally rather than against a near copy of the fetch.""" try: - pem = _public_key_pem_at_url(url, owid.domain, transport) + pem = await _public_key_pem_at_url(url, owid.domain, transport) except PublicKeyFetchError as failed: return failed.status except OwidError: @@ -204,24 +229,61 @@ def _signature_status_at_url( return owid.signature_status(pem, others) -def _public_key_pem_at_url( +async def _public_key_pem_at_url( url: str, domain: str, transport: Optional[Transport] = None ) -> str: """Fetches the PEM at the URL, answering from the cache where the same URL - has already been fetched.""" + has already been fetched, and awaiting the request already in flight + where another caller on this event loop is fetching the same URL now.""" + loop = asyncio.get_running_loop() with _lock: cached = _cache.get(url) - if cached is not None: - return cached - pem = _read(url, domain, transport) - with _lock: - if len(_cache) >= MAXIMUM_CACHED_KEYS: - _cache.clear() - _cache[url] = pem - return pem - - -def _read(url: str, domain: str, transport: Optional[Transport]) -> str: + if cached is not None: + return cached + fetch = _in_flight.get(url) + if fetch is None or fetch.get_loop() is not loop: + fetch = loop.create_task(_fetch_and_hold(url, domain, transport)) + fetch.add_done_callback(_mark_observed) + _in_flight[url] = fetch + # Shielded, because cancelling one caller must not cancel the request + # that other callers are waiting on, and a request on a worker thread + # cannot be stopped part way in any case. The answer still lands in the + # cache for the next caller. + return await asyncio.shield(fetch) + + +async def _fetch_and_hold( + url: str, domain: str, transport: Optional[Transport] +) -> str: + """The one request for a URL, run as a task that every caller waiting for + that URL awaits. Holds the answer, and forgets the task whatever the + outcome so that a failed fetch is tried again by the next caller.""" + task = asyncio.current_task() + pem: Optional[str] = None + try: + pem = await _read(url, domain, transport) + return pem + finally: + with _lock: + if _in_flight.get(url) is task: + del _in_flight[url] + if pem is not None: + if len(_cache) >= MAXIMUM_CACHED_KEYS: + _cache.clear() + _cache[url] = pem + + +def _mark_observed(task: "asyncio.Task[str]") -> None: + """Reads the outcome of a finished fetch, so that a failure nobody was + left waiting for, because every caller was cancelled, is not reported by + asyncio as an exception that was never retrieved.""" + if not task.cancelled(): + task.exception() + + +async def _read( + url: str, domain: str, transport: Optional[Transport] +) -> str: """Performs the request and returns the body as text.""" scheme = urllib.parse.urlsplit(url).scheme.lower() if scheme not in _ACCEPTED_SCHEMES: @@ -235,7 +297,9 @@ def _read(url: str, domain: str, transport: Optional[Transport]) -> str: domain, ) try: - code, body = (transport or _urllib_transport)(url, TIMEOUT_SECONDS) + code, body = await (transport or _urllib_transport)( + url, TIMEOUT_SECONDS + ) except (OSError, ValueError, http.client.HTTPException) as failed: # A refused connection, a name that does not resolve and a timeout # all arrive here, and all of them mean the signature was never @@ -287,11 +351,20 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): _opener = urllib.request.build_opener(_NoRedirects()) -def _urllib_transport(url: str, timeout: float) -> Tuple[int, bytes]: - """The transport used unless the caller supplies one. A refusal carrying - a response code is returned as that code, and only the failure to obtain - any response at all is raised. Redirects are not followed (see - _NoRedirects).""" +async def _urllib_transport(url: str, timeout: float) -> Tuple[int, bytes]: + """The transport used unless the caller supplies one. It runs urllib on + a worker thread, which is blocking I/O on a worker thread rather than a + non-blocking request, so the event loop is free during the request but a + thread is not. Supply an aiohttp or httpx based transport for a fully + non-blocking one. Redirects are not followed (see _NoRedirects).""" + return await asyncio.to_thread(_urllib_request, url, timeout) + + +def _urllib_request(url: str, timeout: float) -> Tuple[int, bytes]: + """The blocking request behind the default transport, run on a worker + thread and never on the event loop. A refusal carrying a response code + is returned as that code, and only the failure to obtain any response at + all is raised.""" request = urllib.request.Request( url, headers={"Accept": "text/plain"}, method="GET" ) diff --git a/tests/test_public_key_fetch.py b/tests/test_public_key_fetch.py index 724162e..e85f63c 100644 --- a/tests/test_public_key_fetch.py +++ b/tests/test_public_key_fetch.py @@ -20,10 +20,16 @@ against a stand in on the loopback address which serves the real published 51d.es schedule. The URL under test is the one the package builds, with only the host replaced, so a fault in the path or the query is caught here. + +The fetch is asynchronous and has no synchronous form, so every test that +reaches it is a coroutine run by the standard library's +IsolatedAsyncioTestCase on an event loop of its own. """ from __future__ import annotations +import asyncio +import inspect import os import pathlib import unittest @@ -70,7 +76,7 @@ def crafted(version: Version, domain: str, date: datetime) -> Owid: return result.owid -class PublicKeyFetchTests(unittest.TestCase): +class PublicKeyFetchTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: # Keys are held against the URL they were fetched from, and a test # that counts requests has to start from nothing held. @@ -89,6 +95,30 @@ def end_point(self, answer: Answer = Answer.SCHEDULE) -> KeyEndPoint: self.started.append(end_point) return end_point + def test_every_function_that_reaches_the_network_is_a_coroutine( + self, + ) -> None: + """The fetch has no synchronous form. Building the URL and emptying + the cache touch nothing outside the process and stay ordinary + functions, and everything that could make a request is awaited.""" + for reaches_the_network in ( + public_key_fetch.public_key_pem, + public_key_fetch.signature_status, + public_key_fetch.verify, + ): + self.assertTrue( + inspect.iscoroutinefunction(reaches_the_network), + "{0} must be awaited".format(reaches_the_network.__name__), + ) + for stays_in_process in ( + public_key_fetch.public_key_url, + public_key_fetch.clear_cache, + ): + self.assertFalse( + inspect.iscoroutinefunction(stays_in_process), + "{0} makes no request".format(stays_in_process.__name__), + ) + def test_url_names_the_minute_the_identifier_was_created(self) -> None: """The URL names the minute the identifier was created, which is the value the end point selects a key by, and it names the well known @@ -124,7 +154,7 @@ def test_url_of_a_newly_signed_owid_names_its_own_minute(self) -> None: "should name the minute the OWID was signed", ) - def test_dated_fetch_verifies_an_identifier_from_an_earlier_key_week( + async def test_dated_fetch_verifies_an_identifier_from_an_earlier_key_week( self, ) -> None: """The fetch asks for the key in force when the identifier was signed @@ -134,7 +164,7 @@ def test_dated_fetch_verifies_an_identifier_from_an_earlier_key_week( end_point = self.end_point() self.assertIs( SignatureStatus.SIGNATURE_VALID, - public_key_fetch._signature_status_at_url( + await public_key_fetch._signature_status_at_url( owid, end_point.url_for(owid), ALONE ), "should verify against the key in force when it was signed", @@ -145,7 +175,7 @@ def test_dated_fetch_verifies_an_identifier_from_an_earlier_key_week( "the request should name the minute the identifier was created", ) - def test_undated_fetch_leaves_an_earlier_weeks_identifier_unverified( + async def test_undated_fetch_leaves_an_earlier_weeks_identifier_unverified( self, ) -> None: """The same identifier against the same end point without the date, @@ -158,13 +188,17 @@ def test_undated_fetch_leaves_an_earlier_weeks_identifier_unverified( undated = end_point.base + "/owid/api/v3/public-key?format=pkcs" self.assertIs( SignatureStatus.SIGNATURE_INVALID, - public_key_fetch._signature_status_at_url(owid, undated, ALONE), + await public_key_fetch._signature_status_at_url( + owid, undated, ALONE + ), "an undated request gets the key in force at the request, which " "did not sign it", ) self.assertEqual([None], end_point.dates(), "the request carried no date") - def test_a_key_the_end_point_cannot_serve_is_key_unavailable(self) -> None: + async def test_a_key_the_end_point_cannot_serve_is_key_unavailable( + self, + ) -> None: """An end point that cannot serve a key for the date leaves the signature unjudged rather than reporting a genuine identifier as a forgery.""" @@ -178,32 +212,34 @@ def test_a_key_the_end_point_cannot_serve_is_key_unavailable(self) -> None: ) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch._signature_status_at_url(owid, url, ALONE), + await public_key_fetch._signature_status_at_url(owid, url, ALONE), "no key means the signature was never examined", ) - def test_a_refused_request_carries_the_status_and_the_code(self) -> None: + async def test_a_refused_request_carries_the_status_and_the_code( + self, + ) -> None: """The refusal carries the code and the domain, not only a message.""" end_point = self.end_point() url = end_point.base + "/owid/api/v3/public-key?date=0&format=pkcs" with self.assertRaises(PublicKeyFetchError) as refused: - public_key_fetch._public_key_pem_at_url(url, "51d.es") + await public_key_fetch._public_key_pem_at_url(url, "51d.es") self.assertIs(SignatureStatus.KEY_UNAVAILABLE, refused.exception.status) self.assertEqual(404, refused.exception.status_code) self.assertEqual("51d.es", refused.exception.domain) - def test_a_malformed_date_is_refused_by_the_end_point(self) -> None: + async def test_a_malformed_date_is_refused_by_the_end_point(self) -> None: """The stand in refuses a date that is not a count of minutes with a 400, as the cloud does, and the package reports the refusal as a key that could not be obtained.""" end_point = self.end_point() url = end_point.base + "/owid/api/v3/public-key?date=abc&format=pkcs" with self.assertRaises(PublicKeyFetchError) as refused: - public_key_fetch._public_key_pem_at_url(url, "51d.es") + await public_key_fetch._public_key_pem_at_url(url, "51d.es") self.assertIs(SignatureStatus.KEY_UNAVAILABLE, refused.exception.status) self.assertEqual(400, refused.exception.status_code) - def test_an_end_point_that_cannot_be_reached_is_key_unavailable( + async def test_an_end_point_that_cannot_be_reached_is_key_unavailable( self, ) -> None: """An end point that cannot be reached at all leaves the signature @@ -212,14 +248,17 @@ def test_an_end_point_that_cannot_be_reached_is_key_unavailable( owid = key_fixtures.identifier() end_point = KeyEndPoint() url = end_point.url_for(owid) - end_point.stop() + # Stopping the server waits for its polling loop to notice, up to + # half a second, so it is done off the event loop rather than + # blocking the loop for that long. + await asyncio.to_thread(end_point.stop) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch._signature_status_at_url(owid, url, ALONE), + await public_key_fetch._signature_status_at_url(owid, url, ALONE), "a connection that is refused leaves the signature unjudged", ) - def test_a_redirect_is_not_followed(self) -> None: + async def test_a_redirect_is_not_followed(self) -> None: """A creator whose domain answers with a redirect does not get the key at the other end trusted as its own. The answer is that the key is unavailable, carrying the 302, and the request that would have @@ -229,31 +268,31 @@ def test_a_redirect_is_not_followed(self) -> None: owid = key_fixtures.identifier() end_point = self.end_point(Answer.REDIRECT) with self.assertRaises(PublicKeyFetchError) as refused: - public_key_fetch._public_key_pem_at_url( + await public_key_fetch._public_key_pem_at_url( end_point.url_for(owid), owid.domain ) self.assertIs(SignatureStatus.KEY_UNAVAILABLE, refused.exception.status) self.assertEqual(302, refused.exception.status_code) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch._signature_status_at_url( + await public_key_fetch._signature_status_at_url( owid, end_point.url_for(owid), ALONE ), ) - def test_a_key_that_cannot_be_read_is_invalid_key(self) -> None: + async def test_a_key_that_cannot_be_read_is_invalid_key(self) -> None: """Text shaped like a PEM that holds no key is a fault in the key, and never a signature that does not match.""" owid = key_fixtures.identifier() end_point = self.end_point(Answer.BROKEN_KEY) self.assertIs( SignatureStatus.INVALID_KEY, - public_key_fetch._signature_status_at_url( + await public_key_fetch._signature_status_at_url( owid, end_point.url_for(owid), ALONE ), ) - def test_keys_are_held_per_request_and_not_per_domain(self) -> None: + async def test_keys_are_held_per_request_and_not_per_domain(self) -> None: """Keys are held against the URL they came from, which names the minute, so two identifiers from different weeks fetch two different keys and a key held for one week never answers for another. A store @@ -268,15 +307,15 @@ def test_keys_are_held_per_request_and_not_per_domain(self) -> None: later = crafted( Version.VERSION3, key_fixtures.IDENTIFIER_DOMAIN, IDENTIFIER_DATE ) - first = public_key_fetch._public_key_pem_at_url( + first = await public_key_fetch._public_key_pem_at_url( end_point.url_for(earlier), key_fixtures.IDENTIFIER_DOMAIN ) - second = public_key_fetch._public_key_pem_at_url( + second = await public_key_fetch._public_key_pem_at_url( end_point.url_for(later), key_fixtures.IDENTIFIER_DOMAIN ) self.assertNotEqual(first, second, "two weeks, two keys") self.assertEqual(2, len(end_point.dates()), "one request per week") - again = public_key_fetch._public_key_pem_at_url( + again = await public_key_fetch._public_key_pem_at_url( end_point.url_for(earlier), key_fixtures.IDENTIFIER_DOMAIN ) self.assertEqual(first, again, "the held key is the one fetched for that week") @@ -285,7 +324,7 @@ def test_keys_are_held_per_request_and_not_per_domain(self) -> None: ) self.assertIn("BEGIN PUBLIC KEY", first) - def test_the_cache_is_bounded(self) -> None: + async def test_the_cache_is_bounded(self) -> None: """When the store reaches its bound it is emptied and filled again, so a long running verifier never holds more than the bound.""" end_point = self.end_point() @@ -299,18 +338,159 @@ def test_the_cache_is_bounded(self) -> None: ] with mock.patch.object(public_key_fetch, "MAXIMUM_CACHED_KEYS", 2): for week in weeks: - public_key_fetch._public_key_pem_at_url( + await public_key_fetch._public_key_pem_at_url( end_point.url_for(week), key_fixtures.IDENTIFIER_DOMAIN ) self.assertEqual(3, len(end_point.dates())) # The third arrival emptied the store, so the first week is # fetched again rather than answered from what was held. - public_key_fetch._public_key_pem_at_url( + await public_key_fetch._public_key_pem_at_url( end_point.url_for(weeks[0]), key_fixtures.IDENTIFIER_DOMAIN ) self.assertEqual(4, len(end_point.dates())) - def test_a_domain_that_is_not_a_domain_name_is_refused_before_any_request( + async def test_concurrent_awaits_for_one_key_share_one_request( + self, + ) -> None: + """Two callers who await the same key while the request for it is in + flight share that request, and both receive its answer. The transport + is held open until both have asked, so the second caller cannot be + answered from the cache and has to join the request itself.""" + owid = key_fixtures.identifier() + pem = key_fixtures.schedule().key_for(owid).public_key_pem + arrived = asyncio.Event() + release = asyncio.Event() + calls: List[str] = [] + + async def held_open(url: str, timeout: float) -> Tuple[int, bytes]: + calls.append(url) + arrived.set() + await release.wait() + return 200, pem.encode("utf-8") + + first = asyncio.ensure_future( + public_key_fetch.public_key_pem(owid, "https", held_open) + ) + second = asyncio.ensure_future( + public_key_fetch.public_key_pem(owid, "https", held_open) + ) + await arrived.wait() + self.assertFalse(first.done(), "the request is still in flight") + self.assertFalse(second.done(), "the second caller is waiting on it") + release.set() + self.assertEqual([pem, pem], await asyncio.gather(first, second)) + self.assertEqual( + [public_key_fetch.public_key_url(owid, "https")], + calls, + "one request for the URL the package builds, shared by both", + ) + self.assertTrue( + await public_key_fetch.verify(owid, "https", ALONE, held_open), + "a later caller is answered from the cache", + ) + self.assertEqual(1, len(calls), "the cache answered the third caller") + + async def test_concurrent_awaits_share_one_request_to_the_end_point( + self, + ) -> None: + """The same sharing over the default transport, against the stand in + end point, which records one request for the three callers.""" + owid = key_fixtures.identifier() + end_point = self.end_point() + url = end_point.url_for(owid) + pems = await asyncio.gather( + *[ + public_key_fetch._public_key_pem_at_url(url, owid.domain) + for _ in range(3) + ] + ) + self.assertEqual(3, len(pems)) + self.assertEqual(1, len(set(pems)), "every caller received the key") + self.assertIn("BEGIN PUBLIC KEY", pems[0]) + self.assertEqual( + [str(key_fixtures.IDENTIFIER_MINUTES)], + end_point.dates(), + "three callers, one request", + ) + + async def test_a_failure_in_flight_reaches_every_waiter(self) -> None: + """A request that fails is reported to every caller waiting on it, and + then forgotten, so the next caller makes a request of its own rather + than being handed the old failure.""" + owid = key_fixtures.identifier() + release = asyncio.Event() + calls: List[str] = [] + + async def held_then_refused( + url: str, timeout: float + ) -> Tuple[int, bytes]: + calls.append(url) + await release.wait() + raise OSError("no route") + + first = asyncio.ensure_future( + public_key_fetch.signature_status( + owid, "https", ALONE, held_then_refused + ) + ) + second = asyncio.ensure_future( + public_key_fetch.signature_status( + owid, "https", ALONE, held_then_refused + ) + ) + while not calls: + await asyncio.sleep(0) + release.set() + self.assertEqual( + [SignatureStatus.KEY_UNAVAILABLE, SignatureStatus.KEY_UNAVAILABLE], + await asyncio.gather(first, second), + ) + self.assertEqual(1, len(calls), "one request, shared by both callers") + self.assertIs( + SignatureStatus.KEY_UNAVAILABLE, + await public_key_fetch.signature_status( + owid, "https", ALONE, held_then_refused + ), + ) + self.assertEqual( + 2, + len(calls), + "a failed request is not held, so the next caller asks again", + ) + + async def test_cancelling_one_waiter_leaves_the_request_running( + self, + ) -> None: + """Cancelling a caller must not cancel the request another caller is + waiting on, and a request on a worker thread could not be stopped + part way in any case, so the request completes and its answer is + held for the caller that is still waiting.""" + owid = key_fixtures.identifier() + pem = key_fixtures.schedule().key_for(owid).public_key_pem + release = asyncio.Event() + calls: List[str] = [] + + async def held_open(url: str, timeout: float) -> Tuple[int, bytes]: + calls.append(url) + await release.wait() + return 200, pem.encode("utf-8") + + first = asyncio.ensure_future( + public_key_fetch.public_key_pem(owid, "https", held_open) + ) + second = asyncio.ensure_future( + public_key_fetch.public_key_pem(owid, "https", held_open) + ) + while not calls: + await asyncio.sleep(0) + first.cancel() + with self.assertRaises(asyncio.CancelledError): + await first + release.set() + self.assertEqual(pem, await second) + self.assertEqual(1, len(calls)) + + async def test_a_domain_that_is_not_a_domain_name_is_refused_before_any_request( self, ) -> None: """The domain arrives inside an OWID, which came from outside, so a @@ -320,15 +500,17 @@ def test_a_domain_that_is_not_a_domain_name_is_refused_before_any_request( with self.assertRaises(OwidError): public_key_fetch.public_key_url(owid, "https") - def no_request(url: str, timeout: float) -> Tuple[int, bytes]: + async def no_request(url: str, timeout: float) -> Tuple[int, bytes]: self.fail("no request should be made for a refused domain") self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch.signature_status(owid, "https", ALONE, no_request), + await public_key_fetch.signature_status( + owid, "https", ALONE, no_request + ), ) - def test_a_scheme_that_does_not_make_an_http_request_is_refused( + async def test_a_scheme_that_does_not_make_an_http_request_is_refused( self, ) -> None: """A caller chooses the scheme, and one that reads something other @@ -339,14 +521,14 @@ def test_a_scheme_that_does_not_make_an_http_request_is_refused( self.assertTrue(url.startswith("file:")) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch._signature_status_at_url(owid, url, ALONE), + await public_key_fetch._signature_status_at_url(owid, url, ALONE), ) with self.assertRaises(PublicKeyFetchError) as refused: - public_key_fetch._public_key_pem_at_url(url, owid.domain) + await public_key_fetch._public_key_pem_at_url(url, owid.domain) self.assertIs(SignatureStatus.KEY_UNAVAILABLE, refused.exception.status) self.assertEqual(0, refused.exception.status_code) - def test_a_missing_owid_or_scheme_is_refused(self) -> None: + async def test_a_missing_owid_or_scheme_is_refused(self) -> None: owid = key_fixtures.identifier() with self.assertRaises(OwidError): public_key_fetch.public_key_url(None, "https") # type: ignore[arg-type] @@ -356,10 +538,12 @@ def test_a_missing_owid_or_scheme_is_refused(self) -> None: public_key_fetch.public_key_url(owid, " ") self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch.signature_status(None, "https"), # type: ignore[arg-type] + await public_key_fetch.signature_status( + None, "https" # type: ignore[arg-type] + ), ) - def test_a_transport_of_the_callers_own_is_used(self) -> None: + async def test_a_transport_of_the_callers_own_is_used(self) -> None: """A caller whose environment needs its own HTTP client supplies a transport, which is asked for the URL the package builds and whose answer is held like any other.""" @@ -367,16 +551,18 @@ def test_a_transport_of_the_callers_own_is_used(self) -> None: pem = key_fixtures.schedule().key_for(owid).public_key_pem calls: List[Tuple[str, float]] = [] - def transport(url: str, timeout: float) -> Tuple[int, bytes]: + async def transport(url: str, timeout: float) -> Tuple[int, bytes]: calls.append((url, timeout)) return 200, pem.encode("utf-8") self.assertIs( SignatureStatus.SIGNATURE_VALID, - public_key_fetch.signature_status(owid, "https", ALONE, transport), + await public_key_fetch.signature_status( + owid, "https", ALONE, transport + ), ) self.assertTrue( - public_key_fetch.verify(owid, "https", ALONE, transport) + await public_key_fetch.verify(owid, "https", ALONE, transport) ) self.assertEqual( [ @@ -389,56 +575,64 @@ def transport(url: str, timeout: float) -> Tuple[int, bytes]: "one request, for the URL the package builds, then the cache", ) - def test_verify_answers_true_only_for_a_genuine_signature(self) -> None: + async def test_verify_answers_true_only_for_a_genuine_signature( + self, + ) -> None: owid = key_fixtures.identifier() schedule = key_fixtures.schedule() keys = schedule.keys signing = schedule.key_for(owid) following = keys[keys.index(signing) + 1] - def wrong_week(url: str, timeout: float) -> Tuple[int, bytes]: + async def wrong_week(url: str, timeout: float) -> Tuple[int, bytes]: return 200, following.public_key_pem.encode("utf-8") self.assertFalse( - public_key_fetch.verify(owid, "https", ALONE, wrong_week), + await public_key_fetch.verify(owid, "https", ALONE, wrong_week), "the following week's key did not sign the identifier", ) - def test_a_transport_that_raises_is_key_unavailable(self) -> None: + async def test_a_transport_that_raises_is_key_unavailable(self) -> None: owid = key_fixtures.identifier() - def unreachable(url: str, timeout: float) -> Tuple[int, bytes]: + async def unreachable(url: str, timeout: float) -> Tuple[int, bytes]: raise OSError("no route") - def unusable(url: str, timeout: float) -> Tuple[int, bytes]: + async def unusable(url: str, timeout: float) -> Tuple[int, bytes]: raise ValueError("unknown url type") self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch.signature_status(owid, "https", ALONE, unreachable), + await public_key_fetch.signature_status( + owid, "https", ALONE, unreachable + ), ) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch.signature_status(owid, "https", ALONE, unusable), + await public_key_fetch.signature_status( + owid, "https", ALONE, unusable + ), ) - def test_a_response_larger_than_a_key_is_refused(self) -> None: + async def test_a_response_larger_than_a_key_is_refused(self) -> None: """A body beyond the bound is not a key, and is neither held nor decoded.""" owid = key_fixtures.identifier() too_large = b"x" * (public_key_fetch.MAXIMUM_RESPONSE_BYTES + 1) - def oversized(url: str, timeout: float) -> Tuple[int, bytes]: + async def oversized(url: str, timeout: float) -> Tuple[int, bytes]: return 200, too_large with self.assertRaises(PublicKeyFetchError) as refused: - public_key_fetch.public_key_pem(owid, "https", oversized) + await public_key_fetch.public_key_pem(owid, "https", oversized) self.assertIs(SignatureStatus.KEY_UNAVAILABLE, refused.exception.status) self.assertEqual(200, refused.exception.status_code) public_key_fetch.clear_cache() self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - public_key_fetch.signature_status(owid, "https", ALONE, oversized), + await public_key_fetch.signature_status( + owid, "https", ALONE, oversized + ), ) From 8532a6ac34a143843d58e592396a7070b69c98ad Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 7 Sep 2026 09:07:53 +0100 Subject: [PATCH 2/8] Hold each key against the span of minutes the creator confirmed it for The cache was keyed by the whole key url, and the url carries the date of the identifier being verified in minutes. A creator's key changes on the order of a week, so two identifiers signed a minute apart never shared an entry and a hundred identifiers over a hundred minutes made a hundred requests for one key. The cache only ever served a repeat verification of one identifier. Keys are now held by end point, which is the key url without its date, and each key carries the span of minutes the creator has confirmed it for. A key is in force from the start of its period until the next key starts, so a key the creator answers with at two minutes was in force at every minute between them. An identifier dated inside a confirmed span is verified without a request. One dated outside every span is asked about, and the answer widens the span when the same key comes back or adds a key when it does not. The span is never widened across a minute the creator has answered with another key for. A date later than now is held against now, because that is how a creator reads it. Held against the future minute, the key would still be served for that minute after the creator had rotated. A request without a date is read as now for the same reason. The bound of 1024 keys and the empty and refill are unchanged, as is the sharing of one request between callers awaiting together and the refusal to hold a failure. The same change is made to every port that caches, so the ports behave the same way. Tests cover a minute between two confirmed minutes being served without a request, a hundred identifiers inside one confirmed period making none, a key never being served outside its span across a rotation, and a future date being read as now. The bound test and the shared request test were checked by breaking the code and watching them fail. --- README.md | 22 ++-- owid/public_key_fetch.py | 211 +++++++++++++++++++++++++++------ tests/test_public_key_fetch.py | 171 +++++++++++++++++++++++++- 3 files changed, 362 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index d850b7f..5f055a4 100644 --- a/README.md +++ b/README.md @@ -171,13 +171,21 @@ the path is the version byte of the identifier being checked and the minutes are counted from 2020-01-01 in the same way the identifier stores its date. A creator that ignores the parameter returns its current key, so every identifier it signed under an earlier key reads as not matching, which is why -a creator that rotates its key has to honour the date. Keys already fetched -are held against the URL they came from, which names the domain, the version -and the minute, up to 1024 of them before the store is emptied, and -`clear_cache()` empties it on demand. Two callers who await the same key at -the same moment share one request rather than making two. Each request waits -at most ten seconds. Every function that reaches the network is a coroutine, -so a caller awaits it, and there is no synchronous form. +a creator that rotates its key has to honour the date. + +Keys already fetched are held by creator, each against the span of minutes +the creator has confirmed it for. A key is in force from the start of its +period until the next key starts, so a key the creator answers with at two +minutes was in force at every minute between them, and an identifier dated +inside a confirmed span is verified without a request whichever minute it +carries. One dated outside every span is asked about, which widens the span +when the same key comes back. At most 1024 keys are held across every +creator before the store is emptied and filled again, and `clear_cache()` +empties it on demand, which is how a long running process drops a key it has +learned it should no longer trust. Two callers who await the same key at the +same moment share one request rather than making two. Each request waits at +most ten seconds. Every function that reaches the network is a coroutine, so +a caller awaits it, and there is no synchronous form. ```python import asyncio diff --git a/owid/public_key_fetch.py b/owid/public_key_fetch.py index e6299df..da43db0 100644 --- a/owid/public_key_fetch.py +++ b/owid/public_key_fetch.py @@ -38,8 +38,11 @@ one place in the package that reaches the network, and it is imported only when a caller asks for it. -Keys already fetched are held, and two callers who await the same key at the -same moment share one request rather than making two. +Keys already fetched are held by creator, each against the span of minutes +the creator has confirmed it for, so an identifier dated inside a confirmed +span is verified without a request whichever minute it carries. Two callers +who await the same key at the same moment share one request rather than +making two. The Java port answers the same question with PublicKeyFetch, the Rust port with Owid::verify_status and the Go port with SignatureStatusFromDomain. @@ -53,7 +56,8 @@ import urllib.error import urllib.parse import urllib.request -from typing import Awaitable, Callable, Dict, Optional, Sequence, Tuple +from datetime import datetime, timezone +from typing import Awaitable, Callable, Dict, List, Optional, Sequence, Tuple from . import endpoints, io from .error import OwidError, PublicKeyFetchError @@ -63,9 +67,10 @@ #: How long to wait for the connection and then for the response, in seconds. TIMEOUT_SECONDS = 10.0 -#: The most keys held before the cache is emptied and filled again. A bound is -#: needed because a verifier sees identifiers from many domains and many -#: weeks, and an unbounded store would grow for as long as the process runs. +#: The most keys held before the cache is emptied and filled again, across +#: every creator. A bound is needed because a verifier sees identifiers from +#: many domains and many weeks, and an unbounded store would grow for as long +#: as the process runs. MAXIMUM_CACHED_KEYS = 1024 #: The most bytes accepted from a response. A public key PEM is a few hundred @@ -86,15 +91,48 @@ #: than opened. _ACCEPTED_SCHEMES = ("http", "https") -#: Keys already fetched, held against the URL they were fetched from. +class _HeldKey: + """One key a creator has answered with, and the span of minutes the + creator has confirmed it was in force for. + + A creator's key is in force from the start of its period until the next + key starts, so a key the creator confirms at two minutes was in force at + every minute between them. The span grows as the creator confirms the + same key for more minutes, and an identifier dated inside it is verified + without a request. + """ + + __slots__ = ("pem", "first", "last") + + def __init__(self, pem: str, minute: int) -> None: + #: The key in PEM form, as the creator served it. + self.pem = pem + #: The earliest minute the creator has confirmed the key for. + self.first = minute + #: The latest minute the creator has confirmed the key for. + self.last = minute + + def covers(self, minute: int) -> bool: + """Whether the minute lies within the confirmed span.""" + return self.first <= minute <= self.last + + +#: Keys already fetched, by the creator's key end point, which is the key URL +#: without its date. Each end point holds the keys the creator has answered +#: with, each with the span of minutes the creator has confirmed it for. #: #: The specification asks implementations to cache so that verifying many -#: identifiers does not mean repeating requests to another processor. Holding -#: the key against the whole URL is safe because the URL names the domain, -#: the version and the minute, and the key a creator published for a minute -#: in the past does not change. -_cache: Dict[str, str] = {} - +#: identifiers does not mean repeating requests to another processor. The key +#: URL carries the date of the identifier being verified, in minutes, and a +#: creator's key changes on the order of a week. Keyed by the whole URL, as +#: this cache once was, two identifiers signed a minute apart never shared an +#: entry, so a hundred identifiers over a hundred minutes made a hundred +#: requests for one key. Keyed by end point and span, an identifier dated +#: between two minutes the creator has already answered for is verified +#: without a request. +_cache: Dict[str, List[_HeldKey]] = {} +#: How many keys are held across every end point. +_held_keys = 0 #: Fetches still running, held against the URL, so that a caller who asks for #: a key while the request for it is in flight awaits that request rather #: than making another. An entry is removed when its fetch finishes, whatever @@ -102,10 +140,9 @@ #: on a different loop, which only happens where two loops run in two #: threads, cannot await it and starts a fetch of its own. _in_flight: Dict[str, "asyncio.Task[str]"] = {} - -#: Guards both stores. An event loop runs one coroutine at a time, but the -#: stores are shared by every loop in the process, and the lock is only ever -#: held across a few dictionary operations and never across an await. +#: Guards all three stores. An event loop runs one coroutine at a time, but +#: the stores are shared by every loop in the process, and the lock is only +#: ever held across a few dictionary operations and never across an await. _lock = threading.Lock() @@ -204,13 +241,23 @@ async def verify( def clear_cache() -> None: """Empties the cache of keys already fetched, and forgets the fetches still in flight so that the next caller for any key starts a request of - its own. Provided so that a long running process can release the memory, - and so that a test can start from a known state.""" + its own. A fetch already running is not stopped, and the callers awaiting + it still receive its answer. This is how a long running process drops a + key it has learned it should no longer trust, after a creator rotates its + key following a compromise, and how a test starts from a known state.""" + global _held_keys with _lock: _cache.clear() + _held_keys = 0 _in_flight.clear() +def _cached_key_count() -> int: + """How many keys the cache holds, for the tests.""" + with _lock: + return _held_keys + + async def _signature_status_at_url( owid: Owid, url: str, @@ -232,17 +279,22 @@ async def _signature_status_at_url( async def _public_key_pem_at_url( url: str, domain: str, transport: Optional[Transport] = None ) -> str: - """Fetches the PEM at the URL, answering from the cache where the same URL - has already been fetched, and awaiting the request already in flight - where another caller on this event loop is fetching the same URL now.""" + """Fetches the PEM at the URL. Answered from the cache where the creator + has already confirmed a key for the minute the URL names, from the + request already in flight where another caller on this event loop is + fetching the same URL now, and otherwise by asking the creator.""" loop = asyncio.get_running_loop() + end_point = _end_point_of(url) + minute = _minute_of(url) with _lock: - cached = _cache.get(url) - if cached is not None: - return cached + held = _held_pem(end_point, minute) + if held is not None: + return held fetch = _in_flight.get(url) if fetch is None or fetch.get_loop() is not loop: - fetch = loop.create_task(_fetch_and_hold(url, domain, transport)) + fetch = loop.create_task( + _fetch_and_hold(url, end_point, minute, domain, transport) + ) fetch.add_done_callback(_mark_observed) _in_flight[url] = fetch # Shielded, because cancelling one caller must not cancel the request @@ -253,11 +305,16 @@ async def _public_key_pem_at_url( async def _fetch_and_hold( - url: str, domain: str, transport: Optional[Transport] + url: str, + end_point: str, + minute: int, + domain: str, + transport: Optional[Transport], ) -> str: """The one request for a URL, run as a task that every caller waiting for - that URL awaits. Holds the answer, and forgets the task whatever the - outcome so that a failed fetch is tried again by the next caller.""" + that URL awaits. Holds the answer against the minute asked about, and + forgets the task whatever the outcome so that a failed fetch is tried + again by the next caller.""" task = asyncio.current_task() pem: Optional[str] = None try: @@ -265,12 +322,100 @@ async def _fetch_and_hold( return pem finally: with _lock: + if pem is not None: + _hold(end_point, minute, pem) if _in_flight.get(url) is task: del _in_flight[url] - if pem is not None: - if len(_cache) >= MAXIMUM_CACHED_KEYS: - _cache.clear() - _cache[url] = pem + + +def _end_point_of(url: str) -> str: + """The key URL without its query, which names the scheme, the creator and + the version, and so the key end point being asked.""" + return url.split("?", 1)[0] + + +def _minute_of(url: str) -> int: + """The minute the cache reads the URL as asking about. + + The date parameter where the URL carries one, and otherwise now, because + a creator answers a request without a date with the key in force now. A + date later than now is read as now as well, because that is how a creator + reads it. A schedule is published ahead of time and a key that has not + started has signed nothing, so the creator answers a future date with the + key in force now, and that answer must be held against now rather than + against a minute the creator has not spoken for. Held against the future + minute, the key would still be served for that minute after the creator + had rotated, and a genuine identifier signed then would read as not + matching. + """ + now = io.minutes_since_base(datetime.now(timezone.utc)) + query = urllib.parse.urlsplit(url).query + for name, value in urllib.parse.parse_qsl(query): + if name == "date": + try: + return min(int(value), now) + except ValueError: + return now + return now + + +def _held_pem(end_point: str, minute: int) -> Optional[str]: + """The key held for the end point whose confirmed span covers the minute, + or None where no held key does. Called under the lock.""" + for key in _cache.get(end_point, ()): + if key.covers(minute): + return key.pem + return None + + +def _hold(end_point: str, minute: int, pem: str) -> None: + """Records that the creator answered the minute with the key. Called + under the lock. + + A key already held for the end point has its span widened to take in the + minute. A key not held before is added, emptying the cache first when it + is full, because the domains and dates asked about come from the + identifiers presented to this process and the cache must not grow on + their input. + """ + global _held_keys + keys = _cache.get(end_point) + if keys is not None: + for key in keys: + if key.pem == pem and _widen(keys, key, minute): + return + if _held_keys >= MAXIMUM_CACHED_KEYS: + _cache.clear() + _held_keys = 0 + keys = None + if keys is None: + keys = [] + _cache[end_point] = keys + keys.append(_HeldKey(pem, minute)) + _held_keys += 1 + + +def _widen(keys: List[_HeldKey], key: _HeldKey, minute: int) -> bool: + """Widens the span of a held key to take in the minute, and says whether + the minute is now within it. + + The span is not widened across a minute the creator has answered with + another key for, because that would mean the creator had gone back to a + key it had left, and the minutes between the two spans are then not this + key's to claim. The key is held again as a separate span instead. + """ + if key.covers(minute): + return True + start = min(minute, key.first) + end = max(minute, key.last) + for other in keys: + if other is not key and other.last > start and other.first < end: + return False + if minute < key.first: + key.first = minute + else: + key.last = minute + return True def _mark_observed(task: "asyncio.Task[str]") -> None: diff --git a/tests/test_public_key_fetch.py b/tests/test_public_key_fetch.py index e85f63c..3e4bed7 100644 --- a/tests/test_public_key_fetch.py +++ b/tests/test_public_key_fetch.py @@ -293,8 +293,8 @@ async def test_a_key_that_cannot_be_read_is_invalid_key(self) -> None: ) async def test_keys_are_held_per_request_and_not_per_domain(self) -> None: - """Keys are held against the URL they came from, which names the - minute, so two identifiers from different weeks fetch two different + """Keys are held against the span of minutes the creator confirmed + them for, so two identifiers from different weeks fetch two different keys and a key held for one week never answers for another. A store keyed by domain alone would hand the second identifier the first one's key.""" @@ -349,6 +349,173 @@ async def test_the_cache_is_bounded(self) -> None: ) self.assertEqual(4, len(end_point.dates())) + @staticmethod + def _at(moment: datetime) -> Owid: + """An identifier from the fixture domain dated at the moment.""" + return crafted(Version.VERSION3, key_fixtures.IDENTIFIER_DOMAIN, moment) + + @staticmethod + async def _pem_at(end_point: KeyEndPoint, moment: datetime) -> str: + """The PEM the fetch answers for an identifier dated at the moment.""" + return await public_key_fetch._public_key_pem_at_url( + end_point.url_for(PublicKeyFetchTests._at(moment)), + key_fixtures.IDENTIFIER_DOMAIN, + ) + + @staticmethod + def _in_force(moment: datetime) -> str: + """The PEM the published schedule says was in force at the moment.""" + key = key_fixtures.schedule().key_in_force(moment) + assert key is not None + return key.public_key_pem + + async def test_a_minute_between_two_confirmed_minutes_is_served_from_the_cache( + self, + ) -> None: + """A key the creator has confirmed for two minutes is served for every + minute between them without a request, because a key is in force from + the start of its period until the next key starts. A minute outside + every confirmed span is asked about.""" + end_point = self.end_point() + # The week of 31 August 2026, which the fixture identifier was signed + # in, and which is wholly in the past so the cache reads each minute + # as itself rather than as now. + first = datetime(2026, 8, 31, 0, 1, tzinfo=timezone.utc) + last = datetime(2026, 9, 6, 23, 0, tzinfo=timezone.utc) + pem = await self._pem_at(end_point, first) + self.assertEqual(pem, await self._pem_at(end_point, last), "one key covers the week") + self.assertEqual(2, len(end_point.dates()), "the two ends of the span were asked about") + for between in ( + first + timedelta(minutes=1), + first + timedelta(days=3), + last - timedelta(minutes=1), + ): + self.assertEqual(pem, await self._pem_at(end_point, between)) + self.assertEqual( + 2, len(end_point.dates()), "a minute between two confirmed minutes is not asked about" + ) + self.assertEqual( + 1, public_key_fetch._cached_key_count(), "one key is held however many minutes it covers" + ) + self.assertNotEqual( + pem, + await self._pem_at(end_point, first - timedelta(minutes=2)), + "a minute in the week before is the earlier week's key", + ) + self.assertEqual(3, len(end_point.dates()), "a minute before the span is asked about") + self.assertEqual( + 2, public_key_fetch._cached_key_count(), "the earlier week's key is held as a second key" + ) + + async def test_a_hundred_identifiers_in_one_confirmed_period_make_no_request( + self, + ) -> None: + """The case that made the cache almost useless when it was keyed by + the whole URL. A hundred identifiers with a hundred different minutes + inside one key's period cost a hundred requests then. With the ends + of the period confirmed they cost none.""" + end_point = self.end_point() + start = datetime(2026, 9, 1, tzinfo=timezone.utc) + await self._pem_at(end_point, start) + await self._pem_at(end_point, start + timedelta(minutes=100)) + for i in range(1, 101): + await self._pem_at(end_point, start + timedelta(minutes=i)) + self.assertEqual( + 2, + len(end_point.dates()), + "a hundred identifiers over a hundred minutes made no request " + "once both ends of the span were known", + ) + + async def test_a_key_is_never_served_for_a_minute_outside_its_confirmed_span( + self, + ) -> None: + """A key is only ever served for a minute inside the span the creator + has confirmed it for. Where the creator rotated between two confirmed + minutes, the minutes between them belong to neither key until the + creator is asked, and every answer agrees with the published + schedule.""" + end_point = self.end_point() + rotation = datetime(2026, 8, 31, tzinfo=timezone.utc) + week = timedelta(days=7) + minute = timedelta(minutes=1) + # The start of the week before the rotation and the end of the week + # after it, so the two keys are held with the rotation between. + await self._pem_at(end_point, rotation - week) + await self._pem_at(end_point, rotation + week - minute) + self.assertEqual(2, len(end_point.dates())) + self.assertEqual(2, public_key_fetch._cached_key_count()) + + # Every minute across the rotation, in an order that walks in from + # both sides, is answered with the key the schedule gives, whether + # from the cache or by asking. + moments = [ + rotation - minute, + rotation, + rotation - 2 * minute, + rotation + minute, + rotation - week / 2, + rotation + week / 2, + rotation - 3 * minute, + rotation + 2 * minute, + rotation - minute, + rotation, + ] + for moment in moments: + self.assertEqual( + self._in_force(moment), + await self._pem_at(end_point, moment), + "the key served for {0}".format(moment), + ) + self.assertEqual( + 2, public_key_fetch._cached_key_count(), "two keys are held, each with its own span" + ) + asked = len(end_point.dates()) + self.assertTrue( + 2 < asked < 2 + len(moments), + "some minutes were asked about and some were served: {0}".format(asked), + ) + + # The minute either side of the rotation is now confirmed, so nothing + # across the whole fortnight needs asking. + moment = rotation - week + while moment < rotation + week: + self.assertEqual( + self._in_force(moment), + await self._pem_at(end_point, moment), + "the key served for {0}".format(moment), + ) + moment += timedelta(hours=1) + self.assertEqual( + asked, len(end_point.dates()), "both spans are fully confirmed, so nothing was asked" + ) + + async def test_a_future_date_is_held_against_now(self) -> None: + """A date later than now is held against now, because a creator + answers a future date with the key in force now and a key held + against a minute the creator has not spoken for would be served for + that minute after the creator had rotated. Two future dates therefore + share one request, and so does a request with no date.""" + end_point = self.end_point() + started = io.minutes_since_base(datetime.now(timezone.utc)) + now = datetime.now(timezone.utc) + await self._pem_at(end_point, now + timedelta(days=7)) + await self._pem_at(end_point, now + timedelta(days=14)) + await public_key_fetch._public_key_pem_at_url( + end_point.base + "/owid/api/v3/public-key?format=pkcs", + key_fixtures.IDENTIFIER_DOMAIN, + ) + if io.minutes_since_base(datetime.now(timezone.utc)) != started: + self.skipTest( + "the minute changed during the test, so the calls were not " + "all about the same now" + ) + self.assertEqual( + 1, + len(end_point.dates()), + "two future dates and no date are all now, and now was asked about once", + ) + async def test_concurrent_awaits_for_one_key_share_one_request( self, ) -> None: From f108fc5c5f994fd2206b4ccbc5098cdc1a8e5c20 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 7 Sep 2026 10:12:14 +0100 Subject: [PATCH 3/8] Ask about minutes within the clock drift allowance rather than holding them The span cache read a date later than now as now and held the answer against now. A creator whose clock runs ahead of this one's signs identifiers dated in this process's future, and just after a rotation the cache would have served such an identifier the old key from a span confirmed up to now. It would have read as not matching until this clock caught up, for as long as the two clocks differ. The cache keyed by url did not have this fault, because it asked the creator for the exact minute. A minute within fifteen minutes of now, or later, is now neither served from the cache nor held in it. The creator may have read such a minute as its present rather than as the minute named, so its answer says nothing certain about the minute. A request with no date is treated the same way. Live identifiers therefore cost one request per minute per creator, which is what they always cost, and every identifier older than the allowance is served from the spans. The same allowance is applied on every port that caches. The test that read a future date as now is replaced by one that shows a recent minute asked about twice, a future minute and an undated request asked about, and a minute beyond the allowance held after its first request. --- README.md | 24 ++++++++++------- owid/public_key_fetch.py | 49 +++++++++++++++++++++------------- tests/test_public_key_fetch.py | 31 ++++++++++++++------- 3 files changed, 67 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 5f055a4..6dd6af8 100644 --- a/README.md +++ b/README.md @@ -173,19 +173,23 @@ creator that ignores the parameter returns its current key, so every identifier it signed under an earlier key reads as not matching, which is why a creator that rotates its key has to honour the date. -Keys already fetched are held by creator, each against the span of minutes -the creator has confirmed it for. A key is in force from the start of its -period until the next key starts, so a key the creator answers with at two -minutes was in force at every minute between them, and an identifier dated -inside a confirmed span is verified without a request whichever minute it -carries. One dated outside every span is asked about, which widens the span -when the same key comes back. At most 1024 keys are held across every -creator before the store is emptied and filled again, and `clear_cache()` +Keys already fetched are held by creator, each against the span of minutes the +creator has confirmed it for. A key is in force from the start of its period +until the next key starts, so a key the creator answers with at two minutes was +in force at every minute between them, and an identifier dated inside a +confirmed span is verified without a request whichever minute it carries. One +dated outside every span is asked about, which widens the span when the same +key comes back. One dated within fifteen minutes of now, or later, is asked +about every time and never held, because a creator whose clock differs from +this one's may have read that minute as its present rather than as the minute +named. Live identifiers therefore cost one request per minute per creator, as +they always did, and older ones cost none. At most 1024 keys are held across +every creator before the store is emptied and filled again, and `clear_cache()` empties it on demand, which is how a long running process drops a key it has learned it should no longer trust. Two callers who await the same key at the same moment share one request rather than making two. Each request waits at -most ten seconds. Every function that reaches the network is a coroutine, so -a caller awaits it, and there is no synchronous form. +most ten seconds. Every function that reaches the network is a coroutine, so a +caller awaits it, and there is no synchronous form. ```python import asyncio diff --git a/owid/public_key_fetch.py b/owid/public_key_fetch.py index da43db0..b2fb795 100644 --- a/owid/public_key_fetch.py +++ b/owid/public_key_fetch.py @@ -72,6 +72,20 @@ #: many domains and many weeks, and an unbounded store would grow for as long #: as the process runs. MAXIMUM_CACHED_KEYS = 1024 +#: How far a creator's clock may run ahead of or behind this one's, in +#: minutes. A minute closer to now than this, or later, is asked about rather +#: than served from the cache, and is not held. +#: +#: A creator reads a date later than its own now as now, and answers with the +#: key in force now. Within this window this process cannot tell whether the +#: creator read the minute as its past or as its present, so the answer says +#: nothing certain about the minute. An identifier signed just after a +#: rotation by a creator whose clock runs ahead would otherwise be served the +#: old key from a span confirmed up to now, and would read as not matching +#: until this clock caught up. Identifiers dated within the window are asked +#: about once per minute per creator, as they always were, and every older +#: identifier is served from the spans. +CLOCK_DRIFT_ALLOWANCE_MINUTES = 15 #: The most bytes accepted from a response. A public key PEM is a few hundred #: bytes, so a body beyond this is not a key and is not held or decoded. @@ -287,7 +301,7 @@ async def _public_key_pem_at_url( end_point = _end_point_of(url) minute = _minute_of(url) with _lock: - held = _held_pem(end_point, minute) + held = None if minute is None else _held_pem(end_point, minute) if held is not None: return held fetch = _in_flight.get(url) @@ -307,7 +321,7 @@ async def _public_key_pem_at_url( async def _fetch_and_hold( url: str, end_point: str, - minute: int, + minute: Optional[int], domain: str, transport: Optional[Transport], ) -> str: @@ -322,7 +336,7 @@ async def _fetch_and_hold( return pem finally: with _lock: - if pem is not None: + if pem is not None and minute is not None: _hold(end_point, minute, pem) if _in_flight.get(url) is task: del _in_flight[url] @@ -334,29 +348,28 @@ def _end_point_of(url: str) -> str: return url.split("?", 1)[0] -def _minute_of(url: str) -> int: - """The minute the cache reads the URL as asking about. +def _minute_of(url: str) -> Optional[int]: + """The minute the cache reads the URL as asking about, or None where the + cache must not be used for the request. - The date parameter where the URL carries one, and otherwise now, because - a creator answers a request without a date with the key in force now. A - date later than now is read as now as well, because that is how a creator - reads it. A schedule is published ahead of time and a key that has not - started has signed nothing, so the creator answers a future date with the - key in force now, and that answer must be held against now rather than - against a minute the creator has not spoken for. Held against the future - minute, the key would still be served for that minute after the creator - had rotated, and a genuine identifier signed then would read as not - matching. + The date parameter where the URL carries one and it is at least + CLOCK_DRIFT_ALLOWANCE_MINUTES behind now. A request without a date asks + for the key in force now, and one dated within the allowance, or later, + may be read by the creator as its present rather than as the minute + named, so neither is served from the cache nor held in it. """ now = io.minutes_since_base(datetime.now(timezone.utc)) query = urllib.parse.urlsplit(url).query for name, value in urllib.parse.parse_qsl(query): if name == "date": try: - return min(int(value), now) + minute = int(value) except ValueError: - return now - return now + return None + if 0 <= minute <= now - CLOCK_DRIFT_ALLOWANCE_MINUTES: + return minute + return None + return None def _held_pem(end_point: str, minute: int) -> Optional[str]: diff --git a/tests/test_public_key_fetch.py b/tests/test_public_key_fetch.py index 3e4bed7..3361b9a 100644 --- a/tests/test_public_key_fetch.py +++ b/tests/test_public_key_fetch.py @@ -490,30 +490,43 @@ async def test_a_key_is_never_served_for_a_minute_outside_its_confirmed_span( asked, len(end_point.dates()), "both spans are fully confirmed, so nothing was asked" ) - async def test_a_future_date_is_held_against_now(self) -> None: - """A date later than now is held against now, because a creator - answers a future date with the key in force now and a key held - against a minute the creator has not spoken for would be served for - that minute after the creator had rotated. Two future dates therefore - share one request, and so does a request with no date.""" + async def test_a_minute_within_the_drift_allowance_is_not_held(self) -> None: + """A minute within the clock drift allowance of now, or later, is + asked about every time and never held, because a creator whose clock + differs from this one's may have read it as its present rather than + as the minute named. A minute beyond the allowance is held as usual. + Live identifiers therefore cost one request per minute per creator, + as they always did, and older ones cost none.""" end_point = self.end_point() started = io.minutes_since_base(datetime.now(timezone.utc)) now = datetime.now(timezone.utc) + recent = now - timedelta(minutes=1) + await self._pem_at(end_point, recent) + await self._pem_at(end_point, recent) await self._pem_at(end_point, now + timedelta(days=7)) - await self._pem_at(end_point, now + timedelta(days=14)) await public_key_fetch._public_key_pem_at_url( end_point.base + "/owid/api/v3/public-key?format=pkcs", key_fixtures.IDENTIFIER_DOMAIN, ) + old = now - timedelta( + minutes=public_key_fetch.CLOCK_DRIFT_ALLOWANCE_MINUTES + 1 + ) + await self._pem_at(end_point, old) + await self._pem_at(end_point, old) if io.minutes_since_base(datetime.now(timezone.utc)) != started: self.skipTest( "the minute changed during the test, so the calls were not " "all about the same now" ) self.assertEqual( - 1, + 5, len(end_point.dates()), - "two future dates and no date are all now, and now was asked about once", + "the recent minute was asked about twice, the future minute and " + "the request with no date once each, and the old minute once " + "with the second call held", + ) + self.assertEqual( + 1, public_key_fetch._cached_key_count(), "only the old minute's key is held" ) async def test_concurrent_awaits_for_one_key_share_one_request( From 17c8f2141f019b78cb0507688d1ac70001116e07 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 7 Sep 2026 13:40:18 +0100 Subject: [PATCH 4/8] Answer the public key request with the span the key covers, and hold it The public key end point now answers with a JSON object carrying the key as publicKeySPKI together with validFrom and validTo, the UTC moments the key came into force and the next key starts. validFrom is null for a creator with one key and no schedule, and validTo is null for the last key in a schedule. The PEM alone as text is not a valid answer, and a client that receives it reports the key as one it cannot read. The answer is checked before it is sent, by the same code a client checks it with. The key must be a public key this library can read, a key valid to a moment must be valid from an earlier one, and the key must have been in force at the moment asked about. A creator whose store or schedule fails that check answers with a server error rather than a bad answer, so a fault on the server side shows up in the server's own tests and never reaches a client. The client holds the key for the whole span the creator stated, so an identifier dated anywhere in it is verified without a request whatever the clock drift, and live identifiers cost one request per key rather than one per minute. A creator that states no span has its key held against the minutes it confirms, with the fifteen minute clock drift allowance kept out of that cache as before. A signature that does not verify under the key selected for the identifier's minute, where that minute is within the drift allowance of an edge of the key's span, is checked against the neighbouring key before it is reported as not matching, because a creator's signing machines may not agree with its schedule to the minute. Where the key tried was never in force at the identifier's minute the neighbours are not tried. Callers verifying the same identifier at the same moment make one request between them, and a test with many threads proves it. The stand in creator in the tests answers with what this library's own server side code builds, so every client test runs against the response a creator built on this library sends, and the loop between the two halves is closed. --- README.md | 37 ++- owid/endpoints.py | 157 +++++++++-- owid/public_key_fetch.py | 383 +++++++++++++++++++-------- owid/public_key_schedule.py | 18 +- tests/key_end_point.py | 43 ++- tests/test_endpoints.py | 5 +- tests/test_public_key_fetch.py | 196 +++++++++++++- tests/test_public_key_response_at.py | 67 ++++- 8 files changed, 727 insertions(+), 179 deletions(-) diff --git a/README.md b/README.md index 6dd6af8..cd92e7a 100644 --- a/README.md +++ b/README.md @@ -179,17 +179,25 @@ until the next key starts, so a key the creator answers with at two minutes was in force at every minute between them, and an identifier dated inside a confirmed span is verified without a request whichever minute it carries. One dated outside every span is asked about, which widens the span when the same -key comes back. One dated within fifteen minutes of now, or later, is asked -about every time and never held, because a creator whose clock differs from -this one's may have read that minute as its present rather than as the minute -named. Live identifiers therefore cost one request per minute per creator, as -they always did, and older ones cost none. At most 1024 keys are held across -every creator before the store is emptied and filled again, and `clear_cache()` -empties it on demand, which is how a long running process drops a key it has -learned it should no longer trust. Two callers who await the same key at the -same moment share one request rather than making two. Each request waits at -most ten seconds. Every function that reaches the network is a coroutine, so a -caller awaits it, and there is no synchronous form. +key comes back. The creator answers with the key and the moments it is valid +from and to, so the whole span is held from one answer and an identifier dated +anywhere in it is verified without a request whatever the clock drift. An +answer in any other form, the PEM alone among them, is reported as a key that +cannot be read. A signature that does not verify under the key selected, where +the identifier is dated within fifteen minutes of the edge of that key's span, +is checked against the neighbouring key before it is reported as not matching, +because a creator's signing machines may not agree with its schedule to the +minute. One dated within fifteen minutes of now, or later, is asked about every +time and never held, because a creator whose clock differs from this one's may +have read that minute as its present rather than as the minute named. Live +identifiers therefore cost one request per minute per creator, as they always +did, and older ones cost none. At most 1024 keys are held across every creator +before the store is emptied and filled again, and `clear_cache()` empties it on +demand, which is how a long running process drops a key it has learned it +should no longer trust. Two callers who await the same key at the same moment +share one request rather than making two. Each request waits at most ten +seconds. Every function that reaches the network is a coroutine, so a caller +awaits it, and there is no synchronous form. ```python import asyncio @@ -452,7 +460,12 @@ opaque crypto error. paths. - `creator_response(creator, name, contract_url)` returns the creator JSON with the `domain`, `name`, `publicKeySPKI`, and `contractURL` fields. -- `public_key_response(creator, format)` returns the public key PEM. The +- `public_key_response(creator, format)` returns the JSON body of the public + key end point for a creator with one key, the key as `publicKeySPKI` with + `validFrom` and `validTo` null. `public_key_response_at` states both moments + from the schedule, and `public_key_answer` builds and checks any answer, so + a key that cannot be read or a schedule that contradicts itself is refused + before it is sent. The PEM alone as text is no longer a valid answer. The format must be `spki` or `pkcs`. - `public_key_response_at(schedule, format, date, now=None)` returns the status code and body for a creator that rotates its key, choosing from a diff --git a/owid/endpoints.py b/owid/endpoints.py index 8141137..ac36c31 100644 --- a/owid/endpoints.py +++ b/owid/endpoints.py @@ -23,19 +23,25 @@ /owid/api/v{version}/public-key returning the public key as PEM text. The format query parameter must be spki or pkcs. -A creator that rotates its signing key answers the optional date parameter of -the public key end point with public_key_response_at, which chooses from the -published schedule the way the specification requires. +The public key end point answers with a JSON object carrying the key as +publicKeySPKI together with validFrom and validTo, the UTC moments the key came +into force and the next key starts, so a client holds the key for the whole +span from one answer. A creator that rotates its signing key answers the +optional date parameter with public_key_response_at, which chooses from the +published schedule the way the specification requires. Every answer is checked +with validate_public_key_answer before it is returned, so a creator never sends +an answer a client would refuse. """ from __future__ import annotations import json from datetime import datetime, timedelta, timezone -from typing import Optional, Tuple, Union +from typing import Any, Mapping, Optional, Tuple, Union from . import io from .creator import Creator +from .crypto import Crypto from .error import OwidError from .public_key_schedule import PublicKeySchedule from .version import Version @@ -69,29 +75,138 @@ def creator_response(creator: Creator, name: str, contract_url: str = "") -> str def public_key_response(creator: Creator, format: str) -> str: - """Returns the text body for the public key end point. + """Returns the JSON body for the public key end point of a creator with + one key and no schedule. The key is stated as publicKeySPKI and both + validFrom and validTo are null, because the creator knows nothing about + when the key started or will stop. The specification allows the key to be requested in SPKI or PKCS form. This implementation returns the SPKI PEM for both values because the importers in every implementation accept it. - Raises OwidError if the format is not spki or pkcs. + Raises OwidError if the format is not spki or pkcs, or the key cannot be + read. """ - if format in ("spki", "pkcs"): - return creator.crypto.subject_public_key_info() - raise OwidError( - "format parameter 'spki' or 'pkcs' must be provided, " - "received '{0}'".format(format) - ) + if format not in ("spki", "pkcs"): + raise OwidError( + "format parameter 'spki' or 'pkcs' must be provided, " + "received '{0}'".format(format) + ) + return public_key_answer(creator.crypto.subject_public_key_info(), None, None, None) + + +def public_key_answer( + public_key_pem: str, + valid_from: Optional[datetime], + valid_to: Optional[datetime], + asked: Optional[datetime], +) -> str: + """Returns the JSON body of the public key end point for the key and the + span it covers, checked with validate_public_key_answer first so that a + creator never sends an answer it would itself refuse. The moment asked + about, where known, is checked against the span as well. + + Raises OwidError if the answer would not be valid. + """ + answer = { + "publicKeySPKI": public_key_pem, + "validFrom": _moment_text(valid_from), + "validTo": _moment_text(valid_to), + } + validate_public_key_answer(answer, asked) + return json.dumps(answer) + + +def validate_public_key_answer( + answer: Mapping[str, Any], asked: Optional[datetime] +) -> Tuple[str, Optional[datetime], Optional[datetime]]: + """Checks a public key answer the way both the creator that sends it and + the client that reads it must, returning the key and the moments it is + valid from and to. + + The key must be a public key this package can read, a key valid to a + moment must be valid from an earlier one, and where the moment asked about + is known the key must have come into force by then and, if it has an end, + not have ended. A creator that fails this check has a fault in its + schedule or its store, and answering with a server error shows it up + rather than passing it on. + Raises OwidError where the answer is not valid. + """ + if not isinstance(answer, Mapping): + raise OwidError("the public key answer is not a JSON object") + pem = answer.get("publicKeySPKI") + if not isinstance(pem, str) or not pem.strip(): + raise OwidError("the public key answer holds no key") + try: + Crypto.new_verify_only(pem) + except OwidError as failed: + raise OwidError( + "the public key answer holds a key that cannot be read" + ) from failed + valid_from = _moment_of(answer.get("validFrom"), "validFrom") + valid_to = _moment_of(answer.get("validTo"), "validTo") + if valid_to is not None: + if valid_from is None: + raise OwidError( + "the public key answer states when the key ends but not when it started" + ) + if valid_to <= valid_from: + raise OwidError( + "the public key answer states a key that ends before it starts" + ) + if asked is not None: + moment = asked if asked.tzinfo is not None else asked.replace(tzinfo=timezone.utc) + if valid_from is not None and valid_from > moment: + raise OwidError( + "the public key answer states a key that had not started at the " + "moment asked about" + ) + if valid_to is not None and valid_to <= moment: + raise OwidError( + "the public key answer states a key that had ended at the moment " + "asked about" + ) + return pem, valid_from, valid_to + + +def _moment_text(moment: Optional[datetime]) -> Optional[str]: + """The moment as an RFC 3339 string in UTC, or None.""" + if moment is None: + return None + if moment.tzinfo is None: + moment = moment.replace(tzinfo=timezone.utc) + return moment.astimezone(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + +def _moment_of(value: Any, field: str) -> Optional[datetime]: + """The field's value as an aware UTC datetime, or None where it is null. + Raises OwidError where it is anything else.""" + if value is None: + return None + if not isinstance(value, str): + raise OwidError("the public key answer's {0} is not a moment".format(field)) + text = value.strip() + if text.endswith("Z") or text.endswith("z"): + text = text[:-1] + "+00:00" + try: + moment = datetime.fromisoformat(text) + except ValueError as failed: + raise OwidError( + "the public key answer's {0} is not a moment".format(field) + ) from failed + if moment.tzinfo is None: + raise OwidError( + "the public key answer's {0} does not say it is UTC".format(field) + ) + return moment.astimezone(timezone.utc) def public_key_response_at( schedule: PublicKeySchedule, format: str, date: Union[str, int, None], now: Optional[datetime] = None, ) -> Tuple[int, str]: - """Returns the status code and text body for the public key end point of + """Returns the status code and JSON body for the public key end point of a creator that rotates its key, chosen from the schedule the way the specification requires. @@ -100,12 +215,14 @@ def public_key_response_at( key whose start is at or before it. A request without a date, or with a date later than the moment of the request, is served the key in force at that moment, so a caller cannot ask for a key whose period has not begun. - The answer is 200 with the PEM, 404 with an empty body where no key is in - force at the date, and 400 with an empty body where the date is not a - count of minutes. The moment of the request is now, and a test may supply - it. + The answer is 200 with the JSON body from public_key_answer, stating the + key and the moments it is valid from and to, 404 with an empty body where + no key is in force at the date, and 400 with an empty body where the date + is not a count of minutes. The moment of the request is now, and a test + may supply it. - Raises OwidError if the format is not spki or pkcs. + Raises OwidError if the format is not spki or pkcs, or the answer would + fail validate_public_key_answer, which is a fault in the schedule. """ if format not in ("spki", "pkcs"): raise OwidError( @@ -129,7 +246,9 @@ def public_key_response_at( key = schedule.key_in_force(asked) if key is None: return 404, "" - return 200, key.public_key_pem + return 200, public_key_answer( + key.public_key_pem, key.starts_at, schedule.next_start_after(key), asked + ) def _minutes(date: Union[str, int]) -> Optional[int]: diff --git a/owid/public_key_fetch.py b/owid/public_key_fetch.py index b2fb795..83ad37f 100644 --- a/owid/public_key_fetch.py +++ b/owid/public_key_fetch.py @@ -38,11 +38,14 @@ one place in the package that reaches the network, and it is imported only when a caller asks for it. -Keys already fetched are held by creator, each against the span of minutes -the creator has confirmed it for, so an identifier dated inside a confirmed -span is verified without a request whichever minute it carries. Two callers -who await the same key at the same moment share one request rather than -making two. +The creator answers with the key and the moments it is valid from and to, and +the key is held by creator for that whole span, so an identifier dated inside +it is verified without a request whichever minute it carries. A creator that +states no span has its key held against the minutes it confirms. A signature +that fails under the key selected, where the identifier is dated within the +clock drift allowance of the edge of that key's span, is checked against the +neighbouring key before it is reported as not matching. Two callers who await +the same key at the same moment share one request rather than making two. The Java port answers the same question with PublicKeyFetch, the Rust port with Owid::verify_status and the Go port with SignatureStatusFromDomain. @@ -51,7 +54,9 @@ from __future__ import annotations import asyncio +import concurrent.futures import http.client +import json import threading import urllib.error import urllib.parse @@ -60,6 +65,7 @@ from typing import Awaitable, Callable, Dict, List, Optional, Sequence, Tuple from . import endpoints, io +from .endpoints import validate_public_key_answer from .error import OwidError, PublicKeyFetchError from .owid import Owid from .status import SignatureStatus @@ -73,18 +79,16 @@ #: as the process runs. MAXIMUM_CACHED_KEYS = 1024 #: How far a creator's clock may run ahead of or behind this one's, in -#: minutes. A minute closer to now than this, or later, is asked about rather -#: than served from the cache, and is not held. +#: minutes. #: -#: A creator reads a date later than its own now as now, and answers with the -#: key in force now. Within this window this process cannot tell whether the -#: creator read the minute as its past or as its present, so the answer says -#: nothing certain about the minute. An identifier signed just after a -#: rotation by a creator whose clock runs ahead would otherwise be served the -#: old key from a span confirmed up to now, and would read as not matching -#: until this clock caught up. Identifiers dated within the window are asked -#: about once per minute per creator, as they always were, and every older -#: identifier is served from the spans. +#: It is used in two places. A creator that does not state the span of the key +#: it answers with reads a date later than its own now as now, so within this +#: window of now this process cannot tell whether the creator read the minute +#: as its past or as its present, and nothing learned from such an answer is +#: held or served. And a creator's signing machines may not agree with the +#: creator's own schedule to the minute, so an identifier dated within this +#: window of a key's edge that does not verify under that key is checked +#: against the neighbouring key before it is reported as not matching. CLOCK_DRIFT_ALLOWANCE_MINUTES = 15 #: The most bytes accepted from a response. A public key PEM is a few hundred @@ -106,31 +110,54 @@ _ACCEPTED_SCHEMES = ("http", "https") class _HeldKey: - """One key a creator has answered with, and the span of minutes the - creator has confirmed it was in force for. + """One key a creator has answered with, and the span of minutes the key + is known to cover. A creator's key is in force from the start of its period until the next key starts, so a key the creator confirms at two minutes was in force at - every minute between them. The span grows as the creator confirms the - same key for more minutes, and an identifier dated inside it is verified - without a request. + every minute between them. Where the creator stated the span in its + answer the span is explicit and complete, and an identifier dated + anywhere inside it is verified without a request. Otherwise the span + grows as the creator confirms the same key for more minutes. """ - __slots__ = ("pem", "first", "last") + __slots__ = ("pem", "first", "last", "explicit") - def __init__(self, pem: str, minute: int) -> None: + def __init__(self, pem: str, first: int, last: int, explicit: bool) -> None: #: The key in PEM form, as the creator served it. self.pem = pem - #: The earliest minute the creator has confirmed the key for. - self.first = minute - #: The latest minute the creator has confirmed the key for. - self.last = minute + #: The earliest minute the key is known to cover. + self.first = first + #: The latest minute the key is known to cover. + self.last = last + #: Whether the creator stated the whole span itself. + self.explicit = explicit def covers(self, minute: int) -> bool: - """Whether the minute lies within the confirmed span.""" + """Whether the minute lies within the known span.""" return self.first <= minute <= self.last +class _KeyAnswer: + """What the cache or a fetch answers with. The key, and where it is known, + the span of minutes the key covers, so that a caller can tell whether the + identifier it is checking sits near the edge of the span.""" + + __slots__ = ("pem", "first", "last", "known") + + def __init__( + self, pem: str, first: int = 0, last: int = 0, known: bool = False + ) -> None: + self.pem = pem + self.first = first + self.last = last + self.known = known + + def covers(self, minute: int) -> bool: + """Whether the minute lies within the known span.""" + return self.known and self.first <= minute <= self.last + + #: Keys already fetched, by the creator's key end point, which is the key URL #: without its date. Each end point holds the keys the creator has answered #: with, each with the span of minutes the creator has confirmed it for. @@ -138,10 +165,7 @@ def covers(self, minute: int) -> bool: #: The specification asks implementations to cache so that verifying many #: identifiers does not mean repeating requests to another processor. The key #: URL carries the date of the identifier being verified, in minutes, and a -#: creator's key changes on the order of a week. Keyed by the whole URL, as -#: this cache once was, two identifiers signed a minute apart never shared an -#: entry, so a hundred identifiers over a hundred minutes made a hundred -#: requests for one key. Keyed by end point and span, an identifier dated +#: creator's key changes on the order of a week. Keyed by end point and span rather than by the whole URL, an identifier dated #: between two minutes the creator has already answered for is verified #: without a request. _cache: Dict[str, List[_HeldKey]] = {} @@ -149,11 +173,10 @@ def covers(self, minute: int) -> bool: _held_keys = 0 #: Fetches still running, held against the URL, so that a caller who asks for #: a key while the request for it is in flight awaits that request rather -#: than making another. An entry is removed when its fetch finishes, whatever -#: the outcome. A task belongs to the event loop that created it, so a caller -#: on a different loop, which only happens where two loops run in two -#: threads, cannot await it and starts a fetch of its own. -_in_flight: Dict[str, "asyncio.Task[str]"] = {} +#: than making another. Each is a future any thread can wait on, so callers +#: on different event loops in different threads share one request as well. +#: An entry is removed when its fetch finishes, whatever the outcome. +_in_flight: Dict[str, "concurrent.futures.Future[_KeyAnswer]"] = {} #: Guards all three stores. An event loop runs one coroutine at a time, but #: the stores are shared by every loop in the process, and the lock is only #: ever held across a few dictionary operations and never across an await. @@ -282,64 +305,173 @@ async def _signature_status_at_url( that the tests drive the real fetch against a key end point the tests can stand up locally rather than against a near copy of the fetch.""" try: - pem = await _public_key_pem_at_url(url, owid.domain, transport) + answer = await _key_at_url(url, owid.domain, transport) except PublicKeyFetchError as failed: return failed.status except OwidError: return SignatureStatus.KEY_UNAVAILABLE - return owid.signature_status(pem, others) + status = owid.signature_status(answer.pem, others) + if status is SignatureStatus.SIGNATURE_INVALID and await _neighbour_verifies( + owid, url, answer, others, transport + ): + return SignatureStatus.SIGNATURE_VALID + return status + + +async def _neighbour_verifies( + owid: Owid, + url: str, + tried: _KeyAnswer, + others: Optional[Sequence[Owid]], + transport: Optional[Transport], +) -> bool: + """Whether a key neighbouring the one the OWID's own minute selected + verifies the signature instead. + + A creator's signing machines may not agree with its own schedule to the + minute, so an identifier dated just after a key started may have been + signed with the key before it, and one dated just before may have been + signed with the key after. Where the signature does not verify under the + key selected and the OWID's minute is within the clock drift allowance of + the edge of the span that key is known to cover, the key for the minute + just beyond that edge is asked for and tried. A key already known to cover + the neighbouring minute is not asked for again, and a neighbour that turns + out to be the same key is not tried again. This costs at most two more + requests, and only for a signature that has already failed. + """ + minute = io.minutes_since_base(owid.date) + if minute < 0: + return False + if tried.known and not tried.covers(minute): + # The key tried was never in force at the identifier's minute, so + # the identifier is not near an edge of that key's span. This is an + # undated request answered with the current key, or a creator whose + # answer did not cover the minute asked about, and the neighbours of + # the minute have nothing to do with the key tried. + return False + for at in (minute - CLOCK_DRIFT_ALLOWANCE_MINUTES, minute + CLOCK_DRIFT_ALLOWANCE_MINUTES): + if at < 0 or at > 0xFFFFFFFF or tried.covers(at): + continue + try: + neighbour = await _key_at_url( + "{0}?date={1}&format=pkcs".format(_end_point_of(url), at), + owid.domain, + transport, + ) + except OwidError: + # A neighbour that cannot be obtained leaves the failure under + # the selected key standing. + continue + if neighbour.pem == tried.pem: + continue + if owid.signature_status(neighbour.pem, others) is SignatureStatus.SIGNATURE_VALID: + return True + return False async def _public_key_pem_at_url( url: str, domain: str, transport: Optional[Transport] = None ) -> str: - """Fetches the PEM at the URL. Answered from the cache where the creator - has already confirmed a key for the minute the URL names, from the - request already in flight where another caller on this event loop is - fetching the same URL now, and otherwise by asking the creator.""" + """The PEM of the key the URL asks for. See _key_at_url.""" + return (await _key_at_url(url, domain, transport)).pem + + +async def _key_at_url( + url: str, domain: str, transport: Optional[Transport] = None +) -> _KeyAnswer: + """Fetches the key the URL asks for, with the span it is known to cover. + Answered from the cache where a held key is known to cover the minute the + URL names, from the request already in flight where another caller on this + event loop is fetching the same URL now, and otherwise by asking the + creator.""" loop = asyncio.get_running_loop() end_point = _end_point_of(url) - minute = _minute_of(url) with _lock: - held = None if minute is None else _held_pem(end_point, minute) + held = _held_for(end_point, url) if held is not None: return held - fetch = _in_flight.get(url) - if fetch is None or fetch.get_loop() is not loop: - fetch = loop.create_task( - _fetch_and_hold(url, end_point, minute, domain, transport) - ) - fetch.add_done_callback(_mark_observed) - _in_flight[url] = fetch + shared = _in_flight.get(url) + lead = shared is None + if shared is None: + shared = concurrent.futures.Future() + _in_flight[url] = shared + if lead: + # This caller is the one that adds the entry, so this caller is the + # one that performs the request. Every other caller arriving before + # it ends, on this loop or any other, waits on the future just added. + task = loop.create_task(_fetch_and_hold(url, end_point, domain, transport, shared)) + task.add_done_callback(_mark_observed) # Shielded, because cancelling one caller must not cancel the request # that other callers are waiting on, and a request on a worker thread # cannot be stopped part way in any case. The answer still lands in the # cache for the next caller. - return await asyncio.shield(fetch) + return await asyncio.shield(asyncio.wrap_future(shared)) async def _fetch_and_hold( url: str, end_point: str, - minute: Optional[int], domain: str, transport: Optional[Transport], -) -> str: - """The one request for a URL, run as a task that every caller waiting for - that URL awaits. Holds the answer against the minute asked about, and - forgets the task whatever the outcome so that a failed fetch is tried - again by the next caller.""" - task = asyncio.current_task() - pem: Optional[str] = None + shared: "concurrent.futures.Future[_KeyAnswer]", +) -> _KeyAnswer: + """The one request for a URL, run as a task whose outcome every caller + waiting for that URL receives through the shared future. Holds the answer + against the span it states, or the minute asked about where it states + none, and forgets the request whatever the outcome so that a failed fetch + is tried again by the next caller.""" try: - pem = await _read(url, domain, transport) - return pem - finally: + body = await _read(url, domain, transport) + pem, start, end = _parse_answer(body, domain) + with _lock: + answer = _hold(end_point, url, pem, start, end) + if _in_flight.get(url) is shared: + del _in_flight[url] + shared.set_result(answer) + return answer + except BaseException as failed: with _lock: - if pem is not None and minute is not None: - _hold(end_point, minute, pem) - if _in_flight.get(url) is task: + if _in_flight.get(url) is shared: del _in_flight[url] + shared.set_exception(failed) + raise + + +def _parse_answer(body: str, domain: str) -> Tuple[str, Optional[int], Optional[int]]: + """Reads a public key answer, returning the PEM and the span in minutes + since the base date. The end is the minute the next key starts. Either is + None where the answer does not state it. An answer that is not the JSON + form the specification requires, the PEM alone among the other forms, or + that fails the checks a creator applies before sending it, is reported as + a key that cannot be read.""" + try: + answer = json.loads(body) + except ValueError as failed: + raise PublicKeyFetchError( + "domain {0} did not answer with the JSON form the specification " + "requires for the public key".format(_quoted(domain)), + SignatureStatus.INVALID_KEY, + domain, + ) from failed + try: + pem, valid_from, valid_to = validate_public_key_answer(answer, None) + except OwidError as failed: + raise PublicKeyFetchError( + "domain {0} answered with a public key answer that is not valid: " + "{1}".format(_quoted(domain), failed), + SignatureStatus.INVALID_KEY, + domain, + ) from failed + return pem, _minutes_or_none(valid_from), _minutes_or_none(valid_to) + + +def _minutes_or_none(moment: Optional[datetime]) -> Optional[int]: + """The moment as minutes since the base date, or None where there is no + moment or it is before the count begins.""" + if moment is None: + return None + minutes = io.minutes_since_base(moment) + return minutes if minutes >= 0 else None def _end_point_of(url: str) -> str: @@ -348,16 +480,11 @@ def _end_point_of(url: str) -> str: return url.split("?", 1)[0] -def _minute_of(url: str) -> Optional[int]: - """The minute the cache reads the URL as asking about, or None where the - cache must not be used for the request. - - The date parameter where the URL carries one and it is at least - CLOCK_DRIFT_ALLOWANCE_MINUTES behind now. A request without a date asks - for the key in force now, and one dated within the allowance, or later, - may be read by the creator as its present rather than as the minute - named, so neither is served from the cache nor held in it. - """ +def _minute_and_recency(url: str) -> Tuple[Optional[int], bool]: + """The minute the URL asks about, or None where it names none, and whether + it lies within the clock drift allowance of now or later, which is a + minute a creator that does not state its spans may have read as its + present rather than as the minute named.""" now = io.minutes_since_base(datetime.now(timezone.utc)) query = urllib.parse.urlsplit(url).query for name, value in urllib.parse.parse_qsl(query): @@ -365,38 +492,71 @@ def _minute_of(url: str) -> Optional[int]: try: minute = int(value) except ValueError: - return None - if 0 <= minute <= now - CLOCK_DRIFT_ALLOWANCE_MINUTES: - return minute - return None - return None + return None, False + return minute, minute > now - CLOCK_DRIFT_ALLOWANCE_MINUTES + return None, False -def _held_pem(end_point: str, minute: int) -> Optional[str]: - """The key held for the end point whose confirmed span covers the minute, - or None where no held key does. Called under the lock.""" +def _held_for(end_point: str, url: str) -> Optional[_KeyAnswer]: + """The key held for the end point that is known to cover the minute the + URL asks about, or None where none is. Called under the lock. + + A minute within the drift allowance of now is only served where the + creator itself stated the span, because a span confirmed minute by minute + says nothing certain about such a minute.""" + minute, recent = _minute_and_recency(url) + if minute is None: + return None for key in _cache.get(end_point, ()): - if key.covers(minute): - return key.pem + if key.covers(minute) and (key.explicit or not recent): + return _KeyAnswer(key.pem, key.first, key.last, True) return None -def _hold(end_point: str, minute: int, pem: str) -> None: - """Records that the creator answered the minute with the key. Called - under the lock. - - A key already held for the end point has its span widened to take in the - minute. A key not held before is added, emptying the cache first when it - is full, because the domains and dates asked about come from the - identifiers presented to this process and the cache must not grow on - their input. +def _hold( + end_point: str, url: str, pem: str, start: Optional[int], end: Optional[int] +) -> _KeyAnswer: + """Records the creator's answer to the URL, being the key and, where the + creator stated it, the span the key covers as the minute it came into + force and the minute the next key starts. Returns the key with the span it + is now known to cover. Called under the lock. + + With both the start and the end the whole span is held as the creator's + own statement. With the start alone the key is held from the start up to + the drift allowance behind now, because no later key can have started + before then. With neither the minute asked about is held on its own, as + long as it is not within the drift allowance of now. A key already held + for the end point has its span widened to take in the new one. A key not + held before is added, emptying the cache first when it is full, because + the cache must not grow on the input of whoever presents the identifiers. """ global _held_keys + minute, recent = _minute_and_recency(url) + explicit = False + if start is not None and end is not None and end > start: + first, last, explicit = start, end - 1, True + elif start is not None: + now = io.minutes_since_base(datetime.now(timezone.utc)) + first = start + last = max(start, now - CLOCK_DRIFT_ALLOWANCE_MINUTES) + elif minute is not None and not recent: + first = last = minute + else: + return _KeyAnswer(pem) keys = _cache.get(end_point) if keys is not None: for key in keys: - if key.pem == pem and _widen(keys, key, minute): - return + if key.pem == pem: + if _widen(keys, key, first, last): + key.explicit = key.explicit or explicit + return _KeyAnswer(pem, key.first, key.last, True) + # The creator has answered with another key inside this span + # before, which it does not do unless it went back to a key + # it had left. Nothing more is held about this key. + return _KeyAnswer(pem) + for other in keys: + if other.last >= first and other.first <= last: + return _KeyAnswer(pem) if _held_keys >= MAXIMUM_CACHED_KEYS: _cache.clear() _held_keys = 0 @@ -404,34 +564,31 @@ def _hold(end_point: str, minute: int, pem: str) -> None: if keys is None: keys = [] _cache[end_point] = keys - keys.append(_HeldKey(pem, minute)) + keys.append(_HeldKey(pem, first, last, explicit)) _held_keys += 1 + return _KeyAnswer(pem, first, last, True) -def _widen(keys: List[_HeldKey], key: _HeldKey, minute: int) -> bool: - """Widens the span of a held key to take in the minute, and says whether - the minute is now within it. +def _widen(keys: List[_HeldKey], key: _HeldKey, first: int, last: int) -> bool: + """Widens the span of a held key to take in the span given, and says + whether it did. The span is not widened across a minute the creator has answered with another key for, because that would mean the creator had gone back to a key it had left, and the minutes between the two spans are then not this - key's to claim. The key is held again as a separate span instead. + key's to claim. """ - if key.covers(minute): - return True - start = min(minute, key.first) - end = max(minute, key.last) + first = min(first, key.first) + last = max(last, key.last) for other in keys: - if other is not key and other.last > start and other.first < end: + if other is not key and other.last >= first and other.first <= last: return False - if minute < key.first: - key.first = minute - else: - key.last = minute + key.first = first + key.last = last return True -def _mark_observed(task: "asyncio.Task[str]") -> None: +def _mark_observed(task: "asyncio.Task[_KeyAnswer]") -> None: """Reads the outcome of a finished fetch, so that a failure nobody was left waiting for, because every caller was cancelled, is not reported by asyncio as an exception that was never retrieved.""" diff --git a/owid/public_key_schedule.py b/owid/public_key_schedule.py index db73c19..ae7ec10 100644 --- a/owid/public_key_schedule.py +++ b/owid/public_key_schedule.py @@ -26,9 +26,7 @@ start is at or before the date asked about. Keys are generated in batches, often many weeks ahead of the weeks the keys cover, so the moment key material was generated says nothing about which key signed anything and is -not held here at all. Selecting on a generation moment picks a key that has -not started yet and reports a genuine identifier as not matching, which is -what the .NET port did before that port was fixed. +not held here at all. Selecting on a generation moment picks a key that has not started yet and reports a genuine identifier as not matching. A date the schedule does not reach, being one earlier than the first start, has no key. That answer is reported as SignatureStatus.KEY_UNAVAILABLE rather @@ -148,6 +146,15 @@ def key_in_force(self, date: Optional[datetime]) -> Optional[DatedPublicKey]: index -= 1 return None + def next_start_after(self, key: DatedPublicKey) -> Optional[datetime]: + """Returns the earliest start in the schedule after the key's own, + being the moment the key stops being in force, or None where the key + is the last in the schedule and is in force until further notice.""" + following = [ + other.starts_at for other in self._keys if other.starts_at > key.starts_at + ] + return min(following) if following else None + def last(self) -> Optional[DatedPublicKey]: """Returns the key with the latest start, or None where the schedule holds no keys. @@ -155,10 +162,7 @@ def last(self) -> Optional[DatedPublicKey]: This is not the key in force now. A creator publishes its schedule ahead of time, so the last key by start is usually one whose period has not begun and which has signed nothing yet. The key in force now - is current(). Serving the last key where the current one was meant is - the same fault as selecting by the generation moment, being a key - from a period that has not started, and it is the fault the .NET port - carried in its answer to a request that named no date. + is current(). Serving the last key where the current one was meant is the same fault as selecting by the generation moment, being a key from a period that has not started. """ if not self._keys: return None diff --git a/tests/key_end_point.py b/tests/key_end_point.py index 016631c..be3b941 100644 --- a/tests/key_end_point.py +++ b/tests/key_end_point.py @@ -31,6 +31,7 @@ from __future__ import annotations +import json import threading import urllib.parse from datetime import datetime, timedelta, timezone @@ -38,7 +39,7 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import List, Optional -from owid import Owid, io +from owid import Owid, endpoints, io from owid.public_key_fetch import public_key_url from tests import key_fixtures @@ -57,6 +58,11 @@ class Answer(Enum): #: The published schedule, chosen by the date requested. SCHEDULE = "schedule" #: Text shaped like a PEM that no key can be read out of. + #: The published schedule as JSON with the key alone and no moments, as a + #: creator with one key and no schedule answers. + SPANLESS = "spanless" + #: The key alone as text, which the specification does not allow. + PEM_ONLY = "pem-only" BROKEN_KEY = "broken-key" #: A redirect to a host that is not the creator, which a client must #: not follow. @@ -98,7 +104,10 @@ def do_GET(self) -> None: # noqa: N802 - the name is the protocol's. return data = body.encode("utf-8") self.send_response(200) - self.send_header("Content-Type", "text/plain") + self.send_header( + "Content-Type", + "text/plain" if end_point.answer is Answer.PEM_ONLY else "application/json", + ) self.send_header("Content-Length", str(len(data))) self.end_headers() self.wfile.write(data) @@ -156,13 +165,17 @@ def body(self, date: Optional[str]) -> Optional[str]: """The body to serve, or None where the end point has no key. Raises ValueError where the date is not a count of minutes.""" if self._answer is Answer.BROKEN_KEY: - # Shaped like a PEM, with a body no key can be read out of. This - # is the 30 August 2026 fault, where the end points served PEM a - # strict parser refused and good identifiers went unverified. - return ( - "-----BEGIN PUBLIC KEY-----\n" - "bm90IGEga2V5\n" - "-----END PUBLIC KEY-----\n" + # Shaped like a PEM, with a body no key can be read out of. It is sent as the JSON form without the check a creator applies, because that check is what catches it. + return json.dumps( + { + "publicKeySPKI": ( + "-----BEGIN PUBLIC KEY-----\n" + "bm90IGEga2V5\n" + "-----END PUBLIC KEY-----\n" + ), + "validFrom": None, + "validTo": None, + } ) asked = REQUEST_MOMENT if date is not None: @@ -174,4 +187,14 @@ def body(self, date: Optional[str]) -> Optional[str]: key = self._schedule.key_in_force(asked) if key is None: return None - return key.public_key_pem + if self._answer is Answer.PEM_ONLY: + return key.public_key_pem + if self._answer is Answer.SPANLESS: + return endpoints.public_key_answer(key.public_key_pem, None, None, None) + # The answer the package's own server side helper builds, so the + # client is tested against what a creator built on it sends. + status, body = endpoints.public_key_response_at( + self._schedule, "pkcs", date, REQUEST_MOMENT + ) + assert status == 200, status + return body diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 69c18a0..62674cf 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -61,7 +61,10 @@ def test_public_key_response_formats(self) -> None: creator = _new_creator() for fmt in ("spki", "pkcs"): body = endpoints.public_key_response(creator, fmt) - self.assertIn("BEGIN PUBLIC KEY", body) + answer = json.loads(body) + self.assertIn("BEGIN PUBLIC KEY", answer["publicKeySPKI"]) + self.assertIsNone(answer["validFrom"], "a single key has no schedule") + self.assertIsNone(answer["validTo"]) def test_public_key_response_invalid_format(self) -> None: creator = _new_creator() diff --git a/tests/test_public_key_fetch.py b/tests/test_public_key_fetch.py index 3361b9a..1f4a8a9 100644 --- a/tests/test_public_key_fetch.py +++ b/tests/test_public_key_fetch.py @@ -33,12 +33,19 @@ import os import pathlib import unittest +import time +import threading +import urllib.parse +import json from datetime import datetime, timedelta, timezone from typing import List, Optional, Tuple from unittest import mock from owid import ( Creator, + DatedPublicKey, + PublicKeySchedule, + endpoints, Crypto, Owid, OwidError, @@ -376,7 +383,7 @@ async def test_a_minute_between_two_confirmed_minutes_is_served_from_the_cache( minute between them without a request, because a key is in force from the start of its period until the next key starts. A minute outside every confirmed span is asked about.""" - end_point = self.end_point() + end_point = self.end_point(Answer.SPANLESS) # The week of 31 August 2026, which the fixture identifier was signed # in, and which is wholly in the past so the cache reads each minute # as itself rather than as now. @@ -414,7 +421,7 @@ async def test_a_hundred_identifiers_in_one_confirmed_period_make_no_request( the whole URL. A hundred identifiers with a hundred different minutes inside one key's period cost a hundred requests then. With the ends of the period confirmed they cost none.""" - end_point = self.end_point() + end_point = self.end_point(Answer.SPANLESS) start = datetime(2026, 9, 1, tzinfo=timezone.utc) await self._pem_at(end_point, start) await self._pem_at(end_point, start + timedelta(minutes=100)) @@ -435,7 +442,7 @@ async def test_a_key_is_never_served_for_a_minute_outside_its_confirmed_span( minutes, the minutes between them belong to neither key until the creator is asked, and every answer agrees with the published schedule.""" - end_point = self.end_point() + end_point = self.end_point(Answer.SPANLESS) rotation = datetime(2026, 8, 31, tzinfo=timezone.utc) week = timedelta(days=7) minute = timedelta(minutes=1) @@ -495,9 +502,8 @@ async def test_a_minute_within_the_drift_allowance_is_not_held(self) -> None: asked about every time and never held, because a creator whose clock differs from this one's may have read it as its present rather than as the minute named. A minute beyond the allowance is held as usual. - Live identifiers therefore cost one request per minute per creator, - as they always did, and older ones cost none.""" - end_point = self.end_point() + Live identifiers therefore cost one request per minute per creator and older ones cost none.""" + end_point = self.end_point(Answer.SPANLESS) started = io.minutes_since_base(datetime.now(timezone.utc)) now = datetime.now(timezone.utc) recent = now - timedelta(minutes=1) @@ -529,6 +535,176 @@ async def test_a_minute_within_the_drift_allowance_is_not_held(self) -> None: 1, public_key_fetch._cached_key_count(), "only the old minute's key is held" ) + async def test_a_key_answered_with_its_span_is_held_for_the_whole_span(self) -> None: + """A creator that states the moments the key is valid from and to, + which is what the package's own server side helper answers, has the + whole span held from that one answer, so every other minute of the + span is served without a request.""" + end_point = self.end_point() + pem = await self._pem_at(end_point, datetime(2026, 8, 31, 0, 1, tzinfo=timezone.utc)) + for moment in ( + datetime(2026, 9, 6, 23, 59, tzinfo=timezone.utc), + datetime(2026, 9, 3, 12, tzinfo=timezone.utc), + datetime(2026, 8, 31, tzinfo=timezone.utc), + ): + self.assertEqual(pem, await self._pem_at(end_point, moment), str(moment)) + self.assertEqual(1, len(end_point.dates()), "the whole week was held from one answer") + self.assertEqual(1, public_key_fetch._cached_key_count()) + before = await self._pem_at(end_point, datetime(2026, 8, 30, 23, 59, tzinfo=timezone.utc)) + self.assertNotEqual(pem, before, "the minute before the week is the earlier week's key") + await self._pem_at(end_point, datetime(2026, 8, 24, tzinfo=timezone.utc)) + self.assertEqual(2, len(end_point.dates()), "the earlier week was held from its one answer") + + async def test_a_recent_minute_is_served_where_the_creator_stated_the_span(self) -> None: + """The drift allowance, which keeps minutes near now out of a cache + built from confirmed minutes, does not apply to a span the creator + stated itself, so live identifiers cost one request per key rather + than one per minute.""" + end_point = self.end_point() + now = datetime.now(timezone.utc) + current = key_fixtures.schedule().key_in_force(now) + if current is None or key_fixtures.schedule().next_start_after(current) is None: + self.skipTest("the fixture schedule has no key after the one in force now") + await self._pem_at(end_point, now - timedelta(minutes=1)) + await self._pem_at(end_point, now) + await self._pem_at(end_point, now - timedelta(minutes=10)) + self.assertEqual( + 1, len(end_point.dates()), "the current key was served for every recent minute from one answer" + ) + + @staticmethod + def _signed_at(domain: str, moment: datetime, crypto: Crypto) -> Owid: + """An identifier for the domain dated at the moment and signed with + the crypto given, standing for one whose signing machine's clock did + not agree with the creator's schedule to the minute.""" + owid = Owid._create( + version=Version.VERSION3, domain=domain, date=moment, payload=b"payload" + ) + owid._signature = crypto.sign_byte_array(owid.data_for_crypto([])) + return owid + + async def test_a_signature_failing_near_the_edge_of_a_span_is_checked_against_the_neighbour( + self, + ) -> None: + """An identifier dated just after a key started, but signed with the + key before it, verifies, and one dated just before a key started but + signed with it verifies too, because the neighbouring key is tried + when the selected key fails within the drift allowance of the span's + edge. Further from the edge the failure stands. The stand in creator + answers with the package's own server side helper, so the loop + between the two halves of the package is closed.""" + first, second, third = Crypto.new(), Crypto.new(), Crypto.new() + rotation = datetime(2026, 8, 31, tzinfo=timezone.utc) + week = timedelta(days=7) + schedule = PublicKeySchedule( + [ + DatedPublicKey(rotation - week, first.public_key_pem()), + DatedPublicKey(rotation, second.public_key_pem()), + DatedPublicKey(rotation + week, third.public_key_pem()), + ] + ) + requests: List[str] = [] + + async def creator(url: str, timeout: float) -> Tuple[int, bytes]: + requests.append(url) + date = dict(urllib.parse.parse_qsl(urllib.parse.urlsplit(url).query)).get("date") + status, body = endpoints.public_key_response_at(schedule, "pkcs", date) + return status, body.encode("utf-8") + + url = "https://creator.test/owid/api/v3/public-key?date={0}&format=pkcs" + + async def status_of(owid: Owid) -> SignatureStatus: + return await public_key_fetch._signature_status_at_url( + owid, url.format(io.minutes_since_base(owid.date)), None, creator + ) + + late = self._signed_at("creator.test", rotation + timedelta(minutes=5), first) + self.assertIs(SignatureStatus.SIGNATURE_VALID, await status_of(late), + "signed with the earlier key just after the rotation") + self.assertEqual(2, len(requests), "the selected key and then the earlier key were asked for") + early = self._signed_at("creator.test", rotation - timedelta(minutes=5), second) + self.assertIs(SignatureStatus.SIGNATURE_VALID, await status_of(early), + "signed with the later key just before the rotation") + self.assertEqual(2, len(requests), "both keys are held with their spans") + far = self._signed_at("creator.test", rotation + timedelta(minutes=20), first) + self.assertIs(SignatureStatus.SIGNATURE_INVALID, await status_of(far), + "well inside the later key's span") + self.assertEqual(2, len(requests), "the neighbouring minutes lie inside the spans held") + genuine = self._signed_at("creator.test", rotation + timedelta(days=3), second) + self.assertIs(SignatureStatus.SIGNATURE_VALID, await status_of(genuine)) + forged = self._signed_at("creator.test", rotation + timedelta(days=3), third) + self.assertIs(SignatureStatus.SIGNATURE_INVALID, await status_of(forged), + "signed with a key not in force at its date") + + async def test_an_answer_that_is_not_the_json_form_is_a_key_that_cannot_be_read(self) -> None: + """The PEM alone as text is reported as a key this package cannot read rather than used, and so is a span that ends before it starts.""" + end_point = self.end_point(Answer.PEM_ONLY) + owid = key_fixtures.identifier() + self.assertIs( + SignatureStatus.INVALID_KEY, + await public_key_fetch._signature_status_at_url(owid, end_point.url_for(owid)), + ) + + async def contradictory(url: str, timeout: float) -> Tuple[int, bytes]: + return 200, json.dumps( + { + "publicKeySPKI": key_fixtures.schedule().keys[0].public_key_pem, + "validFrom": "2026-08-31T00:00:00Z", + "validTo": "2026-08-24T00:00:00Z", + } + ).encode("utf-8") + + self.assertIs( + SignatureStatus.INVALID_KEY, + await public_key_fetch._signature_status_at_url( + owid, end_point.url_for(owid), None, contradictory + ), + ) + + def test_many_threads_verifying_one_owid_together_make_one_request(self) -> None: + """Threads verifying the same OWID at the same moment, each on its own + event loop, make one request for its key between them, and every one + of them gets the answer. The stand in transport holds its answer until + every thread has asked, so all of them are in flight together against + one request.""" + callers = 8 + owid = key_fixtures.identifier() + pem = key_fixtures.schedule().key_for(owid).public_key_pem + answer = endpoints.public_key_answer(pem, None, None, None).encode("utf-8") + requests: List[str] = [] + release = threading.Event() + started = threading.Barrier(callers + 1) + + async def transport(url: str, timeout: float) -> Tuple[int, bytes]: + requests.append(url) + await asyncio.get_running_loop().run_in_executor(None, release.wait) + return 200, answer + + url = public_key_fetch.public_key_url(owid, "https") + statuses: List[SignatureStatus] = [] + + def verify() -> None: + started.wait() + statuses.append( + asyncio.run( + public_key_fetch._signature_status_at_url(owid, url, None, transport) + ) + ) + + threads = [threading.Thread(target=verify) for _ in range(callers)] + for thread in threads: + thread.start() + # Every thread goes at the same moment, and the transport only + # answers once they are all waiting on it. + started.wait() + time.sleep(0.3) + release.set() + for thread in threads: + thread.join(30) + self.assertEqual(callers, len(statuses), "every thread finished") + self.assertEqual({SignatureStatus.SIGNATURE_VALID}, set(statuses)) + self.assertEqual(1, len(requests), "one request for {0} threads".format(callers)) + async def test_concurrent_awaits_for_one_key_share_one_request( self, ) -> None: @@ -546,7 +722,7 @@ async def held_open(url: str, timeout: float) -> Tuple[int, bytes]: calls.append(url) arrived.set() await release.wait() - return 200, pem.encode("utf-8") + return 200, endpoints.public_key_answer(pem, None, None, None).encode("utf-8") first = asyncio.ensure_future( public_key_fetch.public_key_pem(owid, "https", held_open) @@ -653,7 +829,7 @@ async def test_cancelling_one_waiter_leaves_the_request_running( async def held_open(url: str, timeout: float) -> Tuple[int, bytes]: calls.append(url) await release.wait() - return 200, pem.encode("utf-8") + return 200, endpoints.public_key_answer(pem, None, None, None).encode("utf-8") first = asyncio.ensure_future( public_key_fetch.public_key_pem(owid, "https", held_open) @@ -733,7 +909,7 @@ async def test_a_transport_of_the_callers_own_is_used(self) -> None: async def transport(url: str, timeout: float) -> Tuple[int, bytes]: calls.append((url, timeout)) - return 200, pem.encode("utf-8") + return 200, endpoints.public_key_answer(pem, None, None, None).encode("utf-8") self.assertIs( SignatureStatus.SIGNATURE_VALID, @@ -765,7 +941,7 @@ async def test_verify_answers_true_only_for_a_genuine_signature( following = keys[keys.index(signing) + 1] async def wrong_week(url: str, timeout: float) -> Tuple[int, bytes]: - return 200, following.public_key_pem.encode("utf-8") + return 200, endpoints.public_key_answer(following.public_key_pem, None, None, None).encode("utf-8") self.assertFalse( await public_key_fetch.verify(owid, "https", ALONE, wrong_week), diff --git a/tests/test_public_key_response_at.py b/tests/test_public_key_response_at.py index 6d8e70f..ea201be 100644 --- a/tests/test_public_key_response_at.py +++ b/tests/test_public_key_response_at.py @@ -21,6 +21,7 @@ from __future__ import annotations +import json import unittest from datetime import datetime, timedelta, timezone @@ -52,16 +53,31 @@ def setUp(self) -> None: def minutes(self, moment: datetime) -> str: return str(io.minutes_since_base(moment)) + def answer(self, key: DatedPublicKey) -> str: + """The JSON body the end point answers with for the key, stating the + moments it is valid from and to from the schedule.""" + return json.dumps( + { + "publicKeySPKI": key.public_key_pem, + "validFrom": key.starts_at.strftime("%Y-%m-%dT%H:%M:%SZ"), + "validTo": ( + None + if self.schedule.next_start_after(key) is None + else self.schedule.next_start_after(key).strftime("%Y-%m-%dT%H:%M:%SZ") + ), + } + ) + def test_a_dated_request_is_served_the_key_in_force_then(self) -> None: asked = datetime(2026, 8, 26, tzinfo=timezone.utc) self.assertEqual( - (200, self.last_week.public_key_pem), + (200, self.answer(self.last_week)), endpoints.public_key_response_at( self.schedule, "pkcs", self.minutes(asked), self.now ), ) self.assertEqual( - (200, self.this_week.public_key_pem), + (200, self.answer(self.this_week)), endpoints.public_key_response_at( self.schedule, "spki", self.minutes(self.now), self.now ), @@ -70,7 +86,7 @@ def test_a_dated_request_is_served_the_key_in_force_then(self) -> None: def test_the_date_may_arrive_as_a_number(self) -> None: asked = datetime(2026, 8, 26, tzinfo=timezone.utc) self.assertEqual( - (200, self.last_week.public_key_pem), + (200, self.answer(self.last_week)), endpoints.public_key_response_at( self.schedule, "pkcs", io.minutes_since_base(asked), self.now ), @@ -81,7 +97,7 @@ def test_an_undated_request_is_served_the_key_in_force_now(self) -> None: one whose period has not begun.""" for absent in (None, ""): self.assertEqual( - (200, self.this_week.public_key_pem), + (200, self.answer(self.this_week)), endpoints.public_key_response_at( self.schedule, "pkcs", absent, self.now ), @@ -92,7 +108,7 @@ def test_a_future_date_is_read_as_now(self) -> None: key that has signed nothing yet has nothing to verify.""" future = datetime(2026, 9, 8, tzinfo=timezone.utc) self.assertEqual( - (200, self.this_week.public_key_pem), + (200, self.answer(self.this_week)), endpoints.public_key_response_at( self.schedule, "pkcs", self.minutes(future), self.now ), @@ -103,7 +119,7 @@ def test_a_date_beyond_the_calendar_is_read_as_now(self) -> None: is after every key, so the answer is the key in force now rather than a failure in the date arithmetic.""" self.assertEqual( - (200, self.this_week.public_key_pem), + (200, self.answer(self.this_week)), endpoints.public_key_response_at( self.schedule, "pkcs", str(0xFFFFFFFF), self.now ), @@ -156,7 +172,44 @@ def test_the_moment_of_the_request_defaults_to_now(self) -> None: key for key in self.schedule.keys if key.starts_at <= datetime.now(timezone.utc) ] - self.assertEqual(started[-1].public_key_pem, body) + self.assertEqual(started[-1].public_key_pem, json.loads(body)["publicKeySPKI"]) + + def test_the_answer_states_the_span_and_is_checked_before_it_is_sent(self) -> None: + """The answer carries the moments the key is valid from and to, the + last key of the schedule has no end, and an answer a client would + refuse is refused by the creator first.""" + status, body = endpoints.public_key_response_at( + self.schedule, "pkcs", self.minutes(self.now), self.now + ) + self.assertEqual(200, status) + answer = json.loads(body) + self.assertEqual(self.this_week.public_key_pem, answer["publicKeySPKI"]) + self.assertEqual("2026-08-31T00:00:00Z", answer["validFrom"]) + self.assertEqual("2026-09-07T00:00:00Z", answer["validTo"]) + status, body = endpoints.public_key_response_at( + self.schedule, "pkcs", None, datetime(2026, 9, 10, tzinfo=timezone.utc) + ) + self.assertIsNone(json.loads(body)["validTo"], "the last key has no end") + with self.assertRaises(OwidError): + endpoints.public_key_answer("not a key", None, None, None) + with self.assertRaises(OwidError): + endpoints.public_key_answer( + self.this_week.public_key_pem, + self.this_week.starts_at, + self.last_week.starts_at, + None, + ) + with self.assertRaises(OwidError): + endpoints.public_key_answer( + self.this_week.public_key_pem, self.this_week.starts_at, None, + self.last_week.starts_at, + ) + with self.assertRaises(OwidError): + endpoints.validate_public_key_answer( + {"publicKeySPKI": self.this_week.public_key_pem, "validFrom": None, + "validTo": "2026-09-07T00:00:00Z"}, + None, + ) if __name__ == "__main__": From a43d26e0937b502ce17c6faaa1b85e123eec1aa6 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 7 Sep 2026 16:08:56 +0100 Subject: [PATCH 5/8] Align the key cache with the specification on the neighbouring key and the stated span The neighbouring key is asked for by the minute just beyond the edge of the span the creator stated, rather than by a minute a fixed distance from the identifier, so a key in force for less than the drift allowance is still the one tried. The span a caller sees is the one the creator stated. A key stated with a start and no end has no later edge, whatever the cache holds it for, so a live identifier dated just after a rotation is checked against the key before it. A creator that stated no span has one key and no neighbour to try. Where the creator's own statement puts the identifier's date outside the span of the key it answered with and nothing verifies, the key is reported as unavailable rather than the signature as not matching, because a key that was not in force proves nothing about the identifier. The key request asks for the JSON form by name in its Accept header, and the README describes the cache as it is, so a creator that answers the PEM alone is refused and a live identifier from a creator that states its spans costs one request per key. --- README.md | 58 +++++++----- owid/endpoints.py | 4 +- owid/public_key_fetch.py | 168 +++++++++++++++++++++++---------- tests/test_public_key_fetch.py | 126 ++++++++++++++++++++++++- 4 files changed, 272 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index cd92e7a..0889291 100644 --- a/README.md +++ b/README.md @@ -173,31 +173,36 @@ creator that ignores the parameter returns its current key, so every identifier it signed under an earlier key reads as not matching, which is why a creator that rotates its key has to honour the date. -Keys already fetched are held by creator, each against the span of minutes the -creator has confirmed it for. A key is in force from the start of its period -until the next key starts, so a key the creator answers with at two minutes was -in force at every minute between them, and an identifier dated inside a -confirmed span is verified without a request whichever minute it carries. One -dated outside every span is asked about, which widens the span when the same -key comes back. The creator answers with the key and the moments it is valid -from and to, so the whole span is held from one answer and an identifier dated +Keys already fetched are held by creator. The request names the minute the +identifier was created, so a creator that rotates its key answers with the key +in force then, and the answer is the JSON form, which carries the moments the +key is valid from and to as well as the key. A creator built on this package +states both, so the whole span is held from one answer and an identifier dated anywhere in it is verified without a request whatever the clock drift. An -answer in any other form, the PEM alone among them, is reported as a key that -cannot be read. A signature that does not verify under the key selected, where -the identifier is dated within fifteen minutes of the edge of that key's span, -is checked against the neighbouring key before it is reported as not matching, -because a creator's signing machines may not agree with its schedule to the -minute. One dated within fifteen minutes of now, or later, is asked about every -time and never held, because a creator whose clock differs from this one's may -have read that minute as its present rather than as the minute named. Live -identifiers therefore cost one request per minute per creator, as they always -did, and older ones cost none. At most 1024 keys are held across every creator -before the store is emptied and filled again, and `clear_cache()` empties it on -demand, which is how a long running process drops a key it has learned it -should no longer trust. Two callers who await the same key at the same moment -share one request rather than making two. Each request waits at most ten -seconds. Every function that reaches the network is a coroutine, so a caller -awaits it, and there is no synchronous form. +answer that states the start alone is held from the start up to fifteen minutes +behind now, because no later key can have started before then. An answer that +states no span comes from a creator with one key and no schedule, and is held +against the minute asked about and every minute between two such answers for +the same key, but never for a minute within fifteen minutes of now, because a +creator whose clock differs from this one's may have read that minute as its +present rather than as the minute named. The PEM alone as text is not a valid +answer and is reported as a key that cannot be read. A signature that does not +verify under the key selected, where the identifier is dated within fifteen +minutes of an edge of the span the creator stated for that key, is checked +against the key for the minute just beyond that edge before it is reported as +not matching, because a creator's signing machines may not agree with its +schedule to the minute. Where the creator's own statement puts the identifier's +date outside the span of the key it answered with and nothing verifies, the key +is reported as unavailable rather than the signature as not matching, because a +key that was not in force proves nothing about the identifier. Live identifiers +from a creator that states its spans cost one request per key, and older ones +cost none. At most 1024 keys are held across every creator before the store is +emptied and filled again, and `clear_cache()` empties it on demand, which is +how a long running process drops a key it has learned it should no longer +trust. Two callers who await the same key at the same moment share one request +rather than making two. Each request waits at most ten seconds. Every function +that reaches the network is a coroutine, so a caller awaits it, and there is no +synchronous form. ```python import asyncio @@ -480,8 +485,9 @@ opaque crypto error. domain and the response code. - `await signature_status(owid, scheme, others=None, transport=None)` answers with the status, so a key that could not be fetched is `KEY_UNAVAILABLE`, - one that could not be read is `INVALID_KEY`, and neither is mistaken for a - signature that does not match. `verify` takes the same arguments, is + as is one the creator says was not in force at the identifier's date, one + that could not be read is `INVALID_KEY`, and none of these is mistaken for + a signature that does not match. `verify` takes the same arguments, is awaited in the same way, and answers True only for `SIGNATURE_VALID`. - `clear_cache()` empties the keys already fetched and forgets the requests still in flight. It makes no request and is not awaited. diff --git a/owid/endpoints.py b/owid/endpoints.py index ac36c31..566e1d4 100644 --- a/owid/endpoints.py +++ b/owid/endpoints.py @@ -20,8 +20,8 @@ server can serve them. The mandatory end points are the creator end point at /owid/api/v{version}/creator returning JSON with the domain, common name, and public key of the creator, and the public key end point at -/owid/api/v{version}/public-key returning the public key as PEM text. The -format query parameter must be spki or pkcs. +/owid/api/v{version}/public-key returning the public key as a JSON object. +The format query parameter must be spki or pkcs. The public key end point answers with a JSON object carrying the key as publicKeySPKI together with validFrom and validTo, the UTC moments the key came diff --git a/owid/public_key_fetch.py b/owid/public_key_fetch.py index 83ad37f..ce127ef 100644 --- a/owid/public_key_fetch.py +++ b/owid/public_key_fetch.py @@ -43,9 +43,14 @@ it is verified without a request whichever minute it carries. A creator that states no span has its key held against the minutes it confirms. A signature that fails under the key selected, where the identifier is dated within the -clock drift allowance of the edge of that key's span, is checked against the -neighbouring key before it is reported as not matching. Two callers who await -the same key at the same moment share one request rather than making two. +clock drift allowance of an edge of the span the creator stated for that key, +is checked against the key for the minute just beyond that edge before it is +reported as not matching. Where the creator's own statement puts the +identifier's date outside the span of the key it answered with and nothing +verifies, the key is reported as unavailable rather than the signature as not +matching, because a key that was not in force proves nothing about the +identifier. Two callers who await the same key at the same moment share one +request rather than making two. The Java port answers the same question with PublicKeyFetch, the Rust port with Owid::verify_status and the Go port with SignatureStatusFromDomain. @@ -81,16 +86,21 @@ #: How far a creator's clock may run ahead of or behind this one's, in #: minutes. #: -#: It is used in two places. A creator that does not state the span of the key -#: it answers with reads a date later than its own now as now, so within this -#: window of now this process cannot tell whether the creator read the minute -#: as its past or as its present, and nothing learned from such an answer is -#: held or served. And a creator's signing machines may not agree with the -#: creator's own schedule to the minute, so an identifier dated within this -#: window of a key's edge that does not verify under that key is checked -#: against the neighbouring key before it is reported as not matching. +#: It is used in two places. A creator that does not state the end of the span +#: of the key it answers with reads a date later than its own now as now, so +#: within this window of now this process cannot tell whether the creator read +#: the minute as its past or as its present, and nothing learned from such an +#: answer is held or served. And a creator's signing machines may not agree +#: with the creator's own schedule to the minute, so an identifier dated within +#: this window of an edge of the span the creator stated for a key that does +#: not verify under that key is checked against the key for the minute just +#: beyond that edge before it is reported as not matching. CLOCK_DRIFT_ALLOWANCE_MINUTES = 15 +#: The last minute the date field of an OWID can hold, which is where a span +#: stated with a start and no end runs to. +_LAST_MINUTE = 0xFFFFFFFF + #: The most bytes accepted from a response. A public key PEM is a few hundred #: bytes, so a body beyond this is not a key and is not held or decoded. MAXIMUM_RESPONSE_BYTES = 65536 @@ -121,9 +131,11 @@ class _HeldKey: grows as the creator confirms the same key for more minutes. """ - __slots__ = ("pem", "first", "last", "explicit") + __slots__ = ("pem", "first", "last", "explicit", "open_ended") - def __init__(self, pem: str, first: int, last: int, explicit: bool) -> None: + def __init__( + self, pem: str, first: int, last: int, explicit: bool, open_ended: bool + ) -> None: #: The key in PEM form, as the creator served it. self.pem = pem #: The earliest minute the key is known to cover. @@ -132,6 +144,10 @@ def __init__(self, pem: str, first: int, last: int, explicit: bool) -> None: self.last = last #: Whether the creator stated the whole span itself. self.explicit = explicit + #: Whether the creator stated the start of the span and no end, so + #: that as far as the creator has said the key is in force until + #: further notice, whatever this cache holds it for. + self.open_ended = open_ended def covers(self, minute: int) -> bool: """Whether the minute lies within the known span.""" @@ -139,22 +155,28 @@ def covers(self, minute: int) -> bool: class _KeyAnswer: - """What the cache or a fetch answers with. The key, and where it is known, - the span of minutes the key covers, so that a caller can tell whether the - identifier it is checking sits near the edge of the span.""" + """What the cache or a fetch answers with. The key, and where the creator + stated one, the span of minutes the creator says the key covers, so that + a caller can tell whether the identifier it is checking sits near an edge + of the span, or outside it altogether. A span stated with a start and no + end runs to the last minute there is.""" __slots__ = ("pem", "first", "last", "known") def __init__( self, pem: str, first: int = 0, last: int = 0, known: bool = False ) -> None: + #: The key in PEM form. self.pem = pem + #: The first minute the creator says the key covers. self.first = first + #: The last minute the creator says the key covers. self.last = last + #: Whether the creator stated a span at all. self.known = known def covers(self, minute: int) -> bool: - """Whether the minute lies within the known span.""" + """Whether the minute lies within the stated span.""" return self.known and self.first <= minute <= self.last @@ -303,7 +325,15 @@ async def _signature_status_at_url( ) -> SignatureStatus: """The work signature_status does once the URL is known, kept apart so that the tests drive the real fetch against a key end point the tests can - stand up locally rather than against a near copy of the fetch.""" + stand up locally rather than against a near copy of the fetch. + + The signature is checked under the key the end point serves for the + OWID's own date, and under the neighbouring key where the date is within + the clock drift allowance of an edge of the span the creator stated. A + key the creator says was not in force at the OWID's date proves nothing + about the identifier, so where nothing verifies under such a key the + answer is that the key is unavailable and not that the signature does not + match.""" try: answer = await _key_at_url(url, owid.domain, transport) except PublicKeyFetchError as failed: @@ -311,15 +341,21 @@ async def _signature_status_at_url( except OwidError: return SignatureStatus.KEY_UNAVAILABLE status = owid.signature_status(answer.pem, others) - if status is SignatureStatus.SIGNATURE_INVALID and await _neighbour_verifies( - owid, url, answer, others, transport - ): + if status is not SignatureStatus.SIGNATURE_INVALID: + return status + minute = io.minutes_since_base(owid.date) + if minute < 0: + return status + if await _neighbour_verifies(owid, minute, url, answer, others, transport): return SignatureStatus.SIGNATURE_VALID + if answer.known and not answer.covers(minute): + return SignatureStatus.KEY_UNAVAILABLE return status async def _neighbour_verifies( owid: Owid, + minute: int, url: str, tried: _KeyAnswer, others: Optional[Sequence[Owid]], @@ -333,25 +369,21 @@ async def _neighbour_verifies( signed with the key before it, and one dated just before may have been signed with the key after. Where the signature does not verify under the key selected and the OWID's minute is within the clock drift allowance of - the edge of the span that key is known to cover, the key for the minute - just beyond that edge is asked for and tried. A key already known to cover - the neighbouring minute is not asked for again, and a neighbour that turns - out to be the same key is not tried again. This costs at most two more - requests, and only for a signature that has already failed. + an edge of the span the creator stated for that key, the key for the + minute just beyond that edge is asked for and tried. A key already held + for that minute is not asked for again, and a neighbour that turns out to + be the same key is not tried again. A creator that stated no span has one + key and no schedule, so there is no neighbour to try. This costs at most + two more requests, and only for a signature that has already failed. """ - minute = io.minutes_since_base(owid.date) - if minute < 0: - return False - if tried.known and not tried.covers(minute): - # The key tried was never in force at the identifier's minute, so - # the identifier is not near an edge of that key's span. This is an - # undated request answered with the current key, or a creator whose - # answer did not cover the minute asked about, and the neighbours of - # the minute have nothing to do with the key tried. + if not tried.known: return False - for at in (minute - CLOCK_DRIFT_ALLOWANCE_MINUTES, minute + CLOCK_DRIFT_ALLOWANCE_MINUTES): - if at < 0 or at > 0xFFFFFFFF or tried.covers(at): - continue + beyond: List[int] = [] + if tried.first > 0 and _near_edge(minute, tried.first): + beyond.append(tried.first - 1) + if tried.last < _LAST_MINUTE and _near_edge(minute, tried.last): + beyond.append(tried.last + 1) + for at in beyond: try: neighbour = await _key_at_url( "{0}?date={1}&format=pkcs".format(_end_point_of(url), at), @@ -369,6 +401,13 @@ async def _neighbour_verifies( return False +def _near_edge(minute: int, edge: int) -> bool: + """Whether the minute is no further from the edge minute than the clocks + of a creator's signing machines are allowed to differ from its + schedule.""" + return abs(minute - edge) <= CLOCK_DRIFT_ALLOWANCE_MINUTES + + async def _public_key_pem_at_url( url: str, domain: str, transport: Optional[Transport] = None ) -> str: @@ -509,17 +548,38 @@ def _held_for(end_point: str, url: str) -> Optional[_KeyAnswer]: return None for key in _cache.get(end_point, ()): if key.covers(minute) and (key.explicit or not recent): - return _KeyAnswer(key.pem, key.first, key.last, True) + return _stated_for(key) return None +def _stated_for(key: _HeldKey) -> _KeyAnswer: + """The span the creator stated for a held key, which is the whole held + span where the creator stated it, runs to the last minute there is where + the creator stated a start and no end, and is nothing where the creator + stated no span.""" + if key.explicit: + return _KeyAnswer(key.pem, key.first, key.last, True) + if key.open_ended: + return _KeyAnswer(key.pem, key.first, _LAST_MINUTE, True) + return _KeyAnswer(key.pem) + + +def _stated(pem: str, start: Optional[int], end: Optional[int]) -> _KeyAnswer: + """The span the creator stated in its answer. See _stated_for.""" + if start is None: + return _KeyAnswer(pem) + if end is not None and end > start: + return _KeyAnswer(pem, start, end - 1, True) + return _KeyAnswer(pem, start, _LAST_MINUTE, True) + + def _hold( end_point: str, url: str, pem: str, start: Optional[int], end: Optional[int] ) -> _KeyAnswer: """Records the creator's answer to the URL, being the key and, where the creator stated it, the span the key covers as the minute it came into - force and the minute the next key starts. Returns the key with the span it - is now known to cover. Called under the lock. + force and the minute the next key starts. Returns the key with the span + the creator stated for it. Called under the lock. With both the start and the end the whole span is held as the creator's own statement. With the start alone the key is held from the start up to @@ -531,32 +591,38 @@ def _hold( the cache must not grow on the input of whoever presents the identifiers. """ global _held_keys + stated = _stated(pem, start, end) minute, recent = _minute_and_recency(url) explicit = False + open_ended = False if start is not None and end is not None and end > start: first, last, explicit = start, end - 1, True elif start is not None: now = io.minutes_since_base(datetime.now(timezone.utc)) first = start last = max(start, now - CLOCK_DRIFT_ALLOWANCE_MINUTES) + open_ended = True elif minute is not None and not recent: first = last = minute else: - return _KeyAnswer(pem) + return stated keys = _cache.get(end_point) if keys is not None: for key in keys: if key.pem == pem: if _widen(keys, key, first, last): key.explicit = key.explicit or explicit - return _KeyAnswer(pem, key.first, key.last, True) - # The creator has answered with another key inside this span - # before, which it does not do unless it went back to a key - # it had left. Nothing more is held about this key. - return _KeyAnswer(pem) + key.open_ended = not key.explicit and ( + key.open_ended or open_ended + ) + # Where the span was not widened the creator has answered + # with another key inside it before, which it does not do + # unless it went back to a key it had left, and nothing more + # is held about this key. + return stated for other in keys: if other.last >= first and other.first <= last: - return _KeyAnswer(pem) + return stated if _held_keys >= MAXIMUM_CACHED_KEYS: _cache.clear() _held_keys = 0 @@ -564,9 +630,9 @@ def _hold( if keys is None: keys = [] _cache[end_point] = keys - keys.append(_HeldKey(pem, first, last, explicit)) + keys.append(_HeldKey(pem, first, last, explicit, open_ended)) _held_keys += 1 - return _KeyAnswer(pem, first, last, True) + return stated def _widen(keys: List[_HeldKey], key: _HeldKey, first: int, last: int) -> bool: @@ -681,7 +747,7 @@ def _urllib_request(url: str, timeout: float) -> Tuple[int, bytes]: is returned as that code, and only the failure to obtain any response at all is raised.""" request = urllib.request.Request( - url, headers={"Accept": "text/plain"}, method="GET" + url, headers={"Accept": "application/json"}, method="GET" ) try: with _opener.open(request, timeout=timeout) as response: diff --git a/tests/test_public_key_fetch.py b/tests/test_public_key_fetch.py index 1f4a8a9..ef348bf 100644 --- a/tests/test_public_key_fetch.py +++ b/tests/test_public_key_fetch.py @@ -66,6 +66,23 @@ IDENTIFIER_DATE = datetime(2026, 9, 4, tzinfo=timezone.utc) +def date_of(url: str) -> Optional[int]: + """The minute a key URL asks about, or None where it names none.""" + query = urllib.parse.urlsplit(url).query + value = dict(urllib.parse.parse_qsl(query)).get("date") + return None if value is None else int(value) + + +def answer( + pem: str, valid_from: Optional[datetime], valid_to: Optional[datetime] +) -> bytes: + """The body a creator answers with for the key and the span it states, + built by the package's own server side helper.""" + return endpoints.public_key_answer(pem, valid_from, valid_to, None).encode( + "utf-8" + ) + + def crafted(version: Version, domain: str, date: datetime) -> Owid: """Builds an OWID with the version, domain and date given and a signature of zeroes, for the cases that are about the URL rather than the @@ -188,18 +205,21 @@ async def test_undated_fetch_leaves_an_earlier_weeks_identifier_unverified( """The same identifier against the same end point without the date, which is the request a port that forgets the date makes. The end point answers with the key in force at the moment of the request, ten - days after the identifier was signed, the signature does not match - that key, and a genuine identifier reads as a forgery.""" + days after the identifier was signed, and states a span that does not + include the identifier's date. The signature does not match that key, + and because the creator has said the key was not in force at that + date the key is reported as unavailable rather than a genuine + identifier as a forgery.""" owid = key_fixtures.identifier() end_point = self.end_point() undated = end_point.base + "/owid/api/v3/public-key?format=pkcs" self.assertIs( - SignatureStatus.SIGNATURE_INVALID, + SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch._signature_status_at_url( owid, undated, ALONE ), "an undated request gets the key in force at the request, which " - "did not sign it", + "the creator says was not in force when the identifier was signed", ) self.assertEqual([None], end_point.dates(), "the request carried no date") @@ -629,13 +649,109 @@ async def status_of(owid: Owid) -> SignatureStatus: far = self._signed_at("creator.test", rotation + timedelta(minutes=20), first) self.assertIs(SignatureStatus.SIGNATURE_INVALID, await status_of(far), "well inside the later key's span") - self.assertEqual(2, len(requests), "the neighbouring minutes lie inside the spans held") + self.assertEqual(2, len(requests), "the identifier is further from every edge than clocks may differ") genuine = self._signed_at("creator.test", rotation + timedelta(days=3), second) self.assertIs(SignatureStatus.SIGNATURE_VALID, await status_of(genuine)) forged = self._signed_at("creator.test", rotation + timedelta(days=3), third) self.assertIs(SignatureStatus.SIGNATURE_INVALID, await status_of(forged), "signed with a key not in force at its date") + async def test_the_neighbour_is_asked_for_by_the_minute_just_beyond_the_edge( + self, + ) -> None: + """The neighbouring key is asked for by the minute just beyond the + edge of the span the creator stated, not by a minute a fixed distance + from the identifier, so a key in force for less than the drift + allowance is still the one tried.""" + first, second = Crypto.new(), Crypto.new() + week = timedelta(days=7) + end = datetime(2026, 9, 7, tzinfo=timezone.utc) + rotation = end - week + start = rotation - week + asked: List[Optional[int]] = [] + + async def creator(url: str, timeout: float) -> Tuple[int, bytes]: + date = date_of(url) + asked.append(date) + if date is not None and date < io.minutes_since_base(rotation): + return 200, answer(first.public_key_pem(), start, rotation) + return 200, answer(second.public_key_pem(), rotation, end) + + late = self._signed_at("creator.test", rotation + timedelta(minutes=5), first) + self.assertIs( + SignatureStatus.SIGNATURE_VALID, + await public_key_fetch.signature_status(late, "https", ALONE, creator), + ) + minute = io.minutes_since_base(rotation) + self.assertEqual( + [minute + 5, minute - 1], + asked, + "the identifier's own minute and then the minute just before the span started", + ) + + async def test_a_key_stated_without_an_end_has_no_later_edge(self) -> None: + """A key the creator states a start for and no end is in force until + further notice as far as the creator has said, so a live identifier + dated just after that start which does not verify under it is checked + against the key before it, even though the cache holds the key only + up to the drift allowance behind now.""" + first, second = Crypto.new(), Crypto.new() + now = datetime.now(timezone.utc).replace(second=0, microsecond=0) + rotation = now - timedelta(minutes=5) + start = rotation - timedelta(days=7) + requests: List[str] = [] + + async def creator(url: str, timeout: float) -> Tuple[int, bytes]: + requests.append(url) + date = date_of(url) + if date is not None and date < io.minutes_since_base(rotation): + return 200, answer(first.public_key_pem(), start, rotation) + return 200, answer(second.public_key_pem(), rotation, None) + + live = self._signed_at("creator.test", rotation + timedelta(minutes=2), first) + self.assertIs( + SignatureStatus.SIGNATURE_VALID, + await public_key_fetch.signature_status(live, "https", ALONE, creator), + "a live identifier signed with the key before the current one verifies", + ) + self.assertEqual( + 2, len(requests), "the current key and then the key before it were asked for" + ) + + async def test_a_key_the_creator_says_was_not_in_force_leaves_the_signature_unjudged( + self, + ) -> None: + """A creator whose own statement puts the identifier's date outside + the span of the key it answered with has said that key did not sign + at that date, so nothing verifying under it leaves the key + unavailable rather than the signature not matching. A forgery dated + inside the span is still reported as not matching.""" + first, second, stranger = Crypto.new(), Crypto.new(), Crypto.new() + end = datetime(2026, 9, 7, tzinfo=timezone.utc) + rotation = end - timedelta(days=7) + + # A creator that ignores the date asked about and answers with the + # current key and its span whatever the request. + async def creator(url: str, timeout: float) -> Tuple[int, bytes]: + return 200, answer(second.public_key_pem(), rotation, end) + + earlier = self._signed_at("creator.test", rotation - timedelta(days=3), first) + self.assertIs( + SignatureStatus.KEY_UNAVAILABLE, + await public_key_fetch.signature_status(earlier, "https", ALONE, creator), + "the key answered with was not in force at the identifier's date", + ) + self.assertFalse( + await public_key_fetch.verify(earlier, "https", ALONE, creator), + "the boolean form answers True for a genuine signature alone", + ) + forged = self._signed_at("creator.test", rotation + timedelta(days=3), stranger) + self.assertIs( + SignatureStatus.SIGNATURE_INVALID, + await public_key_fetch.signature_status(forged, "https", ALONE, creator), + "a signature failing under the key in force at its date does not match", + ) + async def test_an_answer_that_is_not_the_json_form_is_a_key_that_cannot_be_read(self) -> None: """The PEM alone as text is reported as a key this package cannot read rather than used, and so is a span that ends before it starts.""" end_point = self.end_point(Answer.PEM_ONLY) From 4aaa1a02b30eefbe52a7fb363ffd1f90e2ffb1df Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 7 Sep 2026 16:10:11 +0100 Subject: [PATCH 6/8] Drop the creator end point The specification no longer has a creator end point. It repeated the key the public-key end point serves and added fields no verifier read. The end point helpers now build the path and the body for the public-key end point alone, and the tests that exercised both end points exercise the one that remains. --- README.md | 6 ++---- owid/endpoints.py | 25 +------------------------ tests/test_endpoints.py | 23 ----------------------- 3 files changed, 3 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 0889291..1655cdb 100644 --- a/README.md +++ b/README.md @@ -461,10 +461,8 @@ opaque crypto error. `endpoints` -- `creator_path(version)` and `public_key_path(version)` return the well known - paths. -- `creator_response(creator, name, contract_url)` returns the creator JSON - with the `domain`, `name`, `publicKeySPKI`, and `contractURL` fields. +- `public_key_path(version)` returns the well known path of the public key end + point. - `public_key_response(creator, format)` returns the JSON body of the public key end point for a creator with one key, the key as `publicKeySPKI` with `validFrom` and `validTo` null. `public_key_response_at` states both moments diff --git a/owid/endpoints.py b/owid/endpoints.py index 566e1d4..7d0435e 100644 --- a/owid/endpoints.py +++ b/owid/endpoints.py @@ -17,9 +17,7 @@ specification. These are framework agnostic. They return the path and body so that any HTTP -server can serve them. The mandatory end points are the creator end point at -/owid/api/v{version}/creator returning JSON with the domain, common name, and -public key of the creator, and the public key end point at +server can serve them. The mandatory end point is the public key end point at /owid/api/v{version}/public-key returning the public key as a JSON object. The format query parameter must be spki or pkcs. @@ -47,33 +45,12 @@ from .version import Version -def creator_path(version: Version) -> str: - """Returns the path of the creator end point for the version provided. For - example /owid/api/v3/creator.""" - return "/owid/api/v{0}/creator".format(version.as_byte()) - - def public_key_path(version: Version) -> str: """Returns the path of the public key end point for the version provided. For example /owid/api/v3/public-key.""" return "/owid/api/v{0}/public-key".format(version.as_byte()) -def creator_response(creator: Creator, name: str, contract_url: str = "") -> str: - """Returns the JSON body for the creator end point. - - The body contains the domain, name, public key in SPKI form, and the - contract URL. - """ - body = { - "domain": creator.domain, - "name": name, - "publicKeySPKI": creator.crypto.subject_public_key_info(), - "contractURL": contract_url, - } - return json.dumps(body) - - def public_key_response(creator: Creator, format: str) -> str: """Returns the JSON body for the public key end point of a creator with one key and no schedule. The key is stated as publicKeySPKI and both diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 62674cf..9278a31 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -29,34 +29,11 @@ def _new_creator() -> Creator: class EndpointTests(unittest.TestCase): def test_paths(self) -> None: - self.assertEqual( - endpoints.creator_path(Version.VERSION3), "/owid/api/v3/creator" - ) self.assertEqual( endpoints.public_key_path(Version.VERSION3), "/owid/api/v3/public-key", ) - def test_creator_response_fields(self) -> None: - creator = _new_creator() - body = endpoints.creator_response( - creator, "Example Org", "https://example.com/contract" - ) - parsed = json.loads(body) - self.assertEqual(parsed["domain"], "example.com") - self.assertEqual(parsed["name"], "Example Org") - self.assertEqual(parsed["contractURL"], "https://example.com/contract") - self.assertIn("BEGIN PUBLIC KEY", parsed["publicKeySPKI"]) - # The JSON field names must match the specification. - self.assertIn("publicKeySPKI", body) - self.assertIn("contractURL", body) - - def test_creator_response_default_contract_url(self) -> None: - creator = _new_creator() - body = endpoints.creator_response(creator, "Example Org") - parsed = json.loads(body) - self.assertEqual(parsed["contractURL"], "") - def test_public_key_response_formats(self) -> None: creator = _new_creator() for fmt in ("spki", "pkcs"): From 0ee7e733e8e3e2d2335c1d4415f2e2b2b4775299 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 7 Sep 2026 16:14:03 +0100 Subject: [PATCH 7/8] Answer with publicKey and format The public-key answer carries the key as publicKey and the encoding it is in as format. The one format served is spki, which a request without the parameter receives, and any other value is answered 400 rather than in an encoding the caller did not ask for. The client asks for spki by name, reads the key from publicKey and refuses an answer that states another format as a key it cannot read. --- README.md | 25 +++--- owid/endpoints.py | 112 ++++++++++++++++----------- owid/public_key_fetch.py | 15 ++-- tests/key_end_point.py | 24 ++++-- tests/test_endpoints.py | 14 +++- tests/test_public_key_fetch.py | 54 ++++++++++--- tests/test_public_key_response_at.py | 62 +++++++++------ 7 files changed, 200 insertions(+), 106 deletions(-) diff --git a/README.md b/README.md index 1655cdb..522ea1b 100644 --- a/README.md +++ b/README.md @@ -166,9 +166,10 @@ anything older than a few days means asking for the key that was in force on the date the identifier carries. `owid.public_key_fetch` asks the creator for that key. The request is -`/owid/api/v{n}/public-key?date={minutes}&format=pkcs`, where the version in -the path is the version byte of the identifier being checked and the minutes -are counted from 2020-01-01 in the same way the identifier stores its date. A +`/owid/api/v{n}/public-key?date={minutes}&format=spki`, where the version in +the path is the version byte of the identifier being checked, the minutes are +counted from 2020-01-01 in the same way the identifier stores its date, and +the format names the one encoding of the key this package reads. A creator that ignores the parameter returns its current key, so every identifier it signed under an earlier key reads as not matching, which is why a creator that rotates its key has to honour the date. @@ -464,15 +465,19 @@ opaque crypto error. - `public_key_path(version)` returns the well known path of the public key end point. - `public_key_response(creator, format)` returns the JSON body of the public - key end point for a creator with one key, the key as `publicKeySPKI` with - `validFrom` and `validTo` null. `public_key_response_at` states both moments - from the schedule, and `public_key_answer` builds and checks any answer, so - a key that cannot be read or a schedule that contradicts itself is refused - before it is sent. The PEM alone as text is no longer a valid answer. The - format must be `spki` or `pkcs`. + key end point for a creator with one key, the key as `publicKey`, the + encoding it is in as `format`, and `validFrom` and `validTo` null. + `public_key_response_at` states both moments from the schedule, and + `public_key_answer` builds and checks any answer, so a key that cannot be + read or a schedule that contradicts itself is refused before it is sent. + The PEM alone as text is not a valid answer. The one format defined is + `spki`, a Subject Public Key Info PEM. It is what a request without a + `format` receives, and any other value is refused rather than answered in + an encoding the caller did not ask for. - `public_key_response_at(schedule, format, date, now=None)` returns the status code and body for a creator that rotates its key, choosing from a - `PublicKeySchedule` the way the specification requires. + `PublicKeySchedule` the way the specification requires, and answers 400 to + a `format` other than `spki`. `public_key_fetch` diff --git a/owid/endpoints.py b/owid/endpoints.py index 7d0435e..b62c0b4 100644 --- a/owid/endpoints.py +++ b/owid/endpoints.py @@ -19,16 +19,18 @@ These are framework agnostic. They return the path and body so that any HTTP server can serve them. The mandatory end point is the public key end point at /owid/api/v{version}/public-key returning the public key as a JSON object. -The format query parameter must be spki or pkcs. The public key end point answers with a JSON object carrying the key as -publicKeySPKI together with validFrom and validTo, the UTC moments the key came -into force and the next key starts, so a client holds the key for the whole -span from one answer. A creator that rotates its signing key answers the -optional date parameter with public_key_response_at, which chooses from the -published schedule the way the specification requires. Every answer is checked -with validate_public_key_answer before it is returned, so a creator never sends -an answer a client would refuse. +publicKey, the encoding it is in as format, and validFrom and validTo, the UTC +moments the key came into force and the next key starts, so a client holds the +key for the whole span from one answer. The only format defined is spki, a +Subject Public Key Info PEM. It is the value taken when the request has no +format parameter, and a creator answers 400 to any other value. A creator that +rotates its signing key answers the optional date parameter with +public_key_response_at, which chooses from the published schedule the way the +specification requires. Every answer is checked with +validate_public_key_answer before it is returned, so a creator never sends an +answer a client would refuse. """ from __future__ import annotations @@ -44,6 +46,11 @@ from .public_key_schedule import PublicKeySchedule from .version import Version +#: The one encoding of the key this package reads and writes, a Subject +#: Public Key Info PEM. It is the value of the format parameter a request +#: without one is read as asking for, and the value every answer states. +SPKI_FORMAT = "spki" + def public_key_path(version: Version) -> str: """Returns the path of the public key end point for the version provided. @@ -51,25 +58,31 @@ def public_key_path(version: Version) -> str: return "/owid/api/v{0}/public-key".format(version.as_byte()) -def public_key_response(creator: Creator, format: str) -> str: +def public_key_response(creator: Creator, format: Optional[str]) -> str: """Returns the JSON body for the public key end point of a creator with - one key and no schedule. The key is stated as publicKeySPKI and both - validFrom and validTo are null, because the creator knows nothing about - when the key started or will stop. + one key and no schedule. The key is stated as publicKey in the spki + format and both validFrom and validTo are null, because the creator knows + nothing about when the key started or will stop. - The specification allows the key to be requested in SPKI or PKCS form. - This implementation returns the SPKI PEM for both values because the - importers in every implementation accept it. + The format is the request's format parameter, or None where the request + has none, which is read as spki. - Raises OwidError if the format is not spki or pkcs, or the key cannot be - read. + Raises OwidError if the format is not spki, which a creator answers 400, + or the key cannot be read. """ - if format not in ("spki", "pkcs"): + _check_format(format) + return public_key_answer(creator.crypto.subject_public_key_info(), None, None, None) + + +def _check_format(format: Optional[str]) -> None: + """Raises OwidError where the format is present and is not the one this + package serves.""" + if format is not None and format != SPKI_FORMAT: raise OwidError( - "format parameter 'spki' or 'pkcs' must be provided, " - "received '{0}'".format(format) + "the only format defined is '{0}', received '{1}'".format( + SPKI_FORMAT, format + ) ) - return public_key_answer(creator.crypto.subject_public_key_info(), None, None, None) def public_key_answer( @@ -79,14 +92,16 @@ def public_key_answer( asked: Optional[datetime], ) -> str: """Returns the JSON body of the public key end point for the key and the - span it covers, checked with validate_public_key_answer first so that a - creator never sends an answer it would itself refuse. The moment asked - about, where known, is checked against the span as well. + span it covers, stating the key in the spki format and checked with + validate_public_key_answer first so that a creator never sends an answer + it would itself refuse. The moment asked about, where known, is checked + against the span as well. Raises OwidError if the answer would not be valid. """ answer = { - "publicKeySPKI": public_key_pem, + "format": SPKI_FORMAT, + "publicKey": public_key_pem, "validFrom": _moment_text(valid_from), "validTo": _moment_text(valid_to), } @@ -101,18 +116,22 @@ def validate_public_key_answer( the client that reads it must, returning the key and the moments it is valid from and to. - The key must be a public key this package can read, a key valid to a - moment must be valid from an earlier one, and where the moment asked about - is known the key must have come into force by then and, if it has an end, - not have ended. A creator that fails this check has a fault in its - schedule or its store, and answering with a server error shows it up - rather than passing it on. + The format, where stated, must be the one this package reads and the key + must be a public key in it, a key valid to a moment must be valid from an + earlier one, and where the moment asked about is known the key must have + come into force by then and, if it has an end, not have ended. A creator + that fails this check has a fault in its schedule or its store, and + answering with a server error shows it up rather than passing it on. Raises OwidError where the answer is not valid. """ if not isinstance(answer, Mapping): raise OwidError("the public key answer is not a JSON object") - pem = answer.get("publicKeySPKI") + if answer.get("format", SPKI_FORMAT) != SPKI_FORMAT: + raise OwidError( + "the public key answer states a format this package does not read" + ) + pem = answer.get("publicKey") if not isinstance(pem, str) or not pem.strip(): raise OwidError("the public key answer holds no key") try: @@ -179,7 +198,7 @@ def _moment_of(value: Any, field: str) -> Optional[datetime]: return moment.astimezone(timezone.utc) def public_key_response_at( schedule: PublicKeySchedule, - format: str, + format: Optional[str], date: Union[str, int, None], now: Optional[datetime] = None, ) -> Tuple[int, str]: @@ -187,25 +206,28 @@ def public_key_response_at( a creator that rotates its key, chosen from the schedule the way the specification requires. - The date parameter is the OWID's own date, counted in whole minutes since + The format parameter names the encoding of the key in the answer. The + only value defined is spki, which a request without the parameter is + read as asking for, and any other value is answered 400 with an empty + body rather than in an encoding the caller did not ask for. The date + parameter is the OWID's own date, counted in whole minutes since 2020-01-01, and the key served is the one in force then, being the latest key whose start is at or before it. A request without a date, or with a date later than the moment of the request, is served the key in force at that moment, so a caller cannot ask for a key whose period has not begun. The answer is 200 with the JSON body from public_key_answer, stating the - key and the moments it is valid from and to, 404 with an empty body where - no key is in force at the date, and 400 with an empty body where the date - is not a count of minutes. The moment of the request is now, and a test - may supply it. + key, its format and the moments it is valid from and to, 404 with an + empty body where no key is in force at the date, and 400 with an empty + body where the date is not a count of minutes. The moment of the request + is now, and a test may supply it. - Raises OwidError if the format is not spki or pkcs, or the answer would - fail validate_public_key_answer, which is a fault in the schedule. + Raises OwidError if the answer would fail validate_public_key_answer, + which is a fault in the schedule. """ - if format not in ("spki", "pkcs"): - raise OwidError( - "format parameter 'spki' or 'pkcs' must be provided, " - "received '{0}'".format(format) - ) + try: + _check_format(format) + except OwidError: + return 400, "" moment = now if now is not None else datetime.now(timezone.utc) if moment.tzinfo is None: # Read as UTC, the only zone the wire format knows, so this agrees diff --git a/owid/public_key_fetch.py b/owid/public_key_fetch.py index ce127ef..6be3b00 100644 --- a/owid/public_key_fetch.py +++ b/owid/public_key_fetch.py @@ -17,10 +17,11 @@ on the domain the OWID carries, asking for the key that was in force on the date the OWID carries. -The end point is /owid/api/v{n}/public-key?date={minutes}&format=pkcs, where +The end point is /owid/api/v{n}/public-key?date={minutes}&format=spki, where the version in the path is the version byte of the OWID being checked rather -than a constant, and the minutes are counted from 2020-01-01 in the same way -the OWID stores the date. Creators rotate weekly, so without the date only +than a constant, the minutes are counted from 2020-01-01 in the same way the +OWID stores the date, and the format names the one encoding of the key this +package reads. Creators rotate weekly, so without the date only identifiers signed since the most recent rotation can be verified and every older one reads as not matching. A creator that ignores the parameter returns its current key, so every identifier it signed under an earlier key reads as @@ -235,9 +236,9 @@ def public_key_url(owid: Owid, scheme: str) -> str: _check_domain(domain) minutes = io.minutes_since_base(owid.date) if minutes >= 0: - query = "date={0}&format=pkcs".format(minutes) + query = "date={0}&format={1}".format(minutes, endpoints.SPKI_FORMAT) else: - query = "format=pkcs" + query = "format={0}".format(endpoints.SPKI_FORMAT) return "{0}://{1}{2}?{3}".format( scheme, domain, endpoints.public_key_path(owid.version), query ) @@ -386,7 +387,9 @@ async def _neighbour_verifies( for at in beyond: try: neighbour = await _key_at_url( - "{0}?date={1}&format=pkcs".format(_end_point_of(url), at), + "{0}?date={1}&format={2}".format( + _end_point_of(url), at, endpoints.SPKI_FORMAT + ), owid.domain, transport, ) diff --git a/tests/key_end_point.py b/tests/key_end_point.py index be3b941..137b6ea 100644 --- a/tests/key_end_point.py +++ b/tests/key_end_point.py @@ -81,6 +81,7 @@ def do_GET(self) -> None: # noqa: N802 - the name is the protocol's. query = urllib.parse.urlsplit(self.path).query values = urllib.parse.parse_qs(query, keep_blank_values=True) date = values.get("date", [None])[0] + format = values.get("format", [None])[0] end_point.record(date) if end_point.answer is Answer.REDIRECT: self.send_response(302) @@ -89,10 +90,11 @@ def do_GET(self) -> None: # noqa: N802 - the name is the protocol's. self.end_headers() return try: - body = end_point.body(date) + body = end_point.body(date, format) except ValueError: - # A date that is not a number is refused, as the cloud refuses - # it, rather than failing inside the handler. + # A date that is not a number, or a format the end point does not + # serve, is refused as the cloud refuses it, rather than failing + # inside the handler. self.send_response(400) self.send_header("Content-Length", "0") self.end_headers() @@ -161,14 +163,18 @@ def answer(self) -> Answer: """What this end point serves.""" return self._answer - def body(self, date: Optional[str]) -> Optional[str]: + def body(self, date: Optional[str], format: Optional[str]) -> Optional[str]: """The body to serve, or None where the end point has no key. Raises - ValueError where the date is not a count of minutes.""" + ValueError where the date is not a count of minutes or the format is + not one the end point serves.""" if self._answer is Answer.BROKEN_KEY: - # Shaped like a PEM, with a body no key can be read out of. It is sent as the JSON form without the check a creator applies, because that check is what catches it. + # Shaped like a PEM, with a body no key can be read out of. It is + # sent as the JSON form without the check a creator applies, + # because that check is what catches it. return json.dumps( { - "publicKeySPKI": ( + "format": endpoints.SPKI_FORMAT, + "publicKey": ( "-----BEGIN PUBLIC KEY-----\n" "bm90IGEga2V5\n" "-----END PUBLIC KEY-----\n" @@ -194,7 +200,9 @@ def body(self, date: Optional[str]) -> Optional[str]: # The answer the package's own server side helper builds, so the # client is tested against what a creator built on it sends. status, body = endpoints.public_key_response_at( - self._schedule, "pkcs", date, REQUEST_MOMENT + self._schedule, format, date, REQUEST_MOMENT ) + if status == 400: + raise ValueError("the format is not one the end point serves") assert status == 200, status return body diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 9278a31..169d294 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -35,18 +35,24 @@ def test_paths(self) -> None: ) def test_public_key_response_formats(self) -> None: + """The one format defined is answered whether it is asked for by name + or the request has no format, and the answer names it.""" creator = _new_creator() - for fmt in ("spki", "pkcs"): + for fmt in ("spki", None): body = endpoints.public_key_response(creator, fmt) answer = json.loads(body) - self.assertIn("BEGIN PUBLIC KEY", answer["publicKeySPKI"]) + self.assertEqual("spki", answer["format"], "the answer names the encoding of the key") + self.assertIn("BEGIN PUBLIC KEY", answer["publicKey"]) self.assertIsNone(answer["validFrom"], "a single key has no schedule") self.assertIsNone(answer["validTo"]) def test_public_key_response_invalid_format(self) -> None: + """Any other format is refused rather than answered in an encoding + the caller did not ask for.""" creator = _new_creator() - with self.assertRaises(OwidError): - endpoints.public_key_response(creator, "other") + for fmt in ("pkcs", "other"): + with self.assertRaises(OwidError): + endpoints.public_key_response(creator, fmt) if __name__ == "__main__": diff --git a/tests/test_public_key_fetch.py b/tests/test_public_key_fetch.py index ef348bf..7a844a8 100644 --- a/tests/test_public_key_fetch.py +++ b/tests/test_public_key_fetch.py @@ -148,7 +148,7 @@ def test_url_names_the_minute_the_identifier_was_created(self) -> None: value the end point selects a key by, and it names the well known path from the specification.""" self.assertEqual( - "https://51d.es/owid/api/v3/public-key?date={0}&format=pkcs".format( + "https://51d.es/owid/api/v3/public-key?date={0}&format=spki".format( key_fixtures.IDENTIFIER_MINUTES ), public_key_fetch.public_key_url(key_fixtures.identifier(), "https"), @@ -162,7 +162,7 @@ def test_url_uses_the_version_the_identifier_carries(self) -> None: version2 = crafted(Version.VERSION2, "example.com", IDENTIFIER_DATE) self.assertIs(Version.VERSION2, version2.version) self.assertEqual( - "https://example.com/owid/api/v2/public-key?date={0}&format=pkcs" + "https://example.com/owid/api/v2/public-key?date={0}&format=spki" .format(key_fixtures.IDENTIFIER_MINUTES), public_key_fetch.public_key_url(version2, "https"), "should ask the version 2 end point", @@ -172,7 +172,7 @@ def test_url_of_a_newly_signed_owid_names_its_own_minute(self) -> None: creator = Creator("example.com", Crypto.new()) owid = creator.create_string("payload") self.assertEqual( - "https://example.com/owid/api/v3/public-key?date={0}&format=pkcs" + "https://example.com/owid/api/v3/public-key?date={0}&format=spki" .format(io.minutes_since_base(owid.date)), public_key_fetch.public_key_url(owid, "https"), "should name the minute the OWID was signed", @@ -212,7 +212,7 @@ async def test_undated_fetch_leaves_an_earlier_weeks_identifier_unverified( identifier as a forgery.""" owid = key_fixtures.identifier() end_point = self.end_point() - undated = end_point.base + "/owid/api/v3/public-key?format=pkcs" + undated = end_point.base + "/owid/api/v3/public-key?format=spki" self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch._signature_status_at_url( @@ -234,7 +234,7 @@ async def test_a_key_the_end_point_cannot_serve_is_key_unavailable( # A fortnight before the schedule begins, which no key in it covers, # so the end point answers 404 the way the cloud does. before = key_fixtures.scheduled_keys()[0].starts_at - timedelta(days=14) - url = "{0}/owid/api/v3/public-key?date={1}&format=pkcs".format( + url = "{0}/owid/api/v3/public-key?date={1}&format=spki".format( end_point.base, io.minutes_since_base(before) ) self.assertIs( @@ -248,7 +248,7 @@ async def test_a_refused_request_carries_the_status_and_the_code( ) -> None: """The refusal carries the code and the domain, not only a message.""" end_point = self.end_point() - url = end_point.base + "/owid/api/v3/public-key?date=0&format=pkcs" + url = end_point.base + "/owid/api/v3/public-key?date=0&format=spki" with self.assertRaises(PublicKeyFetchError) as refused: await public_key_fetch._public_key_pem_at_url(url, "51d.es") self.assertIs(SignatureStatus.KEY_UNAVAILABLE, refused.exception.status) @@ -260,7 +260,7 @@ async def test_a_malformed_date_is_refused_by_the_end_point(self) -> None: 400, as the cloud does, and the package reports the refusal as a key that could not be obtained.""" end_point = self.end_point() - url = end_point.base + "/owid/api/v3/public-key?date=abc&format=pkcs" + url = end_point.base + "/owid/api/v3/public-key?date=abc&format=spki" with self.assertRaises(PublicKeyFetchError) as refused: await public_key_fetch._public_key_pem_at_url(url, "51d.es") self.assertIs(SignatureStatus.KEY_UNAVAILABLE, refused.exception.status) @@ -531,7 +531,7 @@ async def test_a_minute_within_the_drift_allowance_is_not_held(self) -> None: await self._pem_at(end_point, recent) await self._pem_at(end_point, now + timedelta(days=7)) await public_key_fetch._public_key_pem_at_url( - end_point.base + "/owid/api/v3/public-key?format=pkcs", + end_point.base + "/owid/api/v3/public-key?format=spki", key_fixtures.IDENTIFIER_DOMAIN, ) old = now - timedelta( @@ -628,10 +628,10 @@ async def test_a_signature_failing_near_the_edge_of_a_span_is_checked_against_th async def creator(url: str, timeout: float) -> Tuple[int, bytes]: requests.append(url) date = dict(urllib.parse.parse_qsl(urllib.parse.urlsplit(url).query)).get("date") - status, body = endpoints.public_key_response_at(schedule, "pkcs", date) + status, body = endpoints.public_key_response_at(schedule, "spki", date) return status, body.encode("utf-8") - url = "https://creator.test/owid/api/v3/public-key?date={0}&format=pkcs" + url = "https://creator.test/owid/api/v3/public-key?date={0}&format=spki" async def status_of(owid: Owid) -> SignatureStatus: return await public_key_fetch._signature_status_at_url( @@ -764,7 +764,7 @@ async def test_an_answer_that_is_not_the_json_form_is_a_key_that_cannot_be_read( async def contradictory(url: str, timeout: float) -> Tuple[int, bytes]: return 200, json.dumps( { - "publicKeySPKI": key_fixtures.schedule().keys[0].public_key_pem, + "publicKey": key_fixtures.schedule().keys[0].public_key_pem, "validFrom": "2026-08-31T00:00:00Z", "validTo": "2026-08-24T00:00:00Z", } @@ -777,6 +777,38 @@ async def contradictory(url: str, timeout: float) -> Tuple[int, bytes]: ), ) + async def test_an_answer_in_another_format_is_a_key_that_cannot_be_read(self) -> None: + """The request asks for the spki format by name, and an answer that + states another format is a key this package cannot read rather than + one it guesses at. An answer that states no format is read as spki, + which is the value a request without the parameter receives.""" + owid = key_fixtures.identifier() + pem = key_fixtures.schedule().key_for(owid).public_key_pem + formats: List[Optional[str]] = [] + + async def creator(url: str, timeout: float) -> Tuple[int, bytes]: + query = urllib.parse.urlsplit(url).query + formats.append(dict(urllib.parse.parse_qsl(query)).get("format")) + return 200, json.dumps( + {"format": "pkcs", "publicKey": pem, "validFrom": None, "validTo": None} + ).encode("utf-8") + + self.assertIs( + SignatureStatus.INVALID_KEY, + await public_key_fetch.signature_status(owid, "https", ALONE, creator), + ) + self.assertEqual(["spki"], formats, "the request asks for spki by name") + + async def unstated(url: str, timeout: float) -> Tuple[int, bytes]: + return 200, json.dumps( + {"publicKey": pem, "validFrom": None, "validTo": None} + ).encode("utf-8") + + self.assertIs( + SignatureStatus.SIGNATURE_VALID, + await public_key_fetch.signature_status(owid, "https", ALONE, unstated), + ) + def test_many_threads_verifying_one_owid_together_make_one_request(self) -> None: """Threads verifying the same OWID at the same moment, each on its own event loop, make one request for its key between them, and every one diff --git a/tests/test_public_key_response_at.py b/tests/test_public_key_response_at.py index ea201be..fd8e16d 100644 --- a/tests/test_public_key_response_at.py +++ b/tests/test_public_key_response_at.py @@ -58,7 +58,8 @@ def answer(self, key: DatedPublicKey) -> str: moments it is valid from and to from the schedule.""" return json.dumps( { - "publicKeySPKI": key.public_key_pem, + "format": "spki", + "publicKey": key.public_key_pem, "validFrom": key.starts_at.strftime("%Y-%m-%dT%H:%M:%SZ"), "validTo": ( None @@ -73,7 +74,7 @@ def test_a_dated_request_is_served_the_key_in_force_then(self) -> None: self.assertEqual( (200, self.answer(self.last_week)), endpoints.public_key_response_at( - self.schedule, "pkcs", self.minutes(asked), self.now + self.schedule, "spki", self.minutes(asked), self.now ), ) self.assertEqual( @@ -88,7 +89,7 @@ def test_the_date_may_arrive_as_a_number(self) -> None: self.assertEqual( (200, self.answer(self.last_week)), endpoints.public_key_response_at( - self.schedule, "pkcs", io.minutes_since_base(asked), self.now + self.schedule, "spki", io.minutes_since_base(asked), self.now ), ) @@ -99,7 +100,7 @@ def test_an_undated_request_is_served_the_key_in_force_now(self) -> None: self.assertEqual( (200, self.answer(self.this_week)), endpoints.public_key_response_at( - self.schedule, "pkcs", absent, self.now + self.schedule, "spki", absent, self.now ), ) @@ -110,7 +111,7 @@ def test_a_future_date_is_read_as_now(self) -> None: self.assertEqual( (200, self.answer(self.this_week)), endpoints.public_key_response_at( - self.schedule, "pkcs", self.minutes(future), self.now + self.schedule, "spki", self.minutes(future), self.now ), ) @@ -121,7 +122,7 @@ def test_a_date_beyond_the_calendar_is_read_as_now(self) -> None: self.assertEqual( (200, self.answer(self.this_week)), endpoints.public_key_response_at( - self.schedule, "pkcs", str(0xFFFFFFFF), self.now + self.schedule, "spki", str(0xFFFFFFFF), self.now ), ) @@ -130,13 +131,13 @@ def test_a_date_before_the_schedule_is_404(self) -> None: self.assertEqual( (404, ""), endpoints.public_key_response_at( - self.schedule, "pkcs", self.minutes(before), self.now + self.schedule, "spki", self.minutes(before), self.now ), ) self.assertEqual( (404, ""), endpoints.public_key_response_at( - PublicKeySchedule([]), "pkcs", None, self.now + PublicKeySchedule([]), "spki", None, self.now ), ) @@ -146,48 +147,65 @@ def test_a_date_that_is_not_a_count_of_minutes_is_400(self) -> None: self.assertEqual( (400, ""), endpoints.public_key_response_at( - self.schedule, "pkcs", malformed, self.now + self.schedule, "spki", malformed, self.now ), ) self.assertEqual( (400, ""), - endpoints.public_key_response_at(self.schedule, "pkcs", -1, self.now), + endpoints.public_key_response_at(self.schedule, "spki", -1, self.now), ) self.assertEqual( (400, ""), - endpoints.public_key_response_at(self.schedule, "pkcs", True, self.now), - ) - - def test_the_format_must_be_spki_or_pkcs(self) -> None: - with self.assertRaises(OwidError): - endpoints.public_key_response_at(self.schedule, "der", None, self.now) + endpoints.public_key_response_at(self.schedule, "spki", True, self.now), + ) + + def test_the_format_is_spki_and_any_other_is_400(self) -> None: + """The format parameter names the encoding of the key in the answer. + The one encoding defined is answered whether or not it is asked for + by name, the answer echoes it, and any other is refused as a bad + request rather than answered in an encoding the caller did not ask + for.""" + for fmt in ("spki", None): + status, body = endpoints.public_key_response_at( + self.schedule, fmt, None, self.now + ) + self.assertEqual(200, status) + self.assertEqual("spki", json.loads(body)["format"]) + for refused in ("pkcs", "der", ""): + with self.subTest(format=refused): + self.assertEqual( + (400, ""), + endpoints.public_key_response_at( + self.schedule, refused, None, self.now + ), + ) def test_the_moment_of_the_request_defaults_to_now(self) -> None: """Without a moment supplied the clock is used, so the answer is the latest key that has started by the time the test runs.""" - status, body = endpoints.public_key_response_at(self.schedule, "pkcs", None) + status, body = endpoints.public_key_response_at(self.schedule, "spki", None) self.assertEqual(200, status) self.assertIn("BEGIN PUBLIC KEY", body) started = [ key for key in self.schedule.keys if key.starts_at <= datetime.now(timezone.utc) ] - self.assertEqual(started[-1].public_key_pem, json.loads(body)["publicKeySPKI"]) + self.assertEqual(started[-1].public_key_pem, json.loads(body)["publicKey"]) def test_the_answer_states_the_span_and_is_checked_before_it_is_sent(self) -> None: """The answer carries the moments the key is valid from and to, the last key of the schedule has no end, and an answer a client would refuse is refused by the creator first.""" status, body = endpoints.public_key_response_at( - self.schedule, "pkcs", self.minutes(self.now), self.now + self.schedule, "spki", self.minutes(self.now), self.now ) self.assertEqual(200, status) answer = json.loads(body) - self.assertEqual(self.this_week.public_key_pem, answer["publicKeySPKI"]) + self.assertEqual(self.this_week.public_key_pem, answer["publicKey"]) self.assertEqual("2026-08-31T00:00:00Z", answer["validFrom"]) self.assertEqual("2026-09-07T00:00:00Z", answer["validTo"]) status, body = endpoints.public_key_response_at( - self.schedule, "pkcs", None, datetime(2026, 9, 10, tzinfo=timezone.utc) + self.schedule, "spki", None, datetime(2026, 9, 10, tzinfo=timezone.utc) ) self.assertIsNone(json.loads(body)["validTo"], "the last key has no end") with self.assertRaises(OwidError): @@ -206,7 +224,7 @@ def test_the_answer_states_the_span_and_is_checked_before_it_is_sent(self) -> No ) with self.assertRaises(OwidError): endpoints.validate_public_key_answer( - {"publicKeySPKI": self.this_week.public_key_pem, "validFrom": None, + {"publicKey": self.this_week.public_key_pem, "validFrom": None, "validTo": "2026-09-07T00:00:00Z"}, None, ) From 978836013a3160a0dadd35e2a334e7468dcfecd3 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 7 Sep 2026 16:20:13 +0100 Subject: [PATCH 8/8] Sign over the OWID alone A signature covers the OWID's own bytes without the signature field and nothing else. Signing and verifying over other OWIDs is gone from Creator, from every verification surface and from the tests, along with the chained interop fixtures, because nothing in production signs that way and an undocumented signing input is a liability for anyone implementing from the specification. --- README.md | 36 +++++------------ owid/__init__.py | 2 +- owid/creator.py | 23 +++-------- owid/owid.py | 55 +++++++++---------------- owid/public_key_fetch.py | 30 ++++++-------- owid/public_key_schedule.py | 14 +++---- tests/fixtures.py | 26 +----------- tests/test_creator.py | 4 +- tests/test_domain_length.py | 2 +- tests/test_owid.py | 67 +++++-------------------------- tests/test_parse_contract.py | 10 ++--- tests/test_payload_length.py | 2 +- tests/test_public_key_fetch.py | 61 +++++++++++++--------------- tests/test_public_key_schedule.py | 17 +++----- 14 files changed, 107 insertions(+), 242 deletions(-) diff --git a/README.md b/README.md index 522ea1b..888a478 100644 --- a/README.md +++ b/README.md @@ -9,9 +9,8 @@ Python. An Open Web Id (OWID) records that the entity operating a domain captured or generated a payload at a date and time, together with an ECDSA signature over -the OWID and any other OWIDs it was signed with. OWIDs chain together to form -verifiable trees. The curve is NIST P-256 (also known as secp256r1 or -prime256v1) and the hash is SHA-256. +the OWID. The curve is NIST P-256 (also known as secp256r1 or prime256v1) and +the hash is SHA-256. Read the [OWID project](https://github.com/SWAN-community/owid) to learn more about the concepts before looking into this implementation. This package @@ -120,24 +119,13 @@ encoded = owid.as_base64() result = Owid.parse(encoded) if result: public_pem = crypto.public_key_pem() - assert result.owid.verify_with_public_key(public_pem, []) + assert result.owid.verify_with_public_key(public_pem) else: # result.status names which of the expected problems it was, for example # ParseStatus.INVALID_BASE64 or ParseStatus.BYTE_COUNT_MISMATCH. reason = result.status.value ``` -OWIDs chain together. Create one that covers others, and pass the same others, -in the same order, when verifying. - -```python -root = creator.create_string("root") -party = creator.create(b"party", [root]) - -assert party.verify_with_crypto(crypto, [root]) -assert not party.verify_with_crypto(crypto, []) -``` - Where the difference between a signature that does not match and a check that could not be made changes what your code should do, ask for the status rather than a true or false answer. A key that cannot be read is reported as a fault @@ -418,10 +406,9 @@ a separate answer. - `payload_as_string()` decodes the payload as UTF-8, replacing invalid bytes. - `payload_as_printable()` returns the payload as lower case hexadecimal. - `payload_as_base64()` returns the payload as a base 64 string. -- `verify_with_crypto(crypto, others)` and - `verify_with_public_key(public_pem, others)` return True if the signature is - valid. Pass an empty list for `others` when the OWID was signed on its own. -- `signature_status(public_pem, others)` answers the same question with a +- `verify_with_crypto(crypto)` and `verify_with_public_key(public_pem)` return + True if the signature is valid. +- `signature_status(public_pem)` answers the same question with a `SignatureStatus`, which keeps a signature that does not match apart from a check that could not be made at all. - `age_minutes()` returns the whole minutes elapsed since creation. @@ -456,8 +443,7 @@ opaque crypto error. - `Creator(domain, crypto)` binds a domain to a signing crypto instance. - `from_configuration(configuration)` builds a creator from a domain and a private key PEM. -- `create(value, others)` creates and signs a new OWID carrying the bytes, - covering any others with the same signature. +- `create(value)` creates and signs a new OWID carrying the bytes. - `create_string(value)` does the same with the UTF-8 bytes of a string. `endpoints` @@ -486,7 +472,7 @@ opaque crypto error. - `await public_key_pem(owid, scheme, transport=None)` returns the key, raising `PublicKeyFetchError`, which carries the status to report, the domain and the response code. -- `await signature_status(owid, scheme, others=None, transport=None)` answers +- `await signature_status(owid, scheme, transport=None)` answers with the status, so a key that could not be fetched is `KEY_UNAVAILABLE`, as is one the creator says was not in force at the identifier's date, one that could not be read is `INVALID_KEY`, and none of these is mistaken for @@ -503,7 +489,7 @@ opaque crypto error. schedule does not reach back that far. - `current()` returns the key in force now, and `last()` the key with the latest start, which for a schedule published ahead of time is usually a - key that has not begun. `signature_status(owid, others=None)` chooses the + key that has not begun. `signature_status(owid)` chooses the key and answers with the status, and `verify` answers True only for `SIGNATURE_VALID`. - `DatedPublicKey(starts_at, public_key_pem)` is one key and the date the key @@ -530,9 +516,7 @@ minutes. The base date is 2020-01-01T00:00:00 UTC. The signature is the 64 byte concatenation of the 32 byte big endian r value and the 32 byte big endian s value (IEEE P1363 format), not the ASN.1 DER form that most libraries produce by default. The data covered by the signature is -this OWID without its signature, followed by the complete bytes, including the -signature, of each other OWID in the order given. The same others in the same -order must be supplied to verify as were supplied to sign. +this OWID without its signature field and nothing else. A single byte with value 0 is the marker for an absent optional OWID inside a larger byte array, written by `Owid.empty_to_buffer` and reported by both reads diff --git a/owid/__init__.py b/owid/__init__.py index 44d7fd2..ba16593 100644 --- a/owid/__init__.py +++ b/owid/__init__.py @@ -22,7 +22,7 @@ creates, signs, serializes, and verifies OWIDs. An OWID is a compact binary structure signed with an ECDSA NIST P-256 key and -a SHA-256 digest. OWIDs chain together to form verifiable trees. +a SHA-256 digest. """ from __future__ import annotations diff --git a/owid/creator.py b/owid/creator.py index f6f4d5d..247fa41 100644 --- a/owid/creator.py +++ b/owid/creator.py @@ -23,7 +23,7 @@ from __future__ import annotations from datetime import datetime, timezone -from typing import Optional, Sequence +from typing import Optional from .crypto import Crypto from .error import OwidError @@ -108,14 +108,8 @@ def crypto(self) -> Crypto: def _sign(self, owid: Owid) -> None: """Signs the OWID provided, setting the domain to the creator domain, the date to the current time, and the version to the current version, - then signing it.""" - self._sign_with_others(owid, []) - - def _sign_with_others(self, owid: Owid, others: Sequence[Owid]) -> None: - """Signs the OWID provided together with the other OWIDs provided. - - The same others, in the same order, must be passed when verifying. - """ + then signing it. The signature covers the OWID's own bytes without + the signature field and nothing else.""" owid._version = DEFAULT_VERSION owid._domain = self._domain # Truncate to whole minutes so the in-memory date matches the value @@ -123,8 +117,7 @@ def _sign_with_others(self, owid: Owid, others: Sequence[Owid]) -> None: # decoded from its bytes, and ensures signing and verification operate # on the same minute precise value. owid._date = datetime.now(timezone.utc).replace(second=0, microsecond=0) - data = owid.data_for_crypto(others) - owid._signature = self._crypto.sign_byte_array(data) + owid._signature = self._crypto.sign_byte_array(owid.signed_bytes()) if len(owid._signature) != SIGNATURE_LENGTH: raise OwidError( "signature length '{0}' not compatible with '{1}' OWID " @@ -137,7 +130,7 @@ def create_string(self, value: str) -> Owid: raise OwidError("a payload is required") return self.create(value.encode("utf-8")) - def create(self, value: bytes, others: Sequence[Owid] = ()) -> Owid: + def create(self, value: bytes) -> Owid: """Creates and signs a new OWID carrying the bytes as its payload. This is one of only two ways an OWID reaches calling code, the other @@ -145,13 +138,9 @@ def create(self, value: bytes, others: Sequence[Owid] = ()) -> Owid: the date and the signature; a caller supplies the payload and nothing else, so there is no moment at which a partly built OWID exists for anyone to hold or pass on. - - Any others are covered by the signature so that a tree can be verified - as a whole, and the same others in the same order must be passed when - verifying. """ if value is None: raise OwidError("a payload is required") owid = Owid._create(payload=bytes(value)) - self._sign_with_others(owid, list(others)) + self._sign(owid) return owid diff --git a/owid/owid.py b/owid/owid.py index 1126986..e9ba4b5 100644 --- a/owid/owid.py +++ b/owid/owid.py @@ -15,17 +15,16 @@ # **************************************************************************** """The OWID structure and the reading, writing, and verification it provides. -An OWID records that the processor operating a domain handled the payload, and -any other OWIDs covered by the signature, at the date and time given. Once -signed it is immutable. Any change to the fields will cause verification to -fail. +An OWID records that the processor operating a domain handled the payload at +the date and time given. Once signed it is immutable. Any change to the fields +will cause verification to fail. """ from __future__ import annotations import base64 from datetime import datetime, timezone -from typing import TYPE_CHECKING, Optional, Sequence +from typing import TYPE_CHECKING, Optional from . import io from .crypto import Crypto @@ -117,7 +116,8 @@ def payload(self) -> bytes: @property def signature(self) -> bytes: - """Signature for this OWID and any others provided when signing.""" + """Signature over the fields of this OWID without the signature + field.""" return self._signature @classmethod @@ -242,16 +242,11 @@ def _to_buffer_no_signature(self, buffer: bytearray) -> None: io.write_date(buffer, self.date, self.version) io.write_byte_array(buffer, self.payload) - def data_for_crypto(self, others: Sequence["Owid"]) -> bytes: - """Builds the byte array used for signing and verification. - - Contains the fields of this OWID without the signature, followed by - the complete byte form of each of the others in the order provided. - """ + def signed_bytes(self) -> bytes: + """The bytes the signature covers, being the fields of this OWID + without the signature field and nothing else.""" buffer = bytearray() self._to_buffer_no_signature(buffer) - for other in others: - other.to_buffer(buffer) return bytes(buffer) def payload_as_string(self) -> str: @@ -273,30 +268,17 @@ def age_minutes(self) -> int: delta = datetime.now(timezone.utc) - self.date return int(delta.total_seconds() // 60) - def verify_with_crypto( - self, crypto: Crypto, others: Optional[Sequence["Owid"]] = None - ) -> bool: - """Verifies this OWID, and any others that were included when it was - signed, using the crypto instance provided. + def verify_with_crypto(self, crypto: Crypto) -> bool: + """Verifies this OWID using the crypto instance provided.""" + return crypto.verify_byte_array(self.signed_bytes(), self.signature) - Pass an empty sequence for others when the OWID was signed on its own. - """ - if others is None: - others = [] - data = self.data_for_crypto(others) - return crypto.verify_byte_array(data, self.signature) - - def verify_with_public_key( - self, public_pem: str, others: Optional[Sequence["Owid"]] = None - ) -> bool: - """Verifies this OWID, and any others that were included when it was - signed, using the public key in SPKI PEM form provided.""" + def verify_with_public_key(self, public_pem: str) -> bool: + """Verifies this OWID using the public key in SPKI PEM form + provided.""" crypto = Crypto.new_verify_only(public_pem) - return self.verify_with_crypto(crypto, others) + return self.verify_with_crypto(crypto) - def signature_status( - self, public_pem: str, others: Optional[Sequence["Owid"]] = None - ) -> "SignatureStatus": + def signature_status(self, public_pem: str) -> "SignatureStatus": """Says whether the signature is genuine, or why that could not be decided. @@ -320,8 +302,7 @@ def signature_status( # The key is the thing at fault, not the identifier. return SignatureStatus.INVALID_KEY try: - data = self.data_for_crypto(others if others is not None else []) - matched = crypto.verify_byte_array(data, self._signature) + matched = crypto.verify_byte_array(self.signed_bytes(), self._signature) except Exception: # The provider failed on inputs that were themselves fine. return SignatureStatus.VERIFICATION_ERROR diff --git a/owid/public_key_fetch.py b/owid/public_key_fetch.py index 6be3b00..f3d0485 100644 --- a/owid/public_key_fetch.py +++ b/owid/public_key_fetch.py @@ -68,7 +68,7 @@ import urllib.parse import urllib.request from datetime import datetime, timezone -from typing import Awaitable, Callable, Dict, List, Optional, Sequence, Tuple +from typing import Awaitable, Callable, Dict, List, Optional, Tuple from . import endpoints, io from .endpoints import validate_public_key_answer @@ -262,39 +262,35 @@ async def public_key_pem( async def signature_status( owid: Owid, scheme: str, - others: Optional[Sequence[Owid]] = None, transport: Optional[Transport] = None, ) -> SignatureStatus: """Says whether the signature on the OWID is genuine, fetching the key that was in force when the OWID was signed from the creator domain. - A key that cannot be fetched is SignatureStatus.KEY_UNAVAILABLE and one - that arrives in a form this package cannot read is - SignatureStatus.INVALID_KEY. Neither is SignatureStatus.SIGNATURE_INVALID, - because an outage or a badly served key leaves the signature unjudged, and - reporting either as invalid would read as an attack. - - Pass the other OWIDs that were signed together with this one, in the same - order as when signed, or nothing when it was signed on its own. + A key that cannot be fetched is SignatureStatus.KEY_UNAVAILABLE, as is + one the creator says was not in force at the OWID's date, and one that + arrives in a form this package cannot read is SignatureStatus.INVALID_KEY. + None of these is SignatureStatus.SIGNATURE_INVALID, because an outage or a + badly served key leaves the signature unjudged, and reporting either as + invalid would read as an attack. """ try: url = public_key_url(owid, scheme) except OwidError: return SignatureStatus.KEY_UNAVAILABLE - return await _signature_status_at_url(owid, url, others, transport) + return await _signature_status_at_url(owid, url, transport) async def verify( owid: Owid, scheme: str, - others: Optional[Sequence[Owid]] = None, transport: Optional[Transport] = None, ) -> bool: """Returns True only when the signature verifies under the key the creator served for the date the OWID carries. Every other outcome, a signature that does not match included, is False, so ask signature_status where the difference changes what the caller does.""" - status = await signature_status(owid, scheme, others, transport) + status = await signature_status(owid, scheme, transport) return status is SignatureStatus.SIGNATURE_VALID @@ -321,7 +317,6 @@ def _cached_key_count() -> int: async def _signature_status_at_url( owid: Owid, url: str, - others: Optional[Sequence[Owid]] = None, transport: Optional[Transport] = None, ) -> SignatureStatus: """The work signature_status does once the URL is known, kept apart so @@ -341,13 +336,13 @@ async def _signature_status_at_url( return failed.status except OwidError: return SignatureStatus.KEY_UNAVAILABLE - status = owid.signature_status(answer.pem, others) + status = owid.signature_status(answer.pem) if status is not SignatureStatus.SIGNATURE_INVALID: return status minute = io.minutes_since_base(owid.date) if minute < 0: return status - if await _neighbour_verifies(owid, minute, url, answer, others, transport): + if await _neighbour_verifies(owid, minute, url, answer, transport): return SignatureStatus.SIGNATURE_VALID if answer.known and not answer.covers(minute): return SignatureStatus.KEY_UNAVAILABLE @@ -359,7 +354,6 @@ async def _neighbour_verifies( minute: int, url: str, tried: _KeyAnswer, - others: Optional[Sequence[Owid]], transport: Optional[Transport], ) -> bool: """Whether a key neighbouring the one the OWID's own minute selected @@ -399,7 +393,7 @@ async def _neighbour_verifies( continue if neighbour.pem == tried.pem: continue - if owid.signature_status(neighbour.pem, others) is SignatureStatus.SIGNATURE_VALID: + if owid.signature_status(neighbour.pem) is SignatureStatus.SIGNATURE_VALID: return True return False diff --git a/owid/public_key_schedule.py b/owid/public_key_schedule.py index ae7ec10..ee550bb 100644 --- a/owid/public_key_schedule.py +++ b/owid/public_key_schedule.py @@ -37,7 +37,7 @@ from __future__ import annotations from datetime import datetime, timezone -from typing import Iterable, Optional, Sequence, Tuple +from typing import Iterable, Optional, Tuple from .error import OwidError from .owid import Owid @@ -182,9 +182,7 @@ def key_for(self, owid: Optional[Owid]) -> Optional[DatedPublicKey]: return None return self.key_in_force(owid.date) - def signature_status( - self, owid: Optional[Owid], others: Optional[Sequence[Owid]] = None - ) -> SignatureStatus: + def signature_status(self, owid: Optional[Owid]) -> SignatureStatus: """Says whether the signature on the OWID is genuine, using the key that was in force when the OWID was signed. The answer is SignatureStatus.KEY_UNAVAILABLE where the schedule holds no key for @@ -194,12 +192,10 @@ def signature_status( key = self.key_for(owid) if key is None: return SignatureStatus.KEY_UNAVAILABLE - return owid.signature_status(key.public_key_pem, others) + return owid.signature_status(key.public_key_pem) - def verify( - self, owid: Optional[Owid], others: Optional[Sequence[Owid]] = None - ) -> bool: + def verify(self, owid: Optional[Owid]) -> bool: """Returns True only when the signature verifies under the key in force when the OWID was signed.""" - status = self.signature_status(owid, others) + status = self.signature_status(owid) return status is SignatureStatus.SIGNATURE_VALID diff --git a/tests/fixtures.py b/tests/fixtures.py index e122127..40ced0c 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -53,7 +53,7 @@ UTF8_PAYLOAD = "Zürich ❤ OWID £€" # Cross language signed fixtures. Each entry holds the SPKI public key and the -# four signed OWIDs to verify. +# two signed OWIDs to verify. GO = { "domain": "go.swan-demo.uk", @@ -72,14 +72,6 @@ "gUGt84xC9HxHmHXDt+wcB0Y9a6E+Txm2F147Qacbp0CtrF8x7QCWZfkcKCKNGSM8hYZE" "fYjJtViG+tA+" ), - "chain_party": ( - "A2dvLnN3YW4tZGVtby51awA/vTMABQAAAHBhcnR5l7NyNmFw2lxqc4DKJWoq0UVd5ujG" - "V/+fvVxqYTRlwCFxaSuwvnhLQQHjX5spxWb4O08IeuiuGCat1WFB/Wqlyw==" - ), - "chain_root": ( - "A2dvLnN3YW4tZGVtby51awA/vTMABAAAAHJvb3R/bEqzG8gAy9yTF1UMEtOlYXBBmn3a" - "20jxXq5NmxIC8iuZvduOXKMf+K8VoAapkWwfpoDKQHS09IhljasZqC0k" - ), } DOTNET = { @@ -99,14 +91,6 @@ "VuaeaDUej0sF+cHfYj/icDBmlBLOviC6ZE28am8EtY+IGuesFcg2rKMybcsAxMmnrDtF" "2xsk1cJvHgoIYpSJJQ==" ), - "chain_party": ( - "A2RvdG5ldC5zd2FuLWRlbW8udWsAPb0zAAUAAABwYXJ0eXtD6H4R7GbvRyFU+bCKgjMA" - "ZFFm8KHln80XPwQOBb/Ub9EZfE4Ml3ueRkKX51+MD98RFgTSmjbqrAnzFkLlilA=" - ), - "chain_root": ( - "A2RvdG5ldC5zd2FuLWRlbW8udWsAPb0zAAQAAAByb290fErj2LccPYCduWUW8vY2aBjr" - "ecDfnTpVpv3+SESJMFW5pcuPKEQik2rC0fWEoB5Vr6e0k5inrhUGiF2c2Y2YDw==" - ), } RUST = { @@ -126,14 +110,6 @@ "nDds+W587AzXpBb94gmLOloeBJTlHnjCkez4Dz2yAPtjcoQ6M/ZUWDIobtJHE5n9a81p" "Tsn/Kvi74Azzx4s=" ), - "chain_party": ( - "A3J1c3Quc3dhbi1kZW1vLnVrAD69MwAFAAAAcGFydHmJ7qaxWgIZUHmGOQb2xC+RuZNw" - "rkMmo1SA9/MfI4SoEpRYdnteXAKUQXxTOK3lmQ3Qz3UwBB6gBb3Q8hi1Wx0R" - ), - "chain_root": ( - "A3J1c3Quc3dhbi1kZW1vLnVrAD69MwAEAAAAcm9vdFd0+QLaBLGPyBrQO+VNunBIQZzw" - "8/lhEiDOKTx36Dc93A0n0fzPDMt/C+BdWMqhnL4nVvyurb3IHR7DUAmgmO0=" - ), } ALL_LANGUAGES = [("go", GO), ("dotnet", DOTNET), ("rust", RUST)] diff --git a/tests/test_creator.py b/tests/test_creator.py index 1c758ad..7823d28 100644 --- a/tests/test_creator.py +++ b/tests/test_creator.py @@ -30,7 +30,7 @@ def test_sign_sets_domain_date_and_version(self) -> None: self.assertEqual(owid.domain, "example.com") self.assertEqual(owid.version, Version.VERSION3) self.assertEqual(len(owid.signature), 64) - self.assertTrue(owid.verify_with_crypto(crypto, [])) + self.assertTrue(owid.verify_with_crypto(crypto)) def test_sign_string_and_sign_bytes_match(self) -> None: crypto = Crypto.new() @@ -63,7 +63,7 @@ def test_from_configuration(self) -> None: self.assertEqual(creator.domain, "example.com") owid = creator.create_string("Hello World") # The OWID must verify with the public key from the original crypto. - self.assertTrue(owid.verify_with_public_key(crypto.public_key_pem(), [])) + self.assertTrue(owid.verify_with_public_key(crypto.public_key_pem())) def test_from_configuration_empty_key_raises(self) -> None: configuration = Configuration(domain="example.com", private_key="") diff --git a/tests/test_domain_length.py b/tests/test_domain_length.py index 0569414..6a4102f 100644 --- a/tests/test_domain_length.py +++ b/tests/test_domain_length.py @@ -186,7 +186,7 @@ def test_library_output_parses(self) -> None: self.assertEqual(parsed.domain, MAXIMUM_DOMAIN) self.assertEqual(parsed, original) - self.assertTrue(parsed.verify_with_crypto(crypto, [])) + self.assertTrue(parsed.verify_with_crypto(crypto)) class CountingCrypto: diff --git a/tests/test_owid.py b/tests/test_owid.py index 2db85c6..c264dd5 100644 --- a/tests/test_owid.py +++ b/tests/test_owid.py @@ -81,9 +81,8 @@ def test_bad_vector_parses_only(self) -> None: class CrossLanguageFixtureTests(unittest.TestCase): """Real signatures produced by other implementations must verify. - For each language the simple and utf8 OWIDs verify on their own, the chain - root verifies on its own, and the chain party verifies only when the chain - root is supplied as the single other. A tampered copy must fail. + For each language the simple and utf8 OWIDs verify, and a tampered copy + must fail. """ def test_simple_and_utf8_verify(self) -> None: @@ -92,7 +91,7 @@ def test_simple_and_utf8_verify(self) -> None: with self.subTest(language=name, fixture="simple"): simple = _parsed(Owid.parse(data["simple"])) self.assertEqual(simple.payload_as_string(), "example") - self.assertTrue(simple.verify_with_public_key(spki, [])) + self.assertTrue(simple.verify_with_public_key(spki)) with self.subTest(language=name, fixture="utf8"): utf8 = _parsed(Owid.parse(data["utf8"])) self.assertEqual( @@ -100,52 +99,20 @@ def test_simple_and_utf8_verify(self) -> None: fixtures.UTF8_PAYLOAD, "the UTF-8 payload must decode to the expected text", ) - self.assertTrue(utf8.verify_with_public_key(spki, [])) - - def test_chain_verifies_with_root_as_other(self) -> None: - for name, data in fixtures.ALL_LANGUAGES: - spki = data["spki"] - root = _parsed(Owid.parse(data["chain_root"])) - party = _parsed(Owid.parse(data["chain_party"])) - with self.subTest(language=name): - self.assertEqual(root.payload_as_string(), "root") - self.assertEqual(party.payload_as_string(), "party") - # Root verifies on its own. - self.assertTrue(root.verify_with_public_key(spki, [])) - # Party verifies only with the root supplied as the other. - self.assertTrue(party.verify_with_public_key(spki, [root])) - - def test_chain_party_fails_without_others(self) -> None: - for name, data in fixtures.ALL_LANGUAGES: - spki = data["spki"] - party = _parsed(Owid.parse(data["chain_party"])) - with self.subTest(language=name): - self.assertFalse( - party.verify_with_public_key(spki, []), - "the chain party must not verify without the root", - ) + self.assertTrue(utf8.verify_with_public_key(spki)) def test_tampered_fixtures_fail(self) -> None: for name, data in fixtures.ALL_LANGUAGES: spki = data["spki"] - root = _parsed(Owid.parse(data["chain_root"])) - for fixture in ("simple", "utf8", "chain_root"): + for fixture in ("simple", "utf8"): with self.subTest(language=name, fixture=fixture): tampered = _parsed(Owid.parse( fixtures.flip_last_byte(data[fixture])) ) self.assertFalse( - tampered.verify_with_public_key(spki, []), + tampered.verify_with_public_key(spki), "a flipped signature byte must fail to verify", ) - with self.subTest(language=name, fixture="chain_party"): - tampered_party = _parsed(Owid.parse( - fixtures.flip_last_byte(data["chain_party"])) - ) - self.assertFalse( - tampered_party.verify_with_public_key(spki, [root]), - "a flipped signature byte must fail to verify", - ) class SignAndSelfVerifyTests(unittest.TestCase): @@ -158,12 +125,12 @@ def test_sign_and_self_verify(self) -> None: creator = Creator("example.com", crypto) owid = creator.create_string("Hello World") # Verifies with the crypto instance and with the exported public key. - self.assertTrue(owid.verify_with_crypto(crypto, [])) - self.assertTrue(owid.verify_with_public_key(crypto.public_key_pem(), [])) + self.assertTrue(owid.verify_with_crypto(crypto)) + self.assertTrue(owid.verify_with_public_key(crypto.public_key_pem())) # A copy decoded from base 64 also verifies. copy = _parsed(Owid.parse(owid.as_base64())) self.assertEqual(copy, owid) - self.assertTrue(copy.verify_with_crypto(crypto, [])) + self.assertTrue(copy.verify_with_crypto(crypto)) def test_tampered_self_signed_fails(self) -> None: from owid import Creator @@ -181,21 +148,7 @@ def test_tampered_self_signed_fails(self) -> None: raw[at] ^= 0xFF result = Owid.parse_bytes(bytes(raw)) self.assertTrue(result.ok, result.status) - self.assertFalse(result.owid.verify_with_crypto(crypto, [])) - - def test_sign_with_others_round_trip(self) -> None: - from owid import Creator - - crypto = Crypto.new() - creator = Creator("example.com", crypto) - root = creator.create_string("root") - # Created with the root as the other it is signed alongside, rather - # than assembled and then signed, because a caller no longer makes an - # OWID and hands it over to have a signature put on it. - party = creator.create(b"party", [root]) - # Party verifies with the root as the single other, and fails alone. - self.assertTrue(party.verify_with_crypto(crypto, [root])) - self.assertFalse(party.verify_with_crypto(crypto, [])) + self.assertFalse(result.owid.verify_with_crypto(crypto)) class PayloadAndSerializationTests(unittest.TestCase): diff --git a/tests/test_parse_contract.py b/tests/test_parse_contract.py index 76753cc..8d45add 100644 --- a/tests/test_parse_contract.py +++ b/tests/test_parse_contract.py @@ -158,7 +158,7 @@ def test_structurally_valid_but_unsigned_parses_then_fails(self) -> None: "flipping a signature byte leaves the envelope readable") self.assertEqual(ParseStatus.PARSED, result.status) self.assertFalse( - result.owid.verify_with_crypto(crypto, []), + result.owid.verify_with_crypto(crypto), "and the signature is then found not to match") def test_no_verification_happens_during_a_failed_parse(self) -> None: @@ -281,7 +281,7 @@ def test_valid(self) -> None: owid = Creator("example.com", crypto).create(b"abc") self.assertEqual( SignatureStatus.SIGNATURE_VALID, - owid.signature_status(crypto.public_key_pem(), [])) + owid.signature_status(crypto.public_key_pem())) def test_invalid_only_when_it_really_does_not_match(self) -> None: crypto = Crypto.new() @@ -291,13 +291,13 @@ def test_invalid_only_when_it_really_does_not_match(self) -> None: tampered = Owid.parse_bytes(bytes(raw)).owid self.assertEqual( SignatureStatus.SIGNATURE_INVALID, - tampered.signature_status(crypto.public_key_pem(), [])) + tampered.signature_status(crypto.public_key_pem())) def test_no_key_is_not_a_forgery(self) -> None: crypto = Crypto.new() owid = Creator("example.com", crypto).create(b"abc") self.assertEqual( - SignatureStatus.KEY_UNAVAILABLE, owid.signature_status("", [])) + SignatureStatus.KEY_UNAVAILABLE, owid.signature_status("")) def test_unreadable_key_is_not_a_forgery(self) -> None: """This is the case that happened. The key endpoints served PEM a @@ -309,7 +309,7 @@ def test_unreadable_key_is_not_a_forgery(self) -> None: SignatureStatus.INVALID_KEY, owid.signature_status( "-----BEGIN PUBLIC KEY-----\nnot base 64\n" - "-----END PUBLIC KEY-----", [])) + "-----END PUBLIC KEY-----")) def test_remaining_members_are_unreachable_here(self) -> None: """INVALID_SIGNATURE_LENGTH cannot be reached from a parsed OWID, diff --git a/tests/test_payload_length.py b/tests/test_payload_length.py index 576b5ef..5ed813f 100644 --- a/tests/test_payload_length.py +++ b/tests/test_payload_length.py @@ -84,7 +84,7 @@ def test_library_output_parses(self) -> None: parsed = Owid._from_byte_array_or_raise(original.as_byte_array()) self.assertEqual(parsed.payload, PAYLOAD) self.assertEqual(parsed, original) - self.assertTrue(parsed.verify_with_crypto(crypto, [])) + self.assertTrue(parsed.verify_with_crypto(crypto)) def test_declared_length_off_by_one_is_refused(self) -> None: """One more or one fewer than the bytes present is refused, because diff --git a/tests/test_public_key_fetch.py b/tests/test_public_key_fetch.py index 7a844a8..c176ad1 100644 --- a/tests/test_public_key_fetch.py +++ b/tests/test_public_key_fetch.py @@ -59,9 +59,6 @@ from tests import key_fixtures from tests.key_end_point import Answer, KeyEndPoint -#: No other OWIDs were covered by the signature on the fixture. -ALONE: List[Owid] = [] - #: The date the fixture identifier carries, which the cloud trims to the day. IDENTIFIER_DATE = datetime(2026, 9, 4, tzinfo=timezone.utc) @@ -189,7 +186,7 @@ async def test_dated_fetch_verifies_an_identifier_from_an_earlier_key_week( self.assertIs( SignatureStatus.SIGNATURE_VALID, await public_key_fetch._signature_status_at_url( - owid, end_point.url_for(owid), ALONE + owid, end_point.url_for(owid) ), "should verify against the key in force when it was signed", ) @@ -216,7 +213,7 @@ async def test_undated_fetch_leaves_an_earlier_weeks_identifier_unverified( self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch._signature_status_at_url( - owid, undated, ALONE + owid, undated ), "an undated request gets the key in force at the request, which " "the creator says was not in force when the identifier was signed", @@ -239,7 +236,7 @@ async def test_a_key_the_end_point_cannot_serve_is_key_unavailable( ) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - await public_key_fetch._signature_status_at_url(owid, url, ALONE), + await public_key_fetch._signature_status_at_url(owid, url), "no key means the signature was never examined", ) @@ -281,7 +278,7 @@ async def test_an_end_point_that_cannot_be_reached_is_key_unavailable( await asyncio.to_thread(end_point.stop) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - await public_key_fetch._signature_status_at_url(owid, url, ALONE), + await public_key_fetch._signature_status_at_url(owid, url), "a connection that is refused leaves the signature unjudged", ) @@ -303,7 +300,7 @@ async def test_a_redirect_is_not_followed(self) -> None: self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch._signature_status_at_url( - owid, end_point.url_for(owid), ALONE + owid, end_point.url_for(owid) ), ) @@ -315,7 +312,7 @@ async def test_a_key_that_cannot_be_read_is_invalid_key(self) -> None: self.assertIs( SignatureStatus.INVALID_KEY, await public_key_fetch._signature_status_at_url( - owid, end_point.url_for(owid), ALONE + owid, end_point.url_for(owid) ), ) @@ -600,7 +597,7 @@ def _signed_at(domain: str, moment: datetime, crypto: Crypto) -> Owid: owid = Owid._create( version=Version.VERSION3, domain=domain, date=moment, payload=b"payload" ) - owid._signature = crypto.sign_byte_array(owid.data_for_crypto([])) + owid._signature = crypto.sign_byte_array(owid.signed_bytes()) return owid async def test_a_signature_failing_near_the_edge_of_a_span_is_checked_against_the_neighbour( @@ -635,7 +632,7 @@ async def creator(url: str, timeout: float) -> Tuple[int, bytes]: async def status_of(owid: Owid) -> SignatureStatus: return await public_key_fetch._signature_status_at_url( - owid, url.format(io.minutes_since_base(owid.date)), None, creator + owid, url.format(io.minutes_since_base(owid.date)), creator ) late = self._signed_at("creator.test", rotation + timedelta(minutes=5), first) @@ -680,7 +677,7 @@ async def creator(url: str, timeout: float) -> Tuple[int, bytes]: late = self._signed_at("creator.test", rotation + timedelta(minutes=5), first) self.assertIs( SignatureStatus.SIGNATURE_VALID, - await public_key_fetch.signature_status(late, "https", ALONE, creator), + await public_key_fetch.signature_status(late, "https", creator), ) minute = io.minutes_since_base(rotation) self.assertEqual( @@ -711,7 +708,7 @@ async def creator(url: str, timeout: float) -> Tuple[int, bytes]: live = self._signed_at("creator.test", rotation + timedelta(minutes=2), first) self.assertIs( SignatureStatus.SIGNATURE_VALID, - await public_key_fetch.signature_status(live, "https", ALONE, creator), + await public_key_fetch.signature_status(live, "https", creator), "a live identifier signed with the key before the current one verifies", ) self.assertEqual( @@ -738,17 +735,17 @@ async def creator(url: str, timeout: float) -> Tuple[int, bytes]: earlier = self._signed_at("creator.test", rotation - timedelta(days=3), first) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - await public_key_fetch.signature_status(earlier, "https", ALONE, creator), + await public_key_fetch.signature_status(earlier, "https", creator), "the key answered with was not in force at the identifier's date", ) self.assertFalse( - await public_key_fetch.verify(earlier, "https", ALONE, creator), + await public_key_fetch.verify(earlier, "https", creator), "the boolean form answers True for a genuine signature alone", ) forged = self._signed_at("creator.test", rotation + timedelta(days=3), stranger) self.assertIs( SignatureStatus.SIGNATURE_INVALID, - await public_key_fetch.signature_status(forged, "https", ALONE, creator), + await public_key_fetch.signature_status(forged, "https", creator), "a signature failing under the key in force at its date does not match", ) @@ -773,7 +770,7 @@ async def contradictory(url: str, timeout: float) -> Tuple[int, bytes]: self.assertIs( SignatureStatus.INVALID_KEY, await public_key_fetch._signature_status_at_url( - owid, end_point.url_for(owid), None, contradictory + owid, end_point.url_for(owid), contradictory ), ) @@ -795,7 +792,7 @@ async def creator(url: str, timeout: float) -> Tuple[int, bytes]: self.assertIs( SignatureStatus.INVALID_KEY, - await public_key_fetch.signature_status(owid, "https", ALONE, creator), + await public_key_fetch.signature_status(owid, "https", creator), ) self.assertEqual(["spki"], formats, "the request asks for spki by name") @@ -806,7 +803,7 @@ async def unstated(url: str, timeout: float) -> Tuple[int, bytes]: self.assertIs( SignatureStatus.SIGNATURE_VALID, - await public_key_fetch.signature_status(owid, "https", ALONE, unstated), + await public_key_fetch.signature_status(owid, "https", unstated), ) def test_many_threads_verifying_one_owid_together_make_one_request(self) -> None: @@ -835,7 +832,7 @@ def verify() -> None: started.wait() statuses.append( asyncio.run( - public_key_fetch._signature_status_at_url(owid, url, None, transport) + public_key_fetch._signature_status_at_url(owid, url, transport) ) ) @@ -889,7 +886,7 @@ async def held_open(url: str, timeout: float) -> Tuple[int, bytes]: "one request for the URL the package builds, shared by both", ) self.assertTrue( - await public_key_fetch.verify(owid, "https", ALONE, held_open), + await public_key_fetch.verify(owid, "https", held_open), "a later caller is answered from the cache", ) self.assertEqual(1, len(calls), "the cache answered the third caller") @@ -934,12 +931,12 @@ async def held_then_refused( first = asyncio.ensure_future( public_key_fetch.signature_status( - owid, "https", ALONE, held_then_refused + owid, "https", held_then_refused ) ) second = asyncio.ensure_future( public_key_fetch.signature_status( - owid, "https", ALONE, held_then_refused + owid, "https", held_then_refused ) ) while not calls: @@ -953,7 +950,7 @@ async def held_then_refused( self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch.signature_status( - owid, "https", ALONE, held_then_refused + owid, "https", held_then_refused ), ) self.assertEqual( @@ -1010,7 +1007,7 @@ async def no_request(url: str, timeout: float) -> Tuple[int, bytes]: self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch.signature_status( - owid, "https", ALONE, no_request + owid, "https", no_request ), ) @@ -1025,7 +1022,7 @@ async def test_a_scheme_that_does_not_make_an_http_request_is_refused( self.assertTrue(url.startswith("file:")) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, - await public_key_fetch._signature_status_at_url(owid, url, ALONE), + await public_key_fetch._signature_status_at_url(owid, url), ) with self.assertRaises(PublicKeyFetchError) as refused: await public_key_fetch._public_key_pem_at_url(url, owid.domain) @@ -1062,11 +1059,11 @@ async def transport(url: str, timeout: float) -> Tuple[int, bytes]: self.assertIs( SignatureStatus.SIGNATURE_VALID, await public_key_fetch.signature_status( - owid, "https", ALONE, transport + owid, "https", transport ), ) self.assertTrue( - await public_key_fetch.verify(owid, "https", ALONE, transport) + await public_key_fetch.verify(owid, "https", transport) ) self.assertEqual( [ @@ -1092,7 +1089,7 @@ async def wrong_week(url: str, timeout: float) -> Tuple[int, bytes]: return 200, endpoints.public_key_answer(following.public_key_pem, None, None, None).encode("utf-8") self.assertFalse( - await public_key_fetch.verify(owid, "https", ALONE, wrong_week), + await public_key_fetch.verify(owid, "https", wrong_week), "the following week's key did not sign the identifier", ) @@ -1108,13 +1105,13 @@ async def unusable(url: str, timeout: float) -> Tuple[int, bytes]: self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch.signature_status( - owid, "https", ALONE, unreachable + owid, "https", unreachable ), ) self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch.signature_status( - owid, "https", ALONE, unusable + owid, "https", unusable ), ) @@ -1135,7 +1132,7 @@ async def oversized(url: str, timeout: float) -> Tuple[int, bytes]: self.assertIs( SignatureStatus.KEY_UNAVAILABLE, await public_key_fetch.signature_status( - owid, "https", ALONE, oversized + owid, "https", oversized ), ) diff --git a/tests/test_public_key_schedule.py b/tests/test_public_key_schedule.py index 6178bfb..76743dd 100644 --- a/tests/test_public_key_schedule.py +++ b/tests/test_public_key_schedule.py @@ -28,12 +28,10 @@ import unittest from datetime import datetime, timedelta, timezone -from typing import List from owid import ( Crypto, DatedPublicKey, - Owid, OwidError, PublicKeySchedule, SignatureStatus, @@ -41,9 +39,6 @@ from tests import key_fixtures -#: No other OWIDs were covered by the signature on the fixture. -ALONE: List[Owid] = [] - def fresh_pem() -> str: return Crypto.new().public_key_pem() @@ -66,7 +61,7 @@ def test_genuine_identifier_verifies_against_the_key_in_force_on_its_date( self.assertEqual(1, len(week), "the schedule holds that week once") self.assertIs( SignatureStatus.SIGNATURE_VALID, - owid.signature_status(week[0].pem, ALONE), + owid.signature_status(week[0].pem), ) def test_schedule_verifies_the_genuine_identifier(self) -> None: @@ -76,9 +71,9 @@ def test_schedule_verifies_the_genuine_identifier(self) -> None: self.assertIsNotNone(chosen) self.assertEqual(key_fixtures.WEEK_OF_THE_IDENTIFIER, chosen.starts_at) self.assertIs( - SignatureStatus.SIGNATURE_VALID, schedule.signature_status(owid, ALONE) + SignatureStatus.SIGNATURE_VALID, schedule.signature_status(owid) ) - self.assertTrue(schedule.verify(owid, ALONE)) + self.assertTrue(schedule.verify(owid)) def test_a_later_weeks_key_does_not_verify_an_earlier_weeks_identifier( self, @@ -94,7 +89,7 @@ def test_a_later_weeks_key_does_not_verify_an_earlier_weeks_identifier( self.assertGreater(following.starts_at, owid.date) self.assertIs( SignatureStatus.SIGNATURE_INVALID, - owid.signature_status(following.public_key_pem, ALONE), + owid.signature_status(following.public_key_pem), ) def test_the_last_key_is_not_the_key_in_force(self) -> None: @@ -173,7 +168,7 @@ def test_selection_ignores_the_moment_the_keys_were_generated(self) -> None: ) self.assertIs( SignatureStatus.SIGNATURE_INVALID, - owid.signature_status(newest_generated.pem, ALONE), + owid.signature_status(newest_generated.pem), "selecting on the generation moment reports a genuine identifier " "as not matching", ) @@ -186,7 +181,7 @@ def test_selection_ignores_the_moment_the_keys_were_generated(self) -> None: ) self.assertIs( SignatureStatus.SIGNATURE_VALID, - owid.signature_status(chosen.public_key_pem, ALONE), + owid.signature_status(chosen.public_key_pem), "selecting on the start verifies the genuine identifier", )