cardano-foundation / CIPs

Cardano Improvement Proposals (CIPs)
https://cips.cardano.org/
Creative Commons Attribution 4.0 International
471 stars 318 forks source link

CIP-0030 | CWT as signData payload #448

Open pa-long opened 1 year ago

pa-long commented 1 year ago

Currently, any hybrid dApp can authenticate its users using CIP-0030 signData function. For instance, an implementation of an equivalent of JSON Web Token (JWT) is available here.

The problem relying in those methods is their vulnerability to a phishing attack: a malicious website could forge a JWT made for authentication to another website, make the user sign it, and act as a man in the middle. Example: User fetches malicious jpgs.store, signs a message containing "Connect to jpg.store". Signed message can the be used to access user's account on jpg.store Hybrid dApp (which is critical in this case as you can manage NFT collections and their royalty address from their interface).

A solution could be to implement JW3T signature directly in the wallet API.

import Web3Token from 'web3-cardano-token/dist/browser';

api.signJW3T = async function (
  address: string,
  body: object, 
  expires_in: string
): string {
  /*
    Forges a Web3Token using the 'web3-cardano-token' standard 
    but with addition of domain of the current website:
  */
  return await Web3Token.sign(
    {
       signer: msg => api.signData(address, toHex(msg)),
       expire_in: expires_in,
       body: body,
       domain: 'jpg.store' // Wallet API inserts here the current domain
    }
  );
}

walletApi.signData also need to be display a warning sign when it is called to sign a message with JW3T format but not the good domain.

To verify the token from the backend:

const Web3Token = require('web3-cardano-token/dist/node');

const token = req.headers['Authorization']
const { address, body, domain } = await Web3Token.verify(token);

// Check if domain matches

This standard is very straight forward to implement, as web3-cardano-token package provides it with just the addition of the domain key (which could also simply go inside the body object).

rphair commented 1 year ago

@makerare if you're thinking of writing a CIP for this, it sounds like it would be a good idea to propose. We have a new convention under review for extensions to CIP-0030: https://github.com/cardano-foundation/CIPs/pull/446

If you're suggesting this be added to CIP-0030 under all circumstances then say so here & I'll tag some people involved in the history of CIP-0030 to help discuss that. (You are welcome to do this as well.)

pa-long commented 1 year ago

Yes I would appreciate this as I don't know who to reach, so they can arbitrate as they would know what makes the most sense, thank you. I'd be glad to write the proposal !

rphair commented 1 year ago

cc (may be interested in issue discussion of CIP update or new CIP for this): @KtorZ @SebastienGllmt @alessandrokonrad @rooooooooob @ehanoc @refi93 @MarcelKlammer @Scitz0

pa-long commented 1 year ago

After some thinking, i think what makes the most sense is not to create a new CIP-0030 function, but to specify in the CIP that wallets should check if signData receives a payload corresponding to a CBOR Web Token (CWT).

I'll fork web3-cardano-token package repo, which needs two fixes already (CIP-0030 signData conformity and verification of signing address, both of those I have a patch for locally), and change signed payload to use CWT standard instead of a custom json.

That way, wallets could also possibly have a specific interface for displaying data signature of CWT, but most importantly protect their users by verifying the validity of the claims:

I'll propose an update of CIP-0030 signData section where CWT is referenced.