WalletConnect / WalletConnectKotlinV2

WalletConnect Kotlin SDK v2
Apache License 2.0
251 stars 83 forks source link

Responding to Auth request should not require exposing Private Key #1149

Closed muratogat closed 1 year ago

muratogat commented 1 year ago

In the Setup Auth Requests Handling section, it is suggested that the private key should be passed to the CacaoSigner to be able to successfully sign the Auth request and respond. I could not find a version of the same functionality where the signing can be taken care of by the host wallet itself.

As a picky wallet developer, I am keeping the parts of the wallet that has access to the private key to a minimum, and I'm extremely hesitant to pass around private key data to any third party SDK. Moreover, when working with hardware wallets that don't expose the private key, this may be downright impossible.

Please let me know how an Auth request could be approved without exposing the private key. If no such mechanism currently exists, please provide an additional constructor for AuthRequestResponse that doesn't take a Cacao.Signature as parameter, but a byte array, hex string or similar instead, where the app itself can take care of the signing and just provide the final signature.

Elyniss commented 1 year ago

Thanks for raising this issue @muratogat! I agree that trusting us with the private key is a stretch. It is possible right now, however we build an util called CacaoSigner for easier developement.

Currently it's possible by instead of:

val request: Wallet.Event.AuthRequest = // Request from onAuthRequest
val signature: Wallet.Model.Cacao.Signature = CacaoSigner.sign(
    request.message, // Message to be signed
    PRIVATE_KEY, // Private key used to signing a message
    SignatureType.EIP191 // or EIP1271
)

val issuer = //Check following link for more reference: https://github.com/w3c-ccg/did-pkh/blob/main/did-pkh-method-draft.md

Web3Wallet.respondSessionRequest(Wallet.Params.AuthRequestResponse(request.id, signature, issuer)) { error ->
    Log.e("Wallet respond approve", error.throwable.stackTraceToString())
}

We would have something like:


import com.walletconnect.android.cacao.signature.SignatureType
import com.walletconnect.android.internal.common.signing.signature.Signature
import com.walletconnect.android.internal.common.signing.signature.toCacaoSignature

val request: Wallet.Event.AuthRequest = // Request from onAuthRequest
val signature: String = // Here developers provide signed message from request.message
val cacaoSignature = Wallet.Model.Cacao.Signature(SignatureType.EIP191.header, Signature.fromString(signature).toCacaoSignature())

val issuer = //Check following link for more reference: https://github.com/w3c-ccg/did-pkh/blob/main/did-pkh-method-draft.md

Web3Wallet.respondSessionRequest(Wallet.Params.AuthRequestResponse(request.id, cacaoSignature, issuer)) { error ->
    Log.e("Wallet respond approve", error.throwable.stackTraceToString())
}

Do you think we should make it easier for devs to use latter approach?

muratogat commented 1 year ago

It works. Thank you very much. Signature.fromString(signature).toCacaoSignature() was what I was looking for and it's enough.

Since you also asked for feedback: I think using the method is straightforward. Perhaps you'd also want to mention this version in the documentation.

Thanks again.

nguyennh-0786 commented 1 year ago

@Elyniss Web3Wallet.respondSessionRequest() require SessionRequestResponse not AuthRequestResponse. https://docs.walletconnect.com/web3wallet/wallet-usage?platform=android#setup-auth-requests-handling document is not correct?