fix: transport, IPFS and key-sharing hardening (R2 follow-up)

CHANGELOG.md

@@ -48,6 +48,17 @@ All notable changes to `citrate-labs-sdk` are documented here. This project adhe
   verify `expected_sha256` and self-describing CIDs.
 - **PBA-L6b-042:** manager writes assert `eth_chainId` against the pinned chain.
 
+- **Verifier follow-ups (still 0.6.2, unreleased):**
+  - Transport-gate hardening: characters outside RFC 3986 and userinfo are
+    refused, and the gate checks the same host the HTTP stack connects to
+    (PBA-L6b-026 / PBA-L6b-027).
+  - IPFS downloads of CIDs that cannot verify their own content need
+    `expected_sha256`, or an explicit `verify=False`, which logs a warning
+    (PBA-L6b-030).
+  - `threshold_shares=1` needs `allow_single_holder_recovery=True`.
+  - The deploy guard also refuses values shaped like shares, not only the
+    known field names.
+
 ### Changed (breaking)
 
 - `IdentityClient.refresh(refresh_token, expected_sub)`: `expected_sub` is
@@ -58,6 +69,10 @@ All notable changes to `citrate-labs-sdk` are documented here. This project adhe
   `{"kind": "redirect" | "token", ...}` (the authority never returned the
   access/refresh tokens the old client expected).
 - `reconstruct_key_from_shares(shares, threshold)`: threshold is required.
+- Endpoint URLs with credentials (`user:pass@host`) or non-RFC-3986
+  characters are refused; pass credentials as headers.
+- `download_bytes("Qm...")` without `expected_sha256` raises unless
+  `verify=False`.
 - Unknown `access` / `tier` / `mode` strings raise `ValueError` (PBA-L6b-031)
   instead of silently becoming option 0.
 

citrate_sdk/_url_security.py

@@ -14,9 +14,17 @@
 """
 
 import ipaddress
+import re
 import unicodedata
 from urllib.parse import urlparse
 
+from urllib3.exceptions import LocationParseError
+from urllib3.util import parse_url
+
+# PBA-L6b-026 follow-up: RFC 3986 characters only (unreserved, reserved and
+# "%"). URL parsers disagree on anything else, so it is refused.
+_RFC3986_CHARS = re.compile(r"^[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+,;=%]*$")
+
 # PBA-L6b-026: only these schemes are ever accepted.
 _ALLOWED_SCHEMES = frozenset({"https", "http"})
 
@@ -87,24 +95,51 @@ def enforce_transport_security(url: str, *, allow_insecure_http: bool = False) -
                 "lets an http:// endpoint slip past this check (PBA-L6b-026)."
             )
 
-    parsed = urlparse(url)
+    if not _RFC3986_CHARS.match(url):
+        bad = next(ch for ch in url if not _RFC3986_CHARS.match(ch))
+        raise InsecureTransportError(
+            f"Refusing endpoint URL {url!r}: character {bad!r} is not allowed in a URL "
+            "(RFC 3986) (PBA-L6b-026)."
+        )
+
+    try:
+        parsed = urlparse(url)
+        parsed_host = parsed.hostname
+        requests_view = parse_url(url)
+    except (ValueError, LocationParseError):
+        raise InsecureTransportError(f"Refusing unparseable endpoint URL {url!r} (PBA-L6b-026).")
     scheme = parsed.scheme.lower()
     if scheme not in _ALLOWED_SCHEMES:
         raise InsecureTransportError(
             f"Refusing endpoint URL {url!r}: scheme {parsed.scheme!r} is not https or "
             "http (PBA-L6b-026)."
         )
-    if not parsed.hostname:
+    if not parsed_host:
         raise InsecureTransportError(f"Refusing endpoint URL {url!r}: no host (PBA-L6b-026).")
+    # Credentials in the URL are refused outright; pass them as headers.
+    if "@" in parsed.netloc or parsed.username is not None or requests_view.auth is not None:
+        raise InsecureTransportError(
+            f"Refusing endpoint URL {url!r}: userinfo (user@ / user:pass@) is not "
+            "allowed in an endpoint URL; pass credentials as headers (PBA-L6b-026)."
+        )
+    # Gate on the host exactly as urllib3 (what requests connects to) parses
+    # it, and require urllib.parse to agree.
+    transport_host = (requests_view.host or "").strip("[]").lower()
+    if transport_host != parsed_host.lower():
+        raise InsecureTransportError(
+            f"Refusing endpoint URL {url!r}: urllib.parse sees host {parsed_host!r} but "
+            f"requests would connect to {requests_view.host!r} (PBA-L6b-026)."
+        )
+
     if scheme == "https":
         return url
 
-    if _is_local_host(parsed.hostname):
+    if _is_local_host(parsed_host):
         return url
 
     if not allow_insecure_http:
         raise InsecureTransportError(
-            f"Refusing to connect to remote host {parsed.hostname!r} over "
+            f"Refusing to connect to remote host {parsed_host!r} over "
             f"plaintext http:// ({url!r}). Traffic (including signed "
             f"transactions, private inputs, and bearer credentials) would be "
             f"sent in cleartext and could be intercepted or tampered with. Use "

citrate_sdk/crypto.py

@@ -5,6 +5,7 @@
 import hashlib
 import hmac
 import json
+import re
 import secrets
 from typing import Any, cast
 
@@ -27,8 +28,35 @@
 _MAX_GUARD_DEPTH = 32
 
 
+_HEX_RE = re.compile(r"^(0x)?[0-9a-fA-F]+$")
+
+
+def _looks_like_share(d: dict[Any, Any]) -> bool:
+    """A raw Shamir share ({x, y} with y as hex/bytes) or a holder-wrapped
+    share record ({holder_public_key/holderPublicKey, envelope})."""
+    y = d.get("y")
+    if "x" in d and (isinstance(y, (bytes, bytearray)) or (isinstance(y, str) and _HEX_RE.match(y))):
+        return True
+    return "envelope" in d and ("holder_public_key" in d or "holderPublicKey" in d)
+
+
 def assert_no_key_share_material(value: Any, _depth: int = 0) -> None:
-    """Raise CitrateError if ``value`` carries a key-share field (PBA-L6b-003)."""
+    """Raise CitrateError if ``value`` carries key-share material (PBA-L6b-003).
+
+    Refuses the known share field names AND, by structure, any object that
+    looks like a share ({x, y-hex}) or a wrapped share record, at any depth,
+    including inside JSON-encoded string values (a renamed field such as
+    ``myShares`` is caught too).
+    """
+    if isinstance(value, str):
+        # Any string that parses as JSON is checked too (no size cap: a padded
+        # blob must not slip through).
+        try:
+            decoded = json.loads(value)
+        except (ValueError, RecursionError):
+            return
+        assert_no_key_share_material(decoded, _depth + 1)
+        return
     if isinstance(value, dict):
         items = list(value.items())
     elif isinstance(value, (list, tuple)):
@@ -40,6 +68,12 @@ def assert_no_key_share_material(value: Any, _depth: int = 0) -> None:
             f"deploy_model: metadata is nested more than {_MAX_GUARD_DEPTH} levels deep; refusing to "
             "publish calldata that cannot be fully checked for key-share material (PBA-L6b-003)."
         )
+    if isinstance(value, dict) and _looks_like_share(value):
+        raise CitrateError(
+            "deploy_model: refusing to publish a value shaped like a key share ({x, y} or a "
+            "wrapped share record) in public deploy calldata. Deliver key shares to their "
+            "holders off-chain (PBA-L6b-003)."
+        )
     for k, v in items:
         if k in SHARE_FIELD_DENYLIST:
             raise CitrateError(
@@ -259,6 +293,12 @@ def _plan_key_sharing(config: 'EncryptionConfig') -> tuple[int, int, list[str]]
                 f"total_shares={total!r}); need integers with 1 <= threshold_shares <= "
                 "total_shares <= 255."
             )
+        if threshold == 1 and not config.allow_single_holder_recovery:
+            raise CitrateError(
+                "encrypt_model: threshold_shares=1 lets ANY single holder recover the model key, "
+                "which is not threshold sharing. Pass allow_single_holder_recovery=True if that "
+                "is really intended (PBA-L6b-003 follow-up)."
+            )
         holders = config.share_holder_public_keys
         if not holders:
             raise CitrateError(
@@ -724,6 +764,7 @@ def __init__(
         threshold_shares: int = 0,
         total_shares: int = 0,
         share_holder_public_keys: list[str] | None = None,
+        allow_single_holder_recovery: bool = False,
     ):
         """
         Args:
@@ -734,13 +775,16 @@ def __init__(
                 (PBA-L6b-003): one distinct secp256k1 public key per share. Each
                 share is ECDH-wrapped to its holder and returned for off-chain
                 delivery; shares are never written to deploy calldata.
+            allow_single_holder_recovery: threshold_shares=1 (any one holder
+                alone recovers the key) is refused unless this is True.
         """
         self.algorithm = algorithm
         self.key_derivation = key_derivation
         self.access_control = access_control
         self.threshold_shares = threshold_shares
         self.total_shares = total_shares
         self.share_holder_public_keys = share_holder_public_keys
+        self.allow_single_holder_recovery = allow_single_holder_recovery
 
 
 def generate_model_key() -> str:

citrate_sdk/ipfs.py

@@ -6,13 +6,16 @@
 import hashlib
 import hmac
 import json
+import logging
 from typing import Any, cast
 
 import requests
 
 from ._url_security import enforce_transport_security
 from .errors import IPFSError
 
+_log = logging.getLogger(__name__)
+
 #: PBA-L6b-030: default ceiling on a single download (bytes). Pass
 #: ``max_bytes`` to raise or lower it per call.
 DEFAULT_MAX_DOWNLOAD_BYTES = 1 << 30  # 1 GiB
@@ -124,6 +127,7 @@ def download_bytes(
         *,
         expected_sha256: str | None = None,
         max_bytes: int | None = None,
+        verify: bool = True,
     ) -> bytes:
         """
         Download bytes from IPFS using hash
@@ -139,10 +143,18 @@ def download_bytes(
         - self-describing addresses are verified with no hint: ``sha256:<hex>``
           pointers and CIDv1 raw/sha2-256 (``bafkrei...``).
 
+        Verification is MANDATORY (PBA-L6b-030 follow-up): if the address
+        cannot verify itself and no ``expected_sha256`` is given, the download
+        is refused before any request. ``verify=False`` is the explicit opt-out
+        for callers who accept unverified bytes; it logs a warning. It never
+        skips a supplied ``expected_sha256`` or the size cap.
+
         Args:
             ipfs_hash: IPFS hash (CID)
-            expected_sha256: optional hex sha256 the content must match
+            expected_sha256: hex sha256 the content must match (required for
+                dag-pb CIDs unless ``verify=False``)
             max_bytes: optional size ceiling for this download
+            verify: set False to accept unverifiable content (logged)
 
         Returns:
             Downloaded bytes
@@ -151,6 +163,19 @@ def download_bytes(
             IPFSError: If download fails, exceeds the cap, or fails verification
         """
         limit = DEFAULT_MAX_DOWNLOAD_BYTES if max_bytes is None else max_bytes
+        committed = _expected_digest_from_address(ipfs_hash)
+        if committed is None and expected_sha256 is None:
+            if verify:
+                raise IPFSError(
+                    f"refusing to download {ipfs_hash}: this address does not verify its own "
+                    "content (dag-pb CIDs hash a chunked DAG, not the bytes), so pass "
+                    "expected_sha256 (e.g. the on-chain model_hash), or verify=False to accept "
+                    "unverified bytes explicitly (PBA-L6b-030)."
+                )
+            _log.warning(
+                "IPFS download of %s is UNVERIFIED: verify=False and no expected_sha256; the "
+                "gateway can return arbitrary bytes (PBA-L6b-030).", ipfs_hash,
+            )
         try:
             response = self.session.post(
                 f"{self.api_url}/api/v0/cat",
@@ -177,7 +202,6 @@ def download_bytes(
             want = expected_sha256[2:] if expected_sha256.startswith("0x") else expected_sha256
             if not hmac.compare_digest(digest.hex(), want.lower()):
                 raise IPFSError(f"IPFS content for {ipfs_hash} does not match expected_sha256")
-        committed = _expected_digest_from_address(ipfs_hash)
         if committed is not None and not hmac.compare_digest(digest, committed):
             raise IPFSError(f"IPFS content does not match its address {ipfs_hash}")
         return data
@@ -362,6 +386,7 @@ def download(
         *,
         expected_sha256: str | None = None,
         max_bytes: int | None = None,
+        verify: bool = True,
     ) -> bytes:
         """
         Download data with automatic fallback
@@ -382,7 +407,7 @@ def download(
         if self.active_client:
             try:
                 return self.active_client.download_bytes(
-                    ipfs_hash, expected_sha256=expected_sha256, max_bytes=max_bytes)
+                    ipfs_hash, expected_sha256=expected_sha256, max_bytes=max_bytes, verify=verify)
             except IPFSError:
                 pass
 
@@ -391,7 +416,7 @@ def download(
             try:
                 if client.is_available():
                     return client.download_bytes(
-                        ipfs_hash, expected_sha256=expected_sha256, max_bytes=max_bytes)
+                        ipfs_hash, expected_sha256=expected_sha256, max_bytes=max_bytes, verify=verify)
 
             except IPFSError as e:
                 last_error = e
@@ -471,6 +496,7 @@ def download_from_ipfs(
     *,
     expected_sha256: str | None = None,
     max_bytes: int | None = None,
+    verify: bool = True,
 ) -> bytes:
     """
     Convenience function to download data from IPFS
@@ -483,4 +509,4 @@ def download_from_ipfs(
         Downloaded bytes
     """
     manager = get_ipfs_manager(ipfs_urls)
-    return manager.download(ipfs_hash, expected_sha256=expected_sha256, max_bytes=max_bytes)
+    return manager.download(ipfs_hash, expected_sha256=expected_sha256, max_bytes=max_bytes, verify=verify)

pyproject.toml

@@ -55,6 +55,9 @@ classifiers = [
 # HKDF envelope (crypto.py) and RSA ID-token verification (identity/jwt.py).
 dependencies = [
     "requests~=2.33",
+    # PBA-L6b-026: the transport gate parses URLs with urllib3 (what requests
+    # connects with) to rule out parser differentials; pinned explicitly.
+    "urllib3>=2.0,<3",
     "cryptography>=48.0.1,<51",
     "eth-account~=0.9",
     "web3~=7.15",

tests/test_integration.py

@@ -17,6 +17,7 @@
 import pytest
 
 # Import SDK modules
+from citrate_sdk._url_security import InsecureTransportError
 from citrate_sdk.client import CitrateClient
 from citrate_sdk.crypto import KeyManager
 from citrate_sdk.errors import CitrateError
@@ -486,8 +487,16 @@ def test_invalid_address_error(self, client):
             client.get_balance('not_a_valid_address')
 
     def test_network_error_handling(self):
-        """Network errors are handled gracefully"""
-        bad_client = CitrateClient(rpc_url='http://localhost:99999')
+        """Network errors are handled gracefully.
+
+        PBA-L6b-026 follow-up: this used port 99999, which is not a valid port.
+        The transport gate now parses URLs the way urllib3/requests do and refuses
+        an unparseable one at construction, so the network-error path is
+        exercised with a valid port that has no listener instead.
+        """
+        with pytest.raises(InsecureTransportError):
+            CitrateClient(rpc_url='http://localhost:99999')
+        bad_client = CitrateClient(rpc_url='http://localhost:1')
         with pytest.raises(CitrateError):
             bad_client.get_chain_id()