From 01ef5947261f3268887325234213a48113ebd1a7 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Thu, 2 Jul 2026 18:46:03 +0700 Subject: [PATCH] fix(auth): select signing algorithm explicitly for self-certifying DIDs Willow DIDs are now self-certifying (did:willow:z) and no longer embed the key algorithm in the string. detect_algorithm_from_did could therefore no longer recover it and silently defaulted to Ed25519, so a secp256k1 (Ethereum/wallet) identity signed per-request auth and consensus transactions with the wrong algorithm and was rejected by the server. Thread an explicit `algorithm` argument (default "Ed25519", backward compatible) through the signing paths: - auth.sign_request - WillowClient.set_identity -> stored and passed to its three sign_request call sites - ConsensusClient signing methods -> _sign_and_broadcast detect_algorithm_from_did is retained (it still parses the retired did:willow:: format) but is no longer used to choose a signing algorithm and is documented as deprecated. The DID string is unchanged. Add regression tests proving a secp256k1 identity signs with secp256k1 (and would NOT verify under the old Ed25519 default) and that Ed25519 still defaults correctly, at both the auth.sign_request and WillowClient levels. Co-Authored-By: Claude Fable 5 --- src/willow/auth.py | 25 ++++++++-- src/willow/client.py | 21 +++++++-- src/willow/consensus/client.py | 46 +++++++++++------- tests/test_auth.py | 85 ++++++++++++++++++++++++++++++++++ tests/test_client.py | 57 +++++++++++++++++++++++ 5 files changed, 208 insertions(+), 26 deletions(-) diff --git a/src/willow/auth.py b/src/willow/auth.py index 80effe5..c120ab7 100644 --- a/src/willow/auth.py +++ b/src/willow/auth.py @@ -351,13 +351,25 @@ def verify_signature( def detect_algorithm_from_did(did: str) -> SignatureAlgorithm: """ - Detect signature algorithm from DID. + Detect signature algorithm from a *legacy* DID string. + + .. deprecated:: + Willow DIDs are now self-certifying and no longer embed the key + algorithm in the string:: + + did:willow:z + + The algorithm is therefore **not recoverable** from the DID and this + helper returns ``"Ed25519"`` for every self-certifying id. Do not use + it to pick a signing algorithm: pass the algorithm explicitly instead + (see ``sign_request`` and ``Client.set_identity``). It is retained only + to parse the retired ``did:willow::`` format. Args: did: The DID string Returns: - The detected algorithm + The detected algorithm (``"Ed25519"`` for any self-certifying id) """ if "ed25519" in did.lower(): return "Ed25519" @@ -373,7 +385,8 @@ def sign_request( private_key_hex: str, public_key_id: str, method: str, - path: str + path: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> Dict[str, str]: """ Sign an HTTP request for per-request authentication. @@ -387,13 +400,17 @@ def sign_request( public_key_id: Public key ID from DID document method: HTTP method (e.g., "GET", "POST") path: API path (e.g., "/data/my-subgrove/my-key") + algorithm: Signature algorithm of the identity ("Ed25519" or + "secp256k1"). Must be supplied by the caller because the + self-certifying DID no longer encodes it; the algorithm cannot be + recovered from the DID string. Defaults to "Ed25519" for backward + compatibility with the SDK's Ed25519-first identities. Returns: Dictionary of authentication headers to include in the request """ timestamp = int(time.time()) message = f"{method}:{path}:{timestamp}" - algorithm = detect_algorithm_from_did(did) signature = sign_challenge(message, private_key_hex, algorithm) return { "X-DID": did, diff --git a/src/willow/client.py b/src/willow/client.py index ef33074..19ca9c2 100644 --- a/src/willow/client.py +++ b/src/willow/client.py @@ -34,7 +34,7 @@ HealthStatus, RetryConfig, ) -from .auth import sign_request +from .auth import sign_request, SignatureAlgorithm from .errors import ( WillowError, AuthenticationError, @@ -872,6 +872,7 @@ def __init__( self._did: Optional[str] = None self._private_key: Optional[str] = None self._public_key_id: Optional[str] = None + self._algorithm: SignatureAlgorithm = "Ed25519" # Configure proof verification if options provided if proof_verification_options: @@ -976,7 +977,7 @@ async def _call_validator() -> Dict[str, Any]: if self.is_authenticated(): headers = sign_request( self._did, self._private_key, self._public_key_id, - "POST", path, + "POST", path, self._algorithm, ) url = f"{self.api_url}{path}" resp = await self._http.post(url, json=body, headers=headers) @@ -999,7 +1000,7 @@ async def _call_indexer(info: IndexerInfo) -> Dict[str, Any]: if self.is_authenticated(): headers = sign_request( self._did, self._private_key, self._public_key_id, - "POST", path, + "POST", path, self._algorithm, ) endpoint = info.effective_query_endpoint().rstrip("/") url = f"{endpoint}{path}" @@ -1091,7 +1092,8 @@ def set_identity( self, did: str, private_key_hex: str, - public_key_id: str + public_key_id: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> None: """Set identity for per-request authentication. @@ -1102,10 +1104,17 @@ def set_identity( did: DID to authenticate as private_key_hex: Hex-encoded private key public_key_id: Public key ID from DID document + algorithm: Signature algorithm of this identity ("Ed25519" or + "secp256k1"). Willow DIDs are self-certifying and no longer + encode the algorithm in the string, so it cannot be inferred + from ``did`` — pass "secp256k1" for Ethereum/wallet identities + so per-request auth is signed with the right algorithm. + Defaults to "Ed25519" (the SDK's default identity type). """ self._did = did self._private_key = private_key_hex self._public_key_id = public_key_id + self._algorithm = algorithm def is_authenticated(self) -> bool: """Check if client has identity set for per-request signing.""" @@ -1116,6 +1125,7 @@ def clear_identity(self) -> None: self._did = None self._private_key = None self._public_key_id = None + self._algorithm = "Ed25519" def register_computed_fields( self, @@ -1265,7 +1275,8 @@ async def _request( headers = {} if authenticated and self.is_authenticated(): headers = sign_request( - self._did, self._private_key, self._public_key_id, method, path + self._did, self._private_key, self._public_key_id, method, path, + self._algorithm, ) try: diff --git a/src/willow/consensus/client.py b/src/willow/consensus/client.py index 0d66fcb..13b74bf 100644 --- a/src/willow/consensus/client.py +++ b/src/willow/consensus/client.py @@ -11,7 +11,7 @@ from typing import Optional, Dict, Any, Tuple import logging -from ..auth import sign_message, detect_algorithm_from_did +from ..auth import sign_message, SignatureAlgorithm from .types import ( RegisterDidTx, RegisterSubgroveTx, TransferTx, DataStoreTx, StoreFileManifestTx, DeleteFileManifestTx, DeregisterSubgroveTx, @@ -61,7 +61,8 @@ async def register_did( self, did_document: Dict[str, Any], private_key: str, - public_key_id: str + public_key_id: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: """ Register a DID on the blockchain. @@ -90,7 +91,7 @@ async def register_did( ) # Sign and broadcast - return await self._sign_and_broadcast("RegisterDid", tx, private_key) + return await self._sign_and_broadcast("RegisterDid", tx, private_key, algorithm) async def register_subgrove( self, @@ -99,7 +100,8 @@ async def register_subgrove( owner_did: str, private_key: str, public_key_id: str, - mode: Optional[Dict[str, Any]] = None + mode: Optional[Dict[str, Any]] = None, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: """ Register a subgrove (dataset) on the blockchain. @@ -127,7 +129,7 @@ async def register_subgrove( nonce=await self._get_next_nonce(owner_did) ) - return await self._sign_and_broadcast("RegisterSubgrove", tx, private_key) + return await self._sign_and_broadcast("RegisterSubgrove", tx, private_key, algorithm) async def transfer( self, @@ -136,7 +138,8 @@ async def transfer( amount: int, private_key: str, public_key_id: str, - memo: Optional[str] = None + memo: Optional[str] = None, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: """ Transfer tokens between DIDs. @@ -162,7 +165,7 @@ async def transfer( nonce=await self._get_next_nonce(from_did) ) - return await self._sign_and_broadcast("Transfer", tx, private_key) + return await self._sign_and_broadcast("Transfer", tx, private_key, algorithm) async def store_data( self, @@ -171,7 +174,8 @@ async def store_data( data: Dict[str, Any], owner_did: str, private_key: str, - public_key_id: str + public_key_id: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: """ Store data on the blockchain. @@ -197,7 +201,7 @@ async def store_data( nonce=await self._get_next_nonce(owner_did) ) - return await self._sign_and_broadcast("DataStore", tx, private_key) + return await self._sign_and_broadcast("DataStore", tx, private_key, algorithm) async def store_file_manifest( self, @@ -213,6 +217,7 @@ async def store_file_manifest( owner_did: str, private_key: str, public_key_id: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: """ Store a file manifest on the blockchain. @@ -250,7 +255,7 @@ async def store_file_manifest( nonce=await self._get_next_nonce(owner_did), ) - return await self._sign_and_broadcast("StoreFileManifest", tx, private_key) + return await self._sign_and_broadcast("StoreFileManifest", tx, private_key, algorithm) async def delete_file_manifest( self, @@ -259,6 +264,7 @@ async def delete_file_manifest( owner_did: str, private_key: str, public_key_id: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: """ Delete a file manifest from the blockchain. @@ -282,7 +288,7 @@ async def delete_file_manifest( nonce=await self._get_next_nonce(owner_did), ) - return await self._sign_and_broadcast("DeleteFileManifest", tx, private_key) + return await self._sign_and_broadcast("DeleteFileManifest", tx, private_key, algorithm) async def deregister_subgrove( self, @@ -290,6 +296,7 @@ async def deregister_subgrove( owner_did: str, private_key: str, public_key_id: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: """ Deregister (delete) a subgrove. Remaining funding is refunded to the owner. @@ -311,7 +318,7 @@ async def deregister_subgrove( nonce=await self._get_next_nonce(owner_did), ) - return await self._sign_and_broadcast("DeregisterSubgrove", tx, private_key) + return await self._sign_and_broadcast("DeregisterSubgrove", tx, private_key, algorithm) async def get_transaction_status(self, tx_hash: str) -> TransactionStatus: """ @@ -393,14 +400,19 @@ async def _sign_and_broadcast( self, tx_type: str, transaction: Transaction, - private_key: str + private_key: str, + algorithm: SignatureAlgorithm = "Ed25519", ) -> BroadcastResult: - """Sign a transaction and broadcast it.""" + """Sign a transaction and broadcast it. + + ``algorithm`` selects the signing algorithm. It must be supplied by the + caller because the self-certifying DID no longer encodes it; it cannot + be recovered from the transaction's ``owner_did``. + """ # Create canonical message for signing sign_message_text = create_sign_message(tx_type, transaction) - - # Detect signature algorithm and sign - algorithm = detect_algorithm_from_did(getattr(transaction, 'owner_did', '')) + + # Sign with the identity's algorithm signature_hex = sign_message(sign_message_text, private_key, algorithm) # Update transaction with signature diff --git a/tests/test_auth.py b/tests/test_auth.py index b0941a9..7652058 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -8,6 +8,7 @@ detect_algorithm_from_did, generate_did, sign_challenge, + sign_request, verify_signature, ) @@ -247,6 +248,90 @@ def test_detect_default(self): assert detect_algorithm_from_did("did:willow:unknown:abc123") == "Ed25519" assert detect_algorithm_from_did("did:willow:test:abc123") == "Ed25519" + def test_self_certifying_did_is_not_detectable(self): + """Self-certifying DIDs do not encode the algorithm. + + This documents *why* the algorithm can no longer be parsed from the + DID: a real secp256k1 self-certifying id is (mis)detected as Ed25519. + Callers must therefore pass the algorithm explicitly to ``sign_request`` + rather than relying on this helper. + """ + secp_did = generate_did("secp256k1")["did"] + assert secp_did.startswith("did:willow:z") + # The truth is secp256k1, but the string carries no algorithm marker. + assert detect_algorithm_from_did(secp_did) == "Ed25519" + + +class TestSignRequest: + """Test per-request auth signing selects the correct algorithm. + + Willow DIDs are self-certifying (``did:willow:z...``) and no longer encode + the key algorithm, so ``sign_request`` takes the algorithm explicitly. + """ + + @staticmethod + def _message_for(headers, method, path): + return f"{method}:{path}:{headers['X-Timestamp']}" + + def test_secp256k1_identity_signs_with_secp256k1(self): + """A secp256k1 identity must produce a signature that verifies as + secp256k1 (the regression the self-certifying migration introduced).""" + info = generate_did("secp256k1") + method, path = "POST", "/data/my-subgrove/my-key" + + headers = sign_request( + info["did"], + info["private_key"], + info["public_key_id"], + method, + path, + "secp256k1", + ) + message = self._message_for(headers, method, path) + + # Verifies under secp256k1 (the true algorithm)... + assert verify_signature( + message, headers["X-Signature"], info["public_key"], "secp256k1" + ) + # ...and would NOT have verified had it defaulted to Ed25519. + assert not verify_signature( + message, headers["X-Signature"], info["public_key"], "Ed25519" + ) + assert headers["X-DID"] == info["did"] + assert headers["X-Public-Key-ID"] == info["public_key_id"] + + def test_ed25519_identity_defaults_correctly(self): + """With no explicit algorithm, Ed25519 (the SDK default) is used.""" + info = generate_did("Ed25519") + method, path = "GET", "/data/my-subgrove/my-key" + + headers = sign_request( + info["did"], info["private_key"], info["public_key_id"], method, path + ) + message = self._message_for(headers, method, path) + + assert verify_signature( + message, headers["X-Signature"], info["public_key"], "Ed25519" + ) + + def test_explicit_ed25519_matches_default(self): + """Passing ``"Ed25519"`` explicitly is equivalent to the default.""" + info = generate_did("Ed25519") + method, path = "GET", "/x" + + headers = sign_request( + info["did"], + info["private_key"], + info["public_key_id"], + method, + path, + "Ed25519", + ) + message = self._message_for(headers, method, path) + assert verify_signature( + message, headers["X-Signature"], info["public_key"], "Ed25519" + ) + class TestIntegration: """Integration tests for auth flow.""" diff --git a/tests/test_client.py b/tests/test_client.py index c8076e6..6fc8edb 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -93,6 +93,63 @@ async def test_set_identity(self, client, did_info): assert client._did == did_info["did"] assert client._private_key == did_info["private_key"] assert client._public_key_id == did_info["public_key_id"] + # Algorithm defaults to Ed25519 when not supplied. + assert client._algorithm == "Ed25519" + + @pytest.mark.asyncio + async def test_set_identity_secp256k1_algorithm(self, client): + """set_identity stores an explicit secp256k1 algorithm.""" + info = generate_did("secp256k1") + client.set_identity( + info["did"], info["private_key"], info["public_key_id"], "secp256k1" + ) + assert client._algorithm == "secp256k1" + # clear_identity resets the algorithm back to the default. + client.clear_identity() + assert client._algorithm == "Ed25519" + + @pytest.mark.asyncio + async def test_secp256k1_identity_signs_requests_with_secp256k1(self): + """A secp256k1 identity must sign per-request auth with secp256k1. + + Regression guard for the self-certifying DID migration: the algorithm + can no longer be parsed from the ``did:willow:z...`` string, so it is + threaded through ``set_identity`` into ``sign_request``. Before the fix + the client defaulted to Ed25519 and secp256k1 auth silently failed. + """ + from willow.auth import verify_signature + + info = generate_did("secp256k1") + client = WillowClient("http://localhost:3031") + client.set_identity( + info["did"], info["private_key"], info["public_key_id"], "secp256k1" + ) + + captured = {} + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"success": True} + + async def fake_request(method, url, json=None, headers=None): + captured["headers"] = headers + return mock_response + + client._http.request = AsyncMock(side_effect=fake_request) + + await client.data.store("my-subgrove", {"k": "v"}) + + headers = captured["headers"] + assert headers["X-DID"] == info["did"] + message = f"POST:/data/my-subgrove:{headers['X-Timestamp']}" + # The captured signature verifies under secp256k1 (the true algorithm)... + assert verify_signature( + message, headers["X-Signature"], info["public_key"], "secp256k1" + ) + # ...and would NOT verify had the client defaulted to Ed25519. + assert not verify_signature( + message, headers["X-Signature"], info["public_key"], "Ed25519" + ) + await client.close() @pytest.mark.asyncio async def test_is_authenticated(self, client):