Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make signed data a CWT standard claim set hex encoded CBOR instead of current format #7

Open
pa-long opened this issue Feb 14, 2023 · 1 comment

Comments

@pa-long
Copy link

pa-long commented Feb 14, 2023

I opened a CIP-0030 issue, which is related to this repo.

In order to prevent attacks based on false claims, I think it would be very useful to add to CIP-0030 signData function implementation instructions to verify, in case of a payload corresponding to a CWT claim set, a verification of said claims directly on the wallet side.

It would make sense to edit the current implementation of the claim set to match CWT standard.

It would just mean for this repo changing buildMessage function from sign.js and verify function from verify.js function to make the signer function sign an hex encoded CBOR of the claim set such as the following example given in the CWT standard:

a70175636f61703a2f2f61732e6578616d706c652e636f6d02656572696b77037818636f61703a2f2f6c696768742e6578616d706c652e636f6d041a5612aeb0051a5610d9f0061a5610d9f007420b71

Which corresponds to the claim set:

{
  / iss / 1: "coap://as.example.com",
  / sub / 2: "erikw",
  / aud / 3: "coap://light.example.com",
  / exp / 4: 1444064944,
  / nbf / 5: 1443944944,
  / iat / 6: 1443944944,
  / cti / 7: h'0b71'
}

sign function should be changed accordingly to include issuer, subject, audience, expires-at, not-before, issued-at, token-id arguments as optional parameters. Default claim set could correspond to the following:

{
  / iss / 1: document.location.host,
  / iat / 6: Math.floor(Date.now()/1000),
  / cti / 7: random uuid as bytes
}

If we want to include an additional custom payload argument to the sign function, we could add a custom claim, with id 8, containing a custom body. It is yet to discuss if it should be a hex encoded CBOR of the input body object, or JSON.stringify(body) hex encoded or anything else.

This change could prevent phishing attacks. If I try to connect on jpgs.store and attempt to sign a message that a malicious attacker generated on jpg.store, my wallet would warn me that I am signing a CWT payload with issuer claim different from the current host. It would prevent replay attacks as well.

I am currently writing a PR to CIP-0030 signData implementation to recommend detecting signature of CWT claims and their verification. When it is done, i will propose a PR on open source wallets (nami, yoroi) for the implementation of those recommendations.

What do you think about this change?

By the way, thank you @pyropy for working on this very useful repo, and @gavinharris-dev for adding the signature verification. It helped a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@pa-long and others