From d7537ae11735a04ec6938f26f25c5e3a10c5dd42 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 31 Aug 2026 09:42:34 +0100 Subject: [PATCH 1/6] Point the OWID submodule at the hardened parse branch The 51Degrees owid-python fork now carries the harden/parse-without-throwing branch at f22ac41, the tip of SWAN-community/owid-python pull request 2. The pin is temporary and moves to the merged commit on main once that pull request lands. --- owid-python | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/owid-python b/owid-python index 9f773d6..f22ac41 160000 --- a/owid-python +++ b/owid-python @@ -1 +1 @@ -Subproject commit 9f773d6721201f1c48dbac1e808676206aa11ae2 +Subproject commit f22ac4116141d0cbf4a6068b4ca32e64fa04b64b From 215ca90153947d1d1374b76e1836ac0cf701e2ee Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 31 Aug 2026 09:42:35 +0100 Subject: [PATCH 2/6] Harden 51Did parsing to answer with a reason instead of throwing The hardened OWID library no longer offers a throwing parse or a public constructor, so FodId now reads through Owid.parse and Owid.parse_bytes. FodId.try_from_base64 and FodId.try_from_byte_array read external data without raising and answer with a FodIdParseResult carrying whether the parse succeeded, the value and a FodIdParseStatus. The status vocabulary is the OWID one, carried through unchanged, plus PayloadTooShort for a payload shorter than the five byte header and InvalidTypePayloadLength for a payload shorter than its type needs. Longer payloads, domains and envelopes are accepted, as the lengths beyond the value belong to the cloud. Parsing never checks the signature, and FodId.signature_status exposes the OWID SignatureStatus for callers who want the reason a verification could not be decided. from_base64, from_byte_array, from_owid and the constructor read through the same logic and keep their exception types. The constructor no longer round trips through the removed Owid.from_byte_array and reads the envelope back through parse_bytes instead. --- .../src/fiftyone_pipeline_did/__init__.py | 22 +- .../src/fiftyone_pipeline_did/did_client.py | 5 +- .../src/fiftyone_pipeline_did/fod_id.py | 399 ++++++++++++++---- 3 files changed, 343 insertions(+), 83 deletions(-) diff --git a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/__init__.py b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/__init__.py index d37b086..8d609a0 100644 --- a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/__init__.py +++ b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/__init__.py @@ -27,8 +27,12 @@ OWID form in either alphabet, exposes the three payload fields (Flags, License Id and the value Hash) and the identifier :class:`~fiftyone_pipeline_did.id_type.IdType`, and delegates OWID-level -concerns to the wrapped envelope. Compare 51Dids by their value (``hash``), -never by their envelopes. +concerns to the wrapped envelope. ``FodId.try_from_base64`` and +``FodId.try_from_byte_array`` read external data without raising and answer +with a :class:`~fiftyone_pipeline_did.fod_id.FodIdParseResult` naming the +:class:`~fiftyone_pipeline_did.fod_id.FodIdParseStatus` either way. Parsing +never checks the signature. Compare 51Dids by their value (``hash``), never +by their envelopes. :class:`~fiftyone_pipeline_did.did_client.DidClient` handles every manipulation of a 51Did a server needs against the 51Degrees cloud: the @@ -54,15 +58,18 @@ ) # The OWID library is carried inside this package as the private module # _owid, because it cannot be installed from a package registry (see the -# package readme). The envelope type and the error type are re-exported here -# so that callers have supported names for the two of them that the public -# API refers to, as the private module itself is not part of that API. -from ._owid import Owid, OwidError -from .fod_id import DATE_EPOCH, FodId +# package readme). The envelope type, the error type and the signature +# status vocabulary are re-exported here so that callers have supported +# names for the OWID types the public API refers to, as the private module +# itself is not part of that API. +from ._owid import Owid, OwidError, SignatureStatus +from .fod_id import DATE_EPOCH, FodId, FodIdParseResult, FodIdParseStatus from .id_type import IdType __all__ = [ "FodId", + "FodIdParseResult", + "FodIdParseStatus", "IdType", "DATE_EPOCH", "DidClient", @@ -79,4 +86,5 @@ "DEFAULT_ENDPOINT", "Owid", "OwidError", + "SignatureStatus", ] diff --git a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py index 996fac3..40bcfed 100644 --- a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py +++ b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py @@ -696,7 +696,10 @@ def _urlopen(open_call: Callable[[], Any]) -> Tuple[int, bytes]: def _as_fod_id(value: Union[FodId, str]) -> FodId: """The identifier as a FodId, parsing a base64 string where one was - given.""" + given. The parse happens here, before any key is fetched, so text that + is not a 51Did raises what :meth:`FodId.from_base64` raises and never + reaches the cloud. A caller who would rather have the reason than an + exception parses with :meth:`FodId.try_from_base64` first.""" if isinstance(value, FodId): return value if isinstance(value, str): diff --git a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/fod_id.py b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/fod_id.py index ded5ad1..57190df 100644 --- a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/fod_id.py +++ b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/fod_id.py @@ -23,8 +23,17 @@ from __future__ import annotations from datetime import datetime, timezone - -from ._owid import Owid, Version +from enum import Enum +from typing import NamedTuple, Optional, Tuple + +from ._owid import ( + Owid, + OwidError, + ParseResult, + ParseStatus, + SignatureStatus, + Version, +) from .id_type import IdType @@ -34,6 +43,96 @@ DATE_EPOCH = datetime(2020, 1, 1, tzinfo=timezone.utc) +class FodIdParseStatus(Enum): + """Why reading a 51Did succeeded or failed. + + The vocabulary is the OWID one, member for member and value for value, + with two members added for the checks this package makes on the payload + once the envelope has been read. A failure in the envelope keeps the + OWID status unchanged, so a caller sees the same reason whichever + language read the bytes, and a failure in the payload names which of the + two 51Did rules was broken. + + Every member other than :attr:`PARSED` is an expected outcome for data + that arrived from outside, not a fault in the program. A parse that + reports :attr:`PARSED` says the bytes have the shape of a 51Did and + nothing about whether the signature is genuine, which is a separate + question answered by :meth:`FodId.verify` or + :meth:`FodId.signature_status`. + """ + + #: The bytes form a structurally valid 51Did. Says nothing about the + #: signature. + PARSED = "Parsed" + #: Nothing was supplied to parse. + MISSING_INPUT = "MissingInput" + #: The input was supplied in a form this surface cannot read. + INVALID_INPUT_TYPE = "InvalidInputType" + #: The string is not valid base 64, so there are no bytes to read. + INVALID_BASE64 = "InvalidBase64" + #: The first byte names an envelope version this package does not know. + UNSUPPORTED_VERSION = "UnsupportedVersion" + #: The data stopped in the middle of an envelope field. + UNEXPECTED_END = "UnexpectedEnd" + #: The creator domain is not terminated, or is longer than the OWID + #: maximum. + INVALID_DOMAIN_ENCODING = "InvalidDomainEncoding" + #: The declared payload byte count disagrees with the bytes present. + BYTE_COUNT_MISMATCH = "ByteCountMismatch" + #: The envelope is consistent but larger than this runtime can hold. + IMPLEMENTATION_CAPACITY_EXCEEDED = "ImplementationCapacityExceeded" + #: The version 0 marker, which stands for an absent envelope and never + #: produces a value. + ABSENT_NODE = "AbsentNode" + #: The envelope is malformed in a way none of the above describes. + MALFORMED_ENVELOPE = "MalformedEnvelope" + + #: The envelope was read but its payload is shorter than the 5 byte + #: header (flags and licence id), so the identifier type cannot even be + #: read. + PAYLOAD_TOO_SHORT = "PayloadTooShort" + #: The header was read and names a type whose value needs more bytes + #: than the payload holds, being 16 GUID bytes after the header for + #: Random and 32 hash bytes for Probabilistic and HashedEmail. + INVALID_TYPE_PAYLOAD_LENGTH = "InvalidTypePayloadLength" + + @classmethod + def of(cls, status: ParseStatus) -> "FodIdParseStatus": + """The member carrying an OWID status, unchanged in name and + value.""" + return cls[status.name] + + +class FodIdParseResult(NamedTuple): + """What a 51Did parse produced, and why. + + Three facts, exactly as the OWID library reports them. Whether the parse + succeeded, the value (which is absent on failure, never a partly read + identifier), and the status, which is :attr:`FodIdParseStatus.PARSED` on + success and the specific reason otherwise. Truthy on success, so + ``if result:`` reads naturally. + + A successful parse says the bytes have the shape of a 51Did. The + signature has not been checked, so the value is not known to be genuine + until :meth:`FodId.verify` or a :class:`~fiftyone_pipeline_did.DidClient` + check says so. + """ + + #: True when the input was a complete, structurally valid 51Did. + ok: bool + #: The identifier on success, otherwise None. + value: Optional["FodId"] + #: PARSED on success, otherwise the specific reason. + status: FodIdParseStatus + + def __bool__(self) -> bool: + return self.ok + + +def _failed(status: FodIdParseStatus) -> FodIdParseResult: + return FodIdParseResult(False, None, status) + + class FodId: """A strongly typed reader for the 51Did (51Degrees Identifier) value returned by the 51Degrees Cloud service. @@ -50,12 +149,21 @@ class FodId: Payload layout. The header (offsets 0-4) is shared by every identifier type; bits 6-7 of Flags select the :class:`IdType` and the length of the value that follows (32-byte SHA-256 for Probabilistic and HashedEmail, or - 16 GUID bytes for Random). + 16 GUID bytes for Random). A payload longer than the header and value is + accepted, because the bytes after the value are a creator context + section whose lengths belong to the cloud, so this package places no + upper bound on a payload or an envelope. + + Reading and verifying are separate steps. :meth:`try_from_base64` and + :meth:`try_from_byte_array` read external data without raising and + answer with a :class:`FodIdParseResult` naming the reason either way, + whilst :meth:`from_base64`, :meth:`from_byte_array` and the constructor + raise for the same inputs. None of them checks the signature, so a + parsed 51Did is not known to be genuine until :meth:`verify` or + :meth:`signature_status` says so. This type **composes** an OWID (holds the wrapped envelope and delegates - OWID-level concerns to it) rather than inheriting from it. Constructing a - ``FodId`` does **not** verify the signature; call :meth:`verify` - explicitly. + OWID-level concerns to it) rather than inheriting from it. """ #: Byte offset of the Flags field within the payload. @@ -81,73 +189,116 @@ def __init__(self, owid: Owid) -> None: """Promotes an already-parsed :class:`~fiftyone_pipeline_did.Owid` into a 51Did by unpacking its payload. - The OWID is **copied** (round-tripped through its byte form), not - aliased, so a ``FodId`` can never desync from its envelope if the caller - later mutates the OWID they passed in. The OWID must therefore be signed - (serializable). + The envelope is written out and read back through this package's + own parser rather than held by reference, so the ``FodId`` owns an + envelope of its own whatever object the caller passed in. - Raises :class:`TypeError` if ``owid`` is ``None``, :class:`ValueError` - if the payload is shorter than the minimum for its identifier type, and - :class:`~fiftyone_pipeline_did.OwidError` if the OWID cannot be - serialized (e.g. it is unsigned). + Raises :class:`TypeError` if ``owid`` is ``None``, + :class:`~fiftyone_pipeline_did.OwidError` if the envelope cannot be + written out and read back, and :class:`ValueError` if the payload is + shorter than the header or than the minimum for its identifier + type. """ if owid is None: raise TypeError("owid must not be None") - self._owid = Owid.from_byte_array(owid.as_byte_array()) - payload = self._owid.payload - if payload is None or len(payload) < self.HEADER_LENGTH: - raise ValueError( - "51Did payload must be at least {0} bytes; got {1}.".format( - self.HEADER_LENGTH, 0 if payload is None else len(payload) - ) - ) - self._flags = payload[self.FLAGS_OFFSET] - # Little-endian uint32, unsigned (Python ints are unbounded and - # non-negative here, so the high bit never becomes negative). - self._license_id = int.from_bytes( - payload[self.LICENSE_ID_OFFSET:self.LICENSE_ID_OFFSET - + self.LICENSE_ID_LENGTH], - byteorder="little", - signed=False, - ) - id_type = IdType.from_flags(self._flags) - if id_type is IdType.RANDOM: - value_length = self.GUID_LENGTH - elif id_type is IdType.RESERVED: - value_length = len(payload) - self.HEADER_LENGTH - else: - value_length = self.HASH_LENGTH - if len(payload) < self.HEADER_LENGTH + value_length: - raise ValueError( - "51Did payload for the {0} type must be at least {1} bytes; " - "got {2}.".format( - id_type.name, self.HEADER_LENGTH + value_length, - len(payload) - ) - ) - # bytes is immutable, so slicing yields a value that cannot be used to - # mutate the underlying payload - no defensive copy is required. - self._hash = bytes( - payload[self.HASH_OFFSET:self.HASH_OFFSET + value_length]) + read = Owid.parse_bytes(owid.as_byte_array()) + if not read.ok: + raise OwidError( + "the envelope could not be read back: {0}".format( + read.status.value)) + self._assign(read.owid, *_unpack_or_raise(read.owid.payload)) + + def _assign(self, owid: Owid, flags: int, license_id: int, + value: bytes) -> None: + self._owid = owid + self._flags = flags + self._license_id = license_id + self._hash = value @classmethod - def from_base64(cls, base64: str) -> "FodId": - """Parses a 51Did from its base64-encoded OWID string in either - alphabet. + def _build(cls, owid: Owid, flags: int, license_id: int, + value: bytes) -> "FodId": + """An identifier over fields :func:`_read_payload` has already + checked, so the constructor's read is not repeated.""" + fod_id = cls.__new__(cls) + fod_id._assign(owid, flags, license_id, value) + return fod_id - The cloud issues a 51Did in the standard alphabet with padding, and a - page puts one in a link in the URL-safe alphabet (``-`` and ``_``) - without padding. Both are accepted here, with or without padding, - by normalising to the standard form before decoding, so a server - never converts an identifier it received from a link. + @classmethod + def _from_read(cls, read: ParseResult) -> FodIdParseResult: + """The non-raising reader over an OWID read. Carries an OWID failure + through unchanged, then applies the two 51Did payload rules, and + builds the identifier only when both have passed.""" + if not read.ok: + return _failed(FodIdParseStatus.of(read.status)) + status, flags, license_id, value = _read_payload(read.owid.payload) + if status is not FodIdParseStatus.PARSED: + return _failed(status) + return FodIdParseResult( + True, cls._build(read.owid, flags, license_id, value), + FodIdParseStatus.PARSED) - Raises :class:`TypeError` if ``base64`` is ``None`` and - :class:`~fiftyone_pipeline_did.OwidError` if it is not valid base64 - or not a valid OWID. + @classmethod + def _from_read_or_raise(cls, read: ParseResult, argument: str) \ + -> "FodId": + """The raising reader over the same OWID read and the same payload + rules, so there is one reading and not two. The exception type + follows the kind of failure, which is what the raising readers have + always done.""" + if not read.ok: + if read.status is ParseStatus.INVALID_INPUT_TYPE: + raise TypeError( + "{0} is not a type this reader accepts".format(argument)) + raise OwidError("{0} is not a valid 51Did: {1}".format( + argument, read.status.value)) + return cls._build(read.owid, *_unpack_or_raise(read.owid.payload)) + + @classmethod + def try_from_base64(cls, value) -> FodIdParseResult: + """Reads a 51Did from its base64 form in either alphabet without + raising. + + The cloud issues a 51Did in the standard alphabet with padding, and + a page puts one in a link in the URL-safe alphabet (``-`` and ``_``) + without padding. Both are accepted, with or without padding, by + normalising to the standard form before the envelope is read. + + The value may be anything at all, as external data is. ``None`` and + the empty string report :attr:`FodIdParseStatus.MISSING_INPUT`, + anything other than a string reports + :attr:`FodIdParseStatus.INVALID_INPUT_TYPE`, and every other failure + names its reason. The signature is not checked. + """ + return cls._from_read(_read_base64(value)) + + @classmethod + def try_from_byte_array(cls, buffer) -> FodIdParseResult: + """Reads a 51Did from the raw bytes of an envelope without raising. + + The buffer must hold exactly one envelope. ``None`` and an empty + buffer report :attr:`FodIdParseStatus.MISSING_INPUT`, anything that + is not ``bytes``, ``bytearray`` or ``memoryview`` reports + :attr:`FodIdParseStatus.INVALID_INPUT_TYPE`, and every other failure + names its reason. The signature is not checked. + """ + return cls._from_read(Owid.parse_bytes(buffer)) + + @classmethod + def from_base64(cls, base64: str) -> "FodId": + """Parses a 51Did from its base64-encoded OWID string in either + alphabet, raising when the value is not one. + + The same reading as :meth:`try_from_base64`, for callers who prefer + an exception. Raises :class:`TypeError` if ``base64`` is ``None`` or + not a string, :class:`ValueError` if the envelope was read but its + payload is shorter than the header or than the minimum for its + identifier type, and :class:`~fiftyone_pipeline_did.OwidError` for + every other failure, with the message naming the + :class:`FodIdParseStatus`. """ if base64 is None: raise TypeError("base64 must not be None") - return cls(Owid.from_base64(cls.to_standard_base64(base64))) + return cls._from_read_or_raise(_read_base64(base64), "base64") @staticmethod def to_standard_base64(value: str) -> str: @@ -176,25 +327,30 @@ def to_base64_url(value: str) -> str: @classmethod def from_byte_array(cls, buffer: bytes) -> "FodId": - """Parses a 51Did from the raw bytes of an OWID envelope. - - Raises :class:`TypeError` if ``buffer`` is ``None`` and - :class:`~fiftyone_pipeline_did.OwidError` if the bytes are not a - valid OWID. + """Parses a 51Did from the raw bytes of an OWID envelope, raising + when the bytes are not one. + + The same reading as :meth:`try_from_byte_array`, for callers who + prefer an exception. Raises :class:`TypeError` if ``buffer`` is + ``None`` or not a bytes-like object, :class:`ValueError` if the + envelope was read but its payload is shorter than the header or than + the minimum for its identifier type, and + :class:`~fiftyone_pipeline_did.OwidError` for every other failure, + with the message naming the :class:`FodIdParseStatus`. """ if buffer is None: raise TypeError("buffer must not be None") - return cls(Owid.from_byte_array(buffer)) + return cls._from_read_or_raise(Owid.parse_bytes(buffer), "buffer") @classmethod def from_owid(cls, owid: Owid) -> "FodId": """Promotes an already-parsed OWID into a 51Did. - The constructor **copies** the OWID (round-tripped through its byte - form), not aliases it, so a ``FodId`` can never desync from its - envelope if the caller later mutates the OWID it passed in. The - supplied OWID must therefore be signed (serializable). Raises - :class:`TypeError` if ``owid`` is ``None``. + The constructor writes the envelope out and reads it back through + this package's own parser rather than holding the caller's object, + so the ``FodId`` owns an envelope of its own. Raises + :class:`TypeError` if ``owid`` is ``None``, and otherwise what the + constructor raises. """ if owid is None: raise TypeError("owid must not be None") @@ -282,7 +438,100 @@ def as_byte_array(self) -> bytes: return self._owid.as_byte_array() def verify(self, public_pem: str) -> bool: - """Verifies the OWID signature against the supplied public key. This is - an explicit, separate step - construction never verifies. + """Verifies the OWID signature against the supplied public key. This + is an explicit, separate step, because parsing never verifies. + + Answers ``False`` only when the signature is well formed and does + not match. A key that cannot be decoded raises, as the fault is in + the key and not the identifier, so an outage is never reported as a + forgery. :meth:`signature_status` gives the same answer as a named + status without raising. """ return self._owid.verify_with_public_key(public_pem, []) + + def signature_status(self, public_pem: str) -> SignatureStatus: + """Says whether the signature is genuine, or why that could not be + decided, in the OWID vocabulary. + + Only :attr:`~fiftyone_pipeline_did.SignatureStatus.SIGNATURE_VALID` + and :attr:`~fiftyone_pipeline_did.SignatureStatus.SIGNATURE_INVALID` + are about the signature. The others say the question could not be + answered, for example + :attr:`~fiftyone_pipeline_did.SignatureStatus.KEY_UNAVAILABLE` when + no key was given, and must never be read as a forgery. + """ + return self._owid.signature_status(public_pem, []) + + +def _read_base64(value) -> ParseResult: + """The OWID read of a base64 string in either alphabet. Anything that is + not a string goes to the OWID reader as given, so the reason it reports + (nothing supplied, or a type it cannot read) is the one carried.""" + if isinstance(value, str): + value = FodId.to_standard_base64(value) + return Owid.parse(value) + + +def _read_payload(payload: bytes) -> Tuple[FodIdParseStatus, int, int, bytes]: + """Applies the two 51Did payload rules and unpacks the three fields. + + The header must be present before the type can be read, and the type + then says how many value bytes must follow. Anything beyond the value is + a creator context section whose lengths belong to the cloud, so a longer + payload passes. A Reserved type has no known value length and keeps the + documented best-effort reading, being the header fields and whatever + bytes follow. + + Returns the status and, on success, the flags, the licence id and the + value bytes. On failure the three fields are zero and empty. + """ + if payload is None or len(payload) < FodId.HEADER_LENGTH: + return FodIdParseStatus.PAYLOAD_TOO_SHORT, 0, 0, b"" + flags = payload[FodId.FLAGS_OFFSET] + value_length = _value_length(IdType.from_flags(flags), payload) + if len(payload) < FodId.HEADER_LENGTH + value_length: + return FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH, 0, 0, b"" + # Little-endian uint32, unsigned (Python ints are unbounded and + # non-negative here, so the high bit never becomes negative). + license_id = int.from_bytes( + payload[FodId.LICENSE_ID_OFFSET:FodId.LICENSE_ID_OFFSET + + FodId.LICENSE_ID_LENGTH], + byteorder="little", + signed=False, + ) + # bytes is immutable, so slicing yields a value that cannot be used to + # change the underlying payload and no defensive copy is required. + value = bytes(payload[FodId.HASH_OFFSET:FodId.HASH_OFFSET + value_length]) + return FodIdParseStatus.PARSED, flags, license_id, value + + +def _unpack_or_raise(payload: bytes) -> Tuple[int, int, bytes]: + """The payload rules for the raising readers, with the messages they + have always given.""" + status, flags, license_id, value = _read_payload(payload) + if status is not FodIdParseStatus.PARSED: + raise ValueError(_payload_message(status, payload)) + return flags, license_id, value + + +def _value_length(id_type: IdType, payload: bytes) -> int: + """How many value bytes the type needs after the header.""" + if id_type is IdType.RANDOM: + return FodId.GUID_LENGTH + if id_type is IdType.RESERVED: + return len(payload) - FodId.HEADER_LENGTH + return FodId.HASH_LENGTH + + +def _payload_message(status: FodIdParseStatus, payload: bytes) -> str: + """The message the raising readers give for a payload failure.""" + length = 0 if payload is None else len(payload) + if status is FodIdParseStatus.PAYLOAD_TOO_SHORT: + return "51Did payload must be at least {0} bytes; got {1}.".format( + FodId.HEADER_LENGTH, length) + id_type = IdType.from_flags(payload[FodId.FLAGS_OFFSET]) + return ("51Did payload for the {0} type must be at least {1} bytes; " + "got {2}.".format( + id_type.name, + FodId.HEADER_LENGTH + _value_length(id_type, payload), + length)) From aec95cd959167ba29c80e5650e8ac2b2ef5c21ce Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 31 Aug 2026 09:42:36 +0100 Subject: [PATCH 3/6] Cover the 51Did parse contract in the tests and the example The test envelope builder writes the wire fields with the OWID library own helpers and signs them by hand, because a Creator always writes version 3 and the current time and the removed constructor is gone. The FodId factory and the offline example create envelopes through Creator.create. New tests assert the three facts on every result for longer domains, longer creator context sections, each payload rule, each OWID failure carried through unchanged, absent and wrongly typed input, a tampered signature that parses and then verifies as invalid, a missing key that is not a forgery, the raising readers over the same inputs, and the client refusing malformed text before any key is fetched. --- .../examples/fodid_example.py | 12 +- fiftyone_pipeline_did/tests/envelope.py | 44 +- .../tests/test_did_client.py | 95 +++++ fiftyone_pipeline_did/tests/test_fodid.py | 380 ++++++++++++++++-- 4 files changed, 480 insertions(+), 51 deletions(-) diff --git a/fiftyone_pipeline_did/examples/fodid_example.py b/fiftyone_pipeline_did/examples/fodid_example.py index 21c0ff7..a98fc12 100644 --- a/fiftyone_pipeline_did/examples/fodid_example.py +++ b/fiftyone_pipeline_did/examples/fodid_example.py @@ -30,7 +30,7 @@ changes), but the value (the Hash) is stable. Compare values, never envelopes. """ -from owid import Crypto, Creator, Owid +from owid import Crypto, Creator from fiftyone_pipeline_did import FodId @@ -50,10 +50,12 @@ def sample_payload(): def issue(creator, payload): - """Issues (signs) a 51Did over the payload and returns it as base64.""" - owid = Owid(domain=DOMAIN, payload=payload) - creator.sign(owid) - return owid.as_base64() + """Issues (signs) a 51Did over the payload and returns it as base64. + + The creator is the only way a new envelope comes into being, because an + OWID is worth nothing unsigned, so the payload goes in and a signed + envelope comes out with no unsigned step in between.""" + return creator.create(payload).as_base64() def run(): diff --git a/fiftyone_pipeline_did/tests/envelope.py b/fiftyone_pipeline_did/tests/envelope.py index 7a00c2d..2c2bf56 100644 --- a/fiftyone_pipeline_did/tests/envelope.py +++ b/fiftyone_pipeline_did/tests/envelope.py @@ -30,6 +30,7 @@ from datetime import datetime, timedelta, timezone from owid import Crypto, Owid, Version +from owid import io as owid_io from fiftyone_pipeline_did import FodId @@ -67,19 +68,42 @@ def context_payload(): return probabilistic_payload() + bytes([0]) + bytes(range(1, 24)) +def envelope_bytes(crypto, payload, date=None, version=Version.VERSION3, + domain=TEST_DOMAIN, signature=None): + """The wire bytes of an envelope over the payload, dated as given (to + the minute, as the wire format stores it), stamped with the version + and signed with the key pair, or carrying the signature given instead. + + The OWID library only hands out an envelope from a parse or from a + Creator, and a Creator always writes version 3, its own domain and the + current time, so the tests write the fields with the library's own + wire helpers and sign the result by hand. The bytes signed are the + fields without the signature, which is what the library signs too.""" + if date is None: + date = datetime.now(timezone.utc) + buffer = bytearray() + owid_io.write_byte(buffer, version.as_byte()) + owid_io.write_string(buffer, domain) + owid_io.write_date(buffer, date.replace(second=0, microsecond=0), + version) + owid_io.write_byte_array(buffer, bytes(payload)) + if signature is None: + signature = crypto.sign_byte_array(bytes(buffer)) + owid_io.write_signature(buffer, signature) + return bytes(buffer) + + def signed_envelope(crypto, payload, date=None, version=Version.VERSION3, domain=TEST_DOMAIN): """An OWID over the payload, signed with the key pair, dated as given - (to the minute, as the wire format stores it) and stamped with the - version. The Creator class always writes version 3 and the current - time, so the fields are set and signed by hand here.""" - if date is None: - date = datetime.now(timezone.utc) - owid = Owid(version=version, domain=domain, - date=date.replace(second=0, microsecond=0), - payload=bytes(payload)) - owid.signature = crypto.sign_byte_array(owid.data_for_crypto([])) - return owid + and stamped with the version, read back through the library's own + parser so the tests hold the same kind of envelope a caller would.""" + read = Owid.parse_bytes( + envelope_bytes(crypto, payload, date, version, domain)) + if not read.ok: + raise AssertionError( + "the test envelope did not parse: {0}".format(read.status)) + return read.owid def signed_fod_id(crypto, payload=None, date=None, diff --git a/fiftyone_pipeline_did/tests/test_did_client.py b/fiftyone_pipeline_did/tests/test_did_client.py index 9f39a13..1e911ac 100644 --- a/fiftyone_pipeline_did/tests/test_did_client.py +++ b/fiftyone_pipeline_did/tests/test_did_client.py @@ -20,6 +20,7 @@ # such notice(s) shall fulfill the requirements of that article. # ********************************************************************* +import base64 import json import os import struct @@ -39,6 +40,8 @@ DidNotSupportedError, FactorResult, FodId, + FodIdParseStatus, + OwidError, RedeemResult, SignatureReason, SignatureResult, @@ -51,6 +54,7 @@ FixedClock, KeySchedule, context_payload, + envelope_bytes, form_of, probabilistic_payload, random_payload, @@ -744,5 +748,96 @@ def close(self): self.assertEqual(1, len(opener.calls)) +class MalformedIdentifierTests(unittest.TestCase): + """A malformed identifier is refused by the offline surfaces before + any key is fetched, and the parser's answer is separate from the + client's guard on obviously oversized text.""" + + def setUp(self): + self.schedule = KeySchedule() + self.transport = FakeTransport({"id/key/": (200, self.schedule.json())}) + self.client = DidClient(RESOURCE, LICENCE, ENDPOINT, + transport=self.transport) + self.date = self.schedule.start(1) + timedelta(days=2) + self.crypto = self.schedule.crypto(1) + + def malformed(self): + """Text the parser refuses, one case per kind of refusal: not + base64, an absent envelope marker, a truncated envelope, and an + envelope whose payload is too short for its type.""" + raw = signed_fod_id(self.crypto, date=self.date).as_byte_array() + short = envelope_bytes( + self.crypto, random_payload()[:-1], date=self.date) + return ( + "not-a-51did!!", + "AA", + FodId.to_base64_url(base64.b64encode(raw[:6]).decode()), + FodId.to_base64_url(base64.b64encode(short).decode()), + ) + + def test_offline_surfaces_refuse_malformed_text_before_any_key_fetch( + self): + for text in self.malformed(): + for call in (self.client.verify_signature, + self.client.verify_signature_detailed, + self.client.public_key_for): + with self.assertRaises((OwidError, ValueError), msg=text): + call(text) + self.assertEqual(0, len(self.transport.requests)) + + def test_parser_names_the_reason_before_the_client_is_asked(self): + expected = (FodIdParseStatus.INVALID_BASE64, + FodIdParseStatus.ABSENT_NODE, + FodIdParseStatus.UNEXPECTED_END, + FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH) + for text, status in zip(self.malformed(), expected): + result = FodId.try_from_base64(text) + self.assertFalse(result.ok, text) + self.assertIsNone(result.value) + self.assertIs(status, result.status) + self.assertEqual(0, len(self.transport.requests)) + + def test_tampered_signature_parses_then_verifies_as_signature(self): + raw = bytearray( + signed_fod_id(self.crypto, date=self.date).as_byte_array()) + raw[-1] ^= 0xFF + result = FodId.try_from_byte_array(bytes(raw)) + self.assertTrue(result.ok) + check = self.client.verify_signature_detailed(result.value) + self.assertFalse(check.valid) + self.assertEqual(SignatureReason.SIGNATURE, check.reason) + + def test_no_key_for_the_date_is_not_reported_as_a_bad_signature(self): + before = self.schedule.start(0) - timedelta(days=30) + check = self.client.verify_signature_detailed( + signed_fod_id(self.crypto, date=before)) + self.assertFalse(check.valid) + self.assertEqual(SignatureReason.NO_KEY, check.reason) + self.assertNotEqual(SignatureReason.SIGNATURE, check.reason) + + def test_key_fetch_failure_is_an_error_not_a_verdict(self): + self.transport.answers["id/key/"] = urllib.error.URLError( + "no route to host") + with self.assertRaises(OSError): + self.client.verify_signature( + signed_fod_id(self.crypto, date=self.date)) + self.transport.answers["id/key/"] = (500, "boom") + with self.assertRaises(DidClientError): + self.client.verify_signature_detailed( + signed_fod_id(self.crypto, date=self.date)) + + def test_oversized_text_is_the_client_guard_not_a_parse_status(self): + # The parser has no size limit of its own and answers the oversized + # text with an ordinary result, whilst the client refuses the same + # text as an argument failure before parsing it or fetching a key. + text = "A" * 5000 + result = FodId.try_from_base64(text) + self.assertFalse(result.ok) + self.assertIsInstance(result.status, FodIdParseStatus) + with self.assertRaises(ValueError): + self.client.verify_signature(text) + self.assertEqual(0, len(self.transport.requests)) + + if __name__ == "__main__": unittest.main() diff --git a/fiftyone_pipeline_did/tests/test_fodid.py b/fiftyone_pipeline_did/tests/test_fodid.py index 517acaf..048c3be 100644 --- a/fiftyone_pipeline_did/tests/test_fodid.py +++ b/fiftyone_pipeline_did/tests/test_fodid.py @@ -21,12 +21,22 @@ # ********************************************************************* import base64 +import struct import unittest from datetime import datetime, timezone -from owid import Owid, Crypto, Creator +from owid import Crypto, Creator, Owid, ParseStatus -from fiftyone_pipeline_did import FodId, IdType, OwidError +from fiftyone_pipeline_did import ( + FodId, + FodIdParseResult, + FodIdParseStatus, + IdType, + OwidError, + SignatureStatus, +) + +from .envelope import envelope_bytes, signed_envelope TEST_DOMAIN = "51degrees.com" # 0xA5: usage bits plus the HashedEmail type tag in bits 6-7. @@ -34,6 +44,10 @@ CANONICAL_LICENSE_ID = 0x12345678 CANONICAL_HASH = bytes((0x20 + i) for i in range(FodId.HASH_LENGTH)) +#: A creator domain longer than the one the cloud signs with, as a +#: self-hosted container may be configured to use. +LONG_DOMAIN = "identifiers." + ("a" * 120) + ".example" + def _write_license_id(payload): # Little-endian: low byte first (0x12345678 -> 78 56 34 12). @@ -62,21 +76,25 @@ def canonical_random_payload(): class FodIdTestFactory: - """Generates a fresh ECDSA P-256 key pair and signs real OWID envelopes.""" + """Generates a fresh ECDSA P-256 key pair and signs real OWID + envelopes. A Creator is the only way the OWID library brings a new + envelope into being, so the payload goes in and a signed envelope comes + out with no unsigned step in between.""" def __init__(self): - crypto = Crypto.new() - self.public_pem = crypto.public_key_pem() - self._creator = Creator(TEST_DOMAIN, crypto) + self.crypto = Crypto.new() + self.public_pem = self.crypto.public_key_pem() + self._creator = Creator(TEST_DOMAIN, self.crypto) def signed_owid(self, payload): - owid = Owid(domain=TEST_DOMAIN, payload=bytes(payload)) - self._creator.sign(owid) - return owid + return self._creator.create(bytes(payload)) def signed_owid_base64(self, payload): return self.signed_owid(payload).as_base64() + def signed_bytes(self, payload): + return self.signed_owid(payload).as_byte_array() + class FodIdTests(unittest.TestCase): @@ -109,7 +127,7 @@ def test_from_base64_unpacks_all_three_fields(self): self.assertEqual(TEST_DOMAIN, fod.domain) def test_from_byte_array_unpacks_all_three_fields(self): - buffer = self.factory.signed_owid(canonical_payload()).as_byte_array() + buffer = self.factory.signed_bytes(canonical_payload()) fod = FodId.from_byte_array(buffer) self.assertEqual(CANONICAL_FLAGS, fod.flags) self.assertEqual(CANONICAL_LICENSE_ID, fod.license_id) @@ -211,11 +229,13 @@ def test_payload_larger_than_spec_uses_first_37_bytes(self): def test_long_envelope_parses_and_keeps_the_header_fields(self): # No upper bound belongs in the reader: a creator domain is a # deployment parameter and a context section of a version this - # package does not know about may be any length. + # package does not know about may be any length. The signature is + # all zeros, which parses because parsing never verifies. payload = bytearray(canonical_payload()) + bytearray(200) - owid = Owid(domain="identifiers." + ("a" * 120) + ".example", - payload=bytes(payload), signature=bytes(64)) - fod = FodId.from_base64(owid.as_base64()) + raw = envelope_bytes(self.factory.crypto, payload, + domain=LONG_DOMAIN, signature=bytes(64)) + fod = FodId.from_byte_array(raw) + self.assertEqual(LONG_DOMAIN, fod.domain) self.assertEqual(CANONICAL_FLAGS, fod.flags) self.assertEqual(CANONICAL_LICENSE_ID, fod.license_id) self.assertEqual(CANONICAL_HASH, fod.hash) @@ -225,6 +245,8 @@ def test_is_cryptographically_verifiable(self): fod = FodId.from_base64( self.factory.signed_owid_base64(canonical_payload())) self.assertTrue(fod.verify(self.factory.public_pem)) + self.assertIs(SignatureStatus.SIGNATURE_VALID, + fod.signature_status(self.factory.public_pem)) def test_base64_roundtrip_preserves_all_fields(self): fod1 = FodId.from_base64( @@ -292,13 +314,15 @@ def test_reserved_header_only_parses(self): # ----- Gap tests (runbook section 6b) ----- def test_compare_two_51dids_same_payload(self): + # Two reissues of the same value at different times: the envelope + # differs and the value inside is the same. payload = canonical_payload() - a = self.factory.signed_owid(payload) - b = self.factory.signed_owid(payload) - # sign() stamps "now" to the minute, so set distinct dates to - # represent two reissues at different times. - a.date = datetime(2026, 1, 1, 0, 0, tzinfo=timezone.utc) - b.date = datetime(2026, 1, 1, 0, 5, tzinfo=timezone.utc) + a = signed_envelope( + self.factory.crypto, payload, + date=datetime(2026, 1, 1, 0, 0, tzinfo=timezone.utc)) + b = signed_envelope( + self.factory.crypto, payload, + date=datetime(2026, 1, 1, 0, 5, tzinfo=timezone.utc)) fa = FodId.from_base64(a.as_base64()) fb = FodId.from_base64(b.as_base64()) @@ -310,42 +334,46 @@ def test_compare_two_51dids_same_payload(self): def test_construction_does_not_verify(self): # An OWID with a present but tampered (invalid) signature still - # constructs and exposes all three fields - construction must not - # verify. - raw = bytearray(base64.b64decode( - self.factory.signed_owid_base64(canonical_payload()))) + # constructs and exposes all three fields, because construction + # never verifies. + raw = bytearray(self.factory.signed_bytes(canonical_payload())) raw[-1] ^= 0xFF # corrupt the signature - tampered = Owid.from_byte_array(bytes(raw)) - fod = FodId.from_owid(tampered) + fod = FodId.from_byte_array(bytes(raw)) self.assertEqual(CANONICAL_FLAGS, fod.flags) self.assertEqual(CANONICAL_LICENSE_ID, fod.license_id) self.assertEqual(CANONICAL_HASH, fod.hash) + self.assertFalse(fod.verify(self.factory.public_pem)) - def test_from_owid_is_decoupled_from_source_owid(self): - # Mutating the source OWID after construction must not affect the - # FodId (it holds an independent copy). + def test_source_envelope_cannot_be_changed_after_construction(self): + # The FodId used to copy the OWID it was given so a later change to + # the source could not reach it. The OWID library now hands out an + # envelope that cannot be changed at all, which is what makes the + # copy unnecessary, so that is the fact this test pins. owid = self.factory.signed_owid(canonical_payload()) fod = FodId.from_owid(owid) - owid.payload = bytes(FodId.PAYLOAD_LENGTH) # mutate the source - self.assertEqual(CANONICAL_FLAGS, fod.flags) + with self.assertRaises(AttributeError): + owid.payload = bytes(FodId.PAYLOAD_LENGTH) + with self.assertRaises(AttributeError): + owid.signature = bytes(64) self.assertEqual(CANONICAL_HASH, fod.hash) self.assertEqual(0x20, fod.payload[FodId.HASH_OFFSET]) - def test_constructor_is_decoupled_from_source_owid(self): - # The constructor must copy the OWID too, not just from_owid - - # mutating the source afterwards must not affect the FodId. + def test_constructor_reads_the_envelope_back_through_the_parser(self): + # The envelope handed in is written out and read back, so the FodId + # holds the same bytes whatever object the caller passed. owid = self.factory.signed_owid(canonical_payload()) fod = FodId(owid) - owid.payload = bytes(FodId.PAYLOAD_LENGTH) # mutate the source + self.assertEqual(owid.as_byte_array(), fod.as_byte_array()) self.assertEqual(CANONICAL_FLAGS, fod.flags) self.assertEqual(CANONICAL_HASH, fod.hash) - self.assertEqual(0x20, fod.payload[FodId.HASH_OFFSET]) def test_verify_with_wrong_key_returns_false(self): fod = FodId.from_base64( self.factory.signed_owid_base64(canonical_payload())) other_public_pem = Crypto.new().public_key_pem() self.assertFalse(fod.verify(other_public_pem)) + self.assertIs(SignatureStatus.SIGNATURE_INVALID, + fod.signature_status(other_public_pem)) def test_roundtrip_through_bytes_constructor_preserves_all_fields(self): fod1 = FodId.from_base64( @@ -357,5 +385,285 @@ def test_roundtrip_through_bytes_constructor_preserves_all_fields(self): self.assertEqual(fod1.domain, fod2.domain) +def _declared_length_offset(raw): + """The offset of the four byte payload length declaration in a version + 3 envelope: the version byte, the domain and its terminator, then the + four date bytes.""" + return 1 + raw.index(0, 1) + 1 + 4 + + +class FodIdTryParseTests(unittest.TestCase): + """The non-raising readers. Every case asserts the three facts a result + carries, being whether the parse succeeded, the value, and the status, + and the raising readers are checked against the same inputs.""" + + def setUp(self): + self.factory = FodIdTestFactory() + + def assert_parsed(self, result): + self.assertIsInstance(result, FodIdParseResult) + self.assertTrue(result.ok) + self.assertTrue(bool(result)) + self.assertIsInstance(result.value, FodId) + self.assertIs(FodIdParseStatus.PARSED, result.status) + return result.value + + def assert_failed(self, result, status): + self.assertIsInstance(result, FodIdParseResult) + self.assertFalse(result.ok) + self.assertFalse(bool(result)) + self.assertIsNone(result.value) + self.assertIs(status, result.status) + + def assert_canonical(self, fod): + self.assertEqual(CANONICAL_FLAGS, fod.flags) + self.assertEqual(CANONICAL_LICENSE_ID, fod.license_id) + self.assertEqual(CANONICAL_HASH, fod.hash) + + # ----- Vocabulary ----- + + def test_status_vocabulary_is_the_owid_one_plus_two(self): + # Every OWID status has a member of the same name and value, so an + # OWID failure is carried through unchanged, and the two 51Did + # payload statuses are the only additions. + for status in ParseStatus: + member = FodIdParseStatus.of(status) + self.assertEqual(status.name, member.name) + self.assertEqual(status.value, member.value) + owid_names = {status.name for status in ParseStatus} + extra = {member.name for member in FodIdParseStatus} - owid_names + self.assertEqual( + {"PAYLOAD_TOO_SHORT", "INVALID_TYPE_PAYLOAD_LENGTH"}, extra) + + def test_result_is_immutable_and_carries_exactly_three_facts(self): + result = FodId.try_from_base64( + self.factory.signed_owid_base64(canonical_payload())) + self.assertEqual(3, len(result)) + self.assertEqual(("ok", "value", "status"), result._fields) + with self.assertRaises(AttributeError): + result.ok = False + + # ----- Success ----- + + def test_valid_identifier_parses_in_both_alphabets(self): + standard = self.factory.signed_owid_base64(canonical_payload()) + url_safe = FodId.to_base64_url(standard) + for form in (standard, url_safe, standard.rstrip("="), + " " + url_safe + "\n"): + fod = self.assert_parsed(FodId.try_from_base64(form)) + self.assert_canonical(fod) + self.assertEqual(standard, fod.as_base64()) + + def test_valid_identifier_parses_from_bytes(self): + raw = self.factory.signed_bytes(canonical_payload()) + for form in (raw, bytearray(raw), memoryview(raw)): + fod = self.assert_parsed(FodId.try_from_byte_array(form)) + self.assert_canonical(fod) + self.assertEqual(raw, fod.as_byte_array()) + + def test_longer_self_hosted_creator_domain_is_accepted(self): + raw = envelope_bytes(self.factory.crypto, canonical_payload(), + domain=LONG_DOMAIN) + fod = self.assert_parsed(FodId.try_from_byte_array(raw)) + self.assertEqual(LONG_DOMAIN, fod.domain) + self.assert_canonical(fod) + self.assertTrue(fod.verify(self.factory.public_pem)) + + def test_longer_creator_context_section_is_accepted(self): + # An older reader meets a context section of a version it does not + # know. The header and value are read and the rest is kept. + payload = bytes(canonical_payload()) + bytes(range(64)) + fod = self.assert_parsed(FodId.try_from_base64( + self.factory.signed_owid_base64(payload))) + self.assert_canonical(fod) + self.assertEqual(FodId.HASH_LENGTH, len(fod.hash)) + self.assertEqual(payload, fod.payload) + + def test_far_longer_payload_is_not_rejected_for_its_length(self): + payload = bytes(canonical_payload()) + bytes(3000) + fod = self.assert_parsed(FodId.try_from_byte_array( + self.factory.signed_bytes(payload))) + self.assert_canonical(fod) + self.assertEqual(len(payload), len(fod.payload)) + + def test_random_identifier_parses_with_a_sixteen_byte_value(self): + fod = self.assert_parsed(FodId.try_from_base64( + self.factory.signed_owid_base64(canonical_random_payload()))) + self.assertEqual(IdType.RANDOM, fod.type) + self.assertEqual(FodId.GUID_LENGTH, len(fod.hash)) + + def test_reserved_header_only_parses_best_effort(self): + payload = bytearray(FodId.HEADER_LENGTH) + payload[FodId.FLAGS_OFFSET] = 0b1100_0000 + fod = self.assert_parsed(FodId.try_from_base64( + self.factory.signed_owid_base64(payload))) + self.assertEqual(IdType.RESERVED, fod.type) + self.assertEqual(b"", fod.hash) + + def test_success_does_not_verify_the_signature(self): + # All zero signature: the shape is right, the signature is not. + raw = envelope_bytes(self.factory.crypto, canonical_payload(), + signature=bytes(64)) + fod = self.assert_parsed(FodId.try_from_byte_array(raw)) + self.assertFalse(fod.verify(self.factory.public_pem)) + + # ----- The two 51Did payload rules ----- + + def test_short_random_payload_reports_invalid_type_payload_length(self): + payload = canonical_random_payload()[:FodId.RANDOM_PAYLOAD_LENGTH - 1] + self.assert_failed( + FodId.try_from_base64(self.factory.signed_owid_base64(payload)), + FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH) + self.assert_failed( + FodId.try_from_byte_array(self.factory.signed_bytes(payload)), + FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH) + + def test_short_probabilistic_payload_reports_invalid_type_length(self): + payload = canonical_payload()[:FodId.PAYLOAD_LENGTH - 1] + payload[FodId.FLAGS_OFFSET] = 0b0000_0101 + self.assert_failed( + FodId.try_from_base64(self.factory.signed_owid_base64(payload)), + FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH) + + def test_short_hashed_email_payload_reports_invalid_type_length(self): + payload = canonical_payload()[:FodId.PAYLOAD_LENGTH - 1] + self.assertEqual(IdType.HASHED_EMAIL, + IdType.from_flags(payload[FodId.FLAGS_OFFSET])) + self.assert_failed( + FodId.try_from_base64(self.factory.signed_owid_base64(payload)), + FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH) + + def test_header_only_random_payload_reports_invalid_type_length(self): + payload = canonical_random_payload()[:FodId.HEADER_LENGTH] + self.assert_failed( + FodId.try_from_base64(self.factory.signed_owid_base64(payload)), + FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH) + + def test_payload_shorter_than_the_header_reports_payload_too_short(self): + for length in range(FodId.HEADER_LENGTH): + payload = bytes([CANONICAL_FLAGS] * length) + self.assert_failed( + FodId.try_from_base64( + self.factory.signed_owid_base64(payload)), + FodIdParseStatus.PAYLOAD_TOO_SHORT) + self.assert_failed( + FodId.try_from_byte_array(self.factory.signed_bytes(payload)), + FodIdParseStatus.PAYLOAD_TOO_SHORT) + + # ----- OWID failures carried through unchanged ----- + + def test_invalid_base64_reports_the_owid_invalid_base64_status(self): + for text in ("This is not valid Base64!@#$", "A", "===="): + self.assert_failed(FodId.try_from_base64(text), + FodIdParseStatus.INVALID_BASE64) + + def test_declaration_mismatch_is_propagated_unchanged(self): + # The declared payload length is raised by one, so the declaration + # disagrees with the bytes present. The status is the OWID one, + # and nothing cryptographic is involved because parsing takes no + # key and checks no signature. + raw = bytearray(self.factory.signed_bytes(canonical_payload())) + at = _declared_length_offset(raw) + declared = struct.unpack(" Date: Mon, 31 Aug 2026 09:42:37 +0100 Subject: [PATCH 4/6] Document parsing without exceptions in the 51Did readme Adds the parse result and its three facts, the status meanings, the type specific lower bounds and the absence of a package upper bound, the 4096 character client guard as client policy rather than a format limit, which failures are data results and which remain exceptions, and a before and after for callers who used the removed OWID API through this package. --- fiftyone_pipeline_did/readme.md | 151 ++++++++++++++++++++++++++++++-- 1 file changed, 146 insertions(+), 5 deletions(-) diff --git a/fiftyone_pipeline_did/readme.md b/fiftyone_pipeline_did/readme.md index bb2c704..08f5b9a 100644 --- a/fiftyone_pipeline_did/readme.md +++ b/fiftyone_pipeline_did/readme.md @@ -62,10 +62,14 @@ Run `pwsh ./setup.ps1` from the repository root, or tests and examples import the fork under its own name, `owid`, which is how they build signed envelopes to test against. -The two OWID types the public API refers to are re-exported from the package +The OWID types the public API refers to are re-exported from the package itself, so a caller never has to reach into the private module. Catch -`fiftyone_pipeline_did.OwidError` for an OWID level failure, and use -`fiftyone_pipeline_did.Owid` for the envelope that `FodId.from_owid` takes. +`fiftyone_pipeline_did.OwidError` for an OWID level failure raised by the +raising readers, use `fiftyone_pipeline_did.Owid` for the envelope that +`FodId.from_owid` takes, and read `fiftyone_pipeline_did.SignatureStatus` +from `FodId.signature_status`. An `Owid` only ever comes from a successful +parse or from an OWID `Creator` that signs one into being, so there is no +way to hold an unsigned or partly built envelope. ## Usage @@ -94,6 +98,135 @@ encrypted value that only 51Degrees can turn back into a licence identifier, so `license_id` is the field's raw value and identifies nothing outside 51Degrees. +## Parsing without exceptions + +An identifier arriving from outside, in a query string, a header or a +form field, may be anything at all, and failing to be a 51Did is an +ordinary outcome rather than a fault. `try_from_base64` and +`try_from_byte_array` read such input without raising and answer with a +`FodIdParseResult`, a small immutable tuple carrying three facts: + +- `ok`, whether the parse succeeded; +- `value`, the `FodId` on success and `None` on failure, never a partly + read identifier; +- `status`, a `FodIdParseStatus`, which is `PARSED` on success and the + specific reason otherwise. + +The result is truthy on success, so `if result:` reads naturally. + +```python +from fiftyone_pipeline_did import FodId, FodIdParseStatus + +result = FodId.try_from_base64(text_from_the_request) # either alphabet +if result: + fod_id = result.value +else: + reason = result.status # for example FodIdParseStatus.INVALID_BASE64 +``` + +Parsing and verifying are separate steps. A successful parse says the +bytes have the shape of a 51Did and nothing about whether the signature +is genuine, so a parsed identifier is not known to be genuine until +`fod_id.verify(public_key_pem)`, `fod_id.signature_status(public_key_pem)` +or a `DidClient` check says so. `signature_status` answers in the OWID +`SignatureStatus` vocabulary, where only `SIGNATURE_VALID` and +`SIGNATURE_INVALID` are about the signature. `KEY_UNAVAILABLE`, +`INVALID_KEY` and `VERIFICATION_ERROR` say the question could not be +answered, which must never be read as a forgery, and the boolean `verify` +raises for a key it cannot use rather than answering `False` for the same +reason. + +### Status meanings + +The `FodIdParseStatus` vocabulary is the OWID one, member for member and +value for value, plus two members for the payload rules this package +applies once the envelope has been read. A failure inside the envelope is +carried through with the OWID status unchanged, so the reason reads the +same whichever language parsed the bytes. + +| Status | Meaning | +| --- | --- | +| `PARSED` | A structurally valid 51Did. The signature has not been checked | +| `MISSING_INPUT` | `None`, an empty string or an empty buffer | +| `INVALID_INPUT_TYPE` | Not a string (base64 reader) or not a bytes-like object (byte reader) | +| `INVALID_BASE64` | The text is not base64 in either alphabet | +| `UNSUPPORTED_VERSION` | The first byte names an envelope version this package does not know | +| `UNEXPECTED_END` | The data stopped in the middle of an envelope field | +| `INVALID_DOMAIN_ENCODING` | The creator domain is not terminated or is longer than the OWID maximum | +| `BYTE_COUNT_MISMATCH` | The declared payload length disagrees with the bytes present | +| `IMPLEMENTATION_CAPACITY_EXCEEDED` | The envelope is consistent but larger than this runtime can hold | +| `ABSENT_NODE` | The version 0 marker, which stands for an absent envelope | +| `MALFORMED_ENVELOPE` | Malformed in a way none of the above describes | +| `PAYLOAD_TOO_SHORT` | The envelope was read but the payload is shorter than the 5 byte header, so the type cannot be read | +| `INVALID_TYPE_PAYLOAD_LENGTH` | The header names a type whose value needs more bytes than the payload holds | + +### Lower bounds and no upper bound + +The payload must hold the 5 byte header before the type can be read, and +the type then says how many value bytes must follow, being 16 for +`RANDOM` and 32 for `PROBABILISTIC` and `HASHED_EMAIL`, as the payload +layout table above shows. `RESERVED` keeps the best-effort reading, being +the header fields and whatever bytes follow. Anything beyond the value is +a creator context section whose lengths belong to the cloud, so a longer +payload, a longer creator domain (a self-hosted container may sign with +one) or a longer envelope is accepted and this package places no upper +bound of its own on any of them. An older reader meeting a context +section of a version it does not know still reads the header and the +value. + +`DidClient` refuses text longer than 4096 characters before it parses +it, fetches a key or calls the cloud. That figure is client policy, +deliberately arbitrary and generous, and it is not a statement of how +long a 51Did can be. The parser answers such text with an ordinary +result, whilst the client raises its usual `ValueError`. + +### Expected results and exceptions + +Every `FodIdParseStatus` other than `PARSED` is an expected data result +from the `try_` readers and never an exception. The raising readers, +`from_base64`, `from_byte_array`, `from_owid` and the constructor, read +through the same logic and keep their documented exceptions for callers +who prefer them, being `TypeError` for `None` or a wrong input type, +`ValueError` for `PAYLOAD_TOO_SHORT` and `INVALID_TYPE_PAYLOAD_LENGTH`, +and `OwidError` for every other status, with the message naming the +status. Signature verification against a key that cannot be decoded, a +key list that cannot be fetched, and a cloud answer other than the one +asked for remain exceptions, because they are faults in the surroundings +and not properties of the identifier. + +### Migrating from the removed OWID API + +The OWID library no longer offers a throwing parse or a public +constructor, so an envelope cannot be assembled by hand, and code that +used those through this package changes as follows. + +```python +# Before the hardening, external input was read by catching what the +# reader raised. +from fiftyone_pipeline_did import FodId, OwidError +try: + fod_id = FodId.from_base64(text) +except (OwidError, ValueError): + fod_id = None + +# After the hardening, ask for the result and its reason. +from fiftyone_pipeline_did import FodId +result = FodId.try_from_base64(text) +fod_id = result.value if result else None + +# Before the hardening, an envelope was built by hand and signed +# afterwards, as the tests and the offline example did. +owid = Owid(domain=domain, payload=payload) +creator.sign(owid) + +# After the hardening, the creator signs a new envelope into being from +# the payload. +owid = creator.create(payload) +``` + +`from_base64`, `from_byte_array`, `from_owid` and the constructor keep +working and keep their exception types. + ## Comparing two 51Dids ```python @@ -138,6 +271,9 @@ Every request carries a `User-Agent` naming this package and its version. **1. Parse.** The identifier arrives from a page in the URL-safe alphabet and from the cloud in the standard one. `from_base64` takes either, with or without padding, and `as_base64_url()` gives the form to put in a URL. +Input that may not be a 51Did at all is better read with +`try_from_base64`, which names the reason instead of raising (see +"Parsing without exceptions" above). Neither checks the signature. ```python fod_id = FodId.from_base64(fifty_one_did) @@ -347,7 +483,12 @@ is refreshed by common-ci's `update-example-assets` step. ## Non-goals -- **No signature verification on construction.** Call `verify(public_key_pem)` - when needed. +- **No signature verification on parsing.** A parsed 51Did is not known to + be genuine. Call `verify(public_key_pem)`, `signature_status(public_key_pem)` + or a `DidClient` check when needed. +- **No upper bound on the size of an identifier.** The lengths beyond the + header and value belong to the cloud. The 4096 character figure in + `DidClient` is client policy against obviously malformed text, not a + format limit. - **No creation of new 51Dids.** This is a parser; new 51Dids are issued by the 51Degrees cloud / on-premise hashing engines. From 09cd5dddb630fac8ced5918761809fc044d9c4fa Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 31 Aug 2026 10:06:23 +0100 Subject: [PATCH 5/6] Refuse a malformed identifier before the cloud is called DidClient.verify and DidClient.redeem used to send a string identifier to the cloud as given so the cloud reported its own parse error. They now parse the string after the unchanged encoded size guard and refuse text that is not a 51Did with the existing DidArgumentError, naming the parse status, before any request is made, which is the same rule the offline surfaces already followed. A string that parses is still sent as given, and a string the cloud itself refuses still raises DidArgumentError with the cloud message and status code. The tests that sent a malformed string to the scripted cloud now send real identifiers, and two new tests prove no request is made for a malformed one and that the error names the status. --- fiftyone_pipeline_did/readme.md | 14 +++-- .../src/fiftyone_pipeline_did/did_client.py | 44 +++++++++---- .../tests/test_did_client.py | 62 +++++++++++++++---- 3 files changed, 92 insertions(+), 28 deletions(-) diff --git a/fiftyone_pipeline_did/readme.md b/fiftyone_pipeline_did/readme.md index 08f5b9a..d0a80d9 100644 --- a/fiftyone_pipeline_did/readme.md +++ b/fiftyone_pipeline_did/readme.md @@ -302,9 +302,11 @@ key = client.public_key_for(fod_id) # the entry in force, or None **3. Verify the signature through the cloud.** The open `verify` endpoint, one use against the resource key and no licence key needed. The identifier is sent under both the `51did` and `owid` query names, so the -call works with hosts that read either parameter. A value the cloud cannot -parse as a 51Did raises -`DidArgumentError` (a `ValueError`) carrying the cloud's message. +call works with hosts that read either parameter. A string that does not +parse as a 51Did raises `DidArgumentError` (a `ValueError`) before any +request is made, with the message naming the `FodIdParseStatus`, and a +value the cloud itself refuses raises the same error carrying the cloud's +message and `status_code` 400. ```python valid = client.verify(fod_id) # bool @@ -343,8 +345,10 @@ A context string this package does not know maps to `UNREADABLE`, so an unrecognised outcome is never mistaken for a good one, and `context_raw` keeps the string as sent. Every cryptographic failure comes back from the cloud as the one word `unreadable` by design, a missing licence key -included, so the client does not try to tell them apart either. A cloud -that cannot parse the 51Did raises `DidArgumentError` (HTTP 400), a host +included, so the client does not try to tell them apart either. A string +that does not parse as a 51Did raises `DidArgumentError` before any +request is made, a cloud that refuses the 51Did raises the same error +with HTTP 400, a host that does not offer the creator context raises `DidNotSupportedError` (HTTP 404), and any other status raises `DidClientError` carrying `status_code` and `body`. A transport failure raises the `OSError` the diff --git a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py index 40bcfed..b523424 100644 --- a/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py +++ b/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py @@ -177,10 +177,13 @@ def __init__(self, message: str, status_code: Optional[int] = None, class DidArgumentError(DidClientError, ValueError): - """The cloud refused the request because the 51Did sent was not a valid - identifier (HTTP 400 with an ``errors`` list). The message carries the - cloud's own text. Also a :class:`ValueError`, the language's argument - error.""" + """The 51Did given to a cloud call was not a valid identifier. Raised by + the client itself, before any transport, when the text does not parse + as a 51Did, with the message naming the + :class:`~fiftyone_pipeline_did.FodIdParseStatus` and no status code, + and raised for a cloud answer of HTTP 400 with an ``errors`` list, with + the cloud's own text and the status code. Also a :class:`ValueError`, + the language's argument error.""" class DidNotSupportedError(DidClientError): @@ -482,11 +485,12 @@ def verify(self, fod_id: Union[FodId, str]) -> bool: endpoint, the open endpoint that needs no licence key. One use against the resource key. - Raises :class:`DidArgumentError` (also a :class:`ValueError`) when - the cloud could not parse the value as a 51Did, with the cloud's - message, and :class:`DidClientError` on any answer other than - valid or invalid. Text far longer than any identifier raises - :class:`ValueError` before transport. A transport failure raises + Raises :class:`DidArgumentError` (also a :class:`ValueError`) + before transport when a string does not parse as a 51Did, with the + message naming the parse status, or when the cloud refused the + value, with the cloud's message, and :class:`DidClientError` on any + answer other than valid or invalid. Text far longer than any + identifier raises :class:`ValueError` before transport. A transport failure raises the :class:`OSError` the transport raised (:class:`urllib.error.URLError` by default).""" text = _identifier_text(fod_id) @@ -537,8 +541,10 @@ def redeem(self, fod_id: Union[FodId, str], result: str, returned it to the page. :param challenge: the single-use challenge given to the verify endpoint, where one was. - :raises DidArgumentError: when the cloud could not parse the value - as a 51Did (HTTP 400), with the cloud's message. + :raises DidArgumentError: before transport when a string does not + parse as a 51Did, with the message naming the parse status, or + when the cloud refused the value (HTTP 400), with the cloud's + message. :raises ValueError: before transport when the text is far longer than any identifier. :raises DidNotSupportedError: when the host does not offer the @@ -711,15 +717,29 @@ def _as_fod_id(value: Union[FodId, str]) -> FodId: def _identifier_text(value: Union[FodId, str]) -> str: """The text sent to the cloud for an identifier. A parsed identifier goes in the URL-safe alphabet, which needs no further encoding, and a - string goes as given so the cloud can report its own parse error.""" + string goes as given, in whichever alphabet and padding it arrived, + once the client has checked that the string parses as a 51Did, so a + value that is not one never reaches the cloud.""" if isinstance(value, FodId): return value.as_base64_url() if isinstance(value, str) and value != "": _ensure_encoded_size(value) + _ensure_parses(value) return value raise TypeError("fod_id must be a FodId or a non-empty base64 string") +def _ensure_parses(value: str) -> None: + """Refuses text that is not a 51Did before any transport, naming the + parse status in the client's argument error. The signature is not + checked here, as the cloud call is what checks it.""" + result = FodId.try_from_base64(value) + if not result.ok: + raise DidArgumentError( + "The identifier is not a 51Did: {0}.".format( + result.status.value)) + + def _ensure_encoded_size(value: str) -> None: """Refuses obviously malformed text before it is parsed, a key is fetched or the cloud is called. Surrounding whitespace is stripped diff --git a/fiftyone_pipeline_did/tests/test_did_client.py b/fiftyone_pipeline_did/tests/test_did_client.py index 1e911ac..2508ffc 100644 --- a/fiftyone_pipeline_did/tests/test_did_client.py +++ b/fiftyone_pipeline_did/tests/test_did_client.py @@ -473,22 +473,34 @@ def test_400_invalid(self): self.transport.answers["id/verify/"] = (400, '{"valid":false}') self.assertFalse(self.client.verify(self.fod_id)) - def test_400_errors_raises_the_argument_error_with_the_message(self): + def test_400_errors_from_the_cloud_raises_the_argument_error(self): + # A string that parses here can still be refused by the cloud, for + # example one from a creator the cloud does not know, and the + # cloud's own message is what the error then carries. self.transport.answers["id/verify/"] = ( - 400, '{"errors":["Value for 51did is not a valid ' - 'Base64-encoded 51Did: \'zzz\'."]}') + 400, '{"errors":["Value for 51did is not a 51Did this service ' + 'issued."]}') with self.assertRaises(DidArgumentError) as raised: - self.client.verify("zzz") + self.client.verify(self.fod_id.as_base64_url()) self.assertIsInstance(raised.exception, ValueError) - self.assertIn("not a valid Base64-encoded 51Did", + self.assertIn("not a 51Did this service issued", str(raised.exception)) self.assertEqual(400, raised.exception.status_code) def test_string_form_is_sent_as_given_and_encoded(self): + # A payload of 0xFB bytes encodes to "+/v7" whatever the alignment, + # so the standard form carries both characters that need encoding. + payload = bytearray(probabilistic_payload()) + for i in range(FodId.HASH_LENGTH): + payload[FodId.HASH_OFFSET + i] = 0xFB + standard = signed_fod_id(Crypto.new(), bytes(payload)).as_base64() + self.assertIn("+", standard) + self.assertIn("/", standard) self.transport.answers["id/verify/"] = (200, '{"valid":true}') - self.client.verify("AwB+/x==") - self.assertIn("51did=AwB%2B%2Fx%3D%3D", self.transport.last().full_url) - self.assertIn("owid=AwB%2B%2Fx%3D%3D", self.transport.last().full_url) + self.client.verify(standard) + encoded = urllib.parse.quote(standard, safe="") + self.assertIn("51did=" + encoded, self.transport.last().full_url) + self.assertIn("owid=" + encoded, self.transport.last().full_url) def test_padded_and_unpadded_forms_are_both_accepted(self): fod_id = signed_fod_id(Crypto.new(), payload=context_payload()) @@ -685,10 +697,14 @@ def test_transport_failure_propagates_as_the_io_error(self): self.client.redeem(self.fod_id, "sealed-result", "abc123") def test_string_identifier_is_sent_as_given(self): + # The padded standard form goes as given rather than being + # converted to the URL-safe form a parsed identifier is sent in. + text = self.fod_id.as_base64() + self.assertTrue(text.endswith("=")) self.transport.answers["id/redeem"] = (200, '{"context":"unreadable"}') - self.client.redeem("AwB-_x", "sealed-result", None) + self.client.redeem(text, "sealed-result", None) form = form_of(self.transport.last()) - self.assertEqual("AwB-_x", form["51did"]) + self.assertEqual(text, form["51did"]) self.assertEqual("", form["challenge"]) def test_redeem_refuses_far_too_long_text_before_the_form(self): @@ -744,7 +760,8 @@ def close(self): opener = Opener() client = DidClient(RESOURCE, endpoint=ENDPOINT, transport=opener) - self.assertFalse(client.verify("AwB-_x")) + self.assertFalse(client.verify( + signed_fod_id(Crypto.new()).as_base64_url())) self.assertEqual(1, len(opener.calls)) @@ -797,6 +814,29 @@ def test_parser_names_the_reason_before_the_client_is_asked(self): self.assertIs(status, result.status) self.assertEqual(0, len(self.transport.requests)) + def test_cloud_surfaces_refuse_malformed_text_before_any_transport( + self): + for text in self.malformed(): + with self.assertRaises(DidArgumentError, msg=text) as raised: + self.client.verify(text) + self.assertIsInstance(raised.exception, ValueError) + # Refused here, so there is no cloud status to carry. + self.assertIsNone(raised.exception.status_code) + with self.assertRaises(DidArgumentError, msg=text): + self.client.redeem(text, "sealed-result", "abc") + self.assertEqual(0, len(self.transport.requests)) + + def test_cloud_surface_refusal_names_the_parse_status(self): + expected = (FodIdParseStatus.INVALID_BASE64, + FodIdParseStatus.ABSENT_NODE, + FodIdParseStatus.UNEXPECTED_END, + FodIdParseStatus.INVALID_TYPE_PAYLOAD_LENGTH) + for text, status in zip(self.malformed(), expected): + with self.assertRaises(DidArgumentError) as raised: + self.client.redeem(text, "sealed-result", "abc") + self.assertIn(status.value, str(raised.exception)) + self.assertEqual(0, len(self.transport.requests)) + def test_tampered_signature_parses_then_verifies_as_signature(self): raw = bytearray( signed_fod_id(self.crypto, date=self.date).as_byte_array()) From 020ad7ea99aba9f974f19cfb34672f0c3cca2b82 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 31 Aug 2026 10:34:27 +0100 Subject: [PATCH 6/6] Point the OWID submodule at the merged hardening commit The owid-python submodule now records 09443229f98f81bd1cedc624eec74f9486b9042d, the squash merged hardening commit on main of the 51Degrees/owid-python fork. The temporary pin at f22ac41, the tip of the branch harden/parse-without-throwing, is gone. --- owid-python | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/owid-python b/owid-python index f22ac41..0944322 160000 --- a/owid-python +++ b/owid-python @@ -1 +1 @@ -Subproject commit f22ac4116141d0cbf4a6068b4ca32e64fa04b64b +Subproject commit 09443229f98f81bd1cedc624eec74f9486b9042d