fix: address feedback

Signed-off-by: Daniel Bluhm <[email protected]>

Author: dbluhm

didcomm_messaging/crypto/backend/askar.py

@@ -116,11 +116,6 @@ def multikey(self) -> str:
         """Get the key in multibase format."""
         return self._multikey
 
-    @property
-    def key_bytes(self) -> bytes:
-        """Get the bytes of the key."""
-        return self.key.get_public_bytes()
-
 
 class AskarSecretKey(SecretKey):
     """Secret key implementation for Askar."""

didcomm_messaging/crypto/backend/authlib.py

@@ -56,18 +56,6 @@ def multikey(self) -> str:
         """Return the key in multikey format."""
         return self._multikey
 
-    @property
-    def key_bytes(self) -> bytes:
-        """Get the bytes of the key."""
-        jwk = self.key.as_dict(is_private=False)
-        codec = self.kty_crv_to_codec.get((jwk["kty"], jwk.get("crv")))
-
-        if not codec:
-            raise ValueError("Unsupported key type")
-
-        key_bytes = b64url.decode(jwk["x"])
-        return key_bytes
-
     @classmethod
     def key_to_multikey(cls, key: AsymmetricKey) -> str:
         """Convert an Authlib key to a multikey."""

didcomm_messaging/crypto/base.py

@@ -69,11 +69,6 @@ def kid(self) -> str:
     def multikey(self) -> str:
         """Get the key in multikey format."""
 
-    @property
-    @abstractmethod
-    def key_bytes(self) -> bytes:
-        """Get the bytes of the key."""
-
 
 class SecretKey(ABC):
     """Secret Key Type."""

didcomm_messaging/messaging.py

@@ -41,7 +41,7 @@ class UnpackResult:
     """Result of unpacking a message."""
 
     unpacked: bytes
-    encrytped: bool
+    encrypted: bool
     authenticated: bool
     recipient_kid: str
     sender_kid: Optional[str] = None
@@ -139,7 +139,7 @@ async def unpack(
         )
         return UnpackResult(
             unpacked,
-            encrytped=bool(metadata.method),
+            encrypted=bool(metadata.method),
             authenticated=bool(metadata.sender_kid),
             recipient_kid=metadata.recip_key.kid,
             sender_kid=metadata.sender_kid,

didcomm_messaging/quickstart.py

@@ -258,7 +258,7 @@ async def setup_relay(
     await secrets.add_secret(AskarSecretKey(verkey, f"{new_did}#key-1"))
     await secrets.add_secret(AskarSecretKey(xkey, f"{new_did}#key-2"))
 
-    # V1 formats
+    # Legacy formats
     # verkey
     await secrets.add_secret(AskarSecretKey(verkey, doc.authentication[0]))
     # xkey

didcomm_messaging/resolver/peer.py

@@ -32,9 +32,7 @@ class Peer4(DIDResolver):
 
     async def is_resolvable(self, did: str) -> bool:
         """Check to see if a DID is resolvable."""
-        return bool(peer_4_pattern_short.match(did)) or bool(
-            peer_4_pattern_long.match(did)
-        )
+        return bool(peer_4_pattern_short.match(did) or peer_4_pattern_long.match(did))
 
     async def resolve(self, did: str) -> dict:
         """Resolve a did:peer:4 DID."""

didcomm_messaging/v1/crypto/askar.py

@@ -4,12 +4,14 @@
 from typing import Optional, Sequence, Tuple, cast
 
 from base58 import b58decode
+import base58
 from pydid import VerificationMethod
 
 from didcomm_messaging.crypto.jwe import JweBuilder, JweEnvelope, JweRecipient
 from .base import (
     V1CryptoService,
-    V1UnpackResult,
+    V1CryptoServiceError,
+    V1CryptoUnpackResult,
     RecipData,
 )
 
@@ -24,13 +26,19 @@
 class AskarV1CryptoService(V1CryptoService[AskarKey, AskarSecretKey]):
     """V1 crypto service implementation for askar."""
 
-    def kid_to_public_key(self, kid: str) -> AskarKey:
+    def v1_kid_to_public_key(self, kid: str) -> AskarKey:
         """Get a public key from a kid.
 
         In DIDComm v1, kids are the base58 encoded keys.
         """
         return AskarKey(Key.from_public_bytes(KeyAlg.ED25519, b58decode(kid)), kid)
 
+    def public_key_to_v1_kid(self, key: AskarKey) -> str:
+        """Convert a public key into a v1 kid representation."""
+        if key.key.algorithm != KeyAlg.ED25519:
+            raise V1CryptoServiceError()
+        return base58.b58encode(key.key.get_public_bytes()).decode()
+
     @classmethod
     def verification_method_to_public_key(cls, vm: VerificationMethod) -> AskarKey:
         """Convert a verification method to a public key."""
@@ -72,8 +80,6 @@ async def pack_message(
                     )
                 )
             else:
-                enc_sender = None
-                nonce = None
                 enc_cek = crypto_box.crypto_box_seal(target_xk, cek_b)
                 builder.add_recipient(
                     JweRecipient(encrypted_key=enc_cek, header={"kid": target_vk.kid})
@@ -97,7 +103,7 @@ async def unpack_message(
         wrapper: JweEnvelope,
         recip_key: AskarSecretKey,
         recip_data: RecipData,
-    ) -> V1UnpackResult:
+    ) -> V1CryptoUnpackResult:
         """Decode a message using the DIDComm v1 'unpack' algorithm."""
         payload_key, sender_vk = self._extract_payload_key(recip_key.key, recip_data)
 
@@ -108,7 +114,7 @@ async def unpack_message(
             tag=wrapper.tag,
             aad=wrapper.protected_b64,
         )
-        return V1UnpackResult(message, recip_key.kid, sender_vk)
+        return V1CryptoUnpackResult(message, recip_key.kid, sender_vk)
 
     def _extract_payload_key(
         self, recip_key: Key, recip_data: RecipData

didcomm_messaging/v1/crypto/base.py

@@ -18,26 +18,34 @@ class RecipData(NamedTuple):
     enc_cek: bytes
 
 
-class V1UnpackResult(NamedTuple):
+class V1CryptoUnpackResult(NamedTuple):
     """Result of unpacking."""
 
-    message: bytes
+    unpacked: bytes
     recip: str
     sender: Optional[str]
 
 
+class V1CryptoServiceError(Exception):
+    """Raised on errors in crypto service."""
+
+
 class V1CryptoService(ABC, Generic[P, S]):
     """CryptoService interface for DIDComm v1."""
 
     b64url = Base64UrlEncoder()
 
     @abstractmethod
-    def kid_to_public_key(self, kid: str) -> P:
+    def v1_kid_to_public_key(self, kid: str) -> P:
         """Get a public key from a kid.
 
         In DIDComm v1, kids are the base58 encoded keys.
         """
 
+    @abstractmethod
+    def public_key_to_v1_kid(self, key: P) -> str:
+        """Return the DIDComm v1 kid representation for a key."""
+
     @classmethod
     @abstractmethod
     def verification_method_to_public_key(cls, vm: VerificationMethod) -> P:
@@ -52,5 +60,5 @@ async def pack_message(
     @abstractmethod
     async def unpack_message(
         self, wrapper: JweEnvelope, recip_key: S, recip_data: RecipData
-    ) -> V1UnpackResult:
+    ) -> V1CryptoUnpackResult:
         """Decode a message using DIDCvomm v1 'unpack' algorithm."""

didcomm_messaging/v1/crypto/nacl.py

@@ -10,7 +10,7 @@
 from didcomm_messaging.crypto.jwe import JweBuilder, JweEnvelope, JweRecipient
 from didcomm_messaging.multiformats import multibase, multicodec
 
-from .base import V1CryptoService, V1UnpackResult, RecipData
+from .base import V1CryptoService, V1CryptoUnpackResult, RecipData
 
 try:
     import nacl.bindings
@@ -60,7 +60,7 @@ def key(self) -> str:
     @property
     def kid(self) -> str:
         """Get the key ID."""
-        return self.key
+        raise NotImplementedError()
 
     @property
     def multikey(self) -> str:
@@ -69,22 +69,21 @@ def multikey(self) -> str:
             multicodec.wrap("ed25519-pub", base58.b58decode(self.key)), "base58btc"
         )
 
-    @property
-    def key_bytes(self) -> bytes:
-        """Get the bytes of the key."""
-        return self.value
-
 
 class NaclV1CryptoService(V1CryptoService[EdPublicKey, KeyPair]):
     """V1 crypto service using pynacl."""
 
-    def kid_to_public_key(self, kid: str):
+    def v1_kid_to_public_key(self, kid: str):
         """Get a public key from a kid.
 
         In DIDComm v1, kids are the base58 encoded keys.
         """
         return EdPublicKey(base58.b58decode(kid))
 
+    def public_key_to_v1_kid(self, key: EdPublicKey) -> str:
+        """Convert a public key into a v1 kid representation."""
+        return base58.b58encode(key.value).decode()
+
     @classmethod
     def verification_method_to_public_key(cls, vm: VerificationMethod) -> EdPublicKey:
         """Convert a verification method to a public key."""
@@ -120,19 +119,20 @@ async def pack_message(
                         encrypted_key=enc_cek,
                         header=OrderedDict(
                             [
-                                ("kid", target_vk.kid),
+                                ("kid", self.public_key_to_v1_kid(target_vk)),
                                 ("sender", self.b64url.encode(enc_sender)),
                                 ("iv", self.b64url.encode(nonce)),
                             ]
                         ),
                     )
                 )
             else:
-                enc_sender = None
-                nonce = None
                 enc_cek = nacl.bindings.crypto_box_seal(cek, target_xk)
                 builder.add_recipient(
-                    JweRecipient(encrypted_key=enc_cek, header={"kid": target_vk.kid})
+                    JweRecipient(
+                        encrypted_key=enc_cek,
+                        header={"kid": self.public_key_to_v1_kid(target_vk)},
+                    )
                 )
 
         builder.set_protected(
@@ -180,15 +180,15 @@ def _extract_payload_key(self, recip_key: KeyPair, recip_data: RecipData):
 
     async def unpack_message(
         self, wrapper: JweEnvelope, recip_key: KeyPair, recip_data: RecipData
-    ) -> V1UnpackResult:
+    ) -> V1CryptoUnpackResult:
         """Decode a message using DIDCvomm v1 'unpack' algorithm."""
         cek, sender_vk = self._extract_payload_key(recip_key, recip_data)
 
         payload_bin = wrapper.ciphertext + wrapper.tag
         message = nacl.bindings.crypto_aead_chacha20poly1305_ietf_decrypt(
             payload_bin, wrapper.protected_b64, wrapper.iv, cek
         )
-        return V1UnpackResult(message, recip_key.kid, sender_vk)
+        return V1CryptoUnpackResult(message, recip_key.kid, sender_vk)
 
 
 class InMemSecretsManager(SecretsManager[KeyPair]):
@@ -206,7 +206,7 @@ def _create_keypair(self, seed: Optional[bytes] = None) -> Tuple[bytes, bytes]:
         """Create a keypair."""
         if seed:
             if not isinstance(seed, bytes):
-                raise ValueError("Seed value is not a string or bytes")
+                raise ValueError("Seed value is not bytes")
             if len(seed) != 32:
                 raise ValueError("Seed value must be 32 bytes in length")
         else:

didcomm_messaging/v1/messaging.py

@@ -4,15 +4,14 @@
 import json
 from typing import Generic, Optional, Sequence, Union
 
-import base58
 from pydantic import AnyUrl
-from pydid import VerificationMethod
+from pydid import DIDDocument, VerificationMethod
 from pydid.service import DIDCommV1Service
 
 from didcomm_messaging.crypto import P, S, SecretsManager
+from didcomm_messaging.resolver import DIDResolver
 from didcomm_messaging.v1.crypto.base import V1CryptoService
 from didcomm_messaging.v1.packaging import V1PackagingService
-from didcomm_messaging.resolver import DIDResolver
 
 
 class V1DIDCommMessagingError(Exception):
@@ -32,7 +31,7 @@ class V1UnpackResult:
     """Result of unpacking a message."""
 
     unpacked: bytes
-    encrytped: bool
+    encrypted: bool
     authenticated: bool
     recipient_kid: str
     sender_kid: Optional[str] = None
@@ -58,6 +57,14 @@ class Target:
 class V1DIDCommMessagingService(Generic[P, S]):
     """Main entrypoint for DIDComm Messaging."""
 
+    def vm_to_v1_kid(self, crypto: V1CryptoService, doc: DIDDocument, ref: str) -> str:
+        """Convert a verification method ref to a DIDComm v1 kid."""
+        return crypto.public_key_to_v1_kid(
+            crypto.verification_method_to_public_key(
+                doc.dereference_as(VerificationMethod, ref)
+            )
+        )
+
     async def did_to_target(
         self, crypto: V1CryptoService[P, S], resolver: DIDResolver, did: str
     ) -> Target:
@@ -73,19 +80,10 @@ async def did_to_target(
         target = services[0]
 
         recipient_keys = [
-            base58.b58encode(
-                crypto.verification_method_to_public_key(
-                    doc.dereference_as(VerificationMethod, recip)
-                ).key_bytes
-            ).decode()
-            for recip in target.recipient_keys
+            self.vm_to_v1_kid(crypto, doc, recip) for recip in target.recipient_keys
         ]
         routing_keys = [
-            base58.b58encode(
-                crypto.verification_method_to_public_key(
-                    doc.dereference_as(VerificationMethod, routing_key)
-                ).key_bytes
-            ).decode()
+            self.vm_to_v1_kid(crypto, doc, routing_key)
             for routing_key in target.routing_keys
         ]
         endpoint = target.service_endpoint
@@ -113,12 +111,7 @@ async def from_did_to_kid(
         target = services[0]
 
         recipient_keys = [
-            base58.b58encode(
-                crypto.verification_method_to_public_key(
-                    doc.dereference_as(VerificationMethod, recip)
-                ).key_bytes
-            ).decode()
-            for recip in target.recipient_keys
+            self.vm_to_v1_kid(crypto, doc, recip) for recip in target.recipient_keys
         ]
         return recipient_keys[0]
 
@@ -211,7 +204,7 @@ async def unpack(
         unpacked, recip, sender = await packaging.unpack(crypto, secrets, encoded_message)
         return V1UnpackResult(
             unpacked,
-            encrytped=bool(recip),
+            encrypted=bool(recip),
             authenticated=bool(sender),
             recipient_kid=recip,
             sender_kid=sender,